Add filtering by machine applicability

Author: killercup

cargo-fix/src/main.rs

@@ -165,6 +165,10 @@ fn rustfix_crate(rustc: &Path, filename: &str) -> Result<FixedCrate, Error> {
         return Ok(Default::default())
     }
 
+    let fix_mode = env::var_os("__CARGO_FIX_YOLO")
+        .map(|_| rustfix::Filter::Everything)
+        .unwrap_or(rustfix::Filter::MachineApplicableOnly);
+
     // Sift through the output of the compiler to look for JSON messages
     // indicating fixes that we can apply.
     let stderr = str::from_utf8(&output.stderr).context("failed to parse rustc stderr as utf-8")?;
@@ -176,7 +180,7 @@ fn rustfix_crate(rustc: &Path, filename: &str) -> Result<FixedCrate, Error> {
         .filter_map(|line| serde_json::from_str::<Diagnostic>(line).ok())
 
         // From each diagnostic try to extract suggestions from rustc
-        .filter_map(|diag| rustfix::collect_suggestions(&diag, &only));
+        .filter_map(|diag| rustfix::collect_suggestions(&diag, &only, fix_mode));
 
     // Collect suggestions by file so we can apply them one at a time later.
     let mut file_map = HashMap::new();

cargo-fix/tests/all/broken_build.rs

@@ -36,7 +36,10 @@ fn fix_broken_if_requested() {
         )
         .build();
 
-    p.expect_cmd("cargo-fix fix --broken-code").status(0).run();
+    p.expect_cmd("cargo-fix fix --broken-code")
+        .env("__CARGO_FIX_YOLO", "true")
+        .status(0)
+        .run();
 }
 
 #[test]
@@ -111,6 +114,7 @@ fn broken_fixes_backed_out() {
 
     // Attempt to fix code, but our shim will always fail the second compile
     p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
         .cwd("bar")
         .env("RUSTC", p.root.join("foo/target/debug/foo"))
         .stderr_contains("not rust code")

cargo-fix/tests/all/dependencies.rs

@@ -54,6 +54,7 @@ fn fix_path_deps() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
         .stdout("")
         .stderr(stderr)
         .run();

cargo-fix/tests/all/smoke.rs

@@ -34,6 +34,7 @@ fn fixes_extra_mut() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
         .stdout("")
         .stderr(stderr)
         .run();
@@ -60,6 +61,7 @@ fn fixes_two_missing_ampersands() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
         .stdout("")
         .stderr(stderr)
         .run();
@@ -85,6 +87,7 @@ fn tricky() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
         .stdout("")
         .stderr(stderr)
         .run();
@@ -102,7 +105,9 @@ fn preserve_line_endings() {
         )
         .build();
 
-    p.expect_cmd("cargo-fix fix").run();
+    p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
+        .run();
     assert!(p.read("src/lib.rs").contains("\r\n"));
 }
 
@@ -118,7 +123,9 @@ fn fix_deny_warnings() {
         )
         .build();
 
-    p.expect_cmd("cargo-fix fix").run();
+    p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
+        .run();
 }
 
 #[test]
@@ -139,7 +146,9 @@ fn fix_deny_warnings_but_not_others() {
         )
         .build();
 
-    p.expect_cmd("cargo-fix fix").run();
+    p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
+        .run();
     assert!(!p.read("src/lib.rs").contains("let mut x = 3;"));
     assert!(p.read("src/lib.rs").contains("fn bar() {}"));
 }
