From 314bddee95f88a405ec4b44db5576e3745ee1699 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Mon, 29 Jan 2024 11:29:15 +0800 Subject: [PATCH] add more test cases & improve docs & replace `Vec` with `FxHashSet` for segments --- clippy_config/src/conf.rs | 17 +++++++++++++--- clippy_lints/src/wildcard_imports.rs | 10 ++++------ tests/ui-toml/wildcard_imports/clippy.toml | 3 +++ .../wildcard_imports/wildcard_imports.fixed | 19 ++++++++++++++++++ .../wildcard_imports/wildcard_imports.rs | 19 ++++++++++++++++++ .../wildcard_imports/wildcard_imports.stderr | 20 +++++++++++++++---- .../wildcard_imports.fixed | 8 ++++++++ .../wildcard_imports.rs | 8 ++++++++ .../wildcard_imports.stderr | 2 +- 9 files changed, 92 insertions(+), 14 deletions(-) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index e4e512482ac03..d03f805e2fc96 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -573,9 +573,20 @@ define_Conf! { (allow_comparison_to_zero: bool = true), /// Lint: WILDCARD_IMPORTS. /// - /// List of path segments to ignore when checking wildcard imports, - /// could get overrided by `warn_on_all_wildcard_imports`. - (ignored_wildcard_imports: Vec = Vec::new()), + /// List of path segments to ignore when checking wildcard imports. + /// + /// #### Example + /// + /// ```toml + /// ignored-wildcard-imports = [ "utils", "common" ] + /// ``` + /// + /// #### Noteworthy + /// + /// 1. This configuration has no effects if used with `warn_on_all_wildcard_imports = true`. + /// 2. Paths with any segment that containing the word 'prelude' + /// are already ignored by default. + (ignored_wildcard_imports: FxHashSet = FxHashSet::default()), } /// Search for the configuration file. diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 4eb45e6f8da52..7318ffc958751 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -101,11 +101,11 @@ declare_clippy_lint! { pub struct WildcardImports { warn_on_all: bool, test_modules_deep: u32, - ignored_segments: Vec, + ignored_segments: FxHashSet, } impl WildcardImports { - pub fn new(warn_on_all: bool, ignored_wildcard_imports: Vec) -> Self { + pub fn new(warn_on_all: bool, ignored_wildcard_imports: FxHashSet) -> Self { Self { warn_on_all, test_modules_deep: 0, @@ -212,10 +212,8 @@ fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool { // Allow skipping imports containing user configured segments, // i.e. "...::utils::...::*" if user put `ignored-wildcard-imports = ["utils"]` in `Clippy.toml` -fn is_ignored_via_config(segments: &[PathSegment<'_>], ignored_segments: &[String]) -> bool { - let segments_set: FxHashSet<&str> = segments.iter().map(|s| s.ident.as_str()).collect(); - let ignored_set: FxHashSet<&str> = ignored_segments.iter().map(String::as_str).collect(); +fn is_ignored_via_config(segments: &[PathSegment<'_>], ignored_segments: &FxHashSet) -> bool { // segment matching need to be exact instead of using 'contains', in case user unintentionaly put // a single character in the config thus skipping most of the warnings. - segments_set.intersection(&ignored_set).next().is_some() + segments.iter().any(|seg| ignored_segments.contains(seg.ident.as_str())) } diff --git a/tests/ui-toml/wildcard_imports/clippy.toml b/tests/ui-toml/wildcard_imports/clippy.toml index 875aaeef6c935..56a945e48bb21 100644 --- a/tests/ui-toml/wildcard_imports/clippy.toml +++ b/tests/ui-toml/wildcard_imports/clippy.toml @@ -1 +1,4 @@ warn-on-all-wildcard-imports = true + +# This should be ignored since `warn-on-all-wildcard-imports` has higher precedence +ignored-wildcard-imports = ["utils"] diff --git a/tests/ui-toml/wildcard_imports/wildcard_imports.fixed b/tests/ui-toml/wildcard_imports/wildcard_imports.fixed index 1752f48856c2b..af72d6be0e096 100644 --- a/tests/ui-toml/wildcard_imports/wildcard_imports.fixed +++ b/tests/ui-toml/wildcard_imports/wildcard_imports.fixed @@ -3,9 +3,28 @@ mod prelude { pub const FOO: u8 = 1; } + +mod utils { + pub const BAR: u8 = 1; + pub fn print() {} +} + +mod my_crate { + pub mod utils { + pub fn my_util_fn() {} + } +} + +use utils::{BAR, print}; +//~^ ERROR: usage of wildcard import +use my_crate::utils::my_util_fn; +//~^ ERROR: usage of wildcard import use prelude::FOO; //~^ ERROR: usage of wildcard import fn main() { let _ = FOO; + let _ = BAR; + print(); + my_util_fn(); } diff --git a/tests/ui-toml/wildcard_imports/wildcard_imports.rs b/tests/ui-toml/wildcard_imports/wildcard_imports.rs index 331c2c59c222f..91009dd8835f8 100644 --- a/tests/ui-toml/wildcard_imports/wildcard_imports.rs +++ b/tests/ui-toml/wildcard_imports/wildcard_imports.rs @@ -3,9 +3,28 @@ mod prelude { pub const FOO: u8 = 1; } + +mod utils { + pub const BAR: u8 = 1; + pub fn print() {} +} + +mod my_crate { + pub mod utils { + pub fn my_util_fn() {} + } +} + +use utils::*; +//~^ ERROR: usage of wildcard import +use my_crate::utils::*; +//~^ ERROR: usage of wildcard import use prelude::*; //~^ ERROR: usage of wildcard import fn main() { let _ = FOO; + let _ = BAR; + print(); + my_util_fn(); } diff --git a/tests/ui-toml/wildcard_imports/wildcard_imports.stderr b/tests/ui-toml/wildcard_imports/wildcard_imports.stderr index f11fda6a0c6e1..a733d786d0e64 100644 --- a/tests/ui-toml/wildcard_imports/wildcard_imports.stderr +++ b/tests/ui-toml/wildcard_imports/wildcard_imports.stderr @@ -1,11 +1,23 @@ error: usage of wildcard import - --> $DIR/wildcard_imports.rs:6:5 + --> $DIR/wildcard_imports.rs:18:5 | -LL | use prelude::*; - | ^^^^^^^^^^ help: try: `prelude::FOO` +LL | use utils::*; + | ^^^^^^^^ help: try: `utils::{BAR, print}` | = note: `-D clippy::wildcard-imports` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::wildcard_imports)]` -error: aborting due to 1 previous error +error: usage of wildcard import + --> $DIR/wildcard_imports.rs:20:5 + | +LL | use my_crate::utils::*; + | ^^^^^^^^^^^^^^^^^^ help: try: `my_crate::utils::my_util_fn` + +error: usage of wildcard import + --> $DIR/wildcard_imports.rs:22:5 + | +LL | use prelude::*; + | ^^^^^^^^^^ help: try: `prelude::FOO` + +error: aborting due to 3 previous errors diff --git a/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.fixed b/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.fixed index 5ef30b054b3d3..d539525c45ed6 100644 --- a/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.fixed +++ b/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.fixed @@ -8,11 +8,19 @@ mod utils_plus { pub fn do_something() {} } +mod my_crate { + pub mod utils { + pub fn my_util_fn() {} + } +} + +use my_crate::utils::*; use utils::*; use utils_plus::do_something; //~^ ERROR: usage of wildcard import fn main() { print(); + my_util_fn(); do_something(); } diff --git a/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.rs b/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.rs index 5ea91a8d70aea..9169d16a08be1 100644 --- a/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.rs +++ b/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.rs @@ -8,11 +8,19 @@ mod utils_plus { pub fn do_something() {} } +mod my_crate { + pub mod utils { + pub fn my_util_fn() {} + } +} + +use my_crate::utils::*; use utils::*; use utils_plus::*; //~^ ERROR: usage of wildcard import fn main() { print(); + my_util_fn(); do_something(); } diff --git a/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.stderr b/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.stderr index 15f522cb5af1c..12c2f9cbada70 100644 --- a/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.stderr +++ b/tests/ui-toml/wildcard_imports_whitelist/wildcard_imports.stderr @@ -1,5 +1,5 @@ error: usage of wildcard import - --> $DIR/wildcard_imports.rs:12:5 + --> $DIR/wildcard_imports.rs:19:5 | LL | use utils_plus::*; | ^^^^^^^^^^^^^ help: try: `utils_plus::do_something`