Report <> and ++ used to concatenate strings or text
Ships `HsNoSemigroupOnText`, the sixth rule.
## What it reports
A concatenation with a string literal as one of its operands, `<>` or `++`.
Both operators, because `++` is otherwise the way around the rule and
concatenates the same things, and they go in one chain together since the fix is
the same list either way: `"at " ++ s <> "."` is one finding of three operands.
The rule was filed as tier `types` and the types tier is not built, so what
ships is the literal-operand approximation `plan/plan.md` already sanctioned for
M5. On `++` that approximation is exact, since the only `IsString` instance for
a list of characters is the one for `String`. M7 widens it to the
concatenations with no literal in them, which is where `T.pack x <> y` and
`show x <> y` live.
## Two choices worth reviewing
**Read off the parse tree, not the token stream.** Every other syntactic fact in
this tool comes from the tokens, and here that would be wrong: a token before an
operator is not an operand of it, so `text "a" <> b` has a string literal beside
the `<>` while concatenating whatever `text` returns. That shape is all over the
corpus (`chunk ("Building " <> x)`, `toHtml ("Status: " <> s)`,
`textValue ("#" <> slug)`), and a token scan calls all of it a violation.
**One finding per chain, not per operator.** Two findings inside one statement
cannot both be answered: the second suppression a reader wrote would be one that
suppresses nothing. So `"at " <> path <> ": " <> msg` reports once, and
parentheses are peeled before the operands are read so `("[" <> t) <> "]"` is
one chain as well.
## Corpus
436 findings over five repositories:
| | |
|---|---|
| sydtest | 159 of 239 total findings |
| smos | 151 of 243 |
| centjes | 67 of 136 |
| autodocodec | 51 of 98 |
| feedback | 8 of 12 |
A sample of fifty `<>` sites was a `String` or a `Text` every time. The one class
where the rule asks for something it should not is the lazy representations: a
`<>` on a `Data.Text.Lazy.Builder` or a lazy `ByteString` appends a chunk rather
than copying, so it is the operator that type is for. Five sites of the 436, and
`<>` only. `ratchet` for anyone adopting it.
## What it cost here
Fourteen sites in this repository were the rule's own subject, almost all of
them a piece of punctuation appended inside a `unwords` list, and they are now
written as `concat`. hlint's `Use ++` hint rewrites exactly that back into the
operator, so the two cannot both be satisfied and the hint is turned off in
`.hlint.yaml` with the standard named as the reason.
29 lines for the rule, but it is the first one to want a fact extraction did not
produce, so it touched five Haskell files rather than the budgeted one:
`ConcatChain` and the `ModuleContext` field, the extraction that fills them, the
generator, the spec over the generator, and the registry. It also brings `syb`,
because finding every concatenation in a module means walking the whole
expression tree and hand-writing that over `HsExpr` would have been most of the
rule.
Misses, all accepted: the section `("a" <>)` and prefix `(<>) "a" x` forms.