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

Correctly walk from atypically named top files using the target path #183

Merged
merged 3 commits into from
Dec 16, 2023
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
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