Rollup merge of rust-lang#121958 - chenyukang:yukang-fix-121915-import, r=petrochenkov

matthiaskrgr · web-flow · commit 8f99d571a36e · 2024-03-06T19:00:17.000+01:00
Fix redundant import errors for preload extern crate Fixes rust-lang#121915
diff --git 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
@@ -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<ast::NodeId, Span>,
+    ) {
+        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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+

Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,11 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di`
`143`	`143`	`BuiltinLintDiag::RedundantImport(spans, ident) => {`
`144`	`144`	`for (span, is_imported) in spans {`
`145`	`145`	`let introduced = if is_imported { "imported" } else { "defined" };`
`146`		- diag.span_label(span, format!("the item `{ident}` is already {introduced} here"));
	`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`	`+ );`
`147`	`151`	`}`
`148`	`152`	`}`
`149`	`153`	`BuiltinLintDiag::DeprecatedMacro(suggestion, span) => {`
Original file line number	Diff line number	Diff line change
`@@ -1336,9 +1336,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {`
`1336`	`1336`	`}`
`1337`	`1337`
`1338`	`1338`	`let mut is_redundant = true;`
`1339`		`-`
`1340`	`1339`	`let mut redundant_span = PerNS { value_ns: None, type_ns: None, macro_ns: None };`
`1341`		`-`
`1342`	`1340`	`self.per_ns(\|this, ns\| {`
`1343`	`1341`	`if is_redundant && let Ok(binding) = source_bindings[ns].get() {`
`1344`	`1342`	`if binding.res() == Res::Err {`