Skip to content

Adopt §15 lint, and delete a dead buttons option six call sites were passing - #2

Merged
MichalAFerber merged 1 commit into
mainfrom
feat/adopt-15-lint
Sep 3, 2026
Merged

MichalAFerber merged 1 commit into
mainfrom
feat/adopt-15-lint

Conversation

@MichalAFerber

Copy link
Copy Markdown
Owner

First source-only package to adopt, using the library CI template from tgwab-standards #107. This repo had no CI at all, because the only workflow template available gated a build it does not have.

The unused-variable rule found the thing it was added for

createEditor() in features/_editor.js destructured buttons and never used it — while six callers passed buttons: ACTIONS into it: text-tools, analyzers, translators, case-converter, code-data, and generators. toolbarLeft and toolbarRight beside it are rendered; buttons was dropped on the floor.

The actions still appear on screen, because the host reads the tool descriptor's own actions: ACTIONS key. So six call sites passed an argument that did nothing and looked exactly like the thing that made them work.

That is the herald defect — a parameter standing in as evidence for wiring that is not there — and it is the first time this rule has caught its own class in the estate rather than style noise.

Removed in full: the ignored binding and all six call sites. Leaving callers passing a key nothing reads would have preserved the confusion the rule just removed.

Worth a second opinion: if buttons was meant to render into the toolbar and never got wired, this change surfaces that rather than hiding it — the revert is one line plus a real implementation. On the evidence I could see it is a leftover from an earlier design, since the actions already render via the descriptor.

The other ten findings

