chore: introduce a broader set of workspace clippy lints by emilk · Pull Request #24066 · apache/datafusion

emilk · 2026-08-03T19:56:20Z

Which issue does this PR close?

Part of Stricter Clippy checks in CI #18467.

Rationale for this change

The workspace already has a [workspace.lints] table, but it was missing from three crates and only enabled a handful of lints. This grows that set slightly, saving bigger changes for future PRs.

What changes are included in this PR?

One commit per step, each of which leaves cargo clippy -D warnings green:

Sort the existing [workspace.lints.clippy] list (no lint added/removed).
Add [lints] workspace = true to datafusion-proto, -proto-common and -proto-models
Enable a batch of non-default clippy lints that are already violation-free.

CI already runs cargo clippy ... -- -D warnings via ci/scripts/rust_clippy.sh,
so no CI change was needed.

Are these changes tested?

Covered by existing tests plus the clippy CI job.

Are there any user-facing changes?

No.

Pure reordering, no lint is added or removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`datafusion/proto`, `datafusion/proto-common` and `datafusion/proto-models` were the only workspace members without `[lints] workspace = true`, so the workspace lint configuration silently did not apply to them. Enabling it surfaces a handful of violations, fixed here: * `unused_qualifications`: redundant path prefixes (mostly `cargo fix`ed) * `clippy::or_fun_call`: `unwrap_or(HashMap::new())` -> `unwrap_or_default()` * `clippy::needless_pass_by_value`: one `#[expect]` on a public API, plus a crate-level exemption for the integration tests matching what the other DataFusion crates already do for their unit tests The generated protobuf modules opt out of `unused_qualifications` alongside the `clippy::all` opt-out they already had. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

All of these are already violation-free across the workspace, so this is pure future-proofing: they only fire on newly written code. They are a subset of the lint set used by https://github.com/emilk/egui, picked because they trigger no (or almost no) violations in DataFusion today. The single existing violation is a deliberate `mem::forget` in an FFI test helper, now marked with `#[expect]`. Part of apache#18467 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`HashTable::find_entry` takes `&mut self`; `find` does the same lookup through `&self`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Gating the `include_str!` docs on `cfg(doc)` means touching the included markdown no longer forces a rebuild of the crate for non-doc builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`SAFETY:` comments should sit next to the `unsafe` block they justify. The existing ones either documented safe code (reworded) or were attached to the enclosing `if` rather than the `unsafe` block (moved). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Enables the rustc lint groups and individual lints from https://github.com/emilk/egui/blob/main/Cargo.toml that DataFusion violates at most 10 times today: * `future_incompatible`, `nonstandard_style`, `rust_2018_idioms` (groups) * `rust_2021_prelude_collisions`, `semicolon_in_expressions_from_macros`, `unsafe_op_in_unsafe_fn`, `unused_extern_crates`, `unused_import_braces`, `unused_lifetimes` `elided_lifetimes_in_paths` is part of `rust_2018_idioms` but has ~800 violations, so it is explicitly allowed for now. `trivial_numeric_casts` (31 violations) and `unsafe_code` are left out entirely. The violations fixed here are vestigial `extern crate` items, redundant import braces, and two `rstest` helpers whose lifetime is unused after macro expansion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Enables the `rustdoc::all` group. Two of its lints have more than 10 violations today and are explicitly allowed for now: `missing_crate_level_docs` (18) and `unescaped_backticks` (28). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Removing it made `libc` an unused dependency (`cargo machete` CI failure): the crate has no path references, it is only linked so the `fesetround`/`fegetround` symbols declared in this module resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Keep the `LogicalPlanType::` prefix on the `CustomScan` match arm so it matches its sibling arms; drop the variant import instead. * Restore a `SAFETY:` comment on the second `unsafe` block in `cursor.rs`. * Drop `rustdoc::broken_intra_doc_links`, already covered by `rustdoc::all`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

codecov-commenter · 2026-08-03T21:10:46Z

Codecov Report

