From a0eb55d593899dc92925c6b5de59e409e8ce8c84 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 8 Dec 2018 22:38:57 +0100 Subject: [PATCH 1/4] allow multiple solutions --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 7d51080..c44a099 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, From 4a577a6999124054b25d0dbef114aded0ef29729 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 8 Dec 2018 23:10:01 +0100 Subject: [PATCH 2/4] remove test about multi-option lints --- .../skip-multi-option-lints.fixed.rs | 5 - tests/edge-cases/skip-multi-option-lints.json | 100 ------------------ tests/edge-cases/skip-multi-option-lints.rs | 5 - tests/edge_cases.rs | 12 --- 4 files changed, 122 deletions(-) delete mode 100644 tests/edge-cases/skip-multi-option-lints.fixed.rs delete mode 100644 tests/edge-cases/skip-multi-option-lints.json delete mode 100644 tests/edge-cases/skip-multi-option-lints.rs delete mode 100644 tests/edge_cases.rs diff --git a/tests/edge-cases/skip-multi-option-lints.fixed.rs b/tests/edge-cases/skip-multi-option-lints.fixed.rs deleted file mode 100644 index 9e12371..0000000 --- a/tests/edge-cases/skip-multi-option-lints.fixed.rs +++ /dev/null @@ -1,5 +0,0 @@ -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 deleted file mode 100644 index dc61ae6..0000000 --- a/tests/edge-cases/skip-multi-option-lints.json +++ /dev/null @@ -1,100 +0,0 @@ -{ - "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 deleted file mode 100644 index 9e12371..0000000 --- a/tests/edge-cases/skip-multi-option-lints.rs +++ /dev/null @@ -1,5 +0,0 @@ -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 deleted file mode 100644 index 8848988..0000000 --- a/tests/edge_cases.rs +++ /dev/null @@ -1,12 +0,0 @@ -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()); -} From 5f1c320c3276fc6a786c30a4dd840f06e3451435 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 9 Dec 2018 18:47:30 +0100 Subject: [PATCH 3/4] tests: use the cached json errors instead of calling rustc --- tests/parse_and_replace.rs | 50 +++----------------------------------- 1 file changed, 4 insertions(+), 46 deletions(-) diff --git a/tests/parse_and_replace.rs b/tests/parse_and_replace.rs index cc361cb..dfdfc77 100644 --- a/tests/parse_and_replace.rs +++ b/tests/parse_and_replace.rs @@ -29,8 +29,6 @@ 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"; } @@ -62,20 +60,6 @@ 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)?; @@ -122,7 +106,8 @@ fn diff(expected: &str, actual: &str) -> String { write!( &mut res, "differences found (+ == actual, - == expected):\n" - ).unwrap(); + ) + .unwrap(); different = true; } for diff in diff.lines() { @@ -149,39 +134,12 @@ 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 = compile_and_get_json_errors(file, mode) - .context(format!("could compile {}", file.display()))?; + let errors = read_file(&json_file) + .with_context(|_| format!("could not load json suggestions for {}", 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()))?; From 4595c3bf5182db006b6ad4f9ace6771553bcebfa Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 9 Dec 2018 18:47:54 +0100 Subject: [PATCH 4/4] tests: add test for multiple suggestions --- tests/everything/multiple-solutions.fixed.rs | 5 + tests/everything/multiple-solutions.json | 114 +++++++++++++++++++ tests/everything/multiple-solutions.rs | 5 + 3 files changed, 124 insertions(+) create mode 100644 tests/everything/multiple-solutions.fixed.rs create mode 100644 tests/everything/multiple-solutions.json create mode 100644 tests/everything/multiple-solutions.rs diff --git a/tests/everything/multiple-solutions.fixed.rs b/tests/everything/multiple-solutions.fixed.rs new file mode 100644 index 0000000..1a26178 --- /dev/null +++ b/tests/everything/multiple-solutions.fixed.rs @@ -0,0 +1,5 @@ +use std::collections::{HashSet}; + +fn main() { + let _: HashSet<()>; +} diff --git a/tests/everything/multiple-solutions.json b/tests/everything/multiple-solutions.json new file mode 100644 index 0000000..89b14cc --- /dev/null +++ b/tests/everything/multiple-solutions.json @@ -0,0 +1,114 @@ +{ + "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 new file mode 100644 index 0000000..401198f --- /dev/null +++ b/tests/everything/multiple-solutions.rs @@ -0,0 +1,5 @@ +use std::collections::{HashMap, HashSet, VecDeque}; + +fn main() { + let _: HashSet<()>; +}