Use hidden suggestions for unused imports lint

estebank · estebank · commit 87dd2e1df95f · 2019-02-11T11:16:22.000-08:00
diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs
@@ -557,7 +557,7 @@ impl BuiltinLintDiagnostics {
             }
             BuiltinLintDiagnostics::UnusedImports(message, replaces) => {
                 if !replaces.is_empty() {
-                    db.multipart_suggestion(
+                    db.tool_only_multipart_suggestion(
                         &message,
                         replaces,
                         Applicability::MachineApplicable,
diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs
@@ -250,6 +250,32 @@ impl Diagnostic {
         self
     }
 
+    /// Prints out a message with for a multipart suggestion without showing the suggested code.
+    ///
+    /// This is intended to be used for suggestions that are obvious in what the changes need to
+    /// be from the message, showing the span label inline would be visually unpleasant
+    /// (marginally overlapping spans or multiline spans) and showing the snippet window wouldn't
+    /// improve understandability.
+    pub fn tool_only_multipart_suggestion(
+        &mut self,
+        msg: &str,
+        suggestion: Vec<(Span, String)>,
+        applicability: Applicability,
+    ) -> &mut Self {
+        self.suggestions.push(CodeSuggestion {
+            substitutions: vec![Substitution {
+                parts: suggestion
+                    .into_iter()
+                    .map(|(span, snippet)| SubstitutionPart { snippet, span })
+                    .collect(),
+            }],
+            msg: msg.to_owned(),
+            style: SuggestionStyle::CompletelyHidden,
+            applicability,
+        });
+        self
+    }
+
     /// Prints out a message with a suggested edit of the code.
     ///
     /// In case of short messages and a simple suggestion, rustc displays it as a label:
@@ -318,7 +344,7 @@ impl Diagnostic {
             }],
             msg: msg.to_owned(),
             style: SuggestionStyle::HideCodeInline,
-            applicability: applicability,
+            applicability,
         });
         self
     }
@@ -341,7 +367,7 @@ impl Diagnostic {
             }],
             msg: msg.to_owned(),
             style: SuggestionStyle::HideCodeInline,
-            applicability: applicability,
+            applicability,
         });
         self
     }
diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs
@@ -205,6 +205,24 @@ impl<'a> DiagnosticBuilder<'a> {
         self
     }
 
