FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Enable more clippy lints by emilk · Pull Request #24566 · apache/datafusion · GitHub

Enable more clippy lints - #24566

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

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

Conversation

emilk commented Aug 21, 2026
edited
Loading

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.

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 marked this pull request as ready for review August 21, 2026 17:41

codecov-commenter commented Aug 21, 2026
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.83146% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.48%. Comparing base (4d3e79e) to head (107c72a).

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 25 more
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #24566    +/-   ##
========================================
  Coverage   81.48%   81.48%            
========================================
  Files        1122     1122            
  Lines      404248   404381   +133     
  Branches   404248   404381   +133     
========================================
+ Hits       329390   329511   +121     
- Misses      55547    55558    +11     
- Partials    19311    19312     +1     

☔ 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.

emilk and others added 9 commits August 24, 2026 11:51
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>
emilk force-pushed the emilk/enable-more-clippy-lints-2 branch from 3965bd2 to c0eb49d Compare August 24, 2026 09:54

Copy link
Copy Markdown
Contributor

@emilk can you fix some more? :D

emilk commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

I'd love to, but want this PR merged first!

Conflicts:
* `operator_statistics/mod.rs`: upstream replaced the provider loop that
  the `needless_continue` fix touched, so the loop is gone; took upstream.
* `topk/mod.rs`: upstream re-indexed `equal_indices` to the gathered
  sub-batch; kept that and dropped the `continue` per `needless_continue`.

New upstream code that trips the lints this branch enables:
* `joins/utils.rs`: `&Option<RecordBatch>` -> `Option<&RecordBatch>` (`ref_option`)
* `hash_utils.rs`: `cloned` -> `copied` (`cloned_instead_of_copied`)
* `statistics_order_tests.rs`: trailing comma (`unnecessary_trailing_comma`)
* `fixed_size_binary_filter.rs`, `in_list_strategy.rs`: `#[expect]` for the
  alignment-check casts (`cast_ptr_alignment`)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL