Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 000d1c6

Browse files
borsbors
committed
Auto merge of rust-lang#122165 - chenyukang:yukang-fix-121315-suggest-removing, r=<try>
Merge RedundantImport into UnusedImport for suggesting removing spans Fixes rust-lang#121315 I also changed the label of diagnostics for RedundantImport to be compatible with UnusedImport, or any other better ways? r? `@petrochenkov`
2 parents d255c6a + 5a1485b commit 000d1c6

25 files changed

+245
-143
lines changed

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

Lines changed: 10 additions & 10 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,15 +139,15 @@ 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) => {
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
}
152152
}
153153
BuiltinLintDiag::DeprecatedMacro(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),
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: 117 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ 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};
@@ -43,7 +42,7 @@ struct UnusedImport {
4342
use_tree: ast::UseTree,
4443
use_tree_id: ast::NodeId,
4544
item_span: Span,
46-
unused: UnordSet<ast::NodeId>,
45+
unused: FxIndexSet<ast::NodeId>,
4746
}
4847

4948
impl UnusedImport {
@@ -96,7 +95,7 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
9695
// FIXME(#120456) - is `swap_remove` correct?
9796
self.r.maybe_unused_trait_imports.swap_remove(&def_id);
9897
if let Some(i) = self.unused_imports.get_mut(&self.base_id) {
99-
i.unused.remove(&id);
98+
i.unused.swap_remove(&id);
10099
}
101100
}
102101
}
@@ -138,6 +137,16 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
138137
}
139138
}
140139

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+
141150
fn report_unused_extern_crate_items(
142151
&mut self,
143152
maybe_unused_extern_crates: FxHashMap<ast::NodeId, Span>,
@@ -265,6 +274,40 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
265274
}
266275
}
267276

277+
struct ImportFinderVisitor {
278+
unused_import: Option<UnusedImport>,
279+
root_node_id: ast::NodeId,
280+
node_id: ast::NodeId,
281+
item_span: Span,
282+
}
283+
284+
impl Visitor<'_> for ImportFinderVisitor {
285+
fn visit_item(&mut self, item: &ast::Item) {
286+
match item.kind {
287+
ast::ItemKind::Use(..) if item.span.is_dummy() => return,
288+
_ => {}
289+
}
290+
291+
self.item_span = item.span_with_attributes();
292+
visit::walk_item(self, item);
293+
}
294+
295+
fn visit_use_tree(&mut self, use_tree: &ast::UseTree, id: ast::NodeId, _nested: bool) {
296+
if id == self.root_node_id {
297+
let mut unused_import = UnusedImport {
298+
use_tree: use_tree.clone(),
299+
use_tree_id: id,
300+
item_span: self.item_span,
301+
unused: Default::default(),
302+
};
303+
unused_import.unused.insert(self.node_id);
304+
self.unused_import = Some(unused_import);
305+
}
306+
visit::walk_use_tree(self, use_tree, id);
307+
}
308+
}
309+
310+
#[derive(Debug)]
268311
enum UnusedSpanResult {
269312
Used,
270313
FlatUnused(Span, Span),
@@ -412,6 +455,46 @@ impl Resolver<'_, '_> {
412455

413456
visitor.report_unused_extern_crate_items(maybe_unused_extern_crates);
414457

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+
415498
for unused in visitor.unused_imports.values() {
416499
let mut fixes = Vec::new();
417500
let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) {
@@ -432,19 +515,35 @@ impl Resolver<'_, '_> {
432515
}
433516
};
434517

435-
let ms = MultiSpan::from_spans(spans);
518+
let redundant_sources = unused
519+
.unused
520+
.clone()
521+
.into_iter()
522+
.filter_map(|id| redundant_source_spans.get(&id))
523+
.cloned()
524+
.collect();
436525

437-
let mut span_snippets = ms
526+
let multi_span = MultiSpan::from_spans(spans);
527+
let mut span_snippets = multi_span
438528
.primary_spans()
439529
.iter()
440530
.filter_map(|span| tcx.sess.source_map().span_to_snippet(*span).ok())
441531
.map(|s| format!("`{s}`"))
442532
.collect::<Vec<String>>();
443533
span_snippets.sort();
444534

535+
let remove_type =
536+
if unused.unused.iter().all(|i| redundant_source_spans.get(i).is_none()) {
537+
"unused import"
538+
} else if unused.unused.iter().all(|i| redundant_source_spans.get(i).is_some()) {
539+
"redundant import"
540+
} else {
541+
"unused or redundant import"
542+
};
445543
let msg = format!(
446-
"unused import{}{}",
447-
pluralize!(ms.primary_spans().len()),
544+
"{}{}{}",
545+
remove_type,
546+
pluralize!(multi_span.primary_spans().len()),
448547
if !span_snippets.is_empty() {
449548
format!(": {}", span_snippets.join(", "))
450549
} else {
@@ -453,11 +552,11 @@ impl Resolver<'_, '_> {
453552
);
454553

455554
let fix_msg = if fixes.len() == 1 && fixes[0].0 == unused.item_span {
456-
"remove the whole `use` item"
457-
} else if ms.primary_spans().len() > 1 {
458-
"remove the unused imports"
555+
"remove the whole `use` item".to_owned()
556+
} else if multi_span.primary_spans().len() > 1 {
557+
format!("remove the {}s", remove_type)
459558
} else {
460-
"remove the unused import"
559+
format!("remove the {}", remove_type)
461560
};
462561

463562
// If we are in the `--test` mode, suppress a help that adds the `#[cfg(test)]`
@@ -487,35 +586,15 @@ impl Resolver<'_, '_> {
487586
visitor.r.lint_buffer.buffer_lint_with_diagnostic(
488587
UNUSED_IMPORTS,
489588
unused.use_tree_id,
490-
ms,
589+
multi_span,
491590
msg,
492-
BuiltinLintDiag::UnusedImports(fix_msg.into(), fixes, test_module_span),
591+
BuiltinLintDiag::UnusedImports(
592+
fix_msg.into(),
593+
fixes,
594+
test_module_span,
595+
redundant_sources,
596+
),
493597
);
494598
}
495-
496-
let unused_imports = visitor.unused_imports;
497-
let mut check_redundant_imports = FxIndexSet::default();
498-
for module in self.arenas.local_modules().iter() {
499-
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
500-
let resolution = resolution.borrow();
501-
502-
if let Some(binding) = resolution.binding
503-
&& let NameBindingKind::Import { import, .. } = binding.kind
504-
&& let ImportKind::Single { id, .. } = import.kind
505-
{
506-
if let Some(unused_import) = unused_imports.get(&import.root_id)
507-
&& unused_import.unused.contains(&id)
508-
{
509-
continue;
510-
}
511-
512-
check_redundant_imports.insert(import);
513-
}
514-
}
515-
}
516-
517-
for import in check_redundant_imports {
518-
self.check_for_redundant_imports(import);
519-
}
520599
}
521600
}

‎compiler/rustc_resolve/src/imports.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13061306
None
13071307
}
13081308

