Enable more clippy lints - #24848
Open
emilk wants to merge 8 commits into
Open
Conversation
`String::new()` instead of `"".to_string()` / `"".to_owned()` / `"".into()`, which skips the copy-from-empty-slice path. All sites fixed by `cargo clippy --fix`.
github-actions
Bot
added
sql
emilk
force-pushed
the
emilk/enable-more-clippy-lints-4
branch
from
September 1, 2026 12:46
c24d083 to
c866451
Compare
`Ok(())` instead of `Ok(_)` where the payload is `()`, so the pattern stops matching silently if the type ever gains a payload.
Dropped `else` blocks after a branch that already diverges, removing one level of indentation at each site.
Nested the or-patterns, e.g. `Time32(Microsecond) | Time32(Nanosecond)` -> `Time32(Microsecond | Nanosecond)`, so the shared prefix is written once.
`if`/`else` instead of `match` on a bool. Two sites cascaded into `redundant_else`, fixed in the same commit.
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
emilk
force-pushed
the
emilk/enable-more-clippy-lints-4
branch
from
September 1, 2026 14:03
c866451 to
f32b894
Compare
Manual `Debug` impls that skip a field now end in `finish_non_exhaustive()`, so the output says a field was omitted instead of implying the struct only has the ones listed. `Column` keeps `finish()` under an `#[expect]`: its `Debug` output appears verbatim in user-facing error messages.
`cargo clippy --all-features` never compiles this branch, so the manual_string_new pass missed it and the lint fired for anyone building datafusion-common without the `backtrace` feature.
Prefer the positive condition first: `if x { .. } else { .. }` rather than
`if !x { .. } else { .. }`, so the reader does not have to negate mentally
to follow which branch runs.
Mostly `cargo clippy --fix`. Comments that described the negated branch were
moved to the branch they actually describe, and thirteen sites in
datafusion-sql were done by hand because one suggestion in that crate did not
type-check, which made rustfix roll back the whole crate.
emilk
force-pushed
the
emilk/enable-more-clippy-lints-4
branch
from
September 1, 2026 14:18
f32b894 to
e448a28
Compare
emilk
marked this pull request as ready for review
September 1, 2026 14:19
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24848 +/- ##
==========================================
- Coverage 81.59% 81.58% -0.01%
==========================================
Files 1123 1123
Lines 406811 408199 +1388
Branches 406811 408199 +1388
==========================================
+ Hits 331923 333019 +1096
- Misses 55451 55618 +167
- Partials 19437 19562 +125 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
…this might be a bit big 😬 let me know if I should break it up. (each commit in itself is pretty reviewable though) |
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Clippychecks in CI #18467.Rationale for this change
Continuing the work in:
Turn on more
clippy::pedanticlints from the opt-out list inCargo.toml.What changes are included in this PR?
One commit per lint, each removing its
"allow"line fromCargo.tomlandfixing every site. Review one commit at a time!
Let me know if you disagree with any and I'll revert it.
manual_string_newString::new()instead of"".to_string()/"".to_owned()/"".into()ignored_unit_patternsOk(())instead ofOk(_), so the pattern stops matching if a payload is ever addedredundant_elseelseafter a diverging branchunnested_or_patternsTime32(Microsecond | Nanosecond)instead of repeating the prefixmatch_boolif/elseinstead ofmatchon a boolmissing_fields_in_debugfinish_non_exhaustive()on manualDebugimpls that skip a fieldif_not_elseWhat is the testing strategy for this PR?
Clippy is clean both with
--all-featuresand with default features (the latteris what caught the
cfg-gated site). The extended suite passes: 69 testbinaries, 10999 tests. The changes are mechanical, so no new tests.
Are there any user-facing changes?
No