diff --git a/README.md b/README.md index b957b7e5b..69cb68639 100644 --- a/README.md +++ b/README.md @@ -427,6 +427,8 @@ npx impeccable ignores add-value overused-font Inter --reason "Brand font" The detector catches 61 deterministic issues across AI slop (side-tab borders, purple gradients, bounce easing, dark glows) and general design quality (line length, cramped padding, small touch targets, skipped headings, and more). +Human-readable findings are diagnostics written to stderr, so redirect them with `2> findings.txt`. Use `--json` for machine-readable results on stdout. URL scans inspect the rendered DOM, computed layout, and accessible linked stylesheets; browser security still prevents reading cross-origin CSS without CORS. A clean detector run is evidence, not proof of visual or accessibility quality: it does not replace inspecting the rendered experience across relevant viewports. + By default, `detect` respects the same `.impeccable/config.json` and `.impeccable/config.local.json` detector config as the design hook: `detector.ignoreRules`, `detector.ignoreFiles`, `detector.ignoreValues`, and `detector.designSystem.enabled`. Hook lifecycle settings such as `hook.enabled` only affect automatic hook execution. For a waiver that should travel with one file instead of the repo config, add an inline comment in the file: ``. The marker works in any comment syntax, scopes to the whole file (or one line with `impeccable-disable-line` / `impeccable-disable-next-line`), and is bypassed by `--no-inline-ignores` or `--no-config`. diff --git a/cli/engine/browser/injected/index.mjs b/cli/engine/browser/injected/index.mjs index febf7f297..76fc558e5 100644 --- a/cli/engine/browser/injected/index.mjs +++ b/cli/engine/browser/injected/index.mjs @@ -1228,14 +1228,17 @@ if (IS_BROWSER) { isHidden: isElementHidden(el), findings: findings.map(f => { const ap = ANTIPATTERNS.find(a => a.id === (f.type || f.id)); + const severity = f.severity || ap?.severity || 'warning'; return { type: f.type || f.id, category: ap ? ap.category : 'quality', - severity: f.severity || ap?.severity || 'warning', + severity, // Advisory findings (em-dash overuse, etc.) are surfaced but never // treated as failures; carry the flag so the overlay/extension can // render them with the mildest affordance and consumers can filter. - advisory: (ap && ap.advisory === true) || f.advisory === true, + // Per-finding promotions override the registry default, so derive + // this strictly from the effective severity. + advisory: severity === 'advisory', detail: f.detail || f.snippet, ignoreValue: f.ignoreValue || f.value || '', name: ap ? ap.name : (f.type || f.id), @@ -1277,6 +1280,381 @@ if (IS_BROWSER) { else groupMap.set(el, [...kept]); } + function pseudoElementHostSelector(selector) { + const raw = String(selector || ''); + const legacyNames = new Set(['before', 'after', 'first-letter', 'first-line']); + const isNameChar = char => /[a-zA-Z0-9_-]/.test(char || ''); + const consumeFunction = (start) => { + let depth = 0; + let quote = ''; + for (let i = start; i < raw.length; i += 1) { + const char = raw[i]; + if (char === '\\') { + i += 1; + continue; + } + if (quote) { + if (char === quote) quote = ''; + continue; + } + if (char === '"' || char === "'") { + quote = char; + continue; + } + if (char === '(') depth += 1; + if (char === ')' && --depth === 0) return i + 1; + } + return raw.length; + }; + + let output = ''; + let found = false; + for (let i = 0; i < raw.length;) { + const char = raw[i]; + if (char === '\\') { + output += raw.slice(i, Math.min(raw.length, i + 2)); + i += 2; + continue; + } + if (char === '"' || char === "'") { + const quote = char; + const start = i; + i += 1; + while (i < raw.length) { + if (raw[i] === '\\') { + i += 2; + continue; + } + const value = raw[i]; + i += 1; + if (value === quote) break; + } + output += raw.slice(start, i); + continue; + } + if (char !== ':') { + output += char; + i += 1; + continue; + } + + let end = i + 1; + let isPseudoElement = false; + if (raw[end] === ':') { + end += 1; + const nameStart = end; + while (isNameChar(raw[end])) end += 1; + isPseudoElement = end > nameStart; + } else { + const nameStart = end; + while (isNameChar(raw[end])) end += 1; + isPseudoElement = legacyNames.has(raw.slice(nameStart, end).toLowerCase()); + } + if (!isPseudoElement) { + output += char; + i += 1; + continue; + } + if (raw[end] === '(') end = consumeFunction(end); + found = true; + if (!output || /[\s>+~,]/.test(output[output.length - 1])) output += '*'; + i = end; + } + if (!found) return null; + return output.trim().replace(/,\s*(?=,|$)/g, ''); + } + + function selectorNodesForLiveDom(root, selector) { + const raw = String(selector || '').trim(); + if (!raw) return null; + const fallback = pseudoElementHostSelector(raw); + if (fallback == null) { + // An empty result from a valid full selector is authoritative. In + // particular, do not broaden inactive :hover/:focus/:not() rules to + // their host element by stripping pseudo-classes. + try { return Array.from(root.querySelectorAll(raw)); } + catch { return null; } + } + + // Resolve pseudo-elements to their originating live elements. An attached + // pseudo-element (`.card::before`) belongs to the element before it, while + // a hostless pseudo-element after a combinator (`main > ::before`) belongs + // to a matching element at that position (`main > *`). Replacing every + // pseudo indiscriminately with an empty string leaves the latter as the + // invalid selector `main >` and makes absent hosts indistinguishable from + // selectors the DOM API cannot parse. + if (!fallback || /^[,\s]*$/.test(fallback)) return null; + try { return Array.from(root.querySelectorAll(fallback)); } + catch { return null; } + } + + let containerProbeSequence = 0; + + function isContainerCssRule(rule) { + return rule?.constructor?.name === 'CSSContainerRule' + || /^\s*@container\b/i.test(rule?.cssText || ''); + } + + function styleRuleAppliesToLiveMatches(rule, matches) { + const style = rule?.style; + if (!style || !matches?.length || typeof getComputedStyle !== 'function') return false; + const sequence = ++containerProbeSequence; + const property = `--impeccable-container-probe-${sequence}-${Math.random().toString(36).slice(2)}`; + const value = `impeccable-container-active-${sequence}`; + const previousValue = style.getPropertyValue(property); + const previousPriority = style.getPropertyPriority(property); + try { + style.setProperty(property, value, 'important'); + } catch { + return false; + } + + const pseudoElements = [...new Set( + String(rule.selectorText || '').match(/::[a-zA-Z-]+(?:\([^)]*\))?/g) || [], + )]; + try { + return matches.some(el => [null, ...pseudoElements].some(pseudo => { + try { + const computed = pseudo ? getComputedStyle(el, pseudo) : getComputedStyle(el); + return computed.getPropertyValue(property).trim() === value; + } catch { + return false; + } + })); + } finally { + if (previousValue) style.setProperty(property, previousValue, previousPriority); + else style.removeProperty(property); + } + } + + function conditionalCssRuleIsActive(rule) { + const type = Number(rule?.type); + const constructorName = rule?.constructor?.name || ''; + if (constructorName === 'CSSMediaRule' || type === 4) { + const condition = rule.conditionText || rule.media?.mediaText || ''; + if (!condition || typeof window.matchMedia !== 'function') return true; + try { return window.matchMedia(condition).matches; } + catch { return true; } + } + if (constructorName === 'CSSSupportsRule' || type === 12) { + const condition = rule.conditionText || ''; + if (!condition || typeof CSS === 'undefined' || typeof CSS.supports !== 'function') return true; + try { return CSS.supports(condition); } + catch { return true; } + } + return true; + } + + function splitCssCommaList(value) { + const parts = []; + let current = ''; + let quote = ''; + let escaped = false; + for (const char of String(value || '')) { + if (escaped) { + current += char; + escaped = false; + continue; + } + if (char === '\\') { + current += char; + escaped = true; + continue; + } + if (quote) { + current += char; + if (char === quote) quote = ''; + continue; + } + if (char === '"' || char === "'") { + quote = char; + current += char; + continue; + } + if (char === ',') { + parts.push(current); + current = ''; + continue; + } + current += char; + } + parts.push(current); + return parts; + } + + function normalizeAnimationName(value) { + const name = String(value || '').trim(); + if (name.length >= 2 && name[0] === name[name.length - 1] && (name[0] === '"' || name[0] === "'")) { + return name.slice(1, -1); + } + return name; + } + + function animationNamesDeclaredByRule(rule) { + const style = rule?.style; + if (!style) return []; + let value = ''; + try { + value = style.animationName + || style.getPropertyValue?.('animation-name') + || style.webkitAnimationName + || style.getPropertyValue?.('-webkit-animation-name') + || ''; + } catch { + return []; + } + return splitCssCommaList(value) + .map(normalizeAnimationName) + .filter(name => name && name.toLowerCase() !== 'none'); + } + + function keyframesRuleName(rule, cssText) { + const constructorName = rule?.constructor?.name || ''; + const type = Number(rule?.type); + const isKeyframes = constructorName === 'CSSKeyframesRule' + || constructorName === 'WebKitCSSKeyframesRule' + || type === 7 + || /^\s*@(?:-webkit-)?keyframes\b/i.test(cssText); + if (!isKeyframes) return ''; + const match = String(cssText || '').match(/^\s*@(?:-webkit-)?keyframes\s+([^\s{]+)/i); + return normalizeAnimationName(rule?.name || match?.[1] || ''); + } + + function cssPropertyName(property) { + if (property.startsWith('--')) return property; + return property.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); + } + + function resolvedAnimationKeyframes(candidateNames) { + if (typeof document.getAnimations !== 'function') return null; + let animations; + try { animations = document.getAnimations(); } + catch { return null; } + + const resolved = new Map(); + const metadata = new Set(['offset', 'computedOffset', 'easing', 'composite']); + for (const animation of animations) { + const name = normalizeAnimationName(animation?.animationName || ''); + if (!name || !candidateNames.has(name) || resolved.has(name)) continue; + let frames; + try { frames = animation.effect?.getKeyframes?.() || []; } + catch { continue; } + const blocks = []; + for (const frame of frames) { + const rawOffset = Number.isFinite(frame.computedOffset) ? frame.computedOffset : frame.offset; + if (!Number.isFinite(rawOffset)) continue; + const offset = Math.round(rawOffset * 1000000) / 10000; + const declarations = Object.entries(frame) + .filter(([property, value]) => !metadata.has(property) && value != null && value !== '') + .map(([property, value]) => `${cssPropertyName(property)}: ${value};`); + const easing = String(frame.easing || '').trim(); + if (easing && easing.toLowerCase() !== 'linear') { + declarations.push(`animation-timing-function: ${easing};`); + } + if (declarations.length === 0) continue; + blocks.push(`${offset}% { ${declarations.join(' ')} }`); + } + if (blocks.length > 0) resolved.set(name, `@keyframes ${name} { ${blocks.join(' ')} }`); + } + return resolved; + } + + // Read CSS that is absent from document.outerHTML. Inline
${primary}${secondary}
`); + await fontPage.evaluate(() => { window.__IMPECCABLE_CONFIG__ = { autoScan: false }; }); + await fontPage.evaluate(browserScript); + const fontFindings = await fontPage.evaluate(() => window.impeccableDetect({ serialize: true }) + .flatMap(group => group.findings || []) + .filter(finding => finding.type === 'overused-font')); + assert.equal(fontFindings.length, 1, JSON.stringify(fontFindings)); + assert.match(fontFindings[0].detail, /Primary font: geist \(82% of text\)/i); + assert.doesNotMatch(fontFindings[0].detail, /geist mono/i); + await fontPage.close(); + } finally { + await browser.close().catch(() => {}); + } + }); + // Only a real browser reproduces this one: Chrome keeps oklch(), lch(), and // color(srgb ...) verbatim in getComputedStyle output, so a detector that // cannot parse those reads every surface as unset, walks out of the page, diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js index 9ad3a7a47..30c5e8f18 100644 --- a/tests/detect-antipatterns.test.js +++ b/tests/detect-antipatterns.test.js @@ -2717,23 +2717,30 @@ describe('CLI', () => { expect(code).toBe(0); expect(stdout).toContain('Usage:'); expect(stdout).toContain('--quiet'); + expect(stdout).toContain('Human-readable findings go to stderr'); expect(stdout).not.toContain('--gpt'); expect(stdout).not.toContain('--gemini'); }); - test('generated-UI tells run by default in the CLI', () => { + test('severity advisory is non-blocking, flagged in JSON, and suppressible', () => { const { stdout, code } = run('--json', path.join(FIXTURES, 'gpt-tells.html')); - expect(code).toBe(2); - const ids = JSON.parse(stdout).map(f => f.antipattern); + expect(code).toBe(0); + const findings = JSON.parse(stdout); + const ids = findings.map(f => f.antipattern); expect(ids).toContain('gpt-thin-border-wide-shadow'); expect(ids).toContain('repeating-stripes-gradient'); expect(ids).toContain('codex-grid-background'); expect(ids).toContain('theater-slop-phrase'); + expect(findings.every(f => f.severity === 'advisory' && f.advisory === true)).toBe(true); + + const hidden = run('--json', '--no-advisory', path.join(FIXTURES, 'gpt-tells.html')); + expect(hidden.code).toBe(0); + expect(JSON.parse(hidden.stdout)).toEqual([]); }); test('legacy provider flags are accepted as deprecated no-ops', () => { const { stdout, stderr, code } = run('--gpt', '--json', path.join(FIXTURES, 'gpt-tells.html')); - expect(code).toBe(2); + expect(code).toBe(0); expect(stderr).toContain('--gpt and --gemini are deprecated and ignored'); expect(JSON.parse(stdout).some(f => f.antipattern === 'codex-grid-background')).toBe(true); }); @@ -2744,14 +2751,30 @@ describe('CLI', () => { expect(stderr).not.toContain('cannot access detect'); }); + test('keeps a local path containing spaces as one scan target', () => { + const fixture = writeStaticFixture({ + 'page with spaces.html': '

Plain page

', + }); + const file = path.join(fixture.dir, 'page with spaces.html'); + try { + const { stdout, stderr, code } = run('--json', file); + expect(code).toBe(0); + expect(JSON.parse(stdout)).toEqual([]); + expect(stderr).not.toContain('cannot access'); + } finally { + fs.rmSync(fixture.dir, { recursive: true, force: true }); + } + }); + test('should-pass exits 0', () => { const { code } = run(path.join(FIXTURES, 'should-pass.html')); expect(code).toBe(0); }); test('should-flag exits 2 with findings', () => { - const { code, stderr } = run(path.join(FIXTURES, 'should-flag.html')); + const { stdout, code, stderr } = run(path.join(FIXTURES, 'should-flag.html')); expect(code).toBe(2); + expect(stdout).toBe(''); expect(stderr).toContain('side-tab'); }); @@ -2899,7 +2922,7 @@ colors: `); const full = runIn(dir, '--json', 'index.css'); - expect(full.code).toBe(2); + expect(full.code).toBe(0); const fullIds = JSON.parse(full.stdout).map((finding) => finding.antipattern); expect(fullIds).toContain('design-system-font-size'); expect(fullIds).toContain('design-system-color'); diff --git a/tests/detect-cli-stdin-dispatch.test.mjs b/tests/detect-cli-stdin-dispatch.test.mjs index 2481d00f8..a0ee03e34 100644 --- a/tests/detect-cli-stdin-dispatch.test.mjs +++ b/tests/detect-cli-stdin-dispatch.test.mjs @@ -10,7 +10,7 @@ const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const cli = path.join(root, 'cli', 'bin', 'cli.js'); const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-stdin-dispatch-')); -function detectStdinFile(filePath) { +function detectStdinFile(filePath, expectedStatus = 2) { const result = spawnSync( process.execPath, [cli, 'detect', '--json', '--no-config', '--no-design-system'], @@ -19,7 +19,7 @@ function detectStdinFile(filePath) { encoding: 'utf8', }, ); - assert.equal(result.status, 2, result.stderr); + assert.equal(result.status, expectedStatus, result.stderr); return JSON.parse(result.stdout); } @@ -57,9 +57,11 @@ describe('detect CLI stdin file dispatch', () => { } `); - const findings = detectStdinFile(filePath); + const findings = detectStdinFile(filePath, 0); assert.ok(findings.some( - (item) => item.file === filePath && item.antipattern === 'codex-grid-background', + (item) => item.file === filePath + && item.antipattern === 'codex-grid-background' + && item.advisory === true, )); }); }); diff --git a/tests/fixtures/antipatterns/linked-url-patterns.css b/tests/fixtures/antipatterns/linked-url-patterns.css new file mode 100644 index 000000000..c0f750408 --- /dev/null +++ b/tests/fixtures/antipatterns/linked-url-patterns.css @@ -0,0 +1,192 @@ +@media (min-width: 1px) { + [data-token="::before"] { + width: 160px; + height: 80px; + background: linear-gradient(90deg, #d9d9d9 1px, transparent 1px), linear-gradient(180deg, #d9d9d9 1px, transparent 1px); + background-size: 72px 72px; + } +} + +body { + background: #111; + color: #fff; +} + +/* Literal pseudo-element text inside an attribute value is data, not selector + syntax. This rule has no live match even though an empty-value decoy does. */ +[data-decoy="::before"] { + color: #7c3aed; +} + +/* Escaped colons are identifier data, not a legacy pseudo-element. */ +.\:\:before { + width: 240px; + height: 160px; + clip-path: polygon(2% 4%, 17% 1%, 31% 7%, 47% 3%, 62% 9%, 79% 2%, 96% 13%, 91% 31%, 98% 49%, 89% 68%, 95% 87%, 74% 96%, 51% 91%, 29% 98%, 8% 84%, 3% 61%); +} + +/* A valid hostless pseudo-element selector cannot be queried through the DOM + selector API. It must remain in the corpus rather than count as unused. */ +main > ::before { + content: "Rendered pseudo-element text"; + display: block; + width: 160px; + animation: bounce-linked-pseudo 1s ease-in-out infinite; +} + +@keyframes bounce-linked-pseudo { + 50% { transform: translateY(2px); } +} + +.linked-marquee { + animation: linked-horizontal-loop 8s linear infinite; +} + +@keyframes linked-horizontal-loop { + from { transform: translateX(0); } + to { transform: translateX(-50%); } +} + +.linked-keyframe-overshoot { + animation: linked-keyframe-curve 2s linear infinite; +} + +@keyframes linked-keyframe-curve { + from { + transform: translateY(0); + animation-timing-function: cubic-bezier(0.34, 1.56, 0.64, 1); + } + to { transform: translateY(2px); } +} + +.overridden-keyframes-animation { + animation: overridden-horizontal-loop 2s linear infinite; +} + +@keyframes overridden-horizontal-loop { + from { transform: translateX(0); } + to { transform: translateX(-50%); } +} + +/* The later same-name definition is the one Chromium renders. */ +@keyframes overridden-horizontal-loop { + 50% { opacity: 0.4; } +} + +@layer linked-keyframes-low, linked-keyframes-high; + +.layered-keyframes-animation { + animation: layered-horizontal-loop 2s linear infinite; +} + +@layer linked-keyframes-high { + @keyframes layered-horizontal-loop { + from { transform: translateX(0); } + to { transform: translateX(-50%); } + } +} + +/* Lower layer appears later in source, but does not override the high layer. */ +@layer linked-keyframes-low { + @keyframes layered-horizontal-loop { + 50% { opacity: 0.4; } + } +} + +.linked-pulse-dot { + width: 8px; + height: 8px; + border-radius: 50%; + animation: linked-signal-cycle 1.5s ease-in-out infinite; +} + +@keyframes linked-signal-cycle { + 50% { opacity: 0.35; } +} + +/* Chromium makes nested keyframes globally available even while the enclosing + container condition is false, so this live reference must still scan. */ +.inactive-container-animation-reference { + animation: inactive-container-horizontal-loop 8s linear infinite; +} + +@container (width > 2000px) { + @keyframes inactive-container-horizontal-loop { + from { transform: translateX(0); } + to { transform: translateX(-50%); } + } +} + +.active-container-animation-reference { + animation: active-container-horizontal-loop 8s linear infinite; +} + +/* The declaration is intentionally outside the container group. */ +@container (width > 900px) { + @keyframes active-container-horizontal-loop { + from { transform: translateX(0); } + to { transform: translateX(-50%); } + } +} + +/* A pseudo-element whose originating element is absent must remain outside + live URL findings instead of being retained as an unresolvable selector. */ +.absent > ::before { + background: repeating-linear-gradient(45deg, #eee, #eee 10px, #fafafa 10px, #fafafa 20px); +} + +/* These selectors exist in the live DOM, but their conditions are inactive. + URL scans must not treat their declarations as rendered page styles. */ +@media (max-width: 1px) { + .inactive-media-stripes { + background: repeating-linear-gradient(45deg, #eee, #eee 10px, #fafafa 10px, #fafafa 20px); + } +} + +@supports (display: imaginary-layout) { + .inactive-supports-stripes { + background: repeating-linear-gradient(45deg, #eee, #eee 10px, #fafafa 10px, #fafafa 20px); + } +} + +/* The host exists, but the complete pseudo-class selector is inactive. */ +.inactive-pseudo-stripes:not(.active) { + background: repeating-linear-gradient(45deg, #eee, #eee 10px, #fafafa 10px, #fafafa 20px); +} + +.container-query-host { + container-type: inline-size; + width: 240px; +} + +.container-query-host-active { + width: 960px; +} + +@container (width > 900px) { + .inactive-container-stripes { + background: repeating-linear-gradient(45deg, #eee, #eee 10px, #fafafa 10px, #fafafa 20px); + } + + .active-container-halo { + width: 640px; + height: 400px; + background: radial-gradient(circle, rgba(80, 111, 255, 0.85), transparent 70%); + } +} + +/* Non-selector at-rules in an inactive container must not enter page-level + pattern scans just because their CSSOM text is readable. */ +@container (width > 2000px) { + @keyframes inactive-container-gradient-text { + from { + background: linear-gradient(90deg, #111, #999); + background-clip: text; + } + to { background: none; } + } +} + +.unused-linked-transition { + transition: width 200ms ease; +} diff --git a/tests/fixtures/antipatterns/linked-url-patterns.html b/tests/fixtures/antipatterns/linked-url-patterns.html new file mode 100644 index 000000000..eebb3acc5 --- /dev/null +++ b/tests/fixtures/antipatterns/linked-url-patterns.html @@ -0,0 +1,32 @@ + + + + + Linked URL pattern detection + + + +
+

Linked stylesheet pattern

+
Rendered decorative grid
+
Empty attribute-value decoy
+
Escaped identifier selector
+
Rendered linked marquee animation
+
Rendered linked keyframe easing
+
Overridden linked keyframes
+
Layer-priority linked keyframes
+
+
Inactive media stripes
+
Inactive supports stripes
+
Inactive pseudo-class stripes
+
+
Inactive container-query stripes
+
False-container keyframes reference
+
+
+
Active container-query halo
+
Active container keyframes reference
+
+
+ + diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index ee12a5876..660b43817 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -665,6 +665,7 @@ describe('filterFindings()', () => { const filtered = filterFindings(findings, content, '.ts', { ignoreRules: ['side-tab'], minSeverity: 'error', + advisoryRules: 'include', limits: DEFAULT_CONFIG.limits, }); assert.deepEqual(filtered.map((f) => f.antipattern), ['gradient-text', 'overused-font']); @@ -674,6 +675,7 @@ describe('filterFindings()', () => { const findings = [ finding('side-tab', 1), finding('em-dash-overuse', 2), + finding('design-system-radius', 3, { severity: 'advisory' }), finding('gradient-text', 3), ]; const filtered = filterFindings(findings, '', '.html', { @@ -700,6 +702,7 @@ describe('filterFindings()', () => { assert.ok(ADVISORY_RULES.has('em-dash-overuse')); assert.equal(isAdvisoryFinding(finding('em-dash-overuse', 1)), true); assert.equal(isAdvisoryFinding({ antipattern: 'anything', advisory: true }), true); + assert.equal(isAdvisoryFinding({ antipattern: 'anything', severity: 'advisory' }), true); assert.equal(isAdvisoryFinding(finding('side-tab', 1)), false); });