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.

Sign up for GitHub

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

Jump to bottom

enum_variant_names false positive when first word has common prefix as part of word #8090

Closed
tspiteri opened this issue Dec 7, 2021 · 1 comment · Fixed by #8127
Closed

enum_variant_names false positive when first word has common prefix as part of word #8090

tspiteri opened this issue Dec 7, 2021 · 1 comment · Fixed by #8127
Assignees
@dswij
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@tspiteri
Copy link
Contributor

tspiteri commented Dec 7, 2021 •
edited by rustbot
Loading

Summary

The lint warns if all variants have the same prefix word. If the first variant has the prefix word at the start of its first word, it wrongly triggers the lint even if the first word is longer than the prefix word. If this variant is not the first variant, the warning is not triggered.

Lint Name

enum_variant_names

Reproducer

I tried this code:

enum _E {
    Normal,
    NoLeft,
    NoRight,
}

I saw this happen:

warning: all variants have the same prefix: `No`
 --> src/lib.rs:1:1
  |
1 | / enum _E {
2 | |     Normal,
3 | |     NoLeft,
4 | |     NoRight,
5 | | }
  | |_^
  |
  = note: `#[warn(clippy::enum_variant_names)]` on by default
  = 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

I expected no warning.

Version

rustc 1.58.0-beta.2 (0e07bcb68 2021-12-04)
binary: rustc
commit-hash: 0e07bcb68b82b54c0c4ec6fe076e9d75b02109cf
commit-date: 2021-12-04
host: x86_64-unknown-linux-gnu
release: 1.58.0-beta.2
LLVM version: 13.0.0

clippy 0.1.58 (0e07bcb 2021-12-04)

Additional Labels

No response
@tspiteri tspiteri added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Dec 7, 2021
@dswij
Copy link
Member

dswij commented Dec 9, 2021

Huh 🤔 I can take a look on this

@rustbot claim

@dswij dswij mentioned this issue Dec 23, 2021
Merged
bors added a commit that referenced this issue Dec 27, 2021
@bors
Auto merge of #8127 - dswij:8090, r=xFrednet
ab66862 
Fix `enum_variants` FP on prefixes that are not camel-case

closes #8090

Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.

This changes how the lint behaves:

1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
    cFoo,
    cBar,
    cBaz,
}

enum Something {
    CCall,
    CCreate,
    CCryogenize,
}
```

2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
    Prefix的,
    PrefixTea,
    PrefixCake,
}
```
will be considered as `Prefix的`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.

changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.
bors added a commit that referenced this issue Dec 27, 2021
@bors
Auto merge of #8127 - dswij:8090, r=xFrednet
e5293c5 
Fix `enum_variants` FP on prefixes that are not camel-case

closes #8090

Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.

This changes how the lint behaves:

1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
    cFoo,
    cBar,
    cBaz,
}

enum Something {
    CCall,
    CCreate,
    CCryogenize,
}
```

2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
    PrefixXXX,
    PrefixTea,
    PrefixCake,
}
```
will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.

changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.

---

 (Edited by `@xFrednet` removed some non ascii characters)
bors added a commit that referenced this issue Dec 27, 2021
@bors
Auto merge of #8127 - dswij:8090, r=xFrednet
3f2bb30 
Fix `enum_variants` FP on prefixes that are not camel-case

closes #8090

Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.

This changes how the lint behaves:

1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
    cFoo,
    cBar,
    cBaz,
}

enum Something {
    CCall,
    CCreate,
    CCryogenize,
}
```

2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
    PrefixXXX,
    PrefixTea,
    PrefixCake,
}
```
will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.

changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.

---

 (Edited by `@xFrednet` removed some non ascii characters)
@bors bors closed this as completed in 56ccd30 Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dswij dswij

Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix enum_variants FP on prefixes that are not camel-case dswij/rust-clippy
2 participants
@tspiteri @dswij