Merge pull request #97 from killercup/feature/filter-by-machine-applicability

Filter by machine applicability

Author: killercup

cargo-fix/src/cli.rs

@@ -112,8 +112,7 @@ pub fn run() -> Result<(), Error> {
     //
     // TODO: we probably want to do something fancy here like collect results
     // from the client processes and print out a summary of what happened.
-    let status = cmd
-        .status()
+    let status = cmd.status()
         .with_context(|e| format!("failed to execute `{}`: {}", cargo.to_string_lossy(), e))?;
     exit_with(status);
 }

cargo-fix/src/main.rs

@@ -21,9 +21,9 @@ use std::str;
 
 use failure::{Error, ResultExt};
 use rustfix::diagnostics::Diagnostic;
-use termcolor::{ColorSpec, WriteColor, Color};
+use termcolor::{Color, ColorSpec, WriteColor};
 
-use diagnostics::{Message, output_stream};
+use diagnostics::{output_stream, Message};
 
 mod cli;
 mod diagnostics;
@@ -174,6 +174,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")?;
@@ -185,7 +189,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")
+        .fix_everything()
+        .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")
+        .fix_everything()
         .cwd("bar")
         .env("RUSTC", p.root.join("foo/target/debug/foo"))
         .stderr_contains("not rust code")

cargo-fix/tests/all/broken_lints.rs

@@ -19,21 +19,18 @@
 //         .build();
 
 //     p.expect_cmd("cargo-fix fix")
+//         .fix_everything()
 //         .stderr_contains(r"warning: error applying suggestions to `src/lib.rs`")
 //         .stderr_contains("The full error message was:")
-//         .stderr_contains(
-//             "> Could not replace range 56...60 in file -- maybe parts of it were already replaced?",
-//         )
-//         .stderr_contains(
-//             "\
-//              This likely indicates a bug in either rustc or rustfix itself,\n\
-//              and we would appreciate a bug report! You're likely to see \n\
-//              a number of compiler warnings after this message which rustfix\n\
-//              attempted to fix but failed. If you could open an issue at\n\
-//              https://github.com/rust-lang-nursery/rustfix/issues\n\
-//              quoting the full output of this command we'd be very appreciative!\n\n\
-//              ",
-//         )
+//         .stderr_contains("> Could not replace range 56...60 in file -- maybe parts of it were already replaced?")
+//         .stderr_contains("\
+//             This likely indicates a bug in either rustc or rustfix itself,\n\
+//             and we would appreciate a bug report! You're likely to see \n\
+//             a number of compiler warnings after this message which rustfix\n\
+//             attempted to fix but failed. If you could open an issue at\n\
+//             https://github.com/rust-lang-nursery/rustfix/issues\n\
+//             quoting the full output of this command we'd be very appreciative!\n\n\
+//         ")
 //         .status(0)
 //         .run();
 // }

cargo-fix/tests/all/dependencies.rs

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

cargo-fix/tests/all/edition_upgrade.rs

@@ -161,6 +161,7 @@ fn upgrade_extern_crate() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .fix_everything()
         .stdout("")
         .stderr(stderr)
         .run();

cargo-fix/tests/all/main.rs

@@ -151,6 +151,11 @@ impl<'a> ExpectCmd<'a> {
         self
     }
 
+    fn fix_everything(&mut self) -> &mut Self {
+        self.env("__CARGO_FIX_YOLO", "true");
+        self
+    }
+
     fn stdout(&mut self, s: &str) -> &mut Self {
         self.stdout = Some(s.to_string());
         self

cargo-fix/tests/all/smoke.rs

@@ -34,6 +34,7 @@ fn fixes_extra_mut() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .fix_everything()
         .stdout("")
         .stderr(stderr)
         .run();
@@ -60,6 +61,7 @@ fn fixes_two_missing_ampersands() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .fix_everything()
         .stdout("")
         .stderr(stderr)
         .run();
@@ -85,6 +87,7 @@ fn tricky() {
 [FINISHED] dev [unoptimized + debuginfo]
 ";
     p.expect_cmd("cargo-fix fix")
+        .fix_everything()
         .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")
+        .fix_everything()
+        .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")
+        .fix_everything()
+        .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")
+        .fix_everything()
+        .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")
+        .fix_everything()
         .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")
+        .fix_everything()
         .stdout("")
         .stderr_contains("[COMPILING] foo v0.1.0 (CWD)")
         .stderr_contains("[FIXING] build.rs (1 fix)")

cargo-fix/tests/all/vcs.rs

@@ -71,5 +71,8 @@ fn does_not_warn_about_clean_working_directory() {
         .run();
     p.expect_cmd("git config user.name RustFix").run();
     p.expect_cmd("git commit -m Initial-commit").run();
-    p.expect_cmd("cargo-fix fix").check_vcs(true).status(0).run();
+    p.expect_cmd("cargo-fix fix")
+        .check_vcs(true)
+        .status(0)
+        .run();
 }

examples/fix-json.rs

@@ -15,7 +15,11 @@ fn main() -> Result<(), Error> {
     };
 
     let suggestions = fs::read_to_string(&suggestions_file)?;
-    let suggestions = rustfix::get_suggestions_from_json(&suggestions, &HashSet::new())?;
+    let suggestions = rustfix::get_suggestions_from_json(
+        &suggestions,
+        &HashSet::new(),
+        rustfix::Filter::Everything,
+    )?;
 
     let source = fs::read_to_string(&source_file)?;
 

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,21 @@ 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 Filter::*;
+                    use diagnostics::Applicability::*;
+
+                    match (filter, &span.suggestion_applicability) {
+                        (MachineApplicableOnly, Some(MachineApplicable)) => true,
+                        (MachineApplicableOnly, _) => false,
+                        (Everything, _) => true,
+                    }
+                })
+                .filter_map(collect_span)
+                .collect();
             if replacements.len() == 1 {
                 Some(Solution {
                     message: child.message.clone(),

tests/edge_cases.rs

@@ -5,6 +5,8 @@ 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()).unwrap();
+    let expected_suggestions =
+        rustfix::get_suggestions_from_json(&json, &HashSet::new(), rustfix::Filter::Everything)
+            .unwrap();
     assert!(expected_suggestions.is_empty());
 }

tests/parse_and_replace.rs

@@ -43,7 +43,6 @@ fn compile(file: &Path, mode: &str) -> Result<Output, Error> {
         "-Zunstable-options".into(),
         "--emit=metadata".into(),
         "--crate-name=rustfix_test".into(),
-        "-Zsuggestion-applicability".into(),
         "--out-dir".into(),
         tmp.path().into(),
     ];
@@ -142,12 +141,19 @@ 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;
@@ -163,7 +169,7 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
             file.display()
         ))?;
         let expected_suggestions =
-            rustfix::get_suggestions_from_json(&expected_json, &HashSet::new())
+            rustfix::get_suggestions_from_json(&expected_json, &HashSet::new(), filter_suggestions)
                 .context("could not load expected suggesitons")?;
 
         ensure!(