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

move upper_case_acronyms back to style, but make the default behaviour less aggressive by default (can be unleashed via config option) #6788

Merged
merged 3 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let enum_variant_name_threshold = conf.enum_variant_name_threshold;
store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
store.register_early_pass(|| box upper_case_acronyms::UpperCaseAcronyms);
let upper_case_acronyms_aggressive = conf.upper_case_acronyms_aggressive;
store.register_early_pass(move || box upper_case_acronyms::UpperCaseAcronyms::new(upper_case_acronyms_aggressive));
store.register_late_pass(|| box default::Default::default());
store.register_late_pass(|| box unused_self::UnusedSelf);
store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
Expand Down Expand Up @@ -1416,7 +1417,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(&unused_self::UNUSED_SELF),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
LintId::of(&wildcard_imports::ENUM_GLOB_USE),
LintId::of(&wildcard_imports::WILDCARD_IMPORTS),
LintId::of(&zero_sized_map_values::ZERO_SIZED_MAP_VALUES),
Expand Down Expand Up @@ -1716,6 +1716,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&unwrap::PANICKING_UNWRAP),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
LintId::of(&useless_conversion::USELESS_CONVERSION),
LintId::of(&vec::USELESS_VEC),
LintId::of(&vec_init_then_push::VEC_INIT_THEN_PUSH),
Expand Down Expand Up @@ -1835,6 +1836,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
LintId::of(&write::PRINTLN_EMPTY_STRING),
LintId::of(&write::PRINT_LITERAL),
LintId::of(&write::PRINT_WITH_NEWLINE),
Expand Down
40 changes: 32 additions & 8 deletions clippy_lints/src/upper_case_acronyms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,20 @@ use rustc_ast::ast::{Item, ItemKind, Variant};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::Ident;

declare_clippy_lint! {
/// **What it does:** Checks for camel case name containing a capitalized acronym.
/// **What it does:** Checks for fully capitalized names and optionally names containing a capitalized acronym.
///
/// **Why is this bad?** In CamelCase, acronyms count as one word.
/// See [naming conventions](https://rust-lang.github.io/api-guidelines/naming.html#casing-conforms-to-rfc-430-c-case)
/// for more.
///
/// By default, the lint only triggers on fully-capitalized names.
/// You can use the `upper-case-acronyms-aggressive: true` config option to enable linting
/// on all camel case names
///
/// **Known problems:** When two acronyms are contiguous, the lint can't tell where
/// the first acronym ends and the second starts, so it suggests to lowercase all of
/// the letters in the second acronym.
Expand All @@ -29,11 +33,24 @@ declare_clippy_lint! {
/// struct HttpResponse;
/// ```
pub UPPER_CASE_ACRONYMS,
pedantic,
style,
"capitalized acronyms are against the naming convention"
}

declare_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]);
#[derive(Default)]
pub struct UpperCaseAcronyms {
upper_case_acronyms_aggressive: bool,
}

impl UpperCaseAcronyms {
pub fn new(aggressive: bool) -> Self {
Self {
upper_case_acronyms_aggressive: aggressive,
}
}
}

impl_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]);

