Skip to content

Enable more clippy lints - #24566

Open
emilk wants to merge 9 commits into
apache:mainfrom
emilk:emilk/enable-more-clippy-lints-2
Open

Enable more clippy lints#24566
emilk wants to merge 9 commits into
apache:mainfrom
emilk:emilk/enable-more-clippy-lints-2

Conversation

@emilk

@emilk emilk commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Continuing #24466: turn on more clippy::pedantic lints (opt-out list in Cargo.toml).

What changes are included in this PR?

One commit per lint, each removing its "allow" line from Cargo.toml
and fixing every site. Review one commit at a time!

Let me know if you disagree with any and I'll revert it

Lint Sites Fix
no_effect_underscore_binding 1 removed a dead let _expected = … binding
case_sensitive_file_extension_comparisons 1 Path::extension() instead of str::ends_with(".csv")
cast_ptr_alignment 5 #[expect] on FFI tests that downcast to a concrete, aligned type
doc_link_with_quotes 25 quoted doc "links" turned into real intra-doc links or backticked code
ref_option 36 &Option<T> params/returns changed to Option<&T>
needless_continue 37 dropped continues that already ended the loop iteration
cloned_instead_of_copied 38 copied() instead of cloned() for Copy types
unnecessary_semicolon 185 dropped semicolons after tail match/if
unnecessary_trailing_comma 49 dropped trailing commas after single-item parens/brackets

The ref_option commit also changes the signature of the public apply_masking fn in datafusion-substrait (&Option<MaskExpression> -> Option<&MaskExpression>), since it forwards to an internal fn that was flagged.

Are these changes tested?

cargo clippy --workspace --all-targets --all-features reports no
warnings. The changes are mechanical and behavior-preserving, so no new
tests.

Are there any user-facing changes?

The ref_option commit changes the public apply_masking signature in
datafusion-substrait, see above.

emilk and others added 9 commits August 21, 2026 16:42
Remove a dead `let _expected = …` binding left over from a
removed assertion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Compare the file extension with `Path::extension()` instead of
`str::ends_with(".csv")`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All five hits are FFI local-bypass tests that downcast a trait
object to its concrete type with a pointer cast. The casts are
aligned, so mark them with targeted `#[expect]` attributes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Turn quoted doc links into real intra-doc links where a target
exists, and wrap array examples and SQL snippets in backticks or
text code fences.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Change internal fn parameters and return types from `&Option<T>`
to `Option<&T>`, updating callers to pass `.as_ref()`.

Also changes the public `apply_masking` in datafusion-substrait,
which forwards from a flagged internal fn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop `continue` expressions that end a loop iteration anyway,
flattening the surrounding `match`/`if` where that reads better.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Use `Iterator::copied`/`Option::copied` instead of `cloned` for
`Copy` types. Applied with `cargo clippy --fix`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop semicolons after `match`/`if` expressions in tail position.
Applied with `cargo clippy --fix`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop trailing commas after single-item non-tuple parens/brackets.
Applied with `cargo clippy --fix`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) substrait Changes to the substrait crate catalog Related to the catalog crate common Related to common crate execution Related to the execution crate proto Related to proto crate functions Changes to functions implementation datasource Changes to the datasource crate ffi Changes to the ffi crate physical-plan Changes to the physical-plan crate spark labels Aug 21, 2026
@emilk
emilk marked this pull request as ready for review August 21, 2026 17:41
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.55744% with 82 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.37%. Comparing base (5f0ba13) to head (3965bd2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tafusion/functions-aggregate-common/src/min_max.rs 91.37% 10 Missing ⚠️
datafusion/common/src/format.rs 27.27% 0 Missing and 8 partials ⚠️
datafusion/functions/src/regex/mod.rs 30.00% 7 Missing ⚠️
...gate-common/src/aggregate/count_distinct/native.rs 14.28% 6 Missing ⚠️
datafusion/functions/src/datetime/date_trunc.rs 88.88% 5 Missing and 1 partial ⚠️
...tafusion/common/src/file_options/parquet_writer.rs 37.50% 0 Missing and 5 partials ⚠️
datafusion/expr/src/logical_plan/display.rs 0.00% 4 Missing ⚠️
datafusion/sql/src/parser.rs 73.33% 1 Missing and 3 partials ⚠️
...egate-common/src/aggregate/count_distinct/bytes.rs 0.00% 0 Missing and 2 partials ⚠️
datafusion/physical-plan/src/test/exec.rs 0.00% 2 Missing ⚠️
... and 26 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24566      +/-   ##
==========================================
+ Coverage   81.34%   81.37%   +0.02%     
==========================================
  Files        1117     1117              
  Lines      397528   398005     +477     
  Branches   397528   398005     +477     
==========================================
+ Hits       323385   323857     +472     
- Misses      55225    55227       +2     
- Partials    18918    18921       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

catalog Related to the catalog crate common Related to common crate core Core DataFusion crate datasource Changes to the datasource crate execution Related to the execution crate ffi Changes to the ffi crate functions Changes to functions implementation logical-expr Logical plan and expressions optimizer Optimizer rules physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate proto Related to proto crate spark sql SQL Planner sqllogictest SQL Logic Tests (.slt) substrait Changes to the substrait crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants