fixes rust-lang#121331

surechen · Mar 12, 2024 · 118673b · 118673b
1 parent 9fb91aa
commit 118673b
Show file tree

Hide file tree

Showing 18 changed files with 232 additions and 34 deletions.
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -553,6 +553,7 @@ declare_lint! {
     /// }
     ///
     /// fn main() {
+    ///     #[allow(unused_imports)]
     ///     use foo::bar;
     ///     foo::bar();
     /// }

diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -16,7 +16,7 @@ use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, Used, VisResolutio
 
 use rustc_ast::visit::{self, AssocCtxt, Visitor};
 use rustc_ast::{self as ast, AssocItem, AssocItemKind, MetaItemKind, StmtKind};
-use rustc_ast::{Block, ForeignItem, ForeignItemKind, Impl, Item, ItemKind, NodeId};
+use rustc_ast::{AttrVec, Block, ForeignItem, ForeignItemKind, Impl, Item, ItemKind, NodeId};
 use rustc_attr as attr;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::{codes::*, struct_span_code_err, Applicability};
@@ -30,6 +30,7 @@ use rustc_middle::{bug, ty};
 use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::Span;
+use thin_vec::ThinVec;
 
 use std::cell::Cell;
 
@@ -360,6 +361,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
             use_span: item.span,
             use_span_with_attributes: item.span_with_attributes(),
             has_attributes: !item.attrs.is_empty(),
+            has_allow_unused_imports_attribute: check_allow_unused_imports_attr(&item.attrs),
             root_span,
             root_id,
             vis: Cell::new(Some(vis)),
@@ -884,6 +886,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
             parent_scope: self.parent_scope,
             imported_module: Cell::new(module),
             has_attributes: !item.attrs.is_empty(),
+            has_allow_unused_imports_attribute: check_allow_unused_imports_attr(&item.attrs),
             use_span_with_attributes: item.span_with_attributes(),
             use_span: item.span,
             root_span: item.span,
@@ -1091,6 +1094,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
                 imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
                 use_span_with_attributes: item.span_with_attributes(),
                 has_attributes: !item.attrs.is_empty(),
+                has_allow_unused_imports_attribute: check_allow_unused_imports_attr(&item.attrs),
                 use_span: item.span,
                 root_span: span,
                 span,
@@ -1262,6 +1266,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
                     parent_scope: self.parent_scope,
                     imported_module: Cell::new(None),
                     has_attributes: false,
+                    has_allow_unused_imports_attribute: false,
                     use_span_with_attributes: span,
                     use_span: span,
                     root_span: span,
@@ -1534,3 +1539,16 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
         }
     }
 }
+
+fn check_allow_unused_imports_attr(attrs: &AttrVec) -> bool {
+    attrs.iter().any(|attr| {
+        if attr.has_name(sym::allow) {
+            for item in attr.meta_item_list().unwrap_or_else(ThinVec::new) {
+                if item.has_name(sym::unused_imports) {
+                    return true;
+                }
+            }
+        }
+        false
+    })
+}
diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs
@@ -27,14 +27,16 @@ use crate::imports::ImportKind;
 use crate::module_to_string;
 use crate::Resolver;
 
-use crate::NameBindingKind;
+use crate::{LexicalScopeBinding, NameBindingKind};
 use rustc_ast as ast;
 use rustc_ast::visit::{self, Visitor};
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
 use rustc_data_structures::unord::UnordSet;
 use rustc_errors::{pluralize, MultiSpan};
 use rustc_hir::def::{DefKind, Res};
-use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS};
+use rustc_session::lint::builtin::{
+    MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS, UNUSED_QUALIFICATIONS,
+};
 use rustc_session::lint::BuiltinLintDiag;
 use rustc_span::symbol::{kw, Ident};
 use rustc_span::{Span, DUMMY_SP};
@@ -494,6 +496,30 @@ impl Resolver<'_, '_> {
         }
 
         let unused_imports = visitor.unused_imports;
+
+        // The lint fixes for unused_import and unnecessary_qualification may conflict.
+        // Deleting both unused imports and unnecessary segments of an item may result
+        // in the item not being found.
+        for v in &self.potentially_unnecessary_qualification {
+            if let LexicalScopeBinding::Item(name_binding) = v.binding
+                && let NameBindingKind::Import { import, .. } = name_binding.kind
+                && let Some(unused_import) = unused_imports.get(&import.root_id)
+                && let Some(id) = import.id()
+                && unused_import.unused.contains(&id)
+                && !import.has_allow_unused_imports_attribute
+            {
+                continue;
+            }
+
+            self.lint_buffer.buffer_lint_with_diagnostic(
+                UNUSED_QUALIFICATIONS,
+                v.finalize.node_id,
+                v.finalize.path_span,
+                "unnecessary qualification",
+                BuiltinLintDiag::UnusedQualifications { removal_span: v.span },
+            );
+        }
+
         let mut check_redundant_imports = FxIndexSet::default();
         for module in self.arenas.local_modules().iter() {
             for (_key, resolution) in self.resolutions(*module).borrow().iter() {

diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -162,6 +162,10 @@ pub(crate) struct ImportData<'a> {
     /// Did the use statement have any attributes?
     pub has_attributes: bool,
 
+    /// Only for avoiding reporting unnecessary_qualification and unused_imports
+    /// at same time.
+    pub has_allow_unused_imports_attribute: bool,
+
     /// Span of this use tree.
     pub span: Span,
 

diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -580,6 +580,14 @@ impl MaybeExported<'_> {
     }
 }
 