❌ Patch coverage is 75.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.88%. Comparing base (62650ef) to head (c159311).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/proto-common/src/to_proto/mod.rs	64.28%	3 Missing and 2 partials ⚠️
datafusion/datasource-arrow/src/source.rs	50.00%	0 Missing and 4 partials ⚠️
datafusion/functions-nested/src/range.rs	50.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #24066   +/-   ##
=======================================
  Coverage   80.87%   80.88%           
=======================================
  Files        1101     1102    +1     
  Lines      375765   375789   +24     
  Branches   375765   375789   +24     
=======================================
+ Hits       303915   303939   +24     
- Misses      53747    53748    +1     
+ Partials    18103    18102    -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.

2010YOUY01 · 2026-08-04T03:51:34Z

Thank you. I suggest including only the first two commits in this PR to make it easier to verify.

For any additional rules, I think we should first confirm that they are useful, so it would be better to add them in separate PRs.

emilk · 2026-08-04T07:27:16Z

I can do that - but I've tested out all these lints (and more) myself over the last couple of years in both https://github.com/rerun-io/rerun and https://github.com/emilk/egui and can vouch for their usefulness

emilk · 2026-08-04T07:31:28Z

Just the two first commits:

chore: apply workspace lints to all crates #24076

2010YOUY01 · 2026-08-04T08:35:02Z

I can do that - but I've tested out all these lints (and more) myself over the last couple of years in both https://github.com/rerun-io/rerun and https://github.com/emilk/egui and can vouch for their usefulness

Splitting them into separate PRs can make them easier to review. I suggest to do the following steps

chore: apply workspace lints to all crates #24076 LGTM, I'll merge it shortly
Open a separate PR for commit feat: enable a batch of non-default clippy lints workspace-wide
I will review the extra lint rule list later (that list is really helpful!), if there is no issue, we could do that in another PR.

emilk and others added 6 commits August 3, 2026 21:30

chore: sort the workspace clippy lint list alphabetically

18caf39

Pure reordering, no lint is added or removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

feat: enable clippy::manual_instant_elapsed

b5f0376

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

feat: enable clippy::pathbuf_init_then_push

a81afe4

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

feat: enable clippy::ref_option_ref

3cb5e7a

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions Bot added 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 labels Aug 3, 2026

feat: enable clippy::debug_assert_with_mut_call

f93cbf3

`HashTable::find_entry` takes `&mut self`; `find` does the same lookup through `&self`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

emilk marked this pull request as ready for review August 3, 2026 19:59

emilk mentioned this pull request Aug 3, 2026

Rerun PRs rerun-io/opensource#2

Open

emilk and others added 3 commits August 3, 2026 21:59

feat: enable clippy::str_split_at_newline

0743b1d

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

feat: enable clippy::flat_map_option

687fe8c

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

feat: enable clippy::verbose_file_reads

935177b

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions Bot added logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates core Core DataFusion crate common Related to common crate labels Aug 3, 2026

emilk and others added 4 commits August 3, 2026 22:08

feat: enable clippy::doc_include_without_cfg

f1f5d8e

Gating the `include_str!` docs on `cfg(doc)` means touching the included markdown no longer forces a rebuild of the crate for non-doc builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions Bot added optimizer Optimizer rules spark labels Aug 3, 2026

emilk and others added 2 commits August 3, 2026 22:37

emilk mentioned this pull request Aug 4, 2026

chore: apply workspace lints to all crates #24076

Open

emilk marked this pull request as draft August 4, 2026 07:31

Navigation Menu

Uh oh!

chore: introduce a broader set of workspace clippy lints - #24066

chore: introduce a broader set of workspace clippy lints#24066
emilk wants to merge 16 commits into
apache:mainfrom
emilk:emilk/workspace-lints

emilk commented Aug 3, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 3, 2026

Uh oh!

2010YOUY01 commented Aug 4, 2026

Uh oh!

emilk commented Aug 4, 2026

Uh oh!

emilk commented Aug 4, 2026

Uh oh!

2010YOUY01 commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

emilk commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Aug 3, 2026

Codecov Report

Uh oh!

2010YOUY01 commented Aug 4, 2026

Uh oh!

emilk commented Aug 4, 2026

Uh oh!

emilk commented Aug 4, 2026

Uh oh!

2010YOUY01 commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

emilk commented Aug 3, 2026 •

edited

Loading