Skip to content

Commit 723ac6f

Browse files
chenyukangchenyukang
committed
merge redundant import into unused import for suggesting removing
1 parent 290be1b commit 723ac6f

24 files changed

+234
-201
lines changed

‎compiler/rustc_lint/src/context/diagnostics.rs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di
124124
BuiltinLintDiag::UnknownCrateTypes(span, note, sugg) => {
125125
diag.span_suggestion(span, note, sugg, Applicability::MaybeIncorrect);
126126
}
127-
BuiltinLintDiag::UnusedImports(message, replaces, in_test_module) => {
127+
BuiltinLintDiag::UnusedImports(message, replaces, in_test_module, redundant_sources) => {
128128
if !replaces.is_empty() {
129129
diag.tool_only_multipart_suggestion(
130130
message,
@@ -139,22 +139,16 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di
139139
"if this is a test module, consider adding a `#[cfg(test)]` to the containing module",
140140
);
141141
}
142-
}
143-
BuiltinLintDiag::RedundantImport(spans, ident, import_span) => {
144-
for (span, is_imported) in spans {
145-
let introduced = if is_imported { "imported" } else { "defined" };
146-
let span_msg = if span.is_dummy() { "by prelude" } else { "here" };
147-
diag.span_label(
148-
span,
149-
format!("the item `{ident}` is already {introduced} {span_msg}"),
150-
);
142+
for (ident, spans) in redundant_sources {
143+
for (span, is_imported) in spans {
144+
let introduced = if is_imported { "imported" } else { "defined" };
145+
let span_msg = if span.is_dummy() { "by prelude" } else { "here" };
146+
diag.span_label(
147+
span,
148+
format!("the item `{ident}` is already {introduced} {span_msg}"),
149+
);
150+
}
151151
}
152-
diag.span_suggestion(
153-
import_span,
154-
"remove this import",
155-
"",
156-
Applicability::MachineApplicable,
157-
);
158152
}
159153
BuiltinLintDiag::DeprecatedMacro(suggestion, span) => {
160154
stability::deprecation_suggestion(diag, "macro", suggestion, span)

‎compiler/rustc_lint_defs/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,8 +576,7 @@ pub enum BuiltinLintDiag {
576576
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
577577
ElidedLifetimesInPaths(usize, Span, bool, Span),
578578
UnknownCrateTypes(Span, String, String),
579-
UnusedImports(String, Vec<(Span, String)>, Option<Span>),
580-
RedundantImport(Vec<(Span, bool)>, Ident, Span),
579+
UnusedImports(String, Vec<(Span, String)>, Option<Span>, Vec<(Ident, Vec<(Span, bool)>)>),
581580
DeprecatedMacro(Option<Symbol>, Span),
582581
MissingAbi(Span, Abi),
583582
UnusedDocComment(Span),

‎compiler/rustc_resolve/src/check_unused.rs

Lines changed: 82 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,18 @@ use crate::NameBindingKind;
3131
use rustc_ast as ast;
3232
use rustc_ast::visit::{self, Visitor};
3333
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
34-
use rustc_data_structures::unord::UnordSet;
3534
use rustc_errors::{pluralize, MultiSpan};
3635
use rustc_hir::def::{DefKind, Res};
3736
use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS};
3837
use rustc_session::lint::BuiltinLintDiag;
3938
use rustc_span::symbol::{kw, Ident};
4039
use rustc_span::{Span, DUMMY_SP};
4140

42-
#[derive(Debug)]
4341
struct UnusedImport {
4442
use_tree: ast::UseTree,
4543
use_tree_id: ast::NodeId,
4644
item_span: Span,
47-
unused: UnordSet<ast::NodeId>,
45+
unused: FxIndexSet<ast::NodeId>,
4846
}
4947

5048
impl UnusedImport {
@@ -97,7 +95,7 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
9795
// FIXME(#120456) - is `swap_remove` correct?
9896
self.r.maybe_unused_trait_imports.swap_remove(&def_id);
9997
if let Some(i) = self.unused_imports.get_mut(&self.base_id) {
100-
i.unused.remove(&id);
98+
i.unused.swap_remove(&id);
10199
}
102100
}
103101
}
@@ -139,6 +137,16 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
139137
}
140138
}
141139

