From 5a4ff2779eda36d187b4d5b0cd8d18a7a3baa709 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 4 Mar 2024 10:24:23 +0800 Subject: [PATCH] Fix redundant import errors for preload extern crate --- .../rustc_lint/src/context/diagnostics.rs | 6 +- compiler/rustc_resolve/src/check_unused.rs | 146 +++++++++--------- compiler/rustc_resolve/src/imports.rs | 2 - .../ui/imports/auxiliary/aux-issue-121915.rs | 1 + .../redundant-import-issue-121915-2015.rs | 11 ++ .../redundant-import-issue-121915-2015.stderr | 17 ++ .../imports/redundant-import-issue-121915.rs | 9 ++ .../redundant-import-issue-121915.stderr | 14 ++ .../ui/imports/suggest-remove-issue-121315.rs | 40 +++++ .../suggest-remove-issue-121315.stderr | 56 +++++++ 10 files changed, 230 insertions(+), 72 deletions(-) create mode 100644 tests/ui/imports/auxiliary/aux-issue-121915.rs create mode 100644 tests/ui/imports/redundant-import-issue-121915-2015.rs create mode 100644 tests/ui/imports/redundant-import-issue-121915-2015.stderr create mode 100644 tests/ui/imports/redundant-import-issue-121915.rs create mode 100644 tests/ui/imports/redundant-import-issue-121915.stderr create mode 100644 tests/ui/imports/suggest-remove-issue-121315.rs create mode 100644 tests/ui/imports/suggest-remove-issue-121315.stderr diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index a58a37bf3ac08..a0be1c09c9a3d 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -143,7 +143,11 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di BuiltinLintDiag::RedundantImport(spans, ident) => { 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}"), + ); } } BuiltinLintDiag::DeprecatedMacro(suggestion, span) => { diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 13fec70e0a7f3..bf1ea2e270932 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -137,6 +137,81 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> { self.check_import_as_underscore(item, *id); } } + + fn report_unused_extern_crate_items( + &mut self, + maybe_unused_extern_crates: FxHashMap, + ) { + let tcx = self.r.tcx(); + for extern_crate in &self.extern_crate_items { + let warn_if_unused = !extern_crate.ident.name.as_str().starts_with('_'); + + // If the crate is fully unused, we suggest removing it altogether. + // We do this in any edition. + if warn_if_unused { + if let Some(&span) = maybe_unused_extern_crates.get(&extern_crate.id) { + self.r.lint_buffer.buffer_lint_with_diagnostic( + UNUSED_EXTERN_CRATES, + extern_crate.id, + span, + "unused extern crate", + BuiltinLintDiag::UnusedExternCrate { + removal_span: extern_crate.span_with_attributes, + }, + ); + continue; + } + } + + // If we are not in Rust 2018 edition, then we don't make any further + // suggestions. + if !tcx.sess.at_least_rust_2018() { + continue; + } + + // If the extern crate has any attributes, they may have funky + // semantics we can't faithfully represent using `use` (most + // notably `#[macro_use]`). Ignore it. + if extern_crate.has_attrs { + continue; + } + + // If the extern crate is renamed, then we cannot suggest replacing it with a use as this + // would not insert the new name into the prelude, where other imports in the crate may be + // expecting it. + if extern_crate.renames { + continue; + } + + // If the extern crate isn't in the extern prelude, + // there is no way it can be written as a `use`. + if !self + .r + .extern_prelude + .get(&extern_crate.ident) + .is_some_and(|entry| !entry.introduced_by_item) + { + continue; + } + + let vis_span = extern_crate + .vis_span + .find_ancestor_inside(extern_crate.span) + .unwrap_or(extern_crate.vis_span); + let ident_span = extern_crate + .ident + .span + .find_ancestor_inside(extern_crate.span) + .unwrap_or(extern_crate.ident.span); + self.r.lint_buffer.buffer_lint_with_diagnostic( + UNUSED_EXTERN_CRATES, + extern_crate.id, + extern_crate.span, + "`extern crate` is not idiomatic in the new edition", + BuiltinLintDiag::ExternCrateNotIdiomatic { vis_span, ident_span }, + ); + } + } } impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { @@ -335,6 +410,8 @@ impl Resolver<'_, '_> { }; visit::walk_crate(&mut visitor, krate); + visitor.report_unused_extern_crate_items(maybe_unused_extern_crates); + for unused in visitor.unused_imports.values() { let mut fixes = Vec::new(); let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) { @@ -416,75 +493,6 @@ impl Resolver<'_, '_> { ); } - for extern_crate in visitor.extern_crate_items { - let warn_if_unused = !extern_crate.ident.name.as_str().starts_with('_'); - - // If the crate is fully unused, we suggest removing it altogether. - // We do this in any edition. - if warn_if_unused { - if let Some(&span) = maybe_unused_extern_crates.get(&extern_crate.id) { - visitor.r.lint_buffer.buffer_lint_with_diagnostic( - UNUSED_EXTERN_CRATES, - extern_crate.id, - span, - "unused extern crate", - BuiltinLintDiag::UnusedExternCrate { - removal_span: extern_crate.span_with_attributes, - }, - ); - continue; - } - } - - // If we are not in Rust 2018 edition, then we don't make any further - // suggestions. - if !tcx.sess.at_least_rust_2018() { - continue; - } - - // If the extern crate has any attributes, they may have funky - // semantics we can't faithfully represent using `use` (most - // notably `#[macro_use]`). Ignore it. - if extern_crate.has_attrs { - continue; - } - - // If the extern crate is renamed, then we cannot suggest replacing it with a use as this - // would not insert the new name into the prelude, where other imports in the crate may be - // expecting it. - if extern_crate.renames { - continue; - } - - // If the extern crate isn't in the extern prelude, - // there is no way it can be written as a `use`. - if !visitor - .r - .extern_prelude - .get(&extern_crate.ident) - .is_some_and(|entry| !entry.introduced_by_item) - { - continue; - } - - let vis_span = extern_crate - .vis_span - .find_ancestor_inside(extern_crate.span) - .unwrap_or(extern_crate.vis_span); - let ident_span = extern_crate - .ident - .span - .find_ancestor_inside(extern_crate.span) - .unwrap_or(extern_crate.ident.span); - visitor.r.lint_buffer.buffer_lint_with_diagnostic( - UNUSED_EXTERN_CRATES, - extern_crate.id, - extern_crate.span, - "`extern crate` is not idiomatic in the new edition", - BuiltinLintDiag::ExternCrateNotIdiomatic { vis_span, ident_span }, - ); - } - let unused_imports = visitor.unused_imports; let mut check_redundant_imports = FxIndexSet::default(); for module in self.arenas.local_modules().iter() { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 6647a9a279285..80ec69ab318af 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1336,9 +1336,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } 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 { diff --git a/tests/ui/imports/auxiliary/aux-issue-121915.rs b/tests/ui/imports/auxiliary/aux-issue-121915.rs new file mode 100644 index 0000000000000..7f9f5bda79ffd --- /dev/null +++ b/tests/ui/imports/auxiliary/aux-issue-121915.rs @@ -0,0 +1 @@ +pub fn item() {} diff --git a/tests/ui/imports/redundant-import-issue-121915-2015.rs b/tests/ui/imports/redundant-import-issue-121915-2015.rs new file mode 100644 index 0000000000000..d41d190bb58c4 --- /dev/null +++ b/tests/ui/imports/redundant-import-issue-121915-2015.rs @@ -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(); +} diff --git a/tests/ui/imports/redundant-import-issue-121915-2015.stderr b/tests/ui/imports/redundant-import-issue-121915-2015.stderr new file mode 100644 index 0000000000000..174ed4fb96b11 --- /dev/null +++ b/tests/ui/imports/redundant-import-issue-121915-2015.stderr @@ -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; + | ^^^^^^^^^^^^^^^^ + | +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 + diff --git a/tests/ui/imports/redundant-import-issue-121915.rs b/tests/ui/imports/redundant-import-issue-121915.rs new file mode 100644 index 0000000000000..237acc4af2565 --- /dev/null +++ b/tests/ui/imports/redundant-import-issue-121915.rs @@ -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(); +} diff --git a/tests/ui/imports/redundant-import-issue-121915.stderr b/tests/ui/imports/redundant-import-issue-121915.stderr new file mode 100644 index 0000000000000..0047d7c34207c --- /dev/null +++ b/tests/ui/imports/redundant-import-issue-121915.stderr @@ -0,0 +1,14 @@ +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 + | +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 + diff --git a/tests/ui/imports/suggest-remove-issue-121315.rs b/tests/ui/imports/suggest-remove-issue-121315.rs new file mode 100644 index 0000000000000..63533480ec127 --- /dev/null +++ b/tests/ui/imports/suggest-remove-issue-121315.rs @@ -0,0 +1,40 @@ +//@ compile-flags: --edition 2021 +#![deny(unused_imports)] +#![allow(dead_code)] + +fn test0() { + // Test remove FlatUnused + use std::convert::TryFrom; + //~^ ERROR the item `TryFrom` is imported redundantly + let _ = u32::try_from(5i32); +} + +fn test1() { + // FIXME(yukang) Test remove NestedFullUnused + use std::convert::{TryFrom, TryInto}; + //~^ ERROR the item `TryFrom` is imported redundantly + //~| ERROR the item `TryInto` is imported redundantly + + let _ = u32::try_from(5i32); + let _a: i32 = u32::try_into(5u32).unwrap(); +} + +fn test2() { + // FIXME(yukang): Test remove both redundant and unused + use std::convert::{AsMut, Into}; + //~^ ERROR unused import: `AsMut` + //~| ERROR the item `Into` is imported redundantly + + let _a: u32 = (5u8).into(); +} + +fn test3() { + // Test remove NestedPartialUnused + use std::convert::{From, Infallible}; + //~^ ERROR unused import: `From` + + trait MyTrait {} + impl MyTrait for fn() -> Infallible {} +} + +fn main() {} diff --git a/tests/ui/imports/suggest-remove-issue-121315.stderr b/tests/ui/imports/suggest-remove-issue-121315.stderr new file mode 100644 index 0000000000000..dbd742f6c781f --- /dev/null +++ b/tests/ui/imports/suggest-remove-issue-121315.stderr @@ -0,0 +1,56 @@ +error: the item `TryFrom` is imported redundantly + --> $DIR/suggest-remove-issue-121315.rs:7:9 + | +LL | use std::convert::TryFrom; + | ^^^^^^^^^^^^^^^^^^^^^ + --> $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:2:9 + | +LL | #![deny(unused_imports)] + | ^^^^^^^^^^^^^^ + +error: the item `TryFrom` is imported redundantly + --> $DIR/suggest-remove-issue-121315.rs:14:24 + | +LL | use std::convert::{TryFrom, TryInto}; + | ^^^^^^^ + --> $SRC_DIR/std/src/prelude/mod.rs:LL:COL + | + = note: the item `TryFrom` is already defined here + +error: the item `TryInto` is imported redundantly + --> $DIR/suggest-remove-issue-121315.rs:14:33 + | +LL | use std::convert::{TryFrom, TryInto}; + | ^^^^^^^ + --> $SRC_DIR/std/src/prelude/mod.rs:LL:COL + | + = note: the item `TryInto` is already defined here + +error: unused import: `AsMut` + --> $DIR/suggest-remove-issue-121315.rs:24:24 + | +LL | use std::convert::{AsMut, Into}; + | ^^^^^ + +error: the item `Into` is imported redundantly + --> $DIR/suggest-remove-issue-121315.rs:24:31 + | +LL | use std::convert::{AsMut, Into}; + | ^^^^ + --> $SRC_DIR/std/src/prelude/mod.rs:LL:COL + | + = note: the item `Into` is already defined here + +error: unused import: `From` + --> $DIR/suggest-remove-issue-121315.rs:33:24 + | +LL | use std::convert::{From, Infallible}; + | ^^^^ + +error: aborting due to 6 previous errors +