Check a repository against one rule Replaces #5, which GitHub will not let me reopen: I merged it by mistake and then took it back off master, so the commits are no longer there. Stacked on the merged #4. `hopinion check .` reads every module of every cabal component and reports what the rules have to say about it. One rule ships. 51 files, 3,766 lines. 110 tests. ## The invariants, each of which fails the run when broken **A rule says there is something to fix, or there is not.** No third answer. A checker whose silence can mean "I could not tell" is a checker whose silence means nothing. So a module that did not parse is a failure of the run rather than a module with nothing wrong in it, and a module whose source is a preprocessor's input is counted as unread rather than passing quietly. **A rule is a value, not an instance of a class.** No generic code is polymorphic in a check type, so a record of functions is the right encoding. Adding a rule is a module and a line in `Hopinion.Rule.Registry`, plus a directory under `hopinion-gen/test_resources/Rule` named after it, which `Hopinion.RuleSpec` turns into a golden test per file with no test code to write. **Facts come from the token stream and the parse tree, never from exact-print annotations.** That placement answers "where must this be reprinted", not "what is this about", and the two differ exactly where it would matter. **Every failure is a type, and one function turns it into words.** `Failure`, `RuleSetError`, `DiscoveryError` and `ReportError` each say what happened rather than carrying the sentence about it, and each has exactly one renderer. Three of `Failure`'s constructors do carry rendered text, deliberately and with the reason written down: what they are about is an absolute path, which under Nix is a store path nobody reading a report can navigate to, so there is nothing for a consumer to do with it but print the sentence. **Every path is a typed path.** `path` and `path-io` throughout, with no `FilePath` in any signature. What a person writes on the command line is parsed at the boundary: a repository root is a `SomeBase Dir`, so it is resolved once against the working directory, and the module command's argument is a `Path Rel File`, so an absolute one is refused by the option parser rather than thrown from the middle of a run. Existence is asked with `forgivingAbsence` rather than a `doesFileExist` before a read that can still race. **Nothing is built for a rule that does not exist yet.** One level, no fact store, no cross-process split, nothing reads what the compiler wrote down. The build holds that line rather than my judgement: `-Wunused-packages`, `-Wunused-top-binds` and weeder run as errors and removed the last of the machinery this rule turned out not to need. ## The rule `HsNoCustomShowRead`, from `haskell-style.md`: `Show` and `Read` are derived, never written out. An `InstanceFact` whose class is one of those and whose origin is a written instance declaration. ## This is deliberately not adoptable yet It finds 22 real instances on nix-ci. All 22 are the secret-hiding `Show` instances that `haskell-style.md` explicitly permits, and there is no way to tell hopinion so, because there is no suppression mechanism yet. That is the argument for the next PR rather than a gap I am hiding: the rule the plan already annotates with "escape hatch needed for secret data" is the one that makes suppressions unavoidable. ## Where to start reading `Hopinion.Rule` for what a rule is, then `Hopinion.Check.Hs.NoCustomShowRead` for what one looks like, then `Hopinion.Run` for how they are run. `Hopinion.Facts` and its neighbours are the vocabulary everything else is a function between. ## Since the first review Four things came out of the inline comments and are in this diff: - **No README changes.** It stays at master's six lines in every PR of the stack. - **No statistics.** `RunStats`, `--stats` and `renderStats` are gone, and with only complaints left `RunReport` was a one-field record, so a run produces `Complaints` directly. - **All non-diagnose output is `[Chunk]`**, through `safe-coloured-text`. Colour and nothing else: `WithoutColours` strips colours but emits `bold`/`faint`/`italic`/`underline` regardless, so styling with those would put escape sequences in every captured build log. A test pins that the plain rendering holds no `\ESC`. - **No `RelPath` newtype.** `Path Rel File` is the domain type; the codec is a value rather than an instance so it needs no orphan. The one wrapper left is `StoredPath` in `Hopinion.Store`, because persistent's quasi-quoter needs a name it can own an instance for. ## Review loop ``` nix flake check nix develop --command cabal test hopinion-test --test-options="--ai-executor" ``` ## What comes after | PR | adds | brought in by | |---|---|---| | #10 | comment attachment, scopes, suppressions, `hopinion.yaml` | `CommentBareTodo` | | #11 | the fact store and the package level | `HsGenValidInGenPackage` | | #12 | the compiler's artifacts, the process split, the Nix builders | `TestGenValidSpecPerGenValid` |