Remove is_test_module_or_function and use is_in_test instead.

This commit is contained in:
Jason Newcomb 2024-06-09 22:11:47 -04:00
parent d2400a49a4
commit 4c44b4e3c8
11 changed files with 47 additions and 64 deletions

View File

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint; use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_test_module_or_function; use clippy_utils::is_in_test;
use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{Item, Pat, PatKind}; use rustc_hir::{Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass; use rustc_session::impl_lint_pass;
@ -27,52 +27,30 @@
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct DisallowedNames { pub struct DisallowedNames {
disallow: FxHashSet<String>, disallow: FxHashSet<String>,
test_modules_deep: u32,
} }
impl DisallowedNames { impl DisallowedNames {
pub fn new(disallowed_names: &[String]) -> Self { pub fn new(disallowed_names: &[String]) -> Self {
Self { Self {
disallow: disallowed_names.iter().cloned().collect(), disallow: disallowed_names.iter().cloned().collect(),
test_modules_deep: 0,
} }
} }
fn in_test_module(&self) -> bool {
self.test_modules_deep != 0
}
} }
impl_lint_pass!(DisallowedNames => [DISALLOWED_NAMES]); impl_lint_pass!(DisallowedNames => [DISALLOWED_NAMES]);
impl<'tcx> LateLintPass<'tcx> for DisallowedNames { impl<'tcx> LateLintPass<'tcx> for DisallowedNames {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_add(1);
}
}
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
// Check whether we are under the `test` attribute. if let PatKind::Binding(.., ident, _) = pat.kind
if self.in_test_module() { && self.disallow.contains(&ident.name.to_string())
return; && !is_in_test(cx.tcx, pat.hir_id)
} {
span_lint(
if let PatKind::Binding(.., ident, _) = pat.kind { cx,
if self.disallow.contains(&ident.name.to_string()) { DISALLOWED_NAMES,
span_lint( ident.span,
cx, format!("use of a disallowed/placeholder name `{}`", ident.name),
DISALLOWED_NAMES, );
ident.span,
format!("use of a disallowed/placeholder name `{}`", ident.name),
);
}
}
}
fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_sub(1);
} }
} }
} }

View File

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_test_module_or_function; use clippy_utils::is_in_test;
use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::source::{snippet, snippet_with_applicability};
use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -100,7 +100,6 @@
#[derive(Default)] #[derive(Default)]
pub struct WildcardImports { pub struct WildcardImports {
warn_on_all: bool, warn_on_all: bool,
test_modules_deep: u32,
allowed_segments: FxHashSet<String>, allowed_segments: FxHashSet<String>,
} }
@ -108,7 +107,6 @@ impl WildcardImports {
pub fn new(warn_on_all: bool, allowed_wildcard_imports: FxHashSet<String>) -> Self { pub fn new(warn_on_all: bool, allowed_wildcard_imports: FxHashSet<String>) -> Self {
Self { Self {
warn_on_all, warn_on_all,
test_modules_deep: 0,
allowed_segments: allowed_wildcard_imports, allowed_segments: allowed_wildcard_imports,
} }
} }
@ -122,15 +120,12 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
return; return;
} }
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_add(1);
}
let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id); let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id);
if cx.tcx.visibility(item.owner_id.def_id) != ty::Visibility::Restricted(module.to_def_id()) { if cx.tcx.visibility(item.owner_id.def_id) != ty::Visibility::Restricted(module.to_def_id()) {
return; return;
} }
if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind
&& (self.warn_on_all || !self.check_exceptions(item, use_path.segments)) && (self.warn_on_all || !self.check_exceptions(cx, item, use_path.segments))
&& let used_imports = cx.tcx.names_imported_by_glob_use(item.owner_id.def_id) && let used_imports = cx.tcx.names_imported_by_glob_use(item.owner_id.def_id)
&& !used_imports.is_empty() // Already handled by `unused_imports` && !used_imports.is_empty() // Already handled by `unused_imports`
&& !used_imports.contains(&kw::Underscore) && !used_imports.contains(&kw::Underscore)
@ -180,20 +175,14 @@ fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
span_lint_and_sugg(cx, lint, span, message, "try", sugg, applicability); span_lint_and_sugg(cx, lint, span, message, "try", sugg, applicability);
} }
} }
fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if is_test_module_or_function(cx.tcx, item) {
self.test_modules_deep = self.test_modules_deep.saturating_sub(1);
}
}
} }
impl WildcardImports { impl WildcardImports {
fn check_exceptions(&self, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool { fn check_exceptions(&self, cx: &LateContext<'_>, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool {
item.span.from_expansion() item.span.from_expansion()
|| is_prelude_import(segments) || is_prelude_import(segments)
|| (is_super_only_import(segments) && self.test_modules_deep > 0)
|| is_allowed_via_config(segments, &self.allowed_segments) || is_allowed_via_config(segments, &self.allowed_segments)
|| (is_super_only_import(segments) && is_in_test(cx.tcx, item.hir_id()))
} }
} }

View File

@ -60,6 +60,7 @@ fn issue_1647_ref_mut() {
//~^ ERROR: use of a disallowed/placeholder name `quux` //~^ ERROR: use of a disallowed/placeholder name `quux`
} }
#[cfg(test)]
mod tests { mod tests {
fn issue_7305() { fn issue_7305() {
// `disallowed_names` lint should not be triggered inside of the test code. // `disallowed_names` lint should not be triggered inside of the test code.

View File

@ -204,6 +204,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass { mod test_should_pass {
use super::*; use super::*;
@ -212,6 +213,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass_inside_function { mod test_should_pass_inside_function {
fn with_super_inside_function() { fn with_super_inside_function() {
use super::*; use super::*;
@ -219,6 +221,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass_further_inside { mod test_should_pass_further_inside {
fn insidefoo() {} fn insidefoo() {}
mod inner { mod inner {

View File

@ -205,6 +205,7 @@ fn with_super() {
} }
} }
#[cfg(test)]
mod test_should_pass { mod test_should_pass {
use super::*; use super::*;
@ -213,6 +214,7 @@ fn with_super() {
} }
} }
#[cfg(test)]
mod test_should_pass_inside_function { mod test_should_pass_inside_function {
fn with_super_inside_function() { fn with_super_inside_function() {
use super::*; use super::*;
@ -220,6 +222,7 @@ fn with_super_inside_function() {
} }
} }
#[cfg(test)]
mod test_should_pass_further_inside { mod test_should_pass_further_inside {
fn insidefoo() {} fn insidefoo() {}
mod inner { mod inner {

View File

@ -106,31 +106,31 @@ LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo` | ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:236:17 --> tests/ui/wildcard_imports.rs:239:17
| |
LL | use super::*; LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo` | ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:244:13 --> tests/ui/wildcard_imports.rs:247:13
| |
LL | use crate::super_imports::*; LL | use crate::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:253:17 --> tests/ui/wildcard_imports.rs:256:17
| |
LL | use super::super::*; LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo` | ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:262:13 --> tests/ui/wildcard_imports.rs:265:13
| |
LL | use super::super::super_imports::*; LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports.rs:270:13 --> tests/ui/wildcard_imports.rs:273:13
| |
LL | use super::*; LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo` | ^^^^^^^^ help: try: `super::foofoo`

View File

@ -198,6 +198,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass { mod test_should_pass {
use super::*; use super::*;
@ -206,6 +207,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass_inside_function { mod test_should_pass_inside_function {
fn with_super_inside_function() { fn with_super_inside_function() {
use super::*; use super::*;
@ -213,6 +215,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass_further_inside { mod test_should_pass_further_inside {
fn insidefoo() {} fn insidefoo() {}
mod inner { mod inner {

View File

@ -106,31 +106,31 @@ LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo` | ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:230:17 --> tests/ui/wildcard_imports_2021.rs:233:17
| |
LL | use super::*; LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo` | ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:238:13 --> tests/ui/wildcard_imports_2021.rs:241:13
| |
LL | use crate::super_imports::*; LL | use crate::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:247:17 --> tests/ui/wildcard_imports_2021.rs:250:17
| |
LL | use super::super::*; LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo` | ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:256:13 --> tests/ui/wildcard_imports_2021.rs:259:13
| |
LL | use super::super::super_imports::*; LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:264:13 --> tests/ui/wildcard_imports_2021.rs:267:13
| |
LL | use super::*; LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo` | ^^^^^^^^ help: try: `super::foofoo`

View File

@ -198,6 +198,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass { mod test_should_pass {
use super::*; use super::*;
@ -206,6 +207,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass_inside_function { mod test_should_pass_inside_function {
fn with_super_inside_function() { fn with_super_inside_function() {
use super::*; use super::*;
@ -213,6 +215,7 @@ mod super_imports {
} }
} }
#[cfg(test)]
mod test_should_pass_further_inside { mod test_should_pass_further_inside {
fn insidefoo() {} fn insidefoo() {}
mod inner { mod inner {

View File

@ -106,31 +106,31 @@ LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo` | ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:230:17 --> tests/ui/wildcard_imports_2021.rs:233:17
| |
LL | use super::*; LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo` | ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:238:13 --> tests/ui/wildcard_imports_2021.rs:241:13
| |
LL | use crate::super_imports::*; LL | use crate::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo` | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::super_imports::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:247:17 --> tests/ui/wildcard_imports_2021.rs:250:17
| |
LL | use super::super::*; LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo` | ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:256:13 --> tests/ui/wildcard_imports_2021.rs:259:13
| |
LL | use super::super::super_imports::*; LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: usage of wildcard import error: usage of wildcard import
--> tests/ui/wildcard_imports_2021.rs:264:13 --> tests/ui/wildcard_imports_2021.rs:267:13
| |
LL | use super::*; LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo` | ^^^^^^^^ help: try: `super::foofoo`

View File

@ -199,6 +199,7 @@ fn with_super() {
} }
} }
#[cfg(test)]
mod test_should_pass { mod test_should_pass {
use super::*; use super::*;
@ -207,6 +208,7 @@ fn with_super() {
} }
} }
#[cfg(test)]
mod test_should_pass_inside_function { mod test_should_pass_inside_function {
fn with_super_inside_function() { fn with_super_inside_function() {
use super::*; use super::*;
@ -214,6 +216,7 @@ fn with_super_inside_function() {
} }
} }
#[cfg(test)]
mod test_should_pass_further_inside { mod test_should_pass_further_inside {
fn insidefoo() {} fn insidefoo() {}
mod inner { mod inner {