Rollup merge of #121528 - Alexendoo:unused_qualifications, r=petroche…

…nkov Consider middle segments of paths in `unused_qualifications` Currently `unused_qualifications` looks at the last segment of a path to see if it can be trimmed, this PR extends the check to the middle segments also ```rust // currently linted use std::env::args(); std::env::args(); // Removes `std::env::` ``` ```rust // newly linted use std::env; std::env::args(); // Removes `std::` ``` Paths with generics in them are now linted as long as the part being trimmed is before any generic args, e.g. it will now suggest trimming `std::vec::` from `std::vec::Vec<usize>` Paths with any segments that are from an expansion are no longer linted Fixes #100979 Fixes #96698
rust-lang · Mar 3, 2024 · ed6d175 · ed6d175
2 parents d37ad03 + 4ea9f72
commit ed6d175
Show file tree

Hide file tree

Showing 4 changed files with 151 additions and 29 deletions.
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -4150,34 +4150,36 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             PathResult::Indeterminate => bug!("indeterminate path result in resolve_qpath"),
         };
 
-        if path.len() > 1
-            && let Some(res) = result.full_res()
-            && let Some((&last_segment, prev_segs)) = path.split_last()
-            && prev_segs.iter().all(|seg| !seg.has_generic_args)
-            && res != Res::Err
-            && path[0].ident.name != kw::PathRoot
-            && path[0].ident.name != kw::DollarCrate
-        {
-            let unqualified_result = {
-                match self.resolve_path(&[last_segment], Some(ns), None) {
-                    PathResult::NonModule(path_res) => path_res.expect_full_res(),
-                    PathResult::Module(ModuleOrUniformRoot::Module(module)) => {
-                        module.res().unwrap()
-                    }
-                    _ => return Ok(Some(result)),
-                }
-            };
-            if res == unqualified_result {
-                let lint = lint::builtin::UNUSED_QUALIFICATIONS;
+        if path.iter().all(|seg| !seg.ident.span.from_expansion()) {
+            let end_pos =
+                path.iter().position(|seg| seg.has_generic_args).map_or(path.len(), |pos| pos + 1);
+            let unqualified =
+                path[..end_pos].iter().enumerate().skip(1).rev().find_map(|(i, seg)| {
+                    // Preserve the current namespace for the final path segment, but use the type
+                    // namespace for all preceding segments
+                    //
+                    // e.g. for `std::env::args` check the `ValueNS` for `args` but the `TypeNS` for
+                    // `std` and `env`
+                    //
+                    // If the final path segment is beyond `end_pos` all the segments to check will
+                    // use the type namespace
+                    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)
+                });
+
+            if let Some(unqualified) = unqualified {
                 self.r.lint_buffer.buffer_lint_with_diagnostic(
-                    lint,
+                    lint::builtin::UNUSED_QUALIFICATIONS,
                     finalize.node_id,
                     finalize.path_span,
                     "unnecessary qualification",
                     lint::BuiltinLintDiagnostics::UnusedQualifications {
-                        removal_span: finalize.path_span.until(last_segment.ident.span),
+                        removal_span: finalize.path_span.until(unqualified.ident.span),
                     },
-                )
+                );
             }
         }
 

diff --git a/tests/ui/lint/lint-qualification.fixed b/tests/ui/lint/lint-qualification.fixed
@@ -1,6 +1,6 @@
 //@ run-rustfix
 #![deny(unused_qualifications)]
