Skip to content

use more robust doc attr span checking for proc-macro#16713

Open
zihan0822 wants to merge 2 commits into
rust-lang:masterfrom
zihan0822:fix-doc-header-span-check
Open

use more robust doc attr span checking for proc-macro#16713
zihan0822 wants to merge 2 commits into
rust-lang:masterfrom
zihan0822:fix-doc-header-span-check

Conversation

@zihan0822

@zihan0822 zihan0822 commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

View all comments

fixes #16317
fixes #16382

When checking the span of doc attributes, we use the span of the lit str instead of the span of hir::Attribute. This makes the span checking more robust when we are dealing with certain proc-macro crates.

For example, cxx's bridge takes doc lit str from user and attach them to a new #[doc] created by the proc-macro. In such case, checking the span of lit str makes sure that doc related lints can still fire properly on user-provided docs.

changelog: [empty_docs]: lint user-provided doc on proc-macro expanded codes
changelog: [missing_safety_doc]: lint user-provided doc on proc-macro expanded codes
changelog: [missing_panics_doc]: lint user-provided doc on proc-macro expanded codes

@rustbot

rustbot commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in clippy_lints/src/doc

cc @notriddle

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 13, 2026
@rustbot

rustbot commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@github-actions

github-actions Bot commented Mar 13, 2026

Copy link
Copy Markdown

Lintcheck changes for 22aa352

Lint Added Removed Changed
clippy::doc_markdown 8 13 5
clippy::doc_paragraphs_missing_punctuation 1 0 1
clippy::too_long_first_doc_paragraph 1 0 0

This comment will be updated if you push new changes

@zihan0822 zihan0822 marked this pull request as draft March 13, 2026 18:27
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 13, 2026
@zihan0822 zihan0822 force-pushed the fix-doc-header-span-check branch 2 times, most recently from 84e7d0a to 3d1dce6 Compare March 14, 2026 08:28
@zihan0822

Copy link
Copy Markdown
Contributor Author

dogfood output in ci makes sense to me.

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 14, 2026
@zihan0822 zihan0822 marked this pull request as ready for review March 14, 2026 08:44
@samueltardieu

samueltardieu commented Mar 17, 2026

Copy link
Copy Markdown
Member

dogfood output in ci makes sense to me.

What do you mean? If you mean that some other Clippy sources need to be fixed, by all means do that in a separate commit that you would then move before the current one, so that at any point the CI would be green. Moreover that will show us some real-life consequences of those changes.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 17, 2026
@rustbot

rustbot commented Mar 17, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@zihan0822 zihan0822 force-pushed the fix-doc-header-span-check branch from 3d1dce6 to d5ffe16 Compare March 17, 2026 13:00
@zihan0822

Copy link
Copy Markdown
Contributor Author

fixed clippy source in a separate commit.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Mar 17, 2026
Comment thread clippy_lints/src/doc/mod.rs Outdated
Comment thread clippy_lints/src/doc/mod.rs
Comment thread clippy_lints/src/methods/mod.rs Outdated
@zihan0822 zihan0822 force-pushed the fix-doc-header-span-check branch 3 times, most recently from c8c6833 to b7034e0 Compare March 26, 2026 05:41
@zihan0822 zihan0822 requested a review from notriddle March 26, 2026 09:38
/// /// [`SmallVec<[T; INLINE_CAPACITY]>`][SmallVec].
/// /// [SmallVec]: SmallVec
/// fn main() {}
/// fn foo() {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the rename here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fn main() triggers clippy::needless_doctest_main

Comment thread clippy_lints/src/methods/mod.rs Outdated
Comment thread clippy_lints/src/methods/mod.rs Outdated
/// let a = 0123;
/// println!("{}", a);
/// }
/// let a = 0123;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove fn main()? Is that some cleanup or the result of applying the lint?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fn main() in rust listing triggers clippy::needless_doctest_main, main() in C was removed to pair with that.

Comment thread clippy_lints/src/misc_early/mod.rs
/// | Group | Item Kinds |
/// |--------------------|----------------------|
/// | `modules` | "mod", "foreign_mod" |
/// | `modules` | "mod", "foreign mod" |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove the underscores? I'm not saying this is wrong, I'd like to understand this.

@zihan0822 zihan0822 Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Snake-cased word is treated as item in markdown and it triggers clippy::doc_markdown with warning "item in documentation is missing backticks".

I actually don't know what we should do about this. I think it might be better to keep those words as snake-cased, since they seem to be used as keywords in clippy::arbitrary_source_item_ordering's config. There is no way to suppress this lint locally, because declare_clippy_lint! only accepts doc attribute. Can we just add backticks to all the words in Item Kinds column or add those words to clippy's allowed markdown idents?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i have added those snake-cased item kind names to the markdown ident allowed list and keep this part unchanged. let me know if there is better workaround for this.

Comment thread clippy_lints/src/doc/mod.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 26, 2026
@zihan0822 zihan0822 force-pushed the fix-doc-header-span-check branch from b7034e0 to f0c1cb6 Compare March 27, 2026 06:19
Comment thread clippy_lints/src/arbitrary_source_item_ordering.rs Outdated
@zihan0822 zihan0822 force-pushed the fix-doc-header-span-check branch from f0c1cb6 to 57e7641 Compare April 30, 2026 16:57
zihan0822 added 2 commits May 1, 2026 01:06
clippy source

Signed-off-by: Zihan <zihanli0822@gmail.com>
When checking the span of doc attributes, we use the span of the lit str
instead of the span of `hir::Attribute`. This makes the span checking
more robust when we are dealing with certain proc-macro crates.

For example, some proc-macro crates may take doc lit str from user and
attach them to a new `#[doc]` created by the proc-macro. In such cases,
checking the span of lit str makes sure that doc related lints can still
fire properly on user-provided docs.

changelog: [`empty_docs`]: lint user-provided doc on proc-macro expanded
codes
changelog: [`missing_safety_doc`]: lint user-provided doc on proc-macro expanded
codes
changelog: [`missing_panics_doc`]: lint user-provided doc on proc-macro expanded
codes
changelog: [`needless_doctest_main`]: lint user-provided doc on proc-macro expanded
codes
changelog: [`doc_markdown`]: lint user-provided doc on proc-macro expanded
codes

Signed-off-by: Zihan <zihanli0822@gmail.com>
@zihan0822 zihan0822 force-pushed the fix-doc-header-span-check branch from 57e7641 to 22aa352 Compare April 30, 2026 17:06
@zihan0822 zihan0822 requested a review from samueltardieu April 30, 2026 17:20
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 30, 2026
@rustbot

rustbot commented May 20, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #16767) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False negative of empty_docs when processing proc-macros clippy::missing_safety_doc triggers on cxx::bridge-generated code

4 participants