testing_conventions/
lint.rs

1//! Integration-test lints (issue #19; rules #48–#52) — the `integration lint`
2//! command.
3//!
4//! A *lint* here is a deterministic style/mechanism check on test code, as
5//! opposed to the structural `colocated-test` / `coverage` rules. This module hosts
6//! the mocking mechanism & style lints; more lints will join them under the
7//! same command.
8//!
9//! Detection is AST-based: each Python test file is parsed with
10//! `rustpython_parser` and the tree is walked with a [`Visitor`].
11//!
12//! Implemented lints:
13//! - **`no-monkeypatch`** (#49): a test/fixture function that declares the
14//!   `monkeypatch` parameter (pytest's fixture). Patch with `unittest.mock`
15//!   wrapped in a `pytest.fixture` instead.
16//! - **`no-inline-patch`** (#50): a `patch(...)` / `patch.object(...)` /
17//!   `patch.dict(...)` call inside a test body — the `with patch(...)` form or a
18//!   bare call. Patches belong in a `pytest.fixture`; a patch *inside* a fixture
19//!   is allowed.
20//! - **`no-environ-mutation`** (#51): direct mutation of `os.environ` —
21//!   `os.environ[...] = …`, `del os.environ[...]`, or a mutating method
22//!   (`update` / `pop` / `setdefault` / `clear` / `popitem`). Set env via
23//!   `patch.dict(os.environ, {...})` instead.
24//! - **`no-constant-patch`** (#52): patching a module-global UPPER_CASE constant,
25//!   e.g. `patch("pkg.config.CACHE_DIR", …)`. Inject config explicitly. Waivable
26//!   per file via the config `exempt` list.
27
28use std::path::{Path, PathBuf};
29
30use anyhow::{anyhow, Context, Result};
31use rustpython_ast::Visitor;
32use rustpython_parser::ast::{
33    self, Arg, Arguments, Constant, Expr, ExprCall, StmtAssign, StmtAsyncFunctionDef,
34    StmtAugAssign, StmtDelete, StmtFunctionDef, WithItem,
35};
36use rustpython_parser::text_size::{TextRange, TextSize};
37use rustpython_parser::Parse;
38
39/// A single lint violation found in a test file.
40#[derive(Debug, Clone, PartialEq, Eq)]
41pub struct Violation {
42    /// File the violation was found in.
43    pub file: PathBuf,
44    /// 1-based line number of the offending construct.
45    pub line: usize,
46    /// Short lint identifier (e.g. `no-monkeypatch`).
47    pub rule: &'static str,
48    /// Human-readable explanation.
49    pub message: String,
50}
51
52/// Scan the Python test files under `root` and return every lint violation,
53/// sorted by `(file, line)` for deterministic output.
54///
55/// A *Python test file* is `*_test.py`, the legacy `test_*.py`, or
56/// `conftest.py` (where fixtures live). Each is parsed and walked. A file that
57/// cannot be read or parsed is an error.
58pub fn find_violations(root: impl AsRef<Path>) -> Result<Vec<Violation>> {
59    let root = root.as_ref();
60    let mut files = Vec::new();
61    collect_python_test_files(root, &mut files)?;
62    files.sort();
63
64    let mut violations = Vec::new();
65    for file in &files {
66        let source = std::fs::read_to_string(file)
67            .with_context(|| format!("reading test file `{}`", file.display()))?;
68        let suite = ast::Suite::parse(&source, &file.to_string_lossy())
69            .map_err(|err| anyhow!("parsing `{}`: {err}", file.display()))?;
70        let mut visitor = LintVisitor {
71            file,
72            source: &source,
73            fixture_depth: 0,
74            violations: Vec::new(),
75        };
76        for stmt in suite {
77            visitor.visit_stmt(stmt);
78        }
79        violations.append(&mut visitor.violations);
80    }
81
82    violations.sort_by(|a, b| a.file.cmp(&b.file).then(a.line.cmp(&b.line)));
83    Ok(violations)
84}
85
86/// Walks one parsed test file, collecting lint violations. Tracks how deep we
87/// are inside `@pytest.fixture` functions so `no-inline-patch` can allow patches
88/// there while flagging them in test bodies.
89struct LintVisitor<'a> {
90    file: &'a Path,
91    source: &'a str,
92    fixture_depth: usize,
93    violations: Vec<Violation>,
94}
95
96impl LintVisitor<'_> {
97    fn report(&mut self, range: TextRange, rule: &'static str, message: &str) {
98        self.violations.push(Violation {
99            file: self.file.to_path_buf(),
100            line: line_of(self.source, range.start()),
101            rule,
102            message: message.to_string(),
103        });
104    }
105
106    /// Shared entry for both function kinds: run the parameter lint, then return
107    /// whether this function is a fixture (so the caller bumps `fixture_depth`).
108    fn enter_function(&mut self, args: &Arguments, decorators: &[Expr], range: TextRange) -> bool {
109        // `no-monkeypatch` (#49): the `monkeypatch` parameter is the signal.
110        let takes_monkeypatch = args
111            .posonlyargs
112            .iter()
113            .chain(&args.args)
114            .chain(&args.kwonlyargs)
115            .any(|arg| arg.def.arg.as_str() == "monkeypatch")
116            || arg_named(&args.vararg, "monkeypatch")
117            || arg_named(&args.kwarg, "monkeypatch");
118        if takes_monkeypatch {
119            self.report(
120                range,
121                "no-monkeypatch",
122                "test takes pytest's `monkeypatch` fixture; patch with `unittest.mock` wrapped in a `pytest.fixture` instead",
123            );
124        }
125
126        decorators.iter().any(is_fixture_decorator)
127    }
128}
129
130impl Visitor for LintVisitor<'_> {
131    fn visit_stmt_function_def(&mut self, node: StmtFunctionDef) {
132        let is_fixture = self.enter_function(&node.args, &node.decorator_list, node.range);
133        if is_fixture {
134            self.fixture_depth += 1;
135        }
136        self.generic_visit_stmt_function_def(node);
137        if is_fixture {
138            self.fixture_depth -= 1;
139        }
140    }
141
142    fn visit_stmt_async_function_def(&mut self, node: StmtAsyncFunctionDef) {
143        let is_fixture = self.enter_function(&node.args, &node.decorator_list, node.range);
144        if is_fixture {
145            self.fixture_depth += 1;
146        }
147        self.generic_visit_stmt_async_function_def(node);
148        if is_fixture {
149            self.fixture_depth -= 1;
150        }
151    }
152
153    fn visit_expr_call(&mut self, node: ExprCall) {
154        let is_patch = is_patch_call(&node);
155        // `no-inline-patch` (#50): a patch(...) call outside any fixture is a
156        // patch in a test body. Inside a fixture it is the right place.
157        if is_patch && self.fixture_depth == 0 {
158            self.report(
159                node.range,
160                "no-inline-patch",
161                "patch is called inline in a test body; move it into a `pytest.fixture`",
162            );
163        }
164        // `no-constant-patch` (#52): patching a module-global UPPER_CASE constant.
165        // Fires regardless of fixture — config constants are usually patched in one.
166        if is_patch && patches_constant(&node) {
167            self.report(node.range, "no-constant-patch", CONSTANT_PATCH_MSG);
168        }
169        // `no-environ-mutation` (#51): `os.environ.update(...)` and friends.
170        if is_environ_mutation_call(&node) {
171            self.report(node.range, "no-environ-mutation", ENVIRON_MUTATION_MSG);
172        }
173        self.generic_visit_expr_call(node);
174    }
175
176    // The generated `generic_visit_withitem` is a no-op, so a `with patch(...)`
177    // context expression is never walked unless we descend into it here.
178    fn visit_withitem(&mut self, node: WithItem) {
179        self.visit_expr(node.context_expr);
180        if let Some(optional_vars) = node.optional_vars {
181            self.visit_expr(*optional_vars);
182        }
183    }
184
185    // `no-environ-mutation` (#51): `os.environ[...] = …`, augmented assignment,
186    // and `del os.environ[...]`.
187    fn visit_stmt_assign(&mut self, node: StmtAssign) {
188        if node.targets.iter().any(is_os_environ_subscript) {
189            self.report(node.range, "no-environ-mutation", ENVIRON_MUTATION_MSG);
190        }
191        self.generic_visit_stmt_assign(node);
192    }
193
194    fn visit_stmt_aug_assign(&mut self, node: StmtAugAssign) {
195        if is_os_environ_subscript(node.target.as_ref()) {
196            self.report(node.range, "no-environ-mutation", ENVIRON_MUTATION_MSG);
197        }
198        self.generic_visit_stmt_aug_assign(node);
199    }
200
201    fn visit_stmt_delete(&mut self, node: StmtDelete) {
202        if node.targets.iter().any(is_os_environ_subscript) {
203            self.report(node.range, "no-environ-mutation", ENVIRON_MUTATION_MSG);
204        }
205        self.generic_visit_stmt_delete(node);
206    }
207}
208
209/// `true` when a `*args` / `**kwargs` arg is named `name`.
210fn arg_named(arg: &Option<Box<Arg>>, name: &str) -> bool {
211    arg.as_ref().is_some_and(|arg| arg.arg.as_str() == name)
212}
213
214/// `true` for an `@pytest.fixture` / `@fixture` decorator, with or without a
215/// call (`@pytest.fixture(autouse=True)`).
216fn is_fixture_decorator(decorator: &Expr) -> bool {
217    let target = match decorator {
218        Expr::Call(call) => call.func.as_ref(),
219        other => other,
220    };
221    match target {
222        Expr::Name(name) => name.id.as_str() == "fixture",
223        Expr::Attribute(attr) => attr.attr.as_str() == "fixture",
224        _ => false,
225    }
226}
227
228/// `true` when a call is `patch(...)`, `patch.object(...)`, `patch.dict(...)`, or
229/// the same reached through a module (`mock.patch(...)`, `unittest.mock.patch`).
230fn is_patch_call(call: &ExprCall) -> bool {
231    match call.func.as_ref() {
232        Expr::Name(name) => name.id.as_str() == "patch",
233        Expr::Attribute(attr) => {
234            let name = attr.attr.as_str();
235            name == "patch"
236                || ((name == "object" || name == "dict") && attr_base_is_patch(attr.value.as_ref()))
237        }
238        _ => false,
239    }
240}
241
242/// `true` when an attribute's base resolves to `patch` — the receiver of
243/// `patch.object` / `patch.dict`.
244fn attr_base_is_patch(expr: &Expr) -> bool {
245    match expr {
246        Expr::Name(name) => name.id.as_str() == "patch",
247        Expr::Attribute(attr) => attr.attr.as_str() == "patch",
248        _ => false,
249    }
250}
251
252/// Message for the `no-constant-patch` lint.
253const CONSTANT_PATCH_MSG: &str = "patches a module-global config constant; inject config explicitly (a consumer that did `from pkg import CONSTANT` snapshots the value at import time and ignores the patch)";
254
255/// `true` when a `patch(...)` call's first string argument names a module-global
256/// UPPER_CASE constant, e.g. `patch("pkg.config.CACHE_DIR", …)`.
257fn patches_constant(call: &ExprCall) -> bool {
258    let Some(Expr::Constant(constant)) = call.args.first() else {
259        return false;
260    };
261    let Constant::Str(target) = &constant.value else {
262        return false;
263    };
264    target.rsplit('.').next().is_some_and(is_upper_constant)
265}
266
267/// `true` for an ALL-CAPS constant name — letters uppercase, digits and
268/// underscores allowed, at least one letter (`CACHE_DIR`, `DEBUG`, `MAX_SIZE`).
269fn is_upper_constant(name: &str) -> bool {
270    !name.is_empty()
271        && name
272            .chars()
273            .all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')
274        && name.chars().any(|c| c.is_ascii_uppercase())
275}
276
277/// Message for the `no-environ-mutation` lint.
278const ENVIRON_MUTATION_MSG: &str =
279    "os.environ is mutated directly; set env via `patch.dict(os.environ, {...})` instead";
280
281/// `true` for the expression `os.environ`.
282fn is_os_environ(expr: &Expr) -> bool {
283    matches!(
284        expr,
285        Expr::Attribute(attr)
286            if attr.attr.as_str() == "environ"
287                && matches!(attr.value.as_ref(), Expr::Name(name) if name.id.as_str() == "os")
288    )
289}
290
291/// `true` for `os.environ[...]` — a subscript of `os.environ`, the form used as
292/// an assignment or `del` target.
293fn is_os_environ_subscript(expr: &Expr) -> bool {
294    matches!(expr, Expr::Subscript(sub) if is_os_environ(sub.value.as_ref()))
295}
296
297/// `true` for a mutating method call on `os.environ` (`os.environ.update(...)`
298/// and friends).
299fn is_environ_mutation_call(call: &ExprCall) -> bool {
300    matches!(
301        call.func.as_ref(),
302        Expr::Attribute(attr)
303            if is_os_environ(attr.value.as_ref()) && is_environ_mutator(attr.attr.as_str())
304    )
305}
306
307/// `true` for a `dict` method that mutates in place.
308fn is_environ_mutator(method: &str) -> bool {
309    matches!(
310        method,
311        "update" | "pop" | "setdefault" | "clear" | "popitem"
312    )
313}
314
315/// The 1-based line containing byte `offset` in `source`.
316fn line_of(source: &str, offset: TextSize) -> usize {
317    let offset = (u32::from(offset) as usize).min(source.len());
318    source.as_bytes()[..offset]
319        .iter()
320        .filter(|&&byte| byte == b'\n')
321        .count()
322        + 1
323}
324
325/// Recursively collect every Python test file under `dir` into `out`.
326fn collect_python_test_files(dir: &Path, out: &mut Vec<PathBuf>) -> Result<()> {
327    let entries =
328        std::fs::read_dir(dir).with_context(|| format!("reading directory `{}`", dir.display()))?;
329    for entry in entries {
330        let path = entry
331            .with_context(|| format!("reading an entry under `{}`", dir.display()))?
332            .path();
333        if path.is_dir() {
334            collect_python_test_files(&path, out)?;
335        } else if is_python_test_file(&path) {
336            out.push(path);
337        }
338    }
339    Ok(())
340}
341
342/// `true` for a file the lints scan: `*_test.py`, legacy `test_*.py`, or
343/// `conftest.py`.
344fn is_python_test_file(path: &Path) -> bool {
345    let name = path
346        .file_name()
347        .and_then(|n| n.to_str())
348        .unwrap_or_default();
349    name == "conftest.py"
350        || name.ends_with("_test.py")
351        || (name.starts_with("test_") && name.ends_with(".py"))
352}
353
354#[cfg(test)]
355mod tests {
356    use super::*;
357
358    #[test]
359    fn recognizes_python_test_files() {
360        assert!(is_python_test_file(Path::new("widget_test.py")));
361        assert!(is_python_test_file(Path::new("pkg/widget_test.py")));
362        assert!(is_python_test_file(Path::new("test_widget.py")));
363        assert!(is_python_test_file(Path::new("conftest.py")));
364    }
365
366    #[test]
367    fn ignores_non_test_files() {
368        assert!(!is_python_test_file(Path::new("widget.py")));
369        assert!(!is_python_test_file(Path::new("conftest.pyi")));
370        assert!(!is_python_test_file(Path::new("README.md")));
371        assert!(!is_python_test_file(Path::new("testing.py")));
372    }
373
374    #[test]
375    fn line_of_counts_newlines() {
376        let src = "a\nb\nc\n";
377        assert_eq!(line_of(src, TextSize::from(0)), 1);
378        assert_eq!(line_of(src, TextSize::from(2)), 2);
379        assert_eq!(line_of(src, TextSize::from(4)), 3);
380    }
381
382    #[test]
383    fn recognizes_environ_mutators() {
384        assert!(is_environ_mutator("update"));
385        assert!(is_environ_mutator("pop"));
386        assert!(is_environ_mutator("clear"));
387        assert!(!is_environ_mutator("get"));
388        assert!(!is_environ_mutator("keys"));
389    }
390
391    #[test]
392    fn recognizes_upper_constants() {
393        assert!(is_upper_constant("CACHE_DIR"));
394        assert!(is_upper_constant("DEBUG"));
395        assert!(is_upper_constant("MAX_2"));
396        assert!(!is_upper_constant("cache_dir"));
397        assert!(!is_upper_constant("CacheDir"));
398        assert!(!is_upper_constant("fetch"));
399        assert!(!is_upper_constant(""));
400        assert!(!is_upper_constant("_"));
401        assert!(!is_upper_constant("123"));
402    }
403}
testing_conventions/lint.rs

testing_conventions/
lint.rs
