Skip to main content

testing_conventions/
isolation.rs

1//! Rust unit-isolation lint (#44): an inline `#[cfg(test)] mod` may call only into
2//! the unit under test — its parent module, reached via `super::`. A call *out of
3//! the test's own module* — into another first-party module (`crate::…`), an
4//! external crate, or effectful `std` — is a violation. Inject a trait double
5//! (hand-rolled or `mockall`) instead; the compiler checks the double.
6//!
7//! Detection is AST-based: each `*.rs` file under the crate root is parsed with
8//! `syn` and its `#[cfg(test)]` modules are walked with a [`Visit`]or. This is the
9//! deterministic `syn` heuristic; full name-resolution precision is a future
10//! `dylint` pass. The design and its precision limits live in
11//! `internals/rust/isolation.md`.
12//!
13//! Implemented detectors:
14//! - **`no-out-of-module-call`** (D1): a call expression `A::…::f(…)` inside a
15//!   `#[cfg(test)]` module whose leading segment `A` reaches out of the module —
16//!   `crate::` (first-party, another module), `super::super::…` (an ancestor),
17//!   an external crate from `Cargo.toml`, or effectful `std`. A single `super::`,
18//!   `self`/`Self`, a bare/unqualified call, and pure `std` (incl. `io::Cursor`)
19//!   stay in-module and are not flagged.
20//! - **`no-out-of-module-import`** (D2): a `use` inside a `#[cfg(test)]` module
21//!   that brings in a foreign surface — a glob of anything but `super::*`, or a
22//!   named import rooted at `crate::`, an external crate, or effectful `std`.
23//!   `use super::*` / `use super::Thing` (the unit under test), `self`, and pure
24//!   `std` (e.g. `collections`, `io::Cursor`) are in-module. Catches a collaborator
25//!   imported then called unqualified, which D1's call check can't see.
26
27use std::collections::BTreeSet;
28use std::path::{Path, PathBuf};
29
30use anyhow::{anyhow, Context, Result};
31use syn::spanned::Spanned;
32use syn::visit::{self, Visit};
33
34pub use crate::violation::Violation;
35
36/// Rule id reported for an out-of-module call (D1).
37const RULE_CALL: &str = "no-out-of-module-call";
38/// Rule id reported for an out-of-module `use` import (D2).
39const RULE_IMPORT: &str = "no-out-of-module-import";
40
41/// A language whose unit-isolation convention can be checked (Python #42 is a
42/// separate detector). Each detector lives in its own module; this enum is the
43/// shared `unit isolation` language selector.
44#[derive(Debug, Clone, Copy, PartialEq, Eq, clap::ValueEnum)]
45pub enum Language {
46    /// Inline `#[cfg(test)]` modules in `*.rs` files (`no-out-of-module-call`).
47    #[value(name = "rust")]
48    Rust,
49    /// `*.test.{ts,tsx,mts,cts}` unit tests (`unmocked-collaborator`, #43 / #76);
50    /// the detector lives in [`crate::ts`].
51    #[value(name = "typescript")]
52    TypeScript,
53}
54
55/// Scan the Rust source files under `root` and return every isolation violation,
56/// sorted by `(file, line)` for deterministic output.
57///
58/// `root` is the crate root: its `Cargo.toml` names the external crates whose
59/// calls are out-of-module. Every `*.rs` file under it is parsed; a file that
60/// cannot be read or parsed is an error.
61pub fn find_violations(root: impl AsRef<Path>) -> Result<Vec<Violation>> {
62    let root = root.as_ref();
63    let deps = external_deps(root)?;
64
65    let mut files = Vec::new();
66    collect_rust_files(root, &mut files)?;
67    files.sort();
68
69    let mut violations = Vec::new();
70    for file in &files {
71        let source = std::fs::read_to_string(file)
72            .with_context(|| format!("reading source file `{}`", file.display()))?;
73        let ast = syn::parse_file(&source)
74            .map_err(|err| anyhow!("parsing `{}`: {err}", file.display()))?;
75        let mut visitor = IsolationVisitor {
76            file,
77            deps: &deps,
78            test_depth: 0,
79            violations: Vec::new(),
80        };
81        visitor.visit_file(&ast);
82        violations.append(&mut visitor.violations);
83    }
84
85    violations.sort_by(|a, b| a.file.cmp(&b.file).then(a.line.cmp(&b.line)));
86    Ok(violations)
87}
88
89/// Walks one parsed file, flagging out-of-module calls inside `#[cfg(test)]`
90/// modules. `test_depth` counts how deep we are inside such modules, so a call in
91/// non-test code is ignored.
92struct IsolationVisitor<'a> {
93    file: &'a Path,
94    deps: &'a BTreeSet<String>,
95    test_depth: usize,
96    violations: Vec<Violation>,
97}
98
99impl<'ast> Visit<'ast> for IsolationVisitor<'_> {
100    fn visit_item_mod(&mut self, node: &'ast syn::ItemMod) {
101        let is_test = has_cfg_test(&node.attrs);
102        if is_test {
103            self.test_depth += 1;
104        }
105        visit::visit_item_mod(self, node);
106        if is_test {
107            self.test_depth -= 1;
108        }
109    }
110
111    fn visit_expr_call(&mut self, node: &'ast syn::ExprCall) {
112        if self.test_depth > 0 {
113            if let syn::Expr::Path(path_expr) = node.func.as_ref() {
114                if let Some(kind) = classify(&path_expr.path, self.deps) {
115                    self.violations.push(Violation {
116                        file: self.file.to_path_buf(),
117                        line: node.span().start().line,
118                        rule: RULE_CALL,
119                        message: format!(
120                            "unit test calls `{}` out of its own module ({kind}); \
121                             inject a trait double — only `super::` is in-module",
122                            render_path(&path_expr.path),
123                        ),
124                    });
125                }
126            }
127        }
128        visit::visit_expr_call(self, node);
129    }
130
131    fn visit_item_use(&mut self, node: &'ast syn::ItemUse) {
132        if self.test_depth > 0 {
133            let mut imports = Vec::new();
134            flatten_use(&node.tree, &mut Vec::new(), &mut imports);
135            for (segs, is_glob) in &imports {
136                if let Some(kind) = classify_use(segs, *is_glob, self.deps) {
137                    self.violations.push(Violation {
138                        file: self.file.to_path_buf(),
139                        line: node.span().start().line,
140                        rule: RULE_IMPORT,
141                        message: format!(
142                            "unit test imports `{}` out of its own module ({kind}); \
143                             only `super::` (the unit) and pure `std` belong in a unit test",
144                            render_use(segs, *is_glob),
145                        ),
146                    });
147                }
148            }
149        }
150        visit::visit_item_use(self, node);
151    }
152}
153
154/// Why a call's leading path is out-of-module, or `None` when the call stays
155/// in-module (or is unresolvable, and so deliberately not flagged — the `syn`
156/// heuristic's documented limit).
157fn classify(path: &syn::Path, deps: &BTreeSet<String>) -> Option<&'static str> {
158    let segs: Vec<String> = path.segments.iter().map(|s| s.ident.to_string()).collect();
159    match segs.first().map(String::as_str)? {
160        // `self` / `Self` are local; a single `super::` is the unit under test.
161        "self" | "Self" => None,
162        "super" => (segs.get(1).map(String::as_str) == Some("super")).then_some("ancestor module"),
163        "crate" => Some("first-party module"),
164        "std" => is_effectful_std(&segs).then_some("effectful std"),
165        // `core`/`alloc` carry no effectful APIs.
166        "core" | "alloc" => None,
167        // Any other leading segment is in-module unless it names an external
168        // crate; a local type/fn (incl. `super::*`-imported) is not flagged.
169        other => deps.contains(other).then_some("external crate"),
170    }
171}
172
173/// `true` for an effectful `std` path — filesystem, network, process, env,
174/// threads, OS, the clock (`SystemTime::now` / `Instant::now`), or real-handle
175/// I/O (`stdin`/`stdout`/`stderr`). Pure std is allowed: `std::io::Cursor` and the
176/// I/O traits, `time::Duration`, `collections`, `fmt`, … — `internals/rust/`
177/// `testing.md` makes `Cursor` the idiomatic in-memory unit-test tool.
178fn is_effectful_std(segs: &[String]) -> bool {
179    match segs.get(1).map(String::as_str) {
180        Some("fs" | "net" | "process" | "env" | "thread" | "os") => true,
181        Some("io") => matches!(
182            segs.get(2).map(String::as_str),
183            Some("stdin" | "stdout" | "stderr")
184        ),
185        Some("time") => {
186            matches!(
187                segs.get(2).map(String::as_str),
188                Some("SystemTime" | "Instant")
189            ) && segs.get(3).map(String::as_str) == Some("now")
190        }
191        _ => false,
192    }
193}
194
195/// Flatten a `use` tree into `(path, is_glob)` leaves: `use a::{b, c::*}` yields
196/// `([a, b], false)` and `([a, c], true)`. A rename (`use a::b as c`) is judged by
197/// its source path `[a, b]`.
198fn flatten_use(tree: &syn::UseTree, prefix: &mut Vec<String>, out: &mut Vec<(Vec<String>, bool)>) {
199    match tree {
200        syn::UseTree::Path(path) => {
201            prefix.push(path.ident.to_string());
202            flatten_use(&path.tree, prefix, out);
203            prefix.pop();
204        }
205        syn::UseTree::Name(name) => {
206            let mut full = prefix.clone();
207            full.push(name.ident.to_string());
208            out.push((full, false));
209        }
210        syn::UseTree::Rename(rename) => {
211            let mut full = prefix.clone();
212            full.push(rename.ident.to_string());
213            out.push((full, false));
214        }
215        syn::UseTree::Glob(_) => out.push((prefix.clone(), true)),
216        syn::UseTree::Group(group) => {
217            for item in &group.items {
218                flatten_use(item, prefix, out);
219            }
220        }
221    }
222}
223
224/// Why a `use` import reaches out of the test's own module, or `None` when it
225/// stays in-module. The one legal glob is `super::*`; any other glob is foreign. A
226/// named import is judged by its root like a call — `crate::`, an external crate,
227/// or effectful `std` are out; `super`/`self`, pure `std`, and a local name are in.
228fn classify_use(segs: &[String], is_glob: bool, deps: &BTreeSet<String>) -> Option<&'static str> {
229    match segs.first().map(String::as_str)? {
230        // `super::*` / `super::Thing` are the unit under test; `super::super::…`
231        // reaches past it.
232        "super" => (segs.get(1).map(String::as_str) == Some("super")).then_some("ancestor module"),
233        "self" | "Self" => None,
234        "crate" => Some("first-party module"),
235        "std" if is_effectful_std(segs) => Some("effectful std"),
236        // Pure `std` / `core` / `alloc`: a named import is in-module, but a glob of
237        // anything but `super` is foreign (the issue's bright line).
238        "std" | "core" | "alloc" => is_glob.then_some("glob import"),
239        other => {
240            if deps.contains(other) {
241                Some("external crate")
242            } else {
243                // A local module/type: a named import is in-module; a non-`super`
244                // glob is still foreign.
245                is_glob.then_some("glob import")
246            }
247        }
248    }
249}
250
251/// Render a flattened import for the message: `a::b`, or `a::b::*` for a glob.
252fn render_use(segs: &[String], is_glob: bool) -> String {
253    let mut out = segs.join("::");
254    if is_glob {
255        if !out.is_empty() {
256            out.push_str("::");
257        }
258        out.push('*');
259    }
260    out
261}
262
263/// Render a path back to `a::b::c` for the message (idents only; generic args
264/// dropped).
265fn render_path(path: &syn::Path) -> String {
266    let mut out = String::new();
267    if path.leading_colon.is_some() {
268        out.push_str("::");
269    }
270    for (i, seg) in path.segments.iter().enumerate() {
271        if i > 0 {
272            out.push_str("::");
273        }
274        out.push_str(&seg.ident.to_string());
275    }
276    out
277}
278
279/// `true` when `attrs` carries a `#[cfg(test)]` gate (including `cfg(all(test, …))`
280/// / `cfg(any(test, …))`) — the signal for an inline unit-test module.
281fn has_cfg_test(attrs: &[syn::Attribute]) -> bool {
282    attrs.iter().any(|attr| {
283        attr.path().is_ident("cfg")
284            && attr
285                .meta
286                .require_list()
287                .map(|list| cfg_mentions_test(list.tokens.clone()))
288                .unwrap_or(false)
289    })
290}
291
292/// `true` when a `cfg(...)` token stream contains a bare `test` ident (recursing
293/// into `all(...)` / `any(...)` groups). A `feature = "test"` string literal does
294/// not count.
295fn cfg_mentions_test(tokens: proc_macro2::TokenStream) -> bool {
296    tokens.into_iter().any(|tt| match tt {
297        proc_macro2::TokenTree::Ident(id) => id == "test",
298        proc_macro2::TokenTree::Group(group) => cfg_mentions_test(group.stream()),
299        _ => false,
300    })
301}
302
303/// The crate's normal `[dependencies]` names (hyphens normalized to underscores,
304/// the form used in paths) — the external crates whose calls are out-of-module.
305/// `[dev-dependencies]` are test tooling (`mockall`, `rstest`, …) and are
306/// deliberately excluded: a unit test uses its framework for real. Returns an
307/// empty set when there is no `Cargo.toml` at `root`.
308fn external_deps(root: &Path) -> Result<BTreeSet<String>> {
309    let manifest = root.join("Cargo.toml");
310    if !manifest.is_file() {
311        return Ok(BTreeSet::new());
312    }
313    let text = std::fs::read_to_string(&manifest)
314        .with_context(|| format!("reading `{}`", manifest.display()))?;
315    let value: toml::Value =
316        toml::from_str(&text).with_context(|| format!("parsing `{}`", manifest.display()))?;
317    let mut deps = BTreeSet::new();
318    if let Some(table) = value.get("dependencies").and_then(toml::Value::as_table) {
319        for name in table.keys() {
320            deps.insert(name.replace('-', "_"));
321        }
322    }
323    Ok(deps)
324}
325
326/// Recursively collect every `*.rs` file under `dir` into `out`.
327fn collect_rust_files(dir: &Path, out: &mut Vec<PathBuf>) -> Result<()> {
328    let entries =
329        std::fs::read_dir(dir).with_context(|| format!("reading directory `{}`", dir.display()))?;
330    for entry in entries {
331        let path = entry
332            .with_context(|| format!("reading an entry under `{}`", dir.display()))?
333            .path();
334        if path.is_dir() {
335            collect_rust_files(&path, out)?;
336        } else if path.extension().and_then(|ext| ext.to_str()) == Some("rs") {
337            out.push(path);
338        }
339    }
340    Ok(())
341}
342
343#[cfg(test)]
344mod tests {
345    use super::*;
346
347    /// Run the visitor over a source snippet with the given external-crate deps.
348    fn violations_in(src: &str, deps: &[&str]) -> Vec<Violation> {
349        let ast = syn::parse_file(src).expect("snippet parses");
350        let dep_set: BTreeSet<String> = deps.iter().map(|s| (*s).to_string()).collect();
351        let mut visitor = IsolationVisitor {
352            file: Path::new("snippet.rs"),
353            deps: &dep_set,
354            test_depth: 0,
355            violations: Vec::new(),
356        };
357        visitor.visit_file(&ast);
358        visitor.violations
359    }
360
361    #[test]
362    fn flags_each_out_of_module_form() {
363        let src = "\
364#[cfg(test)]
365mod tests {
366    use super::*;
367    #[test]
368    fn t() {
369        let _ = crate::store::load();
370        let _ = std::fs::read(\"x\");
371        let _ = rand::random::<u8>();
372        let _ = super::super::util::help();
373    }
374}
375";
376        let violations = violations_in(src, &["rand"]);
377        assert_eq!(violations.len(), 4, "got {violations:?}");
378        assert!(violations.iter().all(|v| v.rule == RULE_CALL));
379    }
380
381    #[test]
382    fn allows_in_module_calls() {
383        let src = "\
384#[cfg(test)]
385mod tests {
386    use super::*;
387    use std::io::Cursor;
388    #[test]
389    fn t() {
390        let _ = super::widget();
391        let _ = self::helper();
392        let _ = Cursor::new(b\"x\");
393        let _ = std::collections::HashMap::<u8, u8>::new();
394        assert_eq!(1, 1);
395    }
396}
397";
398        assert!(violations_in(src, &["rand"]).is_empty());
399    }
400
401    #[test]
402    fn ignores_calls_outside_test_modules() {
403        let src = "fn run() { let _ = crate::other::go(); }";
404        assert!(violations_in(src, &[]).is_empty());
405    }
406
407    #[test]
408    fn reports_the_call_line() {
409        // Line 1 is `#[cfg(test)]`; the flagged call sits on line 4.
410        let src = "\
411#[cfg(test)]
412mod tests {
413    fn t() {
414        let _ = crate::other::go();
415    }
416}
417";
418        let violations = violations_in(src, &[]);
419        assert_eq!(violations.len(), 1);
420        assert_eq!(violations[0].line, 4);
421    }
422
423    #[test]
424    fn effectful_std_policy() {
425        let segs = |p: &str| p.split("::").map(str::to_string).collect::<Vec<_>>();
426        // effectful — flagged
427        assert!(is_effectful_std(&segs("std::fs::read")));
428        assert!(is_effectful_std(&segs("std::net::TcpStream::connect")));
429        assert!(is_effectful_std(&segs("std::env::var")));
430        assert!(is_effectful_std(&segs("std::process::exit")));
431        assert!(is_effectful_std(&segs("std::thread::sleep")));
432        assert!(is_effectful_std(&segs("std::time::SystemTime::now")));
433        assert!(is_effectful_std(&segs("std::io::stdout")));
434        // pure — allowed
435        assert!(!is_effectful_std(&segs("std::collections::HashMap")));
436        assert!(!is_effectful_std(&segs("std::io::Cursor")));
437        assert!(!is_effectful_std(&segs("std::time::Duration")));
438        assert!(!is_effectful_std(&segs("std::cmp::min")));
439    }
440
441    #[test]
442    fn classify_leading_segment() {
443        let deps: BTreeSet<String> = ["rand"].iter().map(|s| s.to_string()).collect();
444        let path = |s: &str| syn::parse_str::<syn::Path>(s).expect("path parses");
445        assert_eq!(classify(&path("super::foo"), &deps), None);
446        assert_eq!(classify(&path("self::foo"), &deps), None);
447        assert_eq!(classify(&path("Local::new"), &deps), None);
448        assert_eq!(
449            classify(&path("super::super::foo"), &deps),
450            Some("ancestor module")
451        );
452        assert_eq!(
453            classify(&path("crate::a::b"), &deps),
454            Some("first-party module")
455        );
456        assert_eq!(
457            classify(&path("rand::random"), &deps),
458            Some("external crate")
459        );
460        assert_eq!(
461            classify(&path("std::fs::read"), &deps),
462            Some("effectful std")
463        );
464        assert_eq!(classify(&path("std::io::Cursor"), &deps), None);
465    }
466
467    #[test]
468    fn recognizes_cfg_test_attribute() {
469        let module = |s: &str| syn::parse_str::<syn::ItemMod>(s).expect("module parses");
470        assert!(has_cfg_test(&module("#[cfg(test)] mod t {}").attrs));
471        assert!(has_cfg_test(
472            &module("#[cfg(all(test, feature = \"x\"))] mod t {}").attrs
473        ));
474        assert!(!has_cfg_test(
475            &module("#[cfg(feature = \"test\")] mod t {}").attrs
476        ));
477        assert!(!has_cfg_test(&module("mod t {}").attrs));
478    }
479
480    #[test]
481    fn flags_each_foreign_import() {
482        let src = "\
483#[cfg(test)]
484mod tests {
485    use super::*;
486    use super::Thing;
487    use crate::other::*;
488    use crate::other::Named;
489    use rand::Rng;
490    use std::fs;
491    use std::collections::HashMap;
492    use std::io::Cursor;
493}
494";
495        // Flagged: the crate glob, the crate named import, the external crate, and
496        // effectful `std::fs` — not `super::*` / `super::Thing` / pure std.
497        let violations = violations_in(src, &["rand"]);
498        assert_eq!(violations.len(), 4, "got {violations:?}");
499        assert!(violations.iter().all(|v| v.rule == RULE_IMPORT));
500    }
501
502    #[test]
503    fn classify_use_roots() {
504        let deps: BTreeSet<String> = ["rand"].iter().map(|s| s.to_string()).collect();
505        let segs = |p: &str| p.split("::").map(str::to_string).collect::<Vec<_>>();
506        // in-module (None)
507        assert_eq!(classify_use(&segs("super"), true, &deps), None); // `use super::*`
508        assert_eq!(classify_use(&segs("super::Thing"), false, &deps), None);
509        assert_eq!(classify_use(&segs("self::helper"), false, &deps), None);
510        assert_eq!(
511            classify_use(&segs("std::collections::HashMap"), false, &deps),
512            None
513        );
514        assert_eq!(classify_use(&segs("std::io::Cursor"), false, &deps), None);
515        // out-of-module
516        assert_eq!(
517            classify_use(&segs("super::super"), true, &deps),
518            Some("ancestor module")
519        );
520        assert_eq!(
521            classify_use(&segs("crate::other"), true, &deps),
522            Some("first-party module")
523        );
524        assert_eq!(
525            classify_use(&segs("crate::other::Named"), false, &deps),
526            Some("first-party module")
527        );
528        assert_eq!(
529            classify_use(&segs("rand::Rng"), false, &deps),
530            Some("external crate")
531        );
532        assert_eq!(
533            classify_use(&segs("std::fs"), false, &deps),
534            Some("effectful std")
535        );
536        // a non-`super` glob is foreign even for pure std
537        assert_eq!(
538            classify_use(&segs("std::collections"), true, &deps),
539            Some("glob import")
540        );
541    }
542
543    #[test]
544    fn imports_outside_test_modules_are_ignored() {
545        let src = "use crate::other::*; fn run() {}";
546        assert!(violations_in(src, &[]).is_empty());
547    }
548}