diff --git a/Cargo.lock b/Cargo.lock index 1375ba1..fec6a7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,9 +63,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.6.6" +version = "4.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "473c7e07f409a8d772161724aa8db6a765a2532a70f9667eeb7b49d3d02fbdca" +checksum = "aa8876b300ab35ba921adea3dfd70157a46249b33f95c9084ae5709785478946" dependencies = [ "clap_builder", "clap_derive", @@ -73,9 +73,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.6.6" +version = "4.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b48fea5a88e9ae728a2dcbedbfc0e730f7d60da42e1cb049a83c9fb8b789889" +checksum = "ec0797fb7aeb1406c84efac526901f7ec3ead2124f946b494e72879d4b54704d" dependencies = [ "anstream", "anstyle", @@ -85,9 +85,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.6.4" +version = "4.6.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d012d2b9d65aca7f18f4d9878a045bc17899bba951561ba5ec3c2ba1eed9a061" +checksum = "f9c751b79415d4e559e3d1fcf128e09e720eb673a06d26cf6f392d37d75b66e0" dependencies = [ "heck", "proc-macro2", @@ -97,9 +97,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "1.1.0" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" +checksum = "1c133bc6a41be0d194c306b5506d15e6feeea7b1d6604bd3f8310dfb2ca96486" [[package]] name = "colorchoice" @@ -113,13 +113,6 @@ version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7704b5fdd17b18ae31c4c1da5a2e0305a2bf17b5249300a9ee9ed7b72114c636" -[[package]] -name = "direct-cargo-bazel-deps" -version = "0.0.1" -dependencies = [ - "rnix", -] - [[package]] name = "hashbrown" version = "0.14.5" @@ -264,9 +257,9 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" [[package]] name = "syn" -version = "3.0.5" +version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12df2e0110f65b775f769bb17ef989067a1d931b2eb822bd4346631eeada89f9" +checksum = "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee" dependencies = [ "proc-macro2", "quote", @@ -281,9 +274,9 @@ checksum = "f18aa187839b2bdb1ad2fa35ead8c4c2976b64e4363c386d45ac0f7ee85c9233" [[package]] name = "unicode-ident" -version = "1.0.24" +version = "1.0.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +checksum = "ab72a15cf68d77cb0987d3684aa8a45c5ef827e8cb49ee2f30bfd7ba2feb519f" [[package]] name = "utf8parse" @@ -306,6 +299,10 @@ dependencies = [ "windows-link", ] +[[package]] +name = "xatu" +version = "0.1.0" + [[package]] name = "zmij" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index de6df26..2945153 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ members = [ "src/ariadne", "src/nix-style-linter", + "src/xatu", "src/machoke", ] resolver = "3" 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/xatu/BUILD b/src/xatu/BUILD new file mode 100644 index 0000000..c51e802 --- /dev/null +++ b/src/xatu/BUILD @@ -0,0 +1,20 @@ +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") + +rust_library( + name = "xatu_lib", + crate_name = "xatu", + srcs = ["lib.rs"], +) + +rust_binary( + name = "xatu", + crate_name = "xatu_cli", + srcs = ["main.rs"], + deps = [":xatu_lib"], +) + +rust_test( + name = "lib_test", + srcs = ["lib_test.rs"], + deps = [":xatu_lib"], +) diff --git a/src/xatu/Cargo.toml b/src/xatu/Cargo.toml new file mode 100644 index 0000000..1989b10 --- /dev/null +++ b/src/xatu/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "xatu" +version = "0.1.0" +edition = "2024" + +[lib] +path = "lib.rs" + +[[bin]] +name = "xatu" +path = "main.rs" + +[dependencies] diff --git a/src/xatu/lib.rs b/src/xatu/lib.rs new file mode 100644 index 0000000..e3fa985 --- /dev/null +++ b/src/xatu/lib.rs @@ -0,0 +1,5 @@ +pub const GREETING: &str = "Hello from Xatu"; + +pub fn greeting() -> &'static str { + GREETING +} diff --git a/src/xatu/lib_test.rs b/src/xatu/lib_test.rs new file mode 100644 index 0000000..d38e6d4 --- /dev/null +++ b/src/xatu/lib_test.rs @@ -0,0 +1,4 @@ +#[test] +fn greeting_returns_hello_message() { + assert_eq!(xatu::greeting(), "Hello from Xatu"); +} diff --git a/src/xatu/main.rs b/src/xatu/main.rs new file mode 100644 index 0000000..c532cbf --- /dev/null +++ b/src/xatu/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("{}", xatu::greeting()); +}