Count Rule Treatment
3 no-useless-escape \[ inside a character class, and a trailing \-. Verified behaviour-identical before changing — each old and new regex run over 14 samples, byte-identical output.
4 no-empty localStorage try/catch. Annotated, not suppressed — each now says why the failure is ignorable.
1 no-control-regex \x1B in the ANSI stripper. Intentional: ESC is what starts an ANSI sequence. Inline disable with a reason, placed adjacent to the line — my first attempt put it two lines up behind a comment, where ESLint correctly reported it as an unused directive.
2 no-restricted-syntax Declared exemption, not a code change. See below.

The exemption, and why it is narrow

Both no-restricted-syntax hits are in features/code-data.js, which builds a regex-match report: lines.push('@12 "matched"') into an array that ends in lines.join('\n') and renders as plain text. Nothing is parsed as HTML, so the </script> escape the rule exists to stop cannot happen.

The exemption names the file and the reason. "It returns a string" would not be a valid reason; "joined with newlines and shown as text" is, and that is what I actually read before writing it.

Proven narrow: a hazard planted elsewhere fails lint (rc 1), and the same hazard inside the exempted file does not — so this is a scoped exemption, not a repo-wide off switch.

Also created, because this package never had a dependency

  • package-lock.json — npm ci fails outright without one.
  • .nvmrc — node-version-file fails without it.
  • .gitignore — the repo had none, so node_modules/ would have been committed on the first install.

Verification

  • npm ci from a clean tree, rc 0 — the newly created lockfile works
  • npm run lint rc 0
  • 2/2 tests; the fixture control asserts — five hazards flagged, both negative controls untouched
  • planted hazard fails lint (rc 1); removing it returns rc 0

Opened as a draft: devops flips on the green.

🤖 Generated with Claude Code

https://claude.ai/code/session_0126UXfnHngrTg7kqmw87ipV

…e passing

First of the source-only packages to adopt, using the library CI template from
tgwab-standards #107. This repo had no CI at all, because the only workflow
template available gated a build it does not have.

THE UNUSED-VARIABLE RULE FOUND THE THING IT WAS ADDED FOR. `createEditor()` in
features/_editor.js destructured `buttons` and never used it, while SIX callers
passed `buttons: ACTIONS` into it -- text-tools, analyzers, translators,
case-converter, code-data and generators. `toolbarLeft` and `toolbarRight` next
to it ARE rendered; `buttons` was dropped on the floor.

The actions still appear, because the host reads the tool descriptor's own
`actions: ACTIONS` key. So six call sites passed an argument that did nothing and
looked exactly like the thing that made them work. That is the herald defect --
a parameter standing in as evidence for wiring that is not there -- and it is the
first time this rule has caught its own class in the estate rather than style
noise.

Removed in full: the ignored binding and all six call sites. Leaving the callers
passing a key nothing reads would have preserved the confusion the rule just
removed.

WORTH A SECOND OPINION: if `buttons` was MEANT to render into the toolbar and
never got wired, this change surfaces that rather than hiding it -- the revert is
one line plus a real implementation. On the evidence I could see it is a leftover
from an earlier design, since the actions already render via the descriptor.

THE OTHER TEN FINDINGS

  3 no-useless-escape   `\[` inside a character class and a trailing `\-`.
                        Verified behaviour-identical before changing: each old
                        and new regex run over 14 samples, byte-identical output.
  4 no-empty            localStorage try/catch. Annotated, not suppressed --
                        each now says why the failure is ignorable.
  1 no-control-regex    `\x1B` in the ANSI stripper. Intentional: ESC IS what
                        starts an ANSI sequence, so matching it is the point.
                        An inline disable with a reason, placed adjacent to the
                        line -- the first attempt put it two lines up behind a
                        comment, where ESLint correctly reported it as an unused
                        directive.
  2 no-restricted-syntax  DECLARED EXEMPTION, not a code change. Both are in
                        features/code-data.js, which builds a regex-match REPORT:
                        `lines.push('@12  "matched"')` into an array that ends in
                        `lines.join('\n')` and renders as plain text. Nothing is
                        parsed as HTML, so the `</script>` escape cannot happen.
                        The exemption names the file and the reason. "It returns
                        a string" would not be a valid reason; "joined with
                        newlines and shown as text" is, and that is what I read.

ALSO CREATED, because this package never had a dependency: package-lock.json
(npm ci fails outright without one), .nvmrc (node-version-file fails without it),
and a .gitignore -- the repo had none, so node_modules would have been committed.

VERIFIED: npm ci from a clean tree rc 0 (the new lockfile works), lint rc 0,
2/2 tests. The fixture control asserts -- five hazards flagged, both negative
controls untouched. A planted hazard fails lint (rc 1) and the same hazard inside
the exempted file does NOT, which is what makes the exemption narrow rather than
a repo-wide off switch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126UXfnHngrTg7kqmw87ipV
@MichalAFerber

Copy link
Copy Markdown
Owner Author

DevOps gate read — ready for review.

The dead buttons option is the first defect this rule has caught of its own class rather than style noise, and it is worth naming as such. createEditor() destructured buttons and never used it, while six call sites passed buttons: ACTIONS — the actions had been rendering all along through the descriptor's own actions key.

That is the herald FLEET_SQL shape exactly: a parameter that looks wired, is not, and whose deadness is the only evidence anyone had that it was connected. Six callers passing it is what makes it convincing — the more call sites, the more obviously plumbed it appears, and the less likely anyone is to check. A lint rule for unused bindings is a poor tool for finding architectural dead ends and it found one anyway, because the dead end terminated in an unused variable.

Verified the disables: the fixture's expected no-unused-vars, no-undef header, plus one no-control-regex with a stated reason — "intentional: ANSI starts with ESC". A narrow, single-line, justified disable is the correct form; it is a declaration rather than a silence.

This repo is one of the two with no lockfile, so this adoption creates one as part of the change — npm ci would fail outright otherwise. That is the case tgwab-standards#107's template comment now warns about, and this PR is the first instance of it.

Flipping ready.

@MichalAFerber
MichalAFerber marked this pull request as ready for review September 3, 2026 09:13
@MichalAFerber
MichalAFerber merged commit c51f757 into main Sep 3, 2026
1 check passed
@MichalAFerber
MichalAFerber deleted the feat/adopt-15-lint branch September 3, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant