Skip to content

Commit

Permalink
Correctly walk from atypically named top files using the target path (
Browse files Browse the repository at this point in the history
#183)

Fixes #115
  • Loading branch information
sourcefrog authored Dec 16, 2023
2 parents 5cecd31 + b9a2bd6 commit 1d1459f
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 19 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ exclude = [
"testdata/cdylib",
"testdata/cfg_attr_mutants_skip",
"testdata/cfg_attr_test_skip",
"testdata/custom_top_file",
"testdata/dependency",
"testdata/diff0",
"testdata/diff1",
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Fixed: Correctly traverse `mod` statements within package top source files that are not named `lib.rs` or `main.rs`, by following the `path` setting of each target within the manifest.

- Improved: Don't generate function mutants that have the same AST as the code they're replacing.

## 23.12.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -615,9 +615,6 @@ src/visit.rs: replace find_mod_source -> Result<Option<Utf8PathBuf>> with Err(::
src/visit.rs: replace || with && in find_mod_source
src/visit.rs: replace || with == in find_mod_source
src/visit.rs: replace || with != in find_mod_source
src/visit.rs: replace || with && in find_mod_source
src/visit.rs: replace || with == in find_mod_source
src/visit.rs: replace || with != in find_mod_source
src/visit.rs: replace fn_sig_excluded -> bool with true
src/visit.rs: replace fn_sig_excluded -> bool with false
src/visit.rs: replace attrs_excluded -> bool with true
Expand Down
7 changes: 7 additions & 0 deletions src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ pub struct SourceFile {
/// This is held in an [Arc] so that SourceFiles can be cloned without using excessive
/// amounts of memory.
pub code: Arc<String>,

/// True if this is the top source file for its target: typically but
/// not always `lib.rs` or `main.rs`.
pub is_top: bool,
}

impl SourceFile {
Expand All @@ -42,6 +46,7 @@ impl SourceFile {
tree_path: &Utf8Path,
tree_relative_path: Utf8PathBuf,
package: &Arc<Package>,
is_top: bool,
) -> Result<SourceFile> {
let full_path = tree_path.join(&tree_relative_path);
let code = Arc::new(
Expand All @@ -53,6 +58,7 @@ impl SourceFile {
tree_relative_path,
code,
package: Arc::clone(package),
is_top,
})
}

Expand Down Expand Up @@ -96,6 +102,7 @@ mod test {
name: "imaginary-package".to_owned(),
relative_manifest_path: "whatever/Cargo.toml".into(),
}),
true,
)
.unwrap();
assert_eq!(source_file.code(), "fn main() {\n 640 << 10;\n}\n");
Expand Down
35 changes: 19 additions & 16 deletions src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ pub fn walk_tree(
workspace_dir,
mod_path,
&source_file.package,
false,
)?)
}
}
Expand Down Expand Up @@ -398,33 +399,34 @@ fn find_mod_source(
parent: &SourceFile,
mod_name: &str,
) -> Result<Option<Utf8PathBuf>> {
// Both the current module and the included sub-module can be in
// either style: `.../foo.rs` or `.../foo/mod.rs`.
// First, work out whether the mod will be a sibling in the same directory, or
// in a child directory.
//
// If the current file ends with `/mod.rs`, then sub-modules
// will be in the same directory as this file. Otherwise, this is
// `/foo.rs` and sub-modules will be in `foo/`.
// 1. The parent is "src/foo.rs" and `mod bar` means "src/foo/bar.rs".
//
// Having determined the directory then we can look for either
// 2. The parent is "src/lib.rs" (a target top file) and `mod bar` means "src/bar.rs".
//
// 3. The parent is "src/foo/mod.rs" and so `mod bar` means "src/foo/bar.rs".
//
// Having determined the right directory then we can look for either
// `foo.rs` or `foo/mod.rs`.

// TODO: Beyond #115, we should probably remove all special handling of
// `mod.rs` here by remembering how we found this file, and whether it
// is above or inside the directory corresponding to its module?

let parent_path = &parent.tree_relative_path;
// TODO: Maybe matching on the name here is not the right approach and
// we should instead remember how this file was found? This might go wrong
// with unusually-named files.
let dir = if parent_path.ends_with("mod.rs")
|| parent_path.ends_with("lib.rs")
|| parent_path.ends_with("main.rs")
{
let search_dir = if parent.is_top || parent_path.ends_with("mod.rs") {
parent_path
.parent()
.expect("mod path has no parent")
.to_owned()
.to_owned() // src/lib.rs -> src/
} else {
parent_path.with_extension("")
parent_path.with_extension("") // foo.rs -> foo/
};
let mut tried_paths = Vec::new();
for &tail in &[".rs", "/mod.rs"] {
let relative_path = dir.join(mod_name.to_owned() + tail);
let relative_path = search_dir.join(mod_name.to_owned() + tail);
let full_path = tree_root.join(&relative_path);
if full_path.is_file() {
trace!("found submodule in {full_path}");
Expand Down Expand Up @@ -541,6 +543,7 @@ mod test {
relative_manifest_path: "Cargo.toml".into(),
}),
tree_relative_path: Utf8PathBuf::from("src/lib.rs"),
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();
Expand Down
1 change: 1 addition & 0 deletions src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ impl Workspace {
&self.dir,
source_path.to_owned(),
&package,
true,
)?);
}
}
Expand Down
11 changes: 11 additions & 0 deletions testdata/custom_top_file/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[package]
name = "cargo-mutants-testdata-custom-top-file"
description = "A package that overrides the default file name for a target"
version = "0.0.0"
edition = "2018"
authors = ["Martin Pool"]
publish = false

