Rollup merge of rust-lang#113811 - jieyouxu:fix-unused-qualifications-suggestion, r=oli-obk

Fix removal span calculation of `unused_qualifications` suggestion

Given a path such as `std::ops::Index<str>`, calculate the unnecessary qualification removal span by computing the beginning of the entire span until the ident span of the last path segment, which handles generic arguments and lifetime arguments in the last path segment. Previous logic only kept the ident span of the last path segment which is incorrect.

Closes rust-lang#113808.

Author: matthiaskrgr

compiler/rustc_lint/src/context.rs

@@ -956,11 +956,11 @@ pub trait LintContext: Sized {
                     db.span_note(glob_reexport_span, format!("the name `{}` in the {} namespace is supposed to be publicly re-exported here", name, namespace));
                     db.span_note(private_item_span, "but the private item here shadows it".to_owned());
                 }
-                BuiltinLintDiagnostics::UnusedQualifications { path_span, unqualified_path } => {
+                BuiltinLintDiagnostics::UnusedQualifications { removal_span } => {
                     db.span_suggestion_verbose(
-                        path_span,
-                        "replace it with the unqualified path",
-                        unqualified_path,
+                        removal_span,
+                        "remove the unnecessary path segments",
+                        "",
                         Applicability::MachineApplicable
                     );
                 }

compiler/rustc_lint_defs/src/lib.rs

@@ -551,10 +551,8 @@ pub enum BuiltinLintDiagnostics {
         private_item_span: Span,
     },
     UnusedQualifications {
-        /// The span of the unnecessarily-qualified path.
-        path_span: Span,
-        /// The replacement unqualified path.
-        unqualified_path: Ident,
+        /// The span of the unnecessarily-qualified path to remove.
+        removal_span: Span,
     },
 }
 

compiler/rustc_resolve/src/late.rs

