Skip to content

Conversation

@2010YOUY01
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

We have already enforce the lint rule for all workspace crates, now we can enable it globally.

In this PR:

  • adds the lint rule to the top-level cargo.toml
  • There are some utility modules (datafusion-examples, tests) have not been fixed before, and after we have enable the linter rule at workspace level, these violations showed up. This PR suppresses all of them, since those violations are acceptable.
  • Keep the per-crate configuration unchanged
    For example when we enforce it in datafusion-sql, in its lib.rs, we have
// https://github.com/apache/datafusion/issues/18503
#![deny(clippy::needless_pass_by_value)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]

This enforces the rule at the crate level but ignores all violations in tests. We cannot disable lint checks for tests from the global (top-level Cargo.toml) configuration, so we have to keep these per-crate settings until Cargo or Clippy adds support for this in the future. I'll leave a note in the tracking issue.

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added sql SQL Planner physical-expr Changes to the physical-expr crates core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation physical-plan Changes to the physical-plan crate labels Nov 24, 2025
Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that's a lot more expects than I was expecting (heh)

🤔

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @2010YOUY01 and @Jefffrey -- this looks like progress to me

(I agree with @Jefffrey though that it is a lot of expects)

@alamb alamb added this pull request to the merge queue Nov 25, 2025
@alamb alamb added the development-process Related to development process of DataFusion label Nov 25, 2025
Merged via the queue into apache:main with commit 2db3aea Nov 25, 2025
56 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate development-process Related to development process of DataFusion functions Changes to functions implementation physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Tracking] Rollout of new lint clippy::needless_pass_by_value in all datafusion crates

3 participants