Use break api config for enum_variant_names

camsteffen · camsteffen · commit 3d77a2b8617f · 2021-05-26T16:53:13.000-05:00
diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs
@@ -142,6 +142,16 @@ declare_deprecated_lint! {
     "this lint has been replaced by `manual_filter_map`, a more specific lint"
 }
 
+declare_deprecated_lint! {
+    /// **What it does:** Nothing. This lint has been deprecated.
+    ///
+    /// **Deprecation reason:** The `avoid_breaking_exported_api` config option was added, which
+    /// enables the `enum_variant_names` lint for public items.
+    /// ```
+    pub PUB_ENUM_VARIANT_NAMES,
+    "set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items"
+}
+
 declare_deprecated_lint! {
     /// **What it does:** Nothing. This lint has been deprecated.
     ///
diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs
@@ -3,8 +3,8 @@
 use clippy_utils::camel_case;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::source::is_present_in_source;
-use rustc_ast::ast::{EnumDef, Item, ItemKind, VisibilityKind};
-use rustc_lint::{EarlyContext, EarlyLintPass, Lint};
+use rustc_hir::{EnumDef, Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::Symbol;
@@ -39,36 +39,6 @@ declare_clippy_lint! {
     "enums where all variants share a prefix/postfix"
 }
 
-declare_clippy_lint! {
-    /// **What it does:** Detects public enumeration variants that are
-    /// prefixed or suffixed by the same characters.
-    ///
-    /// **Why is this bad?** Public enumeration variant names should specify their variant,
-    /// not repeat the enumeration name.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    /// ```rust
-    /// pub enum Cake {
-    ///     BlackForestCake,
-    ///     HummingbirdCake,
-    ///     BattenbergCake,
-    /// }
-    /// ```
-    /// Could be written as:
-    /// ```rust
-    /// pub enum Cake {
-    ///     BlackForest,
-    ///     Hummingbird,
-    ///     Battenberg,
-    /// }
-    /// ```
-    pub PUB_ENUM_VARIANT_NAMES,
-    pedantic,
-    "public enums where all variants share a prefix/postfix"
-}
-
 declare_clippy_lint! {
     /// **What it does:** Detects type names that are prefixed or suffixed by the
     /// containing module's name.
@@ -127,21 +97,22 @@ declare_clippy_lint! {
 pub struct EnumVariantNames {
     modules: Vec<(Symbol, String)>,
     threshold: u64,
+    avoid_breaking_exported_api: bool,
 }
 
 impl EnumVariantNames {
     #[must_use]
-    pub fn new(threshold: u64) -> Self {
+    pub fn new(threshold: u64, avoid_breaking_exported_api: bool) -> Self {
         Self {
             modules: Vec::new(),
             threshold,
+            avoid_breaking_exported_api,
         }
     }
 }
 
 impl_lint_pass!(EnumVariantNames => [
     ENUM_VARIANT_NAMES,
-    PUB_ENUM_VARIANT_NAMES,
     MODULE_NAME_REPETITIONS,
     MODULE_INCEPTION
 ]);
@@ -167,33 +138,42 @@ fn partial_rmatch(post: &str, name: &str) -> usize {
 }
 
 fn check_variant(
-    cx: &EarlyContext<'_>,
+    cx: &LateContext<'_>,
     threshold: u64,
-    def: &EnumDef,
+    def: &EnumDef<'_>,
     item_name: &str,
     item_name_chars: usize,
     span: Span,
-    lint: &'static Lint,
 ) {
     if (def.variants.len() as u64) < threshold {
         return;
     }
-    for var in &def.variants {
+    for var in def.variants {
         let name = var.ident.name.as_str();
         if partial_match(item_name, &name) == item_name_chars
             && name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase())
             && name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric())
         {
-            span_lint(cx, lint, var.span, "variant name starts with the enum's name");
+            span_lint(
+                cx,
+                ENUM_VARIANT_NAMES,
+                var.span,
+                "variant name starts with the enum's name",
+            );
         }
         if partial_rmatch(item_name, &name) == item_name_chars {
-            span_lint(cx, lint, var.span, "variant name ends with the enum's name");
+            span_lint(
+                cx,
+                ENUM_VARIANT_NAMES,
+                var.span,
+                "variant name ends with the enum's name",
+            );
         }
     }
     let first = &def.variants[0].ident.name.as_str();
     let mut pre = &first[..camel_case::until(&*first)];
     let mut post = &first[camel_case::from(&*first)..];
-    for var in &def.variants {
+    for var in def.variants {
         let name = var.ident.name.as_str();
 
         let pre_match = partial_match(pre, &name);
@@ -226,7 +206,7 @@ fn check_variant(
     };
     span_lint_and_help(
         cx,
-        lint,
+        ENUM_VARIANT_NAMES,
         span,
         &format!("all variants have the same {}fix: `{}`", what, value),
         None,
@@ -261,14 +241,14 @@ fn to_camel_case(item_name: &str) -> String {
     s
 }
 
-impl EarlyLintPass for EnumVariantNames {
-    fn check_item_post(&mut self, _cx: &EarlyContext<'_>, _item: &Item) {
+impl LateLintPass<'_> for EnumVariantNames {
+    fn check_item_post(&mut self, _cx: &LateContext<'_>, _item: &Item<'_>) {
         let last = self.modules.pop();
         assert!(last.is_some());
     }
 
     #[allow(clippy::similar_names)]
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
         let item_name = item.ident.name.as_str();
         let item_name_chars = item_name.chars().count();
         let item_camel = to_camel_case(&item_name);
@@ -286,7 +266,7 @@ impl EarlyLintPass for EnumVariantNames {
                             );
                         }
                     }
-                    if item.vis.kind.is_pub() {
+                    if item.vis.node.is_pub() {
                         let matching = partial_match(mod_camel, &item_camel);
                         let rmatching = partial_rmatch(mod_camel, &item_camel);
                         let nchars = mod_camel.chars().count();
@@ -317,11 +297,9 @@ impl EarlyLintPass for EnumVariantNames {
             }
         }
         if let ItemKind::Enum(ref def, _) = item.kind {
-            let lint = match item.vis.kind {
-                VisibilityKind::Public => PUB_ENUM_VARIANT_NAMES,
-                _ => ENUM_VARIANT_NAMES,
-            };
-            check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span, lint);
+            if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.hir_id())) {
+                check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span);
+            }
         }
         self.modules.push((item.ident.name, item_camel));
     }
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -505,6 +505,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         "clippy::filter_map",
         "this lint has been replaced by `manual_filter_map`, a more specific lint",
     );
+    store.register_removed(
+        "clippy::pub_enum_variant_names",
+        "set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items",
+    );
     store.register_removed(
         "clippy::wrong_pub_self_convention",
         "set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items",
@@ -622,7 +626,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         enum_variants::ENUM_VARIANT_NAMES,
         enum_variants::MODULE_INCEPTION,
         enum_variants::MODULE_NAME_REPETITIONS,
-        enum_variants::PUB_ENUM_VARIANT_NAMES,
         eq_op::EQ_OP,
         eq_op::OP_REF,
         erasing_op::ERASING_OP,
@@ -1080,7 +1083,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(doc::MISSING_PANICS_DOC),
         LintId::of(empty_enum::EMPTY_ENUM),
         LintId::of(enum_variants::MODULE_NAME_REPETITIONS),
-        LintId::of(enum_variants::PUB_ENUM_VARIANT_NAMES),
         LintId::of(eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
         LintId::of(excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
         LintId::of(excessive_bools::STRUCT_EXCESSIVE_BOOLS),
@@ -2015,7 +2017,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let literal_representation_threshold = conf.literal_representation_threshold;
     store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold));
     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_late_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold, avoid_breaking_exported_api));
     store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
     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));
