Don't pass --cap-lints by default (#374)

Fixes #373

* Add a new `--cap-lints=true` command line and config option to turn it
on if it's still wanted

NEWS.md

@@ -6,6 +6,8 @@
 
 - Changed: No build timeouts by default. Previously, cargo-mutants set a default build timeout based on the baseline build, but experience showed that this would sometimes make builds flaky, because build times can be quite variable. If mutants cause builds to hang, then you can still set a timeout using `--build-timeout` or `--build-timeout-multiplier`.
 
+- Changed: cargo-mutants no longer passes `--cap-lints=allow` to rustc. This was previously done so that mutants would not unnecessarily be unviable due to triggering compiler warnings in trees configured to deny some lints, but it had the undesirable effect of disabling rustc's detection of long running const evaluations. If your tree treats some lints as errors then the previous behavior can be restored with `--cap-lints=true` (or the equivalent config option), or you can use `cfg_attr` and a feature flag  to accept those warnings when testing under cargo-mutants.
+
 ## 24.5.0
 
 - Fixed: Follow `path` attributes on `mod` statements.

book/src/SUMMARY.md

@@ -20,6 +20,7 @@
   - [Using nextest](nextest.md)
   - [Baseline tests](baseline.md)
   - [Testing in-place](in-place.md)
+  - [Strict lints](lints.md)
 - [Generating mutants](mutants.md)
   - [Error values](error-values.md)
 - [Improving performance](performance.md)

book/src/lints.md

@@ -0,0 +1,12 @@
+# Strict lints
+
+Because cargo-mutants builds versions of your tree with many heuristically injected errors, it may not work well in trees that are configured to treat warnings as errors.
+
+For example, mutants that delete code are likely to cause some parameters to be seen as unused, which will cause problems with trees that configure `#[deny(unused)]`. This will manifest as an excessive number of mutants being reported as "unviable".
+
+There are a few possible solutions:
+
+1.  Define a feature flag for mutation testing, and use `cfg_attr` to enable strict warnings only when not testing mutants.
+2. Use the `cargo mutants --cap-lints=true` command line option, or the `cap_lints = true` config option.
+
+`--cap_lints=true` also disables rustc's detection of long-running const expression evaluation, so may cause some builds to fail. If that happens in your tree, you can set a [build timeout](timeouts.md).

src/cargo.rs

