Auto merge of #6788 - matthiaskrgr:upper_case_acronyms, r=flip1995

move upper_case_acronyms back to style, but make the default behaviour less aggressive by default (can be unleashed via config option) Previous discussion in the bi-weekly clippy meeting for reference: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Meeting.202021-02-23/near/227458019 Move the `upper_case_acronyms` lint back to the style group. Only warn on fully-capitalized names by default. Add add a clippy-config option `upper-case-acronyms-aggressive: true/false` to enabled more aggressive linting on all substrings that could be capitalized acronyms. --- changelog: reenable upper_case_acronyms by default but make the more aggressive linting opt-in via config option
rust-lang · Feb 25, 2021 · ef41f2b · ef41f2b
2 parents 76a689d + 2a6b061
commit ef41f2b
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 35 deletions.
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -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);
@@ -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),
@@ -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),
@@ -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),

diff --git a/clippy_lints/src/upper_case_acronyms.rs b/clippy_lints/src/upper_case_acronyms.rs
@@ -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.
@@ -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>();
@@ -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,
@@ -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);
     }
 }
diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -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),
 }

diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -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
 
diff --git a/tests/ui-toml/upper_case_acronyms_aggressive/clippy.toml b/tests/ui-toml/upper_case_acronyms_aggressive/clippy.toml
@@ -0,0 +1 @@
+upper-case-acronyms-aggressive = true
diff --git a/tests/ui-toml/upper_case_acronyms_aggressive/upper_case_acronyms.rs b/tests/ui-toml/upper_case_acronyms_aggressive/upper_case_acronyms.rs
@@ -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() {}
diff --git a/tests/ui-toml/upper_case_acronyms_aggressive/upper_case_acronyms.stderr b/tests/ui-toml/upper_case_acronyms_aggressive/upper_case_acronyms.stderr
@@ -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
+
diff --git a/tests/ui/upper_case_acronyms.rs b/tests/ui/upper_case_acronyms.rs
@@ -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,
@@ -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() {}
diff --git a/tests/ui/upper_case_acronyms.stderr b/tests/ui/upper_case_acronyms.stderr
@@ -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
@@ -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