-#![allow(deprecated)]
+#![allow(deprecated, dead_code)]
 
 mod foo {
     pub fn bar() {}
@@ -9,13 +9,37 @@ mod foo {
 fn main() {
     use foo::bar;
     bar(); //~ ERROR: unnecessary qualification
+    bar(); //~ ERROR: unnecessary qualification
     bar();
 
     let _ = || -> Result<(), ()> { try!(Ok(())); Ok(()) }; // issue #37345
 
-    macro_rules! m { () => {
+    let _ = String::new(); //~ ERROR: unnecessary qualification
+    let _ = std::env::current_dir(); //~ ERROR: unnecessary qualification
+
+    let _: Vec<String> = Vec::<String>::new();
+    //~^ ERROR: unnecessary qualification
+    //~| ERROR: unnecessary qualification
+
+    use std::fmt;
+    let _: fmt::Result = Ok(()); //~ ERROR: unnecessary qualification
+
+    macro_rules! m { ($a:ident, $b:ident) => {
         $crate::foo::bar(); // issue #37357
         ::foo::bar(); // issue #38682
+        foo::bar();
+        foo::$b(); // issue #96698
+        $a::bar();
     } }
-    m!();
+    m!(foo, bar);
+}
+
+mod conflicting_names {
+    mod std {}
+    mod cell {}
+
+    fn f() {
+        let _ = ::std::env::current_dir();
+        let _ = core::cell::Cell::new(1);
+    }
 }
diff --git a/tests/ui/lint/lint-qualification.rs b/tests/ui/lint/lint-qualification.rs
@@ -1,6 +1,6 @@
 //@ run-rustfix
 #![deny(unused_qualifications)]
-#![allow(deprecated)]
+#![allow(deprecated, dead_code)]
 
 mod foo {
     pub fn bar() {}
@@ -9,13 +9,37 @@ mod foo {
 fn main() {
     use foo::bar;
     foo::bar(); //~ ERROR: unnecessary qualification
+    crate::foo::bar(); //~ ERROR: unnecessary qualification
     bar();
 
     let _ = || -> Result<(), ()> { try!(Ok(())); Ok(()) }; // issue #37345
 
-    macro_rules! m { () => {
+    let _ = std::string::String::new(); //~ ERROR: unnecessary qualification
+    let _ = ::std::env::current_dir(); //~ ERROR: unnecessary qualification
+
+    let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
+    //~^ ERROR: unnecessary qualification
+    //~| ERROR: unnecessary qualification
+
+    use std::fmt;
+    let _: std::fmt::Result = Ok(()); //~ ERROR: unnecessary qualification
+
+    macro_rules! m { ($a:ident, $b:ident) => {
         $crate::foo::bar(); // issue #37357
         ::foo::bar(); // issue #38682
+        foo::bar();
+        foo::$b(); // issue #96698
+        $a::bar();
     } }
-    m!();
+    m!(foo, bar);
+}
+
+mod conflicting_names {
+    mod std {}
+    mod cell {}
+
+    fn f() {
+        let _ = ::std::env::current_dir();
+        let _ = core::cell::Cell::new(1);
+    }
 }
diff --git a/tests/ui/lint/lint-qualification.stderr b/tests/ui/lint/lint-qualification.stderr
@@ -15,5 +15,77 @@ LL -     foo::bar();
 LL +     bar();
    |
 
-error: aborting due to 1 previous error
+error: unnecessary qualification
+  --> $DIR/lint-qualification.rs:12:5
+   |
+LL |     crate::foo::bar();
+   |     ^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL -     crate::foo::bar();
+LL +     bar();
+   |
+
+error: unnecessary qualification
+  --> $DIR/lint-qualification.rs:17:13
+   |
+LL |     let _ = std::string::String::new();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL -     let _ = std::string::String::new();
+LL +     let _ = String::new();
+   |
+
+error: unnecessary qualification
+  --> $DIR/lint-qualification.rs:18:13
+   |
+LL |     let _ = ::std::env::current_dir();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL -     let _ = ::std::env::current_dir();
+LL +     let _ = std::env::current_dir();
+   |
+
+error: unnecessary qualification
+  --> $DIR/lint-qualification.rs:20:12
+   |
+LL |     let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
+   |            ^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL -     let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
+LL +     let _: Vec<String> = std::vec::Vec::<String>::new();
+   |
+
+error: unnecessary qualification
+  --> $DIR/lint-qualification.rs:20:36
+   |
+LL |     let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
+   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL -     let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
+LL +     let _: std::vec::Vec<String> = Vec::<String>::new();
+   |
+
+error: unnecessary qualification
+  --> $DIR/lint-qualification.rs:25:12
+   |
+LL |     let _: std::fmt::Result = Ok(());
+   |            ^^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL -     let _: std::fmt::Result = Ok(());
+LL +     let _: fmt::Result = Ok(());
+   |
+
+error: aborting due to 7 previous errors