Skip to main content

pedant_core/
checks.rs

1use std::sync::Arc;
2
3use crate::violation::CheckRationale;
4
5const NESTED_CONDITIONAL_PROBLEM: &str = "Conditional-in-conditional creates combinatorial complexity. A 2-branch if inside a 3-branch match is 6 paths. Hard to ensure all paths are tested.";
6const NESTED_CONDITIONAL_FIX: &str = "Use tuple patterns `match (a, b) { ... }`, match guards `Some(x) if x > 0 => ...`, or extract to functions.";
7const NESTED_CONDITIONAL_EXCEPTION: &str = "None. Refactoring is always possible.";
8
9/// Structured detail carried by an `item-visibility-policy` finding, surfaced
10/// as `subject`/`expected`/`observed` in JSON output.
11#[derive(Debug, Clone, PartialEq, Eq)]
12pub struct VisibilityDetail {
13    /// The configured item name.
14    pub subject: Arc<str>,
15    /// The visibility the policy requires.
16    pub expected: Arc<str>,
17    /// The observed visibility, or `missing`/`duplicate`/`wrong-kind`.
18    pub observed: Arc<str>,
19}
20
21/// Catalog entry for a single check, displayed by `--list-checks` and `--explain`.
22#[derive(Debug, Clone, Copy)]
23pub struct CheckInfo {
24    /// Kebab-case identifier (e.g., `"max-depth"`).
25    pub code: &'static str,
26    /// One-line summary for the checks table.
27    pub description: &'static str,
28    /// Grouping key (e.g., `"nesting"`, `"dispatch"`, `"structure"`).
29    pub category: &'static str,
30    /// `true` when the pattern is disproportionately common in LLM output.
31    pub llm_specific: bool,
32}
33
34/// Defines all check metadata in one place and generates:
35/// - `ViolationType` enum (unit and data-carrying variants)
36/// - `ViolationType::code()` returning the short code string
37/// - `ViolationType::category()` returning the category
38/// - `ViolationType::rationale()` returning `CheckRationale`
39/// - `lookup_rationale()` free function
40/// - `ALL_CHECKS` constant array of `CheckInfo`
41macro_rules! define_checks {
42    // Entry point: collect all check declarations, then emit everything.
43    (
44        $(
45            $variant:ident $({ $field:ident : $ftype:ty })? => {
46                code: $code:expr,
47                description: $desc:literal,
48                category: $cat:expr,
49                problem: $problem:expr,
50                fix: $fix:expr,
51                exception: $exception:expr,
52                llm_specific: $llm:expr $(,)?
53            }
54        ),+ $(,)?
55    ) => {
56        /// The kind of violation detected.
57        #[derive(Debug, Clone, PartialEq, Eq)]
58        pub enum ViolationType {
59            $(
60                #[doc = $desc]
61                $variant $({
62                    #[doc = "The matched pattern."]
63                    $field: $ftype
64                })?,
65            )+
66        }
67
68        impl ViolationType {
69            /// Returns the short code string used in output (e.g., `"max-depth"`).
70            pub fn code(&self) -> &'static str {
71                match self {
72                    $(
73                        Self::$variant $({ $field: _ })? => $code,
74                    )+
75                }
76            }
77
78            /// Returns the check category name (e.g., `"nesting"`, `"dispatch"`).
79            pub fn category(&self) -> &'static str {
80                match self {
81                    $(
82                        Self::$variant $({ $field: _ })? => $cat,
83                    )+
84                }
85            }
86
87            /// Returns the detailed rationale explaining why this check exists.
88            pub fn rationale(&self) -> CheckRationale {
89                match self {
90                    $(
91                        Self::$variant $({ $field: _ })? => CheckRationale {
92                            problem: $problem,
93                            fix: $fix,
94                            exception: $exception,
95                            llm_specific: $llm,
96                        },
97                    )+
98                }
99            }
100        }
101
102        /// Look up a ViolationType by its code string for rationale display.
103        pub fn lookup_rationale(code: &str) -> Option<CheckRationale> {
104            match code {
105                $(
106                    $code => Some(CheckRationale {
107                        problem: $problem,
108                        fix: $fix,
109                        exception: $exception,
110                        llm_specific: $llm,
111                    }),
112                )+
113                _ => None,
114            }
115        }
116
117        /// All available checks.
118        pub const ALL_CHECKS: &[CheckInfo] = &[
119            $(
120                CheckInfo {
121                    code: $code,
122                    description: $desc,
123                    category: $cat,
124                    llm_specific: $llm,
125                },
126            )+
127        ];
128    };
129}
130
131define_checks! {
132    MaxDepth => {
133        code: "max-depth",
134        description: "Excessive nesting depth",
135        category: "nesting",
136        problem: "Deeply nested code is hard to read, test, and modify. Each nesting level adds cognitive load. Bugs hide in deep branches.",
137        fix: "Extract functions, use early returns, flatten with guard clauses.",
138        exception: "Complex parsers or state machines may need deeper nesting locally.",
139        llm_specific: false,
140    },
141    NestedIf => {
142        code: "nested-if",
143        description: "If nested inside if",
144        category: "nesting",
145        problem: NESTED_CONDITIONAL_PROBLEM,
146        fix: NESTED_CONDITIONAL_FIX,
147        exception: NESTED_CONDITIONAL_EXCEPTION,
148        llm_specific: false,
149    },
150    IfInMatch => {
151        code: "if-in-match",
152        description: "If inside match arm",
153        category: "nesting",
154        problem: NESTED_CONDITIONAL_PROBLEM,
155        fix: NESTED_CONDITIONAL_FIX,
156        exception: NESTED_CONDITIONAL_EXCEPTION,
157        llm_specific: false,
158    },
159    NestedMatch => {
160        code: "nested-match",
161        description: "Match nested inside match",
162        category: "nesting",
163        problem: NESTED_CONDITIONAL_PROBLEM,
164        fix: NESTED_CONDITIONAL_FIX,
165        exception: NESTED_CONDITIONAL_EXCEPTION,
166        llm_specific: false,
167    },
168    MatchInIf => {
169        code: "match-in-if",
170        description: "Match inside if branch",
171        category: "nesting",
172        problem: NESTED_CONDITIONAL_PROBLEM,
173        fix: NESTED_CONDITIONAL_FIX,
174        exception: NESTED_CONDITIONAL_EXCEPTION,
175        llm_specific: false,
176    },
177    ElseChain => {
178        code: "else-chain",
179        description: "Long if/else if chain",
180        category: "nesting",
181        problem: "Long if/else if/else if chains are unordered match arms in disguise. Easy to miss cases, hard to verify exhaustiveness.",
182        fix: "Use `match` on boolean tuples. Precedence becomes explicit, compiler checks exhaustiveness.",
183        exception: "None. Any boolean chain can be refactored to a tuple match.",
184        llm_specific: false,
185    },
186    ForbiddenAttribute { pattern: Arc<str> } => {
187        code: "forbidden-attribute",
188        description: "Forbidden attribute pattern",
189        category: "forbid_attributes",
190        problem: "Silences warnings that indicate real problems. Dead code is maintenance burden. Unused variables often signal logic errors.",
191        fix: "Remove dead code. Use `_` prefix for intentionally unused bindings. Address the underlying issue rather than suppressing.",
192        exception: "Generated code, FFI bindings, conditional compilation.",
193        llm_specific: true,
194    },
195    ForbiddenType { pattern: Arc<str> } => {
196        code: "forbidden-type",
197        description: "Forbidden type pattern",
198        category: "forbid_types",
199        problem: "Certain type patterns indicate suboptimal design. Arc<String> has double indirection. Box<dyn Error> is superseded by better alternatives.",
200        fix: "Use Arc<str> instead of Arc<String>. Use thiserror or anyhow instead of Box<dyn Error>.",
201        exception: "When mutation methods are needed via Arc::make_mut(), or legacy API interop.",
202        llm_specific: true,
203    },
204    ForbiddenCall { pattern: Arc<str> } => {
205        code: "forbidden-call",
206        description: "Forbidden method call pattern",
207        category: "forbid_calls",
208        problem: ".unwrap() and .expect() panic on failure with no recovery. .clone() hides allocations.",
209        fix: "Use `?` for propagation. Use .unwrap_or(), .unwrap_or_default() for defaults. Restructure ownership to avoid clone.",
210        exception: "Human-authored code may use .unwrap() on provably infallible paths with documented invariants. Does not apply to LLM-generated code.",
211        llm_specific: true,
212    },
213    ForbiddenMacro { pattern: Arc<str> } => {
214        code: "forbidden-macro",
215        description: "Forbidden macro pattern",
216        category: "forbid_macros",
217        problem: "panic!/todo!/unimplemented! crash at runtime. dbg!/println! are debug artifacts that shouldn't be committed.",
218        fix: "Return Result instead of panicking. Use proper logging (tracing, log) for diagnostics. Implement functionality instead of stubbing.",
219        exception: "Invariant assertions for bugs (not expected failures). CLI tools where stdout is the interface.",
220        llm_specific: true,
221    },
222    ForbiddenElse => {
223        code: "forbidden-else",
224        description: "Use of `else` keyword (style preference)",
225        category: "forbid_else",
226        problem: "`else` creates implicit branches. `match` makes all branches explicit and compiler-checked.",
227        fix: "Use `match` for multi-way branches. Use early return with guard clauses instead of if/else.",
228        exception: "This is a style preference. Clippy recommends if/else for simple boolean conditions (match_bool lint). Disable with `forbid_else = false` if you disagree.",
229        llm_specific: false,
230    },
231    ForbiddenUnsafe => {
232        code: "forbidden-unsafe",
233        description: "Use of `unsafe` keyword",
234        category: "forbid_unsafe",
235        problem: "`unsafe` bypasses Rust's safety guarantees. Memory corruption, undefined behavior, and security vulnerabilities become possible.",
236        fix: "Use safe abstractions. Wrap unsafe in minimal, well-audited modules with safe public APIs.",
237        exception: "FFI bindings, performance-critical code with proven safety invariants, implementing safe abstractions over unsafe primitives.",
238        llm_specific: false,
239    },
240    DynReturn => {
241        code: "dyn-return",
242        description: "Dynamic dispatch in return type (`Box<dyn T>`, `Arc<dyn T>`)",
243        category: "dispatch",
244        problem: "Returning Box<dyn Trait> or Arc<dyn Trait> forces vtable dispatch on every call. The vtable lookup prevents inlining and all downstream optimizations.",
245        fix: "Use enum dispatch for a closed set of types. Use `impl Trait` when the caller doesn't need to store heterogeneously. Use a generic type parameter when the concrete type varies per call site.",
246        exception: "Plugin systems or FFI boundaries where the set of concrete types is truly open-ended and unknown at compile time.",
247        llm_specific: true,
248    },
249    DynParam => {
250        code: "dyn-param",
251        description: "Dynamic dispatch in function parameter (`&dyn T`, `Box<dyn T>`)",
252        category: "dispatch",
253        problem: "Accepting &dyn Trait or Box<dyn Trait> as a parameter forces vtable dispatch per call. The compiler cannot monomorphize or inline the callee's methods.",
254        fix: "Use a generic parameter `T: Trait` or `impl Trait` to enable monomorphization. The compiler generates specialized code for each concrete type, enabling inlining.",
255        exception: "When the function is called with many distinct concrete types and binary size is a concern, or when storing heterogeneous collections.",
256        llm_specific: true,
257    },
258    VecBoxDyn => {
259        code: "vec-box-dyn",
260        description: "`Vec<Box<dyn T>>` prevents cache locality and inlining",
261        category: "dispatch",
262        problem: "Vec<Box<dyn Trait>> incurs per-element heap allocation, vtable dispatch on every access, and scattered memory that defeats cache prefetching.",
263        fix: "Use an enum wrapping the known concrete types. Elements are stored inline in the Vec with no vtable and no per-element allocation.",
264        exception: "Plugin systems where concrete types are loaded at runtime and cannot be enumerated at compile time.",
265        llm_specific: true,
266    },
267    DynField => {
268        code: "dyn-field",
269        description: "Dynamic dispatch in struct field (`Box<dyn T>`, `Arc<dyn T>`)",
270        category: "dispatch",
271        problem: "A Box<dyn Trait> or Arc<dyn Trait> struct field permanently commits every method call on that field to vtable dispatch. This prevents inlining for the lifetime of the struct.",
272        fix: "Make the struct generic over the trait: `struct Foo<T: Trait> { field: T }`. The compiler monomorphizes each instantiation, enabling static dispatch and inlining.",
273        exception: "When the struct must hold different concrete types at different times, or when the concrete type is determined at runtime (e.g., configuration-driven).",
274        llm_specific: true,
275    },
276    CloneInLoop => {
277        code: "clone-in-loop",
278        description: "clone() called inside loop body (Arc/Rc suppressed when type is visible)",
279        category: "performance",
280        problem: ".clone() inside a loop body means N heap allocations where N is the iteration count. LLMs add .clone() to satisfy the borrow checker without considering the per-iteration cost. Arc/Rc clones are automatically suppressed when the type is visible (explicit type annotations or containers with Arc/Rc generic args). Type aliases that hide Arc/Rc (e.g., type MyMap = BTreeMap<Arc<str>, Arc<str>>) cannot be resolved and may cause false positives.",
281        fix: "Borrow instead of cloning. Use Cow<T> for conditional ownership. Use Rc/Arc for shared ownership. Restructure to move ownership before the loop.",
282        exception: "When the cloned value is mutated independently per iteration and borrowing is not possible.",
283        llm_specific: true,
284    },
285    DefaultHasher => {
286        code: "default-hasher",
287        description: "HashMap/HashSet with default SipHash hasher",
288        category: "performance",
289        problem: "HashMap/HashSet default to SipHash, designed for HashDoS resistance. SipHash is 2-5x slower than FxHash or AHash for typical keys (integers, short strings).",
290        fix: "Use rustc_hash::FxHashMap for integer keys. Use ahash::AHashMap for general-purpose fast hashing. Specify the hasher explicitly: HashMap<K, V, S>.",
291        exception: "When keys come from untrusted input (network, user-provided) and HashDoS resistance is required.",
292        llm_specific: true,
293    },
294    MixedConcerns => {
295        code: "mixed-concerns",
296        description: "Disconnected type groups indicate mixed concerns",
297        category: "structure",
298        problem: "Disconnected type groups in a single file indicate mixed concerns. Types that share no fields, trait bounds, or function signatures belong in separate modules.",
299        fix: "Split the file along connected components. Each group of related types becomes its own module.",
300        exception: "Re-export modules or files that intentionally collect small, independent items (e.g., error enums).",
301        llm_specific: true,
302    },
303    InlineTests => {
304        code: "inline-tests",
305        description: "Test module embedded in source file",
306        category: "structure",
307        problem: "Test modules embedded in source files mix production code with test code. This inflates source files and makes test organization harder to navigate.",
308        fix: "Move tests to the tests/ directory as integration tests, or to a separate test file alongside the source.",
309        exception: "Small utility modules where colocated unit tests are preferred for locality.",
310        llm_specific: true,
311    },
312    GenericNaming => {
313        code: "generic-naming",
314        description: "High ratio of generic variable names in a function",
315        category: "naming",
316        problem: "LLMs generate generic names like `tmp`, `data`, `val` because training data is saturated with them. System prompt rules like 'use descriptive names' compete with this statistical bias and lose.",
317        fix: "Use domain-specific names that describe what the value represents: `user_id` not `val`, `retry_count` not `tmp`, `response_body` not `data`.",
318        exception: "Small utility functions (fewer than 2 generic names) where short names are conventional.",
319        llm_specific: true,
320    },
321    LetUnderscoreResult => {
322        code: "let-underscore-result",
323        description: "let _ = discards a potentially fallible Result",
324        category: "structure",
325        problem: "Silently discarding a Result hides errors that surface only in production.",
326        fix: "Handle the error with `?`, `match`, or `if let Err`; or use `.expect()` with a reason if the error is truly impossible.",
327        exception: "`write!`/`writeln!` to a `String` binding — fmt::Write for String is infallible.",
328        llm_specific: true,
329    },
330    HighParamCount => {
331        code: "high-param-count",
332        description: "Function has too many parameters",
333        category: "structure",
334        problem: "Functions with many parameters are hard to call correctly. Callers must remember argument order, and adding parameters is a breaking change at every call site.",
335        fix: "Group related parameters into a struct. Use the builder pattern for optional configuration. Split the function if parameters serve different concerns.",
336        exception: "FFI bindings that must match an external C signature.",
337        llm_specific: true,
338    },
339    LongFunctionBody => {
340        code: "long-function-body",
341        description: "Function body exceeds the configured line ceiling",
342        category: "structure",
343        problem: "A single oversized function body concentrates many responsibilities in one scope. It resists testing, hides bugs in the middle, and is the dominant single-responsibility failure mode in AI-generated Rust. Nesting and parameter checks measure body shape, not body extent.",
344        fix: "Extract cohesive sections into named helper functions. Each function should do one job describable without the word `and`.",
345        exception: "Generated code or exhaustive `match` dispatchers where every arm is a trivial one-liner.",
346        llm_specific: false,
347    },
348    ModuleRootDefinitions => {
349        code: "module-root-definitions",
350        description: "Item defined in a module-root file (mod.rs/lib.rs)",
351        category: "structure",
352        problem: "Module-root files should only wire the module tree together with declarations and re-exports. Defining types, functions, or impls in them buries real logic in the file that is supposed to be a table of contents, and creates a decomposed-facade ambiguity with sibling module files.",
353        fix: "Move the definition into a dedicated module file and re-export it from the root with `pub use`.",
354        exception: "None. A module root is for declarations and re-exports; definitions belong in leaf modules.",
355        llm_specific: false,
356    },
357    ItemVisibilityPolicy { detail: VisibilityDetail } => {
358        code: "item-visibility-policy",
359        description: "Configured item does not match its required visibility",
360        category: "structure",
361        problem: "Some items must keep an exact visibility to preserve an architectural boundary — a type sealed to its module, an API kept crate-internal. A drift to `pub`, a rename, a duplicate, or a wrong item kind silently widens or breaks that boundary.",
362        fix: "Restore the item to the configured visibility, or update the policy in `.pedant.toml` if the boundary intentionally changed.",
363        exception: "None. The policy is an explicit, per-item contract; change the contract rather than ignore it.",
364        llm_specific: false,
365    },
366    FeatureBoundary => {
367        code: "feature-boundary",
368        description: "Cargo feature crosses a configured boundary",
369        category: "structure",
370        problem: "Dev-only or test-support features must stay sealed: enabled by no default feature, and reachable only through dev-dependency edges. A normal or build edge — or a default-feature chain — that enables such a feature leaks test scaffolding into production builds.",
371        fix: "Move the feature-enabling dependency to `[dev-dependencies]`, drop it from default features, or stop requesting the feature on normal/build edges.",
372        exception: "None. Change the boundary rule if the feature is intentionally public.",
373        llm_specific: false,
374    },
375    FlatModuleFamily => {
376        code: "flat-module-family",
377        description: "Prefixed module family member outside its package directory",
378        category: "structure",
379        problem: "A configured module family must live below a single directory module. A `prefix.rs`, `prefix_*.rs`, or `prefix_*/` sitting flat beside its package directory scatters the family across the parent, obscuring that the members form one cohesive unit.",
380        fix: "Move the member below the configured package directory (e.g. `parent/package_root/`).",
381        exception: "None. Keep a prefixed family under its one package directory.",
382        llm_specific: false,
383    },
384    ConflictingModuleRoot => {
385        code: "conflicting-module-root",
386        description: "Sibling `<stem>.rs` and `<stem>/` module roots",
387        category: "structure",
388        problem: "A `<stem>.rs` file beside a `<stem>/` directory gives a module two possible roots. The convention is directory modules rooted at `<stem>/mod.rs`; the stray sibling file recreates a decomposed-facade/root ambiguity that hides where the module actually lives.",
389        fix: "Fold the `<stem>.rs` contents into `<stem>/mod.rs` and delete the sibling file.",
390        exception: "None. Pick one module-root form per module.",
391        llm_specific: false,
392    },
393    UngatedTestApi => {
394        code: "ungated-test-api",
395        description: "Test-only API under src/ not gated behind a feature",
396        category: "structure",
397        problem: "A test-only helper (e.g. `*_for_tests`) compiled into production `src/` without a feature gate ships test scaffolding to every consumer, widening the API surface and inviting misuse in non-test code.",
398        fix: "Move the item behind `#[cfg(feature = \"test-support\")]` (on the item or an enclosing module), or relocate it into a test module.",
399        exception: "None. Test-only APIs belong behind the configured feature; adjust the naming pattern or feature in config if the convention differs.",
400        llm_specific: false,
401    },
402    HighMethodCount { type_name: Box<str> } => {
403        code: "high-method-count",
404        description: "Type has too many inherent methods (god-object)",
405        category: "structure",
406        problem: "A type whose inherent methods span many unrelated concerns is a god-object: maximally connected, so `mixed-concerns` stays silent, yet carrying far more than one responsibility. It is the dominant single-responsibility failure mode in large AI-generated Rust.",
407        fix: "Extract cohesive groups of methods onto collaborator types the god-object delegates to. Pure forwarders that preserve the public API are not counted.",
408        exception: "A facade that has genuinely shed its logic into collaborators and keeps only thin forwarders — those are excluded by default.",
409        llm_specific: false,
410    },
411    ScatteredInherentImpl => {
412        code: "scattered-inherent-impl",
413        description: "A type's inherent impls span more than one file",
414        category: "structure",
415        problem: "A type whose own API is spread across files has no single place to read what it does. It also hides god-objects: a per-file method ceiling counts only the slice in front of it, so splitting an `impl` in two silences the ceiling while the type keeps every method it had.",
416        fix: "Gather the type's inherent impls into the file that defines it, or split the type itself so each file owns a type with its own responsibility.",
417        exception: "Impls that never coexist in one build — platform or feature `#[cfg]` splits — are already excluded.",
418        llm_specific: false,
419    },
420    LargeSourceFile => {
421        code: "large-source-file",
422        description: "Source file exceeds the configured line ceiling",
423        category: "structure",
424        problem: "A file that accumulates many unrelated items becomes a dumping ground: hard to navigate, review, and reason about, and a sign that distinct concerns were never split into modules. Per-function and per-type size checks miss it because each item can be small while the file as a whole is enormous.",
425        fix: "Split the file into focused modules grouped by concern, and re-export from the module root.",
426        exception: "Generated files, or a documented aggregation point with a per-path threshold override recording the rationale.",
427        llm_specific: false,
428    },
429}