mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 23:26:39 +03:00
Fix detector URL and advisory handling
Recover joined URL arguments without splitting local paths, derive advisory behavior from registry severity across consumers, inspect readable linked CSS in URL scans, and report only the dominant primary font. AI assistance disclosure: Implemented and verified with Codex under maintainer direction.
This commit is contained in:
@@ -1235,7 +1235,7 @@ if (IS_BROWSER) {
|
||||
// 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,
|
||||
advisory: ap?.severity === 'advisory' || f.severity === 'advisory' || f.advisory === true,
|
||||
detail: f.detail || f.snippet,
|
||||
ignoreValue: f.ignoreValue || f.value || '',
|
||||
name: ap ? ap.name : (f.type || f.id),
|
||||
@@ -1277,6 +1277,36 @@ if (IS_BROWSER) {
|
||||
else groupMap.set(el, [...kept]);
|
||||
}
|
||||
|
||||
// Read CSS that is absent from document.outerHTML. Inline <style> blocks are
|
||||
// already present in the HTML pattern corpus, so limit this walk to linked
|
||||
// stylesheets. Same-origin CSS and readable CORS sheets participate; browser
|
||||
// security exceptions for cross-origin sheets are expected and skipped.
|
||||
function linkedStylesheetText() {
|
||||
const parts = [];
|
||||
const seen = new Set();
|
||||
const appendSheet = (sheet) => {
|
||||
if (!sheet || seen.has(sheet)) return;
|
||||
seen.add(sheet);
|
||||
let rules;
|
||||
try { rules = Array.from(sheet.cssRules || sheet.rules || []); }
|
||||
catch { return; }
|
||||
for (const rule of rules) {
|
||||
if (rule.styleSheet) appendSheet(rule.styleSheet);
|
||||
else if (rule.cssText) parts.push(rule.cssText);
|
||||
}
|
||||
};
|
||||
let sheets;
|
||||
try { sheets = Array.from(document.styleSheets || []); }
|
||||
catch { return ''; }
|
||||
for (const sheet of sheets) {
|
||||
const owner = sheet.ownerNode;
|
||||
if (owner?.tagName?.toLowerCase() !== 'link') continue;
|
||||
if (!/\bstylesheet\b/i.test(owner.getAttribute?.('rel') || '')) continue;
|
||||
appendSheet(sheet);
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
function browserFindingsFromMap(groupMap) {
|
||||
return [...groupMap.entries()].map(([el, findings]) => ({ el, findings }));
|
||||
}
|
||||
@@ -1650,7 +1680,11 @@ if (IS_BROWSER) {
|
||||
// (the CSS ships here, but the pattern never renders — the live DOM is
|
||||
// ground truth in the browser), and a match under a data-impeccable-ignore
|
||||
// ancestor is waived. Selector-less findings stay page-level.
|
||||
const scopedHtmlFindings = checkHtmlPatterns(docClone.outerHTML).filter(f => {
|
||||
const html = docClone.outerHTML;
|
||||
const corpora = buildHtmlPatternCorpora(html);
|
||||
const linkedCss = linkedStylesheetText();
|
||||
if (linkedCss) corpora.styleText += `\n${linkedCss}`;
|
||||
const scopedHtmlFindings = checkHtmlPatterns(html, corpora).filter(f => {
|
||||
if (!f.selector) return true;
|
||||
const query = String(f.selector).replace(/::?[a-zA-Z-]+(\([^)]*\))?/g, '').trim().replace(/,\s*(?=,|$)/g, '');
|
||||
if (!query || /^[,\s]*$/.test(query)) return true;
|
||||
|
||||
+26
-6
@@ -37,13 +37,30 @@ function fileUrlToLocalPath(url) {
|
||||
}
|
||||
}
|
||||
|
||||
const URL_TARGET_RE = /^(?:https?|file):\/\//i;
|
||||
|
||||
// Some agent runners hand a shell-ready URL list to Node as one argv value.
|
||||
// A browser accepts the spaces as part of one encoded URL, producing a
|
||||
// plausible scan attributed to a bogus joined path. Expand only when every
|
||||
// whitespace-delimited token is independently a URL, preserving ordinary
|
||||
// filesystem paths that contain spaces.
|
||||
function expandJoinedUrlTargets(targets) {
|
||||
return targets.flatMap((target) => {
|
||||
if (!/\s/.test(target)) return [target];
|
||||
const parts = target.trim().split(/\s+/).filter(Boolean);
|
||||
return parts.length > 1 && parts.every(part => URL_TARGET_RE.test(part))
|
||||
? parts
|
||||
: [target];
|
||||
});
|
||||
}
|
||||
|
||||
// Advisory findings are detected but never treated as failures: they list in a
|
||||
// separate, visually dimmed section, are excluded from the failure count that
|
||||
// drives the exit code, and carry `"advisory": true` in JSON so consumers can
|
||||
// filter. Every advisory finding carries the flag (stamped by the registry via
|
||||
// findings.mjs).
|
||||
function isAdvisory(finding) {
|
||||
return finding && finding.advisory === true;
|
||||
return Boolean(finding && (finding.advisory === true || finding.severity === 'advisory'));
|
||||
}
|
||||
|
||||
function partitionAdvisory(findings) {
|
||||
@@ -168,6 +185,10 @@ Advisory findings:
|
||||
counted as failures and never changing the exit code. They stay out of the
|
||||
failure count so they never block automation. --no-advisory hides them.
|
||||
|
||||
Output streams:
|
||||
Human-readable findings go to stderr so stdout stays available for structured
|
||||
output. Use --json for JSON on stdout, or redirect text with 2> findings.txt.
|
||||
|
||||
Project config:
|
||||
Respects .impeccable/config.json and .impeccable/config.local.json detector
|
||||
settings: detector.ignoreRules, detector.ignoreFiles, detector.ignoreValues,
|
||||
@@ -185,7 +206,7 @@ Detection modes:
|
||||
HTML files Static HTML/CSS analysis (default, catches linked CSS)
|
||||
Non-HTML files Regex pattern matching (CSS, JSX, TSX, etc.)
|
||||
URLs Puppeteer full browser rendering (auto-detected;
|
||||
http(s):// and file:// URLs)
|
||||
http(s):// and file:// URLs; accessible linked CSS included)
|
||||
|
||||
Examples:
|
||||
impeccable detect src/
|
||||
@@ -283,7 +304,7 @@ async function detectCli() {
|
||||
const designSystem = loadDesignSystemForTarget(localPath, { cache: designSystemCache });
|
||||
return designSystem ? { ...baseScanOptions, designSystem } : baseScanOptions;
|
||||
};
|
||||
const targets = args.filter(a => !a.startsWith('--'));
|
||||
const targets = expandJoinedUrlTargets(args.filter(a => !a.startsWith('--')));
|
||||
|
||||
if (helpMode) { printUsage(); process.exit(0); }
|
||||
|
||||
@@ -297,13 +318,12 @@ async function detectCli() {
|
||||
// real cascade, real computed styles, real layout. Callers that want a
|
||||
// browser-grade scan of a local artifact can pass file:///abs/path.html
|
||||
// instead of the bare path (which stays on the static engine).
|
||||
const urlRe = /^(?:https?|file):\/\//i;
|
||||
const urlTargetCount = paths.filter(target => urlRe.test(target)).length;
|
||||
const urlTargetCount = paths.filter(target => URL_TARGET_RE.test(target)).length;
|
||||
const browserDetector = urlTargetCount > 1 ? await createBrowserDetector() : null;
|
||||
|
||||
try {
|
||||
for (const target of paths) {
|
||||
if (urlRe.test(target)) {
|
||||
if (URL_TARGET_RE.test(target)) {
|
||||
// A file:// URL points at a local artifact, so its design system
|
||||
// resolves from that file's project. A remote http(s) URL has no
|
||||
// local project — it gets base options (no design system), never
|
||||
|
||||
@@ -358,7 +358,7 @@ const ANTIPATTERNS = [
|
||||
// rather than a failure. It fires only on the AI saturation pattern, not on
|
||||
// ordinary prose. Advisory findings are surfaced separately, never counted
|
||||
// as failures, and skipped by the design hook unless a project opts in.
|
||||
advisory: true,
|
||||
severity: 'advisory',
|
||||
name: 'Em-dash overuse',
|
||||
description:
|
||||
'Em-dash saturation in body copy is an AI cadence tell. Advisory only: humans use em-dashes legitimately, so this fires only on saturation — at least 8 em-dashes (— or --) at a density near one per 500 characters of body text — never on a long article that uses a few. Prefer commas, colons, periods, or parentheses.',
|
||||
@@ -5293,14 +5293,17 @@ function checkTypography() {
|
||||
}
|
||||
|
||||
if (totalTextElements >= 20) {
|
||||
// A font is "primary" if it's used by at least 15% of text elements
|
||||
const PRIMARY_THRESHOLD = 0.15;
|
||||
for (const [font, count] of fontUsage) {
|
||||
// Report the actual primary face: the uniquely most-used family. The old
|
||||
// 15% threshold labeled secondary faces as primary (e.g. an 82/18 split).
|
||||
const ranked = [...fontUsage.entries()].sort((a, b) => b[1] - a[1]);
|
||||
const [primary] = ranked;
|
||||
const tied = ranked[1]?.[1] === primary?.[1];
|
||||
if (primary && !tied) {
|
||||
const [font, count] = primary;
|
||||
const share = count / totalTextElements;
|
||||
if (share < PRIMARY_THRESHOLD) continue;
|
||||
if (!OVERUSED_FONTS.has(font)) continue;
|
||||
if (isBrandFontOnOwnDomain(font)) continue;
|
||||
findings.push({ type: 'overused-font', detail: `Primary font: ${font} (${Math.round(share * 100)}% of text)` });
|
||||
if (OVERUSED_FONTS.has(font) && !isBrandFontOnOwnDomain(font)) {
|
||||
findings.push({ type: 'overused-font', detail: `Primary font: ${font} (${Math.round(share * 100)}% of text)` });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8133,7 +8136,7 @@ if (IS_BROWSER) {
|
||||
// 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,
|
||||
advisory: ap?.severity === 'advisory' || f.severity === 'advisory' || f.advisory === true,
|
||||
detail: f.detail || f.snippet,
|
||||
ignoreValue: f.ignoreValue || f.value || '',
|
||||
name: ap ? ap.name : (f.type || f.id),
|
||||
@@ -8175,6 +8178,36 @@ if (IS_BROWSER) {
|
||||
else groupMap.set(el, [...kept]);
|
||||
}
|
||||
|
||||
// Read CSS that is absent from document.outerHTML. Inline <style> blocks are
|
||||
// already present in the HTML pattern corpus, so limit this walk to linked
|
||||
// stylesheets. Same-origin CSS and readable CORS sheets participate; browser
|
||||
// security exceptions for cross-origin sheets are expected and skipped.
|
||||
function linkedStylesheetText() {
|
||||
const parts = [];
|
||||
const seen = new Set();
|
||||
const appendSheet = (sheet) => {
|
||||
if (!sheet || seen.has(sheet)) return;
|
||||
seen.add(sheet);
|
||||
let rules;
|
||||
try { rules = Array.from(sheet.cssRules || sheet.rules || []); }
|
||||
catch { return; }
|
||||
for (const rule of rules) {
|
||||
if (rule.styleSheet) appendSheet(rule.styleSheet);
|
||||
else if (rule.cssText) parts.push(rule.cssText);
|
||||
}
|
||||
};
|
||||
let sheets;
|
||||
try { sheets = Array.from(document.styleSheets || []); }
|
||||
catch { return ''; }
|
||||
for (const sheet of sheets) {
|
||||
const owner = sheet.ownerNode;
|
||||
if (owner?.tagName?.toLowerCase() !== 'link') continue;
|
||||
if (!/\bstylesheet\b/i.test(owner.getAttribute?.('rel') || '')) continue;
|
||||
appendSheet(sheet);
|
||||
}
|
||||
return parts.join('\n');
|
||||
}
|
||||
|
||||
function browserFindingsFromMap(groupMap) {
|
||||
return [...groupMap.entries()].map(([el, findings]) => ({ el, findings }));
|
||||
}
|
||||
@@ -8548,7 +8581,11 @@ if (IS_BROWSER) {
|
||||
// (the CSS ships here, but the pattern never renders — the live DOM is
|
||||
// ground truth in the browser), and a match under a data-impeccable-ignore
|
||||
// ancestor is waived. Selector-less findings stay page-level.
|
||||
const scopedHtmlFindings = checkHtmlPatterns(docClone.outerHTML).filter(f => {
|
||||
const html = docClone.outerHTML;
|
||||
const corpora = buildHtmlPatternCorpora(html);
|
||||
const linkedCss = linkedStylesheetText();
|
||||
if (linkedCss) corpora.styleText += `\n${linkedCss}`;
|
||||
const scopedHtmlFindings = checkHtmlPatterns(html, corpora).filter(f => {
|
||||
if (!f.selector) return true;
|
||||
const query = String(f.selector).replace(/::?[a-zA-Z-]+(\([^)]*\))?/g, '').trim().replace(/,\s*(?=,|$)/g, '');
|
||||
if (!query || /^[,\s]*$/.test(query)) return true;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { getAntipattern } from './registry/antipatterns.mjs';
|
||||
import { getAntipattern, isAdvisoryRule } from './registry/antipatterns.mjs';
|
||||
|
||||
function getAP(id) {
|
||||
return getAntipattern(id);
|
||||
@@ -11,7 +11,7 @@ function finding(id, filePath, snippet, line = 0) {
|
||||
// failures. Carry the flag on the finding so every consumer (CLI, JSON, hook)
|
||||
// can partition without a registry lookup. Only stamped when true to keep the
|
||||
// finding shape stable for the vast majority of rules.
|
||||
if (ap.advisory === true) base.advisory = true;
|
||||
if (isAdvisoryRule(id)) base.advisory = true;
|
||||
return base;
|
||||
}
|
||||
|
||||
|
||||
@@ -233,7 +233,7 @@ const ANTIPATTERNS = [
|
||||
// rather than a failure. It fires only on the AI saturation pattern, not on
|
||||
// ordinary prose. Advisory findings are surfaced separately, never counted
|
||||
// as failures, and skipped by the design hook unless a project opts in.
|
||||
advisory: true,
|
||||
severity: 'advisory',
|
||||
name: 'Em-dash overuse',
|
||||
description:
|
||||
'Em-dash saturation in body copy is an AI cadence tell. Advisory only: humans use em-dashes legitimately, so this fires only on saturation — at least 8 em-dashes (— or --) at a density near one per 500 characters of body text — never on a long article that uses a few. Prefer commas, colons, periods, or parentheses.',
|
||||
@@ -588,9 +588,10 @@ function getAntipattern(id) {
|
||||
// Advisory rules are detected and reported, but never treated as failures:
|
||||
// the CLI lists them under a separate "Advisory" section, they do not affect
|
||||
// exit codes or the failure count, and the design hook skips them by default.
|
||||
// The set is derived from the registry so a rule only needs `advisory: true`.
|
||||
// `severity` is the canonical registry field. The runtime finding serializer
|
||||
// derives its `advisory: true` compatibility/output flag from this set.
|
||||
const ADVISORY_RULE_IDS = new Set(
|
||||
ANTIPATTERNS.filter(rule => rule.advisory === true).map(rule => rule.id),
|
||||
ANTIPATTERNS.filter(rule => rule.severity === 'advisory').map(rule => rule.id),
|
||||
);
|
||||
|
||||
function isAdvisoryRule(id) {
|
||||
|
||||
@@ -4020,14 +4020,17 @@ function checkTypography() {
|
||||
}
|
||||
|
||||
if (totalTextElements >= 20) {
|
||||
// A font is "primary" if it's used by at least 15% of text elements
|
||||
const PRIMARY_THRESHOLD = 0.15;
|
||||
for (const [font, count] of fontUsage) {
|
||||
// Report the actual primary face: the uniquely most-used family. The old
|
||||
// 15% threshold labeled secondary faces as primary (e.g. an 82/18 split).
|
||||
const ranked = [...fontUsage.entries()].sort((a, b) => b[1] - a[1]);
|
||||
const [primary] = ranked;
|
||||
const tied = ranked[1]?.[1] === primary?.[1];
|
||||
if (primary && !tied) {
|
||||
const [font, count] = primary;
|
||||
const share = count / totalTextElements;
|
||||
if (share < PRIMARY_THRESHOLD) continue;
|
||||
if (!OVERUSED_FONTS.has(font)) continue;
|
||||
if (isBrandFontOnOwnDomain(font)) continue;
|
||||
findings.push({ type: 'overused-font', detail: `Primary font: ${font} (${Math.round(share * 100)}% of text)` });
|
||||
if (OVERUSED_FONTS.has(font) && !isBrandFontOnOwnDomain(font)) {
|
||||
findings.push({ type: 'overused-font', detail: `Primary font: ${font} (${Math.round(share * 100)}% of text)` });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user