Skip to content

Commit

Permalink
refactor: Separate Mutant::name from to_styled_string (#440)
Browse files Browse the repository at this point in the history
In most cases we don't need the styles, and the two bool args are
unclear.
  • Loading branch information
sourcefrog authored Nov 13, 2024
2 parents fc8531c + 3221793 commit 3365530
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,8 @@ fn style_mb(bytes: u64) -> StyledObject<String> {

pub fn style_scenario(scenario: &Scenario, line_col: bool) -> Cow<'static, str> {
match scenario {
Scenario::Baseline => "Unmutated baseline".into(),
Scenario::Mutant(mutant) => mutant.name(line_col, true).into(),
Scenario::Baseline => Cow::Borrowed("Unmutated baseline"),
Scenario::Mutant(mutant) => Cow::Owned(mutant.to_styled_string(line_col)),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/in_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub fn diff_filter(mutants: Vec<Mutant>, diff_text: &str) -> Result<Vec<Mutant>>
trace!(
?path,
line,
mutant = mutant.name(true, false),
mutant = mutant.name(true),
"diff matched mutant"
);
matched.push(mutant);
Expand Down
2 changes: 1 addition & 1 deletion src/lab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ impl Worker<'_> {
let Some(mutant) = work_queue.lock().expect("Lock pending work queue").next() else {
return Ok(());
};
let _span = debug_span!("mutant", name = mutant.name(false, false)).entered();
let _span = debug_span!("mutant", name = mutant.name(false)).entered();
let test_package = match &self.options.test_package {
TestPackages::Workspace => PackageSelection::All,
TestPackages::Mutated => {
Expand Down
6 changes: 5 additions & 1 deletion src/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ pub fn list_mutants(mutants: &[Mutant], options: &Options) -> String {
// TODO: Use with_capacity when we can have mutants skip it (#315
let mut out = String::new();
for mutant in mutants {
out.push_str(&mutant.name(options.show_line_col, colors));
if colors {
out.push_str(&mutant.to_styled_string(options.show_line_col));
} else {
out.push_str(&mutant.name(options.show_line_col));
}
out.push('\n');
if options.emit_diffs {
out.push_str(&mutant.diff(&mutant.mutated_code()));
Expand Down
42 changes: 27 additions & 15 deletions src/mutate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,28 @@ impl Mutant {
.join("")
}

pub fn name(&self, show_line_col: bool, styled: bool) -> String {
pub fn name(&self, show_line_col: bool) -> String {
let mut v = Vec::new();
v.push(self.source_file.tree_relative_slashes());
if show_line_col {
v.push(format!(
":{}:{}: ",
self.span.start.line, self.span.start.column
));
} else {
v.push(": ".to_owned());
}
v.extend(
self.styled_parts()
.into_iter()
.map(|x| x.force_styling(false).to_string()),
);
v.join("")
}

/// Return a one-line description of this mutant, with coloring, including the file names
/// and optionally the line and column.
pub fn to_styled_string(&self, show_line_col: bool) -> String {
let mut v = Vec::new();
v.push(self.source_file.tree_relative_slashes());
if show_line_col {
Expand All @@ -98,16 +119,7 @@ impl Mutant {
));
}
v.push(": ".to_owned());
let parts = self.styled_parts();
if styled {
v.extend(parts.into_iter().map(|x| x.to_string()));
} else {
v.extend(
parts
.into_iter()
.map(|x| x.force_styling(false).to_string()),
);
}
v.extend(self.styled_parts().into_iter().map(|x| x.to_string()));
v.join("")
}

Expand Down Expand Up @@ -271,7 +283,7 @@ mod test {
}
);
assert_eq!(
mutants[0].name(true, false),
mutants[0].name(true),
"src/bin/factorial.rs:2:5: replace main with ()"
);
assert_eq!(
Expand All @@ -293,15 +305,15 @@ mod test {
}
);
assert_eq!(
mutants[1].name(false, false),
mutants[1].name(false),
"src/bin/factorial.rs: replace factorial -> u32 with 0"
);
assert_eq!(
mutants[1].name(true, false),
mutants[1].name(true),
"src/bin/factorial.rs:8:5: replace factorial -> u32 with 0"
);
assert_eq!(
mutants[2].name(true, false),
mutants[2].name(true),
"src/bin/factorial.rs:8:5: replace factorial -> u32 with 1"
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ impl OutputDir {
SummaryOutcome::Unviable => &mut self.unviable_list,
_ => return Ok(()),
};
writeln!(file, "{}", mutant.name(true, false)).context("write to list file")?;
writeln!(file, "{}", mutant.name(true)).context("write to list file")?;
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/scenario.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl fmt::Display for Scenario {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Scenario::Baseline => f.write_str("baseline"),
Scenario::Mutant(mutant) => f.write_str(&mutant.name(true, false)),
Scenario::Mutant(mutant) => f.write_str(&mutant.name(true)),
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub struct Discovered {
impl Discovered {
pub(crate) fn remove_previously_caught(&mut self, previously_caught: &[String]) {
self.mutants.retain(|m| {
let name = m.name(true, false);
let name = m.name(true);
let c = previously_caught.contains(&name);
if c {
trace!(?name, "skip previously caught mutant");
Expand Down Expand Up @@ -98,7 +98,7 @@ pub fn walk_tree(
files.push(source_file);
}
mutants.retain(|m| {
let name = m.name(true, false);
let name = m.name(true);
(options.examine_names.is_empty() || options.examine_names.is_match(&name))
&& (options.exclude_names.is_empty() || !options.exclude_names.is_match(&name))
});
Expand Down Expand Up @@ -772,7 +772,7 @@ mod test {
is_top: true,
};
let (mutants, _files) = walk_file(&source_file, &[]).expect("walk_file");
let mutant_names = mutants.iter().map(|m| m.name(false, false)).collect_vec();
let mutant_names = mutants.iter().map(|m| m.name(false)).collect_vec();
// It would be good to suggest replacing this with 'false', breaking a key behavior,
// but bad to replace it with 'true', changing nothing.
assert_eq!(
Expand Down

0 comments on commit 3365530

Please sign in to comment.