diff --git a/src/cargo.rs b/src/cargo.rs index 1cf59e67..79653999 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -2,6 +2,9 @@ //! Run Cargo as a subprocess, including timeouts and propagating signals. +#![warn(clippy::pedantic)] +#![allow(clippy::module_name_repetitions)] + use std::env; use std::iter::once; use std::time::{Duration, Instant}; @@ -23,7 +26,7 @@ use crate::Result; #[allow(clippy::too_many_arguments)] // I agree it's a lot but I'm not sure wrapping in a struct would be better. pub fn run_cargo( build_dir: &BuildDir, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, packages: &PackageSelection, phase: Phase, timeout: Option, @@ -155,12 +158,7 @@ fn cargo_argv(packages: &PackageSelection, phase: Phase, options: &Options) -> V cargo_args.push("--all-features".to_owned()); } // N.B. it can make sense to have --all-features and also explicit features from non-default packages.` - cargo_args.extend( - features - .features - .iter() - .map(|f| format!("--features={}", f)), - ); + cargo_args.extend(features.features.iter().map(|f| format!("--features={f}"))); cargo_args.extend(options.additional_cargo_args.iter().cloned()); if phase == Phase::Test { cargo_args.extend(options.additional_cargo_test_args.iter().cloned()); @@ -168,7 +166,7 @@ fn cargo_argv(packages: &PackageSelection, phase: Phase, options: &Options) -> V cargo_args } -/// Return adjusted CARGO_ENCODED_RUSTFLAGS, including any changes to cap-lints. +/// Return adjusted `CARGO_ENCODED_RUSTFLAGS`, including any changes to cap-lints. /// /// It seems we have to set this in the environment because Cargo doesn't expose /// a way to pass it in as an option from all commands? @@ -240,7 +238,7 @@ mod test { // let relative_manifest_path = Utf8PathBuf::from("testdata/something/Cargo.toml"); options .additional_cargo_test_args - .extend(["--lib", "--no-fail-fast"].iter().map(|s| s.to_string())); + .extend(["--lib", "--no-fail-fast"].iter().map(ToString::to_string)); // TODO: It wolud be a bit better to use `--manifest-path` here, to get // the fix for // but it's temporarily regressed. @@ -292,7 +290,7 @@ mod test { let mut options = Options::default(); options .additional_cargo_test_args - .extend(["--lib", "--no-fail-fast"].iter().map(|s| s.to_string())); + .extend(["--lib", "--no-fail-fast"].iter().map(|&s| s.to_string())); options .additional_cargo_args .extend(["--release".to_owned()]); diff --git a/src/console.rs b/src/console.rs index 57102df1..07062426 100644 --- a/src/console.rs +++ b/src/console.rs @@ -8,7 +8,6 @@ use std::io; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; use console::{style, StyledObject}; use humantime::format_duration; @@ -21,7 +20,7 @@ use crate::options::Colors; use crate::outcome::{LabOutcome, ScenarioOutcome, SummaryOutcome}; use crate::scenario::Scenario; use crate::tail_file::TailFile; -use crate::{Mutant, Options, Phase, Result}; +use crate::{Mutant, Options, Phase}; /// An interface to the console for the rest of cargo-mutants. /// @@ -42,14 +41,6 @@ impl Console { } } - pub fn set_colors_enabled(&self, colors: Colors) { - if let Some(colors) = colors.forced_value() { - ::console::set_colors_enabled(colors); - ::console::set_colors_enabled_stderr(colors); - } - // Otherwise, let the console crate decide, based on isatty, etc. - } - pub fn walk_tree_start(&self) { self.view .update(|model| model.walk_tree = Some(WalkModel::default())); @@ -69,18 +60,12 @@ impl Console { } /// Update that a cargo task is starting. - pub fn scenario_started( - &self, - dir: &Utf8Path, - scenario: &Scenario, - log_file: File, - ) -> Result<()> { + pub fn scenario_started(&self, dir: &Utf8Path, scenario: &Scenario, log_file: File) { let start = Instant::now(); - let scenario_model = ScenarioModel::new(dir, scenario, start, log_file)?; + let scenario_model = ScenarioModel::new(dir, scenario, start, log_file); self.view.update(|model| { model.scenario_models.push(scenario_model); }); - Ok(()) } /// Update that cargo finished. @@ -92,7 +77,9 @@ impl Console { options: &Options, ) { self.view.update(|model| { - model.mutants_done += scenario.is_mutant() as usize; + if scenario.is_mutant() { + model.mutants_done += 1; + } match outcome.summary() { SummaryOutcome::CaughtMutant => model.mutants_caught += 1, SummaryOutcome::MissedMutant => model.mutants_missed += 1, @@ -169,7 +156,7 @@ impl Console { .iter_mut() .find(|m| m.dest == dest) .expect("copy in progress") - .bytes_copied(total_bytes) + .bytes_copied(total_bytes); }); } @@ -183,7 +170,7 @@ impl Console { self.view.update(|model| { model.n_mutants = n_mutants; model.lab_start_time = Some(Instant::now()); - }) + }); } /// Update that work is starting on testing a given number of mutants. @@ -196,13 +183,13 @@ impl Console { pub fn scenario_phase_started(&self, dir: &Utf8Path, phase: Phase) { self.view.update(|model| { model.find_scenario_mut(dir).phase_started(phase); - }) + }); } pub fn scenario_phase_finished(&self, dir: &Utf8Path, phase: Phase) { self.view.update(|model| { model.find_scenario_mut(dir).phase_finished(phase); - }) + }); } pub fn lab_finished(&self, lab_outcome: &LabOutcome, start_time: Instant, options: &Options) { @@ -216,7 +203,7 @@ impl Console { } pub fn clear(&self) { - self.view.clear() + self.view.clear(); } pub fn message(&self, message: &str) { @@ -224,11 +211,11 @@ impl Console { // stderr... // self.view.clear(); - print!("{}", message); + print!("{message}"); } pub fn tick(&self) { - self.view.update(|_| ()) + self.view.update(|_| ()); } /// Return a tracing `MakeWriter` that will send messages via nutmeg to the console. @@ -251,8 +238,8 @@ impl Console { /// Configure tracing to send messages to the console and debug log. /// - /// The debug log is opened later and provided by [Console::set_debug_log]. - pub fn setup_global_trace(&self, console_trace_level: Level, colors: Colors) -> Result<()> { + /// The debug log is opened later and provided by [`Console::set_debug_log`]. + pub fn setup_global_trace(&self, console_trace_level: Level, colors: Colors) { // Show time relative to the start of the program. let uptime = tracing_subscriber::fmt::time::uptime(); let stderr_colors = colors @@ -275,10 +262,17 @@ impl Console { .with(debug_log_layer) .with(console_layer) .init(); - Ok(()) } } +pub fn enable_console_colors(colors: Colors) { + if let Some(colors) = colors.forced_value() { + ::console::set_colors_enabled(colors); + ::console::set_colors_enabled_stderr(colors); + } + // Otherwise, let the console crate decide, based on isatty, etc. +} + impl Default for Console { fn default() -> Self { Self::new() @@ -367,26 +361,27 @@ struct LabModel { } impl nutmeg::Model for LabModel { + #[allow(clippy::cast_precision_loss)] fn render(&mut self, width: usize) -> String { let mut s = String::with_capacity(1024); if let Some(walk_tree) = &mut self.walk_tree { s += &walk_tree.render(width); } - for copy_model in self.copy_models.iter_mut() { + for copy_model in &mut self.copy_models { if !s.is_empty() { - s.push('\n') + s.push('\n'); } s.push_str(©_model.render(width)); } - for sm in self.scenario_models.iter_mut() { + for sm in &mut self.scenario_models { if !s.is_empty() { - s.push('\n') + s.push('\n'); } s.push_str(&sm.render(width)); } if let Some(lab_start_time) = self.lab_start_time { if !s.is_empty() { - s.push('\n') + s.push('\n'); } let elapsed = lab_start_time.elapsed(); s += &format!( @@ -489,21 +484,15 @@ struct ScenarioModel { } impl ScenarioModel { - fn new( - dir: &Utf8Path, - scenario: &Scenario, - start: Instant, - log_file: File, - ) -> Result { - let log_tail = TailFile::new(log_file).context("Failed to open log file")?; - Ok(ScenarioModel { + fn new(dir: &Utf8Path, scenario: &Scenario, start: Instant, log_file: File) -> ScenarioModel { + ScenarioModel { dir: dir.to_owned(), name: style_scenario(scenario, true), phase: None, phase_start: start, - log_tail, + log_tail: TailFile::new(log_file), previous_phase_durations: Vec::new(), - }) + } } fn phase_started(&mut self, phase: Phase) { @@ -569,7 +558,7 @@ impl CopyModel { /// /// `bytes_copied` is the total bytes copied so far. fn bytes_copied(&mut self, bytes_copied: u64) { - self.bytes_copied = bytes_copied + self.bytes_copied = bytes_copied; } } diff --git a/src/copy_tree.rs b/src/copy_tree.rs index a02d2ff2..37661d7c 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -38,7 +38,7 @@ static SOURCE_EXCLUDE: &[&str] = &[ /// If `git` is true, ignore files that are excluded by all the various `.gitignore` /// files. /// -/// Regardless, anything matching [SOURCE_EXCLUDE] is excluded. +/// Regardless, anything matching [`SOURCE_EXCLUDE`] is excluded. pub fn copy_tree( from_path: &Utf8Path, name_base: &str, diff --git a/src/fnvalue.rs b/src/fnvalue.rs index c5e1baab..1408b596 100644 --- a/src/fnvalue.rs +++ b/src/fnvalue.rs @@ -2,6 +2,8 @@ //! Mutations of replacing a function body with a value of a (hopefully) appropriate type. +#![warn(clippy::pedantic)] + use std::iter; use itertools::Itertools; @@ -25,8 +27,7 @@ pub(crate) fn return_type_replacements( } /// Generate some values that we hope are reasonable replacements for a type. -/// -/// This is really the heart of cargo-mutants. +#[allow(clippy::too_many_lines)] fn type_replacements(type_: &Type, error_exprs: &[Expr]) -> impl Iterator { // This could probably change to run from some configuration rather than // hardcoding various types, which would make it easier to support tree-specific @@ -292,7 +293,7 @@ fn known_container(path: &Path) -> Option<(&Ident, &Type)> { /// Match known simple collections that can be empty or constructed from an /// iterator. /// -/// Returns the short name (like "VecDeque") and the inner type. +/// Returns the short name (like `"VecDeque"`) and the inner type. fn known_collection(path: &Path) -> Option<(&Ident, &Type)> { let last = path.segments.last()?; if ![ @@ -445,7 +446,7 @@ mod test { #[test] fn recurse_into_result_bool() { check_replacements( - parse_quote! {-> std::result::Result }, + &parse_quote! {-> std::result::Result }, &[], &["Ok(true)", "Ok(false)"], ); @@ -454,7 +455,7 @@ mod test { #[test] fn recurse_into_result_result_bool_with_error_values() { check_replacements( - parse_quote! {-> std::result::Result> }, + &parse_quote! {-> std::result::Result> }, &[parse_quote! { anyhow!("mutated") }], &[ "Ok(Ok(true))", @@ -467,43 +468,43 @@ mod test { #[test] fn u16_replacements() { - check_replacements(parse_quote! { -> u16 }, &[], &["0", "1"]); + check_replacements(&parse_quote! { -> u16 }, &[], &["0", "1"]); } #[test] fn isize_replacements() { - check_replacements(parse_quote! { -> isize }, &[], &["0", "1", "-1"]); + check_replacements(&parse_quote! { -> isize }, &[], &["0", "1", "-1"]); } #[test] fn nonzero_integer_replacements() { check_replacements( - parse_quote! { -> std::num::NonZeroIsize }, + &parse_quote! { -> std::num::NonZeroIsize }, &[], &["1", "-1"], ); - check_replacements(parse_quote! { -> std::num::NonZeroUsize }, &[], &["1"]); + check_replacements(&parse_quote! { -> std::num::NonZeroUsize }, &[], &["1"]); - check_replacements(parse_quote! { -> std::num::NonZeroU32 }, &[], &["1"]); + check_replacements(&parse_quote! { -> std::num::NonZeroU32 }, &[], &["1"]); } #[test] fn unit_replacement() { - check_replacements(parse_quote! { -> () }, &[], &["()"]); + check_replacements(&parse_quote! { -> () }, &[], &["()"]); } #[test] fn result_unit_replacement() { - check_replacements(parse_quote! { -> Result<(), Error> }, &[], &["Ok(())"]); + check_replacements(&parse_quote! { -> Result<(), Error> }, &[], &["Ok(())"]); - check_replacements(parse_quote! { -> Result<()> }, &[], &["Ok(())"]); + check_replacements(&parse_quote! { -> Result<()> }, &[], &["Ok(())"]); } #[test] fn http_response_replacement() { check_replacements( - parse_quote! { -> HttpResponse }, + &parse_quote! { -> HttpResponse }, &[], &["HttpResponse::Ok().finish()"], ); @@ -512,7 +513,7 @@ mod test { #[test] fn option_usize_replacement() { check_replacements( - parse_quote! { -> Option }, + &parse_quote! { -> Option }, &[], &["None", "Some(0)", "Some(1)"], ); @@ -521,7 +522,7 @@ mod test { #[test] fn box_usize_replacement() { check_replacements( - parse_quote! { -> Box }, + &parse_quote! { -> Box }, &[], &["Box::new(0)", "Box::new(1)"], ); @@ -530,7 +531,7 @@ mod test { #[test] fn box_unrecognized_type_replacement() { check_replacements( - parse_quote! { -> Box }, + &parse_quote! { -> Box }, &[], &["Box::new(Default::default())"], ); @@ -539,7 +540,7 @@ mod test { #[test] fn vec_string_replacement() { check_replacements( - parse_quote! { -> std::vec::Vec }, + &parse_quote! { -> std::vec::Vec }, &[], &["vec![]", "vec![String::new()]", r#"vec!["xyzzy".into()]"#], ); @@ -547,18 +548,18 @@ mod test { #[test] fn float_replacement() { - check_replacements(parse_quote! { -> f32 }, &[], &["0.0", "1.0", "-1.0"]); + check_replacements(&parse_quote! { -> f32 }, &[], &["0.0", "1.0", "-1.0"]); } #[test] fn ref_replacement_recurses() { - check_replacements(parse_quote! { -> &bool }, &[], &["&true", "&false"]); + check_replacements(&parse_quote! { -> &bool }, &[], &["&true", "&false"]); } #[test] fn array_replacement() { check_replacements( - parse_quote! { -> [u8; 256] }, + &parse_quote! { -> [u8; 256] }, &[], &["[0; 256]", "[1; 256]"], ); @@ -569,7 +570,7 @@ mod test { // Also checks that it matches the path, even using an atypical path. // TODO: Ideally this would be fully qualified like `alloc::sync::Arc::new(String::new())`. check_replacements( - parse_quote! { -> alloc::sync::Arc }, + &parse_quote! { -> alloc::sync::Arc }, &[], &["Arc::new(String::new())", r#"Arc::new("xyzzy".into())"#], ); @@ -580,7 +581,7 @@ mod test { // Also checks that it matches the path, even using an atypical path. // TODO: Ideally this would be fully qualified like `alloc::sync::Rc::new(String::new())`. check_replacements( - parse_quote! { -> alloc::sync::Rc }, + &parse_quote! { -> alloc::sync::Rc }, &[], &["Rc::new(String::new())", r#"Rc::new("xyzzy".into())"#], ); @@ -652,7 +653,7 @@ mod test { #[test] fn btreeset_replacement() { check_replacements( - parse_quote! { -> std::collections::BTreeSet }, + &parse_quote! { -> std::collections::BTreeSet }, &[], &[ "BTreeSet::new()", @@ -665,7 +666,7 @@ mod test { #[test] fn cow_generates_borrowed_and_owned() { check_replacements( - parse_quote! { -> Cow<'static, str> }, + &parse_quote! { -> Cow<'static, str> }, &[], &[ r#"Cow::Borrowed("")"#, @@ -681,7 +682,7 @@ mod test { // This looks like something that holds a &str, and maybe can be constructed // from a &str, but we don't know anything else about it, so we just guess. check_replacements( - parse_quote! { -> UnknownContainer<'static, str> }, + &parse_quote! { -> UnknownContainer<'static, str> }, &[], &[ "UnknownContainer::new()", @@ -698,16 +699,16 @@ mod test { #[test] fn tuple_combinations() { check_replacements( - parse_quote! { -> (bool, usize) }, + &parse_quote! { -> (bool, usize) }, &[], &["(true, 0)", "(true, 1)", "(false, 0)", "(false, 1)"], - ) + ); } #[test] fn tuple_combination_longer() { check_replacements( - parse_quote! { -> (bool, Option) }, + &parse_quote! { -> (bool, Option) }, &[], &[ "(true, None)", @@ -717,13 +718,13 @@ mod test { "(false, Some(String::new()))", r#"(false, Some("xyzzy".into()))"#, ], - ) + ); } #[test] fn iter_replacement() { check_replacements( - parse_quote! { -> impl Iterator }, + &parse_quote! { -> impl Iterator }, &[], &[ "::std::iter::empty()", @@ -755,7 +756,7 @@ mod test { #[test] fn slice_replacement() { check_replacements( - parse_quote! { -> [u8] }, + &parse_quote! { -> [u8] }, &[], &[ "Vec::leak(Vec::new())", @@ -768,7 +769,7 @@ mod test { #[test] fn btreemap_replacement() { check_replacements( - parse_quote! { -> BTreeMap }, + &parse_quote! { -> BTreeMap }, &[], &[ "BTreeMap::new()", @@ -780,9 +781,9 @@ mod test { ); } - fn check_replacements(return_type: ReturnType, error_exprs: &[Expr], expected: &[&str]) { + fn check_replacements(return_type: &ReturnType, error_exprs: &[Expr], expected: &[&str]) { assert_eq!( - return_type_replacements(&return_type, error_exprs) + return_type_replacements(return_type, error_exprs) .into_iter() .map(|t| t.to_pretty_string()) .collect_vec(), diff --git a/src/in_diff.rs b/src/in_diff.rs index ab9a30e7..84efe18e 100644 --- a/src/in_diff.rs +++ b/src/in_diff.rs @@ -51,7 +51,7 @@ pub fn diff_filter(mutants: Vec, diff_text: &str) -> Result> } } let mut matched: Vec = Vec::with_capacity(mutants.len()); - 'mutant: for mutant in mutants.into_iter() { + 'mutant: for mutant in mutants { let path = mutant.source_file.path(); if let Some(lines_changed) = lines_changed_by_path.get(path) { // We could do be smarter about searching for an intersection of ranges, rather @@ -206,8 +206,8 @@ fn partial_new_file<'d>(patch: &Patch<'d>) -> Vec<(usize, &'d str)> { } } debug_assert_eq!( - lineno, - (hunk.new_range.start + hunk.new_range.count) as usize, + Ok(lineno), + (hunk.new_range.start + hunk.new_range.count).try_into(), "Wrong number of resulting lines?" ); } @@ -271,7 +271,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_single_insertion() { - let orig_lines = (1..=4).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=4).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=5 { let mut new = orig_lines.clone(); let new_value = "new line\n".to_owned(); @@ -291,7 +291,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_single_deletion() { - let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=5).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=5 { let mut new = orig_lines.clone(); new.remove(i - 1); @@ -312,7 +312,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_double_deletion() { - let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=5).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=4 { let mut new = orig_lines.clone(); new.remove(i - 1); @@ -332,7 +332,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_replacement() { - let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=5).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=5 { let insertion = ["new 1\n".to_owned(), "new 2\n".to_owned()]; let new = orig_lines[..(i - 1)] diff --git a/src/lab.rs b/src/lab.rs index cd487a3e..81873a79 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -203,7 +203,7 @@ impl Lab<'_> { Worker { build_dir, output_mutex: &self.output_mutex, - jobserver: &self.jobserver, + jobserver: self.jobserver.as_ref(), options: self.options, console: self.console, } @@ -217,7 +217,7 @@ impl Lab<'_> { struct Worker<'a> { build_dir: &'a BuildDir, output_mutex: &'a Mutex, - jobserver: &'a Option, + jobserver: Option<&'a jobserver::Client>, options: &'a Options, console: &'a Console, } @@ -262,7 +262,7 @@ impl Worker<'_> { .start_scenario(scenario)?; let dir = self.build_dir.path(); self.console - .scenario_started(dir, scenario, scenario_output.open_log_read()?)?; + .scenario_started(dir, scenario, scenario_output.open_log_read()?); if let Some(mutant) = scenario.mutant() { let mutated_code = mutant.mutated_code(); diff --git a/src/main.rs b/src/main.rs index 4c193d02..c9fe8a21 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,9 @@ //! //! See for the manual and more information. +#![warn(clippy::pedantic)] +#![allow(clippy::module_name_repetitions, clippy::needless_raw_string_hashes)] + mod build_dir; mod cargo; mod config; @@ -49,6 +52,7 @@ use clap::builder::Styles; use clap::{ArgAction, CommandFactory, Parser, ValueEnum}; use clap_complete::{generate, Shell}; use color_print::cstr; +use console::enable_console_colors; use output::{load_previously_caught, OutputDir}; use tracing::{debug, info}; @@ -102,6 +106,7 @@ pub enum BaselineStrategy { /// Find inadequately-tested code that can be removed without any tests failing. /// /// See for more information. +#[allow(clippy::struct_excessive_bools)] #[derive(Parser, PartialEq, Debug)] #[command( author, @@ -425,8 +430,8 @@ fn main() -> Result<()> { } let console = Console::new(); - console.setup_global_trace(args.level, args.colors)?; // We don't have Options yet. - console.set_colors_enabled(args.colors); + console.setup_global_trace(args.level, args.colors); // We don't have Options yet. + enable_console_colors(args.colors); interrupt::install_handler(); let start_dir: &Utf8Path = if let Some(manifest_path) = &args.manifest_path { @@ -514,7 +519,7 @@ mod test { let args = super::Args::command(); let mut problems = Vec::new(); for arg in args.get_arguments() { - if let Some(help) = arg.get_help().map(|s| s.to_string()) { + if let Some(help) = arg.get_help().map(ToString::to_string) { if !help.starts_with(char::is_uppercase) { problems.push(format!( "Help for {:?} does not start with a capital letter: {:?}", @@ -538,9 +543,9 @@ mod test { problems.push(format!("No help for {:?}", arg.get_id())); } } - problems.iter().for_each(|s| eprintln!("{s}")); - if !problems.is_empty() { - panic!("Problems with help text"); + for problem in &problems { + eprintln!("{problem}"); } + assert!(problems.is_empty(), "Problems with help text"); } } diff --git a/src/manifest.rs b/src/manifest.rs index 09d578c4..50894202 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -18,6 +18,7 @@ use crate::Result; /// /// `manifest_source_dir` is the directory originally containing the manifest, from /// which the absolute paths are calculated. +#[allow(clippy::module_name_repetitions)] pub fn fix_manifest(manifest_scratch_path: &Utf8Path, source_dir: &Utf8Path) -> Result<()> { let toml_str = read_to_string(manifest_scratch_path).with_context(|| { format!("failed to read manifest from build directory: {manifest_scratch_path}") diff --git a/src/mutate.rs b/src/mutate.rs index fd5c3525..a21634ef 100644 --- a/src/mutate.rs +++ b/src/mutate.rs @@ -57,6 +57,7 @@ pub struct Mutant { #[derive(Eq, PartialEq, Debug, Serialize)] pub struct Function { /// The function that's being mutated, including any containing namespaces. + #[allow(clippy::struct_field_names)] pub function_name: String, /// The return type of the function, including a leading "-> ", as a fragment of Rust syntax. @@ -84,8 +85,7 @@ impl Mutant { self.styled_parts() .into_iter() .map(|x| x.force_styling(false).to_string()) - .collect::>() - .join("") + .collect::() } pub fn name(&self, show_line_col: bool) -> String { @@ -126,6 +126,7 @@ impl Mutant { fn styled_parts(&self) -> Vec> { // This is like `impl Display for Mutant`, but with colors. // The text content should be the same. + #[allow(clippy::needless_pass_by_value)] // actually is needed for String vs &str? fn s(s: S) -> StyledObject { style(s.to_string()) } @@ -186,7 +187,7 @@ impl Mutant { .to_string() } - /// Apply this mutant to the relevant file within a BuildDir. + /// Apply this mutant to the relevant file within a `BuildDir`. pub fn apply(&self, build_dir: &BuildDir, mutated_code: &str) -> Result<()> { trace!(?self, "Apply mutant"); build_dir.overwrite_file(&self.source_file.tree_relative_path, mutated_code) @@ -236,7 +237,7 @@ impl Serialize for Mutant { let mut ss = serializer.serialize_struct("Mutant", 7)?; ss.serialize_field("package", &self.source_file.package_name)?; ss.serialize_field("file", &self.source_file.tree_relative_slashes())?; - ss.serialize_field("function", &self.function.as_ref().map(|a| a.as_ref()))?; + ss.serialize_field("function", &self.function.as_ref().map(Arc::as_ref))?; ss.serialize_field("span", &self.span)?; ss.serialize_field("replacement", &self.replacement)?; ss.serialize_field("genre", &self.genre)?; @@ -327,21 +328,21 @@ mod test { .unwrap() .mutants; let descriptions = mutants.iter().map(Mutant::describe_change).collect_vec(); - insta::assert_snapshot!( - descriptions.join("\n"), - @r###" - replace controlled_loop with () - replace > with == in controlled_loop - replace > with < in controlled_loop - replace * with + in controlled_loop - replace * with / in controlled_loop - "### + assert_eq!( + descriptions, + [ + "replace controlled_loop with ()", + "replace > with == in controlled_loop", + "replace > with < in controlled_loop", + "replace * with + in controlled_loop", + "replace * with / in controlled_loop", + ] ); } #[test] fn always_skip_constructors_called_new() { - let code = indoc! { r#" + let code = indoc! { r" struct S { x: i32, } @@ -351,7 +352,7 @@ mod test { Self { x } } } - "# }; + " }; let mutants = mutate_source_str(code, &Options::default()).unwrap(); assert_eq!(mutants, []); } @@ -422,6 +423,6 @@ mod test { fn strip_trailing_space(s: &str) -> String { // Split on \n so that we retain empty lines etc - s.split('\n').map(|l| l.trim_end()).join("\n") + s.split('\n').map(str::trim_end).join("\n") } } diff --git a/src/options.rs b/src/options.rs index 0f596736..5e62e9b5 100644 --- a/src/options.rs +++ b/src/options.rs @@ -8,10 +8,14 @@ //! 2. Config options (read from `.cargo/mutants.toml`) //! 3. Built-in defaults +#![warn(clippy::pedantic)] + +use std::env; #[cfg(test)] use std::ffi::OsString; use std::time::Duration; +use camino::Utf8PathBuf; use globset::GlobSet; use regex::RegexSet; use serde::Deserialize; @@ -21,11 +25,12 @@ use tracing::warn; use crate::config::Config; use crate::glob::build_glob_set; -use crate::*; +use crate::{Args, BaselineStrategy, Context, Phase, Result, ValueEnum}; /// Options for mutation testing, based on both command-line arguments and the /// config file. #[derive(Default, Debug, Clone)] +#[allow(clippy::struct_excessive_bools)] pub struct Options { /// Run tests in an unmutated tree? pub baseline: BaselineStrategy, @@ -197,7 +202,7 @@ impl Colors { /// /// Otherwise, return None, meaning we should decide based on the /// detected terminal characteristics. - pub fn forced_value(&self) -> Option { + pub fn forced_value(self) -> Option { // From https://bixense.com/clicolors/ if env::var("NO_COLOR").is_ok_and(|x| x != "0") { Some(false) @@ -213,7 +218,7 @@ impl Colors { } #[mutants::skip] // depends on a real tty etc, hard to test - pub fn active_stdout(&self) -> bool { + pub fn active_stdout(self) -> bool { self.forced_value() .unwrap_or_else(::console::colors_enabled) } @@ -240,7 +245,7 @@ impl Options { args.test_package .iter() .flat_map(|s| s.split(',')) - .map(|s| s.to_string()) + .map(ToString::to_string) .collect(), ) } else if args.test_workspace.is_none() && config.test_workspace == Some(true) { @@ -255,7 +260,7 @@ impl Options { .skip_calls .iter() .flat_map(|s| s.split(',')) - .map(|s| s.to_string()) + .map(ToString::to_string) .chain(config.skip_calls.iter().cloned()) .collect(); if args @@ -334,6 +339,8 @@ impl Options { /// If the arguments are invalid. #[cfg(test)] pub fn from_arg_strs, S: Into + Clone>(args: I) -> Options { + use crate::Args; + use clap::Parser; let args = Args::try_parse_from(args).expect("Failed to parse args"); Options::from_args(&args).expect("Build options from args") } @@ -373,11 +380,13 @@ mod test { use std::io::Write; use std::str::FromStr; + use clap::Parser; use indoc::indoc; use rusty_fork::rusty_fork_test; use tempfile::NamedTempFile; use super::*; + use crate::Args; #[test] fn default_options() { @@ -441,10 +450,10 @@ mod test { #[test] fn cli_timeout_multiplier_overrides_config() { - let config = indoc! { r#" + let config = indoc! { r" timeout_multiplier = 1.0 build_timeout_multiplier = 2.0 - "#}; + "}; let mut config_file = NamedTempFile::new().unwrap(); config_file.write_all(config.as_bytes()).unwrap(); let args = Args::parse_from([ @@ -678,9 +687,9 @@ mod test { #[test] fn test_workspace_config_true() { let args = Args::try_parse_from(["mutants"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = true - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Workspace); @@ -689,9 +698,9 @@ mod test { #[test] fn test_workspace_config_false() { let args = Args::try_parse_from(["mutants"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = false - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Mutated); @@ -700,9 +709,9 @@ mod test { #[test] fn test_workspace_args_override_config_true() { let args = Args::try_parse_from(["mutants", "--test-workspace=true"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = false - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Workspace); @@ -711,9 +720,9 @@ mod test { #[test] fn test_workspace_args_override_config_false() { let args = Args::try_parse_from(["mutants", "--test-workspace=false"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = true - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Mutated); @@ -800,10 +809,10 @@ mod test { // You can configure off the default `with_capacity` skip_calls let args = Args::try_parse_from(["mutants"]).unwrap(); let config = Config::from_str( - r#" + r" skip_calls_defaults = false skip_calls = [] - "#, + ", ) .unwrap(); let options = Options::new(&args, &config).unwrap(); diff --git a/src/outcome.rs b/src/outcome.rs index 0ca5cab3..90e6c22c 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -3,9 +3,11 @@ //! The outcome of running a single mutation scenario, or a whole lab. use std::fmt; -use std::time::Duration; -use std::time::Instant; +use std::fs::read_to_string; +use std::time::{Duration, Instant}; +use anyhow::Context; +use camino::Utf8PathBuf; use humantime::format_duration; use output::ScenarioOutput; use serde::ser::SerializeStruct; @@ -15,7 +17,7 @@ use tracing::warn; use crate::console::plural; use crate::process::Exit; -use crate::*; +use crate::{exit_code, output, Options, Result, Scenario}; /// What phase of running a scenario. /// @@ -36,7 +38,7 @@ pub enum Phase { } impl Phase { - pub fn name(&self) -> &'static str { + pub fn name(self) -> &'static str { match self { Phase::Check => "check", Phase::Build => "build", @@ -53,6 +55,7 @@ impl fmt::Display for Phase { /// The outcome from a whole lab run containing multiple mutants. #[derive(Debug, Default, Serialize)] +#[allow(clippy::module_name_repetitions)] pub struct LabOutcome { /// All the scenario outcomes, including baseline builds. pub outcomes: Vec, @@ -140,6 +143,7 @@ impl LabOutcome { /// The result of running one mutation scenario. #[derive(Debug, Clone, Eq, PartialEq)] +#[allow(clippy::module_name_repetitions)] pub struct ScenarioOutcome { /// A file holding the text output from running this test. // TODO: Maybe this should be a log object? @@ -173,9 +177,9 @@ impl Serialize for ScenarioOutcome { impl ScenarioOutcome { pub fn new(scenario_output: &ScenarioOutput, scenario: Scenario) -> ScenarioOutcome { ScenarioOutcome { - output_dir: scenario_output.output_dir.to_owned(), + output_dir: scenario_output.output_dir.clone(), log_path: scenario_output.log_path().to_owned(), - diff_path: scenario_output.diff_path.to_owned(), + diff_path: scenario_output.diff_path.clone(), scenario, phase_results: Vec::new(), } @@ -311,6 +315,7 @@ impl Serialize for PhaseResult { /// Overall summary outcome for one mutant. #[derive(Debug, Clone, Eq, PartialEq, Serialize, Hash)] +#[allow(clippy::module_name_repetitions)] pub enum SummaryOutcome { Success, CaughtMutant, diff --git a/src/output.rs b/src/output.rs index 1505489a..71fd0616 100644 --- a/src/output.rs +++ b/src/output.rs @@ -2,14 +2,14 @@ //! A `mutants.out` directory holding logs and other output. -use std::collections::hash_map::Entry; -use std::collections::HashMap; -use std::fs::{create_dir, remove_dir_all, rename, write, File, OpenOptions}; +use std::collections::{hash_map::Entry, HashMap}; +use std::fs::{create_dir, read_to_string, remove_dir_all, rename, write, File, OpenOptions}; use std::io::{BufWriter, Write}; use std::path::Path; use std::thread::sleep; use std::time::Duration; +use camino::{Utf8Path, Utf8PathBuf}; use fs2::FileExt; use path_slash::PathExt; use serde::Serialize; @@ -18,7 +18,7 @@ use time::OffsetDateTime; use tracing::{info, trace}; use crate::outcome::{LabOutcome, SummaryOutcome}; -use crate::*; +use crate::{check_interrupted, Context, Mutant, Result, Scenario, ScenarioOutcome}; const OUTDIR_NAME: &str = "mutants.out"; const ROTATED_NAME: &str = "mutants.out.old"; @@ -86,6 +86,7 @@ impl LockFile { /// A `mutants.out` directory holding logs and other output information. #[derive(Debug)] +#[allow(clippy::module_name_repetitions)] pub struct OutputDir { path: Utf8PathBuf, @@ -276,7 +277,7 @@ pub fn load_previously_caught(output_parent_dir: &Utf8Path) -> Result Result Result<()> { - self.message(&format!("mutation diff:\n{}", diff))?; + self.message(&format!("mutation diff:\n{diff}"))?; let diff_path = self.diff_path.as_ref().expect("should know the diff path"); write(self.output_dir.join(diff_path), diff.as_bytes()) .with_context(|| format!("write diff to {diff_path}")) @@ -332,7 +334,7 @@ impl ScenarioOutput { OpenOptions::new() .read(true) .open(&path) - .with_context(|| format!("reopen {} for read", path)) + .with_context(|| format!("reopen {path} for read")) } /// Open a new handle that appends to the log file, so that it can be passed to a subprocess. @@ -341,7 +343,7 @@ impl ScenarioOutput { OpenOptions::new() .append(true) .open(&path) - .with_context(|| format!("reopen {} for append", path)) + .with_context(|| format!("reopen {path} for append")) } /// Write a message, with a marker. @@ -370,6 +372,7 @@ mod test { use tempfile::{tempdir, TempDir}; use super::*; + use crate::workspace::Workspace; fn minimal_source_tree() -> TempDir { let tmp = tempdir().unwrap(); diff --git a/src/package.rs b/src/package.rs index f11d4785..bb2c86e8 100644 --- a/src/package.rs +++ b/src/package.rs @@ -20,6 +20,7 @@ pub struct Package { /// A more specific view of which packages to mutate, after resolving `PackageFilter::Auto`. #[derive(Debug, Clone)] +#[allow(clippy::module_name_repetitions)] pub enum PackageSelection { All, Explicit(Vec), diff --git a/src/path.rs b/src/path.rs index 75eb304b..99027c7c 100644 --- a/src/path.rs +++ b/src/path.rs @@ -28,7 +28,7 @@ pub fn ascent(path: &Utf8Path) -> isize { max_ascent } -/// An extension trait that helps Utf8Path print with forward slashes, +/// An extension trait that helps `Utf8Path` print with forward slashes, /// even on Windows. /// /// This makes the output more consistent across platforms and so easier diff --git a/src/pretty.rs b/src/pretty.rs index ee1cec5c..1e91a112 100644 --- a/src/pretty.rs +++ b/src/pretty.rs @@ -1,4 +1,4 @@ -// Copyright 2021-2023 Martin Pool +// Copyright 2021-2024 Martin Pool //! Convert a token stream back to (reasonably) pretty Rust code in a string. @@ -10,10 +10,10 @@ pub(crate) trait ToPrettyString { fn to_pretty_string(&self) -> String; } -/// Convert a TokenStream representing some code to a reasonably formatted +/// Convert a `TokenStream` representing some code to a reasonably formatted /// string of Rust code. /// -/// [TokenStream] has a `to_string`, but it adds spaces in places that don't +/// `TokenStream` has a `to_string`, but it adds spaces in places that don't /// look idiomatic, so this reimplements it in a way that looks better. /// /// This is probably not correctly formatted for all Rust syntax, and only tries @@ -23,7 +23,7 @@ where T: ToTokens, { fn to_pretty_string(&self) -> String { - use TokenTree::*; + use TokenTree::{Group, Ident, Literal, Punct}; let mut b = String::with_capacity(200); let mut ts = self.to_token_stream().into_iter().peekable(); while let Some(tt) = ts.next() { diff --git a/src/process.rs b/src/process.rs index 17bbfb44..439c1f67 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,7 +5,8 @@ //! On Unix, the subprocess runs as its own process group, so that any //! grandchild processes are also signalled if it's interrupted. -#![allow(clippy::option_map_unit_fn)] // I don't think it's clearer with if/let. +#![warn(clippy::pedantic)] +#![allow(clippy::redundant_else)] use std::ffi::OsStr; use std::process::{Child, Command, Stdio}; @@ -49,7 +50,7 @@ impl Process { env: &[(String, String)], cwd: &Utf8Path, timeout: Option, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, console: &Console, ) -> Result { @@ -57,10 +58,9 @@ impl Process { let process_status = loop { if let Some(exit_status) = child.poll()? { break exit_status; - } else { - console.tick(); - sleep(WAIT_POLL_INTERVAL); } + console.tick(); + sleep(WAIT_POLL_INTERVAL); }; scenario_output.message(&format!("result: {process_status:?}"))?; Ok(process_status) @@ -72,7 +72,7 @@ impl Process { env: &[(String, String)], cwd: &Utf8Path, timeout: Option, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, ) -> Result { let start = Instant::now(); @@ -88,7 +88,9 @@ impl Process { .stdout(scenario_output.open_log_append()?) .stderr(scenario_output.open_log_append()?) .current_dir(cwd); - jobserver.as_ref().map(|js| js.configure(&mut command)); + if let Some(js) = jobserver { + js.configure(&mut command); + } configure_command(&mut command); let child = command .spawn() @@ -143,26 +145,26 @@ pub enum Exit { /// Exited with status 0. Success, /// Exited with status non-0. - Failure(u32), + Failure(i32), /// Exceeded its timeout, and killed. Timeout, /// Killed by some signal. #[cfg(unix)] - Signalled(u8), + Signalled(i32), /// Unknown or unexpected situation. Other, } impl Exit { - pub fn is_success(&self) -> bool { - *self == Exit::Success + pub fn is_success(self) -> bool { + self == Exit::Success } - pub fn is_timeout(&self) -> bool { - *self == Exit::Timeout + pub fn is_timeout(self) -> bool { + self == Exit::Timeout } - pub fn is_failure(&self) -> bool { + pub fn is_failure(self) -> bool { matches!(self, Exit::Failure(_)) } } @@ -201,13 +203,13 @@ mod test { fn shell_quoting() { assert_eq!(quote_argv(["foo".to_string()]), "foo"); assert_eq!( - quote_argv(["foo bar", r#"\blah\x"#, r#""quoted""#]), + quote_argv(["foo bar", r"\blah\x", r#""quoted""#]), r#"foo\ bar \\blah\\x \"quoted\""# ); assert_eq!(quote_argv([""]), ""); assert_eq!( quote_argv(["with whitespace", "\r\n\t\t"]), - r#"with\ whitespace \r\n\t\t"# + r"with\ whitespace \r\n\t\t" ); } } diff --git a/src/process/unix.rs b/src/process/unix.rs index 5243456f..d5f52aac 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -43,10 +43,10 @@ impl From for Exit { if code == 0 { Exit::Success } else { - Exit::Failure(code as u32) + Exit::Failure(code) } } else if let Some(signal) = status.signal() { - return Exit::Signalled(signal as u8); + return Exit::Signalled(signal); } else { Exit::Other } diff --git a/src/process/windows.rs b/src/process/windows.rs index 9bb913bf..c37950fd 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -20,7 +20,7 @@ impl From for Exit { if code == 0 { Exit::Success } else { - Exit::Failure(code as u32) + Exit::Failure(code) } } else { Exit::Other diff --git a/src/source.rs b/src/source.rs index 0240cedd..057cff91 100644 --- a/src/source.rs +++ b/src/source.rs @@ -21,6 +21,7 @@ use crate::span::LineColumn; /// Code is normalized to Unix line endings as it's read in, and modified /// files are written with Unix line endings. #[derive(Clone, Debug, Eq, PartialEq)] +#[allow(clippy::module_name_repetitions)] pub struct SourceFile { /// Which package within the workspace contains this file? pub package_name: String, @@ -30,7 +31,7 @@ pub struct SourceFile { /// Full copy of the unmodified source. /// - /// This is held in an [Arc] so that SourceFiles can be cloned without using excessive + /// This is held in an [Arc] so that `SourceFile`s can be cloned without using excessive /// amounts of memory. pub code: Arc, @@ -40,7 +41,7 @@ pub struct SourceFile { } impl SourceFile { - /// Construct a SourceFile representing a file within a tree. + /// Construct a `SourceFile` representing a file within a tree. /// /// This eagerly loads the text of the file. /// diff --git a/src/span.rs b/src/span.rs index 5dd0b817..6f12fe0e 100644 --- a/src/span.rs +++ b/src/span.rs @@ -3,7 +3,7 @@ //! Locations (line/column) and spans between them in source code. //! //! This is similar to, and can be automatically derived from, -//! [proc_macro2::Span] and [proc_macro2::LineColumn], but is +//! `proc_macro2::Span` and `proc_macro2::LineColumn`, but is //! a bit more convenient for our purposes. use std::fmt; @@ -183,13 +183,13 @@ mod test { #[test] fn linecolumn_debug_form() { let lc = LineColumn { line: 1, column: 2 }; - assert_eq!(format!("{:?}", lc), "LineColumn(1, 2)"); + assert_eq!(format!("{lc:?}"), "LineColumn(1, 2)"); } #[test] fn span_debug_form() { let span = Span::quad(1, 2, 3, 4); - assert_eq!(format!("{:?}", span), "Span(1, 2, 3, 4)"); + assert_eq!(format!("{span:?}"), "Span(1, 2, 3, 4)"); } #[test] @@ -230,7 +230,7 @@ mod test { } #[test] fn span_ops() { - let source = indoc! { r#" + let source = indoc! { r" fn foo() { some(); @@ -238,19 +238,19 @@ mod test { } const BAR: u32 = 32; - "# }; + " }; // typical multi-line case let span = Span::quad(2, 10, 5, 2); assert_eq!(span.extract(source), "{\n some();\n stuff();\n}"); let replaced = span.replace(source, "{ /* body deleted */ }"); assert_eq!( replaced, - indoc! { r#" + indoc! { r" fn foo() { /* body deleted */ } const BAR: u32 = 32; - "# } + " } ); // single-line case @@ -259,7 +259,7 @@ mod test { let replaced = span.replace(source, "69"); assert_eq!( replaced, - indoc! { r#" + indoc! { r" fn foo() { some(); @@ -267,7 +267,7 @@ mod test { } const BAR: u32 = 69; - "# } + " } ); } } diff --git a/src/tail_file.rs b/src/tail_file.rs index ee5e386c..71789f1e 100644 --- a/src/tail_file.rs +++ b/src/tail_file.rs @@ -23,12 +23,12 @@ pub struct TailFile { impl TailFile { /// Watch lines appended to the given file, which should be open for reading. - pub fn new(file: File) -> Result { - Ok(TailFile { + pub fn new(file: File) -> TailFile { + TailFile { file, last_line_seen: String::new(), read_buf: Vec::new(), - }) + } } /// Return the last non-empty, non-whitespace line from this file, or an empty string @@ -67,7 +67,7 @@ mod test { let mut tempfile = tempfile::NamedTempFile::new().unwrap(); let path: Utf8PathBuf = tempfile.path().to_owned().try_into().unwrap(); let reopened = File::open(&path).unwrap(); - let mut tailer = TailFile::new(reopened).unwrap(); + let mut tailer = TailFile::new(reopened); assert_eq!( tailer.last_line().unwrap(), diff --git a/src/timeouts.rs b/src/timeouts.rs index 47d191df..2d191442 100644 --- a/src/timeouts.rs +++ b/src/timeouts.rs @@ -31,17 +31,17 @@ impl Timeouts { baseline.phase_result(Phase::Build).map(|pr| pr.duration), options, ), - test: test_timeout( + test: Some(test_timeout( baseline.phase_result(Phase::Test).map(|pr| pr.duration), options, - ), + )), } } pub fn without_baseline(options: &Options) -> Timeouts { Timeouts { build: build_timeout(None, options), - test: test_timeout(None, options), + test: Some(test_timeout(None, options)), } } } @@ -51,15 +51,15 @@ fn warn_fallback_timeout(phase_name: &str, option: &str) { warn!("An explicit {phase_name} timeout is recommended when using {option}; using {FALLBACK_TIMEOUT_SECS} seconds by default"); } -fn test_timeout(baseline_duration: Option, options: &Options) -> Option { +fn test_timeout(baseline_duration: Option, options: &Options) -> Duration { if let Some(explicit) = options.test_timeout { - Some(explicit) + explicit } else if let Some(baseline_duration) = baseline_duration { let timeout = max( options.minimum_test_timeout, - Duration::from_secs( + Duration::from_secs_f64( (baseline_duration.as_secs_f64() * options.test_timeout_multiplier.unwrap_or(5.0)) - .ceil() as u64, + .ceil(), ), ); if options.show_times { @@ -68,10 +68,13 @@ fn test_timeout(baseline_duration: Option, options: &Options) -> Optio humantime::format_duration(timeout) ); } - Some(timeout) + timeout + } else if options.check_only { + // We won't have run baseline tests, and we won't run any other tests either. + Duration::from_secs(0) } else { warn_fallback_timeout("test", "--baseline=skip"); - Some(Duration::from_secs(FALLBACK_TIMEOUT_SECS)) + Duration::from_secs(FALLBACK_TIMEOUT_SECS) } } @@ -113,7 +116,7 @@ mod test { assert_eq!(options.test_timeout_multiplier, Some(1.5)); assert_eq!( test_timeout(Some(Duration::from_secs(40)), &options), - Some(Duration::from_secs(60)), + Duration::from_secs(60), ); } @@ -124,7 +127,7 @@ mod test { assert_eq!( test_timeout(Some(Duration::from_secs(40)), &options), - Some(Duration::from_secs(60)), + Duration::from_secs(60), ); } @@ -167,7 +170,7 @@ mod test { assert_eq!(options.test_timeout_multiplier, Some(2.0)); assert_eq!( test_timeout(Some(Duration::from_secs(42)), &options), - Some(Duration::from_secs(42 * 2)), + Duration::from_secs(42 * 2), ); } @@ -195,7 +198,7 @@ mod test { assert_eq!(options.test_timeout_multiplier, None); assert_eq!( test_timeout(Some(Duration::from_secs(42)), &options), - Some(Duration::from_secs(42 * 5)), + Duration::from_secs(42 * 5), ); } @@ -240,7 +243,7 @@ mod test { let options = Options::new(&args, &Config::default()).unwrap(); assert_eq!(options.test_timeout_multiplier, None); - assert_eq!(test_timeout(None, &options), Some(Duration::from_secs(300))); + assert_eq!(test_timeout(None, &options), Duration::from_secs(300)); assert_eq!(build_timeout(None, &options), None); } } diff --git a/src/visit.rs b/src/visit.rs index babe4117..83547444 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -7,10 +7,13 @@ //! e.g. for cargo they are identified from the targets. The tree walker then //! follows `mod` statements to recursively visit other referenced files. +#![warn(clippy::pedantic)] + use std::collections::VecDeque; use std::sync::Arc; use std::vec; +use camino::{Utf8Path, Utf8PathBuf}; use proc_macro2::{Ident, TokenStream}; use quote::{quote, ToTokens}; use syn::ext::IdentExt; @@ -24,7 +27,7 @@ use crate::mutate::Function; use crate::pretty::ToPrettyString; use crate::source::SourceFile; use crate::span::Span; -use crate::*; +use crate::{check_interrupted, Console, Context, Genre, Mutant, Options, Result}; /// Mutants and files discovered in a source tree. /// @@ -44,7 +47,7 @@ impl Discovered { trace!(?name, "skip previously caught mutant"); } !c - }) + }); } } @@ -72,13 +75,13 @@ pub fn walk_tree( // collect any mutants from them, and they don't count as "seen" for // `--list-files`. for mod_namespace in &external_mods { - if let Some(mod_path) = find_mod_source(workspace_dir, &source_file, mod_namespace)? { + if let Some(mod_path) = find_mod_source(workspace_dir, &source_file, mod_namespace) { file_queue.extend(SourceFile::load( workspace_dir, &mod_path, &source_file.package_name, false, - )?) + )?); } } let path = &source_file.tree_relative_path; @@ -184,8 +187,7 @@ impl ModNamespace { fn get_filesystem_name(&self) -> &Utf8Path { self.path_attribute .as_ref() - .map(Utf8PathBuf::as_path) - .unwrap_or(Utf8Path::new(&self.name)) + .map_or(Utf8Path::new(&self.name), Utf8PathBuf::as_path) } } @@ -263,14 +265,14 @@ impl DiscoveryVisitor<'_> { } /// Record that we generated some mutants. - fn collect_mutant(&mut self, span: Span, replacement: TokenStream, genre: Genre) { + fn collect_mutant(&mut self, span: Span, replacement: &TokenStream, genre: Genre) { self.mutants.push(Mutant { source_file: self.source_file.clone(), function: self.fn_stack.last().cloned(), span, replacement: replacement.to_pretty_string(), genre, - }) + }); } fn collect_fn_mutants(&mut self, sig: &Signature, block: &Block) { @@ -299,7 +301,7 @@ impl DiscoveryVisitor<'_> { if orig_block == new_block { debug!("Replacement is the same as the function body; skipping"); } else { - self.collect_mutant(body_span, rep, Genre::FnValue); + self.collect_mutant(body_span, &rep, Genre::FnValue); } } } @@ -527,10 +529,8 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { BinOp::Ge(_) => vec![quote! {<}], BinOp::Add(_) => vec![quote! {-}, quote! {*}], BinOp::AddAssign(_) => vec![quote! {-=}, quote! {*=}], - BinOp::Sub(_) => vec![quote! {+}, quote! {/}], - BinOp::SubAssign(_) => vec![quote! {+=}, quote! {/=}], - BinOp::Mul(_) => vec![quote! {+}, quote! {/}], - BinOp::MulAssign(_) => vec![quote! {+=}, quote! {/=}], + BinOp::Sub(_) | BinOp::Mul(_) => vec![quote! {+}, quote! {/}], + BinOp::SubAssign(_) | BinOp::MulAssign(_) => vec![quote! {+=}, quote! {/=}], BinOp::Div(_) => vec![quote! {%}, quote! {*}], BinOp::DivAssign(_) => vec![quote! {%=}, quote! {*=}], BinOp::Rem(_) => vec![quote! {/}, quote! {+}], @@ -555,7 +555,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { }; replacements .into_iter() - .for_each(|rep| self.collect_mutant(i.op.span().into(), rep, Genre::BinaryOperator)); + .for_each(|rep| self.collect_mutant(i.op.span().into(), &rep, Genre::BinaryOperator)); syn::visit::visit_expr_binary(self, i); } @@ -567,7 +567,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { } match i.op { UnOp::Not(_) | UnOp::Neg(_) => { - self.collect_mutant(i.op.span().into(), quote! {}, Genre::UnaryOperator); + self.collect_mutant(i.op.span().into(), "e! {}, Genre::UnaryOperator); } _ => { trace!( @@ -596,7 +596,7 @@ fn find_mod_source( tree_root: &Utf8Path, parent: &SourceFile, mod_namespace: &ExternalModRef, -) -> Result> { +) -> Option { // First, work out whether the mod will be a sibling in the same directory, or // in a child directory. // @@ -663,15 +663,14 @@ fn find_mod_source( let full_path = tree_root.join(&relative_path); if full_path.is_file() { trace!("found submodule in {full_path}"); - return Ok(Some(relative_path)); - } else { - tried_paths.push(full_path); + return Some(relative_path); } + tried_paths.push(full_path); } let mod_name = &mod_child.name; let definition_site = parent.format_source_location(mod_child.source_location.start); warn!(?definition_site, %mod_name, ?tried_paths, "referent of mod not found"); - Ok(None) + None } /// True if the signature of a function is such that it should be excluded. @@ -738,15 +737,12 @@ fn path_is(path: &syn::Path, idents: &[&str]) -> bool { /// /// This does not check type arguments. fn path_ends_with(path: &syn::Path, ident: &str) -> bool { - path.segments - .last() - .map(|s| s.ident == ident) - .unwrap_or(false) + path.segments.last().is_some_and(|s| s.ident == ident) } /// True if the attribute contains `mutants::skip`. /// -/// This for example returns true for `#[mutants::skip] or `#[cfg_attr(test, mutants::skip)]`. +/// This for example returns true for `#[mutants::skip]` or `#[cfg_attr(test, mutants::skip)]`. fn attr_is_mutants_skip(attr: &Attribute) -> bool { if path_is(attr.path(), &["mutants", "skip"]) { return true; @@ -757,7 +753,7 @@ fn attr_is_mutants_skip(attr: &Attribute) -> bool { let mut skip = false; if let Err(err) = attr.parse_nested_meta(|meta| { if path_is(&meta.path, &["mutants", "skip"]) { - skip = true + skip = true; } Ok(()) }) { @@ -802,11 +798,12 @@ fn find_path_attribute(attrs: &[Attribute]) -> std::result::Result std::result::Result, String> { let token_string = token_stream.to_string(); let item_mod = syn::parse_str::(&token_string).unwrap_or_else(|err| { @@ -880,13 +877,13 @@ mod test { #[test] fn find_path_attribute_on_module_item() { - let outer = run_find_path_attribute(quote! { + let outer = run_find_path_attribute("e! { #[path = "foo_file.rs"] mod foo; }); assert_eq!(outer, Ok(Some(Utf8PathBuf::from("foo_file.rs")))); - let inner = run_find_path_attribute(quote! { + let inner = run_find_path_attribute("e! { mod foo { #![path = "foo_folder"] @@ -900,26 +897,26 @@ mod test { #[test] fn reject_module_path_absolute() { // dots are valid - let dots = run_find_path_attribute(quote! { + let dots = run_find_path_attribute("e! { #[path = "contains/../dots.rs"] mod dots; }); assert_eq!(dots, Ok(Some(Utf8PathBuf::from("contains/../dots.rs")))); - let dots_inner = run_find_path_attribute(quote! { + let dots_inner = run_find_path_attribute("e! { mod dots_in_path { #![path = "contains/../dots"] } }); assert_eq!(dots_inner, Ok(Some(Utf8PathBuf::from("contains/../dots")))); - let leading_slash = run_find_path_attribute(quote! { + let leading_slash = run_find_path_attribute("e! { #[path = "/leading_slash.rs"] mod dots; }); assert_eq!(leading_slash, Err("/leading_slash.rs".to_owned())); - let allow_other_slashes = run_find_path_attribute(quote! { + let allow_other_slashes = run_find_path_attribute("e! { #[path = "foo/other/slashes/are/allowed.rs"] mod dots; }); @@ -928,7 +925,7 @@ mod test { Ok(Some(Utf8PathBuf::from("foo/other/slashes/are/allowed.rs"))) ); - let leading_slash2 = run_find_path_attribute(quote! { + let leading_slash2 = run_find_path_attribute("e! { #[path = "/leading_slash/../and_dots.rs"] mod dots; }); @@ -978,9 +975,7 @@ mod test { #[test] fn skip_with_capacity_by_default() { - let args = Args::try_parse_from(["mutants"]).unwrap(); - let config = Config::default(); - let options = Options::new(&args, &config).unwrap(); + let options = Options::from_arg_strs(["mutants"]); let mut mutants = mutate_source_str( indoc! {" fn main() { @@ -997,9 +992,7 @@ mod test { #[test] fn mutate_vec_with_capacity_when_default_skips_are_turned_off() { - let args = Args::try_parse_from(["mutants", "--skip-calls-defaults", "false"]).unwrap(); - let config = Config::default(); - let options = Options::new(&args, &config).unwrap(); + let options = Options::from_arg_strs(["mutants", "--skip-calls-defaults", "false"]); let mutants = mutate_source_str( indoc! {" fn main() { diff --git a/tests/check_build.rs b/tests/check_build.rs index 6401c5d8..ecf044f3 100644 --- a/tests/check_build.rs +++ b/tests/check_build.rs @@ -2,6 +2,7 @@ //! Tests for `--check` +use indoc::indoc; use predicates::prelude::*; use pretty_assertions::assert_eq; @@ -16,8 +17,7 @@ fn small_well_tested_tree_check_only() { .current_dir(tmp_src_dir.path()) .assert() .success() - .stdout(predicate::function(|stdout: &str| { - insta::assert_snapshot!(stdout, @r###" + .stdout(indoc! {r" Found 4 mutants to test ok Unmutated baseline ok src/lib.rs:5:5: replace factorial -> u32 with 0 @@ -25,9 +25,8 @@ fn small_well_tested_tree_check_only() { ok src/lib.rs:7:11: replace *= with += in factorial ok src/lib.rs:7:11: replace *= with /= in factorial 4 mutants tested: 4 succeeded - "###); - true - })); + "}) + .stderr(""); let outcomes = outcome_json_counts(&tmp_src_dir); assert_eq!( outcomes, @@ -124,8 +123,7 @@ fn check_tree_with_mutants_skip() { .env_remove("RUST_BACKTRACE") .assert() .success() - .stdout(predicate::function(|stdout: &str| { - insta::assert_snapshot!(stdout, @r###" + .stdout(indoc! { r" Found 5 mutants to test ok Unmutated baseline ok src/lib.rs:15:5: replace controlled_loop with () @@ -134,9 +132,8 @@ fn check_tree_with_mutants_skip() { ok src/lib.rs:21:53: replace * with + in controlled_loop ok src/lib.rs:21:53: replace * with / in controlled_loop 5 mutants tested: 5 succeeded - "###); - true - })); + "}) + .stderr(""); assert_eq!( outcome_json_counts(&tmp_src_dir), serde_json::json!({ diff --git a/tests/in_place.rs b/tests/in_place.rs index 49bfd6a9..9e248c97 100644 --- a/tests/in_place.rs +++ b/tests/in_place.rs @@ -30,17 +30,18 @@ fn in_place_check_leaves_no_changes() -> Result<()> { "stderr:\n{}", String::from_utf8_lossy(&cmd.get_output().stderr) ); - fn check_file(tmp: &Path, new_name: &str, old_name: &str) -> Result<()> { - let orig_path = Path::new("testdata/small_well_tested"); - println!("comparing {new_name} and {old_name}"); - assert_eq!( - read_to_string(tmp.join(new_name))?.replace("\r\n", "\n"), - read_to_string(orig_path.join(old_name))?.replace("\r\n", "\n"), - "{old_name} should be the same as {new_name}" - ); - Ok(()) - } check_file(tmp_path, "Cargo.toml", "Cargo_test.toml")?; check_file(tmp_path, "src/lib.rs", "src/lib.rs")?; Ok(()) } + +fn check_file(tmp: &Path, new_name: &str, old_name: &str) -> Result<()> { + let orig_path = Path::new("testdata/small_well_tested"); + println!("comparing {new_name} and {old_name}"); + assert_eq!( + read_to_string(tmp.join(new_name))?.replace("\r\n", "\n"), + read_to_string(orig_path.join(old_name))?.replace("\r\n", "\n"), + "{old_name} should be the same as {new_name}" + ); + Ok(()) +} diff --git a/tests/iterate.rs b/tests/iterate.rs index 2252bd9d..e90a4c14 100644 --- a/tests/iterate.rs +++ b/tests/iterate.rs @@ -15,6 +15,7 @@ use tempfile::tempdir; use self::util::run; #[test] +#[allow(clippy::too_many_lines)] // long but pretty straightforward fn iterate() { let temp = tempdir().unwrap(); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 07ec37ec..ae9fe246 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -89,7 +89,7 @@ pub fn copy_testdata_to>(tree_name: &str, dest: P) { }) .after_entry_copied(|path, _file_type, _stats| { if path.ends_with("Cargo_test.toml") || path.ends_with(".cargo_test") { - renames.push(dest.join(path)) + renames.push(dest.join(path)); } Ok(()) })