+/// Used for recording UnnecessaryQualification.
+#[derive(Debug)]
+pub(crate) struct UnnecessaryQualification<'a> {
+    pub binding: LexicalScopeBinding<'a>,
+    pub finalize: Finalize,
+    pub span: Span,
+}
+
 #[derive(Default)]
 struct DiagMetadata<'ast> {
     /// The current trait's associated items' ident, used for diagnostic suggestions.
@@ -4653,20 +4661,15 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             let ns = if i + 1 == path.len() { ns } else { TypeNS };
             let res = self.r.partial_res_map.get(&seg.id?)?.full_res()?;
             let binding = self.resolve_ident_in_lexical_scope(seg.ident, ns, None, None)?;
-
-            (res == binding.res()).then_some(seg)
+            (res == binding.clone().res()).then_some((seg, binding))
         });
 
         if let Some(unqualified) = unqualified {
-            self.r.lint_buffer.buffer_lint_with_diagnostic(
-                lint::builtin::UNUSED_QUALIFICATIONS,
-                finalize.node_id,
-                finalize.path_span,
-                "unnecessary qualification",
-                lint::BuiltinLintDiag::UnusedQualifications {
-                    removal_span: path[0].ident.span.until(unqualified.ident.span),
-                },
-            );
+            self.r.potentially_unnecessary_qualification.push(UnnecessaryQualification {
+                binding: unqualified.1,
+                finalize: finalize,
+                span: path[0].ident.span.until(unqualified.0.ident.span),
+            });
         }
     }
 }

diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -68,7 +68,7 @@ use std::fmt;
 
 use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion};
 use imports::{Import, ImportData, ImportKind, NameResolution};
-use late::{HasGenericParams, PathSource, PatternSource};
+use late::{HasGenericParams, PathSource, PatternSource, UnnecessaryQualification};
 use macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
 
 use crate::effective_visibilities::EffectiveVisibilitiesVisitor;
