Repository: sourcefrog/cargo-mutants (rust)

Issue:
fails to traverse from `mod foo` to `foo.rs` in some cases
In 9617adc5ddad3603d730dc3306870364624625bf of https://github.com/openmls/openmls, cargo-mutants 765fca95e5a027367184867004620a28b1782046 says

```
   0.561492717s  WARN traits/src/traits.rs:6: referent of mod "crypto" not found: tried ["/home/mbp/src/openmls/traits/src/traits/crypto.rs", "/home/mbp/src/openmls/traits/src/traits/crypto/mod.rs"]
   0.561519127s  WARN traits/src/traits.rs:7: referent of mod "key_store" not found: tried ["/home/mbp/src/openmls/traits/src/traits/key_store.rs", "/home/mbp/src/openmls/traits/src/traits/key_store/mod.rs"]
   0.561535347s  WARN traits/src/traits.rs:8: referent of mod "random" not found: tried ["/home/mbp/src/openmls/traits/src/traits/random.rs", "/home/mbp/src/openmls/traits/src/traits/random/mod.rs"]
   0.561550568s  WARN traits/src/traits.rs:9: referent of mod "signatures" not found: tried ["/home/mbp/src/openmls/traits/src/traits/signatures.rs", "/home/mbp/src/openmls/traits/src/traits/signatures/mod.rs"]
   0.561570058s  WARN traits/src/traits.rs:10: referent of mod "types" not found: tried ["/home/mbp/src/openmls/traits/src/traits/types.rs", "/home/mbp/src/openmls/traits/src/traits/types/mod.rs"]
Found 1287 mutants to test
Unmutated baseline ... ok in 40.8s build + 46.8s test
```

It still carries on with testing but is perhaps failing to generate some mutants.

This crate has

```
.
├── Cargo.toml
├── CHANGELOG.md
├── README.md
└── src
    ├── crypto.rs
    ├── key_store.rs
    ├── random.rs
    ├── signatures.rs
    ├── traits.rs
    └── types.rs
```

So it should find `mod traits` in `src/traits.rs`. I think it's confused because the top-level file is `traits.rs`, not `lib.rs`, due to configuration in `Cargo.toml`.  

Specifically, `openmls/traits/Cargo.toml` has

```toml
[lib]
path = "src/traits.rs"
```

This code https://github.com/sourcefrog/cargo-mutants/blob/765fca95e5a027367184867004620a28b1782046/src/visit.rs#L250-L298 assumes the top level file will always be `lib.rs` or `main.rs` but that's not reliable.

Maybe we ought to remember if the file is a top source file for the target and use that instead: see <https://doc.rust-lang.org/cargo/reference/cargo-targets.html>. We probably already have this in the cargo metadata.

Pull request:
Correctly walk from atypically named top files using the target `path`
Remember whether the file is a target top, rather than relying on the file name.

Fixes #115 

- [x] Tests with a Cargo.toml using the target `path` feature

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 0b78612..75360de 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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",
diff --git a/NEWS.md b/NEWS.md
index 753ac5a..d9da59f 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -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
diff --git a/src/source.rs b/src/source.rs
index c3f31b3..ec43c2c 100644
--- a/src/source.rs
+++ b/src/source.rs
@@ -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 {
@@ -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(
@@ -53,6 +58,7 @@ impl SourceFile {
             tree_relative_path,
             code,
             package: Arc::clone(package),
+            is_top,
         })
     }
 
@@ -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");
diff --git a/src/visit.rs b/src/visit.rs
index 95978a4..80bc933 100644
--- a/src/visit.rs
+++ b/src/visit.rs
@@ -68,6 +68,7 @@ pub fn walk_tree(
                     workspace_dir,
                     mod_path,
                     &source_file.package,
+                    false,
                 )?)
             }
         }
@@ -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}");
@@ -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();
diff --git a/src/workspace.rs b/src/workspace.rs
index 69fc862..33e7fd7 100644
--- a/src/workspace.rs
+++ b/src/workspace.rs
@@ -194,6 +194,7 @@ impl Workspace {
                     &self.dir,
                     source_path.to_owned(),
                     &package,
+                    true,
                 )?);
             }
         }


Test patch:
diff --git a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap
index 6c63632..726d07a 100644
--- a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap
+++ b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap
@@ -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
diff --git a/testdata/custom_top_file/Cargo.toml b/testdata/custom_top_file/Cargo.toml
new file mode 100644
index 0000000..9908406
--- /dev/null
+++ b/testdata/custom_top_file/Cargo.toml
@@ -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"
diff --git a/testdata/custom_top_file/src/custom_top.rs b/testdata/custom_top_file/src/custom_top.rs
new file mode 100644
index 0000000..9115a6f
--- /dev/null
+++ b/testdata/custom_top_file/src/custom_top.rs
@@ -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));
+    }
+}
diff --git a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap
index fb7ed6b..3e77f81 100644
--- a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap
+++ b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap
@@ -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
diff --git a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap
index 72bcdbc..428dc60 100644
--- a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap
+++ b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap
@@ -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
 
 ```