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
The first argument of `Either`, `Except`, `ExceptT`, `MonadError` and `Validation`, when that argument is `String`, `Text`, `[Char]`, or a list or `NonEmpty` of those. The finding underlines the argument alone, not the whole signature, and names the carrier:
```
[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 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 written as `Validation (NonEmpty Text)`, and the totals below are unchanged by the narrowing.
**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` moved** from `Hopinion.Extract` to `Hopinion.Extract.Ghc`, which is where the reading no rule owns lives. Neither function changed.
## Tried out on seven repositories
**1531 findings, no false positives, no parse failures.**
| repository | findings |
| --- | --- |
| kronor | 1265 |
| graphql-engine | 94 |
| smos | 60 |
| salsa | 54 |
| nix-ci8 | 39 |
| sydtest | 18 |
| validity | 1 |
I read every distinct flagged line in all seven, except for the generated clients described below, which were sampled. All five carriers fire on real code: `Either` and `ExceptT` everywhere, `Validation (NonEmpty Text) Text` through kronor's validation modules, and `Except String` and `MonadError Text` in graphql-engine. None of the five is carried by the resources alone.
The dominant shape is the rule's exact target:
```haskell
parseTag :: Text -> Either String Tag
parseGlobText :: Text -> Either String Glob
parsePaymentStatus :: Text -> Either Text PaymentStatusEnum
decodePGScalarType :: Text -> Either String PGScalarType
readSmosFile :: Path Abs File -> IO (Maybe (Either String SmosFile))
```
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 of them `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. Excluding them, kronor has 175 findings over 166 distinct signatures, every one hand-written.
## What it does not see
Across nix-ci8, smos and salsa there are 95 `Left "literal"` sites, and 14 files hold one and get no finding at all. The cause is the same every time: the `Either String` is never written down, because a library combinator supplies it and the lambda handed to `bimapCodec`, `eitherReader`, `checkMapEither` or `fromPersistValue` only writes `Left "..."`. `good.hs` pins that silence as a decision rather than an accident. A rule of its own if it is worth one, and it would fire mostly where the shape is the library's rather than ours.
## 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.
`nix flake check` passes.