By: Tom Sydney Kerckhove <syd@cs-syd.eu>
Ask for an error type rather than a String HsNoStringError reports a failure written as prose, in the two places prose gets written. The first is a type that carries one: the first argument of Either, Except, ExceptT, MonadError or Validation, when that argument is String, Text, [Char], or a list or NonEmpty of those. The second is a declaration that names itself an error and is prose underneath, because naming the prose is the obvious way to answer the first half without changing anything. The finding underlines the prose rather than the signature or the declaration around it, since that is the part to change. A caller can branch on a constructor and cannot branch on prose without taking the message apart again, so a String error asks every handler to treat every failure the same way. Writing the words at the point of failure also fixes them there, which is the place that knows least about who will read them. Read off the type syntax rather than off the names in it, because the question is which argument of which type a name is written as: the String in Either String a is a failure and the one in Either a String is a result. A type node arrives more than once, since a strict field is a bang around parentheses around the application, so applications are peeled and deduplicated by location, and the function half of an application is dropped: the Either String inside Either String Int is the same String. peelType and peelApp move to Hopinion.Extract.Ghc, which is where the reading no rule owns lives. Two of the readings are narrower than they look, and both narrowings are what make the rule worth having rather than worth turning off. base, mtl and transformers fix the argument order of four of the 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 container of prose is read as prose in that argument, which is a shape a success-first type would not have, and the recall it costs is recall over code nobody writes, since a Validation that does not accumulate is an Either with more syllables. A wrapper counts only when its constructor is the type's name over again, with or without Mk. That is the shape a reader learns nothing from, and its renderer is id: renderNarInfoParseError (NarInfoParseError s) = s. A constructor that names the failure holds what was not understood rather than words about it, and the words come from the renderer that matches on the name. Reporting every one-constructor wrapper found seventeen across the repositories below, and eleven of them were that second shape, written deliberately. Reporting only the echo finds three. Tried out over nix-ci8, smos, salsa, kronor, sydtest, validity and graphql-engine: 1534 findings, no false positives, every distinct flagged line read apart from the generated clients, and every one of the seventeen candidate wrappers opened to see what its renderer does. All five carriers fire on real code, so none of them is carried by the resources alone. Two things it does not see, both held in good.hs so that they stay decisions. An Either String that is never written down, where a combinator supplies it and the lambda handed to bimapCodec, eitherReader or fromPersistValue only writes Left "...". And a wrapper named better than it is, where the constructor names the failure and the renderer hands its payload straight back: what decides that is a module away from the declaration and would be a larger rule than this one. The one place here is fromPersistValueViaCodec, which is persistent's fromPersistValue under another name. Its reason moves from the module header into the suppression, so it is said once.
Total
8s
Created Started Finished
| Time to Start | Worker time | Duration | Time to finish | Idle | |
|---|---|---|---|---|---|
| Config | 0s | 0s | 0s | 0s | 0s |
| Eval | 0s | 7s | 7s | 8s | 0s |
| Build | 7s | 0s | 0s | 8s | 0s |
| Suite | 0s | 8s | 8s | 8s | 0s |