@@ -26,7 +26,7 @@ pub fn run_cargo(
     let start = Instant::now();
     let argv = cargo_argv(build_dir.path(), packages, phase, options);
     let env = vec![
-        ("CARGO_ENCODED_RUSTFLAGS".to_owned(), rustflags()),
+        ("CARGO_ENCODED_RUSTFLAGS".to_owned(), rustflags(options)),
         // The tests might use Insta <https://insta.rs>, and we don't want it to write
         // updates to the source tree, and we *certainly* don't want it to write
         // updates and then let the test pass.
@@ -140,7 +140,7 @@ fn cargo_argv(
 ///
 /// See <https://doc.rust-lang.org/cargo/reference/environment-variables.html>
 /// <https://doc.rust-lang.org/rustc/lints/levels.html#capping-lints>
-fn rustflags() -> String {
+fn rustflags(options: &Options) -> String {
     let mut rustflags: Vec<String> = if let Some(rustflags) = env::var_os("CARGO_ENCODED_RUSTFLAGS")
     {
         rustflags
@@ -163,7 +163,9 @@ fn rustflags() -> String {
         // TODO: build.rustflags config value.
         Vec::new()
     };
-    rustflags.push("--cap-lints=allow".to_owned());
+    if options.cap_lints {
+        rustflags.push("--cap-lints=warn".to_owned());
+    }
     // debug!("adjusted rustflags: {:?}", rustflags);
     rustflags.join("\x1f")
 }
@@ -291,6 +293,17 @@ mod test {
         );
     }
 
+    #[test]
+    fn cap_lints_passed_to_cargo() {
+        let args = Args::try_parse_from(["mutants", "--cap-lints=true"].as_slice()).unwrap();
+        let options = Options::from_args(&args).unwrap();
+        let build_dir = Utf8Path::new("/tmp/buildXYZ");
+        assert_eq!(
+            cargo_argv(build_dir, None, Phase::Check, &options)[1..],
+            ["check", "--tests", "--workspace",]
+        );
+    }
+
     #[test]
     fn feature_args_passed_to_cargo() {
         let args = Args::try_parse_from(
@@ -316,21 +329,34 @@ mod test {
         fn rustflags_with_no_environment_variables() {
             env::remove_var("RUSTFLAGS");
             env::remove_var("CARGO_ENCODED_RUSTFLAGS");
-            assert_eq!(rustflags(), "--cap-lints=allow");
+            assert_eq!(
+                rustflags(&Options {
+                    cap_lints: true,
+                    ..Default::default()
+                }),
+                "--cap-lints=warn"
+            );
         }
 
         #[test]
         fn rustflags_added_to_existing_encoded_rustflags() {
             env::set_var("RUSTFLAGS", "--something\x1f--else");
             env::remove_var("CARGO_ENCODED_RUSTFLAGS");
-            assert_eq!(rustflags(), "--something\x1f--else\x1f--cap-lints=allow");
+            let options = Options {
+                cap_lints: true,
+                ..Default::default()
+            };
+            assert_eq!(rustflags(&options), "--something\x1f--else\x1f--cap-lints=warn");
         }
 
         #[test]
         fn rustflags_added_to_existing_rustflags() {
             env::set_var("RUSTFLAGS", "-Dwarnings");
             env::remove_var("CARGO_ENCODED_RUSTFLAGS");
-            assert_eq!(rustflags(), "-Dwarnings\x1f--cap-lints=allow");
+            assert_eq!(rustflags(&Options {
+                cap_lints: true,
+                ..Default::default()
+            }), "-Dwarnings\x1f--cap-lints=warn");
         }
     }
 }

src/config.rs

@@ -27,6 +27,8 @@ use crate::Result;
 #[derive(Debug, Default, Clone, Deserialize)]
 #[serde(default, deny_unknown_fields)]
 pub struct Config {
+    /// Pass `--cap-lints` to rustc.
+    pub cap_lints: bool,
     /// Generate these error values from functions returning Result.
     pub error_values: Vec<String>,
     /// Generate mutants from source files matching these globs.

src/main.rs

@@ -116,6 +116,10 @@ pub struct Args {
     #[arg(long, value_enum, default_value_t = BaselineStrategy::Run, help_heading = "Execution")]
     baseline: BaselineStrategy,
 
+    /// Turn off all rustc lints, so that denied warnings won't make mutants unviable.
+    #[arg(long, action = ArgAction::Set, help_heading = "Build")]
+    cap_lints: Option<bool>,
+
     /// Print mutants that were caught by tests.
     #[arg(long, short = 'v', help_heading = "Output")]
     caught: bool,

src/options.rs

@@ -24,6 +24,9 @@ pub struct Options {
     /// Run tests in an unmutated tree?
     pub baseline: BaselineStrategy,
 
+    /// Turn off all lints.
+    pub cap_lints: bool,
+
     /// Don't run the tests, just see if each mutant builds.
     pub check_only: bool,
 
@@ -196,6 +199,7 @@ impl Options {
                 &config.additional_cargo_test_args,
             ),
             baseline: args.baseline,
+            cap_lints: args.cap_lints.unwrap_or(config.cap_lints),
             check_only: args.check,
             colors: args.colors,
             emit_json: args.json,
@@ -270,6 +274,7 @@ mod test {
         let options = Options::new(&args, &Config::default()).unwrap();
         assert!(!options.check_only);
         assert_eq!(options.test_tool, TestTool::Cargo);
+        assert!(!options.cap_lints);
     }
 
     #[test]
@@ -358,16 +363,18 @@ mod test {
     }
 
     #[test]
-    fn test_tool_from_config() {
+    fn from_config() {
         let config = indoc! { r#"
             test_tool = "nextest"
+            cap_lints = true
         "#};
         let mut config_file = NamedTempFile::new().unwrap();
         config_file.write_all(config.as_bytes()).unwrap();
         let args = Args::parse_from(["mutants"]);
         let config = Config::read_file(config_file.path()).unwrap();
         let options = Options::new(&args, &config).unwrap();
         assert_eq!(options.test_tool, TestTool::Nextest);
+        assert!(options.cap_lints);
     }
 
     #[test]

tests/main.rs

@@ -743,25 +743,32 @@ fn interrupt_caught_and_kills_children() {
 fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() {
     let tmp_src_dir = copy_of_testdata("hang_when_mutated");
     // Also test that it accepts decimal seconds
-    run()
+    let out = run()
         .arg("mutants")
-        .args(["-t", "8.1", "--build-timeout-multiplier=3"])
+        .args(["-t", "8.1", "--build-timeout=15.5"])
         .current_dir(tmp_src_dir.path())
         .env_remove("RUST_BACKTRACE")
         .timeout(OUTER_TIMEOUT)
         .assert()
         .code(3); // exit_code::TIMEOUT
+    println!(
+        "output:\n{}",
+        String::from_utf8_lossy(&out.get_output().stdout)
+    );
+    let unviable_txt = read_to_string(tmp_src_dir.path().join("mutants.out/unviable.txt"))
+        .expect("read timeout.txt");
+    let caught_txt = read_to_string(tmp_src_dir.path().join("mutants.out/caught.txt"))
+        .expect("read timeout.txt");
     let timeout_txt = read_to_string(tmp_src_dir.path().join("mutants.out/timeout.txt"))
         .expect("read timeout.txt");
     assert!(
         timeout_txt.contains("replace should_stop -> bool with false"),
         "expected text not found in:\n{timeout_txt}"
     );
     assert!(
-        timeout_txt.contains("replace should_stop_const -> bool with false"),
-        "expected text not found in:\n{timeout_txt}"
+        unviable_txt.contains("replace should_stop_const -> bool with false"),
+        "expected text not found in:\n{unviable_txt}"
     );
-    let caught_txt = read_to_string(tmp_src_dir.path().join("mutants.out/caught.txt")).unwrap();
     assert!(
         caught_txt.contains("replace should_stop -> bool with true"),
         "expected text not found in:\n{caught_txt}"
@@ -775,7 +782,7 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() {
             .expect("read outcomes.json")
             .parse()
             .expect("parse outcomes.json");
-    assert_eq!(outcomes_json["timeout"], 2);
+    assert_eq!(outcomes_json["timeout"], 1);
 
     let phases_for_const_fn = outcomes_json["outcomes"]
         .as_array()
@@ -793,11 +800,15 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() {
 }
 
 #[test]
-fn mutants_causing_check_to_timeout_are_stopped_by_manual_timeout() {
+fn hang_avoided_by_build_timeout_with_cap_lints() {
     let tmp_src_dir = copy_of_testdata("hang_when_mutated");
     run()
         .arg("mutants")
-        .args(["--check", "--build-timeout=4"])
+        .args([
+            "--build-timeout-multiplier=4",
+            "--regex=const",
+            "--cap-lints=true",
+        ])
         .current_dir(tmp_src_dir.path())
         .env_remove("RUST_BACKTRACE")
         .timeout(OUTER_TIMEOUT)
@@ -811,6 +822,20 @@ fn mutants_causing_check_to_timeout_are_stopped_by_manual_timeout() {
     );
 }
 
+#[test]
+fn constfn_mutation_passes_check() {
+    let tmp_src_dir = copy_of_testdata("hang_when_mutated");
+    let cmd = run()
+        .arg("mutants")
+        .args(["--check", "--build-timeout=4"])
+        .current_dir(tmp_src_dir.path())
+        .env_remove("RUST_BACKTRACE")
+        .timeout(OUTER_TIMEOUT)
+        .assert()
+        .code(0);
+    println!("{}", String::from_utf8_lossy(&cmd.get_output().stdout));
+}
+
 #[test]
 fn log_file_names_are_short_and_dont_collide() {
     // The "well_tested" tree can generate multiple mutants from single lines. They get distinct file names.