Skip to content

overhaul unused doc comments lint #57882

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 9, 2019
Merged

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Jan 24, 2019

This PR contains a number of improvements to the unused_doc_comments lint.

  • Extends the span to cover the entire comment when using sugared doc comments.
  • Triggers the lint for all unused doc comments on a node, instead of just the first one.
  • Triggers the lint on macro expansions, and provides a help note explaining that doc comments must be expanded by the macro.
  • Adds a label pointing at the node that cannot be documented.

Furthermore, this PR fixes any instances in rustc where a macro expansion was erroneously documented.

@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 24, 2019
@michaelwoerister
Copy link
Member

r? @estebank

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 25, 2019

📌 Commit 871e460 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 25, 2019
Centril added a commit to Centril/rust that referenced this pull request Jan 26, 2019
…ebank

overhaul unused doc comments lint

This PR contains a number of improvements to the `unused_doc_comments` lint.

- Extends the span to cover the entire comment when using sugared doc comments.
- Triggers the lint for all unused doc comments on a node, instead of just the first one.
- Triggers the lint on macro expansions, and provides a help note explaining that doc comments must be expanded by the macro.
- Adds a label pointing at the node that cannot be documented.

Furthermore, this PR fixes any instances in rustc where a macro expansion was erroneously documented.
Centril added a commit to Centril/rust that referenced this pull request Jan 26, 2019
…ebank

overhaul unused doc comments lint

This PR contains a number of improvements to the `unused_doc_comments` lint.

- Extends the span to cover the entire comment when using sugared doc comments.
- Triggers the lint for all unused doc comments on a node, instead of just the first one.
- Triggers the lint on macro expansions, and provides a help note explaining that doc comments must be expanded by the macro.
- Adds a label pointing at the node that cannot be documented.

Furthermore, this PR fixes any instances in rustc where a macro expansion was erroneously documented.
@bors
Copy link
Collaborator

bors commented Jan 26, 2019

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

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 26, 2019
@bors

This comment has been minimized.

@euclio euclio force-pushed the unused-doc-attributes branch from 871e460 to 75d30fd Compare January 27, 2019 21:15
@euclio
Copy link
Contributor Author

euclio commented Jan 27, 2019

@estebank Rebased.

@estebank
Copy link
Contributor

@bors try

@euclio let's verify how much this change will impact the ecosystem before we merge.

@bors
Copy link
Collaborator

bors commented Jan 27, 2019

⌛ Trying commit 75d30fd9401bd8f341ca82626edf61ca8e4ddd28 with merge 58c1b81ef6055246e8b6467f767b2e3c1ce4403b...

@bors
Copy link
Collaborator

bors commented Jan 28, 2019

☀️ Test successful - checks-travis
State: approved= try=True

@euclio euclio force-pushed the unused-doc-attributes branch from 75d30fd to 930c6c8 Compare January 31, 2019 16:43
@euclio
Copy link
Contributor Author

euclio commented Jan 31, 2019

@estebank rebased. Did we ever get a crater run going for this?

@bors
Copy link
Collaborator

bors commented Feb 9, 2019

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

@euclio euclio force-pushed the unused-doc-attributes branch from 930c6c8 to 1b5f2c7 Compare February 11, 2019 17:41
@euclio
Copy link
Contributor Author

euclio commented Feb 11, 2019

@estebank Rebased.

kennytm added a commit to kennytm/rust that referenced this pull request Mar 2, 2019
allow specifying attributes for tool lints

Needed for clippy to fix `unused_doc_comments` warnings that will be exposed by rust-lang#57882 (and thus unblock it).
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Mar 2, 2019
allow specifying attributes for tool lints

Needed for clippy to fix `unused_doc_comments` warnings that will be exposed by rust-lang#57882 (and thus unblock it).
@bors
Copy link
Collaborator

bors commented Mar 4, 2019

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

@euclio euclio force-pushed the unused-doc-attributes branch from b5fadf0 to 8edbc64 Compare March 4, 2019 22:36
bors added a commit to rust-lang/rust-clippy that referenced this pull request Mar 6, 2019
move lint documentation into macro invocations

This PR moves lint documentation inside `declare_clippy_lint!` macro invocations, to avoid triggering the `unused_doc_comments` lint once it's modified in rust-lang/rust#57882. This PR is necessary to unblock that work, since the large number of warnings generated in `clippy_lints` causes Travis to hit the log length limit.

This PR also updates the documentation and website generation script.

It would be nice to get a clippy update in the Rust repo once this is merged.

cc @phansch
@phansch phansch mentioned this pull request Mar 6, 2019
@bors
Copy link
Collaborator

bors commented Mar 7, 2019

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

@phansch
Copy link
Member

phansch commented Mar 8, 2019

@euclio The Clippy submodule should now contain the changes to make this pass.

euclio added 2 commits March 8, 2019 12:39
Report all unused attributes on a given doc comment instead of just the
first one, and extend the span of sugared doc comments to encompass the
whole comment.
Report the diagnostic on macro expansions, and add a label indicating
why the comment is unused.
@euclio euclio force-pushed the unused-doc-attributes branch from 8edbc64 to daf80f7 Compare March 8, 2019 17:50
@euclio
Copy link
Contributor Author

euclio commented Mar 8, 2019

@estebank Ok, let's give this another shot!

@estebank
Copy link
Contributor

estebank commented Mar 8, 2019

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 8, 2019

📌 Commit daf80f7 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 8, 2019
@bors
Copy link
Collaborator

bors commented Mar 9, 2019

⌛ Testing commit daf80f7 with merge e1b8898...

bors added a commit that referenced this pull request Mar 9, 2019
overhaul unused doc comments lint

This PR contains a number of improvements to the `unused_doc_comments` lint.

- Extends the span to cover the entire comment when using sugared doc comments.
- Triggers the lint for all unused doc comments on a node, instead of just the first one.
- Triggers the lint on macro expansions, and provides a help note explaining that doc comments must be expanded by the macro.
- Adds a label pointing at the node that cannot be documented.

Furthermore, this PR fixes any instances in rustc where a macro expansion was erroneously documented.
@bors
Copy link
Collaborator

bors commented Mar 9, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: estebank
Pushing e1b8898 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 9, 2019
@bors bors merged commit daf80f7 into rust-lang:master Mar 9, 2019
@euclio euclio deleted the unused-doc-attributes branch March 9, 2019 14:31
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 22, 2020
Clarify unused_doc_comments note on macro invocations

The previous error message used to say:

<pre>
/// doc
^^^^^^^ rustdoc does not generate documentation for <b>macros</b>
</pre>

Obviously we do generate documentation for macros, such as https://docs.rs/bitflags/1.2.1/bitflags/macro.bitflags.html. It's only macro invocations that don't get their own docs. This PR updates the message to say "rustdoc does not generate documentation for <b>macro invocations</b>".

I observe that prior to rust-lang#69084 this used to say "rustdoc does not generate documentation for **macro expansions**", as implemented originally in rust-lang#57882. I don't have a preference between those but I made the commit before looking up the history.

r? @Manishearth
attn: @yaahc @euclio
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants