Skip to content

Enable more clippy lints (part 2/3) - #25080

Draft
emilk wants to merge 6 commits into
apache:mainfrom
emilk:emilk/enable-more-clippy-lints-4b
Draft

Enable more clippy lints (part 2/3)#25080
emilk wants to merge 6 commits into
apache:mainfrom
emilk:emilk/enable-more-clippy-lints-4b

Conversation

@emilk

@emilk emilk commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Part 2 of 3, split out of #24848. Stacked on part 1, so review that first:

Turn on more clippy::pedantic lints from the 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
unnested_or_patterns 68 Time32(Microsecond | Nanosecond) instead of repeating the prefix
match_bool 46 if/else instead of match on a bool
missing_fields_in_debug 29 finish_non_exhaustive() on manual Debug impls that skip a field

What is the testing strategy for this PR?

Clippy is clean both with --all-features and with default features. The
changes are mechanical, so no new tests.

Are there any user-facing changes?

No

`String::new()` instead of `"".to_string()` / `"".to_owned()` / `"".into()`,
which skips the copy-from-empty-slice path. All sites fixed by
`cargo clippy --fix`.
`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.
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.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.46582% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.74%. Comparing base (a5c809f) to head (6599716).

Files with missing lines Patch % Lines
datafusion/datasource/src/write/orchestration.rs 28.57% 5 Missing ⚠️
datafusion/expr/src/type_coercion/functions.rs 54.54% 5 Missing ⚠️
datafusion/sql/src/select.rs 85.71% 2 Missing and 3 partials ⚠️
datafusion/common/src/scalar/mod.rs 63.63% 4 Missing ⚠️
datafusion/datasource-parquet/src/sink.rs 86.66% 2 Missing and 2 partials ⚠️
benchmarks/src/sql_benchmark.rs 86.95% 1 Missing and 2 partials ⚠️
datafusion/common/src/stats.rs 86.36% 3 Missing ⚠️
datafusion/execution/src/async_stream.rs 40.00% 1 Missing and 2 partials ⚠️
datafusion/functions/src/datetime/to_time.rs 50.00% 3 Missing ⚠️
...usion/physical-plan/src/operator_statistics/mod.rs 0.00% 3 Missing ⚠️
... and 47 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25080      +/-   ##
==========================================
- Coverage   81.74%   81.74%   -0.01%     
==========================================
  Files        1128     1128              
  Lines      416644   416631      -13     
  Branches   416644   416631      -13     
==========================================
- Hits       340592   340576      -16     
- Misses      55995    56006      +11     
+ Partials    20057    20049       -8     

☔ 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants