]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/enum_variants.rs
modify code
[rust.git] / clippy_lints / src / enum_variants.rs
index 0ecc0bc3eb60a3b2d5f1bfbf3d9fcbb17451618b..1f4353fa4f72bd73a6028f80c49e71d1b552230e 100644 (file)
@@ -1,24 +1,30 @@
 //! lint on enum variants that are prefixed or suffixed by the same characters
 
-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 clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start};
+use rustc_hir::{EnumDef, Item, ItemKind, Variant};
+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;
 
 declare_clippy_lint! {
-    /// **What it does:** Detects enumeration variants that are prefixed or suffixed
+    /// ### What it does
+    /// Detects enumeration variants that are prefixed or suffixed
     /// by the same characters.
     ///
-    /// **Why is this bad?** Enumeration variant names should specify their variant,
+    /// ### Why is this bad?
+    /// Enumeration variant names should specify their variant,
     /// not repeat the enumeration name.
     ///
-    /// **Known problems:** None.
+    /// ### Limitations
+    /// Characters with no casing will be considered when comparing prefixes/suffixes
+    /// This applies to numbers and non-ascii characters without casing
+    /// e.g. `Foo1` and `Foo2` is considered to have different prefixes
+    /// (the prefixes are `Foo1` and `Foo2` respectively), as also `Bar螃`, `Bar蟹`
     ///
-    /// **Example:**
+    /// ### Example
     /// ```rust
     /// enum Cake {
     ///     BlackForestCake,
     ///     Battenberg,
     /// }
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub ENUM_VARIANT_NAMES,
     style,
     "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
+    /// ### What it does
+    /// Detects type names that are prefixed or suffixed by the
     /// containing module's name.
     ///
-    /// **Why is this bad?** It requires the user to type the module name twice.
-    ///
-    /// **Known problems:** None.
+    /// ### Why is this bad?
+    /// It requires the user to type the module name twice.
     ///
-    /// **Example:**
+    /// ### Example
     /// ```rust
     /// mod cake {
     ///     struct BlackForestCake;
     ///     struct BlackForest;
     /// }
     /// ```
+    #[clippy::version = "1.33.0"]
     pub MODULE_NAME_REPETITIONS,
     pedantic,
     "type names prefixed/postfixed with their containing module's name"
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for modules that have the same name as their
+    /// ### What it does
+    /// Checks for modules that have the same name as their
     /// parent module
     ///
-    /// **Why is this bad?** A typical beginner mistake is to have `mod foo;` and
+    /// ### Why is this bad?
+    /// A typical beginner mistake is to have `mod foo;` and
     /// again `mod foo { ..
     /// }` in `foo.rs`.
     /// The expectation is that items inside the inner `mod foo { .. }` are then
     /// If this is done on purpose, it would be better to choose a more
     /// representative module name.
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
+    /// ### Example
     /// ```ignore
     /// // lib.rs
     /// mod foo;
     ///     ...
     /// }
     /// ```
+    #[clippy::version = "pre 1.29.0"]
     pub MODULE_INCEPTION,
     style,
     "modules that have the same name as their parent module"
 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
 ]);
 
-/// Returns the number of chars that match from the start
-#[must_use]
-fn partial_match(pre: &str, name: &str) -> usize {
-    let mut name_iter = name.chars();
-    let _ = name_iter.next_back(); // make sure the name is never fully matched
-    pre.chars().zip(name_iter).take_while(|&(l, r)| l == r).count()
+fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
+    let name = variant.ident.name.as_str();
+    let item_name_chars = item_name.chars().count();
+
+    if count_match_start(item_name, name).char_count == 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,
+            ENUM_VARIANT_NAMES,
+            variant.span,
+            "variant name starts with the enum's name",
+        );
+    }
 }
 
