Ask for an error type rather than a String
A new module-level rule, `HsNoStringError`: an error is a type of its own, not a `String`.
## What it reports
**A failure written into a type that carries one** — the first argument of `Either`, `Except`, `ExceptT`, `MonadError` and `Validation`, when that argument is `String`, `Text`, `[Char]`, or a list or `NonEmpty` of those:
```
[error HsNoStringError]: The error type of Either is String.
8 | parseAge :: String -> Either String Int
* ------
* `- An error is a type of its own, not a String.
```
**A failure given a name of its own and nothing else**, because naming the prose is the obvious way to answer the first half without changing anything:
```
The error type ParseError is Text. -- type ParseError = Text
The error type LookupError wraps nothing but String. -- newtype LookupError = LookupError String
The error type ValidationErrors is Text. -- type ValidationErrors = NonEmpty Text
```
A caller can branch on a constructor and cannot branch on prose, so a `String` error asks every handler to treat every failure the same way, and the words are chosen at the point of failure, which is the place that knows least about who will read them.
## Three things worth reviewing
**`Validation` is narrowed on purpose.** `base`, `mtl` and `transformers` fix the argument order of the other four carriers, so matching those names is matching the types we think they are. Nothing fixes `Validation`: several packages define it, and one that put its success first would read identically here. Only a list or a `NonEmpty` of prose is read as prose in that position, which is a shape a success-first type would not have. It costs no recall in practice — all eight `Validation` findings in the try-out were already `Validation (NonEmpty Text)`.
**A wrapper counts only when its constructor echoes the type name**, with or without `Mk`. That is the shape a reader learns nothing from, and its renderer is `id`:
```haskell
newtype NarInfoParseError = NarInfoParseError String
renderNarInfoParseError (NarInfoParseError s) = s -- the String IS the message
data RepoTypeParseError = UnknownRepoType !Text
renderRepoTypeParseError (UnknownRepoType rt) = concat ["Unknown Repo Type: ", ...]
```
Reporting every one-constructor wrapper found 17 across the seven repositories below; 11 of them were the second shape, written deliberately. Reporting only the echo finds 3. A `data` with two constructors is the fix, so it is never reported.
**The traversal deduplicates.** A type node arrives more than once: a strict field is a bang around parentheses around the application, and all three answer for the same error argument. Applications are peeled and deduplicated by location, and the function half of an application is dropped, since the `Either String` inside `Either String Int` is the same `String`. Without both, `attemptOutcome :: !(Either String Int)` reported three times.
`peelType` and `peelApp` move from `Hopinion.Extract` to `Hopinion.Extract.Ghc`, which is where the reading no rule owns lives. Neither function changed.
## Tried out on seven repositories
**1534 findings, no false positives, no parse failures.**
| repository | findings | of which declarations |
| --- | --- | --- |
| kronor | 1265 | 0 |
| graphql-engine | 96 | 2 |
| smos | 60 | 0 |
| salsa | 54 | 0 |
| nix-ci8 | 40 | 1 |
| sydtest | 18 | 0 |
| validity | 1 | 0 |
I read every distinct flagged line in all seven apart from the generated clients, and opened every one of the 17 candidate wrappers to see what its renderer does. All five carriers fire on real code: `Validation (NonEmpty Text) Text` through kronor's validation modules, `Except String` and `MonadError Text` in graphql-engine.
Three findings are worth singling out. `validity` has exactly one, `prettyValidate :: (Validity a) => a -> Either String a`, which is where a good share of the `Either String` in smos and salsa comes from in the first place. And `StorePathSpec.hs` and `GlobSpec.hs` in nix-ci8 both assert the exact wording of a failure, because there is no constructor to compare.
**Generated clients dominate the big number.** 1090 of kronor's 1265 are under `third-party-apis/`, and 20 of the smos and salsa findings are in their Stripe clients: all `eitherDecodeStrict body :: Either String ...` in generated OpenAPI code, which no suppression can be written into. A consuming repository leaves those packages out of the `packages` list.
## What it does not see
**An `Either String` that is never written down.** Across nix-ci8, smos and salsa there are 95 `Left "literal"` sites, and 14 files hold one and get no finding at all, because a library combinator supplies the type and the lambda handed to `bimapCodec`, `eitherReader`, `checkMapEither` or `fromPersistValue` only writes `Left "..."`. `good.hs` pins that silence as a decision.
**A wrapper named better than it is.** The echo test is one rename wide, and that is not hypothetical: `SecretNameParseError`, `UsernameParseError` and `EventPayloadParseError` in nix-ci8 all name their constructor for the failure and then render with `id`, so the payload is the message and the rule stays quiet. What decides it is the renderer, which is a module away from the declaration. Reporting those would mean reading renderers, which is a larger rule than this one.
Both are documented in `good.hs` rather than left to be rediscovered.
## This repository
One finding: `fromPersistValueViaCodec`, which is `persistent`'s `fromPersistValue` under another name. It carries an `[allow:HsNoStringError]` with that reason, and the paragraph in the module header that said the same thing is gone, so it is said once.
All 26 nix-ci checks pass.