diff --git a/src/lib.rs b/src/lib.rs index c44a099..7d51080 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -184,7 +184,7 @@ pub fn collect_suggestions( }) .filter_map(collect_span) .collect(); - if replacements.len() >= 1 { + if replacements.len() == 1 { Some(Solution { message: child.message.clone(), replacements, diff --git a/tests/edge-cases/skip-multi-option-lints.fixed.rs b/tests/edge-cases/skip-multi-option-lints.fixed.rs new file mode 100644 index 0000000..9e12371 --- /dev/null +++ b/tests/edge-cases/skip-multi-option-lints.fixed.rs @@ -0,0 +1,5 @@ +fn main() { + let xs = vec![String::from("foo")]; + let d: &Display = &xs; + println!("{}", d); +} diff --git a/tests/edge-cases/skip-multi-option-lints.json b/tests/edge-cases/skip-multi-option-lints.json new file mode 100644 index 0000000..dc61ae6 --- /dev/null +++ b/tests/edge-cases/skip-multi-option-lints.json @@ -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(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" +} diff --git a/tests/edge-cases/skip-multi-option-lints.rs b/tests/edge-cases/skip-multi-option-lints.rs new file mode 100644 index 0000000..9e12371 --- /dev/null +++ b/tests/edge-cases/skip-multi-option-lints.rs @@ -0,0 +1,5 @@ +fn main() { + let xs = vec![String::from("foo")]; + let d: &Display = &xs; + println!("{}", d); +} diff --git a/tests/edge_cases.rs b/tests/edge_cases.rs new file mode 100644 index 0000000..8848988 --- /dev/null +++ b/tests/edge_cases.rs @@ -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()); +} diff --git a/tests/everything/multiple-solutions.fixed.rs b/tests/everything/multiple-solutions.fixed.rs deleted file mode 100644 index 1a26178..0000000 --- a/tests/everything/multiple-solutions.fixed.rs +++ /dev/null @@ -1,5 +0,0 @@ -use std::collections::{HashSet}; - -fn main() { - let _: HashSet<()>; -} diff --git a/tests/everything/multiple-solutions.json b/tests/everything/multiple-solutions.json deleted file mode 100644 index 89b14cc..0000000 --- a/tests/everything/multiple-solutions.json +++ /dev/null @@ -1,114 +0,0 @@ -{ - "message": "unused imports: `HashMap`, `VecDeque`", - "code": { - "code": "unused_imports", - "explanation": null - }, - "level": "warning", - "spans": [ - { - "file_name": "src/main.rs", - "byte_start": 23, - "byte_end": 30, - "line_start": 1, - "line_end": 1, - "column_start": 24, - "column_end": 31, - "is_primary": true, - "text": [ - { - "text": "use std::collections::{HashMap, HashSet, VecDeque};", - "highlight_start": 24, - "highlight_end": 31 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - }, - { - "file_name": "src/main.rs", - "byte_start": 41, - "byte_end": 49, - "line_start": 1, - "line_end": 1, - "column_start": 42, - "column_end": 50, - "is_primary": true, - "text": [ - { - "text": "use std::collections::{HashMap, HashSet, VecDeque};", - "highlight_start": 42, - "highlight_end": 50 - } - ], - "label": null, - "suggested_replacement": null, - "suggestion_applicability": null, - "expansion": null - } - ], - "children": [ - { - "message": "#[warn(unused_imports)] on by default", - "code": null, - "level": "note", - "spans": [], - "children": [], - "rendered": null - }, - { - "message": "remove the unused imports", - "code": null, - "level": "help", - "spans": [ - { - "file_name": "src/main.rs", - "byte_start": 23, - "byte_end": 32, - "line_start": 1, - "line_end": 1, - "column_start": 24, - "column_end": 33, - "is_primary": true, - "text": [ - { - "text": "use std::collections::{HashMap, HashSet, VecDeque};", - "highlight_start": 24, - "highlight_end": 33 - } - ], - "label": null, - "suggested_replacement": "", - "suggestion_applicability": "MachineApplicable", - "expansion": null - }, - { - "file_name": "src/main.rs", - "byte_start": 39, - "byte_end": 49, - "line_start": 1, - "line_end": 1, - "column_start": 40, - "column_end": 50, - "is_primary": true, - "text": [ - { - "text": "use std::collections::{HashMap, HashSet, VecDeque};", - "highlight_start": 40, - "highlight_end": 50 - } - ], - "label": null, - "suggested_replacement": "", - "suggestion_applicability": "MachineApplicable", - "expansion": null - } - ], - "children": [], - "rendered": null - } - ], - "rendered": "warning: unused imports: `HashMap`, `VecDeque`\n --> src/main.rs:1:24\n |\n1 | use std::collections::{HashMap, HashSet, VecDeque};\n | ^^^^^^^ ^^^^^^^^\n |\n = note: #[warn(unused_imports)] on by default\nhelp: remove the unused imports\n |\n1 | use std::collections::{HashSet};\n | -- --\n\n" -} diff --git a/tests/everything/multiple-solutions.rs b/tests/everything/multiple-solutions.rs deleted file mode 100644 index 401198f..0000000 --- a/tests/everything/multiple-solutions.rs +++ /dev/null @@ -1,5 +0,0 @@ -use std::collections::{HashMap, HashSet, VecDeque}; - -fn main() { - let _: HashSet<()>; -} diff --git a/tests/parse_and_replace.rs b/tests/parse_and_replace.rs index 55bbe6d..7879b96 100644 --- a/tests/parse_and_replace.rs +++ b/tests/parse_and_replace.rs @@ -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"; } @@ -61,6 +63,20 @@ fn compile(file: &Path, mode: &str) -> Result { Ok(res) } +fn compile_and_get_json_errors(file: &Path, mode: &str) -> Result { + 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)?; @@ -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() { @@ -135,12 +150,39 @@ fn test_rustfix_with_file>(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()))?;