From ad92486d52fdede5c712dd27c868c942d8240704 Mon Sep 17 00:00:00 2001
From: Glenn Hope <glenn.alexander.hope@gmail.com>
Date: Thu, 7 May 2020 14:41:54 -0700
Subject: [PATCH] Add check for "test" in parent name. Include flag for
disabling wildcard import exceptions
---
clippy_lints/src/lib.rs | 3 +-
clippy_lints/src/utils/conf.rs | 2 +
clippy_lints/src/wildcard_imports.rs | 48 +++++++++++++++----
.../toml_unknown_key/conf_unknown_key.stderr | 2 +-
tests/ui/wildcard_imports.fixed | 17 +++++--
tests/ui/wildcard_imports.rs | 17 +++++--
tests/ui/wildcard_imports.stderr | 14 +++---
7 files changed, 75 insertions(+), 28 deletions(-)
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