Skip to content

Commit

Permalink
Fix redundant import errors for preload extern crate
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Mar 6, 2024
1 parent bdde2a8 commit a2c624b
Show file tree
Hide file tree
Showing 18 changed files with 245 additions and 27 deletions.
14 changes: 12 additions & 2 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,21 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di
);
}
}
BuiltinLintDiag::RedundantImport(spans, ident) => {
BuiltinLintDiag::RedundantImport(spans, ident, import_span) => {
for (span, is_imported) in spans {
let introduced = if is_imported { "imported" } else { "defined" };
diag.span_label(span, format!("the item `{ident}` is already {introduced} here"));
let span_msg = if span.is_dummy() { "by prelude" } else { "here" };
diag.span_label(
span,
format!("the item `{ident}` is already {introduced} {span_msg}"),
);
}
diag.span_suggestion(
import_span,
"remove this import",
"",
Applicability::MachineApplicable,
);
}
BuiltinLintDiag::DeprecatedMacro(suggestion, span) => {
stability::deprecation_suggestion(diag, "macro", suggestion, span)
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 @@ -577,7 +577,7 @@ pub enum BuiltinLintDiag {
ElidedLifetimesInPaths(usize, Span, bool, Span),
UnknownCrateTypes(Span, String, String),
UnusedImports(String, Vec<(Span, String)>, Option<Span>),
RedundantImport(Vec<(Span, bool)>, Ident),
RedundantImport(Vec<(Span, bool)>, Ident, Span),
DeprecatedMacro(Option<Symbol>, Span),
MissingAbi(Span, Abi),
UnusedDocComment(Span),
Expand Down
66 changes: 65 additions & 1 deletion compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use rustc_session::lint::BuiltinLintDiag;
use rustc_span::symbol::{kw, Ident};
use rustc_span::{Span, DUMMY_SP};

#[derive(Debug)]
struct UnusedImport {
use_tree: ast::UseTree,
use_tree_id: ast::NodeId,
Expand Down Expand Up @@ -190,6 +191,40 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
}
}

struct ImportFinderVisitor {
unused_import: Option<UnusedImport>,
root_node_id: ast::NodeId,
node_id: ast::NodeId,
item_span: Span,
}

impl Visitor<'_> for ImportFinderVisitor {
fn visit_item(&mut self, item: &ast::Item) {
match item.kind {
ast::ItemKind::Use(..) if item.span.is_dummy() => return,
_ => {}
}

self.item_span = item.span_with_attributes();
visit::walk_item(self, item);
}

fn visit_use_tree(&mut self, use_tree: &ast::UseTree, id: ast::NodeId, _nested: bool) {
if id == self.root_node_id {
let mut unused_import = UnusedImport {
use_tree: use_tree.clone(),
use_tree_id: id,
item_span: self.item_span,
unused: Default::default(),
};
unused_import.unused.insert(self.node_id);
self.unused_import = Some(unused_import);
}
visit::walk_use_tree(self, use_tree, id);
}
}

#[derive(Debug)]
enum UnusedSpanResult {
Used,
FlatUnused(Span, Span),
Expand Down Expand Up @@ -507,7 +542,36 @@ impl Resolver<'_, '_> {
}

for import in check_redundant_imports {
self.check_for_redundant_imports(import);
if let Some(redundant_spans) = self.check_for_redundant_imports(import)
&& let ImportKind::Single { source, id, .. } = import.kind
{
let mut visitor = ImportFinderVisitor {
node_id: id,
root_node_id: import.root_id,
unused_import: None,
item_span: Span::default(),
};
visit::walk_crate(&mut visitor, krate);
if let Some(unused) = visitor.unused_import {
let remove_span =
match calc_unused_spans(&unused, &unused.use_tree, unused.use_tree_id) {
UnusedSpanResult::FlatUnused(_, remove) => remove,
UnusedSpanResult::NestedFullUnused(_, remove) => remove,
UnusedSpanResult::NestedPartialUnused(_, removes) => {
assert_eq!(removes.len(), 1);
removes[0]
}
_ => import.use_span,
};
self.lint_buffer.buffer_lint_with_diagnostic(
UNUSED_IMPORTS,
id,
import.span,
format!("the item `{source}` is imported redundantly"),
BuiltinLintDiag::RedundantImport(redundant_spans, source, remove_span),
);
}
}
}
}
}
22 changes: 9 additions & 13 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
None
}

