Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a quadradic duplication in json for multi-suggestions #45489

Merged
merged 3 commits into from
Oct 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions src/libsyntax/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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()
}
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-27842.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lint/unused_parens_json_suggestion.stderr
Original file line number Diff line number Diff line change
@@ -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}
21 changes: 21 additions & 0 deletions src/test/ui/lint/use_suggestion_json.rs
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 2 additions & 0 deletions src/test/ui/lint/use_suggestion_json.stderr
Original file line number Diff line number Diff line change
@@ -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}
19 changes: 10 additions & 9 deletions src/tools/compiletest/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ struct DiagnosticSpan {
column_end: usize,
is_primary: bool,
label: Option<String>,
suggested_replacement: Option<String>,
expansion: Option<Box<DiagnosticSpanMacroExpansion>>,
}

Expand Down Expand Up @@ -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(),
});
}
}
}

Expand Down