Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

Revert "Allow multiple solutions in a suggestion" #156

Merged
merged 1 commit into from
Dec 9, 2018
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
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
})
.filter_map(collect_span)
.collect();
if replacements.len() >= 1 {
if replacements.len() == 1 {
Some(Solution {
message: child.message.clone(),
replacements,
Expand Down
5 changes: 5 additions & 0 deletions tests/edge-cases/skip-multi-option-lints.fixed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fn main() {
let xs = vec![String::from("foo")];
let d: &Display = &xs;
println!("{}", d);
}
100 changes: 100 additions & 0 deletions tests/edge-cases/skip-multi-option-lints.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
{
"message": "cannot find type `Display` 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": "./tests/everything/skip-multi-option-lints.rs",
"byte_start": 64,
"byte_end": 71,
"line_start": 3,
"line_end": 3,
"column_start": 13,
"column_end": 20,
"is_primary": true,
"text": [
{
"text": " let d: &Display = &xs;",
"highlight_start": 13,
"highlight_end": 20
}
],
"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": "./tests/everything/skip-multi-option-lints.rs",
"byte_start": 0,
"byte_end": 0,
"line_start": 1,
"line_end": 1,
"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::fmt::Display;\n\n",
"suggestion_applicability": "Unspecified",
"expansion": null
},
{
"file_name": "./tests/everything/skip-multi-option-lints.rs",
"byte_start": 0,
"byte_end": 0,
"line_start": 1,
"line_end": 1,
"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::Display;\n\n",
"suggestion_applicability": "Unspecified",
"expansion": null
}
],
"children": [],
"rendered": null
}
],
"rendered": "error[E0412]: cannot find type `Display` in this scope\n --> ./tests/everything/skip-multi-option-lints.rs:3:13\n |\n3 | let d: &Display = &xs;\n | ^^^^^^^ not found in this scope\nhelp: possible candidates are found in other modules, you can import them into scope\n |\n1 | use std::fmt::Display;\n |\n1 | use std::path::Display;\n |\n\n"
}
{
"message": "aborting due to previous error",
"code": null,
"level": "error",
"spans": [],
"children": [],
"rendered": "error: aborting due to previous error\n\n"
}
{
"message": "For more information about this error, try `rustc --explain E0412`.",
"code": null,
"level": "",
"spans": [],
"children": [],
"rendered": "For more information about this error, try `rustc --explain E0412`.\n"
}
5 changes: 5 additions & 0 deletions tests/edge-cases/skip-multi-option-lints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fn main() {
let xs = vec![String::from("foo")];
let d: &Display = &xs;
println!("{}", d);
}
12 changes: 12 additions & 0 deletions tests/edge_cases.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
extern crate rustfix;
use std::collections::HashSet;
use std::fs;

#[test]
fn multiple_fix_options_yield_no_suggestions() {
let json = fs::read_to_string("./tests/edge-cases/skip-multi-option-lints.json").unwrap();
let expected_suggestions =
rustfix::get_suggestions_from_json(&json, &HashSet::new(), rustfix::Filter::Everything)
.unwrap();
assert!(expected_suggestions.is_empty());
}
5 changes: 0 additions & 5 deletions tests/everything/multiple-solutions.fixed.rs

This file was deleted.

114 changes: 0 additions & 114 deletions tests/everything/multiple-solutions.json

This file was deleted.

5 changes: 0 additions & 5 deletions tests/everything/multiple-solutions.rs

This file was deleted.

50 changes: 46 additions & 4 deletions tests/parse_and_replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ mod fixmode {

mod settings {
// can be set as env var to debug
pub const CHECK_JSON: &str = "RUSTFIX_TEST_CHECK_JSON";
pub const RECORD_JSON: &str = "RUSTFIX_TEST_RECORD_JSON";
pub const RECORD_FIXED_RUST: &str = "RUSTFIX_TEST_RECORD_FIXED_RUST";
}

Expand Down Expand Up @@ -61,6 +63,20 @@ fn compile(file: &Path, mode: &str) -> Result<Output, Error> {
Ok(res)
}

fn compile_and_get_json_errors(file: &Path, mode: &str) -> Result<String, Error> {
let res = compile(file, mode)?;
let stderr = String::from_utf8(res.stderr)?;

match res.status.code() {
Some(0) | Some(1) | Some(101) => Ok(stderr),
_ => Err(format_err!(
"failed with status {:?}: {}",
res.status.code(),
stderr
)),
}
}

fn compiles_without_errors(file: &Path, mode: &str) -> Result<(), Error> {
let res = compile(file, mode)?;

Expand Down Expand Up @@ -107,8 +123,7 @@ fn diff(expected: &str, actual: &str) -> String {
write!(
&mut res,
"differences found (+ == actual, - == expected):\n"
)
.unwrap();
).unwrap();
different = true;
}
for diff in diff.lines() {
Expand All @@ -135,12 +150,39 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err

debug!("next up: {:?}", file);
let code = read_file(file).context(format!("could not read {}", file.display()))?;
let errors = read_file(&json_file)
.with_context(|_| format!("could not load json suggestions for {}", file.display()))?;
let errors = compile_and_get_json_errors(file, mode)
.context(format!("could compile {}", file.display()))?;
let suggestions =
rustfix::get_suggestions_from_json(&errors, &HashSet::new(), filter_suggestions)
.context("could not load suggestions")?;

if std::env::var(settings::RECORD_JSON).is_ok() {
use std::io::Write;
let mut recorded_json = fs::File::create(&file.with_extension("recorded.json")).context(
format!("could not create recorded.json for {}", file.display()),
)?;
recorded_json.write_all(errors.as_bytes())?;
}

if std::env::var(settings::CHECK_JSON).is_ok() {
let expected_json = read_file(&json_file).context(format!(
"could not load json fixtures for {}",
file.display()
))?;
let expected_suggestions =
rustfix::get_suggestions_from_json(&expected_json, &HashSet::new(), filter_suggestions)
.context("could not load expected suggesitons")?;

ensure!(
expected_suggestions == suggestions,
"got unexpected suggestions from clippy:\n{}",
diff(
&format!("{:?}", expected_suggestions),
&format!("{:?}", suggestions)
)
);
}

let fixed = apply_suggestions(&code, &suggestions)
.context(format!("could not apply suggestions to {}", file.display()))?;

Expand Down