Merge pull request #156 from rust-lang-nursery/revert-155-multiple-solutions

Revert "Allow multiple solutions in a suggestion"

Author: killercup

src/lib.rs

@@ -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,

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);
+}

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<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"
+}

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);
+}

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());
+}

tests/everything/multiple-solutions.fixed.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<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)?;
 
@@ -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<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()))?;