@@ -372,7 +372,7 @@ impl<'a> From<&'a ast::PathSegment> for Segment {
 /// This refers to the thing referred by a name. The difference between `Res` and `Item` is that
 /// items are visible in their whole block, while `Res`es only from the place they are defined
 /// forward.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 enum LexicalScopeBinding<'a> {
     Item(NameBinding<'a>),
     Res(Res),
@@ -1105,6 +1105,8 @@ pub struct Resolver<'a, 'tcx> {
 
     potentially_unused_imports: Vec<Import<'a>>,
 
+    potentially_unnecessary_qualification: Vec<UnnecessaryQualification<'a>>,
+
     /// Table for mapping struct IDs into struct constructor IDs,
     /// it's not used during normal resolution, only for better error reporting.
     /// Also includes of list of each fields visibility
@@ -1464,6 +1466,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             local_macro_def_scopes: FxHashMap::default(),
             name_already_seen: FxHashMap::default(),
             potentially_unused_imports: Vec::new(),
+            potentially_unnecessary_qualification: Default::default(),
             struct_constructors: Default::default(),
             unused_macros: Default::default(),
             unused_macro_rules: Default::default(),

diff --git a/tests/ui/lint/lint-qualification.fixed b/tests/ui/lint/lint-qualification.fixed
@@ -21,6 +21,7 @@ fn main() {
     //~^ ERROR: unnecessary qualification
     //~| ERROR: unnecessary qualification
 
+    #[allow(unused_imports)]
     use std::fmt;
     let _: fmt::Result = Ok(()); //~ ERROR: unnecessary qualification
 

diff --git a/tests/ui/lint/lint-qualification.rs b/tests/ui/lint/lint-qualification.rs
@@ -21,6 +21,7 @@ fn main() {
     //~^ ERROR: unnecessary qualification
     //~| ERROR: unnecessary qualification
 
+    #[allow(unused_imports)]
     use std::fmt;
     let _: std::fmt::Result = Ok(()); //~ ERROR: unnecessary qualification
 

diff --git a/tests/ui/lint/lint-qualification.stderr b/tests/ui/lint/lint-qualification.stderr
@@ -76,7 +76,7 @@ LL +     let _: std::vec::Vec<String> = Vec::<String>::new();
    |
 
 error: unnecessary qualification
-  --> $DIR/lint-qualification.rs:25:12
+  --> $DIR/lint-qualification.rs:26:12
    |
 LL |     let _: std::fmt::Result = Ok(());
    |            ^^^^^^^^^^^^^^^^
@@ -88,7 +88,7 @@ LL +     let _: fmt::Result = Ok(());
    |
 
 error: unnecessary qualification
-  --> $DIR/lint-qualification.rs:27:13
+  --> $DIR/lint-qualification.rs:28:13
    |
 LL |     let _ = <bool as ::std::default::Default>::default(); // issue #121999
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff --git a/tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.fixed b/tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.fixed
@@ -0,0 +1,49 @@
+//@ run-rustfix
+//@ edition:2021
+#![deny(unused_imports)]
+#![deny(unused_qualifications)]
+#![feature(coroutines, coroutine_trait)]
+
+use std::ops::{
+    Coroutine,
+    CoroutineState::{self},
+    //~^ ERROR unused import: `*`
+};
+use std::pin::Pin;
+
+#[allow(dead_code)]
+fn finish<T>(mut amt: usize, mut t: T) -> T::Return
+    where T: Coroutine<(), Yield = ()> + Unpin,
+{
+    loop {
+        match Pin::new(&mut t).resume(()) {
+            CoroutineState::Yielded(()) => amt = amt.checked_sub(1).unwrap(),
+            CoroutineState::Complete(ret) => {
+                assert_eq!(amt, 0);
+                return ret
+            }
+        }
+    }
+}
+
+
+mod foo {
+    pub fn bar() {}
+}
+
+mod baz {
+    pub fn qux() {}
+}
+
+pub fn main() {
+
+
+    //~^ ERROR unused import: `foo::bar`
+    foo::bar();
+
+    #[allow(unused_imports)]
+    use baz::qux;
+    qux();
+    //~^ ERROR unnecessary qualification
+
+}
diff --git a/tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.rs b/tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.rs
@@ -0,0 +1,49 @@
+//@ run-rustfix
+//@ edition:2021
+#![deny(unused_imports)]
+#![deny(unused_qualifications)]
+#![feature(coroutines, coroutine_trait)]
+
+use std::ops::{
+    Coroutine,
+    CoroutineState::{self, *},
+    //~^ ERROR unused import: `*`
+};
+use std::pin::Pin;
+
+#[allow(dead_code)]
+fn finish<T>(mut amt: usize, mut t: T) -> T::Return
+    where T: Coroutine<(), Yield = ()> + Unpin,
+{
+    loop {
+        match Pin::new(&mut t).resume(()) {
+            CoroutineState::Yielded(()) => amt = amt.checked_sub(1).unwrap(),
+            CoroutineState::Complete(ret) => {
+                assert_eq!(amt, 0);
+                return ret
+            }
+        }
+    }
+}
+
+
+mod foo {
+    pub fn bar() {}
+}
+
+mod baz {
+    pub fn qux() {}
+}
+
+pub fn main() {
+
+    use foo::bar;
+    //~^ ERROR unused import: `foo::bar`
+    foo::bar();
+
+    #[allow(unused_imports)]
+    use baz::qux;
+    baz::qux();
+    //~^ ERROR unnecessary qualification
+
+}
diff --git a/tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.stderr b/tests/ui/lint/unnecessary-qualification/lint-unnecessary-qualification-issue-121331.stderr
@@ -0,0 +1,37 @@
+error: unused import: `*`
+  --> $DIR/lint-unnecessary-qualification-issue-121331.rs:9:28
+   |
+LL |     CoroutineState::{self, *},
+   |                            ^
+   |
+note: the lint level is defined here
+  --> $DIR/lint-unnecessary-qualification-issue-121331.rs:3:9
+   |
+LL | #![deny(unused_imports)]
+   |         ^^^^^^^^^^^^^^
+
+error: unused import: `foo::bar`
+  --> $DIR/lint-unnecessary-qualification-issue-121331.rs:40:9
+   |
+LL |     use foo::bar;
+   |         ^^^^^^^^
+
+error: unnecessary qualification
+  --> $DIR/lint-unnecessary-qualification-issue-121331.rs:46:5
+   |
+LL |     baz::qux();
+   |     ^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/lint-unnecessary-qualification-issue-121331.rs:4:9
+   |
+LL | #![deny(unused_qualifications)]
+   |         ^^^^^^^^^^^^^^^^^^^^^
+help: remove the unnecessary path segments
+   |
+LL -     baz::qux();
+LL +     qux();
+   |
+
+error: aborting due to 3 previous errors
+
diff --git a/tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.fixed b/tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.fixed
@@ -6,6 +6,7 @@
 
 #[allow(unused_imports)]
 use std::ops;
+#[allow(unused_imports)]
 use std::ops::Index;
 
 pub struct A;

diff --git a/tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.rs b/tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.rs
@@ -6,6 +6,7 @@
 
 #[allow(unused_imports)]
 use std::ops;
+#[allow(unused_imports)]
 use std::ops::Index;
 
 pub struct A;