diff --git a/tests/ui/enum_variants.rs b/tests/ui/enum_variants.rs
@@ -1,5 +1,4 @@
-#![feature(non_ascii_idents)]
-#![warn(clippy::enum_variant_names, clippy::pub_enum_variant_names)]
+#![warn(clippy::enum_variant_names)]
 #![allow(non_camel_case_types, clippy::upper_case_acronyms)]
 
 enum FakeCallType {
@@ -97,8 +96,8 @@ pub enum PubSeall {
     WithOut,
 }
 
-#[allow(clippy::pub_enum_variant_names)]
-mod allowed {
+#[allow(clippy::enum_variant_names)]
+pub mod allowed {
     pub enum PubAllowed {
         SomeThis,
         SomeThat,
diff --git a/tests/ui/enum_variants.stderr b/tests/ui/enum_variants.stderr
@@ -1,31 +1,31 @@
 error: variant name ends with the enum's name
-  --> $DIR/enum_variants.rs:16:5
+  --> $DIR/enum_variants.rs:15:5
    |
 LL |     cFoo,
    |     ^^^^
    |
    = note: `-D clippy::enum-variant-names` implied by `-D warnings`
 
 error: variant name starts with the enum's name
-  --> $DIR/enum_variants.rs:27:5
+  --> $DIR/enum_variants.rs:26:5
    |
 LL |     FoodGood,
    |     ^^^^^^^^
 
 error: variant name starts with the enum's name
-  --> $DIR/enum_variants.rs:28:5
+  --> $DIR/enum_variants.rs:27:5
    |
 LL |     FoodMiddle,
    |     ^^^^^^^^^^
 
 error: variant name starts with the enum's name
-  --> $DIR/enum_variants.rs:29:5
+  --> $DIR/enum_variants.rs:28:5
    |
 LL |     FoodBad,
    |     ^^^^^^^
 
 error: all variants have the same prefix: `Food`
-  --> $DIR/enum_variants.rs:26:1
+  --> $DIR/enum_variants.rs:25:1
    |
 LL | / enum Food {
 LL | |     FoodGood,
@@ -37,7 +37,7 @@ LL | | }
    = help: remove the prefixes and use full paths to the variants instead of glob imports
 
 error: all variants have the same prefix: `CallType`
-  --> $DIR/enum_variants.rs:36:1
+  --> $DIR/enum_variants.rs:35:1
    |
 LL | / enum BadCallType {
 LL | |     CallTypeCall,
@@ -49,7 +49,7 @@ LL | | }
    = help: remove the prefixes and use full paths to the variants instead of glob imports
 
 error: all variants have the same prefix: `Constant`
-  --> $DIR/enum_variants.rs:48:1
+  --> $DIR/enum_variants.rs:47:1
    |
 LL | / enum Consts {
 LL | |     ConstantInt,
@@ -61,7 +61,7 @@ LL | | }
    = help: remove the prefixes and use full paths to the variants instead of glob imports
 
 error: all variants have the same prefix: `With`
-  --> $DIR/enum_variants.rs:82:1
+  --> $DIR/enum_variants.rs:81:1
    |
 LL | / enum Seallll {
 LL | |     WithOutCake,
@@ -73,7 +73,7 @@ LL | | }
    = help: remove the prefixes and use full paths to the variants instead of glob imports
 
 error: all variants have the same prefix: `Prefix`
-  --> $DIR/enum_variants.rs:88:1
+  --> $DIR/enum_variants.rs:87:1
    |
 LL | / enum NonCaps {
 LL | |     Prefix的,
@@ -84,21 +84,8 @@ LL | | }
    |
    = help: remove the prefixes and use full paths to the variants instead of glob imports
 
-error: all variants have the same prefix: `With`
-  --> $DIR/enum_variants.rs:94:1
-   |
-LL | / pub enum PubSeall {
-LL | |     WithOutCake,
-LL | |     WithOutTea,
-LL | |     WithOut,
-LL | | }
-   | |_^
-   |
-   = note: `-D clippy::pub-enum-variant-names` implied by `-D warnings`
-   = help: remove the prefixes and use full paths to the variants instead of glob imports
-
 error: all variants have the same postfix: `IData`
-  --> $DIR/enum_variants.rs:137:1
+  --> $DIR/enum_variants.rs:136:1
    |
 LL | / enum IDataRequest {
 LL | |     PutIData(String),
@@ -110,7 +97,7 @@ LL | | }
    = help: remove the postfixes and use full paths to the variants instead of glob imports
 
 error: all variants have the same postfix: `HIData`
-  --> $DIR/enum_variants.rs:143:1
+  --> $DIR/enum_variants.rs:142:1
    |
 LL | / enum HIDataRequest {
 LL | |     PutHIData(String),
@@ -121,5 +108,5 @@ LL | | }
    |
    = help: remove the postfixes and use full paths to the variants instead of glob imports
 
-error: aborting due to 12 previous errors
+error: aborting due to 11 previous errors
 