-/// Returns the number of chars that match from the end
-#[must_use]
-fn partial_rmatch(post: &str, name: &str) -> usize {
-    let mut name_iter = name.chars();
-    let _ = name_iter.next(); // make sure the name is never fully matched
-    post.chars()
-        .rev()
-        .zip(name_iter.rev())
-        .take_while(|&(l, r)| l == r)
-        .count()
+fn check_enum_end(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) {
+    let name = variant.ident.name.as_str();
+    let item_name_chars = item_name.chars().count();
+
+    if count_match_end(item_name, name).char_count == item_name_chars {
+        span_lint(
+            cx,
+            ENUM_VARIANT_NAMES,
+            variant.span,
+            "variant name ends with the enum's name",
+        );
+    }
 }
 
-fn check_variant(
-    cx: &EarlyContext<'_>,
-    threshold: u64,
-    def: &EnumDef,
-    item_name: &str,
-    item_name_chars: usize,
-    span: Span,
-    lint: &'static Lint,
-) {
+fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_name: &str, span: Span) {
     if (def.variants.len() as u64) < threshold {
         return;
     }
-    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");
-        }
-        if partial_rmatch(item_name, &name) == item_name_chars {
-            span_lint(cx, lint, 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 {
+    let mut pre = camel_case_split(first);
+    let mut post = pre.clone();
+    post.reverse();
+    for var in def.variants {
+        check_enum_start(cx, item_name, var);
+        check_enum_end(cx, item_name, var);
         let name = var.ident.name.as_str();
 
-        let pre_match = partial_match(pre, &name);
-        pre = &pre[..pre_match];
-        let pre_camel = camel_case::until(pre);
-        pre = &pre[..pre_camel];
-        while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() {
-            if next.is_numeric() {
-                return;
-            }
-            if next.is_lowercase() {
-                let last = pre.len() - last.len_utf8();
-                let last_camel = camel_case::until(&pre[..last]);
-                pre = &pre[..last_camel];
-            } else {
-                break;
-            }
+        let variant_split = camel_case_split(name);
+        if variant_split.len() == 1 {
+            return;
         }
 
-        let post_match = partial_rmatch(post, &name);
-        let post_end = post.len() - post_match;
-        post = &post[post_end..];
-        let post_camel = camel_case::from(post);
-        post = &post[post_camel..];
+        pre = pre
+            .iter()
+            .zip(variant_split.iter())
+            .take_while(|(a, b)| a == b)
+            .map(|e| *e.0)
+            .collect();
+        post = post
+            .iter()
+            .zip(variant_split.iter().rev())
+            .take_while(|(a, b)| a == b)
+            .map(|e| *e.0)
+            .collect();
     }
     let (what, value) = match (pre.is_empty(), post.is_empty()) {
         (true, true) => return,
-        (false, _) => ("pre", pre),
-        (true, false) => ("post", post),
+        (false, _) => ("pre", pre.join("")),
+        (true, false) => {
+            post.reverse();
+            ("post", post.join(""))
+        },
     };
     span_lint_and_help(
         cx,
-        lint,
+        ENUM_VARIANT_NAMES,
         span,
         &format!("all variants have the same {}fix: `{}`", what, value),
         None,
@@ -261,17 +234,16 @@ 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);
+        let item_camel = to_camel_case(item_name);
         if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
             if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
                 // constants don't have surrounding modules
@@ -286,14 +258,16 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
                             );
                         }
                     }
-                    if item.vis.kind.is_pub() {
-                        let matching = partial_match(mod_camel, &item_camel);
-                        let rmatching = partial_rmatch(mod_camel, &item_camel);
+                    // The `module_name_repetitions` lint should only trigger if the item has the module in its
+                    // name. Having the same name is accepted.
+                    if item.vis.node.is_pub() && item_camel.len() > mod_camel.len() {
+                        let matching = count_match_start(mod_camel, &item_camel);
+                        let rmatching = count_match_end(mod_camel, &item_camel);
                         let nchars = mod_camel.chars().count();
 
                         let is_word_beginning = |c: char| c == '_' || c.is_uppercase() || c.is_numeric();
 
-                        if matching == nchars {
+                        if matching.char_count == nchars {
                             match item_camel.chars().nth(nchars) {
                                 Some(c) if is_word_beginning(c) => span_lint(
                                     cx,
@@ -304,7 +278,7 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
                                 _ => (),
                             }
                         }
-                        if rmatching == nchars {
+                        if rmatching.char_count == nchars {
                             span_lint(
                                 cx,
                                 MODULE_NAME_REPETITIONS,
@@ -317,11 +291,9 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
             }
         }
         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.def_id)) {
+                check_variant(cx, self.threshold, def, item_name, item.span);
+            }
         }
         self.modules.push((item.ident.name, item_camel));
     }