pub(crate) fn check_for_redundant_imports(&mut self, import: Import<'a>) {
pub(crate) fn check_for_redundant_imports(
&mut self,
import: Import<'a>,
) -> Option<Vec<(Span, bool)>> {
// This function is only called for single imports.
let ImportKind::Single {
source, target, ref source_bindings, ref target_bindings, id, ..
Expand All @@ -1315,12 +1318,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

// Skip if the import is of the form `use source as target` and source != target.
if source != target {
return;
return None;
}

// Skip if the import was produced by a macro.
if import.parent_scope.expansion != LocalExpnId::ROOT {
return;
return None;
}

// Skip if we are inside a named module (in contrast to an anonymous
Expand All @@ -1330,13 +1333,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
if import.used.get() == Some(Used::Other)
|| self.effective_visibilities.is_exported(self.local_def_id(id))
{
return;
return None;
}

let mut is_redundant = true;

let mut redundant_span = PerNS { value_ns: None, type_ns: None, macro_ns: None };

self.per_ns(|this, ns| {
if is_redundant && let Ok(binding) = source_bindings[ns].get() {
if binding.res() == Res::Err {
Expand Down Expand Up @@ -1368,14 +1369,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let mut redundant_spans: Vec<_> = redundant_span.present_items().collect();
redundant_spans.sort();
redundant_spans.dedup();
self.lint_buffer.buffer_lint_with_diagnostic(
UNUSED_IMPORTS,
id,
import.span,
format!("the item `{source}` is imported redundantly"),
BuiltinLintDiag::RedundantImport(redundant_spans, source),
);
return Some(redundant_spans);
}
None
}

fn resolve_glob_import(&mut self, import: Import<'a>) {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/imports/auxiliary/aux-issue-121915.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub fn item() {}
11 changes: 11 additions & 0 deletions tests/ui/imports/redundant-import-issue-121915-2015.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ compile-flags: --extern aux_issue_121915 --edition 2015
//@ aux-build: aux-issue-121915.rs

extern crate aux_issue_121915;

#[deny(unused_imports)]
fn main() {
use aux_issue_121915;
//~^ ERROR the item `aux_issue_121915` is imported redundantly
aux_issue_121915::item();
}
17 changes: 17 additions & 0 deletions tests/ui/imports/redundant-import-issue-121915-2015.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: the item `aux_issue_121915` is imported redundantly
--> $DIR/redundant-import-issue-121915-2015.rs:8:9
|
LL | extern crate aux_issue_121915;
| ------------------------------ the item `aux_issue_121915` is already imported here
...
LL | use aux_issue_121915;
| ----^^^^^^^^^^^^^^^^- help: remove this import
|
note: the lint level is defined here
--> $DIR/redundant-import-issue-121915-2015.rs:6:8
|
LL | #[deny(unused_imports)]
| ^^^^^^^^^^^^^^

error: aborting due to 1 previous error

9 changes: 9 additions & 0 deletions tests/ui/imports/redundant-import-issue-121915.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ compile-flags: --extern aux_issue_121915 --edition 2018
//@ aux-build: aux-issue-121915.rs

#[deny(unused_imports)]
fn main() {
use aux_issue_121915;
//~^ ERROR the item `aux_issue_121915` is imported redundantly
aux_issue_121915::item();
}
17 changes: 17 additions & 0 deletions tests/ui/imports/redundant-import-issue-121915.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: the item `aux_issue_121915` is imported redundantly
--> $DIR/redundant-import-issue-121915.rs:6:9
|
LL | use aux_issue_121915;
| ----^^^^^^^^^^^^^^^^-
| | |
| | the item `aux_issue_121915` is already defined by prelude
| help: remove this import
|
note: the lint level is defined here
--> $DIR/redundant-import-issue-121915.rs:4:8
|
LL | #[deny(unused_imports)]
| ^^^^^^^^^^^^^^

error: aborting due to 1 previous error

27 changes: 27 additions & 0 deletions tests/ui/imports/suggest-remove-issue-121315.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//@ run-rustfix
//@ compile-flags: --edition 2021
#![deny(unused_imports)]
#![allow(dead_code)]

// Test remove FlatUnused
//~^ ERROR the item `TryFrom` is imported redundantly

// Test remove NestedFullUnused
//~^ ERROR the item `TryInto` is imported redundantly

// Test remove NestedPartialUnused
use std::convert::{Infallible};
//~^ ERROR the item `From` is imported redundantly

// Test remove both redundant and unused?
// use std::convert::{AsMut, Into};

trait MyTrait {}
impl MyTrait for fn() -> Infallible {}

fn main() {
let _ = u32::try_from(5i32);
let _a: i32 = u32::try_into(5u32).unwrap();
let _a: String = String::from("hello anan");
//let _a: u32 = (5i32).into();
}
29 changes: 29 additions & 0 deletions tests/ui/imports/suggest-remove-issue-121315.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//@ run-rustfix
//@ compile-flags: --edition 2021
#![deny(unused_imports)]
#![allow(dead_code)]

// Test remove FlatUnused
use std::convert::TryFrom;
//~^ ERROR the item `TryFrom` is imported redundantly

// Test remove NestedFullUnused
use std::convert::{TryInto};
//~^ ERROR the item `TryInto` is imported redundantly

// Test remove NestedPartialUnused
use std::convert::{From, Infallible};
//~^ ERROR the item `From` is imported redundantly

// Test remove both redundant and unused?
// use std::convert::{AsMut, Into};

trait MyTrait {}
impl MyTrait for fn() -> Infallible {}

fn main() {
let _ = u32::try_from(5i32);
let _a: i32 = u32::try_into(5u32).unwrap();
let _a: String = String::from("hello anan");
//let _a: u32 = (5i32).into();
}
37 changes: 37 additions & 0 deletions tests/ui/imports/suggest-remove-issue-121315.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
error: the item `TryFrom` is imported redundantly
--> $DIR/suggest-remove-issue-121315.rs:7:5
|
LL | use std::convert::TryFrom;
| ----^^^^^^^^^^^^^^^^^^^^^- help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `TryFrom` is already defined here
|
note: the lint level is defined here
--> $DIR/suggest-remove-issue-121315.rs:3:9
|
LL | #![deny(unused_imports)]
| ^^^^^^^^^^^^^^

error: the item `TryInto` is imported redundantly
--> $DIR/suggest-remove-issue-121315.rs:11:20
|
LL | use std::convert::{TryInto};
| -------------------^^^^^^^-- help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `TryInto` is already defined here

error: the item `From` is imported redundantly
--> $DIR/suggest-remove-issue-121315.rs:15:20
|
LL | use std::convert::{From, Infallible};
| ^^^^--
| |
| help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `From` is already defined here

error: aborting due to 3 previous errors

2 changes: 1 addition & 1 deletion tests/ui/lint/unused/issue-59896.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | struct S;
| --------- the item `S` is already defined here
...
LL | use S;
| ^
| ----^- help: remove this import
|
note: the lint level is defined here
--> $DIR/issue-59896.rs:1:9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | use bar::*;
| ------ the item `Foo` is already imported here
...
LL | use bar::Foo;
| ^^^^^^^^
| ----^^^^^^^^- help: remove this import
|
note: the lint level is defined here
--> $DIR/use-redundant-glob-parent.rs:2:9
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lint/use-redundant/use-redundant-glob.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ warning: the item `Foo` is imported redundantly
LL | use bar::*;
| ------ the item `Foo` is already imported here
LL | use bar::Foo;
| ^^^^^^^^
| ----^^^^^^^^- help: remove this import
|
note: the lint level is defined here
--> $DIR/use-redundant-glob.rs:2:9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ warning: the item `Some` is imported redundantly
--> $DIR/use-redundant-prelude-rust-2015.rs:5:5
|
LL | use std::option::Option::Some;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ----^^^^^^^^^^^^^^^^^^^^^^^^^- help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `Some` is already defined here
Expand All @@ -17,7 +17,7 @@ warning: the item `None` is imported redundantly
--> $DIR/use-redundant-prelude-rust-2015.rs:6:5
|
LL | use std::option::Option::None;
| ^^^^^^^^^^^^^^^^^^^^^^^^^
| ----^^^^^^^^^^^^^^^^^^^^^^^^^- help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `None` is already defined here
Expand All @@ -26,7 +26,7 @@ warning: the item `Ok` is imported redundantly
--> $DIR/use-redundant-prelude-rust-2015.rs:8:5
|
LL | use std::result::Result::Ok;
| ^^^^^^^^^^^^^^^^^^^^^^^
| ----^^^^^^^^^^^^^^^^^^^^^^^- help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `Ok` is already defined here
Expand All @@ -35,7 +35,7 @@ warning: the item `Err` is imported redundantly
--> $DIR/use-redundant-prelude-rust-2015.rs:9:5
|
LL | use std::result::Result::Err;
| ^^^^^^^^^^^^^^^^^^^^^^^^
| ----^^^^^^^^^^^^^^^^^^^^^^^^- help: remove this import
--> $SRC_DIR/std/src/prelude/mod.rs:LL:COL
|
= note: the item `Err` is already defined here
Expand Down

0 comments on commit a2c624b

Please sign in to comment.