Auto merge of rust-lang#12573 - modelflat:recognize-common-prepositions-in-module-name-repetitions, r=Jarcho

[`module_name_repetition`] Recognize common prepositions

Fixes rust-lang#12544

changelog: [`module_name_repetition`]: don't report an item name if it consists only of a prefix from `allowed-prefixes` list and a module name (e.g. `AsFoo` in module `foo`). Prefixes allowed by default: [`to`, `from`, `into`, `as`, `try_into`, `try_from`]

Author: bors

CHANGELOG.md

@@ -5894,6 +5894,7 @@ Released 2018-09-13
 [`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles
 [`allowed-duplicate-crates`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-duplicate-crates
 [`allowed-idents-below-min-chars`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-idents-below-min-chars
+[`allowed-prefixes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-prefixes
 [`allowed-scripts`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-scripts
 [`allowed-wildcard-imports`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-wildcard-imports
 [`arithmetic-side-effects-allowed`]: https://doc.rust-lang.org/clippy/lint_configuration.html#arithmetic-side-effects-allowed

book/src/lint_configuration.md

@@ -164,6 +164,32 @@ configuration of Clippy. By default, any configuration will replace the default
 * [`min_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars)
 
 
+## `allowed-prefixes`
+List of prefixes to allow when determining whether an item's name ends with the module's name.
+If the rest of an item's name is an allowed prefix (e.g. item `ToFoo` or `to_foo` in module `foo`),
+then don't emit a warning.
+
+#### Example
+
+```toml
+allowed-prefixes = [ "to", "from" ]
+```
+
+#### Noteworthy
+
+- By default, the following prefixes are allowed: `to`, `as`, `into`, `from`, `try_into` and `try_from`
+- PascalCase variant is included automatically for each snake_case variant (e.g. if `try_into` is included,
+  `TryInto` will also be included)
+- Use `".."` as part of the list to indicate that the configured values should be appended to the
+default configuration of Clippy. By default, any configuration will replace the default value
+
+**Default Value:** `["to", "as", "into", "from", "try_into", "try_from"]`
+
+---
+**Affected lints:**
+* [`module_name_repetitions`](https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions)
+
+
 ## `allowed-scripts`
 The list of unicode scripts allowed to be used in the scope.
 

clippy_config/src/conf.rs

@@ -39,6 +39,7 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[
 ];
 const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"];
 const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "w", "n"];
+const DEFAULT_ALLOWED_PREFIXES: &[&str] = &["to", "as", "into", "from", "try_into", "try_from"];
 
 /// Conf with parse errors
 #[derive(Default)]
@@ -589,6 +590,26 @@ define_Conf! {
     /// 2. Paths with any segment that containing the word 'prelude'
     /// are already allowed by default.
     (allowed_wildcard_imports: FxHashSet<String> = FxHashSet::default()),
+    /// Lint: MODULE_NAME_REPETITIONS.
+    ///
+    /// List of prefixes to allow when determining whether an item's name ends with the module's name.
+    /// If the rest of an item's name is an allowed prefix (e.g. item `ToFoo` or `to_foo` in module `foo`),
+    /// then don't emit a warning.
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// allowed-prefixes = [ "to", "from" ]
+    /// ```
+    ///
+    /// #### Noteworthy
+    ///
+    /// - By default, the following prefixes are allowed: `to`, `as`, `into`, `from`, `try_into` and `try_from`
+    /// - PascalCase variant is included automatically for each snake_case variant (e.g. if `try_into` is included,
+    ///   `TryInto` will also be included)
+    /// - Use `".."` as part of the list to indicate that the configured values should be appended to the
+    /// default configuration of Clippy. By default, any configuration will replace the default value
+    (allowed_prefixes: Vec<String> = DEFAULT_ALLOWED_PREFIXES.iter().map(ToString::to_string).collect()),
 }
 
 /// Search for the configuration file.
@@ -649,6 +670,7 @@ fn deserialize(file: &SourceFile) -> TryConf {
         Ok(mut conf) => {
             extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS);
             extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES);
+            extend_vec_if_indicator_present(&mut conf.conf.allowed_prefixes, DEFAULT_ALLOWED_PREFIXES);
             // TODO: THIS SHOULD BE TESTED, this comment will be gone soon
             if conf.conf.allowed_idents_below_min_chars.contains("..") {
                 conf.conf

clippy_lints/src/item_name_repetitions.rs

@@ -5,6 +5,7 @@ use clippy_utils::is_bool;
 use clippy_utils::macros::span_is_local;
 use clippy_utils::source::is_present_in_source;
 use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, Variant, VariantData};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
@@ -147,6 +148,7 @@ pub struct ItemNameRepetitions {
     struct_threshold: u64,
     avoid_breaking_exported_api: bool,
     allow_private_module_inception: bool,
+    allowed_prefixes: FxHashSet<String>,
 }
 
 impl ItemNameRepetitions {
@@ -156,15 +158,21 @@ impl ItemNameRepetitions {
         struct_threshold: u64,
         avoid_breaking_exported_api: bool,
         allow_private_module_inception: bool,
+        allowed_prefixes: &[String],
     ) -> Self {
         Self {
             modules: Vec::new(),
             enum_threshold,
             struct_threshold,
             avoid_breaking_exported_api,
             allow_private_module_inception,
+            allowed_prefixes: allowed_prefixes.iter().map(|s| to_camel_case(s)).collect(),
         }
     }
+
+    fn is_allowed_prefix(&self, prefix: &str) -> bool {
+        self.allowed_prefixes.contains(prefix)
+    }
 }
 
 impl_lint_pass!(ItemNameRepetitions => [
@@ -423,7 +431,9 @@ impl LateLintPass<'_> for ItemNameRepetitions {
                                 _ => (),
                             }
                         }
-                        if rmatching.char_count == nchars {
+                        if rmatching.char_count == nchars
+                            && !self.is_allowed_prefix(&item_camel[..item_camel.len() - rmatching.byte_count])
+                        {
                             span_lint(
                                 cx,
                                 MODULE_NAME_REPETITIONS,

clippy_lints/src/lib.rs

@@ -594,6 +594,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         pub_underscore_fields_behavior,
         ref allowed_duplicate_crates,
         allow_comparison_to_zero,
+        ref allowed_prefixes,
 
         blacklisted_names: _,
         cyclomatic_complexity_threshold: _,
@@ -864,6 +865,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
             struct_field_name_threshold,
             avoid_breaking_exported_api,
             allow_private_module_inception,
+            allowed_prefixes,
         ))
     });
     store.register_early_pass(|| Box::new(tabs_in_doc_comments::TabsInDocComments));

tests/ui-toml/item_name_repetitions/allowed_prefixes/clippy.toml

@@ -0,0 +1 @@
+allowed-prefixes = ["bar"]

tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs

@@ -0,0 +1,15 @@
+#![warn(clippy::module_name_repetitions)]
+#![allow(dead_code)]
+
+mod foo {
+    // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
+    // In this test, allowed prefixes are configured to be ["bar"].
+
+    // this line should produce a warning:
+    pub fn to_foo() {}
+
+    // but this line shouldn't
+    pub fn bar_foo() {}
+}
+
+fn main() {}

tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.stderr

@@ -0,0 +1,11 @@
+error: item name ends with its containing module's name
+  --> tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs:9:12
+   |
+LL |     pub fn to_foo() {}
+   |            ^^^^^^
+   |
+   = note: `-D clippy::module-name-repetitions` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::module_name_repetitions)]`
+
+error: aborting due to 1 previous error
+

tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/clippy.toml

@@ -0,0 +1 @@
+allowed-prefixes = ["..", "bar"]

tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs

@@ -0,0 +1,21 @@
+#![warn(clippy::module_name_repetitions)]
+#![allow(dead_code)]
+
+mod foo {
+    // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
+    // In this test, allowed prefixes are configured to be all of the default prefixes and ["bar"].
+
+    // this line should produce a warning:
+    pub fn something_foo() {}
+
+    // but none of the following should:
+    pub fn bar_foo() {}
+    pub fn to_foo() {}
+    pub fn as_foo() {}
+    pub fn into_foo() {}
+    pub fn from_foo() {}
+    pub fn try_into_foo() {}
+    pub fn try_from_foo() {}
+}
+
+fn main() {}

tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.stderr

@@ -0,0 +1,11 @@
+error: item name ends with its containing module's name
+  --> tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs:9:12
+   |
+LL |     pub fn something_foo() {}
+   |            ^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::module-name-repetitions` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::module_name_repetitions)]`
+
+error: aborting due to 1 previous error
+

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -14,6 +14,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            allowed-dotfiles
            allowed-duplicate-crates
            allowed-idents-below-min-chars
+           allowed-prefixes
            allowed-scripts
            allowed-wildcard-imports
            arithmetic-side-effects-allowed
@@ -93,6 +94,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            allowed-dotfiles
            allowed-duplicate-crates
            allowed-idents-below-min-chars
+           allowed-prefixes
            allowed-scripts
            allowed-wildcard-imports
            arithmetic-side-effects-allowed
@@ -172,6 +174,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            allowed-dotfiles
            allowed-duplicate-crates
            allowed-idents-below-min-chars
+           allowed-prefixes
            allowed-scripts
            allowed-wildcard-imports
            arithmetic-side-effects-allowed

tests/ui/module_name_repetitions.rs

@@ -19,6 +19,20 @@ mod foo {
 
     // Should not warn
     pub struct Foobar;
+
+    // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name.
+    pub fn to_foo() {}
+    pub fn into_foo() {}
+    pub fn as_foo() {}
+    pub fn from_foo() {}
+    pub fn try_into_foo() {}
+    pub fn try_from_foo() {}
+    pub trait IntoFoo {}
+    pub trait ToFoo {}
+    pub trait AsFoo {}
+    pub trait FromFoo {}
+    pub trait TryIntoFoo {}
+    pub trait TryFromFoo {}
 }
 
 fn main() {}