140+
fn merge_unused_import(&mut self, unused_import: UnusedImport) {
141+
if let Some(import) = self.unused_imports.get_mut(&unused_import.use_tree_id) {
142+
for id in unused_import.unused {
143+
import.unused.insert(id);
144+
}
145+
} else {
146+
self.unused_imports.entry(unused_import.use_tree_id).or_insert(unused_import);
147+
}
148+
}
149+
142150
fn report_unused_extern_crate_items(
143151
&mut self,
144152
maybe_unused_extern_crates: FxHashMap<ast::NodeId, Span>,
@@ -447,6 +455,46 @@ impl Resolver<'_, '_> {
447455

448456
visitor.report_unused_extern_crate_items(maybe_unused_extern_crates);
449457

458+
let unused_imports = &visitor.unused_imports;
459+
let mut check_redundant_imports = FxIndexSet::default();
460+
for module in visitor.r.arenas.local_modules().iter() {
461+
for (_key, resolution) in visitor.r.resolutions(*module).borrow().iter() {
462+
let resolution = resolution.borrow();
463+
464+
if let Some(binding) = resolution.binding
465+
&& let NameBindingKind::Import { import, .. } = binding.kind
466+
&& let ImportKind::Single { id, .. } = import.kind
467+
{
468+
if let Some(unused_import) = unused_imports.get(&import.root_id)
469+
&& unused_import.unused.contains(&id)
470+
{
471+
continue;
472+
}
473+
474+
check_redundant_imports.insert(import);
475+
}
476+
}
477+
}
478+
479+
let mut redundant_source_spans = FxHashMap::default();
480+
for import in check_redundant_imports {
481+
if let Some(redundant_spans) = visitor.r.check_for_redundant_imports(import)
482+
&& let ImportKind::Single { source, id, .. } = import.kind
483+
{
484+
let mut finder = ImportFinderVisitor {
485+
node_id: id,
486+
root_node_id: import.root_id,
487+
unused_import: None,
488+
item_span: Span::default(),
489+
};
490+
visit::walk_crate(&mut finder, krate);
491+
if let Some(unused) = finder.unused_import {
492+
visitor.merge_unused_import(unused);
493+
redundant_source_spans.insert(id, (source, redundant_spans));
494+
}
495+
}
496+
}
497+
450498
for unused in visitor.unused_imports.values() {
451499
let mut fixes = Vec::new();
452500
let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) {
@@ -467,19 +515,34 @@ impl Resolver<'_, '_> {
467515
}
468516
};
469517

470-
let ms = MultiSpan::from_spans(spans);
518+
let mut redundant_sources = vec![];
519+
for id in unused.unused.clone() {
520+
if let Some(source) = redundant_source_spans.get(&id) {
521+
redundant_sources.push(source.clone());
522+
}
523+
}
471524

472-
let mut span_snippets = ms
525+
let multi_span = MultiSpan::from_spans(spans);
526+
let mut span_snippets = multi_span
473527
.primary_spans()
474528
.iter()
475529
.filter_map(|span| tcx.sess.source_map().span_to_snippet(*span).ok())
476530
.map(|s| format!("`{s}`"))
477531
.collect::<Vec<String>>();
478532
span_snippets.sort();
479533

534+
let remove_type =
535+
if unused.unused.iter().all(|i| redundant_source_spans.get(i).is_none()) {
536+
"unused import"
537+
} else if unused.unused.iter().all(|i| redundant_source_spans.get(i).is_some()) {
538+
"redundant import"
539+
} else {
540+
"unused or redundant import"
541+
};
480542
let msg = format!(
481-
"unused import{}{}",
482-
pluralize!(ms.primary_spans().len()),
543+
"{}{}{}",
544+
remove_type,
545+
pluralize!(multi_span.primary_spans().len()),
483546
if !span_snippets.is_empty() {
484547
format!(": {}", span_snippets.join(", "))
485548
} else {
@@ -488,11 +551,11 @@ impl Resolver<'_, '_> {
488551
);
489552

490553
let fix_msg = if fixes.len() == 1 && fixes[0].0 == unused.item_span {
491-
"remove the whole `use` item"
492-
} else if ms.primary_spans().len() > 1 {
493-
"remove the unused imports"
554+
"remove the whole `use` item".to_owned()
555+
} else if multi_span.primary_spans().len() > 1 {
556+
format!("remove the {}s", remove_type)
494557
} else {
495-
"remove the unused import"
558+
format!("remove the {}", remove_type)
496559
};
497560

498561
// If we are in the `--test` mode, suppress a help that adds the `#[cfg(test)]`
@@ -522,64 +585,15 @@ impl Resolver<'_, '_> {
522585
visitor.r.lint_buffer.buffer_lint_with_diagnostic(
523586
UNUSED_IMPORTS,
524587
unused.use_tree_id,
525-
ms,
588+
multi_span,
526589
msg,
527-
BuiltinLintDiag::UnusedImports(fix_msg.into(), fixes, test_module_span),
590+
BuiltinLintDiag::UnusedImports(
591+
fix_msg.into(),
592+
fixes,
593+
test_module_span,
594+
redundant_sources,
595+
),
528596
);
529597
}
530-
531-
let unused_imports = visitor.unused_imports;
532-
let mut check_redundant_imports = FxIndexSet::default();
533-
for module in self.arenas.local_modules().iter() {
534-
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
535-
let resolution = resolution.borrow();
536-
537-
if let Some(binding) = resolution.binding
538-
&& let NameBindingKind::Import { import, .. } = binding.kind
539-
&& let ImportKind::Single { id, .. } = import.kind
540-
{
541-
if let Some(unused_import) = unused_imports.get(&import.root_id)
542-
&& unused_import.unused.contains(&id)
543-
{
544-
continue;
545-
}
546-
547-
check_redundant_imports.insert(import);
548-
}
549-
}
550-
}
551-
552-
for import in check_redundant_imports {
553-
if let Some(redundant_spans) = self.check_for_redundant_imports(import)
554-
&& let ImportKind::Single { source, id, .. } = import.kind
555-
{
556-
let mut visitor = ImportFinderVisitor {
557-
node_id: id,
558-
root_node_id: import.root_id,
559-
unused_import: None,
560-
item_span: Span::default(),
561-
};
562-
visit::walk_crate(&mut visitor, krate);
563-
if let Some(unused) = visitor.unused_import {
564-
let remove_span =
565-
match calc_unused_spans(&unused, &unused.use_tree, unused.use_tree_id) {
566-
UnusedSpanResult::FlatUnused(_, remove) => remove,
567-
UnusedSpanResult::NestedFullUnused(_, remove) => remove,
568-
UnusedSpanResult::NestedPartialUnused(_, removes) => {
569-
assert_eq!(removes.len(), 1);
570-
removes[0]
571-
}
572-
_ => import.use_span,
573-
};
574-
self.lint_buffer.buffer_lint_with_diagnostic(
575-
UNUSED_IMPORTS,
576-
id,
577-
import.span,
578-
format!("the item `{source}` is imported redundantly"),
579-
BuiltinLintDiag::RedundantImport(redundant_spans, source, remove_span),
580-
);
581-
}
582-
}
583-
}
584598
}
585599
}

‎tests/ui/imports/redundant-import-issue-121915-2015.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ extern crate aux_issue_121915;
66
#[deny(unused_imports)]
77
fn main() {
88
use aux_issue_121915;
9-
//~^ ERROR the item `aux_issue_121915` is imported redundantly
9+
//~^ ERROR redundant import
1010
aux_issue_121915::item();
1111
}

‎tests/ui/imports/redundant-import-issue-121915-2015.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
error: the item `aux_issue_121915` is imported redundantly
1+
error: redundant import: `aux_issue_121915`
22
--> $DIR/redundant-import-issue-121915-2015.rs:8:9
33
|
44
LL | extern crate aux_issue_121915;
55
| ------------------------------ the item `aux_issue_121915` is already imported here
66
...
77
LL | use aux_issue_121915;
8-
| ----^^^^^^^^^^^^^^^^- help: remove this import
8+
| ^^^^^^^^^^^^^^^^
99
|
1010
note: the lint level is defined here
1111
--> $DIR/redundant-import-issue-121915-2015.rs:6:8

‎tests/ui/imports/redundant-import-issue-121915.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
#[deny(unused_imports)]
55
fn main() {
66
use aux_issue_121915;
7-
//~^ ERROR the item `aux_issue_121915` is imported redundantly
7+
//~^ ERROR redundant import
88
aux_issue_121915::item();
99
}

‎tests/ui/imports/redundant-import-issue-121915.stderr

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
error: the item `aux_issue_121915` is imported redundantly
1+
error: redundant import: `aux_issue_121915`
22
--> $DIR/redundant-import-issue-121915.rs:6:9
33
|
44
LL | use aux_issue_121915;
5-
| ----^^^^^^^^^^^^^^^^-
6-
| | |
7-
| | the item `aux_issue_121915` is already defined by prelude
8-
| help: remove this import
5+
| ^^^^^^^^^^^^^^^^ the item `aux_issue_121915` is already defined by prelude
96
|
107
note: the lint level is defined here
118
--> $DIR/redundant-import-issue-121915.rs:4:8

‎tests/ui/imports/suggest-remove-issue-121315.fixed

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,37 @@
33
#![deny(unused_imports)]
44
#![allow(dead_code)]
55

6-
// Test remove FlatUnused
7-
//~^ ERROR the item `TryFrom` is imported redundantly
6+
fn test0() {
7+
// Test remove FlatUnused
8+
9+
//~^ ERROR redundant import
10+
let _ = u32::try_from(5i32);
11+
}
12+
13+
fn test1() {
14+
// Test remove NestedFullUnused
15+
16+
//~^ ERROR redundant imports
817

9-
// Test remove NestedFullUnused
10-
//~^ ERROR the item `TryInto` is imported redundantly
18+
let _ = u32::try_from(5i32);
19+
let _a: i32 = u32::try_into(5u32).unwrap();
20+
}
1121

12-
// Test remove NestedPartialUnused
13-
use std::convert::{Infallible};
14-
//~^ ERROR the item `From` is imported redundantly
22+
fn test2() {
23+
// Test remove both redundant and unused
24+
25+
//~^ ERROR unused or redundant imports: `AsMut`, `Into`
1526

16-
// Test remove both redundant and unused?
17-
// use std::convert::{AsMut, Into};
27+
let _a: u32 = (5u8).into();
28+
}
1829

19-
trait MyTrait {}
20-
impl MyTrait for fn() -> Infallible {}
30+
fn test3() {
31+
// Test remove NestedPartialUnused
32+
use std::convert::{Infallible};
33+
//~^ ERROR unused import: `From`
2134

22-
fn main() {
23-
let _ = u32::try_from(5i32);
24-
let _a: i32 = u32::try_into(5u32).unwrap();
25-
let _a: String = String::from("hello anan");
26-
//let _a: u32 = (5i32).into();
35+
trait MyTrait {}
36+
impl MyTrait for fn() -> Infallible {}
2737
}
38+
39+
fn main() {}

0 commit comments

Comments
 (0)