@@ -3911,8 +3911,9 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             && path[0].ident.name != kw::PathRoot
             && path[0].ident.name != kw::DollarCrate
         {
+            let last_segment = *path.last().unwrap();
             let unqualified_result = {
-                match self.resolve_path(&[*path.last().unwrap()], Some(ns), None) {
+                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()
@@ -3928,8 +3929,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
                     finalize.path_span,
                     "unnecessary qualification",
                     lint::BuiltinLintDiagnostics::UnusedQualifications {
-                        path_span: finalize.path_span,
-                        unqualified_path: path.last().unwrap().ident
+                        removal_span: finalize.path_span.until(last_segment.ident.span),
                     }
                 )
             }

tests/ui/lint/lint-qualification.fixed

@@ -0,0 +1,21 @@
+// run-rustfix
+#![deny(unused_qualifications)]
+#![allow(deprecated)]
+
+mod foo {
+    pub fn bar() {}
+}
+
+fn main() {
+    use foo::bar;
+    bar(); //~ ERROR: unnecessary qualification
+    bar();
+
+    let _ = || -> Result<(), ()> { try!(Ok(())); Ok(()) }; // issue #37345
+
+    macro_rules! m { () => {
+        $crate::foo::bar(); // issue #37357
+        ::foo::bar(); // issue #38682
+    } }
+    m!();
+}

tests/ui/lint/lint-qualification.rs

@@ -1,3 +1,4 @@
+// run-rustfix
 #![deny(unused_qualifications)]
 #![allow(deprecated)]
 

tests/ui/lint/lint-qualification.stderr

@@ -1,18 +1,19 @@
 error: unnecessary qualification
-  --> $DIR/lint-qualification.rs:10:5
+  --> $DIR/lint-qualification.rs:11:5
    |
 LL |     foo::bar();
    |     ^^^^^^^^
    |
 note: the lint level is defined here
-  --> $DIR/lint-qualification.rs:1:9
+  --> $DIR/lint-qualification.rs:2:9
    |
 LL | #![deny(unused_qualifications)]
    |         ^^^^^^^^^^^^^^^^^^^^^
-help: replace it with the unqualified path
+help: remove the unnecessary path segments
+   |
+LL -     foo::bar();
+LL +     bar();
    |
-LL |     bar();
-   |     ~~~
 
 error: aborting due to previous error
 

tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.fixed

@@ -0,0 +1,31 @@
+// run-rustfix
+
+#![deny(unused_qualifications)]
+#![feature(unsized_fn_params)]
+
+#[allow(unused_imports)]
+use std::ops;
+use std::ops::Index;
+
+pub struct A;
+
+impl Index<str> for A {
+    //~^ ERROR unnecessary qualification
+    type Output = ();
+    fn index(&self, _: str) -> &Self::Output {
+        &()
+    }
+}
+
+mod inner {
+    pub trait Trait<T> {}
+}
+
+// the import needs to be here for the lint to show up
+#[allow(unused_imports)]
+use inner::Trait;
+
+impl Trait<u8> for () {}
+//~^ ERROR unnecessary qualification
+
+fn main() {}

tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.rs

@@ -0,0 +1,31 @@
+// run-rustfix
+
+#![deny(unused_qualifications)]
+#![feature(unsized_fn_params)]
+
+#[allow(unused_imports)]
+use std::ops;
+use std::ops::Index;
+
+pub struct A;
+
+impl ops::Index<str> for A {
+    //~^ ERROR unnecessary qualification
+    type Output = ();
+    fn index(&self, _: str) -> &Self::Output {
+        &()
+    }
+}
+
+mod inner {
+    pub trait Trait<T> {}
+}
+
+// the import needs to be here for the lint to show up
+#[allow(unused_imports)]
+use inner::Trait;
+
+impl inner::Trait<u8> for () {}
+//~^ ERROR unnecessary qualification
+
+fn main() {}

tests/ui/resolve/issue-113808-invalid-unused-qualifications-suggestion.stderr

@@ -0,0 +1,31 @@
+error: unnecessary qualification
+  --> $DIR/issue-113808-invalid-unused-qualifications-suggestion.rs:12:6
+   |
+LL | impl ops::Index<str> for A {
+   |      ^^^^^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/issue-113808-invalid-unused-qualifications-suggestion.rs:3:9
+   |
+LL | #![deny(unused_qualifications)]
+   |         ^^^^^^^^^^^^^^^^^^^^^
+help: remove the unnecessary path segments
+   |
+LL - impl ops::Index<str> for A {
+LL + impl Index<str> for A {
+   |
+
+error: unnecessary qualification
+  --> $DIR/issue-113808-invalid-unused-qualifications-suggestion.rs:28:6
+   |
+LL | impl inner::Trait<u8> for () {}
+   |      ^^^^^^^^^^^^^^^^
+   |
+help: remove the unnecessary path segments
+   |
+LL - impl inner::Trait<u8> for () {}
+LL + impl Trait<u8> for () {}
+   |
+
+error: aborting due to 2 previous errors
+

tests/ui/resolve/unused-qualifications-suggestion.stderr

@@ -9,21 +9,23 @@ note: the lint level is defined here
    |
 LL | #![deny(unused_qualifications)]
    |         ^^^^^^^^^^^^^^^^^^^^^
-help: replace it with the unqualified path
+help: remove the unnecessary path segments
+   |
+LL -     foo::bar();
+LL +     bar();
    |
-LL |     bar();
-   |     ~~~
 
 error: unnecessary qualification
   --> $DIR/unused-qualifications-suggestion.rs:21:5
    |
 LL |     baz::qux::quux();
    |     ^^^^^^^^^^^^^^
    |
-help: replace it with the unqualified path
+help: remove the unnecessary path segments
+   |
+LL -     baz::qux::quux();
+LL +     quux();
    |
-LL |     quux();
-   |     ~~~~
 
 error: aborting due to 2 previous errors