+    pub fn tool_only_multipart_suggestion(
+        &mut self,
+        msg: &str,
+        suggestion: Vec<(Span, String)>,
+        applicability: Applicability,
+    ) -> &mut Self {
+        if !self.allow_suggestions {
+            return self
+        }
+        self.diagnostic.tool_only_multipart_suggestion(
+            msg,
+            suggestion,
+            applicability,
+        );
+        self
+    }
+
+
     pub fn span_suggestion(
         &mut self,
         sp: Span,
diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
@@ -48,7 +48,9 @@ impl Emitter for EmitterWriter {
                // don't display multiline suggestions as labels
                !sugg.substitutions[0].parts[0].snippet.contains('\n') &&
                // when this style is set we want the suggestion to be a message, not inline
-               sugg.style != SuggestionStyle::HideCodeAlways
+               sugg.style != SuggestionStyle::HideCodeAlways &&
+               // trivial suggestion for tooling's sake, never shown
+               sugg.style != SuggestionStyle::CompletelyHidden
             {
                 let substitution = &sugg.substitutions[0].parts[0].snippet.trim();
                 let msg = if substitution.len() == 0 || sugg.style.hide_inline() {
diff --git a/src/test/ui/bad/bad-lint-cap2.stderr b/src/test/ui/bad/bad-lint-cap2.stderr
@@ -2,7 +2,7 @@ error: unused import: `std::option`
   --> $DIR/bad-lint-cap2.rs:6:5
    |
 LL | use std::option; //~ ERROR
-   | ----^^^^^^^^^^^- help: remove the whole `use` item
+   |     ^^^^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/bad-lint-cap2.rs:4:9
diff --git a/src/test/ui/bad/bad-lint-cap3.stderr b/src/test/ui/bad/bad-lint-cap3.stderr
@@ -2,7 +2,7 @@ warning: unused import: `std::option`
   --> $DIR/bad-lint-cap3.rs:7:5
    |
 LL | use std::option; //~ WARN
-   | ----^^^^^^^^^^^- help: remove the whole `use` item
+   |     ^^^^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/bad-lint-cap3.rs:4:9
diff --git a/src/test/ui/imports/unused.stderr b/src/test/ui/imports/unused.stderr
@@ -2,7 +2,7 @@ error: unused import: `super::f`
   --> $DIR/unused.rs:7:24
    |
 LL |         pub(super) use super::f; //~ ERROR unused
-   |         ---------------^^^^^^^^- help: remove the whole `use` item
+   |                        ^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/unused.rs:1:9
diff --git a/src/test/ui/issues/issue-30730.stderr b/src/test/ui/issues/issue-30730.stderr
@@ -2,7 +2,7 @@ error: unused import: `std::thread`
   --> $DIR/issue-30730.rs:3:5
    |
 LL | use std::thread;
-   | ----^^^^^^^^^^^- help: remove the whole `use` item
+   |     ^^^^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/issue-30730.rs:2:9
diff --git a/src/test/ui/lint/lint-directives-on-use-items-issue-10534.stderr b/src/test/ui/lint/lint-directives-on-use-items-issue-10534.stderr
@@ -2,7 +2,7 @@ error: unused import: `a::x`
   --> $DIR/lint-directives-on-use-items-issue-10534.rs:12:9
    |
 LL |     use a::x; //~ ERROR: unused import
-   |     ----^^^^- help: remove the whole `use` item
+   |         ^^^^
    |
 note: lint level defined here
   --> $DIR/lint-directives-on-use-items-issue-10534.rs:1:9
@@ -14,7 +14,7 @@ error: unused import: `a::y`
   --> $DIR/lint-directives-on-use-items-issue-10534.rs:21:9
    |
 LL |     use a::y; //~ ERROR: unused import
-   |     ----^^^^- help: remove the whole `use` item
+   |         ^^^^
    |
 note: lint level defined here
   --> $DIR/lint-directives-on-use-items-issue-10534.rs:20:12
diff --git a/src/test/ui/lint/lint-unused-imports.stderr b/src/test/ui/lint/lint-unused-imports.stderr
@@ -2,7 +2,7 @@ error: unused import: `std::fmt::{}`
   --> $DIR/lint-unused-imports.rs:8:5
    |
 LL | use std::fmt::{};
-   | ----^^^^^^^^^^^^- help: remove the whole `use` item
+   |     ^^^^^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/lint-unused-imports.rs:1:9
@@ -14,39 +14,37 @@ error: unused imports: `None`, `Some`
   --> $DIR/lint-unused-imports.rs:12:27
    |
 LL | use std::option::Option::{Some, None};
-   | --------------------------^^^^--^^^^-- help: remove the whole `use` item
+   |                           ^^^^  ^^^^
 
 error: unused import: `test::A`
   --> $DIR/lint-unused-imports.rs:15:5
    |
 LL | use test::A;       //~ ERROR unused import: `test::A`
-   | ----^^^^^^^- help: remove the whole `use` item
+   |     ^^^^^^^
 
 error: unused import: `bar`
   --> $DIR/lint-unused-imports.rs:24:18
    |
 LL | use test2::{foo, bar}; //~ ERROR unused import: `bar`
-   |                --^^^
-   |                |
-   |                help: remove the unused import
+   |                  ^^^
 
 error: unused import: `foo::Square`
   --> $DIR/lint-unused-imports.rs:52:13
    |
 LL |         use foo::Square; //~ ERROR unused import: `foo::Square`
-   |         ----^^^^^^^^^^^- help: remove the whole `use` item
+   |             ^^^^^^^^^^^
 
 error: unused import: `self::g`
   --> $DIR/lint-unused-imports.rs:68:9
    |
 LL |     use self::g; //~ ERROR unused import: `self::g`
-   |     ----^^^^^^^- help: remove the whole `use` item
+   |         ^^^^^^^
 
 error: unused import: `test2::foo`
   --> $DIR/lint-unused-imports.rs:77:9
    |
 LL |     use test2::foo; //~ ERROR unused import: `test2::foo`
-   |     ----^^^^^^^^^^- help: remove the whole `use` item
+   |         ^^^^^^^^^^
 
 error: unused import: `test::B2`
   --> $DIR/lint-unused-imports.rs:20:5
diff --git a/src/test/ui/lint/lints-in-foreign-macros.stderr b/src/test/ui/lint/lints-in-foreign-macros.stderr
@@ -2,7 +2,7 @@ warning: unused import: `std::string::ToString`
   --> $DIR/lints-in-foreign-macros.rs:11:16
    |
 LL |     () => {use std::string::ToString;} //~ WARN: unused import
-   |            ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item
+   |                ^^^^^^^^^^^^^^^^^^^^^
 ...
 LL | mod a { foo!(); }
    |         ------- in this macro invocation
@@ -17,13 +17,13 @@ warning: unused import: `std::string::ToString`
   --> $DIR/lints-in-foreign-macros.rs:16:18
    |
 LL | mod c { baz!(use std::string::ToString;); } //~ WARN: unused import
-   |              ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item
+   |                  ^^^^^^^^^^^^^^^^^^^^^
 
 warning: unused import: `std::string::ToString`
   --> $DIR/lints-in-foreign-macros.rs:17:19
    |
 LL | mod d { baz2!(use std::string::ToString;); } //~ WARN: unused import
-   |               ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item
+   |                   ^^^^^^^^^^^^^^^^^^^^^
 
 warning: missing documentation for crate
   --> $DIR/lints-in-foreign-macros.rs:4:1
diff --git a/src/test/ui/rfc-2166-underscore-imports/basic.stderr b/src/test/ui/rfc-2166-underscore-imports/basic.stderr
@@ -2,7 +2,7 @@ warning: unused import: `m::Tr1 as _`
   --> $DIR/basic.rs:26:9
    |
 LL |     use m::Tr1 as _; //~ WARN unused import
-   |     ----^^^^^^^^^^^- help: remove the whole `use` item
+   |         ^^^^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/basic.rs:4:9
@@ -14,5 +14,5 @@ warning: unused import: `S as _`
   --> $DIR/basic.rs:27:9
    |
 LL |     use S as _; //~ WARN unused import
-   |     ----^^^^^^- help: remove the whole `use` item
+   |         ^^^^^^
 
diff --git a/src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr b/src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr
@@ -2,7 +2,7 @@ error: unused import: `core::any`
   --> $DIR/unused-2018.rs:6:9
    |
 LL |     use core::any; //~ ERROR unused import: `core::any`
-   |     ----^^^^^^^^^- help: remove the whole `use` item
+   |         ^^^^^^^^^
    |
 note: lint level defined here
   --> $DIR/unused-2018.rs:3:9
@@ -14,7 +14,7 @@ error: unused import: `core`
   --> $DIR/unused-2018.rs:10:9
    |
 LL |     use core; //~ ERROR unused import: `core`
-   |     ----^^^^- help: remove the whole `use` item
+   |         ^^^^
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/span/multispan-import-lint.stderr b/src/test/ui/span/multispan-import-lint.stderr
@@ -10,8 +10,4 @@ note: lint level defined here
 LL | #![warn(unused)]
    |         ^^^^^^
    = note: #[warn(unused_imports)] implied by #[warn(unused)]
-help: remove the unused imports
-   |
-LL | use std::cmp::{min};
-   |               -- --
 
diff --git a/src/test/ui/use/use-nested-groups-unused-imports.stderr b/src/test/ui/use/use-nested-groups-unused-imports.stderr
@@ -2,7 +2,7 @@ error: unused imports: `*`, `Foo`, `baz::{}`, `foobar::*`
   --> $DIR/use-nested-groups-unused-imports.rs:16:11
    |
 LL | use foo::{Foo, bar::{baz::{}, foobar::*}, *};
-   | ----------^^^--------^^^^^^^--^^^^^^^^^---^-- help: remove the whole `use` item
+   |           ^^^        ^^^^^^^  ^^^^^^^^^   ^
    |
 note: lint level defined here
   --> $DIR/use-nested-groups-unused-imports.rs:3:9
@@ -14,15 +14,13 @@ error: unused import: `*`
   --> $DIR/use-nested-groups-unused-imports.rs:18:24
    |
 LL | use foo::bar::baz::{*, *};
-   |                      --^
-   |                      |
-   |                      help: remove the unused import
+   |                        ^
 
 error: unused import: `foo::{}`
   --> $DIR/use-nested-groups-unused-imports.rs:20:5
    |
 LL | use foo::{};
-   | ----^^^^^^^- help: remove the whole `use` item
+   |     ^^^^^^^
 
 error: aborting due to 3 previous errors
 

Original file line number	Diff line number	Diff line change
`@@ -557,7 +557,7 @@ impl BuiltinLintDiagnostics {`
`557`	`557`	`}`
`558`	`558`	`BuiltinLintDiagnostics::UnusedImports(message, replaces) => {`
`559`	`559`	`if !replaces.is_empty() {`
`560`		`- db.multipart_suggestion(`
	`560`	`+ db.tool_only_multipart_suggestion(`
`561`	`561`	`&message,`
`562`	`562`	`replaces,`
`563`	`563`	`Applicability::MachineApplicable,`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: unused import: `std::option`
`2`	`2`	`--> $DIR/bad-lint-cap2.rs:6:5`
`3`	`3`	`\|`
`4`	`4`	`LL \| use std::option; //~ ERROR`
`5`		- \| ----^^^^^^^^^^^- help: remove the whole `use` item
	`5`	`+ \| ^^^^^^^^^^^`
`6`	`6`	`\|`
`7`	`7`	`note: lint level defined here`
`8`	`8`	`--> $DIR/bad-lint-cap2.rs:4:9`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ warning: unused import: `std::option`
`2`	`2`	`--> $DIR/bad-lint-cap3.rs:7:5`
`3`	`3`	`\|`
`4`	`4`	`LL \| use std::option; //~ WARN`
`5`		- \| ----^^^^^^^^^^^- help: remove the whole `use` item
	`5`	`+ \| ^^^^^^^^^^^`
`6`	`6`	`\|`
`7`	`7`	`note: lint level defined here`
`8`	`8`	`--> $DIR/bad-lint-cap3.rs:4:9`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: unused import: `super::f`
`2`	`2`	`--> $DIR/unused.rs:7:24`
`3`	`3`	`\|`
`4`	`4`	`LL \| pub(super) use super::f; //~ ERROR unused`
`5`		- \| ---------------^^^^^^^^- help: remove the whole `use` item
	`5`	`+ \| ^^^^^^^^`
`6`	`6`	`\|`
`7`	`7`	`note: lint level defined here`
`8`	`8`	`--> $DIR/unused.rs:1:9`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: unused import: `std::thread`
`2`	`2`	`--> $DIR/issue-30730.rs:3:5`
`3`	`3`	`\|`
`4`	`4`	`LL \| use std::thread;`
`5`		- \| ----^^^^^^^^^^^- help: remove the whole `use` item
	`5`	`+ \| ^^^^^^^^^^^`
`6`	`6`	`\|`
`7`	`7`	`note: lint level defined here`
`8`	`8`	`--> $DIR/issue-30730.rs:2:9`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: unused import: `a::x`
`2`	`2`	`--> $DIR/lint-directives-on-use-items-issue-10534.rs:12:9`
`3`	`3`	`\|`
`4`	`4`	`LL \| use a::x; //~ ERROR: unused import`
`5`		- \| ----^^^^- help: remove the whole `use` item
	`5`	`+ \| ^^^^`
`6`	`6`	`\|`
`7`	`7`	`note: lint level defined here`
`8`	`8`	`--> $DIR/lint-directives-on-use-items-issue-10534.rs:1:9`
@@ -14,7 +14,7 @@ error: unused import: `a::y`
`14`	`14`	`--> $DIR/lint-directives-on-use-items-issue-10534.rs:21:9`
`15`	`15`	`\|`
`16`	`16`	`LL \| use a::y; //~ ERROR: unused import`
`17`		- \| ----^^^^- help: remove the whole `use` item
	`17`	`+ \| ^^^^`
`18`	`18`	`\|`
`19`	`19`	`note: lint level defined here`
`20`	`20`	`--> $DIR/lint-directives-on-use-items-issue-10534.rs:20:12`