Repository: sourcefrog/cargo-mutants (rust)

Issue:
Second-level modules not found unless their parent is a `mod.rs`
Given a project structure with `src/lib.rs`:

```rust
mod modules;
```

`src/modules.rs`:

```rust
mod foo;
```

and `src/modules/foo.rs` containing a function that could be mutated, `cargo mutants` does not mutate the function and instead produces a warning and finds no mutants:

```rust
   0.645927098s  WARN src/modules.rs:2: mod Ident { sym: foo, span: bytes(28..31) } not found
```

However, it succeeds if `src/modules.rs` is renamed to `src/modules/mod.rs`.

It seems that `cargo mutants` v1.2.0 is not correctly calculating module paths for children of modules whose source files are not named using the `mod.rs` style.

Pull request:
Correctly traverse from src/foo.rs to src/foo/bar.rs
Fixes #102

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 8b59a07..f35c27b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -92,7 +92,7 @@ dependencies = [
 
 [[package]]
 name = "cargo-mutants"
-version = "1.2.0"
+version = "1.2.1-pre"
 dependencies = [
  "anyhow",
  "assert_cmd",
diff --git a/Cargo.toml b/Cargo.toml
index 62a1a9f..d8c032c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "cargo-mutants"
-version = "1.2.0"
+version = "1.2.1-pre"
 edition = "2021"
 authors = ["Martin Pool"]
 license = "MIT"
diff --git a/NEWS.md b/NEWS.md
index ff7fcb6..9414dbd 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -4,6 +4,8 @@
 
 - Converted most of the docs to a book available at <https://mutants.rs/>.
 
+- Fixed: Correctly find submodules that don't use `mod.rs` naming, e.g. when descending from `src/foo.rs` to `src/foo/bar.rs`. Also handle module names that are raw identifiers using `r#`. (Thanks to @kpreid for the report.) 
+
 ## 1.2.0
 
 *Thankful mutants!*
diff --git a/src/path.rs b/src/path.rs
index f8d3403..7ae5d4e 100644
--- a/src/path.rs
+++ b/src/path.rs
@@ -95,6 +95,7 @@ impl TreeRelativePathBuf {
         tree_path.join(&self.0)
     }
 
+    #[allow(dead_code)]
     pub fn join(&self, p: impl AsRef<Utf8Path>) -> Self {
         TreeRelativePathBuf(self.0.join(p))
     }
diff --git a/src/visit.rs b/src/visit.rs
index 2fae590..a50f45c 100644
--- a/src/visit.rs
+++ b/src/visit.rs
@@ -9,9 +9,10 @@ use std::sync::Arc;
 
 use anyhow::Context;
 use quote::ToTokens;
+use syn::ext::IdentExt;
 use syn::visit::Visit;
 use syn::{Attribute, ItemFn};
-use tracing::{debug, debug_span, span, trace, warn, Level};
+use tracing::{debug, debug_span, trace, trace_span, warn};
 
 use crate::path::TreeRelativePathBuf;
 use crate::source::{SourceFile, SourceTree};
@@ -160,7 +161,12 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor {
     fn visit_item_fn(&mut self, i: &'ast ItemFn) {
         // TODO: Filter out more inapplicable fns.
         let function_name = remove_excess_spaces(&i.sig.ident.to_token_stream().to_string());
-        let _span = span!(Level::TRACE, "item_fn", function_name).entered();
+        let _span = trace_span!(
+            "fn",
+            line = i.sig.fn_token.span.start().line,
+            name = function_name
+        )
+        .entered();
         if attrs_excluded(&i.attrs) {
             trace!("excluded by attrs");
             return; // don't look inside it either
@@ -218,38 +224,65 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor {
 
     /// Visit `mod foo { ... }` or `mod foo;`.
     fn visit_item_mod(&mut self, node: &'ast syn::ItemMod) {
+        let mod_name = &node.ident.unraw().to_string();
+        let _span = trace_span!(
+            "mod",
+            line = node.mod_token.span.start().line,
+            name = mod_name
+        )
+        .entered();
         if attrs_excluded(&node.attrs) {
-            trace!("mod {:?} excluded by attrs", node.ident);
+            trace!("mod {:?} excluded by attrs", node.ident,);
             return;
         }
-        trace!("visit mod {:?}", node.ident);
-        // If there's no content in braces, we should look for this file. It
-        // could be 'foo.rs' or 'foo/mod.rs', within the same directory as
-        // the current file.
+        // If there's no content in braces, then this is a `mod foo;`
+        // statement referring to an external file. We find the file name
+        // then remember to visit it later.
+        //
+        // Both the current module and the included sub-module can be in
+        // either style: `.../foo.rs` or `.../foo/mod.rs`.
+        //
+        // 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/`.
+        //
+        // Having determined the directory then we can look for either
+        // `foo.rs` or `foo/mod.rs`.
         if node.content.is_none() {
-            let dir = self.source_file.tree_relative_path.parent();
+            let my_path: &Utf8Path = self.source_file.tree_relative_path().as_ref();
+            // Maybe matching on the name here is no the right approach and
+            // we should instead remember how this file was found?
+            let dir = if my_path.ends_with("mod.rs")
+                || my_path.ends_with("lib.rs")
+                || my_path.ends_with("main.rs")
+            {
+                my_path.parent().expect("mod path has no parent").to_owned()
+            } else {
+                my_path.with_extension("")
+            };
             let mut found = false;
+            let mut tried_paths = Vec::new();
             for &ext in &[".rs", "/mod.rs"] {
-                let path = dir.join(format!("{}{}", node.ident, ext));
-                let full_path = path.within(&self.tree_path);
-                trace!("look in {}", full_path);
+                let relative_path = TreeRelativePathBuf::new(dir.join(format!("{mod_name}{ext}")));
+                let full_path = relative_path.within(&self.tree_path);
                 if full_path.is_file() {
-                    self.more_files.push(path);
+                    trace!("found submodule in {full_path}");
+                    self.more_files.push(relative_path);
                     found = true;
+                    break;
+                } else {
+                    tried_paths.push(full_path);
                 }
             }
             if !found {
                 warn!(
-                    "{}:{}: mod {:?} not found",
-                    self.source_file.tree_relative_path,
-                    node.mod_token.span.start().line,
-                    node.ident
+                    "{path}:{line}: referent of mod {mod_name:#?} not found: tried {tried_paths:?}",
+                    path = self.source_file.tree_relative_path,
+                    line = node.mod_token.span.start().line,
                 );
             }
         }
-        self.in_namespace(&node.ident.to_string(), |v| {
-            syn::visit::visit_item_mod(v, node)
-        });
+        self.in_namespace(mod_name, |v| syn::visit::visit_item_mod(v, node));
     }
 }
 
@@ -308,6 +341,8 @@ fn remove_excess_spaces(s: &str) -> String {
     //
     // This is a bit hacky but seems to give reasonably legible results on
     // typical trees...
+    //
+    // We could instead perhaps do this on the type enum.
     let mut r = String::with_capacity(s.len());
     for c in s.chars() {
         match c {


Test patch:
diff --git a/testdata/tree/with_child_directories/src/module.rs b/testdata/tree/with_child_directories/src/module.rs
new file mode 100644
index 0000000..e09b6f2
--- /dev/null
+++ b/testdata/tree/with_child_directories/src/module.rs
@@ -0,0 +1,3 @@
+mod module_methods;
+// Also an example of handling raw symbols:
+mod r#utils;
diff --git a/testdata/tree/with_child_directories/src/module/mod.rs b/testdata/tree/with_child_directories/src/module/mod.rs
deleted file mode 100644
index 52ab96b..0000000
--- a/testdata/tree/with_child_directories/src/module/mod.rs
+++ /dev/null
@@ -1,2 +0,0 @@
-mod module_methods;
-mod utils;