[lib]
doctest = false
path = "src/custom_top.rs"
14 changes: 14 additions & 0 deletions testdata/custom_top_file/src/custom_top.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
pub fn is_even(n: u32) -> bool {
n % 2 == 0
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_is_even() {
assert!(is_even(2));
assert!(!is_even(3));
}
}
97 changes: 97 additions & 0 deletions tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,103 @@ expression: buf
]
```

## testdata/custom_top_file

```json
[
{
"file": "src/custom_top.rs",
"function": {
"function_name": "is_even",
"return_type": "-> bool",
"span": {
"end": {
"column": 2,
"line": 3
},
"start": {
"column": 1,
"line": 1
}
}
},
"genre": "FnValue",
"package": "cargo-mutants-testdata-custom-top-file",
"replacement": "true",
"span": {
"end": {
"column": 15,
"line": 2
},
"start": {
"column": 5,
"line": 2
}
}
},
{
"file": "src/custom_top.rs",
"function": {
"function_name": "is_even",
"return_type": "-> bool",
"span": {
"end": {
"column": 2,
"line": 3
},
"start": {
"column": 1,
"line": 1
}
}
},
"genre": "FnValue",
"package": "cargo-mutants-testdata-custom-top-file",
"replacement": "false",
"span": {
"end": {
"column": 15,
"line": 2
},
"start": {
"column": 5,
"line": 2
}
}
},
{
"file": "src/custom_top.rs",
"function": {
"function_name": "is_even",
"return_type": "-> bool",
"span": {
"end": {
"column": 2,
"line": 3
},
"start": {
"column": 1,
"line": 1
}
}
},
"genre": "BinaryOperator",
"package": "cargo-mutants-testdata-custom-top-file",
"replacement": "!=",
"span": {
"end": {
"column": 13,
"line": 2
},
"start": {
"column": 11,
"line": 2
}
}
}
]
```

## testdata/dependency

```json
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ src/lib.rs:18:5: replace double -> usize with 0
src/lib.rs:18:5: replace double -> usize with 1
```

## testdata/custom_top_file

```
src/custom_top.rs:2:5: replace is_even -> bool with true
src/custom_top.rs:2:5: replace is_even -> bool with false
src/custom_top.rs:2:11: replace == with != in is_even
```

## testdata/dependency

```
Expand Down

0 comments on commit 1d1459f

Please sign in to comment.