Auto merge of #45489 - oli-obk:json_diagnostics, r=petrochenkov

Fix a quadradic duplication in json for multi-suggestions

r? @petrochenkov

6 files changed, 44 insertions, 28 deletions

diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs
index db49ab10343..b8151819bff 100644
--- a/src/libsyntax/json.rs
+++ b/src/libsyntax/json.rs
@@ -153,17 +153,15 @@ impl Diagnostic {
     fn from_diagnostic_builder(db: &DiagnosticBuilder,
                                je: &JsonEmitter)
                                -> Diagnostic {
-        let sugg = db.suggestions.iter().flat_map(|sugg| {
-            je.render(sugg).into_iter().map(move |rendered| {
-                Diagnostic {
-                    message: sugg.msg.clone(),
-                    code: None,
-                    level: "help",
-                    spans: DiagnosticSpan::from_suggestion(sugg, je),
-                    children: vec![],
-                    rendered: Some(rendered),
-                }
-            })
+        let sugg = db.suggestions.iter().map(|sugg| {
+            Diagnostic {
+                message: sugg.msg.clone(),
+                code: None,
+                level: "help",
+                spans: DiagnosticSpan::from_suggestion(sugg, je),
+                children: vec![],
+                rendered: None,
+            }
         });
         Diagnostic {
             message: db.message(),
@@ -356,9 +354,3 @@ impl DiagnosticCode {
         })
     }
 }
-
-impl JsonEmitter {
-    fn render(&self, suggestion: &CodeSuggestion) -> Vec<String> {
-        suggestion.splice_lines(&*self.cm).iter().map(|line| line.0.to_owned()).collect()
-    }
-}
diff --git a/src/test/compile-fail/issue-27842.rs b/src/test/compile-fail/issue-27842.rs
index 8c71761df2f..eb28e36dc07 100644
--- a/src/test/compile-fail/issue-27842.rs
+++ b/src/test/compile-fail/issue-27842.rs
@@ -14,7 +14,7 @@ fn main() {
     let _ = tup[0];
     //~^ ERROR cannot index into a value of type
     //~| HELP to access tuple elements, use
-    //~| SUGGESTION let _ = tup.0
+    //~| SUGGESTION tup.0
 
     // the case where we show just a general hint
     let i = 0_usize;
diff --git a/src/test/ui/lint/unused_parens_json_suggestion.stderr b/src/test/ui/lint/unused_parens_json_suggestion.stderr
index 4b4dd92855d..701334d8890 100644
--- a/src/test/ui/lint/unused_parens_json_suggestion.stderr
+++ b/src/test/ui/lint/unused_parens_json_suggestion.stderr
@@ -1 +1 @@
-{"message":"unnecessary parentheses around assigned value","code":null,"level":"warning","spans":[{"file_name":"$DIR/unused_parens_json_suggestion.rs","byte_start":976,"byte_end":989,"line_start":22,"line_end":22,"column_start":14,"column_end":27,"is_primary":true,"text":[{"text":"    let _a = (1 / (2 + 3));","highlight_start":14,"highlight_end":27}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[{"message":"#[warn(unused_parens)] on by default","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"remove these parentheses","code":null,"level":"help","spans":[{"file_name":"$DIR/unused_parens_json_suggestion.rs","byte_start":976,"byte_end":989,"line_start":22,"line_end":22,"column_start":14,"column_end":27,"is_primary":true,"text":[{"text":"    let _a = (1 / (2 + 3));","highlight_start":14,"highlight_end":27}],"label":null,"suggested_replacement":"1 / (2 + 3)","expansion":null}],"children":[],"rendered":"    let _a = 1 / (2 + 3);"}],"rendered":null}
+{"message":"unnecessary parentheses around assigned value","code":null,"level":"warning","spans":[{"file_name":"$DIR/unused_parens_json_suggestion.rs","byte_start":976,"byte_end":989,"line_start":22,"line_end":22,"column_start":14,"column_end":27,"is_primary":true,"text":[{"text":"    let _a = (1 / (2 + 3));","highlight_start":14,"highlight_end":27}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[{"message":"#[warn(unused_parens)] on by default","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"remove these parentheses","code":null,"level":"help","spans":[{"file_name":"$DIR/unused_parens_json_suggestion.rs","byte_start":976,"byte_end":989,"line_start":22,"line_end":22,"column_start":14,"column_end":27,"is_primary":true,"text":[{"text":"    let _a = (1 / (2 + 3));","highlight_start":14,"highlight_end":27}],"label":null,"suggested_replacement":"1 / (2 + 3)","expansion":null}],"children":[],"rendered":null}],"rendered":null}
diff --git a/src/test/ui/lint/use_suggestion_json.rs b/src/test/ui/lint/use_suggestion_json.rs
new file mode 100644
index 00000000000..20c24d64050
--- /dev/null
+++ b/src/test/ui/lint/use_suggestion_json.rs
@@ -0,0 +1,21 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: --error-format json
+
+// The output for humans should just highlight the whole span without showing
+// the suggested replacement, but we also want to test that suggested
+// replacement only removes one set of parentheses, rather than naïvely
+// stripping away any starting or ending parenthesis characters—hence this
+// test of the JSON error format.
+
+fn main() {
+    let x: Iter;
+}
diff --git a/src/test/ui/lint/use_suggestion_json.stderr b/src/test/ui/lint/use_suggestion_json.stderr
new file mode 100644
index 00000000000..2ebe212b93d
--- /dev/null
+++ b/src/test/ui/lint/use_suggestion_json.stderr
@@ -0,0 +1,2 @@
+{"message":"cannot find type `Iter` in this scope","code":{"code":"E0412","explanation":"/nThe type name used is not in scope./n/nErroneous code examples:/n/n```compile_fail,E0412/nimpl Something {} // error: type name `Something` is not in scope/n/n// or:/n/ntrait Foo {/n    fn bar(N); // error: type name `N` is not in scope/n}/n/n// or:/n/nfn foo(x: T) {} // type name `T` is not in scope/n```/n/nTo fix this error, please verify you didn't misspell the type name, you did/ndeclare it or imported it into the scope. Examples:/n/n```/nstruct Something;/n/nimpl Something {} // ok!/n/n// or:/n/ntrait Foo {/n    type N;/n/n    fn bar(_: Self::N); // ok!/n}/n/n// or:/n/nfn foo<T>(x: T) {} // ok!/n```/n/nAnother case that causes this error is when a type is imported into a parent/nmodule. To fix this, you can follow the suggestion and use File directly or/n`use super::File;` which will import the types from the parent namespace. An/nexample that causes this error is below:/n/n```compile_fail,E0412/nuse std::fs::File;/n/nmod foo {/n    fn some_function(f: File) {}/n}/n```/n/n```/nuse std::fs::File;/n/nmod foo {/n    // either/n    use super::File;/n    // or/n    // use std::fs::File;/n    fn foo(f: File) {}/n}/n# fn main() {} // don't insert it for us; that'll break imports/n```/n"},"level":"error","spans":[{"file_name":"$DIR/use_suggestion_json.rs","byte_start":862,"byte_end":866,"line_start":20,"line_end":20,"column_start":12,"column_end":16,"is_primary":true,"text":[{"text":"    let x: Iter;","highlight_start":12,"highlight_end":16}],"label":"not found in this scope","suggested_replacement":null,"expansion":null}],"children":[{"message":"possible candidates are found in other modules, you can import them into scope","code":null,"level":"help","spans":[{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::binary_heap::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::btree_map::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::btree_set::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::hash_map::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::hash_set::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::linked_list::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::collections::vec_deque::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::option::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::path::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::result::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::slice::Iter;/n/n","expansion":null},{"file_name":"$DIR/use_suggestion_json.rs","byte_start":839,"byte_end":839,"line_start":19,"line_end":19,"column_start":1,"column_end":1,"is_primary":true,"text":[{"text":"fn main() {","highlight_start":1,"highlight_end":1}],"label":null,"suggested_replacement":"use std::sync::mpsc::Iter;/n/n","expansion":null}],"children":[],"rendered":null}],"rendered":null}
+{"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":null}
diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs
index 77ee93c3007..8e9cd1a12fa 100644
--- a/src/tools/compiletest/src/json.rs
+++ b/src/tools/compiletest/src/json.rs
@@ -36,6 +36,7 @@ struct DiagnosticSpan {
     column_end: usize,
     is_primary: bool,
     label: Option<String>,
+    suggested_replacement: Option<String>,
     expansion: Option<Box<DiagnosticSpanMacroExpansion>>,
 }
 
@@ -164,15 +165,15 @@ fn push_expected_errors(expected_errors: &mut Vec<Error>,
     }
 
     // If the message has a suggestion, register that.
-    if let Some(ref rendered) = diagnostic.rendered {
-        let start_line = primary_spans.iter().map(|s| s.line_start).min().expect("\
-            every suggestion should have at least one span");
-        for (index, line) in rendered.lines().enumerate() {
-            expected_errors.push(Error {
-                line_num: start_line + index,
-                kind: Some(ErrorKind::Suggestion),
-                msg: line.to_string(),
-            });
+    for span in primary_spans {
+        if let Some(ref suggested_replacement) = span.suggested_replacement {
+            for (index, line) in suggested_replacement.lines().enumerate() {
+                expected_errors.push(Error {
+                    line_num: span.line_start + index,
+                    kind: Some(ErrorKind::Suggestion),
+                    msg: line.to_string(),
+                });
+            }
         }
     }