fn correct_ident(ident: &str) -> String {
let ident = ident.chars().rev().collect::<String>();
Expand All @@ -56,11 +73,18 @@ fn correct_ident(ident: &str) -> String {
ident
}

fn check_ident(cx: &EarlyContext<'_>, ident: &Ident) {
fn check_ident(cx: &EarlyContext<'_>, ident: &Ident, be_aggressive: bool) {
let span = ident.span;
let ident = &ident.as_str();
let corrected = correct_ident(ident);
if ident != &corrected {
// warn if we have pure-uppercase idents
// assume that two-letter words are some kind of valid abbreviation like FP for false positive
// (and don't warn)
if (ident.chars().all(|c| c.is_ascii_uppercase()) && ident.len() > 2)
// otherwise, warn if we have SOmeTHING lIKE THIs but only warn with the aggressive
// upper-case-acronyms-aggressive config option enabled
|| (be_aggressive && ident != &corrected)
{
span_lint_and_sugg(
cx,
UPPER_CASE_ACRONYMS,
Expand All @@ -82,12 +106,12 @@ impl EarlyLintPass for UpperCaseAcronyms {
ItemKind::TyAlias(..) | ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Trait(..)
);
then {
check_ident(cx, &it.ident);
check_ident(cx, &it.ident, self.upper_case_acronyms_aggressive);
}
}
}

fn check_variant(&mut self, cx: &EarlyContext<'_>, v: &Variant) {
check_ident(cx, &v.ident);
check_ident(cx, &v.ident, self.upper_case_acronyms_aggressive);
}
}
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ define_Conf! {
(disallowed_methods, "disallowed_methods": Vec<String>, Vec::<String>::new()),
/// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators.
(unreadable_literal_lint_fractions, "unreadable_literal_lint_fractions": bool, true),
/// Lint: UPPER_CASE_ACRONYMS. Enables verbose mode. Triggers if there is more than one uppercase char next to each other
(upper_case_acronyms_aggressive, "upper_case_acronyms_aggressive": bool, false),
/// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest.
(cargo_ignore_publish, "cargo_ignore_publish": bool, false),
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `cargo-ignore-publish`, `third-party` at line 5 column 1
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `third-party` at line 5 column 1

error: aborting due to previous error

1 change: 1 addition & 0 deletions tests/ui-toml/upper_case_acronyms_aggressive/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
upper-case-acronyms-aggressive = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![warn(clippy::upper_case_acronyms)]

struct HTTPResponse; // not linted by default, but with cfg option

struct CString; // not linted

enum Flags {
NS, // not linted
CWR,
ECE,
URG,
ACK,
PSH,
RST,
SYN,
FIN,
}

struct GCCLLVMSomething; // linted with cfg option, beware that lint suggests `GccllvmSomething` instead of
// `GccLlvmSomething`

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
error: name `HTTPResponse` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:3:8
|
LL | struct HTTPResponse; // not linted by default, but with cfg option
| ^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `HttpResponse`
|
= note: `-D clippy::upper-case-acronyms` implied by `-D warnings`

error: name `NS` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:8:5
|
LL | NS, // not linted
| ^^ help: consider making the acronym lowercase, except the initial letter (notice the capitalization): `Ns`

error: name `CWR` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:9:5
|
LL | CWR,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Cwr`

error: name `ECE` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:10:5
|
LL | ECE,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Ece`

error: name `URG` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:11:5
|
LL | URG,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Urg`

error: name `ACK` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:12:5
|
LL | ACK,
| ^^^ help: consider making the acronym lowercase, except the initial letter (notice the capitalization): `Ack`

error: name `PSH` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:13:5
|
LL | PSH,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Psh`

error: name `RST` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:14:5
|
LL | RST,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Rst`

error: name `SYN` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:15:5
|
LL | SYN,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Syn`

error: name `FIN` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:16:5
|
LL | FIN,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Fin`

error: name `GCCLLVMSomething` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:19:8
|
LL | struct GCCLLVMSomething; // linted with cfg option, beware that lint suggests `GccllvmSomething` instead of
| ^^^^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `GccllvmSomething`

error: aborting due to 11 previous errors

7 changes: 4 additions & 3 deletions tests/ui/upper_case_acronyms.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#![warn(clippy::upper_case_acronyms)]

struct HTTPResponse; // linted
struct HTTPResponse; // not linted by default, but with cfg option

struct CString; // not linted

enum Flags {
NS, // linted
NS, // not linted
CWR,
ECE,
URG,
Expand All @@ -16,6 +16,7 @@ enum Flags {
FIN,
}

struct GCCLLVMSomething; // linted, beware that lint suggests `GccllvmSomething` instead of `GccLlvmSomething`
struct GCCLLVMSomething; // linted with cfg option, beware that lint suggests `GccllvmSomething` instead of
// `GccLlvmSomething`

fn main() {}
24 changes: 3 additions & 21 deletions tests/ui/upper_case_acronyms.stderr
Original file line number Diff line number Diff line change
@@ -1,22 +1,10 @@
error: name `HTTPResponse` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:3:8
|
LL | struct HTTPResponse; // linted
| ^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `HttpResponse`
|
= note: `-D clippy::upper-case-acronyms` implied by `-D warnings`

error: name `NS` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:8:5
|
LL | NS, // linted
| ^^ help: consider making the acronym lowercase, except the initial letter (notice the capitalization): `Ns`

error: name `CWR` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:9:5
|
LL | CWR,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Cwr`
|
= note: `-D clippy::upper-case-acronyms` implied by `-D warnings`

error: name `ECE` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:10:5
Expand Down Expand Up @@ -60,11 +48,5 @@ error: name `FIN` contains a capitalized acronym
LL | FIN,
| ^^^ help: consider making the acronym lowercase, except the initial letter: `Fin`

error: name `GCCLLVMSomething` contains a capitalized acronym
--> $DIR/upper_case_acronyms.rs:19:8
|
LL | struct GCCLLVMSomething; // linted, beware that lint suggests `GccllvmSomething` instead of `GccLlvmSomething`
| ^^^^^^^^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `GccllvmSomething`

error: aborting due to 11 previous errors
error: aborting due to 8 previous errors