Skip to content

chore: enforce clippy::allow_attributes for datasource crates#19068

Merged
Jefffrey merged 5 commits intoapache:mainfrom
chakkk309:enforce-clippyallow_attributes-for-datasource-crates
Dec 4, 2025
Merged

chore: enforce clippy::allow_attributes for datasource crates#19068
Jefffrey merged 5 commits intoapache:mainfrom
chakkk309:enforce-clippyallow_attributes-for-datasource-crates

Conversation

@chakkk309
Copy link
Contributor

Which issue does this PR close?

Part of #18881

Rationale for this change

Implement clippy::allow_attributes lint for datasource* crates

What changes are included in this PR?

  1. Added lint enforcement: #![deny(clippy::allow_attributes)] to 6 datasource module mod.rs files
  2. Attribute conversion: Changed 3 #[allow(...)] to #[expect(...)]:
    • deprecated
    • unused_imports
    • rustdoc::broken_intra_doc_links

Are these changes tested?

yes

Are there any user-facing changes?

No user-facing changes.

@github-actions github-actions bot added the datasource Changes to the datasource crate label Dec 3, 2025
@Jefffrey Jefffrey mentioned this pull request Dec 3, 2025
38 tasks
@chakkk309 chakkk309 force-pushed the enforce-clippyallow_attributes-for-datasource-crates branch from 810241e to 9e94f57 Compare December 3, 2025 15:07
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.

looks good to me -- thank you @chakkk309

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.

Thanks for picking this up, left some suggestions

pub use udwf::window_doc_sections;

#[allow(rustdoc::broken_intra_doc_links)]
#[expect(rustdoc::broken_intra_doc_links)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also fix this instead of expecting it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, I've identified 2 options to address the #[expect(rustdoc::broken_intra_doc_links)] in datafusion-doc/src/lib.rs. Since datafusion-doc is designed as an independent crate without dependencies on other DataFusion crates, the original intra-doc links like ScalarUDFImpl cannot be resolved.

Option 1: Simple code references

 /// Documentation for use by `ScalarUDFImpl`, `AggregateUDFImpl` and `WindowUDFImpl` functions.`

Option 2: External links to docs.rs

  /// Documentation for use by [`ScalarUDFImpl`], [`AggregateUDFImpl`] and [`WindowUDFImpl`] functions.
  ///
  /// [`ScalarUDFImpl`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/trait.ScalarUDFImpl.html
  /// [`AggregateUDFImpl`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/trait.AggregateUDFImpl.html
  /// [`WindowUDFImpl`]: https://docs.rs/datafusion-expr/latest/datafusion_expr/trait.WindowUDFImpl.html

Which kind of method would we prefer for this use case?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think option 1 is sufficient

chakkk309 and others added 2 commits December 4, 2025 08:50
Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
@chakkk309 chakkk309 requested a review from Jefffrey December 4, 2025 11:26
@Jefffrey Jefffrey added this pull request to the merge queue Dec 4, 2025
Merged via the queue into apache:main with commit 71fdad0 Dec 4, 2025
27 checks passed
@Jefffrey
Copy link
Contributor

Jefffrey commented Dec 4, 2025

Thanks @chakkk309 & @alamb

@chakkk309 chakkk309 deleted the enforce-clippyallow_attributes-for-datasource-crates branch December 4, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants