2d12b84b

A comment that couples two places carries a tag or a ref

## What this adds

`CommentCouplingConnection`, a module-level rule: a comment that says two
places move together has to carry a tag or a ref, so the other place is named
to a tool rather than only to whoever is reading that paragraph today.

    -- Keep the encoder and the decoder in sync.      -- reported
    -- [tag:WireFormat] Keep the encoder and the decoder in sync.   -- fine

## What "connection" turned out to mean

You asked for prose, and for no false positives, so the check fires only where
an intended connection is certain. Two lists, in
`hopinion/src/Hopinion/Check/Comment/CouplingConnection/Rule.hs`:

- **Phrases that are a coupling on their own**: `change together`,
  `changed together`, `changes together`, and the three `update` spellings.
- **Phrases that only say two things are alike**: `in sync`, `in lockstep`,
  `in tandem`. These count only where the *same sentence* also asks for them to
  be held that way, with one of `keep/keeps/kept/keeping`,
  `stay/stays/stayed/staying`, `remain/remains`.

That second condition is the one that earns the no-false-positives bar. "Waits
until the replica is in sync" is a state somebody waits for; "keep the two in
sync" is an instruction to whoever edits this next. Without it the rule fires on
the first, which is a finding nobody can act on. `good.hs` pins both readings.

Sentence-bounded rather than comment-bounded, so "Keep this simple. The cache is
in sync only by accident." stays quiet.

Phrases I tried and dropped, because each has a reading that is not a coupling:

- `vice versa`: describes a bijection inside one function as often as a
  relationship between two places.
- `counterpart`: "has no counterpart" says there is no coupling.
- `hand in hand`, `must match`, `the same way`: comparisons, which is how most
  prose describes anything.
- `remaining`, `remained` as keeping words: "the remaining rows are in sync" is
  an adjective, not an instruction.

## What satisfies it

Only the paired forms, as you chose: `[tag:X]`, `[ref:X]`, `[check:tag X]`,
`[check:ref X]`, each of which has to name something. A bare `[check]`,
`[check:file]`, `[check:all <glob>]` or an empty `[tag:]` does not, and
`bad-a-bare-check-is-not-a-connection.hs` pins that.

The connection may sit in any comment in the same **scope**, not only in the
same comment block. This shape has to keep working, and the two lines are two
blocks because a change of comment style ends one:

    -- | [check:ref WireFormat]
    --
    -- Keep this in sync with the encoder.

A scope is a declaration, which is also the granularity a suppression is matched
at, so it is the granularity a reader can predict. The cost is that at file
scope one tag anywhere silences every file-level coupling comment. That errs
towards silence, which is the direction you asked for.

## Things to look at

- **The two phrase lists.** They are the whole of the rule's precision, and
  they are deliberately the floor rather than the ceiling. Each entry is one I
  can defend; the question is whether any of them still has a reading I have
  not thought of, and whether something obvious is missing.
- **The sentence split** is on `. ! ? ; :`. An abbreviation splits one sentence
  into two, which costs a finding and never invents one. Colons are common in
  this repository's comments, and splitting there is deliberate.
- **`alikePhrases`' own Haddock fired the rule on itself** while I was writing
  it, because it quoted "kept in sync" to explain the discriminator. I reworded
  it rather than suppressing. Documentation *about* couplings is a real
  false-positive class, and the suppression is the answer where rewording is
  not.
- **A multi-line Haddock is several comment blocks**, since `Hopinion.Comment`
  ends a block at a change of style. So a phrase spanning the first and second
  line of a `-- |` block is missed. That is pre-existing and affects
  `CommentBareTodo` too; I did not touch it. It is a miss, never a wrong
  finding.
- **Suppressions are not excluded.** A suppression whose reason says "kept in
  sync by hand" lands in the scope that suppression covers, so it suppresses
  itself. I tried that out both ways round and it is quiet.

## Things to try out locally

    cabal run hopinion -- explain CommentCouplingConnection
    cabal run hopinion -- module <a file with a coupling comment>

Worth writing a few comments of your own in the shapes you actually use and
seeing whether it fires where you want it to. The recall side is the part I am
least sure of.

## Not done, and why

- `plan/rules.md` and `plan/plan.md`: I updated the two lists of what ships
  (they were already one behind, missing `HsRecordFieldPerLine`) and noted on
  `TagrefConnectCoupledCode` which half now ships. I left the measurement
  paragraph in `plan.md` alone: it says "thirteen rules" and quotes line counts
  I could not reproduce from the tree, so changing the number without the
  method would make it less true rather than more.
- `hopinion check` in-process reports one `UNUSED_SUPPRESSION` for
  `[allow:TestJsonSpecPerJsonType]` in `Hopinion/Check/Obligation.hs`. It does
  the same on `master`, and `nix flake check` is green, so it is an artefact of
  running without `.hie` files rather than anything this branch did.
- This branch is off `master`, since there is no `development` branch here.

Total

1h15m46s

Created 1 month ago Started 1 month ago Finished 1 month ago

Timeline