From f98af6ad3fd7a24560e40450406960d418987979 Mon Sep 17 00:00:00 2001 From: alexion Date: Wed, 16 Sep 2026 23:56:14 -0400 Subject: [PATCH] chore(nix): remove statix and deadnix from fixer Drop the statix and deadnix packages from the fix app now that the repository has a local Nix style linter path. Remove the obsolete statix configuration, document the evaluated rule sets and architectures, and keep vendored third-party files out of treefmt. --- AGENTS.md | 7 + docs/00 INBOX/ai/deadnix-architecture.md | 304 +++++++ docs/00 INBOX/ai/nix-statix-deadnix-rules.md | 791 +++++++++++++++++++ docs/00 INBOX/ai/statix-architecture.md | 261 ++++++ src/dotfiles/apps.nix | 5 - src/dotfiles/formatter.nix | 1 + statix.toml | 3 - 7 files changed, 1364 insertions(+), 8 deletions(-) create mode 100644 docs/00 INBOX/ai/deadnix-architecture.md create mode 100644 docs/00 INBOX/ai/nix-statix-deadnix-rules.md create mode 100644 docs/00 INBOX/ai/statix-architecture.md delete mode 100644 statix.toml diff --git a/AGENTS.md b/AGENTS.md index 993638c..abfc660 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,9 +98,14 @@ - Rust tests in this monorepo live in separate `*_test.rs` files, not inline `#[cfg(test)]` modules. - Ariadne Rust tests are declared in Bazel `BUILD` files; do not add `[[test]]` entries to `src/ariadne/Cargo.toml` unless explicitly requested. +- Rust package validation in this monorepo uses Bazel `rust_test` targets. + Do not claim or require `cargo test` as the standard Rust validation path unless explicitly requested. - In this monorepo, "dotfiles" refers to the dotfiles/NixOS configuration under `src/dotfiles/` and `configs/nixos/`. - The flake does not currently define `formatter.x86_64-linux`. Do not use `nix fmt` as a validation command until a formatter is added. +- Keep `third_party/**` excluded from treefmt. + Vendored code can contain syntax fixtures, generated files, or upstream formatting that local + formatters must not rewrite. - Ariadne extraction must preserve the game-root season prefix for archived source. Different game installs can contain compiled scripts with the same internal path. - Clew owns Ren'Py extraction, preparation, and compiled-script AST export. Ariadne owns analysis over Clew-produced data. @@ -114,3 +119,5 @@ Do not use `git subtree add` or `git subtree pull` for Pi PR branches. Gitea rebases the synthetic upstream root commit without the `third_party/pi` prefix. Preserve `git-subtree-dir` and `git-subtree-split` trailers on Pi vendor commits. +- `rules_rust` `crates_vendor` can vendor direct crate specs through the `packages` attribute. + Do not create a placeholder Rust package only to anchor a future vendored dependency. diff --git a/docs/00 INBOX/ai/deadnix-architecture.md b/docs/00 INBOX/ai/deadnix-architecture.md new file mode 100644 index 0000000..6312ca9 --- /dev/null +++ b/docs/00 INBOX/ai/deadnix-architecture.md @@ -0,0 +1,304 @@ +# Deadnix Architecture + +## Purpose + +This note explains how `deadnix` finds unused Nix code. +It focuses on architecture, not whether each check belongs in our linter. + +Use this note when deciding whether our linter needs scope analysis. + +## What Deadnix Is + +`deadnix` is a dead-code scanner for Nix. +It finds bindings that introduce names which no live code uses. + +It is not a general style linter. +It does not have many independent rules. +It implements one analysis family: unused declarations. + +Like `statix`, it parses source text with `rnix`. +It does not evaluate Nix. +It does not import files. +It reasons over local syntax and local scope only. + +## Minimal Dead-Code Vocabulary + +- A binding introduces a name. +- A scope defines where introduced names are visible. +- A use is an identifier occurrence that refers to a binding. +- Shadowing happens when an inner scope reuses an outer name. +- Dead code is a binding that no live code uses. +- A fixed point is the final result after repeated analysis no longer changes. + +Dead-code analysis needs more state than a simple syntax lint. +It must know which names a scope introduces and where those names can be used. + +## Component Map + +`deadnix` has fewer layers than `statix`. +The command-line entry point, file walk, parsing, analysis, reporting, and editing live in one small tool. + +The important concepts are separate modules: + +- settings control analysis options; +- scopes describe where names are introduced; +- bindings describe individual declared names; +- usage search answers whether a name appears in a live body; +- dead-code results connect a binding to its scope; +- reports render findings; +- edits remove or rename unused declarations. + +## Main Pipeline + +```mermaid +flowchart TD + A[Parse CLI options] --> B[Collect input files] + B --> C[Read each file] + C --> D[Parse with rnix] + D --> E[Skip file on parse errors] + D --> F[Run dead-code analysis] + F --> G[Print reports unless quiet] + G --> H[Apply edits when --edit is set] + H --> I[Exit 1 when --fail and findings exist] +``` + +The CLI accepts files or directories. +Directory mode walks `.nix` files. +Hidden files require an explicit option. +Excludes filter paths before analysis. + +The tool skips dead-code diagnostics when a file has parse errors. +It reports parse errors to the user and continues with other files. + +## Analysis Settings + +The analysis has four important switches. + +`no_lambda_arg` disables checks for single lambda arguments. +This can avoid noise when an API requires an argument name. + +`no_lambda_pattern_names` disables checks for names in attribute-set function patterns. +This matters for `callPackage`-style code, where argument names can be external API. + +`no_underscore` ignores bindings that start with `_`. +This treats `_name` as an intentional unused placeholder. + +`warn_used_underscore` reports underscore-prefixed bindings that code actually uses. +This enforces the convention that `_name` means unused. + +These settings are architecture-relevant because dead-code policy depends on API context. +A repository linter needs a way to express that context. + +## Scope Model + +`deadnix` recognizes four scope types. + +### Lambda pattern scope + +A lambda pattern is a function argument written as an attribute set pattern. +It can introduce field names and an optional alias for the whole argument set. + +Example shape: + +```nix +args @ { pkgs, lib, ... }: body +``` + +This scope can introduce `args`, `pkgs`, and `lib`. + +### Lambda argument scope + +A single lambda argument introduces one name. + +Example shape: + +```nix +pkgs: body +``` + +This scope introduces `pkgs`. + +### Let scope + +A `let` expression introduces bindings and inherited names. +Those names are available in binding expressions and in the body. + +Example shape: + +```nix +let + inherit (pkgs) hello; + value = hello; +in + value +``` + +This scope introduces `hello` and `value`. + +### Recursive attribute-set scope + +A `rec` attribute set introduces names that sibling attributes can reference. +`deadnix` models this scope, but treats its bindings as non-removable. + +That conservative choice avoids deleting public-looking attributes from recursive sets. + +## Binding Model + +A binding stores three facts. + +It stores the identifier name. +It stores the syntax node that declared the name. +It stores whether the binding is mortal. + +A mortal binding can become a dead-code finding. +A non-mortal binding participates in name lookup but will not be removed. + +Single lambda arguments that already start with `_` are non-mortal. +Recursive attribute-set bindings are also non-mortal. + +A binding also checks for the `deadnix: skip` pragma. +The pragma applies to a nearby following declaration. +The scanner searches backward from the declaration and stops after a blank-line boundary. + +## Usage Search + +Usage search asks whether a given name appears inside a syntax subtree. + +The search is scope-aware. +When it enters a child scope that binds the same name, it stops following that branch. +That prevents an inner binding from counting as a use of an outer binding. + +Attribute paths get special handling. +Ordinary attribute keys do not count as uses. +Dynamic or string attribute names can contain expressions, so the search looks inside those parts. + +`inherit` also gets special handling. +A scope can inherit names from an outer scope. +The analysis checks whether the inherited expression uses the searched name. + +## Fixed-Point Analysis + +`deadnix` repeats the scan until the result set stops changing. + +This matters because dead code can depend on other dead code. +A binding can appear used during the first pass because another binding references it. +If the referring binding is dead, the referenced binding is also dead. + +The loop tracks dead declaration nodes. +During later scans, uses inside already-dead bodies do not keep other bindings alive. + +This fixed-point loop is the central architectural difference from simple local syntax lints. +It makes dead-code results transitive. + +## Reporting Model + +A result stores the scope and binding for one finding. +The display text combines both parts. +Examples include unused lambda pattern, unused lambda argument, and unused let binding. + +Human output uses `ariadne`. +The reporter groups ranges by line and labels the exact binding names. +It uses different colors for different scope kinds. + +JSON output exists behind a feature flag. +It emits file, message, line, column, and end column. + +`NO_COLOR` disables colored human output. + +## Edit Model + +`deadnix --edit` converts findings into text edits. +Each edit has a start offset, end offset, and replacement string. + +The edit behavior depends on scope kind. + +For unused lambda pattern names, it removes the name and nearby comma or whitespace. +For unused pattern aliases, it removes the alias syntax. +For unused lambda arguments, it renames the argument with an underscore prefix. +For unused `let` assignments, it deletes the assignment. +For unused inherited names, it deletes only the inherited name. + +After those edits, it parses the edited source again. +Then it removes empty `let in` constructs and empty `inherit` statements. + +This cleanup pass prevents the first edit batch from leaving invalid or useless scaffolding. + +## Error Handling + +If a path cannot be stated or read, the CLI reports the file error and continues. +If a file has parse errors, the CLI reports those errors and skips dead-code analysis for that file. + +The `--fail` flag changes exit status when findings exist. +Without `--fail`, findings do not make the command fail. + +This design separates report generation from CI policy. + +## Architectural Strengths + +The scope model is explicit. +Each scope type declares which bindings it introduces and where those bindings are visible. + +The fixed-point loop catches transitive dead code. +A binding used only by dead code also becomes dead. + +The usage search handles shadowing. +That prevents common false positives in nested functions and nested `let` expressions. + +The edit model is tied to analysis results. +It knows whether a binding came from a lambda, pattern, `let`, or `inherit`. + +The tool has few moving parts. +That makes the analysis easy to follow compared to a general multi-rule linter. + +## Architectural Limits + +`deadnix` does not evaluate Nix. +It cannot know which names external callers depend on. +This matters for module arguments, package function arguments, and public attribute sets. + +It has global switches rather than project-specific rule contexts. +For example, all lambda pattern names can be checked or skipped. +The tool does not know that one directory contains `callPackage` functions. + +The analysis is local to one file. +It cannot see that another file imports a function and relies on its argument names. + +The fixer performs text edits with byte ranges. +It must handle punctuation and whitespace carefully. +Pattern edits are harder than simple assignment deletion. + +The tool has one analysis family. +It is not a framework for many unrelated rules. + +## Lessons for Our Linter + +Dead-code checks need a scope pass. +A per-node rule interface is not enough when a diagnostic depends on name visibility. + +Represent scopes explicitly. +Each scope type should provide introduced bindings and visible bodies. + +Track shadowing before reporting unused names. +A text search for an identifier is not sufficient. + +Use fixed-point analysis for transitive dead code. +One pass misses names used only by dead declarations. + +Separate policy from mechanics. +The mechanics can find unused names. +Repository policy decides whether module arguments, package arguments, or underscore names count. + +Treat autofix as scope-specific. +Removing a pattern name is not the same operation as deleting a `let` assignment. + +Run cleanup after edits. +Dead-code fixes can create empty `let` expressions and empty `inherit` statements. + +## Open Design Questions for Our Linter + +- Should dead-code analysis be part of `nix-style-linter`, or a separate command? +- Should policy vary by path, file kind, or suppression comment? +- Should recursive attribute-set members ever become removable findings? +- Should underscore names mean ignored, or should used underscore names be errors? +- Should the linter report unused names without offering autofix first? +- Should cleanup rules run as ordinary rules or as a post-fix pass? diff --git a/docs/00 INBOX/ai/nix-statix-deadnix-rules.md b/docs/00 INBOX/ai/nix-statix-deadnix-rules.md new file mode 100644 index 0000000..5c07dc2 --- /dev/null +++ b/docs/00 INBOX/ai/nix-statix-deadnix-rules.md @@ -0,0 +1,791 @@ +# Nix Statix and Deadnix Rules + +## Purpose + +This note rewrites the current `statix` and `deadnix` findings as possible `nix-style` rules. +Use the `Decision` line on each rule for `keep`, `discard`, `update`, or `need-more-info`. + +## Source Baseline + +- `statix` rule names come from `nix run --inputs-from . nixpkgs#statix -- list`. +- `deadnix` checks come from `nix run --inputs-from . nixpkgs#deadnix -- --help`. +- `deadnix` reports categories instead of named rules. +- At collection time, `src/dotfiles/apps.nix` ran `statix fix .` and `deadnix --edit` in `nix run .#fix`. +- At collection time, `statix.toml` disabled `repeated_keys`. + +## Statix Rules + +### `bool_comparison` + +Decision: keep + +Source: `statix` `W01`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Do not compare a boolean expression to `true` or `false`. +Use the expression directly for true checks. +Use `!` for false checks. + +Avoid: + +```nix +x == true +x == false +``` + +Use: + +```nix +x +!x +``` + +#### Rationale + +Boolean literal comparisons add noise without changing behavior. +Direct predicates make conditions easier to scan. + +### `empty_let_in` + +Decision: keep + +Source: `statix` `W02`. +Status at collection time: enabled. +Autofix: yes when the `let` body has no comments. + +#### Rule + +Do not keep a `let` expression that declares no bindings. +Replace it with its body. + +Avoid: + +```nix +let in value +``` + +Use: + +```nix +value +``` + +#### Rationale + +An empty `let` adds syntax without naming a dependency. +It usually remains after editing or debugging. + +### `manual_inherit` + +Decision: Keep + +Source: `statix` `W03`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Use `inherit` when an attribute assignment repeats the same name as its value. + +Avoid: + +```nix +{ + package = package; +} +``` + +Use: + +```nix +{ + inherit package; +} +``` + +#### Rationale + +`inherit` states that the attribute comes from the current scope. +It removes duplicated names. + +### `manual_inherit_from` + +Decision: + +Source: `statix` `W04`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Use `inherit (source)` when an assignment selects an attribute with the same name. + +Avoid: + +```nix +package = pkgs.package; +``` + +Use: + +```nix +inherit (pkgs) package; +``` + +#### Rationale + +`inherit (source)` states that the value comes from an attribute set. +It avoids repeated selector names. + +### `legacy_let_syntax` + +Decision: keep + +Source: `statix` `W05`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Do not use legacy `let { ... }` syntax. +Use documented Nix expressions. + +Avoid: + +```nix +let { + body = x + y; + x = 2; + y = 3; +} +``` + +Use: + +```nix +rec { + body = x + y; + x = 2; + y = 3; +}.body +``` + +#### Rationale + +Legacy `let` syntax is undocumented. +Documented forms are clearer to tools and readers. + +### `collapsible_let_in` + +Decision: keep + +Source: `statix` `W06`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Collapse a `let` expression when its body is another `let` expression. +Put both binding groups in one `let` when scope permits it. + +Avoid: + +```nix +let + a = 1; +in +let + b = 2; +in + a + b +``` + +Use: + +```nix +let + a = 1; + b = 2; +in + a + b +``` + +#### Rationale + +Nested `let` expressions add structure when they do not add a scope boundary. +A single binding group shows the local dependencies together. + +### `eta_reduction` + +Decision: keep + +Source: `statix` `W07`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Remove a lambda that only passes its argument to another function. +Keep the target function name when that preserves behavior. + +Avoid: + +```nix +map (x: double x) values +``` + +Use: + +```nix +map double values +``` + +#### Rationale + +The wrapper lambda adds no behavior. +The direct function reference exposes the operation. + +### `useless_parens` + +Decision: keep + +Source: `statix` `W08`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Remove parentheses around expressions when they do not affect parsing. + +Avoid: + +```nix +let + double = (x: x * 2); +in + (value) +``` + +Use: + +```nix +let + double = x: x * 2; +in + value +``` + +#### Rationale + +Redundant parentheses make expressions harder to scan. +Required precedence parentheses remain valid. + +### `empty_pattern` + +Decision: keep + +Source: `statix` `W10`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Use `_` instead of an empty variadic function pattern when the argument is ignored. +Do not apply this rule to NixOS-style modules that declare `imports`. + +Avoid: + +```nix +{ ... }: value +``` + +Use: + +```nix +_: value +``` + +#### Rationale + +`_` directly marks an ignored argument. +An empty variadic pattern looks like partial destructuring. + +### `redundant_pattern_bind` + +Decision: keep + +Source: `statix` `W11`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Do not bind a variadic pattern when the pattern captures no fields. +Use a single argument name instead. + +Avoid: + +```nix +args @ { ... }: args.nixpkgs +``` + +Use: + +```nix +args: args.nixpkgs +``` + +#### Rationale + +The empty pattern adds no destructuring value. +A named argument shows that the whole argument is used. + +### `unquoted_uri` + +Decision: keep + +Source: `statix` `W12`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Quote URI literals. +Do not use Nix's special unquoted URI syntax. + +Avoid: + +```nix +url = github:owner/repository; +``` + +Use: + +```nix +url = "github:owner/repository"; +``` + +#### Rationale + +Quoted strings work for all URI forms. +They avoid ambiguity with punctuation after the URI. + +### `empty_inherit` + +Decision: keep + +Source: `statix` `W14`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Remove `inherit` statements that inherit no names. + +Avoid: + +```nix +inherit; +``` + +Use no replacement. + +#### Rationale + +An empty `inherit` has no effect. +It usually remains after a refactor. + +### `deprecated_to_path` + +Decision: keep + +Source: `statix` `W17`. +Status at collection time: enabled. +Autofix: no. + +#### Rule + +Do not use `builtins.toPath` or unqualified `toPath`. +Use explicit path construction instead. + +Avoid: + +```nix +builtins.toPath "/path" +``` + +Use: + +```nix +/. + "/path" +./. + "/relative-path" +``` + +#### Rationale + +`toPath` is deprecated. +Explicit path construction states whether the result is absolute or relative. + +### `bool_simplification` + +Decision: keep + +Source: `statix` `W18`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Use `!=` instead of negating an equality comparison. + +Avoid: + +```nix +!(x == y) +``` + +Use: + +```nix +x != y +``` + +#### Rationale + +The direct operator expresses inequality without nested boolean syntax. + +### `useless_has_attr` + +Decision: keep + +Source: `statix` `W19`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Use `or` when an `if` expression only checks for an attribute before selecting it. + +Avoid: + +```nix +if x ? a then x.a else default +``` + +Use: + +```nix +x.a or default +``` + +#### Rationale + +The `or` form is the Nix operator for attribute defaults. +It avoids repeating the attribute path. + +### `repeated_keys` + +Decision: update so that it doesn't conflict with the custom module convention. + +Source: `statix` `W20`. +Status at collection time: disabled in `statix.toml`. +Autofix: no. + +#### Rule + +Group repeated attribute path prefixes in a nested attribute set. +Apply this rule only when at least three sibling assignments share the same first key. +Do not apply it inside `rec` attribute sets. + +Avoid: + +```nix +{ + services.a = 1; + services.b = 2; + services.c = 3; +} +``` + +Use: + +```nix +{ + services = { + a = 1; + b = 2; + c = 3; + }; +} +``` + +#### Rationale + +Grouping repeated prefixes reduces visual repetition. +The local config disables this rule, so it needs an explicit decision before promotion. + +### `empty_list_concat` + +Decision: keep + +Source: `statix` `W23`. +Status at collection time: enabled. +Autofix: yes. + +#### Rule + +Remove list concatenation with an empty list. + +Avoid: + +```nix +[] ++ values +values ++ [] +``` + +Use: + +```nix +values +``` + +#### Rationale + +Concatenating `[]` changes no values. +It usually remains after conditional list construction changed. + +## Deadnix Checks + +### Unused `let` bindings + +Decision: keep + +Source: `deadnix` default check. +Status at collection time: active in `nix run .#fix`. +Autofix: yes. + +#### Rule + +Remove `let` bindings when no live expression uses their names. +Do not keep a binding only because another dead binding uses it. +Use `# deadnix: skip` only for intentional unused declarations. + +Avoid: + +```nix +let + unused = 1; +in + value +``` + +Use: + +```nix +value +``` + +#### Rationale + +Unused bindings hide the dependencies that matter. +They also make refactors look unfinished. + +### Unused `inherit` names + +Decision: keep + +Source: `deadnix` default check. +Status at collection time: active in `nix run .#fix`. +Autofix: yes. + +#### Rule + +Remove names from `inherit` statements when no live expression uses those names. +Remove the whole `inherit` statement when it becomes empty. + +Avoid: + +```nix +let + inherit (builtins) unused; +in + value +``` + +Use: + +```nix +value +``` + +#### Rationale + +Unused inherited names create the same noise as unused `let` bindings. +An empty result also overlaps with `statix` `empty_inherit`. + +### Unused lambda arguments + +Decision: keep + +Source: `deadnix` default check. +Status at collection time: active in `nix run .#fix`. +Autofix: yes. + +#### Rule + +Mark an unused single lambda argument with `_`. +Do not keep a meaningful name for an ignored argument. + +Avoid: + +```nix +arg: value +``` + +Use: + +```nix +_: value +``` + +#### Rationale + +`_` states that the argument is intentionally ignored. +A named unused argument implies missing logic. + +### Unused lambda pattern names + +Decision: keep + +Source: `deadnix` default check. +Status at collection time: active in `nix run .#fix`. +Autofix: yes. + +#### Rule + +Remove unused names from lambda attribute patterns. +Keep only names used by the body or defaults. +Use `...` when callers can pass extra attributes. + +Avoid: + +```nix +{ used, unused, ... }: used +``` + +Use: + +```nix +{ used, ... }: used +``` + +#### Rationale + +Pattern names usually declare dependencies. +Unused names overstate the function's inputs. + +### Unused lambda pattern aliases + +Decision: keep + +Source: `deadnix` default check. +Status at collection time: active in `nix run .#fix`. +Autofix: yes. + +#### Rule + +Remove a lambda pattern alias when the body does not use the alias. +Keep the alias only when code reads the whole argument set. + +Avoid: + +```nix +args @ { used, ... }: used +``` + +Use: + +```nix +{ used, ... }: used +``` + +#### Rationale + +The alias says the function needs the complete input set. +Keeping it unused makes the function contract look wider than it is. + +### Ignored underscore bindings + +Decision: keep + +Source: `deadnix` `--no-underscore` option. +Status at collection time: inactive; current fixer does not pass `--no-underscore`. +Autofix: no separate rule. + +#### Rule + +Treat bindings whose names start with `_` as intentionally ignored. +Optionally skip unused diagnostics for those names. + +Avoid, when the skip policy is active: + +```nix +let + _unused = expensiveValue; +in + value +``` + +Use only for intentional placeholders. + +#### Rationale + +An underscore prefix is a common marker for intentionally unused names. +Skipping all underscore bindings can hide real dead code. + +### Used underscore bindings + +Decision: keep + +Source: `deadnix` `--warn-used-underscore` option. +Status at collection time: inactive; current fixer does not pass `--warn-used-underscore`. +Autofix: no. + +#### Rule + +Do not use a binding whose name starts with `_`. +Rename it to a normal name when code references it. + +Avoid: + +```nix +let + _package = pkgs.hello; +in + _package +``` + +Use: + +```nix +let + package = pkgs.hello; +in + package +``` + +#### Rationale + +The underscore prefix communicates that the name is intentionally unused. +Using it contradicts that signal. + +## Notes for Decisions + +- `statix` rules are mostly syntax rewrites. +- `deadnix` rules need scope analysis and dead-code reachability. +- `deadnix` does not report `rec` attribute-set bindings as removable findings. +- `statix` `repeated_keys` is present in the installed rule list but disabled locally. +- Rules that change module argument patterns need extra care around NixOS modules and `callPackage` functions. diff --git a/docs/00 INBOX/ai/statix-architecture.md b/docs/00 INBOX/ai/statix-architecture.md new file mode 100644 index 0000000..ac48494 --- /dev/null +++ b/docs/00 INBOX/ai/statix-architecture.md @@ -0,0 +1,261 @@ +# Statix Architecture + +## Purpose + +This note explains how `statix` works as a Nix linter and fixer. +It focuses on architecture, not the value of each rule. + +Use this note when deciding how complete our repository-local Nix linter should become. + +## What Statix Is + +`statix` is a syntax-tree linter for Nix. +It parses source text into a Nix AST, walks that tree, and runs rules on matching nodes. + +It does not evaluate Nix. +It does not import files. +It does not know the final value of an attribute set. +It sees source structure, not runtime semantics. + +That design makes `statix` fast and deterministic. +It also limits it to rules that the parser can prove from local syntax. + +## Minimal Linter Vocabulary + +- Source text is the bytes in a Nix file. +- A parser converts source text into a syntax tree. +- A syntax tree records nodes such as lambdas, lists, strings, and operators. +- A token is a leaf item such as an identifier, comment, or URI literal. +- A rule inspects one syntax element and optionally returns a diagnostic. +- A diagnostic reports a span and a message. +- A suggestion adds a replacement for that span. +- A fixer applies suggestions back to source text. + +## Component Map + +The upstream architecture document names four major components. + +### CLI component + +The CLI owns operator interaction. +It parses subcommands and options, loads configuration, discovers files, and prints output. + +It supports these user flows: + +- `check`: report diagnostics. +- `fix`: apply all safe suggestions. +- `single`: apply one suggestion at a position. +- `explain`: print the long explanation for a rule code. +- `dump`: print a sample configuration file. +- `list`: print all available rules. + +### Library component + +The library owns lint rules and the data structures that rules return. +It defines `Report`, `Diagnostic`, `Suggestion`, `Rule`, `Metadata`, `Explain`, and `Lint`. + +Each rule is a small unit. +It receives one syntax element and returns zero or one report. + +The library also contains helpers that build replacement syntax. +Those helpers parse small replacement fragments back into AST nodes. +This makes fixes use valid Nix syntax instead of raw string assembly. + +### Virtual filesystem component + +The virtual filesystem stores file paths and file contents in memory. +It assigns each file a small `FileId`. + +The CLI reads files into this store before linting. +Diagnostics then carry `FileId` references instead of path strings. + +The VFS supports sequential iteration and parallel iteration. +`statix check` uses parallel iteration across files. + +### Macro component + +The macro component reduces rule boilerplate. +A `lint` macro generates rule metadata, explanation support, and the static rule object. + +A rule author writes the rule structure, metadata, documentation, and validation logic. +The macro wires that rule into the common linter interface. + +## Check Pipeline + +```mermaid +flowchart TD + A[Parse CLI options] --> B[Discover configuration] + B --> C[Build ignore set] + C --> D[Walk Nix files] + D --> E[Read files into VFS] + E --> F[Build active lint map] + F --> G[Parse each file with rnix] + G --> H[Walk syntax tree] + H --> I[Run rules matching each syntax kind] + I --> J[Collect reports] + J --> K[Render diagnostics] + K --> L[Exit 1 when reports exist] +``` + +The key optimization is the active lint map. +`statix` groups rules by `SyntaxKind` before it walks a file. +During traversal, it looks up only rules that declared interest in the current node kind. + +This avoids running every rule on every node. +It also keeps each rule narrow. + +## Rule Registration + +Rules are built into the binary. +There is no dynamic rule loading. + +A registry macro imports each rule module and builds a static `LINTS` vector. +The CLI filters that vector with configuration. +Disabled rule names disappear from the active lint map. + +This model has a simple trust boundary: + +- all rule code ships with the tool; +- configuration can disable rules; +- configuration cannot define new rules. + +## Rule Execution Model + +Each rule declares the syntax kinds it wants to inspect. +For example, one rule can ask for binary-operation nodes. +Another rule can ask for URI tokens. + +The traversal visits tree entries with `preorder_with_tokens`. +That includes both nodes and tokens. +Rules can therefore inspect structured AST nodes and raw tokens. + +A rule returns a `Report` when it finds a problem. +The report contains one or more diagnostics. +Each diagnostic has a source range and a message. +A diagnostic can also include a `Suggestion`. + +Rules do not share state with each other. +They also do not run a whole-program analysis. +A rule sees the syntax element passed to it and whatever children or parents it chooses to inspect. + +## Parse Errors + +`statix` turns parser errors into reports. +Those reports use code `0` and severity `Error`. + +Parse errors flow through the same output machinery as lint findings. +This lets the CLI render syntax errors and lint diagnostics with one reporter. + +## Output Model + +`statix` has several output formats. + +The human format uses `ariadne` to render annotated source snippets. +The `errfmt` format prints one compact line per diagnostic. +The JSON format exists behind a feature flag. + +The output layer receives reports and the VFS. +It converts byte ranges into line and column positions at render time. + +This separates rule logic from display logic. +Rules do not need to know how the terminal or editor consumes diagnostics. + +## Fix Pipeline + +A fixable diagnostic contains a `Suggestion`. +A suggestion contains the range to replace and replacement syntax. + +`statix fix` uses the same parser and rule walk as `check`, but filters for reports that contain suggestions. +It then sorts and filters suggestions to avoid overlapping edits. + +The fixer applies selected reports to a copy of the source. +Then it parses and scans the changed source again. +It repeats this process until no more suggestions appear. + +That iterative design matters because one edit can expose a later edit. +It also avoids applying stale ranges after source text changes. + +## Single Fix Pipeline + +The `single` command applies one fix at a requested line and column. +It converts the line and column to a byte offset. +Then it finds the first fixable report whose diagnostic range contains that offset. + +This shape supports editor workflows. +An editor can request a fix for the diagnostic under the cursor. + +## Configuration Model + +Configuration is small. +A `statix.toml` file can disable rules by name. +It can also add ignore globs. + +The CLI discovers configuration by walking ancestors from the configured path. +If no config exists, it uses defaults. + +The command also respects `.gitignore` unless unrestricted mode is enabled. +Ignore handling happens before parsing. + +## Architectural Strengths + +`statix` has a clean separation between these concerns: + +- file discovery; +- parsing; +- rule selection; +- rule execution; +- reporting; +- fixing. + +Rules stay small because the framework handles traversal and output. +Fixes are safer because replacement syntax comes from parsed Nix fragments. +The lint map avoids unnecessary rule calls. +Parallel file traversal improves throughput for large trees. + +The static rule registry is easy to audit. +It fits a repository-local linter well when we want deterministic behavior. + +## Architectural Limits + +`statix` does not evaluate Nix. +It cannot know imported module results, overlays, option merges, or generated attributes. + +Most rules must remain local syntax rules. +A rule that needs semantic knowledge can easily become wrong. + +Fixes rely on byte ranges in source text. +They must avoid overlapping ranges. +They must rerun after edits because previous ranges become stale. + +The macro system reduces rule boilerplate, but it hides generated behavior. +That tradeoff is useful for many rules, but it can make debugging harder. + +## Lessons for Our Linter + +Start with a clear rule interface. +A rule should declare its name, matching syntax kinds, and diagnostic output. + +Keep parsing and diagnostics separate from output. +The same diagnostic should support terminal output, editor output, and tests. + +Use syntax-kind indexing when rules are local. +It prevents unnecessary calls and makes rule ownership explicit. + +Treat autofix as a second contract. +A diagnostic can exist without a safe fix. +A fix must define exact replacement ranges and avoid overlap. + +Rerun analysis after applying a batch of fixes. +Source ranges become invalid after edits. + +Do not add Nix evaluation accidentally. +If a rule needs evaluated values, design that as a separate analysis layer. + +## Open Design Questions for Our Linter + +- Should our linter support fixes, or stay report-only for now? +- Should rules register by syntax kind like `statix`? +- Should our suppression model live in the parser layer or rule layer? +- Should parse errors use the same diagnostic type as style rules? +- Should editor-friendly output be part of the first complete architecture? +- Should future semantic rules use a separate pass from local syntax rules? diff --git a/src/dotfiles/apps.nix b/src/dotfiles/apps.nix index a8c7e71..123b6b1 100644 --- a/src/dotfiles/apps.nix +++ b/src/dotfiles/apps.nix @@ -15,14 +15,9 @@ in name = "fix"; runtimeInputs = [ formatter - pkgs.statix - pkgs.deadnix ]; text = '' treefmt - statix fix . - deadnix --edit --exclude configs/nixos/hosts/*/hardware-configuration.nix -- . - treefmt ''; } ); diff --git a/src/dotfiles/formatter.nix b/src/dotfiles/formatter.nix index f4cf85d..86a19d8 100644 --- a/src/dotfiles/formatter.nix +++ b/src/dotfiles/formatter.nix @@ -6,5 +6,6 @@ inputs.treefmt-nix.lib.evalModule inputs.nixpkgs.legacyPackages.${system} { settings.formatter.nixfmt.excludes = [ "configs/nixos/hosts/*/hardware-configuration.nix" + "third_party/**" ]; } diff --git a/statix.toml b/statix.toml deleted file mode 100644 index 1a0d92f..0000000 --- a/statix.toml +++ /dev/null @@ -1,3 +0,0 @@ -disabled = [ - "repeated_keys", -]