Skip to content

upper_case_acronyms warns on public items #6803

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
jyn514 opened this issue Feb 26, 2021 · 3 comments · Fixed by #6805 or #6981
Closed

upper_case_acronyms warns on public items #6803

jyn514 opened this issue Feb 26, 2021 · 3 comments · Fixed by #6805 or #6981
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

@jyn514
Copy link
Member

jyn514 commented Feb 26, 2021

I recently ran nightly clippy on a project and got the following warning:

error: name `YDBError` contains a capitalized acronym
  --> src/simple_api/mod.rs:95:12
   |
95 | pub struct YDBError {
   |            ^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `YdbError`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

The lint is correct, it's not a false positive. However, fixing it would be a breaking change since YDBError is public. Maybe this lint should be off by default since it's only a style lint and it can be hard to fix?

I originally just added #[allow(clippy::upper_case_literals)], but that gives more warnings on stable until this rides the release trains unless I add allow(unknown_renamed_lints), which I'd rather not do.

@matthiaskrgr
Copy link
Member

Huh, does this still happen?
I'm not getting a warning with rustc 1.52.0-nightly (98f8cce6d 2021-02-25)

This should have been "fixed" by #6788 .
Anyway the linting of public items is probably still a bug and still needs fixing (for the gated "aggressive" mode of the lint..)

You mean the upper_case_acronyms lint, right?

@jyn514
Copy link
Member Author

jyn514 commented Feb 26, 2021

This should have been "fixed" by #6788 .

Thanks, it's a lot better after that! It still warns on variants of public enums though:

warning: name `YDB` contains a capitalized acronym
   --> src/context_api/mod.rs:747:5
    |
747 |     YDB(YDBError),
    |     ^^^ help: consider making the acronym lowercase, except the initial letter: `Ydb`
    |
    = note: `#[warn(clippy::upper_case_acronyms)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

for the code

pub enum ParseError<T> {
    YDB(YDBError),
    Utf8(std::string::FromUtf8Error),
    Parse(T, String),
}

@matthiaskrgr matthiaskrgr changed the title Consider turning upper_case_literals off by default upper_case_acronyms warns on public items Feb 26, 2021
@matthiaskrgr matthiaskrgr 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 Feb 26, 2021
bors added a commit that referenced this issue Mar 17, 2021
upper_case_acronyms: don't warn on public items

Fixes #6803

changelog: upper_case_acronyms: ignore public items
@bors bors closed this as completed in 9dba6a9 Mar 17, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 26, 2021

@matthiaskrgr this still seems to be broken with nightly clippy:

$ clippy-driver --version
clippy 0.1.52 (673d0db 2021-03-23)
$ cargo clippy -- -D clippy::all
error: name `YDB` contains a capitalized acronym
 --> src/context_api/mod.rs:747:5
 |
747 | YDB(YDBError),
 | ^^^ help: consider making the acronym lowercase, except the initial letter: `Ydb`
 |
 = note: `-D clippy::upper-case-acronyms` implied by `-D clippy::all`
 = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

@matthiaskrgr matthiaskrgr reopened this Mar 26, 2021
@bors bors closed this as completed in 3e42c35 Mar 31, 2021
nars1 pushed a commit to YottaDB/YDBRust that referenced this issue Jun 9, 2021
These will hit stable in about 6 weeks, it's nice to fix them early.

Note that this does not fix `clippy::upper_case_acronyms` since doing so
would be a breaking change. See
rust-lang/rust-clippy#6803 for more details.

Here are the warnings that were previously emitted:

```
warning: unnecessary trailing semicolon
    --> src/simple_api/mod.rs:1172:10
     |
1172 |         };
     |          ^ help: remove this semicolon
     |
     = note: `#[warn(redundant_semicolons)]` on by default
warning: panic message is not a string literal
   --> examples/threenp1.rs:127:30
    |
127 |             Err(x) => panic!(x),
    |                              ^
    |
    = note: `#[warn(non_fmt_panic)]` on by default
    = note: this is no longer accepted in Rust 2021
help: add a "{}" format string to Display the message
    |
127 |             Err(x) => panic!("{}", x),
    |                              ^^^^^
help: or use std::panic::panic_any instead
    |
127 |             Err(x) => std::panic::panic_any(x),
    |                       ^^^^^^^^^^^^^^^^^^^^^^
error: name `YDB` contains a capitalized acronym
   --> src/context_api/mod.rs:747:5
    |
747 |     YDB(YDBError),
    |     ^^^ help: consider making the acronym lowercase, except the initial letter: `Ydb`
    |
    = note: `-D clippy::upper-case-acronyms` implied by `-D clippy::all`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
```
nars1 pushed a commit to YottaDB/YDBRust that referenced this issue Jun 9, 2021
The lint looked like this:

```
error: name `YDB` contains a capitalized acronym
 --> src/context_api/mod.rs:747:5
 |
747 | YDB(YDBError),
 | ^^^ help: consider making the acronym lowercase, except the initial letter: `Ydb`
 |
 = note: `-D clippy::upper-case-acronyms` implied by `-D clippy::all`
 = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
```

This is a bug in clippy: `YDB` is a public enum variant, and it would be
a breaking change to change the name: rust-lang/rust-clippy#6803

This silences the lint until it's fixed upstream.
# for free to join this conversation on GitHub. Already have an account? # to comment
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
2 participants