Skip to content

Commit

Permalink
clippy: Many more cleanups; enable pedantic warnings (#457)
Browse files Browse the repository at this point in the history
  • Loading branch information
sourcefrog authored Nov 24, 2024
2 parents 83f8591 + a08a737 commit 9d711fc
Show file tree
Hide file tree
Showing 27 changed files with 281 additions and 270 deletions.
18 changes: 8 additions & 10 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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::Client>,
jobserver: Option<&jobserver::Client>,
packages: &PackageSelection,
phase: Phase,
timeout: Option<Duration>,
Expand Down Expand Up @@ -155,20 +158,15 @@ 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());
}
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?
Expand Down Expand Up @@ -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 <https://github.com/sourcefrog/cargo-mutants/issues/117>
// but it's temporarily regressed.
Expand Down Expand Up @@ -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()]);
Expand Down
79 changes: 34 additions & 45 deletions src/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
///
Expand All @@ -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()));
Expand All @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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);
});
}

Expand All @@ -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.
Expand All @@ -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) {
Expand All @@ -216,19 +203,19 @@ impl Console {
}

pub fn clear(&self) {
self.view.clear()
self.view.clear();
}

pub fn message(&self, message: &str) {
// A workaround for nutmeg not being able to coordinate writes to both stdout and
// stderr...
// <https://github.com/sourcefrog/nutmeg/issues/11>
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.
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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(&copy_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!(
Expand Down Expand Up @@ -489,21 +484,15 @@ struct ScenarioModel {
}

impl ScenarioModel {
fn new(
dir: &Utf8Path,
scenario: &Scenario,
start: Instant,
log_file: File,
) -> Result<ScenarioModel> {
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) {
Expand Down Expand Up @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/copy_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 9d711fc

Please sign in to comment.