From 7622cc8440da71a9f3dc17c8bafa677a9f820be1 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sat, 25 Jul 2026 17:45:55 -0700 Subject: [PATCH 1/2] Derive static-cascade color extraction from the shared named-color table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The static-html engine never emitted side-tab for `border-left: 4px solid purple` (or any named color outside a hardcoded 9-name list) in .html files: extractStaticColor's regex dropped the color token from border shorthands, the side defaulted to neutral black, and checkBorders skipped it. The same declaration in a .css file was flagged by the regex engine, so the two engines disagreed while both exited cleanly (issue #359). Build the extraction alternation from the same CSS_NAMED_COLORS table parseAnyColor resolves against (longest-first, whole-token), so the set of names the extractor recognizes and the set the parser can resolve cannot drift apart again. STATIC_NAMED_COLORS shrinks to the one keyword parseAnyColor deliberately refuses (`transparent` as zero-alpha), since parseAnyColor already covers every real named color in the table. New two-column fixture (named-color-borders.html) covers the issue reproducers: purple shorthand + radius, rebeccapurple (substring-safe matching), crimson top stripe, bare 3px teal, var() resolving to a named color, and an inline style attribute — with neutral named colors (dimgray, gainsboro, black), thin, and uniform borders as pass cases. Prepared with AI assistance (Claude Code), directed by @pbakaus. Co-Authored-By: Claude Code --- .../engines/static-html/css-cascade.mjs | 31 ++-- cli/engine/rules/checks.mjs | 1 + tests/detect-antipatterns-fixtures.test.mjs | 30 ++++ .../antipatterns/named-color-borders.html | 137 ++++++++++++++++++ 4 files changed, 189 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/antipatterns/named-color-borders.html diff --git a/cli/engine/engines/static-html/css-cascade.mjs b/cli/engine/engines/static-html/css-cascade.mjs index c0f96f60f..53d7be48c 100644 --- a/cli/engine/engines/static-html/css-cascade.mjs +++ b/cli/engine/engines/static-html/css-cascade.mjs @@ -2,7 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { profileStep, recordProfileEvent } from '../../profile/profiler.mjs'; -import { collectCssCustomProps, cssLengthToPx, parseAnyColor, resolveLengthPx, resolveVarRefs } from '../../rules/checks.mjs'; +import { CSS_NAMED_COLORS, collectCssCustomProps, cssLengthToPx, parseAnyColor, resolveLengthPx, resolveVarRefs } from '../../rules/checks.mjs'; // --------------------------------------------------------------------------- // jsdom CSS-variable border override map @@ -344,18 +344,29 @@ const STATIC_PROP_MAP = { 'overflow-y': 'overflowY', }; +// parseStaticColor tries parseAnyColor first, which already resolves every +// name in the shared CSS_NAMED_COLORS table. This fallback only carries the +// keywords parseAnyColor deliberately returns null for: the cascade needs +// `transparent` to read as an actual zero-alpha color. const STATIC_NAMED_COLORS = { - black: { r: 0, g: 0, b: 0, a: 1 }, - white: { r: 255, g: 255, b: 255, a: 1 }, transparent: { r: 0, g: 0, b: 0, a: 0 }, - gray: { r: 128, g: 128, b: 128, a: 1 }, - grey: { r: 128, g: 128, b: 128, a: 1 }, - silver: { r: 192, g: 192, b: 192, a: 1 }, - red: { r: 255, g: 0, b: 0, a: 1 }, - green: { r: 0, g: 128, b: 0, a: 1 }, - blue: { r: 0, g: 0, b: 255, a: 1 }, }; +// Named-color alternation for plucking a color token out of shorthand values +// (issue #359: a hardcoded 9-name list here silently dropped `purple`, +// `crimson`, `teal`, ... from border shorthands, so the side defaulted to +// neutral black and side-tab never fired on .html files). Derived from the +// same table parseAnyColor resolves against, so extraction and parsing can't +// drift apart. Longest-first so names containing other names as substrings +// (rebeccapurple) are matched whole. +const NAMED_COLOR_TOKENS = [...Object.keys(CSS_NAMED_COLORS), ...Object.keys(STATIC_NAMED_COLORS)] + .sort((a, b) => b.length - a.length) + .join('|'); +const STATIC_COLOR_TOKEN_RE = new RegExp( + `(?:rgba?\\([^)]+\\)|oklch\\([^)]+\\)|oklab\\([^)]+\\)|lch\\([^)]+\\)|lab\\([^)]+\\)|hsla?\\([^)]+\\)|hwb\\([^)]+\\)|#[0-9a-f]{3,8}\\b|\\b(?:${NAMED_COLOR_TOKENS})\\b)`, + 'i' +); + function splitCssList(value) { const parts = []; let depth = 0, quote = '', start = 0; @@ -441,7 +452,7 @@ function extractStaticColor(value) { } return ''; } - const colorLike = raw.match(/(?:rgba?\([^)]+\)|oklch\([^)]+\)|oklab\([^)]+\)|lch\([^)]+\)|lab\([^)]+\)|hsla?\([^)]+\)|hwb\([^)]+\)|#[0-9a-f]{3,8}\b|\b(?:black|white|gray|grey|silver|red|green|blue|transparent)\b)/i); + const colorLike = raw.match(STATIC_COLOR_TOKEN_RE); if (!colorLike) return ''; return colorLike[0]; } diff --git a/cli/engine/rules/checks.mjs b/cli/engine/rules/checks.mjs index 418aa8f45..141a209a2 100644 --- a/cli/engine/rules/checks.mjs +++ b/cli/engine/rules/checks.mjs @@ -5369,6 +5369,7 @@ function checkFirstViewportColumnOverflowDOM() { } export { + CSS_NAMED_COLORS, checkBorders, isEmojiOnlyText, checkColors, diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs index e2ee09ac4..bec634f32 100644 --- a/tests/detect-antipatterns-fixtures.test.mjs +++ b/tests/detect-antipatterns-fixtures.test.mjs @@ -366,6 +366,36 @@ describe('detectHtml — static HTML/CSS fixtures', () => { ); }); + it('named-color-borders: named-color side-tabs are flagged, neutral names pass', async () => { + // Regression for issue #359: the static cascade's shorthand color + // extraction recognized only 9 named colors, so `border-left: 4px solid + // purple` (or any of the other named colors parseAnyColor understands) + // lost its color during expansion, defaulted to neutral black, and never + // fired side-tab — while the same declaration in a .css file was flagged + // by the regex engine. The extraction list is now derived from the same + // CSS_NAMED_COLORS table the parser uses, so the two can't drift apart. + const f = await detectHtml(path.join(FIXTURES, 'named-color-borders.html')); + const sideTabs = f.filter(r => r.antipattern === 'side-tab'); + // Six FLAG cases: purple + radius (the issue reproducer), rebeccapurple + // (contains "purple" as a substring — whole-token matching), crimson + // top stripe, bare 3px teal, a var() resolving to a named color, and an + // inline style attribute. Each must produce exactly one side-tab. + assert.equal( + sideTabs.length, 6, + `expected 6 side-tab findings from the FLAG column, got ${sideTabs.length}: ${sideTabs.map(r => r.snippet).join('; ')}` + ); + const topFindings = sideTabs.filter(r => /border-top/.test(r.snippet || '')); + assert.equal(topFindings.length, 1, `expected 1 border-top finding, got ${topFindings.length}`); + // PASS column (neutral named colors, thin, and uniform borders) must + // contribute nothing — dimgray/gainsboro/black have to parse AND read + // as neutral rather than being dropped as unknown colors. + const borderAccent = f.filter(r => r.antipattern === 'border-accent-on-rounded'); + assert.equal( + borderAccent.length, 0, + `expected 0 border-accent-on-rounded, got ${borderAccent.length}: ${borderAccent.map(r => r.snippet).join('; ')}` + ); + }); + it('modern-color-borders: regex fallback skips neutral 1px oklch dividers', () => { const css = ` .flag-side-tab { diff --git a/tests/fixtures/antipatterns/named-color-borders.html b/tests/fixtures/antipatterns/named-color-borders.html new file mode 100644 index 000000000..10fa21afb --- /dev/null +++ b/tests/fixtures/antipatterns/named-color-borders.html @@ -0,0 +1,137 @@ + + + + + Side-Tab with CSS Named Colors (purple/rebeccapurple/crimson/teal) + + + +
+
+

Should flag

+

named purple

border-left 4px solid purple + radius

+

rebeccapurple

border-left 4px + radius

+

crimson top stripe

border-top 4px, horizontal variant

+

named teal

border-left 3px, no radius

+

var() to named

border-left 4px solid var(--accent)

+
+

inline named purple

style attribute, issue #359 case (a)

+
+
+
+

Should pass

+

dimgray

neutral named color

+

gainsboro

light neutral named color

+

named black

neutral side border

+

1px purple

too thin to qualify

+

uniform purple

all four sides, not a stripe

+
+
+ + + From 1907335ce52aeea7d38b3f86335986132a711890 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sat, 25 Jul 2026 18:01:14 -0700 Subject: [PATCH 2/2] Give each named-color flag case a unique snippet signature Review bots (Greptile, Copilot) correctly noted the aggregate count assertion could pass if one FLAG case stopped emitting while a PASS case started. Each flag case now carries a distinct width/radius combination and the test deep-equals the sorted snippet list, so every finding attributes to exactly one case. Prepared with AI assistance (Claude Code), directed by @pbakaus. Co-Authored-By: Claude Code --- tests/detect-antipatterns-fixtures.test.mjs | 34 +++++++++++-------- .../antipatterns/named-color-borders.html | 14 ++++---- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs index bec634f32..149bea698 100644 --- a/tests/detect-antipatterns-fixtures.test.mjs +++ b/tests/detect-antipatterns-fixtures.test.mjs @@ -375,20 +375,26 @@ describe('detectHtml — static HTML/CSS fixtures', () => { // by the regex engine. The extraction list is now derived from the same // CSS_NAMED_COLORS table the parser uses, so the two can't drift apart. const f = await detectHtml(path.join(FIXTURES, 'named-color-borders.html')); - const sideTabs = f.filter(r => r.antipattern === 'side-tab'); - // Six FLAG cases: purple + radius (the issue reproducer), rebeccapurple - // (contains "purple" as a substring — whole-token matching), crimson - // top stripe, bare 3px teal, a var() resolving to a named color, and an - // inline style attribute. Each must produce exactly one side-tab. - assert.equal( - sideTabs.length, 6, - `expected 6 side-tab findings from the FLAG column, got ${sideTabs.length}: ${sideTabs.map(r => r.snippet).join('; ')}` - ); - const topFindings = sideTabs.filter(r => /border-top/.test(r.snippet || '')); - assert.equal(topFindings.length, 1, `expected 1 border-top finding, got ${topFindings.length}`); - // PASS column (neutral named colors, thin, and uniform borders) must - // contribute nothing — dimgray/gainsboro/black have to parse AND read - // as neutral rather than being dropped as unknown colors. + const sideTabs = f.filter(r => r.antipattern === 'side-tab').map(r => r.snippet).sort(); + // Six FLAG cases, each with a unique width/radius signature so every + // finding attributes to exactly one case (an offsetting miss + false + // positive can't cancel out in an aggregate count): + // purple 4px + radius 8 (the issue reproducer), rebeccapurple 5px + + // radius 4 (contains "purple" as a substring — whole-token matching), + // crimson 4px top stripe, bare 3px teal, var() resolving to a named + // color at 6px + radius 4, and a 7px inline style attribute. + // The PASS column (neutral named colors at 3-4px, 1px thin, uniform) + // must contribute nothing — dimgray/gainsboro/black have to parse AND + // read as neutral rather than being dropped as unknown colors, and none + // of its shapes can produce any of the signatures below. + assert.deepEqual(sideTabs, [ + 'border-left: 3px', + 'border-left: 4px + border-radius: 8px', + 'border-left: 5px + border-radius: 4px', + 'border-left: 6px + border-radius: 4px', + 'border-left: 7px', + 'border-top: 4px', + ]); const borderAccent = f.filter(r => r.antipattern === 'border-accent-on-rounded'); assert.equal( borderAccent.length, 0, diff --git a/tests/fixtures/antipatterns/named-color-borders.html b/tests/fixtures/antipatterns/named-color-borders.html index 10fa21afb..f3e0e2db7 100644 --- a/tests/fixtures/antipatterns/named-color-borders.html +++ b/tests/fixtures/antipatterns/named-color-borders.html @@ -37,12 +37,13 @@ } /* 2: rebeccapurple — a longer name that contains another color name - ("purple") as a substring; must match whole-token */ + ("purple") as a substring; must match whole-token. Width 5px so the + finding snippet is unique to this case. */ #flag-named-rebecca { width: 400px; background: #ffffff; border-radius: 4px; - border-left: 4px solid rebeccapurple; + border-left: 5px solid rebeccapurple; } /* 3: horizontal stripe variant — named crimson riding the top edge */ @@ -59,12 +60,13 @@ border-left: 3px solid teal; } - /* 5: named color behind a var() in the shorthand */ + /* 5: named color behind a var() in the shorthand. Width 6px so the + finding snippet is unique to this case. */ #flag-named-var { width: 400px; background: #ffffff; border-radius: 4px; - border-left: 4px solid var(--accent); + border-left: 6px solid var(--accent); } /* ── PASS cases: neutral named colors and non-side-tab shapes ── */ @@ -119,8 +121,8 @@

crimson top stripe

border-top 4px, horizontal variant

named teal

border-left 3px, no radius

var() to named

border-left 4px solid var(--accent)

-
-

inline named purple

style attribute, issue #359 case (a)

+
+

inline named purple

style attribute, issue #359 case (a); width 7px keeps the snippet unique