Skip to content

Commit

Permalink
Recognize common prefixes when checking for items with module name su…
Browse files Browse the repository at this point in the history
…ffix

Fixes #12544.

- 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`).
- configured by `allowed-prefixes` config entry
- prefixes allowed by default: [`to`, `from`, `into`, `as`, `try_into`, `try_from`]
- update docs
  • Loading branch information
modelflat committed Apr 9, 2024
1 parent 3787a0c commit 3705073
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
22 changes: 22 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion clippy_lints/src/item_name_repetitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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 => [
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: _,
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allowed-prefixes = ["bar"]
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allowed-prefixes = ["..", "bar"]
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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

3 changes: 3 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/module_name_repetitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

0 comments on commit 3705073

Please sign in to comment.