This is an automated email from the ASF dual-hosted git repository.
sdf-jkl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 8c265f8de4 Fix cargo fmt for parquet modules (#11016)
8c265f8de4 is described below
commit 8c265f8de4e44a5fdda8aad79ccf890d53c6e601
Author: Kosta Tarasov <[email protected]>
AuthorDate: Tue Sep 8 09:45:20 2026 -0400
Fix cargo fmt for parquet modules (#11016)
# Which issue does this PR close?
- Closes #6897.
# Rationale for this change
rustfmt does not discover out-of-line modules declared inside macros
(rust-lang/rustfmt#3253). The experimental module macro therefore causes
cargo fmt to skip several Parquet module trees.
This uses the same approach as delta-io/delta-kernel-rs#935.
# What changes are included in this PR?
- Replace experimental module macro declarations with explicit, mutually
exclusive feature-gated declarations while preserving visibility and
documentation behavior.
- Remove the separate Parquet formatting workaround from CI.
- Remove the workaround from the contributor documentation and use the
standard workspace formatting command.
# Are these changes tested?
- cargo fmt --all -- --check
- cargo check -p parquet
- cargo check -p parquet --all-features
- cargo test -p parquet --lib encodings::rle (15 passed)
- Verbose cargo fmt output confirms that the formerly skipped module
trees are traversed.
A full cargo test -p parquet --lib run compiled successfully and passed
1,278 tests; 93 fixture-dependent tests could not run because the local
checkout does not have the testing and parquet-testing data submodules
initialized.
# Are there any user-facing changes?
Contributors can now format all Parquet sources with the standard cargo
fmt --all command. There are no public API changes.
# AI usage
OpenAI Codex was used to investigate the rustfmt behavior, implement the
feature-gated module declarations, update CI and contributor
documentation, and run the validation commands reported above. The
resulting code and PR description were AI-assisted.
---
.github/workflows/rust.yml | 10 +--------
CONTRIBUTING.md | 7 ------
parquet/src/arrow/mod.rs | 14 ++++++++++--
parquet/src/encodings/mod.rs | 8 ++++++-
parquet/src/lib.rs | 53 ++++++++++++++++++++------------------------
5 files changed, 44 insertions(+), 48 deletions(-)
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 4bcd7da3a8..764f7974a9 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -99,16 +99,8 @@ jobs:
uses: ./.github/actions/setup-builder
- name: Setup rustfmt
run: rustup component add rustfmt
- - name: Format arrow
+ - name: Format workspace
run: cargo fmt --all -- --check
- - name: Format parquet
- # Many modules in parquet are skipped, so check parquet separately
- # https://github.com/apache/arrow-rs/issues/6179
- working-directory: parquet
- run: |
- # if this fails, run this from the parquet directory:
- # cargo fmt -p parquet -- --config skip_children=true `find . -name
"*.rs" \! -name format.rs`
- cargo fmt -p parquet -- --check --config skip_children=true `find .
-name "*.rs" \! -name format.rs`
msrv:
name: Verify MSRV (Minimum Supported Rust Version)
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index c5bad1da66..1edb8ed696 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -159,13 +159,6 @@ PR be sure to run the following and check for lint issues:
cargo +stable fmt --all -- --check
```
-Note that currently the above will not check all source files in the parquet
crate. To check all
-parquet files run the following from the top-level `arrow-rs` directory:
-
-```bash
-cargo fmt -p parquet -- --check --config skip_children=true `find ./parquet
-name "*.rs" \! -name format.rs`
-```
-
## Miri
Run tests under [`Miri`](https://github.com/rust-lang/miri) like so, assuming
diff --git a/parquet/src/arrow/mod.rs b/parquet/src/arrow/mod.rs
index ff9924ffef..8309a35e9b 100644
--- a/parquet/src/arrow/mod.rs
+++ b/parquet/src/arrow/mod.rs
@@ -179,7 +179,13 @@
//! assert_eq!(50, record_batch.num_rows());
//! ```
-experimental!(mod array_reader);
+// Keep these module declarations explicit so rustfmt discovers their source
files.
+// See the comment in the crate root.
+#[cfg(feature = "experimental")]
+#[doc(hidden)]
+pub mod array_reader;
+#[cfg(not(feature = "experimental"))]
+mod array_reader;
pub mod arrow_reader;
pub mod arrow_writer;
mod buffer;
@@ -195,7 +201,11 @@ pub mod push_decoder;
mod in_memory_row_group;
mod record_reader;
-experimental!(mod schema);
+#[cfg(feature = "experimental")]
+#[doc(hidden)]
+pub mod schema;
+#[cfg(not(feature = "experimental"))]
+mod schema;
use std::fmt::Debug;
diff --git a/parquet/src/encodings/mod.rs b/parquet/src/encodings/mod.rs
index cb23625a82..759284c54b 100644
--- a/parquet/src/encodings/mod.rs
+++ b/parquet/src/encodings/mod.rs
@@ -20,4 +20,10 @@ pub mod decoding;
pub mod encoding;
pub mod levels;
-experimental!(pub(crate) mod rle);
+// Keep this module declaration explicit so rustfmt discovers its source file.
+// See the comment in the crate root.
+#[cfg(feature = "experimental")]
+#[doc(hidden)]
+pub mod rle;
+#[cfg(not(feature = "experimental"))]
+pub(crate) mod rle;
diff --git a/parquet/src/lib.rs b/parquet/src/lib.rs
index 55788327b9..fda74d37ab 100644
--- a/parquet/src/lib.rs
+++ b/parquet/src/lib.rs
@@ -144,29 +144,6 @@
)]
#![cfg_attr(docsrs, feature(doc_cfg))]
#![warn(missing_docs)]
-/// Defines a an item with an experimental public API
-///
-/// The module will not be documented, and will only be public if the
-/// experimental feature flag is enabled
-///
-/// Experimental components have no stability guarantees
-#[cfg(feature = "experimental")]
-macro_rules! experimental {
- ($(#[$meta:meta])* $vis:vis mod $module:ident) => {
- #[doc(hidden)]
- $(#[$meta])*
- pub mod $module;
- }
-}
-
-#[cfg(not(feature = "experimental"))]
-macro_rules! experimental {
- ($(#[$meta:meta])* $vis:vis mod $module:ident) => {
- $(#[$meta])*
- $vis mod $module;
- }
-}
-
#[cfg(all(
feature = "flate2",
not(any(feature = "flate2-zlib-rs", feature = "flate2-rust_backend"))
@@ -189,19 +166,36 @@ use std::ops::Range;
#[doc(hidden)]
pub use self::encodings::{decoding, encoding};
-experimental!(#[macro_use] mod util);
+// Keep these module declarations explicit: rustfmt does not discover modules
declared by macros.
+// See https://github.com/rust-lang/rustfmt/issues/3253
+#[cfg(feature = "experimental")]
+#[doc(hidden)]
+#[macro_use]
+pub mod util;
+#[cfg(not(feature = "experimental"))]
+#[macro_use]
+mod util;
pub use util::utf8;
#[cfg(feature = "arrow")]
pub mod arrow;
-pub mod column;
-experimental!(mod compression);
-experimental!(mod encodings);
pub mod bloom_filter;
+pub mod column;
+#[cfg(feature = "experimental")]
+#[doc(hidden)]
+pub mod compression;
+#[cfg(not(feature = "experimental"))]
+mod compression;
+#[cfg(feature = "experimental")]
+#[doc(hidden)]
+pub mod encodings;
+#[cfg(not(feature = "experimental"))]
+mod encodings;
#[cfg(feature = "encryption")]
-experimental!(pub mod encryption);
+#[cfg_attr(feature = "experimental", doc(hidden))]
+pub mod encryption;
pub mod file;
pub mod record;
@@ -225,6 +219,7 @@ pub enum DecodeResult<T: Debug> {
Finished,
}
+#[cfg_attr(feature = "experimental", doc(hidden))]
+pub mod geospatial;
#[cfg(feature = "variant_experimental")]
pub mod variant;
-experimental!(pub mod geospatial);
