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

enum_variant_names lint gives invalid suggestions #739

Closed
mrmonday opened this issue Mar 5, 2016 · 8 comments · Fixed by #4345
Closed

enum_variant_names lint gives invalid suggestions #739

mrmonday opened this issue Mar 5, 2016 · 8 comments · Fixed by #4345
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@mrmonday
Copy link
Contributor

mrmonday commented Mar 5, 2016

Given:

pub enum NetworkLayer {
    Layer2,
    Layer3,
}

Clippy gives the following warning/help:

warning: All variants have the same prefix: `Layer`, #[warn(enum_variant_names)] on by default
help: remove the prefixes and use full paths to the variants instead of glob imports

Here, removing the prefix would lead to invalid identifiers.

@Manishearth
Copy link
Member

Oh, that's a good point. @oli-obk want to fix this? (or I can, pretty simple)

@oli-obk
Copy link
Contributor

oli-obk commented Mar 5, 2016

Well. The hint should be changed, but i still consider the names to be bad. Should rather be the name of the layer instead of the number

@Manishearth
Copy link
Member

Yeah, in this case the names could be improved (but in the general case, that's not always true)

@mrmonday
Copy link
Contributor Author

mrmonday commented Mar 5, 2016

(For the record, I plan to completely remove the version of that struct I'm actually using, the poor names are the least of the problems with it...)

I agree about the general case, which is the main reason I reported it.

@oli-obk
Copy link
Contributor

oli-obk commented May 10, 2017

This is fixed.

@oli-obk oli-obk closed this as completed May 10, 2017
@CAD97
Copy link
Contributor

CAD97 commented May 10, 2019

This came back.

enum Peek {
    Peek1,
    Peek2,
    Peek3,
}
warning: Variant name starts with the enum's name
 --> src/lib.rs:2:5
  |
2 |     Peek1,
  |     ^^^^^
  |
  = note: #[warn(clippy::enum_variant_names)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

warning: Variant name starts with the enum's name
 --> src/lib.rs:3:5
  |
3 |     Peek2,
  |     ^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

warning: Variant name starts with the enum's name
 --> src/lib.rs:4:5
  |
4 |     Peek3,
  |     ^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

warning: All variants have the same prefix: `Peek`
 --> src/lib.rs:1:1
  |
1 | / enum Peek {
2 | |     Peek1,
3 | |     Peek2,
4 | |     Peek3,
5 | | }
  | |_^
  |
  = help: remove the prefixes and use full paths to the variants instead of glob imports
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names

Naming the first one just Peek (which is in line with the syn functions this is choosing between) silences the All and first variant warnings, but the 2nd and 3rd variant warnings remain.

@phansch
Copy link
Member

phansch commented May 10, 2019

It looks like this only happens when it's a private enum. Once it's made pub, the warning disappears.

@phansch phansch reopened this May 10, 2019
@phansch phansch added C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels May 10, 2019
@CAD97
Copy link
Contributor

CAD97 commented May 10, 2019

That would be because the pub lint is a different lint in the pedantic category which is allow-by-default.

@phansch phansch self-assigned this Aug 5, 2019
phansch added a commit to phansch/rust-clippy that referenced this issue Aug 6, 2019
As [per the reference](https://doc.rust-lang.org/reference/identifiers.html),
identifiers must start with a letter. So we don't suggest a better
variant naming in these cases.

Fixes rust-lang#739
bors added a commit that referenced this issue Aug 7, 2019
Don't emit enum_variant_names if remainder starts with a numeric

changelog: Fix false positive in `pub_enum_variant_names` and `enum_variant_names`

As [per the reference](https://doc.rust-lang.org/reference/identifiers.html), identifiers must start with a letter. So we don't suggest a better
variant naming in case the remainder would start with a numeric.

Fixes #739
bors added a commit that referenced this issue Aug 7, 2019
Don't emit enum_variant_names if remainder starts with a numeric

changelog: Fix false positive in `pub_enum_variant_names` and `enum_variant_names`

As [per the reference](https://doc.rust-lang.org/reference/identifiers.html), identifiers must start with a letter. So we don't suggest a better
variant naming in case the remainder would start with a numeric.

Fixes #739
@bors bors closed this as completed in #4345 Aug 7, 2019
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 I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants