Skip to content

Commit

Permalink
suggest adding a #[cfg(test)] to test modules
Browse files Browse the repository at this point in the history
remove a empty line

import `module_to_string`

use `contains("test")`

show a suggestion in case module starts_with/ends_with "test"

replace `parent` with `containing`
  • Loading branch information
TaKO8Ki committed Dec 16, 2021
1 parent 3ee016a commit 6f8ad6d
Show file tree
Hide file tree
Showing 6 changed files with 264 additions and 4 deletions.
10 changes: 9 additions & 1 deletion compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,14 +671,22 @@ pub trait LintContext: Sized {
BuiltinLintDiagnostics::UnknownCrateTypes(span, note, sugg) => {
db.span_suggestion(span, &note, sugg, Applicability::MaybeIncorrect);
}
BuiltinLintDiagnostics::UnusedImports(message, replaces) => {
BuiltinLintDiagnostics::UnusedImports(message, replaces, in_test_module) => {
if !replaces.is_empty() {
db.tool_only_multipart_suggestion(
&message,
replaces,
Applicability::MachineApplicable,
);
}

if let Some(span) = in_test_module {
let def_span = self.sess().source_map().guess_head_span(span);
db.span_help(
span.shrink_to_lo().to(def_span),
"consider adding a `#[cfg(test)]` to the containing module",
);
}
}
BuiltinLintDiagnostics::RedundantImport(spans, ident) => {
for (span, is_imported) in spans {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ pub enum BuiltinLintDiagnostics {
ProcMacroDeriveResolutionFallback(Span),
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
UnknownCrateTypes(Span, String, String),
UnusedImports(String, Vec<(Span, String)>),
UnusedImports(String, Vec<(Span, String)>, Option<Span>),
RedundantImport(Vec<(Span, bool)>, Ident),
DeprecatedMacro(Option<Symbol>, Span),
MissingAbi(Span, Abi),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'a> Resolver<'a> {
/// Reachable macros with block module parents exist due to `#[macro_export] macro_rules!`,
/// but they cannot use def-site hygiene, so the assumption holds
/// (<https://github.com/rust-lang/rust/pull/77984#issuecomment-712445508>).
fn get_nearest_non_block_module(&mut self, mut def_id: DefId) -> Module<'a> {
crate fn get_nearest_non_block_module(&mut self, mut def_id: DefId) -> Module<'a> {
loop {
match self.get_module(def_id) {
Some(module) => return module,
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
// in the last step

use crate::imports::ImportKind;
use crate::module_to_string;
use crate::Resolver;

use rustc_ast as ast;
Expand Down Expand Up @@ -314,12 +315,29 @@ impl Resolver<'_> {
"remove the unused import"
};

let parent_module = visitor.r.get_nearest_non_block_module(
visitor.r.local_def_id(unused.use_tree_id).to_def_id(),
);
let test_module_span = match module_to_string(parent_module) {
Some(module)
if module == "test"
|| module == "tests"
|| module.starts_with("test_")
|| module.starts_with("tests_")
|| module.ends_with("_test")
|| module.ends_with("_tests") =>
{
Some(parent_module.span)
}
_ => None,
};

visitor.r.lint_buffer.buffer_lint_with_diagnostic(
UNUSED_IMPORTS,
unused.use_tree_id,
ms,
&msg,
BuiltinLintDiagnostics::UnusedImports(fix_msg.into(), fixes),
BuiltinLintDiagnostics::UnusedImports(fix_msg.into(), fixes, test_module_span),
);
}
}
Expand Down
64 changes: 64 additions & 0 deletions src/test/ui/imports/unused-imports-in-test-module.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#![deny(unused_imports)]

use std::io::BufRead; //~ ERROR unused import: `std::io::BufRead`

fn a() {}
fn b() {}

mod test {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

mod tests {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

mod test_a {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

mod a_test {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

mod tests_a {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

mod a_tests {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

mod fastest_search {
use super::a; //~ ERROR unused import: `super::a`

fn foo() {
use crate::b; //~ ERROR unused import: `crate::b`
}
}

fn main() {}
170 changes: 170 additions & 0 deletions src/test/ui/imports/unused-imports-in-test-module.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
error: unused import: `std::io::BufRead`
--> $DIR/unused-imports-in-test-module.rs:3:5
|
LL | use std::io::BufRead;
| ^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/unused-imports-in-test-module.rs:1:9
|
LL | #![deny(unused_imports)]
| ^^^^^^^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:9:9
|
LL | use super::a;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:8:1
|
LL | mod test {
| ^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:12:13
|
LL | use crate::b;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:8:1
|
LL | mod test {
| ^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:17:9
|
LL | use super::a;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:16:1
|
LL | mod tests {
| ^^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:20:13
|
LL | use crate::b;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:16:1
|
LL | mod tests {
| ^^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:25:9
|
LL | use super::a;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:24:1
|
LL | mod test_a {
| ^^^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:28:13
|
LL | use crate::b;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:24:1
|
LL | mod test_a {
| ^^^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:33:9
|
LL | use super::a;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:32:1
|
LL | mod a_test {
| ^^^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:36:13
|
LL | use crate::b;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:32:1
|
LL | mod a_test {
| ^^^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:41:9
|
LL | use super::a;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:40:1
|
LL | mod tests_a {
| ^^^^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:44:13
|
LL | use crate::b;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:40:1
|
LL | mod tests_a {
| ^^^^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:49:9
|
LL | use super::a;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:48:1
|
LL | mod a_tests {
| ^^^^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:52:13
|
LL | use crate::b;
| ^^^^^^^^
|
help: consider adding a `#[cfg(test)]` to the containing module
--> $DIR/unused-imports-in-test-module.rs:48:1
|
LL | mod a_tests {
| ^^^^^^^^^^^

error: unused import: `super::a`
--> $DIR/unused-imports-in-test-module.rs:57:9
|
LL | use super::a;
| ^^^^^^^^

error: unused import: `crate::b`
--> $DIR/unused-imports-in-test-module.rs:60:13
|
LL | use crate::b;
| ^^^^^^^^

error: aborting due to 15 previous errors

0 comments on commit 6f8ad6d

Please sign in to comment.