Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clippy: Many more cleanups; enable pedantic warnings #457

Merged
merged 21 commits into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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