Skip to content

Tracking issue for inclusion of derive in lint unused_attributes #54651

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

Closed
Tracked by #55112
Havvy opened this issue Sep 28, 2018 · 10 comments · Fixed by #62051
Closed
Tracked by #55112

Tracking issue for inclusion of derive in lint unused_attributes #54651

Havvy opened this issue Sep 28, 2018 · 10 comments · Fixed by #62051
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Havvy
Copy link
Contributor

Havvy commented Sep 28, 2018

The following code emits an unsquelchable warning.

#![crate_type="lib"]

#[derive()]
struct Struct;
warning: empty trait list in `derive`
 --> src/lib.rs:4:1
  |
4 | #[derive()]
  | ^^^^^^^^^^^

This warning should have a named lint unused_derive that is a part of the unused group of lints.

This warning should be a part of unused_attributes.

@Havvy Havvy added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Sep 28, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 28, 2018

empty trait list in derive should have a lint name

... and that name is unused_attributes.

@clintfred
Copy link
Contributor

#[allow(unused_attributes)]
#[derive()]

Does not appear to squelch the warning for me...

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 29, 2018
@Centril Centril changed the title warning: empty trait list in derive should have a lint name Tracking issue for inclusion of derive in lint unused_attributes Sep 29, 2018
@Centril Centril added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Sep 29, 2018
@Centril
Copy link
Contributor

Centril commented Sep 29, 2018

It seems eminently reasonable to me to include #[derive()] in unused_attributes since it is an attribute applied with no effect; so let's kick off fcp for that.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 29, 2018

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 29, 2018
@zackmdavis
Copy link
Member

Implementing this as part of the existing UnusedAttributes pass would be complicated by #45216, but it might also be possible to implement where the existing "empty trait list" warning is issued (possibly as a buffered early lint).

@Havvy
Copy link
Contributor Author

Havvy commented Sep 29, 2018

I need to make UnusedAttributes a buffered early lint for the same thing with cfg_attr.

@joshtriplett
Copy link
Member

Seems reasonable. And a macro that might end up with an empty derive list should have no problem turning off unused_attributes locally.

@scottmcm
Copy link
Member

@rfcbot reviewed

I certainly agree with moving warnings to lints.

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 23, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 23, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Dec 3, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Dec 3, 2018
Centril added a commit to Centril/rust that referenced this issue Jun 22, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 22, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants