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` |