1309-
pub(crate) fn check_for_redundant_imports(&mut self, import: Import<'a>) {
1309+
pub(crate) fn check_for_redundant_imports(
1310+
&mut self,
1311+
import: Import<'a>,
1312+
) -> Option<Vec<(Span, bool)>> {
13101313
// This function is only called for single imports.
13111314
let ImportKind::Single {
13121315
source, target, ref source_bindings, ref target_bindings, id, ..
@@ -1317,12 +1320,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13171320

13181321
// Skip if the import is of the form `use source as target` and source != target.
13191322
if source != target {
1320-
return;
1323+
return None;
13211324
}
13221325

13231326
// Skip if the import was produced by a macro.
13241327
if import.parent_scope.expansion != LocalExpnId::ROOT {
1325-
return;
1328+
return None;
13261329
}
13271330

13281331
// Skip if we are inside a named module (in contrast to an anonymous
@@ -1332,7 +1335,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13321335
if import.used.get() == Some(Used::Other)
13331336
|| self.effective_visibilities.is_exported(self.local_def_id(id))
13341337
{
1335-
return;
1338+
return None;
13361339
}
13371340

13381341
let mut is_redundant = true;
@@ -1368,14 +1371,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13681371
let mut redundant_spans: Vec<_> = redundant_span.present_items().collect();
13691372
redundant_spans.sort();
13701373
redundant_spans.dedup();
1371-
self.lint_buffer.buffer_lint_with_diagnostic(
1372-
UNUSED_IMPORTS,
1373-
id,
1374-
import.span,
1375-
format!("the item `{source}` is imported redundantly"),
1376-
BuiltinLintDiag::RedundantImport(redundant_spans, source),
1377-
);
1374+
return Some(redundant_spans);
13781375
}
1376+
None
13791377
}
13801378

13811379
fn resolve_glob_import(&mut self, import: Import<'a>) {

‎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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
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;

‎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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
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;

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//@ run-rustfix
2+
//@ compile-flags: --edition 2021
3+
#![deny(unused_imports)]
4+
#![allow(dead_code)]
5+
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
17+
18+
let _ = u32::try_from(5i32);
19+
let _a: i32 = u32::try_into(5u32).unwrap();
20+
}
21+
22+
fn test2() {
23+
// Test remove both redundant and unused
24+
25+
//~^ ERROR unused or redundant imports: `AsMut`, `Into`
26+
27+
let _a: u32 = (5u8).into();
28+
}
29+
30+
fn test3() {
31+
// Test remove NestedPartialUnused
32+
use std::convert::{Infallible};
33+
//~^ ERROR unused import: `From`
34+
35+
trait MyTrait {}
36+
impl MyTrait for fn() -> Infallible {}
37+
}
38+
39+
fn main() {}

0 commit comments

Comments
 (0)