diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fb2e9932b8e..4b67c84e38e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1058,7 +1058,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let max_struct_bools = conf.max_struct_bools; store.register_early_pass(move || box excessive_bools::ExcessiveBools::new(max_struct_bools, max_fn_params_bools)); store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap); - store.register_late_pass(|| box wildcard_imports::WildcardImports); + let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports; + store.register_late_pass(move || box wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports)); store.register_early_pass(|| box macro_use::MacroUseImports); store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 4b81ff33495..57b9eafd14d 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -158,6 +158,8 @@ define_Conf! { (max_struct_bools, "max_struct_bools": u64, 3), /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have (max_fn_params_bools, "max_fn_params_bools": u64, 3), + /// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests). + (warn_on_all_wildcard_imports, "warn_on_all_wildcard_imports": bool, false), } impl Default for Conf { diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index a22d0e6775d..43d0d1b9e96 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -6,7 +6,7 @@ use rustc_hir::{ Item, ItemKind, PathSegment, UseKind, }; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::BytePos; declare_clippy_lint! { @@ -73,18 +73,38 @@ declare_clippy_lint! { "lint `use _::*` statements" } -declare_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]); +#[derive(Default)] +pub struct WildcardImports { + warn_on_all: bool, + is_test_module: bool, + test_modules_deep: u32, +} + +impl WildcardImports { + pub fn new(warn_on_all: bool) -> Self { + Self { + warn_on_all, + is_test_module: false, + test_modules_deep: 0, + } + } +} + +impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]); impl LateLintPass<'_, '_> for WildcardImports { fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &Item<'_>) { if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() { return; } + if is_test_module(item) { + self.is_test_module = true; + self.test_modules_deep += 1; + } if_chain! { if !in_macro(item.span); if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind; - if !is_prelude_import(use_path.segments); - if !(is_super_only_import(use_path.segments) && is_in_test_module(cx, item)); + if self.warn_on_all || !self.check_exceptions(use_path.segments); let used_imports = cx.tcx.names_imported_by_glob_use(item.hir_id.owner); if !used_imports.is_empty(); // Already handled by `unused_imports` then { @@ -152,6 +172,19 @@ impl LateLintPass<'_, '_> for WildcardImports { } } } + + fn check_item_post(&mut self, _: &LateContext<'_, '_>, _: &Item<'_>) { + if self.is_test_module { + self.is_test_module = false; + self.test_modules_deep -= 1; + } + } +} + +impl WildcardImports { + fn check_exceptions(&self, segments: &[PathSegment<'_>]) -> bool { + is_prelude_import(segments) || (is_super_only_import(segments) && self.test_modules_deep > 0) + } } // Allow "...prelude::*" imports. @@ -168,9 +201,6 @@ fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool { segments.len() == 1 && segments[0].ident.as_str() == "super" } -fn is_in_test_module(cx: &LateContext<'_, '_>, item: &Item<'_>) -> bool { - let parent = cx.tcx.hir().get_parent_node(item.hir_id); - let parent_item = cx.tcx.hir().expect_item(parent); - let parent_name = parent_item.ident.name.as_str(); - parent_name.contains("test") +fn is_test_module(item: &Item<'_>) -> bool { + item.ident.name.as_str().contains("test") } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 18f5d994ba8..53970af4107 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-struct-bools`, `max-fn-params-bools`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `third-party` at line 5 column 1 error: aborting due to previous error diff --git a/tests/ui/wildcard_imports.fixed b/tests/ui/wildcard_imports.fixed index 1c5c01f65d1..98bf6acfe55 100644 --- a/tests/ui/wildcard_imports.fixed +++ b/tests/ui/wildcard_imports.fixed @@ -156,10 +156,10 @@ fn test_weird_formatting() { foo(); } -mod test_super_imports { +mod super_imports { fn foofoo() {} - mod use_super_should_be_replaced { + mod should_be_replaced { use super::foofoo; fn with_super() { @@ -167,7 +167,7 @@ mod test_super_imports { } } - mod use_super_in_test_should_pass { + mod test_should_pass { use super::*; fn with_super() { @@ -175,8 +175,15 @@ mod test_super_imports { } } + mod inner { + fn test_should_pass() { + use super::*; + let _ = foofoo(); + } + } + mod use_explicit_should_be_replaced { - use test_super_imports::foofoo; + use super_imports::foofoo; fn with_explicit() { let _ = foofoo(); @@ -194,7 +201,7 @@ mod test_super_imports { } mod use_super_explicit_should_be_replaced { - use super::super::test_super_imports::foofoo; + use super::super::super_imports::foofoo; fn with_super_explicit() { let _ = foofoo(); diff --git a/tests/ui/wildcard_imports.rs b/tests/ui/wildcard_imports.rs index f783149ef93..9275c5a0928 100644 --- a/tests/ui/wildcard_imports.rs +++ b/tests/ui/wildcard_imports.rs @@ -157,10 +157,10 @@ fn test_weird_formatting() { foo(); } -mod test_super_imports { +mod super_imports { fn foofoo() {} - mod use_super_should_be_replaced { + mod should_be_replaced { use super::*; fn with_super() { @@ -168,7 +168,7 @@ mod test_super_imports { } } - mod use_super_in_test_should_pass { + mod test_should_pass { use super::*; fn with_super() { @@ -176,8 +176,15 @@ mod test_super_imports { } } + mod inner { + fn test_should_pass() { + use super::*; + let _ = foofoo(); + } + } + mod use_explicit_should_be_replaced { - use test_super_imports::*; + use super_imports::*; fn with_explicit() { let _ = foofoo(); @@ -195,7 +202,7 @@ mod test_super_imports { } mod use_super_explicit_should_be_replaced { - use super::super::test_super_imports::*; + use super::super::super_imports::*; fn with_super_explicit() { let _ = foofoo(); diff --git a/tests/ui/wildcard_imports.stderr b/tests/ui/wildcard_imports.stderr index 649d550a88d..bd000ce8161 100644 --- a/tests/ui/wildcard_imports.stderr +++ b/tests/ui/wildcard_imports.stderr @@ -99,22 +99,22 @@ LL | use super::*; | ^^^^^^^^ help: try: `super::foofoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:180:13 + --> $DIR/wildcard_imports.rs:187:13 | -LL | use test_super_imports::*; - | ^^^^^^^^^^^^^^^^^^^^^ help: try: `test_super_imports::foofoo` +LL | use super_imports::*; + | ^^^^^^^^^^^^^^^^ help: try: `super_imports::foofoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:189:17 + --> $DIR/wildcard_imports.rs:196:17 | LL | use super::super::*; | ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:198:13 + --> $DIR/wildcard_imports.rs:205:13 | -LL | use super::super::test_super_imports::*; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::test_super_imports::foofoo` +LL | use super::super::super_imports::*; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` error: aborting due to 19 previous errors