diff --git a/README.md b/README.md index fbf17d9..540bd07 100644 --- a/README.md +++ b/README.md @@ -4,1331 +4,209 @@ [![crates.io](https://img.shields.io/crates/v/rustqual.svg)](https://crates.io/crates/rustqual) [![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) -Comprehensive Rust code quality analyzer — six dimensions: Complexity, Coupling, DRY, IOSP, SRP, Test Quality — plus 7 structural binary checks integrated into SRP and Coupling. Particularly useful as a structural quality guardrail for AI-generated code, catching the god-functions, mixed concerns, duplicated patterns, and weak tests that AI coding agents commonly produce. +**A structural quality guardrail for Rust — with AI coding agents specifically in mind.** rustqual scores your code across seven dimensions (IOSP, Complexity, DRY, SRP, Coupling, Test Quality, Architecture) and combines them into one quality number. Equally useful for senior teams enforcing architecture in CI. -## Quality Dimensions +What sets it apart from clippy and other Rust linters: rustqual reasons across files and modules, not just within functions. Its architecture rules and call-parity check verify properties that span an entire codebase, which is where most real drift happens. -rustqual analyzes your Rust code across **seven quality dimensions**, each contributing to an overall quality score: +It catches what AI agents consistently produce and what tired humans consistently miss: god-functions that mix orchestration with logic, copy-paste-with-variation, tests without assertions, architectural drift across adapter layers (CLI, MCP, REST, …), and dead code that piles up after a refactor pivot. -| Dimension | Weight | What it checks | -|--------------|--------|----------------| -| IOSP | 22% | Function separation (Integration vs Operation) | -| Complexity | 18% | Cognitive/cyclomatic complexity, magic numbers, nesting depth, function length, unsafe blocks, error handling | -| DRY | 13% | Duplicate functions, fragments, dead code, boilerplate | -| SRP | 18% | Struct cohesion (LCOM4), module length, function clusters, structural checks (BTC, SLM, NMS) | -| Coupling | 9% | Module instability, circular dependencies, SDP, structural checks (OI, SIT, DEH, IET) | -| Test Quality | 10% | Assertion density, no-SUT tests, untested functions, coverage gaps, untested logic | -| Architecture | 10% | Layer ordering, forbidden-edge rules, symbol patterns (path/method/function/macro/derive/item-kind), trait-signature contracts | - -## What is IOSP? - -The **Integration Operation Segregation Principle** (from Ralf Westphal's *Flow Design*) states that every function should be **either**: - -- **Integration** — orchestrates other functions, contains no logic of its own -- **Operation** — contains logic (control flow, computation), but does not call other "own" functions - -A function that does **both** is a **violation**. A function too small to matter (empty body, single expression without logic or own calls) is classified as **Trivial**. - -``` -┌─────────────┐ ┌─────────────┐ ┌────────────────────┐ -│ Integration │ │ Operation │ │ ✗ Violation │ -│ │ │ │ │ │ -│ calls A() │ │ if x > 0 │ │ if x > 0 │ -│ calls B() │ │ y = x*2 │ │ result = calc() │ ← mixes both -│ calls C() │ │ return y │ │ return result + 1 │ -└─────────────┘ └─────────────┘ └────────────────────┘ -``` - -## Installation - -```bash -# From crates.io -cargo install rustqual - -# From source -cargo install --path . - -# Then use either: -rustqual # direct invocation (defaults to .) -cargo qual # as cargo subcommand -``` - -## Quick Start +## What it looks like ```bash -# Analyze current directory (default — matches architecture-rule globs) -rustqual - -# Analyze a specific file or directory -rustqual src/lib.rs +$ cargo install rustqual +$ rustqual . -# Show all functions, not just findings -rustqual --verbose - -# Do not exit with code 1 on findings (for local exploration) -rustqual --no-fail - -# Generate a default config file -rustqual --init - -# Watch mode: re-analyze on file changes -rustqual --watch src/ -``` - -> **Using AI coding agents?** See [Using with AI Coding Agents](#using-with-ai-coding-agents) for integration patterns with Claude Code, Cursor, Copilot, and other tools. - -## Output Formats - -### Text (default) - -```bash -rustqual src/ --verbose -``` - -``` ── src/order.rs ✓ INTEGRATION process_order (line 12) ✓ OPERATION calculate_discount (line 28) - Complexity: logic=2, calls=0, nesting=1, cognitive=2, cyclomatic=3 ✗ VIOLATION process_payment (line 48) [MEDIUM] - Logic: if (line 50), comparison (line 50), if (line 56) - Calls: determine_payment_method (line 55), charge_credit_card (line 59) - Complexity: logic=3, calls=2, nesting=1, cognitive=5, cyclomatic=4 - · TRIVIAL get_name (line 72) - ~ SUPPRESSED legacy_handler (line 85) ═══ Summary ═══ Functions: 24 Quality Score: 82.3% - IOSP: 85.7% (4I, 8O, 10T, 2 violations) - Complexity: 90.0% (3 complexity, 1 magic numbers) - DRY: 95.0% (1 duplicates, 2 dead code) + IOSP: 85.7% + Complexity: 90.0% + DRY: 95.0% SRP: 100.0% Test Quality: 100.0% Coupling: 100.0% - - ~ Suppressed: 1 + Architecture: 100.0% 4 quality findings. Run with --verbose for details. ``` -### JSON - -```bash -rustqual --json -# or -rustqual --format json -``` - -Produces machine-readable output with `summary`, `functions`, `coupling`, `duplicates`, `dead_code`, `fragments`, `boilerplate`, and `srp` sections: - -```json -{ - "summary": { - "total": 24, - "integrations": 4, - "operations": 8, - "violations": 2, - "trivial": 10, - "suppressed": 1, - "iosp_score": 0.857, - "quality_score": 0.823, - "coupling_warnings": 0, - "coupling_cycles": 0, - "duplicate_groups": 0, - "dead_code_warnings": 0, - "fragment_groups": 0, - "boilerplate_warnings": 0, - "srp_struct_warnings": 0, - "srp_module_warnings": 0, - "suppression_ratio_exceeded": false - }, - "functions": [...] -} -``` - -### GitHub Actions Annotations - -```bash -rustqual --format github -``` - -Produces `::warning`, `::error`, and `::notice` annotations that GitHub Actions renders inline on PRs: - -``` -::warning file=src/order.rs,line=48::IOSP violation in process_payment: logic=[if (line 50)], calls=[determine_payment_method (line 55)] -::error::Quality analysis: 2 violation(s), 82.3% quality score -``` - -### DOT (Graphviz) - -```bash -rustqual --format dot > call-graph.dot -dot -Tsvg call-graph.dot -o call-graph.svg -``` - -Generates a call-graph visualization with color-coded nodes: -- Green: Integration -- Blue: Operation -- Red: Violation -- Gray: Trivial - -### SARIF - -```bash -rustqual --format sarif > report.sarif -``` - -Produces [SARIF v2.1.0](https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html) output for integration with GitHub Code Scanning, VS Code SARIF Viewer, and other static analysis platforms. Includes rules for all dimensions (IOSP, complexity, coupling, DRY, SRP, test quality). - -### HTML - -```bash -rustqual --format html > report.html -``` - -Generates a self-contained HTML report with: -- Dashboard showing overall quality score and 6 dimension scores -- Collapsible detail sections for IOSP, Complexity, DRY, SRP, Test Quality, and Coupling findings -- Color-coded severity indicators and inline CSS (no external dependencies) - -## CLI Reference - -``` -rustqual [OPTIONS] [PATH] -``` - -| Argument / Flag | Description | -|---------------------------------|----------------------------------------------------------------------| -| `PATH` | File or directory to analyze. Defaults to `.` | -| `-v, --verbose` | Show all functions, not just findings | -| `--json` | Output as JSON (shorthand for `--format json`) | -| `--format ` | Output format: `text`, `json`, `github`, `dot`, `sarif`, `html` | -| `-c, --config ` | Path to config file. Defaults to auto-discovered `rustqual.toml` | -| `--strict-closures` | Treat closures as logic (stricter analysis) | -| `--strict-iterators` | Treat iterator chains (`.map`, `.filter`, ...) as logic | -| `--allow-recursion` | Don't count recursive calls as violations | -| `--strict-error-propagation` | Count `?` operator as logic (implicit control flow) | -| `--no-fail` | Do not exit with code 1 on quality findings (local exploration) | -| `--fail-on-warnings` | Treat warnings (e.g. suppression ratio exceeded) as errors (exit 1) | -| `--init` | Generate a tailored `rustqual.toml` based on current codebase metrics | -| `--completions ` | Generate shell completions (bash, zsh, fish, elvish, powershell) | -| `--save-baseline ` | Save current results as a JSON baseline | -| `--compare ` | Compare current results against a saved baseline | -| `--fail-on-regression` | Exit with code 1 if quality score regressed vs baseline | -| `--watch` | Watch for file changes and re-analyze continuously | -| `--suggestions` | Show refactoring suggestions for IOSP violations | -| `--sort-by-effort` | Sort violations by refactoring effort score (descending) | -| `--findings` | Show only findings with `file:line` locations (one per line) | -| `--min-quality-score ` | Exit with code 1 if quality score is below threshold (0–100) | -| `--diff [REF]` | Only analyze files changed vs a git ref (default: HEAD) | -| `--coverage ` | Path to LCOV coverage file for test quality analysis (TQ-005) | -| `--explain ` | Architecture dimension: print layer assignment, classified imports, and active rules for one file | - -### Exit Codes - -| Code | Meaning | -|------|---------------------------------------------| -| `0` | Success (no findings, or `--no-fail` set) | -| `1` | Quality findings found (default), regression detected (`--fail-on-regression`), quality gate breached (`--min-quality-score`), or warnings present with `--fail-on-warnings` | -| `2` | Configuration error (invalid or unreadable config file) | - -## Configuration - -The analyzer auto-discovers `rustqual.toml` by searching from the analysis path upward through parent directories. You can also specify a config explicitly with `--config`. Generate a commented default config with `--init`. - -If a `rustqual.toml` exists but cannot be parsed (syntax errors, unknown fields), the analyzer exits with code 2 and an error message instead of silently falling back to defaults. - -### Full `rustqual.toml` Reference - -```toml -# ──────────────────────────────────────────────────────────────── -# Ignore Functions -# ──────────────────────────────────────────────────────────────── -# Functions matching these patterns are completely excluded from analysis. -# Supports full glob syntax: *, ?, [abc], [!abc] -ignore_functions = [ - "main", # entry point, always mixes logic + calls - "run", # composition-root dispatcher - "visit_*", # syn::Visit trait implementations (external dispatch) -] - -# ──────────────────────────────────────────────────────────────── -# Exclude Files -# ──────────────────────────────────────────────────────────────── -# Glob patterns for files to exclude from analysis entirely. -exclude_files = ["examples/**"] # e.g. fixture crates for rule demos - -# ──────────────────────────────────────────────────────────────── -# Strictness -# ──────────────────────────────────────────────────────────────── -strict_closures = false # If true, closures count as logic -strict_iterator_chains = false # If true, iterator chains count as own calls -allow_recursion = false # If true, recursive calls don't violate IOSP -strict_error_propagation = false # If true, ? operator counts as logic - -# ──────────────────────────────────────────────────────────────── -# Suppression Ratio -# ──────────────────────────────────────────────────────────────── -# Maximum fraction of functions that may be suppressed (0.0–1.0). -# Exceeding this ratio produces a warning. -max_suppression_ratio = 0.05 - -# If true, exit with code 1 when warnings are present (e.g. suppression ratio exceeded). -# Default: false. Use --fail-on-warnings CLI flag to enable. -fail_on_warnings = false - -# ──────────────────────────────────────────────────────────────── -# Complexity Analysis -# ──────────────────────────────────────────────────────────────── -[complexity] -enabled = true -max_cognitive = 15 # Cognitive complexity threshold -max_cyclomatic = 10 # Cyclomatic complexity threshold -max_nesting_depth = 4 # Maximum nesting depth before warning -max_function_lines = 60 # Maximum function body lines before warning -detect_magic_numbers = true # Flag numeric literals not in allowed list -allowed_magic_numbers = ["0", "1", "-1", "2", "0.0", "1.0"] -detect_unsafe = true # Flag functions containing unsafe blocks -detect_error_handling = true # Flag unwrap/expect/panic/todo usage -allow_expect = false # If true, .expect() calls don't trigger warnings - -# ──────────────────────────────────────────────────────────────── -# Coupling Analysis -# ──────────────────────────────────────────────────────────────── -[coupling] -enabled = true -max_instability = 0.8 # Instability threshold (Ce / (Ca + Ce)) -max_fan_in = 15 # Maximum afferent coupling -max_fan_out = 12 # Maximum efferent coupling -check_sdp = true # Check Stable Dependencies Principle - -# ──────────────────────────────────────────────────────────────── -# DRY / Duplicate Detection -# ──────────────────────────────────────────────────────────────── -[duplicates] -enabled = true -min_tokens = 50 # Minimum token count for duplicate detection -min_lines = 5 # Minimum line count -min_statements = 3 # Minimum statements for fragment detection -similarity_threshold = 0.85 # Jaccard similarity for near-duplicates -ignore_tests = true # Skip test functions -detect_dead_code = true # Enable dead code detection -detect_wildcard_imports = true # Flag use foo::* imports -detect_repeated_matches = true # Flag repeated match blocks (DRY-005) - -# ──────────────────────────────────────────────────────────────── -# Boilerplate Detection -# ──────────────────────────────────────────────────────────────── -[boilerplate] -enabled = true -suggest_crates = true # Suggest derive macros / crates -patterns = [ # Which patterns to check (BP-001 through BP-010) - "BP-001", "BP-002", "BP-003", "BP-004", "BP-005", - "BP-006", "BP-007", "BP-008", "BP-009", "BP-010", -] - -# ──────────────────────────────────────────────────────────────── -# SRP Analysis -# ──────────────────────────────────────────────────────────────── -[srp] -enabled = true -smell_threshold = 0.6 # Composite score threshold for warnings -max_fields = 12 # Maximum struct fields -max_methods = 15 # Maximum impl methods -max_fan_out = 10 # Maximum external call targets -max_parameters = 5 # Maximum function parameters (AST-based) -lcom4_threshold = 3 # LCOM4 component threshold -weights = [0.4, 0.25, 0.15, 0.2] # [lcom4, fields, methods, fan_out] -file_length_baseline = 300 # Production lines before penalty starts -file_length_ceiling = 800 # Production lines at maximum penalty -max_independent_clusters = 2 # Highest allowed (warn on 3+ clusters) -min_cluster_statements = 5 # Min statements for a function to count in clusters - -# ──────────────────────────────────────────────────────────────── -# Structural Binary Checks -# ──────────────────────────────────────────────────────────────── -[structural] -enabled = true -check_btc = true # Broken Trait Contract (SRP) -check_slm = true # Self-less Methods (SRP) -check_nms = true # Needless &mut self (SRP) -check_oi = true # Orphaned Impl (Coupling) -check_sit = true # Single-Impl Trait (Coupling) -check_deh = true # Downcast Escape Hatch (Coupling) -check_iet = true # Inconsistent Error Types (Coupling) - -# ──────────────────────────────────────────────────────────────── -# Test Quality Analysis -# ──────────────────────────────────────────────────────────────── -[test_quality] -enabled = true -coverage_file = "" # Path to LCOV file (or use --coverage CLI flag) -# Extra macro names (beyond assert*/debug_assert*) to recognize as assertions in TQ-001 -# extra_assertion_macros = ["verify", "check", "expect_that"] - -# ──────────────────────────────────────────────────────────────── -# Quality Weights -# ──────────────────────────────────────────────────────────────── -[weights] -iosp = 0.22 -complexity = 0.18 -dry = 0.13 -srp = 0.18 -coupling = 0.09 -test_quality = 0.10 -architecture = 0.10 -# Weights must sum to 1.0 - -# ──────────────────────────────────────────────────────────────── -# Architecture Dimension (see "Architecture Dimension" section for details) -# ──────────────────────────────────────────────────────────────── -[architecture] -enabled = true - -[architecture.layers] -order = ["domain", "port", "infrastructure", "analysis", "application"] -unmatched_behavior = "strict_error" # or "composition_root" - -[architecture.layers.domain] -paths = ["src/domain/**"] - -[architecture.layers.port] -paths = ["src/ports/**"] - -[architecture.layers.infrastructure] -paths = [ - "src/adapters/config/**", - "src/adapters/source/**", - "src/adapters/suppression/**", -] - -[architecture.layers.analysis] -paths = [ - "src/adapters/analyzers/**", - "src/adapters/shared/**", - "src/adapters/report/**", -] - -[architecture.layers.application] -paths = ["src/app/**"] - -[architecture.reexport_points] -paths = [ - "src/lib.rs", - "src/main.rs", - "src/adapters/mod.rs", - "src/bin/**", - "src/cli/**", - "tests/**", -] - -# Optional: map external crate names to your own layers (for workspaces) -[architecture.external_crates] -# "my_domain_crate" = "domain" -# "my_infra_crate" = "infrastructure" - -# Forbidden edges (cross-branch imports the layer rule permits but you don't want) -[[architecture.forbidden]] -from = "src/adapters/analyzers/**" -to = "src/adapters/report/**" -reason = "Analyzers produce findings; reporters consume them separately" - -# Symbol patterns (see "Architecture Dimension" below for all 7 matcher types) -[[architecture.pattern]] -name = "no_panic_helpers_in_production" -forbid_method_call = ["unwrap", "expect"] -forbidden_in = ["src/**"] -except = ["**/tests/**"] -reason = "Production propagates errors through Result" - -[[architecture.pattern]] -name = "no_syn_in_domain" -forbid_path_prefix = ["syn::", "proc_macro2::", "quote::"] -forbidden_in = ["src/domain/**"] -reason = "Domain types know no AST representation" - -# Trait-signature rule (port contract) -[[architecture.trait_contract]] -name = "port_traits" -scope = "src/ports/**" -receiver_may_be = ["shared_ref"] -forbidden_return_type_contains = ["anyhow::", "Box Result> { - // ... -} - -// qual:api -pub fn decode(data: &[u8], config: &Config) -> Result> { - // ... -} -``` - -Unlike `// qual:allow`, API markers do **not** count against the suppression ratio. Use `// qual:api` for functions that are part of your library's public interface — they have no callers within the project because they're meant to be called by external consumers. - -### Test-Helper Annotation - -Mark integration-test helpers with `// qual:test_helper` to exclude them from dead code (DRY-002 `testonly`) and untested function (TQ-003) detection, **while keeping every other check active**: - -```rust -// qual:test_helper -pub fn assert_in_range(actual: f64, expected: f64, tolerance: f64) { - assert!((actual - expected).abs() < tolerance); -} -``` - -This is the narrow fix for the „helper called from `tests/*.rs` but not from production" case that used to force a choice between `ignore_functions` (which silently disables **every** check for that function) and a `qual:allow(dry)` + `qual:allow(test_quality)` stack (which costs against the suppression ratio). Semantic distinction from `qual:api`: - -| Marker | Intent | What it suppresses | -|---|---|---| -| `// qual:api` | „this is the public library API" | DRY-002 (`testonly` dead code) + TQ-003 (untested) | -| `// qual:test_helper` | „this exists so test binaries can call into it" | DRY-002 `testonly` dead code + TQ-003 (untested) | - -Neither marker counts against `max_suppression_ratio`. Complexity, SRP, duplicate detection, and coupling checks keep applying — if a test helper grows to 200 lines with nested match arms, `LONG_FN` and `COGNITIVE` will still flag it. - -### Inverse Annotation - -Mark inverse method pairs with `// qual:inverse(fn_name)` to suppress near-duplicate DRY findings between them: - -```rust -// qual:inverse(parse) -pub fn as_str(&self) -> &str { - match self { - Self::Function => "fn", - Self::Method => "method", - // ... - } -} - -// qual:inverse(as_str) -pub fn parse(s: &str) -> Self { - match s { - "fn" => Self::Function, - "method" => Self::Method, - // ... - } -} -``` - -Common use cases: `serialize`/`deserialize`, `encode`/`decode`, `to_bytes`/`from_bytes`. Like `// qual:api`, inverse markers do **not** count against the suppression ratio — they document intentional structural similarity. - -### Automatic Leaf Detection - -Functions with no own calls (Operations and Trivials) are automatically recognized as **leaf functions**. Calls to leaves do not count as "own calls" for the caller: - -```rust -fn get_config() -> Config { // Operation (C=0) → leaf - if let Ok(c) = load_file() { c } else { Config::default() } -} - -fn cmd_quality(clear: bool) -> Result<()> { - let config = get_config(); // calling a leaf → not an own call - if clear { /* logic */ } // logic only → Operation, not Violation - Ok(()) -} -``` - -Without leaf detection, `cmd_quality` would be a Violation (logic + own call). With it, the call to `get_config` is recognized as terminal — no orchestration involved. - -More broadly, calls to **any non-Violation function** are treated as safe — this includes Operations (pure logic), Trivials (empty/simple), and Integrations (pure delegation). Only calls to other Violations (functions that themselves mix logic and non-safe calls) remain true Violations. This cascades iteratively until stable. - -> **Design note — pragmatic IOSP relaxation:** In strict IOSP, *any* call to an own function from a function with logic constitutes a Violation. rustqual relaxes this: only calls to Violations count as concern-mixing. Calls to well-structured functions (Operations, Integrations, Trivials) are treated as safe building blocks. This eliminates false positives for common patterns while preserving true Violations where tangled code calls other tangled code (e.g., mutually recursive Violations). - -### Recursive Annotation - -Mark intentionally recursive functions with `// qual:recursive` to prevent the self-call from being counted as an own call: - -```rust -// qual:recursive -fn traverse(node: &Node) -> Vec { - let mut result = vec![node.name.clone()]; - for child in &node.children { - result.extend(traverse(child)); // self-call not counted - } - result -} -``` - -Without the annotation, `traverse` would be a Violation (loop logic + self-call). With it, the self-call is removed before classification. Like `// qual:api` and `// qual:inverse`, recursive markers do **not** count against the suppression ratio. - -### Lenient vs. Strict Mode - -By default the analyzer runs in **lenient mode**. This makes it practical for idiomatic Rust code: - -| Construct | Lenient (default) | `--strict-closures` | `--strict-iterators` | -|---------------------------------|-------------------------|--------------------------|-------------------------| -| `items.iter().map(\|x\| x + 1)` | ignored entirely | closure logic counted | `.map()` as own call | -| `\|\| { if cond { a } }` | closure logic ignored | `if` counted as logic | — | -| `self.do_work()` in closure | call ignored | call counted as own | — | -| `x?` | not logic | — | — | -| `async { if x { } }` | ignored (like closures) | — | — | - -Use `--strict-error-propagation` to count `?` as logic. - -## Features - -### Quality Score - -The overall quality score is a weighted average of seven dimension scores (weights are configurable via `[weights]` in `rustqual.toml`): - -| Dimension | Default Weight | Metric | -|--------------|----------------|--------| -| IOSP | 22% | Compliance ratio (non-trivial functions) | -| Complexity | 18% | 1 - (complexity + magic numbers + nesting + length + unsafe + error handling) / total | -| DRY | 13% | 1 - (duplicates + fragments + dead code + boilerplate + wildcards + repeated matches) / total | -| SRP | 18% | 1 - (struct warnings + module warnings + param warnings + structural BTC/SLM/NMS) / total | -| Coupling | 9% | 1 - (coupling warnings + 2×cycles + SDP violations + structural OI/SIT/DEH/IET) / total | -| Test Quality | 10% | 1 - (assertion-free + no-SUT + untested + uncovered + untested-logic) / total | -| Architecture | 10% | 1 - (layer violations + forbidden edges + pattern hits + trait-contract breaches) / total | - -Quality score ranges from 0% (all findings) to 100% (no findings). Weights must sum to 1.0. - -### Quality Gates - -By default, the analyzer exits with code 1 on any findings — no extra flags needed for CI. Use `--no-fail` for local exploration. - -```bash -# Fail if quality score is below 90% -rustqual src/ --min-quality-score 90 - -# Local exploration (never fail) -rustqual src/ --no-fail -``` - -### Violation Severity - -Violations are categorized by severity based on the number of findings: - -| Severity | Condition | -|----------|--------------------| -| Low | ≤2 total findings | -| Medium | 3–5 total findings | -| High | >5 total findings | - -Severity is shown as `[LOW]`, `[MEDIUM]`, `[HIGH]` in text output and as a `severity` field in JSON/SARIF. - -### Complexity Metrics - -Each analyzed function gets complexity metrics (shown with `--verbose`): - -- **cognitive_complexity**: Cognitive complexity score (increments for nesting depth) -- **cyclomatic_complexity**: Cyclomatic complexity score (decision points + 1) -- **magic_numbers**: Numeric literals not in the configured allowed list -- **logic_count**: Number of logic occurrences (if, match, operators, etc.) -- **call_count**: Number of own-function calls -- **max_nesting**: Maximum nesting depth of control flow -- **function_lines**: Number of lines in the function body -- **unsafe_blocks**: Count of `unsafe` blocks -- **unwrap/expect/panic/todo**: Error handling pattern counts - -### Coupling Analysis - -Detects module-level coupling issues: - -- **Afferent coupling (Ca)**: Modules depending on this one (fan-in) -- **Efferent coupling (Ce)**: Modules this one depends on (fan-out) -- **Instability**: Ce / (Ca + Ce), ranging from 0.0 (stable) to 1.0 (unstable) -- **Circular dependencies**: Detected via Kosaraju's iterative SCC algorithm - -Leaf modules (Ca=0) are excluded from instability warnings since I=1.0 is natural for them. - -- **Stable Dependencies Principle (SDP)**: Flags when a stable module (low instability) depends on a more unstable module. This violates the principle that dependencies should flow toward stability. - -### DRY Analysis - -Detects five categories of repetition: +Exit code `1` on findings — drop it into CI without ceremony. -- **Duplicate functions**: Exact and near-duplicate functions (via AST normalization + Jaccard similarity) -- **Duplicate fragments**: Repeated statement sequences across functions (sliding window + merge) -- **Dead code**: Functions never called from production code, or only called from tests. Detects both direct calls and function references passed as arguments (e.g., `.for_each(some_fn)`). -- **Boilerplate patterns**: 10 common Rust boilerplate patterns (BP-001 through BP-010) including trivial `From`/`Display` impls, manual getters/setters, builder patterns, manual `Default`, repetitive match arms, error enum boilerplate, and clone-heavy conversions -- **Wildcard imports**: Flags `use foo::*` glob imports (excludes `prelude::*` paths and `use super::*` in test modules) -- **Repeated match patterns** (DRY-005): Detects identical `match` blocks (≥3 arms) duplicated across ≥3 instances in ≥2 functions, via AST normalization and structural hashing +## Why it exists -### SRP Analysis +If you've used Claude Code, Cursor, GitHub Copilot, or Codex on Rust projects, you've seen the same patterns: -Detects Single Responsibility Principle violations at three levels: +- Functions that mix orchestration ("call helper, if/else, call another helper") with logic — hard to test, hard to refactor. +- Copy-paste with minor variation when asked to "do the same for X" instead of extracting an abstraction. +- Tests that exercise code without checking it (`#[test] fn it_works() { run_thing(); }`) — coverage looks good, real coverage is zero. +- Architectural drift: new functionality lands in one adapter (CLI, MCP, REST) and silently misses the others. -- **Struct-level**: LCOM4 cohesion analysis using Union-Find on method→field access graph. Composite score combines normalized LCOM4, field count, method count, and fan-out with configurable weights. -- **Module-level (length)**: Production line counting (before `#[cfg(test)]`) with linear penalty between configurable baseline and ceiling. -- **Module-level (cohesion)**: Detects files with too many independent function clusters. Uses Union-Find on private substantive functions, leveraging IOSP own-call data. Functions that call each other or share a common caller are united into the same cluster. A file with more than `max_independent_clusters` (default 2, so 3+ clusters trigger) independent groups indicates multiple responsibilities that should be split into separate modules. +rustqual catches all of these mechanically. Wired into the agent's feedback loop (CI, hooks, instruction file), the agent self-corrects. Reviewer time goes to the actual logic, not to spotting fake tests and inlined god-functions. -### Structural Binary Checks +The same checks help senior teams enforce architecture decisions in CI — the layer rules and forbidden-edge rules don't care whether the code came from a human or an LLM. They keep the codebase coherent over time. -Seven binary (pass/fail) checks for common Rust structural issues, integrated into existing dimensions: +rustqual addresses each of these patterns through a separate quality dimension. Each is independently tunable; together they produce one aggregated quality score. -| Rule | Name | Dimension | What it checks | -|------|------|-----------|----------------| -| BTC | Broken Trait Contract | SRP | Impl blocks missing required trait methods | -| SLM | Self-less Methods | SRP | Methods in impl blocks that don't use `self` (could be free functions) | -| NMS | Needless `&mut self` | SRP | Methods taking `&mut self` that only read from self | -| OI | Orphaned Impl | Coupling | Impl blocks in files that don't define the implemented type | -| SIT | Single-Impl Trait | Coupling | Traits with exactly one implementation (unnecessary abstraction) | -| DEH | Downcast Escape Hatch | Coupling | `.downcast_ref()` / `.downcast_mut()` / `.downcast()` usage (broken abstraction) | -| IET | Inconsistent Error Types | Coupling | Modules returning 3+ different error types (missing unified error type) | +## Seven quality dimensions -Each rule can be individually toggled via `[structural]` config. Suppress with `// qual:allow(srp)` or `// qual:allow(coupling)` depending on the dimension. - -### Architecture Dimension (v1.0) - -Four rule types check the structural shape of the codebase against an -explicit layered architecture. Enabled via `[architecture] enabled = true`. - -**Layer Rule** — files are assigned to layers via path globs; inner layers -(lower rank) may not import from outer layers (higher rank). With -`unmatched_behavior = "strict_error"`, every production file must match -a layer glob; unmatched files become violations. With `"composition_root"`, -unmatched files bypass the rule entirely (useful for Cargo-workspace roots). - -A minimal hexagonal layering: - -```toml -[architecture.layers] -order = ["domain", "port", "application", "adapter"] -unmatched_behavior = "strict_error" - -[architecture.layers.domain] -paths = ["src/domain/**"] - -[architecture.layers.port] -paths = ["src/ports/**"] - -[architecture.layers.application] -paths = ["src/app/**"] - -[architecture.layers.adapter] -paths = ["src/adapters/**"] - -[architecture.reexport_points] -paths = ["src/lib.rs", "src/main.rs"] -``` - -A file in `src/domain/**` importing from `src/adapters/**` is flagged. -Rustqual itself uses a five-rank variant that separates -infrastructure-style adapters (`config`, `source`, `suppression`) from -analysis-logic adapters (`analyzers`, `shared`, `report`) — see the -committed `rustqual.toml` for the full structure. - -**Forbidden Rule** — paired `from` / `to` path globs forbid cross-branch -imports: - -```toml -[[architecture.forbidden]] -from = "src/adapters/analyzers/iosp/**" -to = "src/adapters/analyzers/**" -except = ["src/adapters/analyzers/iosp/**"] -reason = "peer analyzers are isolated" -``` +| Dimension | What it checks | +|---|---| +| **IOSP** | Function separation: every function is either Integration (orchestrates) or Operation (logic), never both. From Ralf Westphal's Flow Design. | +| **Complexity** | Cognitive/cyclomatic complexity, magic numbers, nesting depth, function length, `unsafe`, error-handling style. | +| **DRY** | Duplicate functions, fragments, dead code, boilerplate (10 BP-* rules), repeated match patterns. | +| **SRP** | Struct cohesion (LCOM4), module length, function clusters, structural method-checks (BTC, SLM, NMS). | +| **Coupling** | Module instability, circular deps, Stable Dependencies Principle, structural checks (OI, SIT, DEH, IET). | +| **Test Quality** | Assertion density, no-SUT tests, untested functions, optional LCOV-based coverage gaps. | +| **Architecture** | Layer rules, forbidden edges, symbol patterns, trait contracts, **call parity** across adapters. | -**Symbol Patterns** — ban specific language shapes via seven matchers: +Each dimension contributes to the aggregated quality score with a configurable weight (defaults to a balanced split summing to 1.0). Each dimension can also be tuned or disabled in `rustqual.toml` — full reference: [book/reference-configuration.md](./book/reference-configuration.md). -| Matcher | Hits | -|---------|------| -| `forbid_path_prefix` | any path reference starting with a banned prefix | -| `forbid_glob_import` | `use foo::*;` | -| `forbid_method_call` | `x.unwrap()` / UFCS `Option::unwrap(x)` | -| `forbid_function_call` | `Box::new(…)` via fully-qualified path | -| `forbid_macro_call` | `panic!()`, `println!()`, etc. | -| `forbid_item_kind` | `async_fn`, `unsafe_fn`, `unsafe_impl`, `static_mut`, `extern_c_block`, `inline_cfg_test_module`, `top_level_cfg_test_item` | -| `forbid_derive` | `#[derive(Serialize)]` | +## What's unusual: call parity -Scope is XOR: either `allowed_in` (whitelist) or `forbidden_in` (blocklist), -with `except` as fine-grained overrides. Example: - -```toml -[[architecture.pattern]] -name = "no_panic_in_production" -forbid_macro_call = ["panic", "todo", "unreachable"] -forbidden_in = ["src/**"] -except = ["**/tests/**"] -reason = "production code returns typed errors" -``` - -**Trait-Signature Rule** — structural checks on trait definitions in scope: - -```toml -[[architecture.trait_contract]] -name = "port_traits" -scope = "src/ports/**" -receiver_may_be = ["shared_ref"] -methods_must_be_async = true -forbidden_return_type_contains = ["anyhow::", "Box`, `Box`, `Rc`, `Cow<'_, T>`, - `&T`, `&mut T` — the Deref-transparent smart pointers — strip to the - inner type. `RwLock` / `Mutex` / `RefCell` / `Cell` do - **not** strip by default (their `read` / `lock` / `borrow` / `get` - methods don't exist on the inner type — stripping would synthesize - bogus edges). Opt in per-wrapper via `transparent_wrappers` if your - codebase uses a genuinely Deref-transparent domain wrapper. `Vec` - / `HashMap<_, V>` preserve the element/value type. -- **`Self::xxx`** in impl-method contexts substitutes to the enclosing - type. -- **`if let Some(s) = opt`** binds `s: T` when `opt: Option`, same - for `Ok(x)` / `Err(e)` patterns. -- **Trait dispatch** (`dyn Trait` / `&dyn Trait` / `Box` - receivers): fans out to every workspace impl of the trait. Method - must be declared on the trait — unrelated methods stay unresolved - rather than fabricating edges. Marker traits (`Send`, `Sync`, …) - skipped when picking the dispatch-relevant bound. -- **Turbofish return types**: `get::()` for generic fns — the - turbofish arg is used as the return type when the workspace index has - no concrete return for `get`. Only single-ident paths trigger. -- **Type aliases**: `type Repo = Arc>;` is recorded and - expanded during receiver resolution, so `fn h(r: Repo) { r.insert(..) }` - reaches `Store::insert` through the peeled smart-pointer chain. - Aliases wrapping non-Deref types (`type Db = Arc>`) still - expand, but the `RwLock` stops peeling — methods on the inner `Store` - aren't reached unless `RwLock` is listed in `transparent_wrappers`. - -For framework codebases you can extend the wrapper and macro lists: - -```toml -[architecture.call_parity] -# Framework extractor wrappers peeled like Arc / Box: -transparent_wrappers = ["State", "Extension", "Json", "Data"] -# Attribute macros that don't affect the call graph. The set is -# recorded for future macro-expansion integrations and currently has -# no observable effect on the call-graph / type-inference pipeline. -transparent_macros = ["my_custom_attr"] ``` -Two escape mechanisms: -- `exclude_targets` — glob list in config for whole groups of - legitimately asymmetric target fns. -- `// qual:allow(architecture)` — per-fn escape for individual - exceptions. Counts against `max_suppression_ratio`. - -See `examples/architecture/call_parity/` for a runnable 3-adapter -fixture. - -Known limits (documented, with clear workarounds): -- **Closure-body arg types** `Session::open().map(|r| r.m())` — the - closure arg's type isn't inferred. Inner method call stays - `:m`. Workaround: pull the method call out of the closure. -- **Unannotated generics** `let x = get(); x.m()` where `get() -> T` - — use turbofish `get::()` or `let x: T = get();`. -- **`impl Trait` inherent methods** — `fn make() -> impl Handler; make().trait_method()` resolves to every workspace impl of `Handler::trait_method` via over-approximation, but an inherent method not declared on `Handler` can't be reached (the concrete type is hidden by design). -- **Multi-bound `impl Trait` / `dyn Trait` returns** — `fn make() -> impl Future + Handler` keeps only the first non-marker bound, so `.await` propagation *or* trait-dispatch fires, never both. Marker traits (`Send`/`Sync`/`Unpin`/`Copy`/`Clone`/`Sized`/`Debug`/`Display`) are filtered first, so `impl Future + Send` is unaffected. Workaround: split the return into two methods, or `qual:allow(architecture)` on the call-site. -- **Caller-side `pub use` path-following.** `pub mod outer { mod private { pub struct Hidden; impl Hidden { pub fn op() } } pub use self::private::Hidden; }` with a caller `fn h(x: outer::Hidden) { x.op() }` resolves the parameter to `crate::…::outer::Hidden` while the impl is keyed under `crate::…::outer::private::Hidden`. Visibility is recognised on both paths, but the call-graph edge goes to `:op` because the resolver doesn't follow workspace-wide `pub use` re-exports inside nested modules. Workaround: write `impl outer::Hidden { … }` at the file-level qualified path so impl-canonical and caller-canonical agree, or `qual:allow(architecture)` at the call-site. -- **Re-exported type aliases inside private modules.** `mod private { pub type Public = Hidden; … } pub use private::Public;` doesn't follow into the alias's target — private modules aren't walked by the visibility pass, so the alias's source type stays out of `visible_canonicals`. Workaround: lift the type alias to the parent module (`pub use private::Hidden; pub type Public = Hidden;`) so both the alias declaration and its target are processed. -- **Type-vs-value namespace ambiguity in `pub use`.** A `pub use internal::helper as Hidden;` re-export adds `Hidden` as a workspace-visible *type* canonical without checking whether the leaf is actually a type. If the same scope has a private `struct Hidden`, its impl methods get registered as adapter surface even though the `pub use` only exported a function. Workaround: rename to avoid the value/type collision, or `qual:allow(architecture)` on the affected impl. -- **`impl Alias { … }` with caller-side alias expansion.** `pub type Public = private::Hidden; impl Public { pub fn op(&self) {} }` indexes the method under `crate::…::Public::op` (impl self-type goes through the path canonicaliser), while a caller `fn h(x: Public) { x.op() }` resolves `x` via type-alias expansion to `crate::…::private::Hidden` and produces a `Hidden::op` edge. Visibility recognises `Public`, but the call-graph edges and the indexed method canonical disagree, so Check B reports `Public::op` as unreached. Workaround: declare the `impl` against the source type (`impl private::Hidden { … }`) so impl-canonical and caller-canonical agree, or `qual:allow(architecture)` on the affected impl. -- **Generic type-alias substitution in the visibility chain.** `type Id = T; pub type Public = Id;` doesn't substitute the use-site arg `private::Hidden` into `Id`'s body in the visibility pass — only the immediate alias `Id` enters `visible_canonicals`. Receiver-side resolution does substitute (the workspace alias-index runs after pub-fn enumeration), so callers reach `Hidden::op` correctly, but Check B can drop the public target. Workaround: skip the generic-alias indirection (`pub type Public = private::Hidden;`), or `qual:allow(architecture)` on the affected impl. -- **Arbitrary proc-macros** not listed in `transparent_macros` — - `// qual:allow(architecture)` on the enclosing fn is the escape. - -Design reference: `docs/rustqual-design-receiver-type-inference.md`. - -**`--explain `** diagnostic mode prints the file's layer assignment, -classified imports, and rule hits — useful for understanding why a rule -fires or when tuning config: +Two checks under one rule: -``` -$ cargo run -- --explain src/domain/foo.rs -═══ Architecture Explain: src/domain/foo.rs ═══ -Layer: domain (rank 0) +- **Check A** — every adapter must delegate. A CLI command that doesn't reach into the application layer is logic in the wrong place. +- **Check B** — every application capability must reach every adapter. Add `app::ingest::run`, forget to wire it into CLI, and Check B reports it by name in CI before review. -Imports (1): - line 1: crate::adapters::Foo — crate::adapters → layer adapter +The hard part is making the call graph honest across method chains, field access, trait dispatch, type aliases, framework extractors, and `Self` substitution. rustqual ships a shallow type-inference engine that resolves these cases without fabricating edges. Full write-up: [book/adapter-parity.md](./book/adapter-parity.md). -Layer violations: - line 1: domain ↛ adapter via crate::adapters::Foo -``` +## Use cases -See `examples/architecture/` for a runnable mini-fixture per matcher/rule. -Suppress with `// qual:allow(architecture)` on the file. +- **AI-assisted Rust development** — agent instruction file, pre-commit hook, CI quality gate, baseline tracking. → [book/ai-coding-workflow.md](./book/ai-coding-workflow.md) +- **CI/CD integration** — GitHub Actions, SARIF, baseline comparison, coverage. → [book/ci-integration.md](./book/ci-integration.md) +- **Adopting on a large existing codebase** — four staged adoption patterns from "lightest touch" to full enforcement. → [book/legacy-adoption.md](./book/legacy-adoption.md) +- **Function-level quality** (IOSP, complexity, structural method checks). → [book/function-quality.md](./book/function-quality.md) +- **Module-level quality** (SRP, LCOM4, file length). → [book/module-quality.md](./book/module-quality.md) +- **Coupling quality** (instability, SDP, OI/SIT/DEH/IET). → [book/coupling-quality.md](./book/coupling-quality.md) +- **Architecture rules** (layers, forbidden edges, symbol patterns, trait contracts). → [book/architecture-rules.md](./book/architecture-rules.md) +- **Adapter parity** — call parity, the architecture rule that's unique to rustqual. → [book/adapter-parity.md](./book/adapter-parity.md) +- **Code reuse** (DRY, dead code, boilerplate). → [book/code-reuse.md](./book/code-reuse.md) +- **Test quality** (assertions, untested functions, coverage). → [book/test-quality.md](./book/test-quality.md) -### Baseline Comparison - -Track quality over time: - -```bash -# Save current state as baseline -rustqual src/ --save-baseline baseline.json - -# ... make changes ... - -# Compare against baseline (shows new/fixed findings, score delta) -rustqual src/ --compare baseline.json +## What is IOSP? -# Fail CI only on regression -rustqual src/ --compare baseline.json --fail-on-regression -``` +The **Integration Operation Segregation Principle** ([Ralf Westphal's Flow Design](https://flow-design.info/)) says every function should be: -The baseline format (v2) includes quality score, all dimension counts, and total findings. V1 baselines (IOSP-only) are still supported for backward compatibility. +- **Integration** — orchestrates other functions. No own logic. +- **Operation** — contains logic. No calls to your own project's functions. -### Refactoring Suggestions +A function that does both is a **Violation** — that's the smell to fix. -```bash -rustqual src/ --suggestions ``` - -Provides pattern-based refactoring hints for violations, such as extracting conditions, splitting dispatch logic, or converting loops to iterator chains. - -### Watch Mode - -```bash -rustqual src/ --watch +┌─────────────┐ ┌─────────────┐ ┌────────────────────┐ +│ Integration │ │ Operation │ │ ✗ Violation │ +│ │ │ │ │ │ +│ calls A() │ │ if x > 0 │ │ if x > 0 │ +│ calls B() │ │ y = x*2 │ │ r = calc() │ ← mixes both +│ calls C() │ │ return y │ │ return r + 1 │ +└─────────────┘ └─────────────┘ └────────────────────┘ ``` -Monitors the filesystem for `.rs` file changes and re-runs analysis automatically. Useful during refactoring sessions. +Out of the box rustqual is forgiving where it matters — closures, iterator chains, match-as-dispatch, and trivial self-getters are all leniency cases. Tighten with `--strict-closures` / `--strict-iterators` if you want them counted as logic. Full breakdown: [book/function-quality.md](./book/function-quality.md). -### Shell Completions +## Install & first run ```bash -# Generate completions for your shell -rustqual --completions bash > ~/.bash_completion.d/rustqual -rustqual --completions zsh > ~/.zfunc/_rustqual -rustqual --completions fish > ~/.config/fish/completions/rustqual.fish +cargo install rustqual +cd your-rust-project +rustqual ``` -## Using with AI Coding Agents - -### Why AI-Generated Code Needs Structural Analysis - -AI coding agents (Claude Code, Cursor, Copilot, etc.) are excellent at producing working code quickly, but they consistently exhibit structural problems that rustqual is designed to catch: - -- **IOSP violations**: AI agents routinely generate functions that mix orchestration with logic — calling helper functions inside `if` blocks, combining validation with dispatch. These "god-functions" are hard to test and hard to maintain. -- **Complexity creep**: Generated functions tend to be long, deeply nested, and full of inline logic rather than composed from small, focused operations. -- **Duplication**: When asked to implement similar features, AI agents often copy-paste patterns rather than extracting shared abstractions, leading to DRY violations. -- **Weak tests**: AI-generated tests frequently lack meaningful assertions, contain overly long test functions, or rely heavily on mocks without verifying real behavior. The Test Quality dimension catches assertion-free tests, low assertion density, and coverage gaps. +Walkthrough with `--init`, `--no-fail`, `--findings`, the common flags, and the first-run output: [book/getting-started.md](./book/getting-started.md). Full flag reference: [book/reference-cli.md](./book/reference-cli.md). -IOSP is particularly valuable for AI-generated code because it enforces a strict decomposition: every function is either an Integration (orchestrates, no logic) or an Operation (logic, no own calls). This constraint forces the kind of small, testable, single-purpose functions that AI agents tend not to produce on their own. +## CI integration -### CLAUDE.md / Cursor Rules Integration - -Project-level instruction files (`.claude/CLAUDE.md`, `.cursorrules`, etc.) can teach AI agents to follow IOSP principles. Add rules like these to your project: - -```markdown -## Code Quality Rules +Minimal GitHub Actions step: -- Run `rustqual src/` after making changes. All findings must be resolved. -- Follow IOSP: every function is either an Integration (calls other functions, - no logic) or an Operation (contains logic, no own-function calls). Never mix both. -- Keep functions under 60 lines and cognitive complexity under 15. -- Do not duplicate logic — extract shared patterns into reusable Operations. -- Do not introduce functions with more than 5 parameters. -- Every test function must contain at least one assertion (assert!, assert_eq!, etc.). -- Generate LCOV coverage data and pass it via `--coverage` to verify coverage gaps. +```yaml +- run: cargo install rustqual +- run: rustqual --format github --min-quality-score 90 ``` -This works with any AI tool that reads project-level instruction files. The key insight is that the agent gets actionable feedback: rustqual tells it exactly which function violated which principle, so it can self-correct. - -### CI Quality Gate for AI-Generated Code - -Add rustqual to your CI pipeline so that AI-generated PRs are automatically checked: +With coverage and PR annotations: ```yaml -name: Quality Check -on: [pull_request] - -jobs: - quality: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - run: cargo install rustqual cargo-llvm-cov - - name: Generate coverage data - run: cargo llvm-cov --lcov --output-path lcov.info - - name: Check quality (changed files only) - run: rustqual --diff HEAD~1 --coverage lcov.info --fail-on-warnings --format github +- run: cargo install rustqual cargo-llvm-cov +- run: cargo llvm-cov --lcov --output-path lcov.info +- run: rustqual --diff origin/main --coverage lcov.info --format github ``` -Key flags for AI workflows: -- `--diff HEAD~1` — only analyze files changed in the PR, not the entire codebase -- `--coverage lcov.info` — include test quality coverage analysis (TQ-005) -- `--fail-on-warnings` — treat suppression ratio violations as errors -- `--min-quality-score 90` — reject PRs that drop quality below a threshold -- `--format github` — produces inline annotations on the PR diff - -See [CI Integration](#ci-integration) for more workflow examples including baseline comparison. - -### Pre-commit Hook - -Catch violations before they enter version control — especially useful when AI agents generate code locally: +For codebases that aren't yet at 100% but want to prevent regression: ```bash -#!/bin/bash -# .git/hooks/pre-commit -if ! rustqual src/ 2>/dev/null; then - echo "rustqual: quality findings detected. Please refactor before committing." - exit 1 -fi -``` - -This gives the AI agent (or developer) immediate feedback before the code is committed. See [Pre-commit Hook](#pre-commit-hook) for the basic setup. - -### Recommended Workflow - -The full quality loop for AI-assisted development: - -1. **Agent instructions** — CLAUDE.md / Cursor rules teach the agent IOSP principles and rustqual usage -2. **Pre-commit hook** — catches violations locally before they enter version control -3. **Coverage verification** — generate LCOV data with `cargo llvm-cov` and pass via `--coverage` to detect weak or missing tests -4. **CI quality gate** — prevents merges below quality threshold using `--min-quality-score` or `--fail-on-regression` -5. **Baseline tracking** — `--save-baseline` and `--compare` track quality score over time, ensuring AI-generated code does not erode structural quality - -## Architecture - -The analyzer uses a **two-pass pipeline**: - -``` - ┌──────────────────────────────────┐ - │ Pass 1: Collect │ - .rs files ──read──► │ Read + Parse all files (rayon) │ - │ Build ProjectScope (all names) │ - │ Scan for // qual:allow markers │ - └────────────────┬─────────────────┘ - │ - ┌────────────────▼─────────────────┐ - │ Pass 2: Analyze │ - │ For each function: │ - │ BodyVisitor walks AST │ - │ → logic + call occurrences │ - │ → complexity metrics │ - │ → classify: I / O / V / T │ - │ Coupling analysis (use-graph) │ - │ DRY detection (normalize+hash) │ - │ SRP analysis (LCOM4+composite) │ - │ Compute quality score │ - └────────────────┬─────────────────┘ - │ - ┌────────────────▼─────────────────┐ - │ Output │ - │ Text / JSON / GitHub / DOT / │ - │ SARIF / HTML / Suggestions / │ - │ Baseline comparison │ - └──────────────────────────────────┘ -``` - -### Source Files - -~200 production files in `src/`, ~19 400 lines. Layered per Clean Architecture: - -``` -src/ -├── lib.rs Composition root (run() entry, ~140 lines) -├── main.rs Thin binary wrapper (rustqual) -├── bin/cargo-qual/main.rs Thin binary wrapper (cargo qual) -│ -├── domain/ Pure value types (no syn, no I/O) -│ ├── dimension.rs -│ ├── finding.rs Port-emitted Finding struct -│ ├── score.rs PERCENTAGE_MULTIPLIER -│ ├── severity.rs -│ ├── source_unit.rs -│ └── suppression.rs -│ -├── ports/ Trait contracts -│ ├── dimension_analyzer.rs DimensionAnalyzer + AnalysisContext + ParsedFile -│ ├── reporter.rs -│ ├── source_loader.rs -│ └── suppression_parser.rs -│ -├── adapters/ -│ ├── config/ TOML loading, tailored --init, weight validation -│ ├── source/ Filesystem walk, parse, --watch -│ ├── suppression/ qual:allow marker parsing -│ ├── shared/ Cross-analyzer utilities -│ │ ├── cfg_test.rs has_cfg_test, has_test_attr -│ │ ├── cfg_test_files.rs collect_cfg_test_file_paths -│ │ ├── normalize.rs AST normalization for DRY -│ │ └── use_tree.rs Canonical use-tree walker -│ ├── analyzers/ Seven dimension analyzers -│ │ ├── iosp/ Analyzer, BodyVisitor, classify, scope -│ │ ├── complexity/ -│ │ ├── dry/ Incl. boilerplate/ (BP-001–BP-010) -│ │ ├── srp/ -│ │ ├── coupling/ -│ │ ├── tq/ -│ │ ├── structural/ BTC, SLM, NMS, OI, SIT, DEH, IET -│ │ └── architecture/ Layer + Forbidden + Symbol + Trait-contract rules -│ └── report/ Text, JSON, SARIF, HTML, DOT, GitHub, -│ AI, AI-JSON, baseline, suggestions -│ -├── app/ Application use cases -│ ├── analyze_codebase.rs Port-based use case -│ ├── pipeline.rs Full-pipeline orchestrator -│ ├── secondary.rs Per-dimension secondary passes -│ ├── metrics.rs Coupling/DRY/SRP helpers -│ ├── tq_metrics.rs -│ ├── structural_metrics.rs -│ ├── architecture.rs Architecture dim wiring via port -│ ├── warnings.rs Complexity + leaf reclass + suppression ratio -│ ├── dry_suppressions.rs -│ ├── exit_gates.rs Default-fail, min-quality, fail-on-warnings -│ └── setup.rs Config loading + CLI overrides -│ -└── cli/ - ├── mod.rs Cli struct (clap), OutputFormat - ├── handlers.rs --init, --completions, --save-baseline, --compare - └── explain.rs --explain architecture diagnostic - -tests/ Workspace integration tests -├── integration.rs End-to-end CLI invocations -└── showcase_iosp.rs Before/after IOSP refactor demonstration -``` - -Companion test trees live next to the production code they cover -(`src//tests/.rs`). Workspace-root `tests/**` are Cargo's -integration-test binaries; each is its own crate. - -### How Classification Works - -1. **Trivial check**: Empty bodies are immediately `Trivial`. Single-statement bodies are analyzed — only classified as Trivial if they contain neither logic nor own calls. -2. **AST walking**: `BodyVisitor` implements `syn::visit::Visit` to walk the function body, recording: - - **Logic**: `if`, `match`, `for`, `while`, `loop`, binary operators (`+`, `&&`, `>`, etc.), optionally `?` operator - - **Own calls**: function/method calls that match names defined in the project (via `ProjectScope`) - - **Nesting depth**: tracks control-flow nesting for complexity metrics -3. **Classification**: - - Logic only → **Operation** - - Own calls only → **Integration** - - Both → **Violation** (with severity based on finding count) - - Neither → **Trivial** -4. **Recursion exception**: If `allow_recursion` is enabled and the only own call is to the function itself, it's classified as Operation instead of Violation. - -### ProjectScope: Solving the Method Call Problem - -Without type information, the analyzer cannot distinguish `self.push(x)` (Vec method, external) from `self.analyze(x)` (own method). The `ProjectScope` solves this with a two-pass approach: - -1. **First pass**: Scan all `.rs` files and collect every declared function, method, struct, enum, and trait name. -2. **Second pass**: During analysis, a call is only counted as "own" if the name exists in the project scope. - -This means `v.push(1)` is never counted as own (since `push` is not defined in your project), while `self.analyze_file(f)` is (because `analyze_file` is defined in your project). - -**Universal methods** (~26 entries like `new`, `default`, `fmt`, `clone`, `eq`, ...) are always treated as external, even if your project implements them via trait impls. This prevents false positives from standard trait implementations. - -### IOSP Score - -``` -IOSP Score = (Integrations + Operations) / (Integrations + Operations + Violations) × 100% +rustqual --save-baseline baseline.json +git add baseline.json && git commit -m "Add quality baseline" ``` -Trivial and suppressed functions are excluded because they are too small or explicitly allowed. - -## CI Integration - -### GitHub Actions - ```yaml -name: Quality Check -on: [push, pull_request] - -jobs: - quality: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - name: Install rustqual - run: cargo install --path . - - name: Check code quality - run: rustqual src/ --min-quality-score 90 --format github +- run: rustqual --compare baseline.json --fail-on-regression ``` -### GitHub Actions with Baseline - -```yaml -- name: Check quality regression - run: | - rustqual src/ --compare baseline.json --fail-on-regression --format github -``` +Full patterns: [book/ci-integration.md](./book/ci-integration.md). -### Generic CI (JSON) +## AI coding agent integration -```yaml -- name: Quality Check - run: | - cargo run --release -- src/ --json > quality-report.json - cat quality-report.json -``` +Drop this into `CLAUDE.md`, `.cursorrules`, `.github/copilot-instructions.md`, or whichever instruction file your tool reads: -### Pre-commit Hook +```markdown +## Code Quality Rules -```bash -#!/bin/bash -# .git/hooks/pre-commit -if ! cargo run --quiet -- src/ 2>/dev/null; then - echo "Quality findings detected. Please refactor before committing." - exit 1 -fi +- Run `rustqual` after making changes. All findings must be resolved before marking a task complete. +- Follow IOSP: every function is either an Integration or an Operation, never both. +- Keep functions under 60 lines and cognitive complexity under 15. +- Don't duplicate logic — extract shared patterns into reusable Operations. +- Don't introduce functions with more than 5 parameters. +- Every test function must contain at least one assertion. +- For public-API functions intentionally untested in this crate, mark with `// qual:api`. ``` -## How to Fix Violations +The agent gets actionable feedback: rustqual tells it which function violated which principle, so it can self-correct without you having to point each issue out. Full patterns: [book/ai-coding-workflow.md](./book/ai-coding-workflow.md). -When a function is flagged as a violation, refactor by splitting it into pure integrations and operations: +## Suppression annotations -**Before (violation):** -```rust -fn process(data: &Data) -> Result { - if data.value > threshold() { // logic + call mixed - transform(data) - } else { - default_output() - } -} -``` +For genuine exceptions: -**After (IOSP compliant):** ```rust -// Integration: orchestrates, no logic -fn process(data: &Data) -> Result { - let threshold = threshold(); - let exceeds = check_threshold(data.value, threshold); - select_output(exceeds, data) -} - -// Operation: logic only, no own calls -fn check_threshold(value: f64, threshold: f64) -> bool { - value > threshold -} - -// Integration: delegates to transform or default -fn select_output(exceeds: bool, data: &Data) -> Result { - if exceeds { transform(data) } else { default_output() } - // Note: this is still a violation! Further refactoring needed: - // Move the if-logic into an operation, call it from here. -} +// qual:allow(iosp) — match dispatcher; arms intentionally inlined +fn dispatch(cmd: Command) -> Result<()> { /* … */ } + +// qual:api — public re-export, callers live outside this crate +pub fn parse(input: &str) -> Result { /* … */ } + +// qual:test_helper — used only from integration tests +pub fn build_test_session() -> Session { /* … */ } ``` -**Common refactoring patterns:** +`max_suppression_ratio` (default 5%) caps how much code can be under `qual:allow`. Stale suppressions (no matching finding in their window) are flagged as `ORPHAN-001`. Full reference: [book/reference-suppression.md](./book/reference-suppression.md). -| Pattern | Approach | -|---------|----------| -| `if` + call in branch | Extract the condition into an Operation, use `.then()` or pass result to Integration | -| `for` loop with calls | Use iterator chains (`.iter().map(\|x\| process(x)).collect()`) — closures are lenient | -| Match + calls | Extract match logic into an Operation that returns an enum/value, dispatch in Integration | +## Output formats -Use `--suggestions` to get automated refactoring hints. +`--format ` — `text` (default), `json`, `github`, `sarif`, `dot`, `html`, `ai`, `ai-json`. Same analysis, different serialisation. Full reference: [book/reference-output-formats.md](./book/reference-output-formats.md). -## Self-Compliance +## Self-compliance -rustqual **analyzes itself** with zero findings across all seven dimensions: +rustqual analyses itself — the full source tree (~2.5k functions across all seven dimensions) reports `Quality Score: 100.0%` with zero findings and zero warnings: ```bash $ cargo run -- . --fail-on-warnings --coverage coverage.lcov ═══ Summary ═══ - Functions: 1805 Quality Score: 100.0% + Quality Score: 100.0% - IOSP: 100.0% (996I, 270O, 521T) + IOSP: 100.0% Complexity: 100.0% DRY: 100.0% SRP: 100.0% @@ -1336,59 +214,42 @@ $ cargo run -- . --fail-on-warnings --coverage coverage.lcov Test Quality:100.0% Architecture:100.0% - ~ All allows: 27 (qual:allow + #[allow]) - All quality checks passed! ✓ ``` -This is verified by the integration test suite and CI. Note: use `.` as -the analysis root (not `src/`) so that architecture-rule globs like -`src/adapters/**` match the actual paths. +Verified by the integration test suite and CI on every push. -## Testing +## Build & test ```bash -cargo nextest run # 1114 tests (1107 unit + 4 integration + 3 showcase) -RUSTFLAGS="-Dwarnings" cargo clippy --all-targets # lint check (0 warnings) +cargo nextest run # full test suite +cargo run -- . --fail-on-warnings --coverage coverage.lcov # self-analysis +RUSTFLAGS="-Dwarnings" cargo clippy --all-targets # lints (0 warnings) ``` -The test suite covers: -- **adapters/analyzers/** — classification, closures, iterators, scope, recursion, `?` operator, async/await, severity, complexity, IOSP/DRY/SRP/coupling/TQ/structural/architecture rule behaviour -- **adapters/config/** — ignore patterns, glob compilation, TOML loading, validation, tailored `--init` generation, weight sum check -- **adapters/report/** — summary stats, JSON structure, suppression counting, baseline roundtrip, HTML, SARIF, GitHub annotations, AI/TOON output -- **adapters/shared/** — cfg-test detection, use-tree walking, AST normalization -- **adapters/source/** — filesystem walk, `--watch` loop -- **app/** — pipeline orchestration, exit gates, setup, secondary-pass coordination, warning accumulation -- **domain/** + **ports/** — value-type invariants and trait-contract shape -- **Integration tests** (`tests/integration.rs`): self-analysis, sample expectations, JSON validity, verbose output -- **Showcase tests** (`tests/showcase_iosp.rs`): before/after IOSP refactoring examples - -## Known Limitations - -1. **Syntactic analysis only**: Uses `syn` for AST parsing without type resolution. Cannot determine the receiver type of method calls — relies on `ProjectScope` heuristics and `external_prefixes` config as fallbacks. -2. **Macros**: Macro invocations are not expanded. `println!` etc. are handled as special cases via `external_prefixes`, but custom macros producing logic or calls may be misclassified. -3. **External file modules**: `mod foo;` declarations pointing to separate files are not followed. Only inline modules (`mod foo { ... }`) are analyzed recursively. -4. **Parallelization**: The analysis pass is sequential because `proc_macro2::Span` (with `span-locations` enabled for line numbers) is not `Sync`. File I/O is parallelized via `rayon`. - -## Dependencies - -| Crate | Purpose | -|----------------|-------------------------------------------------| -| `syn` | Rust AST parsing (with `full`, `visit` features)| -| `proc-macro2` | Span locations for line numbers | -| `quote` | Token stream formatting (generic type display) | -| `derive_more` | `Display` derive for analysis types | -| `clap` | CLI argument parsing | -| `clap_complete`| Shell completion generation | -| `walkdir` | Recursive directory traversal | -| `colored` | Terminal color output | -| `serde` | Config deserialization | -| `toml` | TOML config file parsing | -| `serde_json` | JSON output serialization | -| `globset` | Glob pattern matching for ignore/exclude | -| `rayon` | Parallel file I/O | -| `notify` | File system watching for `--watch` mode | +## In use at + +- [rlm](https://github.com/SaschaOnTour/rlm) — Rust local memory manager. The reference adopter codebase that prompted the call-parity rule. +- [turboquant](https://github.com/SaschaOnTour/turboquant) — Rust quantitative finance toolkit (in active development). + +## Known limitations + +1. **Syntactic analysis only.** Uses `syn` for AST parsing. The receiver-type-inference engine (v1.2+) resolves most method-call receivers; what it can't resolve stays unresolved rather than being fabricated. +2. **Macros.** Macro invocations are not expanded. `println!` etc. are special-cased; custom macros producing logic or calls may be misclassified. Configurable via `[architecture.call_parity].transparent_macros`. +3. **External file modules.** `mod foo;` declarations pointing to separate files are not followed. Only inline modules (`mod foo { ... }`) are analysed recursively. +4. **Sequential analysis pass.** `proc_macro2::Span` (with `span-locations` enabled for line numbers) is not `Sync`. File I/O is parallelised via `rayon`. ## License -MIT +MIT. See [LICENSE](./LICENSE). + +## Contributing + +Bug reports and feature requests: open an issue at [github.com/SaschaOnTour/rustqual/issues](https://github.com/SaschaOnTour/rustqual/issues). For PRs: + +1. `cargo nextest run` — all tests must stay green. +2. `cargo run -- . --fail-on-warnings --coverage coverage.lcov` — the source tree must keep its 100% self-compliance score. +3. `RUSTFLAGS="-Dwarnings" cargo clippy --all-targets` — clippy must stay clean. +4. Update `CHANGELOG.md` for any user-visible change; bump `Cargo.toml` version on release-worthy contributions. + +The codebase is its own best reference for IOSP self-compliance and the architecture rules. The CLAUDE.md file documents internal conventions and common pitfalls. diff --git a/book/adapter-parity.md b/book/adapter-parity.md new file mode 100644 index 0000000..e66092b --- /dev/null +++ b/book/adapter-parity.md @@ -0,0 +1,157 @@ +# Use case: adapter parity (call parity) + +If your project has multiple frontends — a CLI, an MCP server, a REST API, a web UI — they're supposed to expose the same underlying capabilities. In theory, every CLI command has a matching MCP handler. In practice, it drifts. Someone adds a new application function, MCP picks it up, CLI forgets, and three months later you discover `cmd_export` exists in one adapter but not the other. + +**Call Parity makes adapter symmetry a CI-checkable property, not a code-review hope.** It's rustqual's most opinionated architecture rule, and one I haven't found a direct equivalent for in any other Rust static analyzer. + +## What it checks + +Configure adapter layers and a shared target. Minimal example with two adapters: + +```toml +[architecture.layers] +order = ["domain", "application", "cli", "mcp"] + +[architecture.layers.application] +paths = ["src/application/**"] + +[architecture.layers.cli] +paths = ["src/cli/**"] + +[architecture.layers.mcp] +paths = ["src/mcp/**"] + +[architecture.call_parity] +adapters = ["cli", "mcp"] +target = "application" +``` + +`adapters` can list any number of peer layers — REST endpoints, web handlers, gRPC servers, message-queue consumers — they're treated identically. + +Two checks run under one rule: + +- **Check A — every adapter must delegate.** Each `pub fn` in an adapter layer must (transitively) reach into the `target` layer. A CLI command that doesn't actually call into the application layer is logic in the wrong place. Caught at build time. +- **Check B — every target capability must reach all adapters.** Each `pub fn` in the `target` layer must be (transitively) reached from *every* adapter layer. Add `app::ingest::run`, forget to wire it into CLI, and Check B reports exactly that — by name, in CI, before review. + +`call_depth` (default 3) controls how many hops the transitive walk traces. + +## Why this is unusual + +Static analyzers traditionally fall into two camps: + +- **Style and local linters** (Clippy, ESLint, RuboCop) enforce per-function rules. They don't know your architecture. +- **Architecture linters** (ArchUnit, dependency-cruiser) enforce **containment**: "domain doesn't import adapters", "infrastructure doesn't depend on application". They prove what *can't* be called. + +Neither proves what **must** be called — that several adapter modules *collectively cover every public capability* of a target module. That requires building a real call graph across files, resolving method receivers through type aliases, wrappers, re-exports, and `Self`, then reverse-walking from each adapter to see what target functions it actually reaches. + +I haven't found another tool — for any language — that does this out of the box. The closest neighbours are general-purpose graph queries on top of CodeQL or Joern, where you write the analysis from scratch every time. If you know of one, I'd genuinely like to hear about it. + +## The hard part: an honest call graph + +The rule itself is simple. The work is making the call graph honest. Real Rust code looks like: + +```rust +let session = Session::open_cwd().map_err(map_err)?; +session.diff(path).map_err(map_err)?; +``` + +A naive analyzer sees `.diff()` on something it can't name and gives up — that turns into a false-positive "your CLI doesn't reach the application." rustqual ships a shallow type-inference engine that resolves receivers end-to-end: + +- Method-chain constructors and stdlib combinator returns (`Result::map_err`, `Option::ok`, `Future::await`, `Result::inspect`, …) +- Field access chains (`ctx.session.diff()`) +- Trait dispatch on `dyn Trait` and `impl Trait` (over-approximated to every workspace impl) +- Type aliases — including chains, wrappers (`Box`), and re-exports +- Renamed imports (`use std::sync::Arc as Shared;`) — with shadow detection so a local `crate::wrap::Arc` doesn't masquerade as stdlib +- `Self` substitution across all resolver paths so impl-internal delegation works + +Anything that can't be resolved cleanly stays unresolved — no fabricated edges. **False positives kill architectural rules**; missing an edge is recoverable, inventing one isn't. + +## Framework extractors and macro transparency + +Web frameworks wrap state in extractor types (`State`, `Data`, `Json`). Without help, the call graph stops at the extractor. Add them as transparent wrappers: + +```toml +[architecture.call_parity] +adapters = ["cli", "mcp"] +target = "application" +transparent_wrappers = ["State", "Extension", "Json", "Data"] +transparent_macros = ["tracing::instrument", "async_trait::async_trait"] +``` + +Now `fn h(State(db): State) { db.query() }` resolves through the `State` peel and the `Db::query` edge is recorded. + +The default `transparent_macros` list already covers the common cases; entries here extend it. + +## What you'll see + +``` +✗ ARCH-CALL-PARITY src/cli/commands/sync.rs::cmd_sync (Check A) + pub fn does not (transitively, depth=3) reach the target layer + 'application' — adapter has no delegation path + +✗ ARCH-CALL-PARITY src/application/export.rs::run_export (Check B) + target fn is unreached by adapter 'cli' + (reachable from: mcp) +``` + +The first finding says "this CLI command does logic locally instead of delegating". The second says "you added a new application capability and forgot to expose it via CLI". + +## Excluding legitimate asymmetries + +Sometimes a target function genuinely shouldn't have an adapter for every frontend — debug endpoints, admin-only tooling, internal setup. Use `exclude_targets`: + +```toml +[architecture.call_parity] +adapters = ["cli", "mcp"] +target = "application" +exclude_targets = [ + "application::admin::*", # admin tools, not exposed via either adapter + "application::setup::run", # bootstrap, called once at startup +] +``` + +Globs match against the *module path* (with `crate::` stripped), not the layer name. `application::admin::*` matches every `pub fn` under `src/application/admin/**`. + +For ad-hoc per-function suppression: + +```rust +// qual:allow(architecture) — internal capability, intentionally MCP-only +pub fn admin_purge() { /* … */ } +``` + +## Why the false-positive rate matters + +False positives don't just waste reviewer time, they *teach the team to ignore the tool*. The whole call-parity approach only works if the false-positive rate stays close to zero — which is why the receiver-type-inference engine refuses to fabricate edges. An honest "I don't know" beats a confident wrong answer when the rule is going to fail builds. + +## For teams using AI coding assistants + +If you're building Rust with Copilot, Claude, Codex, or similar: this rule guards against one of the more common patterns of architectural drift in AI-assisted codebases. When an agent adds `pub fn export_csv()` to your application layer, it tends to wire it into one frontend and forget the others. Check B catches that on the next `cargo` run — before the PR — without you having to write a custom prompt or review checklist. + +Combined with rustqual's other architecture rules (layers, forbidden edges, trait contracts), this gives any LLM agent a *structural* feedback loop that's stricter and more reliable than narrative architectural documentation in a repo's README. + +## Try it + +```toml +# rustqual.toml +[architecture] +enabled = true +[architecture.layers] +order = ["domain", "application", "cli", "mcp"] +# ... layer paths ... + +[architecture.call_parity] +adapters = ["cli", "mcp"] +target = "application" +``` + +```sh +cargo install rustqual +rustqual . --fail-on-warnings +``` + +## Related + +- [architecture-rules.md](./architecture-rules.md) — the broader architecture dimension (layers, forbidden edges, patterns, trait contracts) +- [ai-coding-workflow.md](./ai-coding-workflow.md) — why call parity especially matters for AI-generated code +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-configuration.md](./reference-configuration.md) — every config option diff --git a/book/ai-coding-workflow.md b/book/ai-coding-workflow.md new file mode 100644 index 0000000..f327606 --- /dev/null +++ b/book/ai-coding-workflow.md @@ -0,0 +1,115 @@ +# Use case: AI-assisted Rust development + +This is what rustqual was originally built for. AI coding agents — Claude Code, Cursor, GitHub Copilot, Codex — are productive but consistently produce a recognisable set of structural smells. rustqual catches them mechanically so the agent can self-correct, without you having to spot every issue in code review. + +## What AI agents tend to get wrong + +- **God-functions** — functions that mix orchestration with logic ("call helper, then if/else, then call another helper, then …"). Hard to test, hard to read, hard to refactor. +- **Long functions with deep nesting** — agents err on the side of inlining everything they need. Cognitive complexity climbs fast. +- **Copy-paste with minor variation** — when asked to "do the same for X", agents often copy the implementation rather than extracting a shared abstraction. +- **Tests without assertions** — agents generate test bodies that *exercise* code without *checking* it. Coverage looks good, real coverage is zero. +- **Architectural drift** — adding code "wherever it fits" instead of respecting the project's layering. The domain layer slowly imports adapters, infrastructure leaks into application, etc. +- **Asymmetric adapters** — when a project has multiple frontends (CLI, REST, MCP), agents tend to wire new functionality into the one they're touching and forget the others. + +rustqual catches all of these. The trick is wiring it into the agent's feedback loop so it self-corrects. + +## Pattern 1: agent instruction file + +Drop this into `CLAUDE.md`, `.cursorrules`, `.github/copilot-instructions.md`, or whichever instruction file your tool reads: + +```markdown +## Code Quality Rules + +- Run `rustqual` after making changes. All findings must be resolved before marking a task complete. +- Follow IOSP: every function is either an Integration (calls other functions, no own logic) or an Operation (contains logic, no calls to project functions). Never mix both. +- Keep functions under 60 lines and cognitive complexity under 15. +- Don't duplicate logic — extract shared patterns into reusable Operations. +- Don't introduce functions with more than 5 parameters. +- Every test function must contain at least one assertion (`assert!`, `assert_eq!`, etc.). +- For public-API functions that are intentionally untested in this crate, mark with `// qual:api` instead of writing a stub test. +``` + +The agent gets actionable feedback: rustqual tells it which function violated which principle, so it can self-correct without you having to point each issue out. + +## Pattern 2: pre-commit hook + +Catch violations before they enter version control — useful when the agent runs locally: + +```bash +#!/bin/bash +# .git/hooks/pre-commit +if ! rustqual 2>/dev/null; then + echo "rustqual: quality findings detected. Refactor before committing." + exit 1 +fi +``` + +Make it executable: `chmod +x .git/hooks/pre-commit`. + +This gives the agent immediate feedback before anything reaches the remote. If you're using Claude Code with hooks (`PostToolUse`), you can wire `rustqual` into the same loop: every Edit triggers a re-check. + +## Pattern 3: CI quality gate + +```yaml +# .github/workflows/quality.yml +name: Quality Check +on: [pull_request] + +jobs: + quality: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - run: cargo install rustqual cargo-llvm-cov + - run: cargo llvm-cov --lcov --output-path lcov.info + - run: rustqual --diff HEAD~1 --coverage lcov.info --format github +``` + +`--format github` produces inline annotations on the PR diff — exactly where the issue is, what rule fired, why it matters. `--diff HEAD~1` restricts analysis to the changed files so PRs stay fast even on large codebases. + +## Pattern 4: baseline tracking for AI-velocity codebases + +If you have a codebase already at the limit of what you can refactor right now, but you want to make sure new AI-generated code doesn't make it worse: + +```bash +# Snapshot the current state +rustqual --save-baseline baseline.json + +# In CI: fail only on regression +rustqual --compare baseline.json --fail-on-regression +``` + +This lets you ratchet quality up over time without blocking PRs that don't make things worse. Combined with `--min-quality-score 90`, you get a hard floor plus a no-regression rule — exactly what you want when an agent is generating dozens of PRs a week. + +## Why IOSP specifically + +The Integration/Operation distinction is what separates rustqual from a generic linter for AI-coding contexts. AI agents naturally produce mixed-concern functions — they don't have an internal pressure to decompose. IOSP makes that pressure mechanical: the agent writes a god-function, rustqual marks it as a violation, the agent reads the finding, splits the function. Repeat until the loop converges on small, single-purpose functions. + +Without that constraint, agents settle into "works but unmaintainable" code that passes tests, passes clippy, and rots over six months. With it, the agent is structurally pushed toward decomposition every time. + +## Suppression for legitimate exceptions + +Not every violation is a bug. Use `// qual:allow` annotations sparingly: + +```rust +// qual:allow(iosp) — match dispatcher; splitting would just rename the match +fn dispatch(cmd: Command) -> Result<()> { + match cmd { + Command::Sync => sync_handler(), + Command::Diff => diff_handler(), + // … + } +} +``` + +The `max_suppression_ratio` config (default 5%) caps how much code can be suppressed. If the agent suppresses too much, that itself becomes a finding. + +Full annotation reference: [reference-suppression.md](./reference-suppression.md). + +## Related + +- [function-quality.md](./function-quality.md) — what IOSP/complexity actually check +- [test-quality.md](./test-quality.md) — assertion density, coverage, untested functions +- [legacy-adoption.md](./legacy-adoption.md) — applying this to a codebase that's already grown messy +- [ci-integration.md](./ci-integration.md) — full CI patterns diff --git a/book/architecture-rules.md b/book/architecture-rules.md new file mode 100644 index 0000000..4a9b305 --- /dev/null +++ b/book/architecture-rules.md @@ -0,0 +1,212 @@ +# Use case: architecture rules + +The Architecture dimension is rustqual's "I want this codebase to look like *that* in five years" enforcement layer. Where IOSP, complexity, and SRP catch *local* smells, architecture rules catch the *global* drift — the kind that turns a clean hexagonal design into a tangle of cross-imports over six months. + +Architecture rules are config-driven. You write them once in `rustqual.toml`, and they apply on every analysis run. If a PR violates a rule, CI fails. There's no "I'll fix it later" — the rule is the source of truth. + +## What you can enforce + +- **Layers** — domain → ports → infrastructure → application. Inner layers can't import outer ones. +- **Forbidden edges** — `analyzer X` cannot import `analyzer Y`, regardless of layer. Specific cross-cuts. +- **Symbol patterns** — where specific paths/macros/methods are allowed (e.g., `syn::` only in adapters, `unwrap()` only in tests, `println!` only in CLI). +- **Trait contracts** — every port trait must be object-safe, `Send + Sync`, and not leak adapter error types. + +These are independent rule families. You can use any combination. Most projects start with layers, add forbidden edges as they discover specific cross-cuts to forbid, and add symbol patterns when a particular antipattern keeps coming back. + +## Layers + +The defining structural rule. Inner layers know nothing of outer ones; outer layers can use inner ones freely. + +```toml +[architecture.layers] +order = ["domain", "ports", "infrastructure", "analysis", "application"] +unmatched_behavior = "strict_error" # files outside any layer fail the dim + +[architecture.layers.domain] +paths = ["src/domain/**"] + +[architecture.layers.ports] +paths = ["src/ports/**"] + +[architecture.layers.infrastructure] +paths = ["src/adapters/config/**", "src/adapters/source/**"] + +[architecture.layers.analysis] +paths = ["src/adapters/analyzers/**", "src/adapters/report/**"] + +[architecture.layers.application] +paths = ["src/app/**"] +``` + +A `domain` file importing from `application` fails. An `application` file importing from `domain` is fine. Same-layer imports are always fine. + +### `unmatched_behavior` + +Three options: + +- `"strict_error"` — every production file must match a layer (hard finding otherwise). Recommended; flags new files dropped in arbitrary locations. +- `"composition_root"` — unmatched files act as the composition root and may import any layer. +- `"warn"` — soft warning instead of error. + +### Re-export points + +Some files (`lib.rs`, `main.rs`, `bin/**`, `cli/**`, `tests/**`) live at the root and re-export from every layer. Mark them explicitly: + +```toml +[architecture.reexport_points] +paths = ["src/lib.rs", "src/main.rs", "src/bin/**", "src/cli/**", "tests/**"] +``` + +### Workspaces + +For multi-crate workspaces, map external crates to layers: + +```toml +[architecture.external_crates] +my_domain_types = "domain" +my_port_traits = "ports" +``` + +Now `infrastructure` can import `my_domain_types` (lower-rank, allowed) but not the other way around. + +## Forbidden edges + +Where layers are too coarse, forbidden edges name specific source/destination pairs: + +```toml +[[architecture.forbidden]] +from = "src/adapters/analyzers/iosp/**" +to = "src/adapters/analyzers/**" +except = ["src/adapters/analyzers/iosp/**"] +reason = "Dimension analyzers don't know each other" + +[[architecture.forbidden]] +from = "src/adapters/**" +to = "src/app/**" +reason = "Adapters know domain + ports, not application" +``` + +Each rule has `from`, `to`, an optional `except` list, and a human-readable `reason` that shows up in the finding. + +## Symbol patterns + +The most flexible family — you can forbid specific path prefixes, method calls, macro calls, or glob imports in specific directories: + +```toml +# AST types only in adapters +[[architecture.pattern]] +name = "no_syn_in_domain" +forbid_path_prefix = ["syn::", "proc_macro2::", "quote::"] +forbidden_in = ["src/domain/**"] +reason = "Domain has no AST representation" + +# unwrap()/expect() only in tests +[[architecture.pattern]] +name = "no_panic_helpers_in_production" +forbid_method_call = ["unwrap", "expect"] +forbidden_in = ["src/**"] +except = ["**/tests/**"] +reason = "Production propagates errors typed instead of panicking" + +# println!/print!/dbg! only in CLI/binaries +[[architecture.pattern]] +name = "no_stdout_in_library_code" +forbid_macro_call = ["println", "print", "dbg"] +forbidden_in = ["src/**"] +allowed_in = ["src/main.rs", "src/bin/**", "src/cli/**"] +reason = "stdout is the CLI's channel, not library code's" + +# No glob imports in domain +[[architecture.pattern]] +name = "no_glob_imports_in_domain" +forbid_glob_import = true +forbidden_in = ["src/domain/**"] +reason = "Glob imports hide layer tunneling" +``` + +Each rule carries `forbidden_in` (where it fires) and optional `allowed_in`/`except` (where it's exempted). The `reason` field is mandatory and shows up in the finding so reviewers know *why*. + +## Trait contracts + +The most prescriptive family — used to keep port traits clean: + +```toml +[[architecture.trait_contract]] +name = "port_traits" +scope = "src/ports/**" + +receiver_may_be = ["shared_ref"] # only &self, no &mut self / self +forbidden_return_type_contains = [ + "anyhow::", "Box` instead of a typed error. + +Plus the structural binary check `BTC` (broken trait contract) flags impls that are entirely stubs (`unimplemented!`, `todo!`, `Default::default()` only). + +## What you'll see + +``` +✗ ARCH-LAYER src/domain/order.rs imports src/adapters/source/io.rs + domain (rank 0) cannot import infrastructure (rank 2) + +✗ ARCH-FORBID src/adapters/analyzers/iosp/visitor.rs imports + src/adapters/analyzers/dry/mod.rs + reason: Dimension analyzers don't know each other + +✗ ARCH-PATTERN src/auth/session.rs uses unwrap() (line 88) + rule: no_panic_helpers_in_production + reason: Production propagates errors typed instead of panicking + +✗ ARCH-TRAIT src/ports/storage.rs trait Storage::write returns anyhow::Result + rule: port_traits + reason: forbidden_return_type_contains: anyhow:: +``` + +## Diagnostic mode + +`rustqual --explain src/some/file.rs` prints which layer the file matches, which symbol rules apply, and what would change if you moved it. Useful when you can't tell why a file is failing. + +## Configure + +```toml +[architecture] +enabled = true +# Then add layers, forbidden edges, patterns, trait_contract sections +``` + +`--init` doesn't generate architecture rules — they require an opinion about your design that the tool can't infer. Add them manually, ratchet up over time. + +## Suppression + +Architecture is suppression-resistant by design. The `// qual:allow(architecture)` annotation works at the import site or item, but it counts hard against `max_suppression_ratio`, and you should leave a `reason:` rationale in the comment block: + +```rust +// qual:allow(architecture) — port adapter must call into the registry directly +// here for serialization round-trip; pure domain accessor would lose ordering. +use crate::adapters::registry::lookup; +``` + +The right answer is usually to widen the rule (with a clear `except` clause) or move the file, not to suppress. + +## Why this is unusual + +Most static analyzers ship per-function rules and stop there. Architecture linters (ArchUnit, dependency-cruiser) prove what *can't* be called. rustqual's architecture dimension does both directions: + +- **Negative space** (forbidden edges, layer rules, symbol patterns) — what mustn't happen. +- **Positive space** (call parity — see [adapter-parity.md](./adapter-parity.md)) — what *must* happen across multiple adapters. + +The combination is what makes drift mechanically detectable rather than review-dependent. + +## Related + +- [adapter-parity.md](./adapter-parity.md) — call parity, the architecture rule that's unique to rustqual +- [coupling-quality.md](./coupling-quality.md) — metric-based coupling (instability, SDP) +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-configuration.md](./reference-configuration.md) — every config option diff --git a/book/ci-integration.md b/book/ci-integration.md new file mode 100644 index 0000000..41aa30f --- /dev/null +++ b/book/ci-integration.md @@ -0,0 +1,156 @@ +# Use case: CI/CD integration + +Run rustqual on every push or pull request. The defaults make this easy — the binary already exits with code `1` on any finding, so a single `run:` line in CI is enough for a hard quality gate. + +## GitHub Actions — minimal + +```yaml +name: Quality Check +on: [push, pull_request] + +jobs: + quality: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - run: cargo install rustqual + - run: rustqual +``` + +That's all. If any dimension flags a finding, the job fails. + +## GitHub Actions — with inline PR annotations + +```yaml +- run: rustqual --format github --min-quality-score 90 +``` + +`--format github` emits `::error::` and `::warning::` annotations that GitHub renders inline on the PR diff. `--min-quality-score 90` enforces a hard floor on the overall quality score. + +## GitHub Actions — changed files only + +For large codebases, restrict analysis to files changed in the PR: + +```yaml +- run: rustqual --diff origin/main --format github +``` + +`--diff ` runs the same analysis but only reports findings in files that differ from ``. Cuts CI time for large codebases without losing PR-level signal. + +## GitHub Actions — with coverage + +The Test Quality dimension can use LCOV coverage data to detect untested logic: + +```yaml +- run: cargo install rustqual cargo-llvm-cov +- run: cargo llvm-cov --lcov --output-path lcov.info +- run: rustqual --coverage lcov.info --format github +``` + +This enables TQ-005 (uncovered logic detection) on top of the static checks (assertion-free tests, no-SUT tests, untested public functions). + +## GitHub Actions — baseline comparison + +For codebases that aren't yet at 100% but want to prevent regression: + +```yaml +- run: rustqual --compare baseline.json --fail-on-regression --format github +``` + +The job fails only when the quality score drops or new findings appear. New code can't make things worse, but you don't have to fix everything before the next merge. + +Generate the baseline once and commit it: + +```bash +rustqual --save-baseline baseline.json +git add baseline.json && git commit -m "Add quality baseline" +``` + +Update it intentionally as part of refactor PRs. + +## SARIF for GitHub Code Scanning + +```yaml +- run: rustqual --format sarif > rustqual.sarif +- uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: rustqual.sarif +``` + +Findings appear in the **Security** tab as Code Scanning alerts, with rule IDs for every dimension (IOSP, complexity, coupling, DRY, SRP, test quality, architecture). + +## Other CI systems + +rustqual has no GitHub-specific dependencies — it's a regular Rust binary. For GitLab, CircleCI, Jenkins, etc., the only difference is which output format you want: + +- `--format json` — pipe to whatever tool you have +- `--format html` — self-contained HTML report you can publish as an artifact +- Default text output — printed to stdout + +Example GitLab snippet: + +```yaml +quality: + image: rust:latest + script: + - cargo install rustqual + - rustqual --format json > quality.json + artifacts: + paths: + - quality.json +``` + +## Pre-commit hook + +For local enforcement before code reaches CI: + +```bash +#!/bin/bash +# .git/hooks/pre-commit +if ! rustqual 2>/dev/null; then + echo "rustqual: quality findings detected. Refactor before committing." + exit 1 +fi +``` + +`chmod +x .git/hooks/pre-commit` to enable. + +## Quality gates in practice + +The flags compose. A typical "production-grade" CI step: + +```yaml +- run: | + rustqual \ + --diff origin/main \ + --coverage lcov.info \ + --min-quality-score 90 \ + --fail-on-warnings \ + --format github +``` + +This: +- analyses only files changed vs `main`, +- includes coverage-based test-quality checks, +- requires the overall quality score to be at least 90%, +- treats suppression-ratio overruns as hard errors, +- emits inline PR annotations. + +`--fail-on-warnings` is worth knowing: by default, exceeding `max_suppression_ratio` (5% of functions suppressed) emits a warning but doesn't fail. With this flag, it does. + +## Exit codes + +| Code | Meaning | +|---|---| +| `0` | Success — no findings, or `--no-fail` set | +| `1` | Quality findings; or regression vs baseline (`--fail-on-regression`); or score below threshold (`--min-quality-score`); or warnings present (`--fail-on-warnings`) | +| `2` | Configuration error — invalid or unreadable `rustqual.toml` | + +Full flag reference: [reference-cli.md](./reference-cli.md). + +## Related + +- [ai-coding-workflow.md](./ai-coding-workflow.md) — patterns specific to AI-generated code +- [legacy-adoption.md](./legacy-adoption.md) — onboarding rustqual on existing codebases +- [reference-output-formats.md](./reference-output-formats.md) — every format with examples diff --git a/book/code-reuse.md b/book/code-reuse.md new file mode 100644 index 0000000..736d836 --- /dev/null +++ b/book/code-reuse.md @@ -0,0 +1,173 @@ +# Use case: code reuse (DRY, dead code, boilerplate) + +DRY findings are usually the highest-signal category in the first run. They're concrete: this function looks like that function, this code is never called, this `From` impl can be `derive`d. Easy to act on, easy to verify, big quality wins for not much work. + +rustqual covers four families: + +- **Duplicate functions / fragments / match patterns** — same code in multiple places. +- **Dead code** — defined but never called. +- **Wildcard imports** — `use foo::*;` hiding what's actually pulled in. +- **Boilerplate** — patterns the compiler/macros could write for you. + +## What goes wrong + +- Two functions that are 90% identical with one parameter-change. Someone copied instead of extracting. +- A helper that nobody calls anymore (refactor leftover, or never wired up). +- `use foo::*;` where you only need `foo::Bar` — you've imported 30 names you don't use, and added 30 hidden coupling points. +- Hand-written `impl Display for FooError` that just maps each variant to a string — `#[derive(thiserror::Error)]` does it automatically. + +## What rustqual catches + +| Rule | Meaning | +|---|---| +| `DRY-001` | Two functions are duplicates (95%+ token similarity, one is suggested for removal) | +| `DRY-002` | Dead code — function defined but never called | +| `DRY-003` | Duplicate code fragment (≥6 lines repeated across functions) | +| `DRY-004` | Wildcard import (`use module::*;`) | +| `DRY-005` | Repeated match pattern across functions (≥3 arms identical, ≥3 instances) | +| `BP-001` | Trivial `From` impl (derivable) | +| `BP-002` | Trivial `Display` impl | +| `BP-003` | Trivial getter/setter (consider field visibility) | +| `BP-004` | Builder pattern — consider `derive_builder` or similar | +| `BP-005` | Manual `Default` impl (derivable) | +| `BP-006` | Repetitive match mapping | +| `BP-007` | Error-enum boilerplate (consider `thiserror`) | +| `BP-008` | Clone-heavy conversion | +| `BP-009` | Struct-update boilerplate | +| `BP-010` | Format-string repetition | + +## Duplicates + +`DRY-001` uses token-based similarity with a 95% threshold by default. It's deliberately strict — finding fewer, higher-confidence duplicates is more useful than spamming "these two functions both call `.unwrap()`". When it fires, it tells you: + +- The two functions involved. +- Which one to keep (typically the older / more public). +- The token-level diff between them. + +For inverse-method pairs (encode/decode, serialize/deserialize) where structural duplication is intentional: + +```rust +// qual:inverse(decode) +pub fn encode(input: &Value) -> String { /* … */ } + +// qual:inverse(encode) +pub fn decode(input: &str) -> Value { /* … */ } +``` + +This suppresses the DRY-001 finding without counting against the suppression ratio. + +## Dead code + +`DRY-002` builds a workspace-wide call graph. A function that nobody calls — and isn't a `pub` API entry point — is dead code. + +Two important escape hatches: + +```rust +// qual:api — public re-export, callers live outside this crate +pub fn parse_config(input: &str) -> Result { /* … */ } + +// qual:test_helper — only used from tests/, not from src/ +pub fn assert_in_range(actual: f64, expected: f64, tol: f64) { /* … */ } +``` + +`qual:api` and `qual:test_helper` exclude the function from `DRY-002` *and* from `TQ-003` (untested), without counting against `max_suppression_ratio`. Use them on functions that are exported to consumers outside the crate or used only by integration tests. + +By default, the dead-code analysis treats workspace-root `tests/**` files as call-sites — so a function used only from integration tests is not dead. + +## Code fragments and repeated matches + +`DRY-003` finds ≥6-line blocks repeated across functions — usually setup boilerplate that should be a helper, or assertion patterns that should be a test utility. + +`DRY-005` finds repeated `match` blocks: identical arms, ≥3 of them, in ≥3 different functions. Classic case is dispatching the same enum to slightly different methods in five places — extract a helper. + +## Wildcard imports + +`DRY-004` flags every `use foo::*;`. Wildcards hide: + +- Which symbols you actually depend on. +- Layer-tunneling (a wildcard re-export can pull in domain types into adapters without it being visible). +- Name collisions when the upstream module adds new symbols. + +Replace with explicit imports. If a `prelude::*` is unavoidable (some crates require it), suppress narrowly: + +```rust +// qual:allow(dry) — diesel requires this prelude for query DSL +use diesel::prelude::*; +``` + +## Boilerplate + +The `BP-*` rules detect patterns where the compiler or a derive macro could write the code for you. Each finding includes a suggested replacement. Examples: + +- `BP-001` `impl From for B { fn from(a: A) -> B { B { x: a.x, y: a.y } } }` → `#[derive(...)]` or struct shorthand. +- `BP-005` Manual `impl Default` that just calls every field's default → `#[derive(Default)]`. +- `BP-007` Hand-written error enum with `From` impls and `Display` mapping → `#[derive(thiserror::Error)]`. + +These rarely require thinking — just apply the suggestion and move on. Disable the boilerplate dimension if your project has a reason to avoid derive macros: + +```toml +[boilerplate] +enabled = false +``` + +## Configure thresholds + +```toml +[duplicates] +enabled = true +# similarity_threshold = 0.95 +# min_function_lines = 6 + +[boilerplate] +enabled = true +``` + +Most thresholds are tuned to be opinionated by default. Loosen them via `--init` if you want them calibrated to your current codebase metrics. + +## What you'll see + +``` +✗ DRY-001 src/api/users.rs::format_user (line 88) + duplicate of src/api/orders.rs::format_order (96% similarity) + +✗ DRY-002 src/utils/legacy.rs::old_helper (line 12) — dead code + +⚠ DRY-004 src/api/handlers.rs uses `use db::*;` (line 4) + +✗ BP-001 src/error.rs impl From for AppError — derivable +``` + +## Suppression + +For genuine cases where suppression is right: + +```rust +// qual:allow(dry) — keeping this duplicate temporarily; consolidating in PR-345 +fn old_path() { /* … */ } + +// qual:api — public-API entry, callers outside this crate +pub fn parse(input: &str) -> Result { /* … */ } + +// qual:test_helper — used only from integration tests +pub fn build_test_config() -> Config { /* … */ } + +// qual:inverse(decode) +fn encode(v: &Value) -> Vec { /* … */ } +``` + +`qual:api`, `qual:test_helper`, and `qual:inverse` don't count against `max_suppression_ratio`. `qual:allow(dry)` does. + +Full annotation reference: [reference-suppression.md](./reference-suppression.md). + +## Why this matters for AI-generated code + +AI agents are particularly prone to copy-paste-with-variation: when asked to "do the same for X", they tend to copy the existing implementation rather than extract a shared abstraction. `DRY-001` and `DRY-003` catch that mechanically. After a few cycles of `rustqual` flagging duplicates, the agent learns to extract; the codebase stays denormalised. + +`DRY-002` is the other half of that loop — agents sometimes generate helpers that go unused because they pivoted mid-task. Catching dead code at PR time prevents accumulation. + +## Related + +- [function-quality.md](./function-quality.md) — IOSP, complexity (where duplication often lives) +- [test-quality.md](./test-quality.md) — `TQ-003` (untested) shares the call graph with `DRY-002` +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-suppression.md](./reference-suppression.md) — `qual:api`, `qual:test_helper`, `qual:inverse` diff --git a/book/coupling-quality.md b/book/coupling-quality.md new file mode 100644 index 0000000..3553b0f --- /dev/null +++ b/book/coupling-quality.md @@ -0,0 +1,102 @@ +# Use case: coupling + +Coupling is what makes a refactor cost a week instead of an hour. Two modules that import each other can't be tested independently, can't be deployed independently, and tend to drift toward a single mega-module over time. rustqual measures coupling at the module level and flags the patterns that historically lead to architecture decay. + +## What goes wrong + +- **Circular dependencies** — `a` imports `b`, `b` imports `a`. Either directly or through a chain. Fundamentally breaks layering. +- **Unstable cores** — modules that are heavily depended on (high fan-in) but also depend on a lot of other modules (high fan-out). Every change ripples; nothing is safe to touch. +- **Stable Dependencies Principle violations** — a stable module (lots of incoming deps) depends on an unstable one (few incoming, many outgoing). Should be the other way around. +- **Orphaned impl blocks** — `impl Foo` lives in a different file than `struct Foo`. Useful occasionally; usually a "I'll move it later" smell. +- **Single-impl traits** — a non-public trait with exactly one impl. The trait isn't an abstraction, it's a stub waiting to be inlined. +- **Downcast escape hatches** — `Any::downcast`/`downcast_ref`. Almost always means a missing enum or trait method. +- **Inconsistent error types** — one module returns `Result<_, MyError>` from half its functions and `Result<_, anyhow::Error>` from the other half. + +## What rustqual catches + +| Rule | Meaning | +|---|---| +| `CP-001` | Circular module dependency | +| `CP-002` | Stable Dependencies Principle violation — a stable module depends on a less stable one | +| `OI` | Orphaned impl: `impl Foo` block in a different file from `struct Foo` | +| `SIT` | Single-impl trait: non-`pub` trait with exactly one implementation | +| `DEH` | Downcast escape hatch: use of `Any::downcast` | +| `IET` | Inconsistent error types within a module | + +`OI`, `SIT`, `DEH`, `IET` are part of the **structural binary checks** under the Coupling dimension. They each fire as a single binary signal per module, which is why they don't have numeric thresholds. + +### Instability metric + +For each module, rustqual computes `I = fan_out / (fan_in + fan_out)`: + +- `I = 0` — purely depended-upon, depends on nothing (stable: domain types). +- `I = 1` — depends on everything, nothing depends on it (unstable: top-level orchestration). + +The Stable Dependencies Principle says *dependencies should flow toward stability* — stable modules at the bottom, unstable ones at the top. `CP-002` fires when a stable module imports an unstable one, which inverts the dependency direction. + +## Configure thresholds + +```toml +[coupling] +enabled = true +max_instability = 0.8 # warn above this +max_fan_in = 15 +max_fan_out = 12 +check_sdp = true # disable SDP check if you don't want it + +[structural] +enabled = true +check_oi = true +check_sit = true +check_deh = true +check_iet = true +``` + +`--init` calibrates `max_fan_in` and `max_fan_out` to your current codebase metrics. + +## What you'll see + +``` +✗ CP-001 src/auth/session.rs ↔ src/auth/token.rs (cycle of length 2) +✗ CP-002 src/domain/user.rs depends on src/api/handlers.rs (I=0.91) +⚠ OI impl Order in src/orders/persistence.rs (struct in src/orders/types.rs) +⚠ SIT trait OrderValidator (1 impl: BasicOrderValidator) +⚠ DEH downcast in src/registry/lookup.rs (line 88) +⚠ IET src/payment/mod.rs uses MyError (4×) and anyhow::Error (3×) +``` + +`--format dot` produces a Graphviz module dependency graph — useful for visualising cycles or eyeballing dependency direction. + +## Refactor patterns + +**Cycle**: usually one of the two modules has a function that belongs in the other. Move the function and the cycle dissolves. If both modules genuinely need each other, extract the shared types into a third module they both depend on. + +**SDP violation**: invert the dependency — typically by introducing a trait in the stable module that the unstable one implements. The stable module then knows nothing about the unstable one. + +**Orphaned impl**: move the `impl` block next to the type definition, or — if it's a trait impl that *can't* live there (orphan rules) — accept it and add `// qual:allow(coupling)`. + +**Single-impl trait**: inline the trait. Most "interface for testability" cases can be replaced by direct dependency injection of the concrete type, or by a trait that has more than one real impl somewhere in the codebase. + +**Downcast**: replace with an enum. If you find yourself reaching for `Any::downcast`, the type system is telling you the cases were never enumerated. + +**Inconsistent errors**: pick one. Either every public function in the module returns `MyError` (with `From` impls for upstream errors), or every public function returns `anyhow::Error`. Don't mix. + +## Suppression + +Coupling warnings are *module-level*, not function-level. The `qual:allow(coupling)` annotation on a single function doesn't silence them — that's intentional. To suppress a coupling finding for a whole module: + +```rust +// At the top of the module file: +//! qual:allow(coupling) — orchestration layer, intentionally depends on every adapter. +``` + +Inner doc-comment form (`//!`) attaches to the module, not to a single item. + +For structural-binary checks (`OI`, `SIT`, `DEH`, `IET`) which target specific items, use `// qual:allow(coupling)` at the impl/trait/use site. + +## Related + +- [architecture-rules.md](./architecture-rules.md) — explicit layer rules instead of metrics +- [module-quality.md](./module-quality.md) — within-module structure (SRP) +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-configuration.md](./reference-configuration.md) — every config option diff --git a/book/function-quality.md b/book/function-quality.md new file mode 100644 index 0000000..ef8af76 --- /dev/null +++ b/book/function-quality.md @@ -0,0 +1,163 @@ +# Use case: function-level quality + +Function quality is where most code rot starts. A function that mixes orchestration with logic, or that grows past 60 lines with deep nesting, is the smell that everything else builds on. rustqual catches these mechanically — through IOSP, complexity metrics, and method-shape checks — so the feedback loop runs at every change instead of every six months. + +## IOSP — Integration vs Operation + +Every function should be either: + +- **Integration** — orchestrates other functions. No own logic (no `if`, `for`, `match`, arithmetic, etc.). Just calls and assignments. +- **Operation** — contains logic. No calls into your own project's functions (stdlib and external crates are fine). + +A function that does both is a **Violation** — that's the smell to fix. Splitting it gives you one Integration and one or more Operations, each with a single purpose. + +```rust +// Violation: mixes orchestration and logic +pub fn process_order(order: Order) -> Result { + let total = order.items.iter().map(|i| i.price).sum::(); // logic + let discount = if order.is_premium { 0.1 } else { 0.0 }; // logic + let final_total = total * (1.0 - discount); // logic + let receipt = generate_receipt(&order, final_total)?; // call + save_receipt(&receipt)?; // call + Ok(receipt) +} + +// After refactor: one Integration, two Operations +pub fn process_order(order: Order) -> Result { + let total = calculate_total(&order); + let receipt = generate_receipt(&order, total)?; + save_receipt(&receipt)?; + Ok(receipt) +} + +fn calculate_total(order: &Order) -> f64 { + let raw = order.items.iter().map(|i| i.price).sum::(); + let discount = if order.is_premium { 0.1 } else { 0.0 }; + raw * (1.0 - discount) +} +``` + +### Leniency rules + +Out of the box, IOSP is forgiving where it matters: + +- **Closures and async blocks** don't count as logic. `.map(|x| x.foo())` is fine inside an Integration. +- **For-loops over delegation** — a `for x in xs { handler(x) }` is treated as orchestration, not logic. +- **Match-as-dispatch** — a `match` whose every arm is a single delegation call is orchestration. Add a guard or any logic to one arm and it becomes a Violation. +- **Trivial self-getters** (`fn name(&self) -> &str { &self.name }`) are excluded from own-call counting. +- **`#[test]` functions** are exempt from IOSP — assertions are logic by nature. + +Tighten with `--strict-closures` / `--strict-iterators` if you want closures and iterator chains to count as logic. + +## Complexity + +IOSP shapes the *structure* of functions; complexity caps the *size* of each piece. The defaults are deliberate: + +| Rule | Default | What it catches | +|---|---|---| +| `CX-001` | cognitive ≤ 15 | Hard-to-read control flow (deep nesting + boolean combinators). Cognitive penalises nesting more than cyclomatic does. | +| `CX-002` | cyclomatic ≤ 10 | Decision-point density. | +| `CX-003` | magic number ≤ 0 | Bare numeric literals outside `const`/`static`. Forces named constants. | +| `CX-004` | length ≤ 60 lines | Function size. | +| `CX-005` | nesting ≤ 4 levels | Pyramid-of-doom guard. | +| `CX-006` | `detect_unsafe = true` | `unsafe { … }` blocks. Use `// qual:allow(unsafe)` for genuine FFI. | +| `A20` | `detect_error_handling = true` | `.unwrap()`, `.expect()`, `panic!`, `todo!` in production code. | + +Configure thresholds in `rustqual.toml`: + +```toml +[complexity] +enabled = true +max_cognitive = 15 +max_cyclomatic = 10 +max_function_lines = 60 +max_nesting_depth = 4 +detect_unsafe = true +detect_error_handling = true +allow_expect = false # set true to permit .expect() but still flag .unwrap() +``` + +`--init` calibrates these to your current metrics so the initial run produces actionable findings, not aspirational ones. + +### Test-aware + +`A20` and `CX-004` skip `#[test]` functions and files under workspace-root `tests/**`. Asserting on `.unwrap()` in a test is fine; it's panicking in production that matters. + +## Method-shape checks + +Beyond IOSP and complexity, rustqual flags structural smells at the method level. These are part of the **structural binary checks** under SRP/Coupling: + +| Rule | What it means | +|---|---| +| `SLM` | **Selfless method** — takes `self` but never references it. Should be a free function or associated function. | +| `NMS` | **Needless `&mut self`** — declares mutable receiver but never mutates. Tighten the signature to `&self`. | +| `BTC` | **Broken trait contract** — every method in an `impl Trait` is a stub (`unimplemented!`, `todo!`, `Default::default()`). The trait is unimplemented in spirit. | + +Configure under `[structural]`: + +```toml +[structural] +enabled = true +# check_btc = true +# check_slm = true +# check_nms = true +``` + +## Parameter sprawl + +`SRP-003` flags functions with too many parameters (default: > 5). The fix is usually a context struct: + +```rust +// Flagged +fn render(width: u32, height: u32, dpi: u32, theme: Theme, + locale: Locale, watermark: Option<&str>) { /* … */ } + +// Better +fn render(opts: &RenderOptions) { /* … */ } +``` + +## What you'll see + +``` +✗ VIOLATION process_order (line 48) [MEDIUM] + IOSP — function mixes orchestration with logic + CX-001 cognitive=18 > 15 + CX-004 length=72 > 60 + +⚠ SLM dispatch (line 124) — takes &self but never uses it +``` + +`--findings` gives one finding per line for piping; `--verbose` shows every function with its full metrics. + +## Suppression for legitimate exceptions + +For functions you genuinely cannot refactor right now (legacy entry points, generated code, FFI shims): + +```rust +// qual:allow(iosp) — match-dispatcher; arms intentionally inlined for codegen +fn dispatch(cmd: Command) -> Result<()> { /* … */ } + +// qual:allow(complexity) — large lookup table; splitting hurts readability +fn rule_table() -> &'static [Rule] { /* … */ } + +// qual:allow(unsafe) — FFI boundary, audited 2026-Q1 +unsafe fn raw_call() { /* … */ } +``` + +Suppressions count against `max_suppression_ratio` (default 5%) so they can't silently take over the codebase. `unsafe`-specific suppression has a separate path that doesn't count against that ratio. + +Full annotation reference: [reference-suppression.md](./reference-suppression.md). + +## Why IOSP for AI-assisted code + +AI agents tend to inline everything — they have no internal pressure to decompose. IOSP makes that pressure mechanical: the agent writes a god-function, rustqual marks it, the agent reads the finding and splits. That converges the loop on small, single-purpose functions instead of "works but unmaintainable" code that passes tests and clippy but rots over months. + +Background on the principle itself: [flow-design.info](https://flow-design.info/) — Ralf Westphal's original write-up on Integration Operation Segregation. + +## Related + +- [module-quality.md](./module-quality.md) — when too many functions cluster into one module +- [test-quality.md](./test-quality.md) — assertion density, untested functions +- [code-reuse.md](./code-reuse.md) — duplicates and dead code +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-configuration.md](./reference-configuration.md) — every config option diff --git a/book/getting-started.md b/book/getting-started.md new file mode 100644 index 0000000..3f47e7e --- /dev/null +++ b/book/getting-started.md @@ -0,0 +1,98 @@ +# Getting started + +## Install + +```bash +cargo install rustqual +``` + +Or from source: + +```bash +git clone https://github.com/SaschaOnTour/rustqual +cd rustqual +cargo install --path . +``` + +You can run rustqual two ways — they're equivalent: + +```bash +rustqual # direct invocation +cargo qual # as a cargo subcommand +``` + +## First run + +```bash +cd your-rust-project +rustqual +``` + +By default rustqual analyses `.`, prints a coloured summary, and exits with code `1` if it found anything. For local exploration that shouldn't fail: + +```bash +rustqual --no-fail +``` + +## What you'll see + +``` +── src/order.rs + ✓ INTEGRATION process_order (line 12) + ✓ OPERATION calculate_discount (line 28) + ✗ VIOLATION process_payment (line 48) [MEDIUM] + +═══ Summary ═══ + Functions: 24 Quality Score: 82.3% + + IOSP: 85.7% + Complexity: 90.0% + DRY: 95.0% + SRP: 100.0% + Test Quality: 100.0% + Coupling: 100.0% + Architecture: 100.0% + +4 quality findings. Run with --verbose for details. +``` + +Each function is classified as **Integration** (orchestrates other functions, no logic), **Operation** (logic, no own calls), **Violation** (mixes both — the smell to fix), or **Trivial** (too small to matter). + +`--verbose` shows every function plus its complexity metrics. `--findings` prints one location per line, useful for piping to `grep`. + +## Generate a config + +```bash +rustqual --init +``` + +This writes a `rustqual.toml` next to `Cargo.toml`, with thresholds calibrated to your current codebase metrics. You can edit any section to tighten or relax checks. Full reference: [reference-configuration.md](./reference-configuration.md). + +## Where to go next + +- **Building with AI assistants?** → [ai-coding-workflow.md](./ai-coding-workflow.md) +- **Adopting on a large existing codebase?** → [legacy-adoption.md](./legacy-adoption.md) +- **Setting up CI?** → [ci-integration.md](./ci-integration.md) +- **Specific quality concerns?** → see the use-case files in this directory: + - [function-quality.md](./function-quality.md) — IOSP, complexity, length, magic numbers + - [module-quality.md](./module-quality.md) — module size, cohesion, function clusters + - [coupling-quality.md](./coupling-quality.md) — circular deps, instability, coupling drift + - [code-reuse.md](./code-reuse.md) — duplicates, dead code, boilerplate + - [test-quality.md](./test-quality.md) — assertions, coverage, untested functions + - [architecture-rules.md](./architecture-rules.md) — layers, forbidden edges, trait contracts + - [adapter-parity.md](./adapter-parity.md) — keep adapter layers in sync + +## Common flags worth knowing + +| Flag | Use | +|---|---| +| `--no-fail` | Local exploration; don't exit non-zero | +| `--verbose` | Show every function, not just findings | +| `--findings` | One finding per line: `file:line category in fn_name` | +| `--diff [REF]` | Only analyse files changed vs a git ref | +| `--coverage ` | Include coverage-based test-quality checks | +| `--init` | Generate a config tailored to your codebase | +| `--watch` | Re-analyse on file changes | +| `--explain ` | Architecture diagnostic for one file | + +Full flag list: [reference-cli.md](./reference-cli.md). diff --git a/book/legacy-adoption.md b/book/legacy-adoption.md new file mode 100644 index 0000000..b1a7b03 --- /dev/null +++ b/book/legacy-adoption.md @@ -0,0 +1,153 @@ +# Use case: adopting rustqual on an existing codebase + +Running rustqual against a large legacy Rust codebase for the first time will produce a lot of findings. That's expected — rustqual was built around an opinionated set of structural rules (IOSP especially). The trick is not to fix everything at once. This guide shows four adoption patterns ordered from "lightest touch" to "full enforcement". + +## The general principle + +You don't have to enable everything on day one. Each dimension has an `enabled` flag, and you can ratchet up over time. A typical adoption sequence: + +1. Start with **DRY** and **Test Quality** — high-signal, low-controversy findings (duplicates, dead code, untested functions, weak assertions). +2. Add **Complexity** — function length, magic numbers, error-handling patterns. Most findings are quick fixes. +3. Add **SRP** and **Coupling** — module-level structure. Some refactoring required, but each fix is local. +4. Add **Architecture** with layer rules — once you've decided what your layering should look like. +5. Add **IOSP** last. This is the most invasive and benefits most from existing decomposition. + +## Pattern A: shallow adoption — defaults off, ratchet on + +Disable the dimensions you're not ready for. Start with whatever you do feel ready to enforce: + +```toml +# rustqual.toml — minimal initial config +[complexity] +enabled = true +max_function_lines = 80 # initial floor; tighten over time + +[duplicates] +enabled = true + +[test_quality] +enabled = true + +[srp] +enabled = false # enable later + +[coupling] +enabled = false # enable later + +[architecture] +enabled = false # enable when you've decided on layering +``` + +If a dimension produces too much noise to act on right now, disable it with `enabled = false` and re-enable later as you ratchet up. For dimensions you can't selectively disable, Pattern B (baseline) absorbs existing findings without enforcing them. + +## Pattern B: baseline — accept current state, enforce no regression + +The most common adoption pattern for an active codebase. Snapshot the current quality state, then in CI fail only on regression: + +```bash +# Generate the baseline once +rustqual --save-baseline baseline.json +git add baseline.json +git commit -m "Add quality baseline" +``` + +In CI: + +```yaml +- run: rustqual --compare baseline.json --fail-on-regression +``` + +New code must be at least as good as what's there. Existing findings stay, but PRs can't introduce new ones. Regenerate the baseline as part of dedicated refactor PRs: + +```bash +rustqual --save-baseline baseline.json # after refactor lowers the count +``` + +This works without disabling anything. You get the full set of checks active immediately, but you don't have to refactor everything. + +## Pattern C: per-function suppression with rationale + +For specific functions you genuinely don't want to refactor (legacy entry points, generated code, etc.), use `// qual:allow` annotations: + +```rust +// qual:allow(iosp) — legacy handler from the v1 API; superseded by handler_v2. +// Kept for backward compat until 2027 sunset. +pub fn legacy_dispatch(req: Request) -> Response { + if req.is_v1() { + handle_v1(req) + } else { + handle_v2(req) + } +} +``` + +Pros: +- Explicit, reviewable in PRs. +- The rationale is right next to the code that needs it. +- `max_suppression_ratio` (default 5%) caps how much can be suppressed before rustqual itself complains. + +Cons: +- Each function needs an annotation. +- Easy to over-use if you're not careful. + +For genuine public-API surface, prefer `// qual:api`: + +```rust +// qual:api — public re-export, callers live outside this crate +pub fn encode(data: &[f32]) -> Vec { /* … */ } +``` + +This excludes the function from dead-code (DRY-002) and untested-function (TQ-003) detection without counting against the suppression ratio. Other dimensions (complexity, IOSP, etc.) still apply. + +For test-only helpers in `src/` that are called from `tests/`: + +```rust +// qual:test_helper +pub fn assert_in_range(actual: f64, expected: f64, tol: f64) { + assert!((actual - expected).abs() < tol); +} +``` + +Same treatment as `// qual:api` for DRY-002/TQ-003, also no ratio cost. + +Full annotation reference: [reference-suppression.md](./reference-suppression.md). + +## Pattern D: bulk-suppress a directory + +For directories you genuinely don't want to analyse (vendored code, auto-generated files, examples): + +```toml +exclude_files = [ + "src/legacy/**", + "src/generated/**", + "examples/**", +] +``` + +These files are skipped entirely — no findings, no ratio cost. + +## Recommended onboarding sequence + +1. **Day 1.** `cargo install rustqual && rustqual --init`. Read the generated config. Don't change it yet. +2. **Day 1.** Run `rustqual --no-fail`. Look at the dimension scores. Note which dimensions are at 100% already (free wins) and which are far off. +3. **Day 1.** Disable the dimensions that produce noise you can't act on yet. Keep the ones that produce actionable findings. +4. **Week 1.** Add a CI step with `--compare baseline.json --fail-on-regression`. Commit the baseline. +5. **Week 2–4.** Incrementally fix findings or add `// qual:allow` annotations with rationale. Re-baseline after each batch. +6. **Month 2+.** Re-enable disabled dimensions one by one. Tighten thresholds in `rustqual.toml` as you go. +7. **Month 3+.** Switch to `--min-quality-score 90` or similar, and drop the baseline. + +This stages the cost over weeks instead of front-loading it on day one. + +## Things that often surprise people + +**IOSP scores are usually low at first.** A typical Rust codebase has 30-60% IOSP compliance before refactoring. Don't panic — that's the dimension's whole point. The `--suggestions` flag gives pattern-based hints for fixing common cases. + +**The Architecture dimension defaults to "strict_error" for unmatched files.** If your `[architecture.layers]` globs don't cover every production file, rustqual will tell you. Either widen the globs, mark the file as a re-export point, or set `unmatched_behavior = "composition_root"` to opt out of strict mode. + +**`--init` produces a config tailored to your current metrics.** It's intentional: starting with realistic thresholds means most findings are real ones, not aspirational ones. + +## Related + +- [ci-integration.md](./ci-integration.md) — for `--compare` and `--fail-on-regression` CI patterns +- [reference-suppression.md](./reference-suppression.md) — full annotation reference +- [reference-configuration.md](./reference-configuration.md) — every config option diff --git a/book/module-quality.md b/book/module-quality.md new file mode 100644 index 0000000..d47b7c8 --- /dev/null +++ b/book/module-quality.md @@ -0,0 +1,142 @@ +# Use case: module-level quality + +A module is the unit of cohesion. Functions inside it should belong together — same data, same purpose, same change-axis. When a module grows past a few hundred lines or starts hosting unrelated function clusters, you've usually accumulated two responsibilities pretending to be one. + +rustqual checks this mechanically through SRP — Single Responsibility Principle — applied to both modules (file-level) and structs (struct-level). + +## What goes wrong + +- **Bloated modules** — one file accumulates 800 lines and twelve responsibilities. The new feature lands "wherever there's room" instead of in its own file. +- **God-structs** — a struct with 30 fields and 40 methods, slowly growing because nobody wants to break it apart. +- **Disconnected clusters inside one struct** — methods `a/b/c` only touch fields `x/y`, methods `d/e/f` only touch fields `z/w`. They share a name, not a responsibility. The struct should be two structs. +- **Wide signatures** — functions with 6+ parameters carrying ad-hoc context. The shape of the signature *is* the design: too many parameters means a missing struct. + +## What rustqual catches + +| Rule | Meaning | Default threshold | +|---|---|---| +| `SRP-001` | Struct may violate SRP — too many fields/methods or low cohesion (LCOM4 > 2) | composite score, `max_fields = 12`, `max_methods = 20` | +| `SRP-002` | Module file too long | warn at 300 lines, hard at 800 | +| `SRP-003` | Function has too many parameters | `max_parameters = 5` | + +`SRP-001` is a composite score: it weighs field count, method count, fan-out, and LCOM4 cohesion together. A struct that's slightly over on fields but cohesive elsewhere doesn't fire; one with disjoint clusters does. + +## LCOM4 and responsibility clusters + +LCOM4 (Lack of Cohesion of Methods, version 4) detects when a struct's methods form **disjoint groups** that share no fields. If you have: + +- `Cluster A`: `methods = [authenticate, refresh_token]`, `fields = [token, expires_at]` +- `Cluster B`: `methods = [render_avatar, set_theme]`, `fields = [avatar_url, theme]` + +…rustqual reports two responsibility clusters and flags the struct. The fix is usually to split into two types — one for auth, one for presentation. + +The verbose output names each cluster so the refactor is mechanical: + +``` +✗ SRP-001 UserSession (line 12) — 2 disjoint clusters + cluster 1: [authenticate, refresh_token] over [token, expires_at] + cluster 2: [render_avatar, set_theme] over [avatar_url, theme] +``` + +## Module length + +Production-line counting excludes blank lines, single-line `//` comments, and `#[cfg(test)]` blocks. So a 1000-line file with 600 lines of tests counts as ~400 production lines. The thresholds: + +- `file_length_baseline = 300` — soft warn +- `file_length_ceiling = 800` — hard finding + +Tests don't push you over the limit. Comments don't either. The number tracks production code only, which is what actually carries the maintenance cost. + +## Configure thresholds + +```toml +[srp] +enabled = true +# max_fields = 12 +# max_methods = 20 +# max_fan_out = 10 +# max_parameters = 5 +# lcom4_threshold = 2 +# file_length_baseline = 300 +# file_length_ceiling = 800 +# max_independent_clusters = 2 +# min_cluster_statements = 5 +# smell_threshold = 0.6 # composite score for SRP-001 +``` + +`--init` calibrates these to your current codebase metrics so initial findings are realistic. + +## Refactor patterns + +**Bloated module** — split by responsibility, not by alphabetical order: + +``` +# Before +src/order/mod.rs # 850 lines: validation + pricing + persistence + email + +# After +src/order/mod.rs # re-exports +src/order/validation.rs # 180 lines +src/order/pricing.rs # 220 lines +src/order/persistence.rs # 190 lines +src/order/notification.rs # 140 lines +``` + +**God-struct with disjoint clusters** — split into types that match the clusters: + +```rust +// Before: SRP-001 with 2 clusters +struct UserSession { + token: Token, expires_at: DateTime, + avatar_url: String, theme: Theme, +} + +// After +struct AuthSession { token: Token, expires_at: DateTime } +struct UserPresentation { avatar_url: String, theme: Theme } +``` + +**Parameter sprawl** — context struct or builder: + +```rust +// SRP-003 +fn render(width: u32, height: u32, dpi: u32, theme: Theme, + locale: Locale, watermark: Option<&str>) { /* … */ } + +// Better +fn render(opts: &RenderOptions) { /* … */ } +``` + +## Suppression + +For modules you genuinely can't split right now (legacy entry points, autogenerated config schemas): + +```rust +// qual:allow(srp) — entire module is one well-defined responsibility, +// length comes from a 600-line lookup table that has to live together. +``` + +The annotation goes on the first item of the file (the topmost `pub use`, `pub fn`, struct, etc.) and applies to the file-level finding. Counts against `max_suppression_ratio`. + +For struct-level suppression: + +```rust +// qual:allow(srp) — public-API struct, fields are stable and intentional +pub struct Config { /* 18 fields */ } +``` + +## Self-compliance + +rustqual itself runs at 100% SRP and ratchets file lengths down aggressively. The CLAUDE.md note worth knowing if you adopt the same workflow: + +> New report code pushes files over 300 production line SRP threshold — extract into submodules proactively. + +It's cheaper to split early than to do a 5-file rewrite once the threshold trips on the next feature. + +## Related + +- [function-quality.md](./function-quality.md) — IOSP, complexity, SRP-003 parameter count +- [coupling-quality.md](./coupling-quality.md) — what happens between modules +- [code-reuse.md](./code-reuse.md) — duplication often signals a missing module +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-configuration.md](./reference-configuration.md) — every config option diff --git a/book/reference-cli.md b/book/reference-cli.md new file mode 100644 index 0000000..1462de6 --- /dev/null +++ b/book/reference-cli.md @@ -0,0 +1,108 @@ +# Reference: CLI flags + +``` +rustqual [OPTIONS] [PATH] +``` + +`PATH` defaults to `.`. Run from the project root so architecture globs (`src/**`) match. + +## Output and verbosity + +| Flag | Description | +|---|---| +| `-v`, `--verbose` | Show every function with metrics, not just findings. | +| `--findings` | One finding per line: `file:line category detail in fn_name`. Useful for piping. | +| `--format ` | Output format. One of: `text` (default), `json`, `github`, `dot`, `sarif`, `html`, `ai`, `ai-json`. See [reference-output-formats.md](./reference-output-formats.md). | +| `--json` | Shortcut for `--format json`. | +| `--suggestions` | Show refactoring suggestions for IOSP violations. | + +## Analysis behaviour + +| Flag | Description | +|---|---| +| `-c`, `--config ` | Path to config. Default: `rustqual.toml` in the target directory. | +| `--diff [REF]` | Only analyse files changed vs a git ref (default: `HEAD`). Conflicts with `--watch`. | +| `--coverage ` | Path to LCOV coverage file. Enables TQ-004 / TQ-005. | +| `--explain ` | Diagnostic mode: explain architecture-rule classification for one file. | +| `--watch` | Watch for file changes and re-analyse continuously. | + +## Strictness toggles + +| Flag | Description | +|---|---| +| `--strict-closures` | Treat closures as logic (stricter IOSP). | +| `--strict-iterators` | Treat iterator chains (`.map`, `.filter`, …) as logic. | +| `--strict-error-propagation` | Count `?` as logic (implicit control flow). | +| `--allow-recursion` | Allow recursive calls — don't count as violations. | + +## Exit-code controls + +| Flag | Description | +|---|---| +| `--no-fail` | Don't exit `1` on findings. Useful for local exploration. | +| `--fail-on-warnings` | Treat warnings (suppression-ratio overrun, etc.) as errors. | +| `--min-quality-score ` | Minimum overall quality score (0–100). Exit `1` if below. | +| `--fail-on-regression` | Used with `--compare`. Exit `1` only when quality regresses vs baseline. | + +## Baseline / regression + +| Flag | Description | +|---|---| +| `--save-baseline ` | Save current results as baseline JSON. | +| `--compare ` | Compare current results against a saved baseline. | + +Typical workflow: + +```bash +rustqual --save-baseline baseline.json +git add baseline.json && git commit -m "Add quality baseline" + +# In CI: +rustqual --compare baseline.json --fail-on-regression +``` + +## Sorting + +| Flag | Description | +|---|---| +| `--sort-by-effort` | Sort IOSP violations by refactoring effort (highest first). | + +## Project setup + +| Flag | Description | +|---|---| +| `--init` | Generate a `rustqual.toml` calibrated to your current codebase metrics. | +| `--completions ` | Emit shell completions. Supported: `bash`, `zsh`, `fish`, `elvish`, `powershell`. | + +## Exit codes + +| Code | Meaning | +|---|---| +| `0` | Success — no findings, or `--no-fail` set. | +| `1` | Findings; or regression vs baseline; or score below `--min-quality-score`; or warnings present (`--fail-on-warnings`). | +| `2` | Configuration error — invalid or unreadable `rustqual.toml`. | + +## Common compositions + +```bash +# Local exploration +rustqual --no-fail --verbose + +# CI hard gate with coverage and PR annotations +rustqual --coverage lcov.info --min-quality-score 90 --fail-on-warnings --format github + +# PR-only analysis +rustqual --diff origin/main --format github + +# Baseline-based regression gate +rustqual --compare baseline.json --fail-on-regression --format github + +# Explain why a file is failing the architecture dimension +rustqual --explain src/foo/bar.rs +``` + +## Related + +- [reference-configuration.md](./reference-configuration.md) — every config option in `rustqual.toml` +- [reference-output-formats.md](./reference-output-formats.md) — every `--format` value with examples +- [ci-integration.md](./ci-integration.md) — putting flags together in CI diff --git a/book/reference-configuration.md b/book/reference-configuration.md new file mode 100644 index 0000000..9e0984e --- /dev/null +++ b/book/reference-configuration.md @@ -0,0 +1,297 @@ +# Reference: configuration + +`rustqual.toml` lives next to `Cargo.toml` and configures every dimension. Generate a starter file calibrated to your codebase: + +```bash +rustqual --init +``` + +Below is the full schema, grouped by section. Every field has a default; a minimal config can omit anything you don't want to tune. + +## Top-level + +| Key | Default | Meaning | +|---|---|---| +| `ignore_functions` | `["main", "run", "visit_*"]` | Function names (or `prefix*` patterns) excluded from all dimensions | +| `exclude_files` | `[]` | Glob patterns for files to skip entirely | +| `strict_closures` | `false` | Treat closures as logic (stricter IOSP) | +| `strict_iterator_chains` | `false` | Treat `.map`/`.filter`/`.fold` as logic | +| `allow_recursion` | `false` | Allow self-calls without counting as IOSP violation | +| `strict_error_propagation` | `false` | Count `?` as logic | +| `max_suppression_ratio` | `0.05` | Cap on `qual:allow` annotations as fraction of functions | + +```toml +ignore_functions = ["main", "run", "visit_*"] +exclude_files = ["examples/**", "vendor/**"] +max_suppression_ratio = 0.05 +``` + +## `[complexity]` + +| Key | Default | Meaning | +|---|---|---| +| `enabled` | `true` | Enable the dimension | +| `max_cognitive` | `15` | `CX-001` threshold | +| `max_cyclomatic` | `10` | `CX-002` threshold | +| `max_nesting_depth` | `4` | `CX-005` threshold | +| `max_function_lines` | `60` | `CX-004` threshold | +| `detect_unsafe` | `true` | Emit `CX-006` on `unsafe` blocks | +| `detect_error_handling` | `true` | Emit `A20` on `unwrap`/`expect`/`panic!`/`todo!` | +| `allow_expect` | `false` | Permit `.expect()` while still flagging `.unwrap()` | + +## `[duplicates]` + +```toml +[duplicates] +enabled = true +``` + +`DRY-001` similarity threshold (95% by default) and `DRY-003` minimum-fragment-length (6 lines) are currently fixed. + +## `[boilerplate]` + +```toml +[boilerplate] +enabled = true +``` + +The full `BP-*` family. Disable if your project deliberately avoids derive macros. + +## `[srp]` + +| Key | Default | Meaning | +|---|---|---| +| `enabled` | `true` | Enable the dimension | +| `smell_threshold` | `0.6` | Composite score threshold for `SRP-001` | +| `max_fields` | `12` | Field count over which `SRP-001` weighs more | +| `max_methods` | `20` | Method count over which `SRP-001` weighs more | +| `max_fan_out` | `10` | Per-struct fan-out bound | +| `max_parameters` | `5` | `SRP-003` threshold | +| `lcom4_threshold` | `2` | Number of disjoint clusters before LCOM4 contributes | +| `file_length_baseline` | `300` | Soft warn for `SRP-002` (production lines) | +| `file_length_ceiling` | `800` | Hard finding for `SRP-002` | +| `max_independent_clusters` | `2` | Max disjoint cluster count | +| `min_cluster_statements` | `5` | Minimum statements for a cluster to count | + +## `[coupling]` + +| Key | Default | Meaning | +|---|---|---| +| `enabled` | `true` | Enable the dimension | +| `max_instability` | `0.8` | Warn when module instability exceeds this | +| `max_fan_in` | `15` | Per-module fan-in bound | +| `max_fan_out` | `12` | Per-module fan-out bound | +| `check_sdp` | `true` | Stable Dependencies Principle (`CP-002`) | + +## `[structural]` + +Binary checks: BTC, SLM, NMS, OI, SIT, DEH, IET. + +| Key | Default | Meaning | +|---|---|---| +| `enabled` | `true` | Enable the dimension | +| `check_btc` | `true` | Broken trait contract | +| `check_slm` | `true` | Selfless method | +| `check_nms` | `true` | Needless `&mut self` | +| `check_oi` | `true` | Orphaned impl | +| `check_sit` | `true` | Single-impl trait | +| `check_deh` | `true` | Downcast escape hatch | +| `check_iet` | `true` | Inconsistent error types | + +## `[test_quality]` + +| Key | Default | Meaning | +|---|---|---| +| `enabled` | `true` | Enable the dimension | +| `extra_assertion_macros` | `[]` | Custom macro names to recognise as assertions in `TQ-001` | + +```toml +[test_quality] +extra_assertion_macros = ["verify", "check_invariant", "expect_that"] +``` + +`TQ-004` and `TQ-005` activate when `--coverage ` is supplied. + +## `[weights]` + +Quality-score weights. Must sum to `1.0`. + +```toml +[weights] +iosp = 0.22 +complexity = 0.18 +dry = 0.13 +srp = 0.18 +coupling = 0.09 +test_quality = 0.10 +architecture = 0.10 +``` + +## `[architecture]` + +The largest section. Top-level toggle: + +```toml +[architecture] +enabled = true +``` + +Then one or more rule families: + +### `[architecture.layers]` + +```toml +[architecture.layers] +order = ["domain", "ports", "infrastructure", "analysis", "application"] +unmatched_behavior = "strict_error" # or "composition_root", "warn" + +[architecture.layers.domain] +paths = ["src/domain/**"] + +[architecture.layers.application] +paths = ["src/app/**"] +``` + +`unmatched_behavior` controls files outside any layer: + +- `"strict_error"` — fail (recommended). +- `"composition_root"` — treat as the root that may import any layer. +- `"warn"` — soft warning. + +### `[architecture.reexport_points]` + +```toml +[architecture.reexport_points] +paths = ["src/lib.rs", "src/main.rs", "src/bin/**", "src/cli/**", "tests/**"] +``` + +Files marked here bypass the layer rule. + +### `[architecture.external_crates]` + +For multi-crate workspaces: + +```toml +[architecture.external_crates] +my_domain_types = "domain" +my_port_traits = "ports" +``` + +### `[[architecture.forbidden]]` + +Repeatable. Per-rule fields: `from`, `to`, `except` (optional), `reason`. + +```toml +[[architecture.forbidden]] +from = "src/adapters/**" +to = "src/app/**" +reason = "Adapters know domain + ports, not application" +``` + +### `[[architecture.pattern]]` + +Repeatable symbol-pattern rules. + +| Field | Meaning | +|---|---| +| `name` | Identifier shown in findings | +| `forbid_path_prefix` | List of `crate::` / `module::` prefixes to forbid | +| `forbid_method_call` | List of method names to forbid (`unwrap`, `expect`, …) | +| `forbid_macro_call` | List of macro names to forbid (`println`, `dbg`, …) | +| `forbid_glob_import` | `true` to forbid `use foo::*;` | +| `forbidden_in` | Globs where the rule fires | +| `allowed_in` | Globs where the rule is exempted | +| `except` | Globs in `forbidden_in` that are exempted | +| `reason` | Mandatory rationale | + +### `[[architecture.trait_contract]]` + +Repeatable trait-shape rules. + +| Field | Meaning | +|---|---| +| `name` | Identifier shown in findings | +| `scope` | Glob of files where the rule applies | +| `receiver_may_be` | Allowed receiver kinds: `"shared_ref"`, `"mut_ref"`, `"owned"` | +| `forbidden_return_type_contains` | Substrings forbidden in return types | +| `forbidden_error_variant_contains` | Substrings forbidden in error types (`Result<_, E>`) | +| `must_be_object_safe` | `true` to require object-safety | +| `required_supertraits_contain` | Required supertrait substrings (`"Send"`, `"Sync"`) | + +### `[architecture.call_parity]` + +Single-instance section. + +| Field | Default | Meaning | +|---|---|---| +| `adapters` | (required) | List of adapter layer names | +| `target` | (required) | Target layer name | +| `call_depth` | `3` | Transitive walk depth | +| `exclude_targets` | `[]` | Globs (module-path form) to skip from Check B | +| `transparent_wrappers` | `[]` | Wrapper type names to peel during receiver-type inference | +| `transparent_macros` | (default list) | Attribute macros treated as transparent | + +```toml +[architecture.call_parity] +adapters = ["cli", "mcp"] +target = "application" +call_depth = 3 +exclude_targets = ["application::admin::*"] +transparent_wrappers = ["State", "Extension", "Json", "Data"] +``` + +## `[report]` + +Workspace-mode aggregation: + +```toml +[report] +aggregation = "loc_weighted" +``` + +Aggregation strategies: `"loc_weighted"` (default), `"unweighted"`. + +## Composition + +Most projects converge on a layout like: + +```toml +ignore_functions = ["main", "run"] +exclude_files = ["examples/**"] +max_suppression_ratio = 0.05 + +[complexity] +enabled = true + +[duplicates] +enabled = true + +[srp] +enabled = true + +[coupling] +enabled = true + +[test_quality] +enabled = true + +[architecture] +enabled = true + +[architecture.layers] +order = ["domain", "ports", "infrastructure", "application"] +unmatched_behavior = "strict_error" + +[architecture.layers.domain] +paths = ["src/domain/**"] +# … etc. +``` + +Use `--init` to bootstrap with calibrated thresholds, then trim. + +## Related + +- [reference-cli.md](./reference-cli.md) — flags that override or supplement config +- [reference-rules.md](./reference-rules.md) — every rule that config keys gate +- [reference-suppression.md](./reference-suppression.md) — `qual:allow` etc. +- [getting-started.md](./getting-started.md) — `--init` and first-run workflow diff --git a/book/reference-output-formats.md b/book/reference-output-formats.md new file mode 100644 index 0000000..651beee --- /dev/null +++ b/book/reference-output-formats.md @@ -0,0 +1,156 @@ +# Reference: output formats + +`--format ` switches the output format. All formats render the same underlying analysis — they differ only in serialisation. + +| Format | Use case | +|---|---| +| `text` (default) | Local exploration, terminal use. Coloured summary. | +| `json` | Machine-readable, full detail. Pipe to `jq`, custom dashboards. | +| `github` | `::error::` / `::warning::` annotations on the GitHub PR diff. | +| `sarif` | GitHub Code Scanning, Azure DevOps, any SARIF v2.1.0 consumer. | +| `dot` | Graphviz module dependency graph. | +| `html` | Self-contained HTML report. Publishable as CI artifact. | +| `ai`, `ai-json` | Compact representations tuned for LLM agents. | + +`--json` is shorthand for `--format json`. + +## `text` (default) + +``` +── src/order.rs + ✓ INTEGRATION process_order (line 12) + ✓ OPERATION calculate_discount (line 28) + ✗ VIOLATION process_payment (line 48) [MEDIUM] + +═══ Summary ═══ + Functions: 24 Quality Score: 82.3% + + IOSP: 85.7% + Complexity: 90.0% + DRY: 95.0% + SRP: 100.0% + Test Quality: 100.0% + Coupling: 100.0% + Architecture: 100.0% + +4 quality findings. Run with --verbose for details. +``` + +`--verbose` adds every function with metrics. `--findings` collapses to one finding per line for piping. + +## `json` + +Full structured output. Top-level keys: + +```jsonc +{ + "version": "1.2.0", + "summary": { "score": 82.3, "functions": 24, "findings": 4, "warnings": 0, + "dimensions": { "iosp": 85.7, "complexity": 90.0, /* ... */ } }, + "findings": [ + { "code": "iosp/violation", "severity": "medium", + "file": "src/order.rs", "line": 48, "function": "process_payment", + "message": "function mixes orchestration with logic" } + ], + "files": [ /* per-file analysis */ ], + "config": { /* effective config */ } +} +``` + +Use this for custom dashboards, regression tracking, or piping into shell tooling: + +```bash +rustqual --format json | jq '.summary.score' +rustqual --format json | jq '.findings[] | select(.severity == "high")' +``` + +## `github` + +GitHub Actions workflow-command annotations. Inline on the PR diff: + +``` +::error file=src/order.rs,line=48,title=IOSP::function mixes orchestration with logic +::warning file=src/utils/legacy.rs,line=12,title=DRY-002::dead code +``` + +Combine with `--diff origin/main` for PR-only analysis: + +```yaml +- run: rustqual --diff origin/main --format github +``` + +## `sarif` + +SARIF v2.1.0. Designed for GitHub Code Scanning, but consumed by Azure DevOps, Sonatype, and any SARIF tool. + +```yaml +- run: rustqual --format sarif > rustqual.sarif +- uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: rustqual.sarif +``` + +Findings show up in the **Security** tab as Code Scanning alerts. Each rule has a stable rule ID (`CX-001`, `DRY-002`, etc.) so dismissals persist across runs. + +## `dot` + +Module dependency graph in Graphviz format: + +```bash +rustqual --format dot | dot -Tpng -o deps.png +rustqual --format dot | dot -Tsvg -o deps.svg +``` + +Useful for spotting cycles or visualising layer separation. Pair with `coupling-quality.md`. + +## `html` + +Self-contained HTML report — no external CSS/JS, no network required. Embed in CI as an artifact: + +```yaml +- run: rustqual --format html > quality.html +- uses: actions/upload-artifact@v4 + with: + name: quality-report + path: quality.html +``` + +The HTML report includes: + +- Per-dimension scores with sparklines. +- Sortable / filterable findings table. +- Per-file drilldown. +- Per-function metrics. + +## `ai` / `ai-json` + +Compact representations tuned for LLM consumption — fewer tokens than full JSON, focused on what an agent needs to act: + +- `ai` — token-efficient text format. Findings only, with file:line, code, and a one-line description. +- `ai-json` — minimal JSON: code, file, line, function, message. No metadata, no per-file tables. + +Useful when you're piping rustqual output into a coding agent (Claude Code, Cursor, etc.) and want to keep the prompt small. + +```bash +rustqual --format ai | claude code "Fix these findings" +``` + +## Choosing a format + +| Audience | Format | +|---|---| +| Developer at a terminal | `text` | +| GitHub PR reviewer | `github` | +| GitHub Code Scanning | `sarif` | +| Custom CI dashboard | `json` | +| Architecture review meeting | `dot` (rendered to PNG/SVG) | +| Stakeholder report | `html` artifact | +| LLM agent prompt | `ai` / `ai-json` | + +Most CI configurations use `github` or `sarif` (or both). Local development: `text`. Tooling integration: `json`. + +## Related + +- [reference-cli.md](./reference-cli.md) — `--format` and other flags +- [ci-integration.md](./ci-integration.md) — CI examples for each format +- [reference-rules.md](./reference-rules.md) — codes referenced in every format diff --git a/book/reference-rules.md b/book/reference-rules.md new file mode 100644 index 0000000..97196bf --- /dev/null +++ b/book/reference-rules.md @@ -0,0 +1,126 @@ +# Reference: rule catalog + +Every rule rustqual emits, grouped by dimension. Codes are stable — they appear in JSON output, SARIF, GitHub annotations, and `// qual:allow` rationales. + +For dimension intent and refactor patterns, see the use-case guides linked at the bottom. + +## IOSP + +| Code | Meaning | +|---|---| +| `iosp/violation` | Function mixes orchestration with logic — split into Integration + Operation(s). | + +## Complexity (CX-*) + +| Code | Meaning | Default threshold | +|---|---|---| +| `CX-001` | Cognitive complexity exceeds threshold | ≤ 15 | +| `CX-002` | Cyclomatic complexity exceeds threshold | ≤ 10 | +| `CX-003` | Magic-number literal in non-const context | (any literal not in `const`/`static`) | +| `CX-004` | Function length exceeds threshold | ≤ 60 lines | +| `CX-005` | Nesting depth exceeds threshold | ≤ 4 | +| `CX-006` | Unsafe block detected | `detect_unsafe = true` | +| `A20` | Error-handling issue (`unwrap`/`expect`/`panic!`/`todo!`) | `detect_error_handling = true` | + +`A20` and `CX-004` skip `#[test]` functions and workspace-root `tests/**` files. + +## DRY + +| Code | Meaning | +|---|---| +| `DRY-001` | Duplicate function (95%+ token similarity) | +| `DRY-002` | Dead code — function defined but never called | +| `DRY-003` | Duplicate code fragment (≥6 lines repeated) | +| `DRY-004` | Wildcard import (`use foo::*;`) | +| `DRY-005` | Repeated match pattern across functions (≥3 arms, ≥3 instances) | + +## Boilerplate (BP-*) + +| Code | Meaning | +|---|---| +| `BP-001` | Trivial `From` impl (derivable) | +| `BP-002` | Trivial `Display` impl (derivable) | +| `BP-003` | Trivial getter/setter (consider field visibility) | +| `BP-004` | Builder pattern (consider derive macro) | +| `BP-005` | Manual `Default` impl (derivable) | +| `BP-006` | Repetitive match mapping | +| `BP-007` | Error enum boilerplate (consider `thiserror`) | +| `BP-008` | Clone-heavy conversion | +| `BP-009` | Struct-update boilerplate | +| `BP-010` | Format-string repetition | + +## SRP + +| Code | Meaning | +|---|---| +| `SRP-001` | Struct may violate Single Responsibility Principle (composite: fields + methods + cohesion) | +| `SRP-002` | Module file too long (default warn 300, hard 800 production lines) | +| `SRP-003` | Function has too many parameters (default > 5) | + +## Coupling + +| Code | Meaning | +|---|---| +| `CP-001` | Circular module dependency | +| `CP-002` | Stable Dependencies Principle violation | + +## Structural binary checks + +Part of SRP (BTC, SLM, NMS) and Coupling (OI, SIT, DEH, IET). + +| Code | Meaning | +|---|---| +| `BTC` | Broken trait contract — every method in an `impl Trait` block is a stub | +| `SLM` | Selfless method — takes `self` but never references it | +| `NMS` | Needless `&mut self` — declares mutable receiver but never mutates | +| `OI` | Orphaned impl — `impl Foo` block in different file from `struct Foo` | +| `SIT` | Single-impl trait — non-`pub` trait with exactly one implementation | +| `DEH` | Downcast escape hatch — use of `Any::downcast` | +| `IET` | Inconsistent error types within a module | + +## Test Quality (TQ-*) + +| Code | Meaning | +|---|---| +| `TQ-001` | Test function has no assertions | +| `TQ-002` | Test function does not call any production function | +| `TQ-003` | Production function is untested (no test calls it) | +| `TQ-004` | Production function has no coverage (LCOV-based, requires `--coverage`) | +| `TQ-005` | Untested logic branches — covered function with uncovered lines | + +## Architecture + +Architecture findings carry the originating rule kind and the source rule's name: + +| Code | Meaning | +|---|---| +| `ARCH-LAYER` | Layer rule violation — file imports outside its allowed direction | +| `ARCH-FORBID` | Forbidden-edge violation — `[[architecture.forbidden]]` rule fired | +| `ARCH-PATTERN` | Symbol-pattern violation — `[[architecture.pattern]]` rule fired | +| `ARCH-TRAIT` | Trait-contract violation — `[[architecture.trait_contract]]` rule fired | +| `ARCH-CALL-PARITY` | Call-parity violation — Check A (no delegation) or Check B (missing adapter) | + +## Suppression / governance + +| Code | Meaning | +|---|---| +| `SUP-001` | Suppression ratio exceeds configured maximum (default 5%). Warn by default; error with `--fail-on-warnings`. | +| `ORPHAN-001` | Stale `qual:allow` marker — no finding in the annotation window. | + +## Severity & default-fail + +By default, every finding fails the build (exit code `1`). Override with `--no-fail` for local exploration, or `--min-quality-score ` to allow some findings as long as the overall score holds. + +Warnings (`SUP-001`) don't fail by default — pass `--fail-on-warnings` to flip that. + +## Related + +- [reference-configuration.md](./reference-configuration.md) — every config option in `rustqual.toml` +- [reference-suppression.md](./reference-suppression.md) — `qual:allow`, `qual:api`, etc. +- [function-quality.md](./function-quality.md) — IOSP, CX, A20 +- [module-quality.md](./module-quality.md) — SRP-* +- [coupling-quality.md](./coupling-quality.md) — CP-*, OI, SIT, DEH, IET +- [code-reuse.md](./code-reuse.md) — DRY-*, BP-* +- [test-quality.md](./test-quality.md) — TQ-* +- [architecture-rules.md](./architecture-rules.md) — ARCH-* +- [adapter-parity.md](./adapter-parity.md) — ARCH-CALL-PARITY diff --git a/book/reference-suppression.md b/book/reference-suppression.md new file mode 100644 index 0000000..14ee58f --- /dev/null +++ b/book/reference-suppression.md @@ -0,0 +1,165 @@ +# Reference: suppression annotations + +Five annotation forms, ordered from most-restricted to least: + +| Annotation | Scope | Counts against `max_suppression_ratio` | +|---|---|---| +| `// qual:allow` | All dimensions | Yes | +| `// qual:allow()` | One dimension (`iosp`, `complexity`, `dry`, `srp`, `coupling`, `test_quality`, `architecture`) | Yes | +| `// qual:allow(unsafe)` | `CX-006` only | No | +| `// qual:api` | Excludes from `DRY-002`, `TQ-003` | No | +| `// qual:test_helper` | Excludes from `DRY-002` (testonly), `TQ-003` | No | +| `// qual:inverse()` | Suppresses near-duplicate `DRY-001` for inverse pairs | No | +| `// qual:recursive` | Removes self-calls from own-calls before leaf reclassification | No | + +Each annotation lives in a `//`-comment block immediately above the item it applies to. The block extends upward until a blank line or a non-`//` line breaks it. `#[derive(...)]` and other attributes between the comment block and the item are fine — they don't break the block. + +## `// qual:allow` — suppress all dimensions + +```rust +// qual:allow — autogenerated, hand-edits forbidden +pub fn _generated_table() -> &'static [Entry] { /* … */ } +``` + +Useful for files / functions that genuinely cannot be reasoned about by any quality dimension. Counts against `max_suppression_ratio`. + +## `// qual:allow()` — suppress one dimension + +```rust +// qual:allow(iosp) — match dispatcher; arms intentionally inlined +fn dispatch(cmd: Command) -> Result<()> { + match cmd { + Command::Sync => sync_handler(), + Command::Diff => diff_handler(), + } +} + +// qual:allow(complexity) — large lookup table; splitting hurts readability +fn rule_table() -> &'static [Rule] { /* … */ } + +// qual:allow(architecture) — port adapter must call registry directly here +// for serialization round-trip; pure domain accessor would lose ordering. +use crate::adapters::registry::lookup; +``` + +Always pair with a rationale (`— `). Reviewers and future-you need to know *why* the rule is being bypassed. + +Legacy `// iosp:allow` is an alias for `// qual:allow(iosp)`. + +## `// qual:allow(unsafe)` — for `CX-006` specifically + +```rust +// qual:allow(unsafe) — FFI boundary, audited 2026-Q1 +unsafe fn raw_call() { /* … */ } +``` + +Separate path that does *not* count against `max_suppression_ratio`. Intended for FFI shims and low-level optimisations where `unsafe` is intrinsic. + +## `// qual:api` — public API entry points + +```rust +// qual:api — public re-export, callers live outside this crate +pub fn parse_config(input: &str) -> Result { /* … */ } +``` + +Excludes from: + +- `DRY-002` (dead code) — function isn't dead, it's exported. +- `TQ-003` (untested) — function may be tested by downstream consumers. + +Other dimensions still apply (complexity, IOSP, etc.). Doesn't count against `max_suppression_ratio`. + +## `// qual:test_helper` — `src/` helpers used only from `tests/` + +```rust +// qual:test_helper +pub fn assert_in_range(actual: f64, expected: f64, tol: f64) { + assert!((actual - expected).abs() < tol); +} +``` + +Same exclusions as `qual:api` (`DRY-002`, `TQ-003`). Use when a helper lives in `src/` so it's importable from integration tests in `tests/`, but isn't called from any production code. + +Differs from `ignore_functions` in `rustqual.toml`: `ignore_functions` silences *every* dimension on a function, while `qual:test_helper` only silences DRY-002 and TQ-003 — complexity / SRP / IOSP all still apply. + +## `// qual:inverse()` — inverse method pairs + +```rust +// qual:inverse(decode) +pub fn encode(input: &Value) -> Vec { /* … */ } + +// qual:inverse(encode) +pub fn decode(input: &[u8]) -> Result { /* … */ } +``` + +Suppresses the near-duplicate `DRY-001` finding between encode/decode pairs whose structural similarity is intentional. The annotation must be reciprocal — both functions name each other. Doesn't count against `max_suppression_ratio`. + +## `// qual:recursive` — recursive helpers + +```rust +// qual:recursive +fn collect_descendants(node: &Node, out: &mut Vec) { + out.push(node.id); + for child in &node.children { + collect_descendants(child, out); // self-call: ignored for IOSP reclassification + } +} +``` + +Removes self-calls from `own_calls` before the leaf-reclassification pass. Useful for tree/graph helpers that don't otherwise violate IOSP. Doesn't count against `max_suppression_ratio`. + +## Module-level suppression + +For *coupling* findings (which are module-global, not function-local), use the inner-doc form: + +```rust +//! qual:allow(coupling) — orchestration layer, intentionally depends on every adapter. + +use crate::adapters::a; +use crate::adapters::b; +// … +``` + +The `//!` form attaches to the module, not to a single item. + +## Suppression ratio (`SUP-001`) + +The `max_suppression_ratio` config (default 5%) caps how much of the codebase can be under `qual:allow`. Once you exceed that ratio, `SUP-001` warns (or errors with `--fail-on-warnings`). + +The intent is that suppression is for legitimate exceptions, not a backdoor. If you're hitting the cap, the right response is either: + +- Refactor the suppressed functions, or +- Loosen a threshold so the underlying findings stop firing legitimately, or +- Raise `max_suppression_ratio` *with intent* (and document why in `rustqual.toml`). + +Don't silently raise it to make the warning go away. The whole point of the cap is to keep suppression from drifting upward over months. + +## Orphan detection (`ORPHAN-001`) + +A `// qual:allow(...)` marker that *doesn't match a finding in its window* emits `ORPHAN-001`. This catches stale annotations after a refactor — the underlying issue is gone, but the suppression is still there. + +The detector reads raw complexity metrics against config thresholds, not the `*_warning` flags that suppressions clear. So if you bump a threshold, the finding stops firing, *and* the orphan check then flags the now-redundant suppression. Coupling-only markers are skipped because coupling warnings are module-global. + +`ORPHAN-001` is visible in every output format and counts toward `total_findings()`, `--fail-on-warnings`, and default-fail. + +## Composition-root and reexport-point modules + +Files marked as reexport points in `[architecture.reexport_points]` (typically `lib.rs`, `main.rs`, `bin/**`, `cli/**`, `tests/**`) bypass the layer rule entirely — no `qual:allow` needed. Use this for the composition root that wires layers together. + +## Summary: when to use which + +| You want… | Use | +|---|---| +| To skip one finding in production code temporarily | `// qual:allow()` with rationale | +| To mark a function as exposed externally | `// qual:api` | +| To mark a `src/` helper used only from `tests/` | `// qual:test_helper` | +| To accept structurally-similar inverse pairs | `// qual:inverse()` | +| To allow a recursive helper through IOSP | `// qual:recursive` | +| To accept FFI / `unsafe` blocks | `// qual:allow(unsafe)` | +| To exempt a whole module from coupling | `//! qual:allow(coupling)` | + +## Related + +- [reference-rules.md](./reference-rules.md) — every rule code each annotation can suppress +- [reference-configuration.md](./reference-configuration.md) — `max_suppression_ratio`, `ignore_functions`, layer rules +- [legacy-adoption.md](./legacy-adoption.md) — adoption patterns using suppressions vs baselines diff --git a/book/test-quality.md b/book/test-quality.md new file mode 100644 index 0000000..1da240c --- /dev/null +++ b/book/test-quality.md @@ -0,0 +1,154 @@ +# Use case: test quality + +Coverage is a number; *test quality* is whether the tests actually catch regressions. A 95%-covered codebase with assertion-free tests is worse than a 60%-covered one with sharp ones — the former gives false confidence, the latter is honest about what's checked. + +rustqual's Test Quality dimension measures both. It runs static checks on every test function (assertion presence, SUT calls, untested production functions) and — when LCOV coverage data is supplied — adds branch-level uncovered-logic detection. + +## What goes wrong + +- **Tests without assertions.** `#[test] fn it_works() { run_thing(); }` exercises code without checking anything. Coverage looks great, real coverage is zero. +- **Tests that don't call the SUT.** A test in `src/auth/tests/login.rs` that only constructs values and asserts on them — but never calls `login()` — isn't testing anything in `auth`. +- **Untested production functions.** A `pub fn` with no test anywhere — neither unit, nor integration — slipped through the review. +- **Uncovered logic branches.** A function is "tested" but the `else` arm of its main `if` was never executed. + +## What rustqual catches + +| Rule | Meaning | +|---|---| +| `TQ-001` | Test function has no assertions | +| `TQ-002` | Test function does not call any production function | +| `TQ-003` | Production function is untested (no test anywhere calls it) | +| `TQ-004` | Production function has no coverage (LCOV-based, requires `--coverage`) | +| `TQ-005` | Untested logic branches — covered function with uncovered lines | + +`TQ-001`, `TQ-002`, `TQ-003` are static and run on every analysis. `TQ-004` and `TQ-005` need LCOV data. + +## Static checks + +### TQ-001 — assertion-free tests + +A test is anything with `#[test]`, `#[tokio::test]`, etc. rustqual scans the function body for: + +- `assert!`, `assert_eq!`, `assert_ne!`, `debug_assert*!` (and configurable extras via `extra_assertion_macros`) +- Custom prefixes — anything starting with `assert*` or `debug_assert*` matches + +If none are present, `TQ-001` fires. The fix is usually to add an assertion; if the test exists for compile-time/typecheck reasons only, mark it: + +```rust +// qual:allow(test_quality) — compile-time check only, no runtime assertion needed +#[test] fn signatures_compile() { + let _: fn(&str) -> Result = parse; +} +``` + +### TQ-002 — tests without SUT + +A test that constructs values and asserts on them but never calls a production function. Usually a hint that: + +- the test is testing the test fixtures, not the system under test, or +- the SUT call moved out during a refactor and the test stayed. + +The check looks at all calls in the test body and verifies at least one resolves to a function inside `src/` (excluding test helpers). + +### TQ-003 — untested production functions + +Reverse-walk: for every `pub fn` in `src/`, is there a test (anywhere) that calls it? + +Three escape hatches: + +```rust +// qual:api — public-API entry, callers live outside this crate +pub fn parse_config(input: &str) -> Result { /* … */ } + +// qual:test_helper — used only from tests/ +pub fn build_test_session() -> Session { /* … */ } + +// qual:allow(test_quality) — initialised at startup, untestable in isolation +pub fn install_signal_handlers() { /* … */ } +``` + +`qual:api` and `qual:test_helper` don't count against `max_suppression_ratio`. They mirror the same annotations under DRY-002 (dead code), since a function that's untested *and* unused is usually one finding category, not two. + +## Coverage-based checks + +For TQ-004 and TQ-005, supply LCOV coverage data: + +```bash +cargo install cargo-llvm-cov +cargo llvm-cov --lcov --output-path coverage.lcov +rustqual --coverage coverage.lcov +``` + +### TQ-004 — uncovered production functions + +A `pub fn` whose lines never executed during tests. Differs from `TQ-003` in that it considers *runtime* coverage, not just static call graph: a function might be statically referenced from a test but the test path never executes its body. + +### TQ-005 — uncovered logic branches + +The most surgical of the test-quality checks. For functions that *are* covered, find the **uncovered lines** and report them as "untested logic". This is where coverage gaps actually hurt: a 95%-covered function whose 5% is the error-handling branch is one production incident away from a regression. + +``` +⚠ TQ-005 src/auth/login.rs::authenticate (line 48) + covered: 22/26 lines (84.6%) + uncovered branches at: 51, 52, 67, 88 +``` + +## Configure + +```toml +[test_quality] +enabled = true +# extra_assertion_macros = ["verify", "check", "expect_that"] +``` + +For projects with custom assertion DSLs (`mockall`, `proptest`, in-house frameworks), add the macro names to `extra_assertion_macros` so TQ-001 doesn't flag tests using them. + +## What you'll see + +``` +✗ TQ-001 tests/auth_test.rs::test_login (line 12) — no assertions +✗ TQ-002 tests/parser_test.rs::test_ast (line 38) — does not call any production fn +✗ TQ-003 src/api/handlers.rs::cmd_admin_purge (line 89) — untested +⚠ TQ-004 src/utils/format.rs::pad_left (line 22) — no coverage (LCOV) +⚠ TQ-005 src/auth/login.rs::authenticate (line 48) — 4 uncovered branches +``` + +## Strategy + +The TQ rules form a ladder, lightest to strictest: + +1. **TQ-001 / TQ-002** — fix first. Cheap, mechanical, eliminates fake tests. +2. **TQ-003** — second. Either write a test, mark `qual:api`, or delete the unused function. +3. **TQ-004** — once you have coverage in CI. Catches functions that compile but never run. +4. **TQ-005** — top of the ladder. Forces real branch-level testing. + +Most teams sit at level 2-3 and turn 4-5 on as the coverage culture matures. The dimension's quality score reflects all five, weighted; you don't have to enable every check on day one. + +## CI integration + +```yaml +- run: cargo install rustqual cargo-llvm-cov +- run: cargo llvm-cov --lcov --output-path lcov.info +- run: rustqual --coverage lcov.info --format github +``` + +Inline PR annotations show *exactly which test* lacks an assertion or *which production function* is uncovered. The agent or developer can fix the specific gap rather than guess from a coverage percentage. + +## Why this matters for AI-generated code + +AI agents are notorious for generating tests that exercise code without checking it — `let result = foo(); println!("{result:?}");` looks like a test, passes the type-checker, and bumps the coverage number. `TQ-001` catches that mechanically every PR. + +Pair it with the agent instruction file pattern from [ai-coding-workflow.md](./ai-coding-workflow.md): + +> Every test function must contain at least one assertion (`assert!`, `assert_eq!`, etc.). +> For public-API functions that are intentionally untested in this crate, mark with `// qual:api` instead of writing a stub test. + +The agent self-corrects; reviewer time goes to the actual logic, not to spotting fake tests. + +## Related + +- [function-quality.md](./function-quality.md) — IOSP and complexity for the production code being tested +- [code-reuse.md](./code-reuse.md) — `DRY-002` (dead code) and `TQ-003` (untested) share the call graph +- [ai-coding-workflow.md](./ai-coding-workflow.md) — agent instruction template that includes assertion rules +- [reference-rules.md](./reference-rules.md) — every rule code with details +- [reference-suppression.md](./reference-suppression.md) — `qual:api`, `qual:test_helper`, `qual:allow(test_quality)`