@@ -170,6 +179,7 @@ fn fix_two_files() {
         .build();
 
     p.expect_cmd("cargo-fix fix")
+        .env("__CARGO_FIX_YOLO", "true")
         .stderr_contains("[FIXING] src/bar.rs (1 fix)")
         .stderr_contains("[FIXING] src/lib.rs (1 fix)")
         .run();

cargo-fix/tests/all/subtargets.rs

@@ -40,6 +40,7 @@ fn fixes_missing_ampersand() {
         .build();
 
     p.expect_cmd("cargo fix -- --all-targets")
+        .env("__CARGO_FIX_YOLO", "true")
         .stdout("")
         .stderr_contains("[COMPILING] foo v0.1.0 (CWD)")
         .stderr_contains("[FIXING] build.rs (1 fix)")

src/diagnostics.rs

@@ -41,10 +41,19 @@ pub struct DiagnosticSpan {
     /// load the fully rendered version from the parent `Diagnostic`,
     /// however.
     pub suggested_replacement: Option<String>,
+    pub suggestion_applicability: Option<Applicability>,
     /// Macro invocations that created the code at this span, if any.
     expansion: Option<Box<DiagnosticSpanMacroExpansion>>,
 }
 
+#[derive(Copy, Clone, Debug, PartialEq, Deserialize)]
+pub enum Applicability {
+    MachineApplicable,
+    HasPlaceholders,
+    MaybeIncorrect,
+    Unspecified,
+}
+
 #[derive(Deserialize, Debug)]
 pub struct DiagnosticSpanLine {
     pub text: String,

src/lib.rs

@@ -18,14 +18,21 @@ pub mod diagnostics;
 use diagnostics::{Diagnostic, DiagnosticSpan};
 mod replace;
 
+#[derive(Debug, Clone, Copy)]
+pub enum Filter {
+    MachineApplicableOnly,
+    Everything,
+}
+
 pub fn get_suggestions_from_json<S: ::std::hash::BuildHasher>(
     input: &str,
     only: &HashSet<String, S>,
+    filter: Filter,
 ) -> serde_json::error::Result<Vec<Suggestion>> {
     let mut result = Vec::new();
     for cargo_msg in serde_json::Deserializer::from_str(input).into_iter::<Diagnostic>() {
         // One diagnostic line might have multiple suggestions
-        result.extend(collect_suggestions(&cargo_msg?, only));
+        result.extend(collect_suggestions(&cargo_msg?, only, filter));
     }
     Ok(result)
 }
@@ -142,6 +149,7 @@ fn collect_span(span: &DiagnosticSpan) -> Option<Replacement> {
 pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
     diagnostic: &Diagnostic,
     only: &HashSet<String, S>,
+    filter: Filter,
 ) -> Option<Suggestion> {
     if !only.is_empty() {
         if let Some(ref code) = diagnostic.code {
@@ -165,7 +173,19 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
         .children
         .iter()
         .filter_map(|child| {
-            let replacements: Vec<_> = child.spans.iter().filter_map(collect_span).collect();
+            let replacements: Vec<_> = child.spans.iter()
+                .filter(|span| {
+                    use diagnostics::Applicability::*;
+                    use Filter::*;
+
+                    match (filter, &span.suggestion_applicability) {
+                        (MachineApplicableOnly, Some(MachineApplicable)) => true,
+                        (MachineApplicableOnly, _) => false,
+                        (_, _) => true,
+                    }
+                })
+                .filter_map(collect_span)
+                .collect();
             if replacements.is_empty() {
                 None
             } else {

tests/parse_and_replace.rs

@@ -12,7 +12,7 @@ extern crate failure;
 extern crate difference;
 
 use std::ffi::OsString;
-use std::{env, fs};
+use std::fs;
 use std::path::{Path, PathBuf};
 use std::collections::HashSet;
 use std::process::Output;
@@ -32,9 +32,6 @@ mod settings {
     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";
-
-    // set automatically
-    pub const MODE: &str = "RUSTFIX_MODE";
 }
 
 fn compile(file: &Path, mode: &str) -> Result<Output, Error> {
@@ -118,15 +115,15 @@ fn diff(expected: &str, actual: &str) -> String {
             Difference::Rem(rem) => ("-", rem),
         };
         if !different {
-            write!(&mut res, "differences found (+ == actual, - == expected):\n");
+            write!(&mut res, "differences found (+ == actual, - == expected):\n").unwrap();
             different = true;
         }
         for diff in diff.lines() {
-            writeln!(&mut res, "{} {}", prefix, diff);
+            writeln!(&mut res, "{} {}", prefix, diff).unwrap();
         }
     }
     if different {
-        write!(&mut res, "");
+        write!(&mut res, "").unwrap();
     }
 
     res
@@ -137,13 +134,21 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
     let json_file = file.with_extension("json");
     let fixed_file = file.with_extension("fixed.rs");
 
+    let filter_suggestions = if mode == fixmode::EVERYTHING {
+        rustfix::Filter::Everything
+    } else {
+        rustfix::Filter::MachineApplicableOnly
+    };
+
     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 suggestions = rustfix::get_suggestions_from_json(&errors, &HashSet::new())
-        .context("could not load suggestions")?;
+    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;
@@ -155,8 +160,10 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
     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())
-            .context("could not load expected suggesitons")?;
+        let expected_suggestions =
+            rustfix::get_suggestions_from_json(
+                &expected_json, &HashSet::new(), filter_suggestions,
+            ).context("could not load expected suggesitons")?;
 
         ensure!(
             expected_suggestions == suggestions,