Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

The useless attribute lint doesn't put the ! in the right place. #1522

Open
BookOwl opened this issue Feb 8, 2017 · 7 comments
Open

The useless attribute lint doesn't put the ! in the right place. #1522

BookOwl opened this issue Feb 8, 2017 · 7 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on

Comments

@BookOwl
Copy link

BookOwl commented Feb 8, 2017

When the useless_attribute lint is activated, it says that you most likely forgot an exclamation mark (!). While this is true, it also suggests to stick the ! in the middle of the attribute:

warning: useless lint attribute, #[warn(useless_attribute)] on by default
 --> src/lib.rs:1:38
  |
1 | #[cfg_attr(feature = "cargo-clippy", allow(new_without_default))]
  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
help: if you just forgot a `!`, use
  | #[cfg_attr(feature = "cargo-clippy", a!llow(new_without_default))]
  = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#useless_attribute

The correct response should be to suggest adding it after the #
I'm using the latest clippy on the latest nightly Rust.

@mcarton mcarton added the C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages label Feb 8, 2017
@TimDiekmann
Copy link
Member

I just had this issue with enum_glob_use too. I guess, the message is bugged in general.

@oli-obk oli-obk added C-bug Category: Clippy is not doing the correct thing and removed C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Jan 18, 2018
@phansch
Copy link
Member

phansch commented Jan 29, 2018

It looks like there is no attribute information for cfg_attr (item.attrs does not include it at all). I will take a stab at this and see if I can find a workaround.

I believe rust-lang/rust#25544 is related somehow

@phansch
Copy link
Member

phansch commented Jan 29, 2018

Unfortunately this seems really difficult to find a proper fix for. Since cfg_attr and cfg attributes are not accessible in the linting process, the only way to work around that, would be to use the CodeMap to search for the beginning of the attribute.

I managed to get a fix working, but only if the whole cfg_attr attribute is on a single line. Cases where the cfg_attr attribute is split over multiple lines would still be broken. I'm not sure if it makes sense to parse the whole file ourselves and look for the beginning of each attribute that spans multiple lines.

Maybe it's easier to remove the suggestion from this lint for now? @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Jan 29, 2018

@phansch can you detect the cfg case? if so, just don't emit the suggestion in the cfg case

@phansch
Copy link
Member

phansch commented Jan 30, 2018

Sorry, my previous comment was partly wrong (now updated):

As far as I can tell, only cfg_attr is missing from the item attributes, so I think we don't have to worry about cfg. Using CodeMap, I can detect cfg_attr somewhat easily if it's on the same line, but it's more work to detect if the attribute starts on a previous line, like

#[cfg_attr(feature = "cargo-clippy",
  allow(dead_code),
  allow(new_without_default))]

That would require iterating over each line towards the beginning of the file, making a Span out of each Line, checking if the span contains the beginning of the attribute (a #[), if yes, replacing it with #![.

cfg is included in the item attributes, but I'm not sure if it matters for this lint, because as it is right now, cfg attributes are not linted in this lint.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 30, 2018

So... we could just not create a suggestion at all in the multi line case?

@phansch
Copy link
Member

phansch commented Jan 30, 2018

That would be the easier solution, yes. If the attribute starts on the same line, we show the correct suggestion, otherwise we don't show a suggestion 👍

@phansch phansch added the E-hard Call for participation: This a hard problem and requires more experience or effort to work on label Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on
Projects
None yet
Development

No branches or pull requests

5 participants