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 detector:
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
21use std::collections::BTreeSet;
22use std::path::{Path, PathBuf};
23
24use anyhow::{anyhow, Context, Result};
25use syn::spanned::Spanned;
26use syn::visit::{self, Visit};
27
28pub use crate::violation::Violation;
29
30/// Rule id reported for an out-of-module call.
31const RULE: &str = "no-out-of-module-call";
32
33/// A language whose unit-isolation convention can be checked (Python #42 is a
34/// separate detector). Each detector lives in its own module; this enum is the
35/// shared `unit isolation` language selector.
36#[derive(Debug, Clone, Copy, PartialEq, Eq, clap::ValueEnum)]
37pub enum Language {
38    /// Inline `#[cfg(test)]` modules in `*.rs` files (`no-out-of-module-call`).
39    #[value(name = "rust")]
40    Rust,
41    /// `*.test.{ts,tsx,mts,cts}` unit tests (`unmocked-collaborator`, #43 / #76);
42    /// the detector lives in [`crate::ts`].
43    #[value(name = "typescript")]
44    TypeScript,
45}
46
47/// Scan the Rust source files under `root` and return every isolation violation,
48/// sorted by `(file, line)` for deterministic output.
49///
50/// `root` is the crate root: its `Cargo.toml` names the external crates whose
51/// calls are out-of-module. Every `*.rs` file under it is parsed; a file that
52/// cannot be read or parsed is an error.
53pub fn find_violations(root: impl AsRef<Path>) -> Result<Vec<Violation>> {
54    let root = root.as_ref();
55    let deps = external_deps(root)?;
56
57    let mut files = Vec::new();
58    collect_rust_files(root, &mut files)?;
59    files.sort();
60
61    let mut violations = Vec::new();
62    for file in &files {
63        let source = std::fs::read_to_string(file)
64            .with_context(|| format!("reading source file `{}`", file.display()))?;
65        let ast = syn::parse_file(&source)
66            .map_err(|err| anyhow!("parsing `{}`: {err}", file.display()))?;
67        let mut visitor = IsolationVisitor {
68            file,
69            deps: &deps,
70            test_depth: 0,
71            violations: Vec::new(),
72        };
73        visitor.visit_file(&ast);
74        violations.append(&mut visitor.violations);
75    }
76
77    violations.sort_by(|a, b| a.file.cmp(&b.file).then(a.line.cmp(&b.line)));
78    Ok(violations)
79}
80
81/// Walks one parsed file, flagging out-of-module calls inside `#[cfg(test)]`
82/// modules. `test_depth` counts how deep we are inside such modules, so a call in
83/// non-test code is ignored.
84struct IsolationVisitor<'a> {
85    file: &'a Path,
86    deps: &'a BTreeSet<String>,
87    test_depth: usize,
88    violations: Vec<Violation>,
89}
90
91impl<'ast> Visit<'ast> for IsolationVisitor<'_> {
92    fn visit_item_mod(&mut self, node: &'ast syn::ItemMod) {
93        let is_test = has_cfg_test(&node.attrs);
94        if is_test {
95            self.test_depth += 1;
96        }
97        visit::visit_item_mod(self, node);
98        if is_test {
99            self.test_depth -= 1;
100        }
101    }
102
103    fn visit_expr_call(&mut self, node: &'ast syn::ExprCall) {
104        if self.test_depth > 0 {
105            if let syn::Expr::Path(path_expr) = node.func.as_ref() {
106                if let Some(kind) = classify(&path_expr.path, self.deps) {
107                    self.violations.push(Violation {
108                        file: self.file.to_path_buf(),
109                        line: node.span().start().line,
110                        rule: RULE,
111                        message: format!(
112                            "unit test calls `{}` out of its own module ({kind}); \
113                             inject a trait double — only `super::` is in-module",
114                            render_path(&path_expr.path),
115                        ),
116                    });
117                }
118            }
119        }
120        visit::visit_expr_call(self, node);
121    }
122}
123
124/// Why a call's leading path is out-of-module, or `None` when the call stays
125/// in-module (or is unresolvable, and so deliberately not flagged — the `syn`
126/// heuristic's documented limit).
127fn classify(path: &syn::Path, deps: &BTreeSet<String>) -> Option<&'static str> {
128    let segs: Vec<String> = path.segments.iter().map(|s| s.ident.to_string()).collect();
129    match segs.first().map(String::as_str)? {
130        // `self` / `Self` are local; a single `super::` is the unit under test.
131        "self" | "Self" => None,
132        "super" => (segs.get(1).map(String::as_str) == Some("super")).then_some("ancestor module"),
133        "crate" => Some("first-party module"),
134        "std" => is_effectful_std(&segs).then_some("effectful std"),
135        // `core`/`alloc` carry no effectful APIs.
136        "core" | "alloc" => None,
137        // Any other leading segment is in-module unless it names an external
138        // crate; a local type/fn (incl. `super::*`-imported) is not flagged.
139        other => deps.contains(other).then_some("external crate"),
140    }
141}
142
143/// `true` for an effectful `std` path — filesystem, network, process, env,
144/// threads, OS, the clock (`SystemTime::now` / `Instant::now`), or real-handle
145/// I/O (`stdin`/`stdout`/`stderr`). Pure std is allowed: `std::io::Cursor` and the
146/// I/O traits, `time::Duration`, `collections`, `fmt`, … — `internals/rust/`
147/// `testing.md` makes `Cursor` the idiomatic in-memory unit-test tool.
148fn is_effectful_std(segs: &[String]) -> bool {
149    match segs.get(1).map(String::as_str) {
150        Some("fs" | "net" | "process" | "env" | "thread" | "os") => true,
151        Some("io") => matches!(
152            segs.get(2).map(String::as_str),
153            Some("stdin" | "stdout" | "stderr")
154        ),
155        Some("time") => {
156            matches!(
157                segs.get(2).map(String::as_str),
158                Some("SystemTime" | "Instant")
159            ) && segs.get(3).map(String::as_str) == Some("now")
160        }
161        _ => false,
162    }
163}
164
165/// Render a path back to `a::b::c` for the message (idents only; generic args
166/// dropped).
167fn render_path(path: &syn::Path) -> String {
168    let mut out = String::new();
169    if path.leading_colon.is_some() {
170        out.push_str("::");
171    }
172    for (i, seg) in path.segments.iter().enumerate() {
173        if i > 0 {
174            out.push_str("::");
175        }
176        out.push_str(&seg.ident.to_string());
177    }
178    out
179}
180
181/// `true` when `attrs` carries a `#[cfg(test)]` gate (including `cfg(all(test, …))`
182/// / `cfg(any(test, …))`) — the signal for an inline unit-test module.
183fn has_cfg_test(attrs: &[syn::Attribute]) -> bool {
184    attrs.iter().any(|attr| {
185        attr.path().is_ident("cfg")
186            && attr
187                .meta
188                .require_list()
189                .map(|list| cfg_mentions_test(list.tokens.clone()))
190                .unwrap_or(false)
191    })
192}
193
194/// `true` when a `cfg(...)` token stream contains a bare `test` ident (recursing
195/// into `all(...)` / `any(...)` groups). A `feature = "test"` string literal does
196/// not count.
197fn cfg_mentions_test(tokens: proc_macro2::TokenStream) -> bool {
198    tokens.into_iter().any(|tt| match tt {
199        proc_macro2::TokenTree::Ident(id) => id == "test",
200        proc_macro2::TokenTree::Group(group) => cfg_mentions_test(group.stream()),
201        _ => false,
202    })
203}
204
205/// The crate's normal `[dependencies]` names (hyphens normalized to underscores,
206/// the form used in paths) — the external crates whose calls are out-of-module.
207/// `[dev-dependencies]` are test tooling (`mockall`, `rstest`, …) and are
208/// deliberately excluded: a unit test uses its framework for real. Returns an
209/// empty set when there is no `Cargo.toml` at `root`.
210fn external_deps(root: &Path) -> Result<BTreeSet<String>> {
211    let manifest = root.join("Cargo.toml");
212    if !manifest.is_file() {
213        return Ok(BTreeSet::new());
214    }
215    let text = std::fs::read_to_string(&manifest)
216        .with_context(|| format!("reading `{}`", manifest.display()))?;
217    let value: toml::Value =
218        toml::from_str(&text).with_context(|| format!("parsing `{}`", manifest.display()))?;
219    let mut deps = BTreeSet::new();
220    if let Some(table) = value.get("dependencies").and_then(toml::Value::as_table) {
221        for name in table.keys() {
222            deps.insert(name.replace('-', "_"));
223        }
224    }
225    Ok(deps)
226}
227
228/// Recursively collect every `*.rs` file under `dir` into `out`.
229fn collect_rust_files(dir: &Path, out: &mut Vec<PathBuf>) -> Result<()> {
230    let entries =
231        std::fs::read_dir(dir).with_context(|| format!("reading directory `{}`", dir.display()))?;
232    for entry in entries {
233        let path = entry
234            .with_context(|| format!("reading an entry under `{}`", dir.display()))?
235            .path();
236        if path.is_dir() {
237            collect_rust_files(&path, out)?;
238        } else if path.extension().and_then(|ext| ext.to_str()) == Some("rs") {
239            out.push(path);
240        }
241    }
242    Ok(())
243}
244
245#[cfg(test)]
246mod tests {
247    use super::*;
248
249    /// Run the visitor over a source snippet with the given external-crate deps.
250    fn violations_in(src: &str, deps: &[&str]) -> Vec<Violation> {
251        let ast = syn::parse_file(src).expect("snippet parses");
252        let dep_set: BTreeSet<String> = deps.iter().map(|s| (*s).to_string()).collect();
253        let mut visitor = IsolationVisitor {
254            file: Path::new("snippet.rs"),
255            deps: &dep_set,
256            test_depth: 0,
257            violations: Vec::new(),
258        };
259        visitor.visit_file(&ast);
260        visitor.violations
261    }
262
263    #[test]
264    fn flags_each_out_of_module_form() {
265        let src = "\
266#[cfg(test)]
267mod tests {
268    use super::*;
269    #[test]
270    fn t() {
271        let _ = crate::store::load();
272        let _ = std::fs::read(\"x\");
273        let _ = rand::random::<u8>();
274        let _ = super::super::util::help();
275    }
276}
277";
278        let violations = violations_in(src, &["rand"]);
279        assert_eq!(violations.len(), 4, "got {violations:?}");
280        assert!(violations.iter().all(|v| v.rule == RULE));
281    }
282
283    #[test]
284    fn allows_in_module_calls() {
285        let src = "\
286#[cfg(test)]
287mod tests {
288    use super::*;
289    use std::io::Cursor;
290    #[test]
291    fn t() {
292        let _ = super::widget();
293        let _ = self::helper();
294        let _ = Cursor::new(b\"x\");
295        let _ = std::collections::HashMap::<u8, u8>::new();
296        assert_eq!(1, 1);
297    }
298}
299";
300        assert!(violations_in(src, &["rand"]).is_empty());
301    }
302
303    #[test]
304    fn ignores_calls_outside_test_modules() {
305        let src = "fn run() { let _ = crate::other::go(); }";
306        assert!(violations_in(src, &[]).is_empty());
307    }
308
309    #[test]
310    fn reports_the_call_line() {
311        // Line 1 is `#[cfg(test)]`; the flagged call sits on line 4.
312        let src = "\
313#[cfg(test)]
314mod tests {
315    fn t() {
316        let _ = crate::other::go();
317    }
318}
319";
320        let violations = violations_in(src, &[]);
321        assert_eq!(violations.len(), 1);
322        assert_eq!(violations[0].line, 4);
323    }
324
325    #[test]
326    fn effectful_std_policy() {
327        let segs = |p: &str| p.split("::").map(str::to_string).collect::<Vec<_>>();
328        // effectful — flagged
329        assert!(is_effectful_std(&segs("std::fs::read")));
330        assert!(is_effectful_std(&segs("std::net::TcpStream::connect")));
331        assert!(is_effectful_std(&segs("std::env::var")));
332        assert!(is_effectful_std(&segs("std::process::exit")));
333        assert!(is_effectful_std(&segs("std::thread::sleep")));
334        assert!(is_effectful_std(&segs("std::time::SystemTime::now")));
335        assert!(is_effectful_std(&segs("std::io::stdout")));
336        // pure — allowed
337        assert!(!is_effectful_std(&segs("std::collections::HashMap")));
338        assert!(!is_effectful_std(&segs("std::io::Cursor")));
339        assert!(!is_effectful_std(&segs("std::time::Duration")));
340        assert!(!is_effectful_std(&segs("std::cmp::min")));
341    }
342
343    #[test]
344    fn classify_leading_segment() {
345        let deps: BTreeSet<String> = ["rand"].iter().map(|s| s.to_string()).collect();
346        let path = |s: &str| syn::parse_str::<syn::Path>(s).expect("path parses");
347        assert_eq!(classify(&path("super::foo"), &deps), None);
348        assert_eq!(classify(&path("self::foo"), &deps), None);
349        assert_eq!(classify(&path("Local::new"), &deps), None);
350        assert_eq!(
351            classify(&path("super::super::foo"), &deps),
352            Some("ancestor module")
353        );
354        assert_eq!(
355            classify(&path("crate::a::b"), &deps),
356            Some("first-party module")
357        );
358        assert_eq!(
359            classify(&path("rand::random"), &deps),
360            Some("external crate")
361        );
362        assert_eq!(
363            classify(&path("std::fs::read"), &deps),
364            Some("effectful std")
365        );
366        assert_eq!(classify(&path("std::io::Cursor"), &deps), None);
367    }
368
369    #[test]
370    fn recognizes_cfg_test_attribute() {
371        let module = |s: &str| syn::parse_str::<syn::ItemMod>(s).expect("module parses");
372        assert!(has_cfg_test(&module("#[cfg(test)] mod t {}").attrs));
373        assert!(has_cfg_test(
374            &module("#[cfg(all(test, feature = \"x\"))] mod t {}").attrs
375        ));
376        assert!(!has_cfg_test(
377            &module("#[cfg(feature = \"test\")] mod t {}").attrs
378        ));
379        assert!(!has_cfg_test(&module("mod t {}").attrs));
380    }
381}
testing_conventions/isolation.rs

testing_conventions/
isolation.rs
