Uh oh!
There was an error while loading. Please reload this page.
feat(lint): reject an unpaired getBadgeColorClasses call - #5292
Merged
Conversation
`getBadgeColorClasses(color, value)` returns a class string and therefore cannot carry an author-declared hex — it quantizes the declared colour onto one of nine palette families. The correct answer is `getBadgeHexAppearance(color)`, whose `className` reads CSS custom properties that only its `style` half supplies. A class-only call compiles, renders and looks right for family-name declarations, so it fails only for authors who declared a hex, and it fails by rendering a plausible neighbouring colour rather than by breaking. #5141 fixed that in the cell renderer and #5183 fixed four more sites; both rounds were per-site, because nothing rejected the class-only call at write time. Adds `object-ui/no-unpaired-badge-color-classes`: a `getBadgeColorClasses` call must have a `getBadgeHexAppearance` consult lexically visible from it — the same function scope or an enclosing one. Scope-level rather than syntax-level, so the `if/else` spelling pairs as well as the ternary and a hoisted loop-invariant consult above a `.map()` stays clean; a consult in a sibling function does NOT pair, so one correct site cannot license every unpaired site added beside it. Severity `error`: `.github/workflows/lint.yml` deliberately sets no `--max-warnings`, so a `warn` here could not fail CI. Every live call site already pairs the two helpers (plugin-grid ObjectGrid group header + compact card, plugin-kanban card badges), so the rule lints clean today with no allowlist — measured repo-wide, 3357 files, 0 findings. `packages/fields` is out of scope: it owns both helpers and unit-tests each half in isolation (forcing the rule on there reports its 7 deliberate single-helper calls).
This was referenced Aug 19, 2026
os-support-ai
marked this pull request as ready for review
August 19, 2026 07:36
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5191
Scoped to the lint-rule option only, per the triage comment on the issue. The consolidated-export option is an explicit non-goal and is not touched here:
packages/fields' published surface is unchanged, andObjectGrid.tsxis not re-entered.The class this closes
@object-ui/fieldsanswers "what colour is this badge?" in two incompatible currencies:getBadgeColorClasses(color, value)returns a class string — complete-looking, and structurally unable to carry a runtime colour, so an author-declared hex is quantized onto one of nine palette families.getBadgeHexAppearance(color)returns{ className, style }orundefined— the correct answer, but only if the caller consults it first and carries thestylethrough. That className reads CSS custom properties that only thestylehalf supplies.A class-only call compiles, renders, and looks right for family-name declarations (the common case), so it fails only for authors who declared a hex — and it fails by rendering a plausible neighbouring colour rather than by breaking. #5141 fixed that in the cell renderer; #5183 fixed four more sites. Both rounds were per-site, because nothing rejected the class-only call at write time.
What was verified before writing
Re-derived on
origin/main@12841b6(still the tip when this branch was cut, matching the triage comment):packages/fields/src/index.tsx:1409getBadgeHexAppearance,:1440getBadgeColorClasses— located by reading, not by the cited line numbers.plugin-grid/src/ObjectGrid.tsx:3174/3178(compact card) and:3394/3397(group header),plugin-kanban/src/ObjectKanban.tsx:413/416and:471/474. No live unpaired site exists today, so this rule is preventive.plugin-calendar/src/CalendarView.tsx:34is a comment mention only — confirmed: it is the sole occurrence of the symbol anywhere in that package, there is no import and no call.grep -rni badge eslint-rules/returns nothing; counter-probed withcontext.report, which hits all six existing rule files).The rule
object-ui/no-unpaired-badge-color-classes— agetBadgeColorClassescall must have agetBadgeHexAppearanceconsult lexically visible from it: the same function scope, or one enclosing it (module scope included).Scope-level rather than syntax-level, which decides three cases the rule's test pins:
if/elsespelling pairs exactly as well as the ternary — the rule reads scopes, not conditional shapes, so it cannot be defeated by rewriting the branch;.map()and calling the class helper inside the callback stays clean, because an enclosing scope is what "visible" means (a rule that flagged that correct code is a rule someone would delete);ObjectGrid.tsxandObjectKanban.tsx, are exactly where a fifth badge surface gets written.Import aliases are followed on both halves, and the namespace form (
fields.getBadgeColorClasses(...)) is matched on the member name. Known limit, documented in the rule header: passing either helper around as a bare function reference is not tracked — no call site in this repo does that, and a reference-flow pass would trade a large false-positive surface for a case nobody writes.Severity:
error, and whyerror, matching every otherobject-ui/*ratchet ineslint.config.js. Awarnwould be decoration here:.github/workflows/lint.ymldeliberately sets no--max-warnings(its own header says so, and the repo carries ten thousand warnings), so a warning cannot fail CI. The lint workflow gates pull requests and every one of the 46 workspace packages runseslint .(scripts/check-lint-coverage.mjs: 46/46), so an error here does turn a build red.The rule lints clean today with no allowlist, so landing it at
errorcosts nothing.Both halves of the real eslint run
Not just the unit test — an actual
eslintinvocation, at commit7d1256b32.Half 1 — it fires. A deliberately-unpaired fixture (temporary, never committed) at
packages/plugin-grid/src/tmp-fixture-5191.tsx:Rewriting that same fixture into the paired form makes it silent:
exit=0, no findings.Half 2 — it is quiet on today's correct code, which matters as much: a rule that flags correct code gets deleted.
Repo-wide, at
7d1256b32. All four live paired call sites are inside that set.Reverse verification
Predicted before running: with the fixture present and the config block removed, the fixture stops being flagged — a straight red-to-green flip on the fixture, exit 0, not a count shift elsewhere, since nothing else in the repo emits this diagnostic and no other gate reads its output. No rebuild concern applies:
eslint.config.jsimports./eslint-rules/index.jsas source, andeslint-rules/has no build output, so there is no artifact between the edit and the thing under test.git checkoutLeg B restored the file byte-identically to the commit (
git diff --stat HEAD -- eslint.config.jsempty). The fixture was deleted afterwards; the tree is clean.Why
packages/fieldsis out of scopeThe config block ignores
packages/fields/**, because that package owns both helpers: it defines them, its own badge renderer pairs them anyway (index.tsx:1559/1560), andsrc/__tests__/badge-hex-fidelity-5141.test.tsxdeliberately exercises each half in isolation. That carve-out is load-bearing and was measured, not assumed — forcing the rule on for that file with--rulereports its 7 deliberate single-helper calls.Everywhere else, including tests, is in scope: outside
fields, a call to this helper is a badge surface.Verification
At commit
7d1256b32, clean tree:pnpm exec vitest run eslint-rules/no-unpaired-badge-color-classes.test.js— 18 passed (10 valid shapes, 8 invalid). The valid list is transcribed from the live call sites; the invalid list is the historical defect shape plus the ways a fifth site could look paired without being paired.pnpm exec vitest run eslint-rules/ scripts/— 62 files, 1394 tests passed. Includes the lint-config meta-tests (lint-workflow,turbo-lint-inputs,turbo-task-guard-coverage).pnpm exec eslint .— 3357 files, 0 errors.node scripts/check-control-bytes.mjs— OK, 4684 tracked text files.node scripts/check-lint-coverage.mjs— 46/46 packages linted, 0 with outstanding errors.node scripts/check-changeset-presence.mjs— "No source of a released package changed in this range, so no changeset is owed." CI-time only, matching the triage note; checked with the script rather than assumed.File surface
eslint-rules/no-unpaired-badge-color-classes.js, its test,eslint-rules/index.js,eslint.config.js. Nothing else — nopackages/fieldsexport change, noObjectGrid.tsx, no governed surfaces.Generated by Claude Code
Generated by Claude Code