diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index d07caa7ab..a728d4f9e 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -1625,7 +1625,13 @@ function isZeroOffset(value) { // never see it — pseudo-elements aren't part of the DOM the cascade walks — // so this scans stylesheet text directly, mirroring the border rule's // gates: >= 3px thick, chromatic fill, full height against a side edge. -function scanCssTextForPseudoStripe(content) { +function scanCssTextForPseudoStripe(rawContent) { + // Blank comment bodies byte-for-byte so commented-out rules are not + // scanned as live CSS and every rule keeps its source offset (each + // finding carries `index` so line-based callers can attribute it and + // line-scoped inline ignores can match). + const content = String(rawContent || '').replace(/\/\*[\s\S]*?\*\//g, + (block) => block.replace(/[^\n]/g, ' ')); const customProps = collectCssCustomProps(content); const findings = []; const seen = new Set(); @@ -1734,9 +1740,13 @@ function scanCssTextForPseudoStripe(content) { if (seen.has(selector)) continue; seen.add(selector); + // The selector group absorbs whitespace trailing the previous rule; + // advance past it so `index` points at the selector itself. + const selectorStart = m.index + (m[1].length - m[1].trimStart().length); findings.push({ id: 'side-tab', snippet: `${selector} — absolute ${thicknessPx}px pseudo-element stripe (${edge}: 0)`, + index: selectorStart, }); } return findings; diff --git a/cli/engine/engines/regex/detect-text.mjs b/cli/engine/engines/regex/detect-text.mjs index 882f76b42..c664f70b6 100644 --- a/cli/engine/engines/regex/detect-text.mjs +++ b/cli/engine/engines/regex/detect-text.mjs @@ -2,7 +2,7 @@ import { GENERIC_FONTS, OVERUSED_FONTS, EM_DASH_FLOOR, EM_DASH_CHARS_PER_DASH } import { isNeutralColor } from '../../shared/color.mjs'; import { extractGoogleFontFamilies } from '../../shared/fonts.mjs'; import { checkSourceDesignSystem } from '../../design-system.mjs'; -import { scanCssTextForGlow, scanCssTextForGridBackground, scanCssTextForMarquee, scanCssTextForRadialHalo } from '../../rules/checks.mjs'; +import { scanCssTextForGlow, scanCssTextForGridBackground, scanCssTextForMarquee, scanCssTextForPseudoStripe, scanCssTextForRadialHalo } from '../../rules/checks.mjs'; import { isFullPage } from '../../shared/page.mjs'; import { applyInlineIgnores } from '../../shared/inline-ignores.mjs'; import { finding } from '../../findings.mjs'; @@ -653,7 +653,21 @@ function detectText(content, filePath, options = {}) { profile, phase: 'source', })); - if (cssLike.has(ext)) findings.push(...scanInsetStripeCss(content, filePath)); + // Pseudo-element stripes (::before/::after absolute bars) carry the same + // side-tab silhouette without any border token, so the line matchers can't + // see them (issue #394). The shared scanner already runs on full HTML pages + // via checkHtmlPatterns; give standalone stylesheets, component style + // blocks, and CSS-in-JS templates the same coverage. Each hit carries the + // rule's source offset, so the finding gets a real line and line-scoped + // inline ignores keep working. + const pseudoStripeFindings = (text, lineOffset) => + scanCssTextForPseudoStripe(text).map(hit => + finding(hit.id, filePath, hit.snippet, lineOffset + text.slice(0, hit.index).split('\n').length)); + + if (cssLike.has(ext)) { + findings.push(...scanInsetStripeCss(content, filePath)); + findings.push(...pseudoStripeFindings(content, 0)); + } // Block-level CSS checks that need multiple declarations must run over the // complete source, not line-by-line. This covers standalone stylesheets, @@ -690,6 +704,7 @@ function detectText(content, filePath, options = {}) { // reported every selector one line low. runRegexMatchers keeps startLine - 1 // because it indexes its split lines from zero. findings.push(...scanInsetStripeCss(block.content, filePath, block.startLine - 2)); + findings.push(...pseudoStripeFindings(block.content, block.startLine - 2)); } // Extract and scan CSS-in-JS template literals @@ -708,6 +723,7 @@ function detectText(content, filePath, options = {}) { phase: 'css-in-js', })); findings.push(...scanInsetStripeCss(block.content, filePath, block.startLine - 1)); + findings.push(...pseudoStripeFindings(block.content, block.startLine - 1)); } if (options?.designSystem) { diff --git a/cli/engine/rules/checks.mjs b/cli/engine/rules/checks.mjs index 141a209a2..3e810076b 100644 --- a/cli/engine/rules/checks.mjs +++ b/cli/engine/rules/checks.mjs @@ -823,7 +823,13 @@ function isZeroOffset(value) { // never see it — pseudo-elements aren't part of the DOM the cascade walks — // so this scans stylesheet text directly, mirroring the border rule's // gates: >= 3px thick, chromatic fill, full height against a side edge. -function scanCssTextForPseudoStripe(content) { +function scanCssTextForPseudoStripe(rawContent) { + // Blank comment bodies byte-for-byte so commented-out rules are not + // scanned as live CSS and every rule keeps its source offset (each + // finding carries `index` so line-based callers can attribute it and + // line-scoped inline ignores can match). + const content = String(rawContent || '').replace(/\/\*[\s\S]*?\*\//g, + (block) => block.replace(/[^\n]/g, ' ')); const customProps = collectCssCustomProps(content); const findings = []; const seen = new Set(); @@ -932,9 +938,13 @@ function scanCssTextForPseudoStripe(content) { if (seen.has(selector)) continue; seen.add(selector); + // The selector group absorbs whitespace trailing the previous rule; + // advance past it so `index` points at the selector itself. + const selectorStart = m.index + (m[1].length - m[1].trimStart().length); findings.push({ id: 'side-tab', snippet: `${selector} — absolute ${thicknessPx}px pseudo-element stripe (${edge}: 0)`, + index: selectorStart, }); } return findings; diff --git a/tests/detect-antipatterns-fixtures.test.mjs b/tests/detect-antipatterns-fixtures.test.mjs index 149bea698..78abd7db8 100644 --- a/tests/detect-antipatterns-fixtures.test.mjs +++ b/tests/detect-antipatterns-fixtures.test.mjs @@ -86,6 +86,74 @@ describe('detectText - Astro structural CSS fixtures', () => { }); }); +describe('detectText — pseudo-element stripe fixtures (issue #394)', () => { + // The side-tab silhouette drawn as an absolutely-positioned ::before/::after + // bar instead of a border. The scanner already ran on full HTML pages via + // checkHtmlPatterns; these pin the standalone-stylesheet and component + // style-block paths, which used to pass this construction clean. + const SHOULD_FLAG = [ + 'Inset Shorthand Left Edge', + 'Longhand Left Edge', + 'Bottom Edge', + 'Full Height Right Edge', + ]; + const SHOULD_PASS = [ + 'Neutral Divider', + 'Wide Panel', + 'Static Underline', + 'Hairline Divider', + 'Hover Underline', + 'Floating Badge', + // Commented-out CSS is not a live rule. + 'Commented Out Stripe', + ]; + + // The 1-based line a case's selector sits on in a fixture file, so the + // reported finding line can be checked against the actual source. + const selectorLine = (source, caseName) => { + const idx = source.split('\n').findIndex(l => l.includes(`data-case="${caseName}"`)); + assert.notEqual(idx, -1, `fixture is missing case "${caseName}"`); + return idx + 1; + }; + + it('standalone .css files flag chromatic pseudo-element stripes only', () => { + const filePath = path.join(FIXTURES, 'pseudo-stripe.css'); + const source = fs.readFileSync(filePath, 'utf8'); + const findings = detectText(source, filePath).filter(r => r.antipattern === 'side-tab'); + const snippets = findings.map(r => r.snippet || '').join(' | '); + for (const heading of SHOULD_FLAG) { + assert.match(snippets, new RegExp(`data-case=${JSON.stringify(heading)}`), `expected "${heading}" to flag`); + } + for (const heading of SHOULD_PASS) { + assert.doesNotMatch(snippets, new RegExp(`data-case=${JSON.stringify(heading)}`), `"${heading}" should pass`); + } + // Every finding must carry the selector's real source line, so + // line-scoped inline ignores (impeccable-disable-line and + // impeccable-disable-next-line) can match it. + for (const f of findings) { + const caseName = (f.snippet.match(/data-case="([^"]+)"/) || [])[1]; + assert.equal( + f.line, selectorLine(source, caseName), + `finding for "${caseName}" reports line ${f.line}, selector sits on line ${selectorLine(source, caseName)}`, + ); + } + }); + + it('component style blocks flag pseudo-element stripes at their source line', () => { + const filePath = path.join(FIXTURES, 'pseudo-stripe.vue'); + const source = fs.readFileSync(filePath, 'utf8'); + const findings = detectText(source, filePath).filter(r => r.antipattern === 'side-tab'); + const snippets = findings.map(r => r.snippet || '').join(' | '); + assert.match(snippets, /data-case="Component Left Edge"/, 'expected the component stripe to flag'); + assert.doesNotMatch(snippets, /data-case="Component Neutral Divider"/, 'neutral divider should pass'); + const stripe = findings.find(r => /data-case="Component Left Edge"/.test(r.snippet || '')); + assert.equal( + stripe.line, selectorLine(source, 'Component Left Edge'), + 'style-block finding must map back to the whole-file line, not the block-local one', + ); + }); +}); + describe('detectHtml — static HTML/CSS fixtures', () => { it('should-flag: catches border anti-patterns', async () => { const f = await detectHtml(path.join(FIXTURES, 'should-flag.html')); diff --git a/tests/fixtures/antipatterns/pseudo-stripe.css b/tests/fixtures/antipatterns/pseudo-stripe.css new file mode 100644 index 000000000..91286b50d --- /dev/null +++ b/tests/fixtures/antipatterns/pseudo-stripe.css @@ -0,0 +1,120 @@ +/* + * Pseudo-element stripe fixture for the regex engine (issue #394). + * + * The side-tab silhouette built as an absolutely-positioned ::before/::after + * bar instead of a border. scanCssTextForPseudoStripe already caught these on + * full HTML pages; this fixture pins the standalone-stylesheet path. The + * data-case attribute in each selector lands in the finding snippet, so the + * test can attribute every flag/pass case individually. + */ + +:root { + --stripe-fixture-neutral: #e5e7eb; +} + +/* ── FLAG: the issue reproducer — inset shorthand pin + 4px chromatic bar ── */ +.card[data-case="Inset Shorthand Left Edge"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 4px; + background: #7c3aed; +} + +/* ── FLAG: longhand edge pins; unresolvable var() errs toward detection ── */ +.card[data-case="Longhand Left Edge"]::before { + content: ""; + position: absolute; + top: 0; + bottom: 0; + left: 0; + width: 4px; + background: var(--accent); +} + +/* ── FLAG: horizontal variant riding the bottom edge ── */ +.card[data-case="Bottom Edge"]::after { + content: ""; + position: absolute; + left: 0; + right: 0; + bottom: 0; + height: 4px; + background: #f43f5e; +} + +/* ── FLAG: height:100% full-height stripe on the right edge ── */ +.card[data-case="Full Height Right Edge"]::before { + content: ""; + position: absolute; + top: 0; + right: 0; + height: 100%; + width: 3px; + background: oklch(60% 0.2 300); +} + +/* ── PASS: neutral gray divider is not an accent stripe ── */ +.card[data-case="Neutral Divider"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 4px; + background: var(--stripe-fixture-neutral); +} + +/* ── PASS: 24px is a panel, not a stripe ── */ +.card[data-case="Wide Panel"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 24px; + background: #7c3aed; +} + +/* ── PASS: no position: absolute — not an overlay stripe ── */ +.card[data-case="Static Underline"]::before { + content: ""; + width: 4px; + background: #7c3aed; +} + +/* ── PASS: 1px hairline is a divider ── */ +.card[data-case="Hairline Divider"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 1px; + background: #7c3aed; +} + +/* ── PASS: hover-conditional underline is an affordance, not decoration ── */ +.link-row[data-case="Hover Underline"]:hover::after { + content: ""; + position: absolute; + left: 0; + right: 0; + bottom: 0; + height: 4px; + background: #7c3aed; +} + +/* ── PASS: pinned to one edge but not full-height — a badge, not a stripe ── */ +.card[data-case="Floating Badge"]::before { + content: ""; + position: absolute; + left: 0; + width: 4px; + height: 12px; + background: #7c3aed; +} + +/* ── PASS: commented-out CSS is not a live rule ── +.card[data-case="Commented Out Stripe"]::before { + content: ""; + position: absolute; + inset: 0 auto 0 0; + width: 4px; + background: #7c3aed; +} +*/ diff --git a/tests/fixtures/antipatterns/pseudo-stripe.vue b/tests/fixtures/antipatterns/pseudo-stripe.vue new file mode 100644 index 000000000..a3b53a4ff --- /dev/null +++ b/tests/fixtures/antipatterns/pseudo-stripe.vue @@ -0,0 +1,29 @@ + + + + +