mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 00:26:41 +03:00
Carry source lines on pseudo-stripe findings and skip commented-out rules
Review bots caught two real gaps in the pseudo-stripe wiring: findings had no source line (so line-scoped impeccable-disable directives could not match them), and the scanner read commented-out CSS as live rules. scanCssTextForPseudoStripe now blanks comment bodies byte-for-byte (preserving offsets) and returns each rule's selector offset; the three regex-engine call sites convert that to a real line, including the whole-file line for component style blocks and CSS-in-JS templates. The HTML path ignores the new field. Tests now assert every finding's line against the selector's actual position and cover a commented-out stripe. Prepared with AI assistance (Claude Code), directed by @pbakaus. Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
b8f1dbf92c
commit
a4a076005b
@@ -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;
|
||||
|
||||
@@ -653,14 +653,20 @@ function detectText(content, filePath, options = {}) {
|
||||
profile,
|
||||
phase: 'source',
|
||||
}));
|
||||
// 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));
|
||||
// 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 the
|
||||
// same coverage.
|
||||
findings.push(...scanCssTextForPseudoStripe(content).map(hit => finding(hit.id, filePath, hit.snippet)));
|
||||
findings.push(...pseudoStripeFindings(content, 0));
|
||||
}
|
||||
|
||||
// Block-level CSS checks that need multiple declarations must run over the
|
||||
@@ -698,7 +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(...scanCssTextForPseudoStripe(block.content).map(hit => finding(hit.id, filePath, hit.snippet)));
|
||||
findings.push(...pseudoStripeFindings(block.content, block.startLine - 2));
|
||||
}
|
||||
|
||||
// Extract and scan CSS-in-JS template literals
|
||||
@@ -717,7 +723,7 @@ function detectText(content, filePath, options = {}) {
|
||||
phase: 'css-in-js',
|
||||
}));
|
||||
findings.push(...scanInsetStripeCss(block.content, filePath, block.startLine - 1));
|
||||
findings.push(...scanCssTextForPseudoStripe(block.content).map(hit => finding(hit.id, filePath, hit.snippet)));
|
||||
findings.push(...pseudoStripeFindings(block.content, block.startLine - 1));
|
||||
}
|
||||
|
||||
if (options?.designSystem) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -104,8 +104,18 @@ describe('detectText — pseudo-element stripe fixtures (issue #394)', () => {
|
||||
'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');
|
||||
@@ -117,15 +127,30 @@ describe('detectText — pseudo-element stripe fixtures (issue #394)', () => {
|
||||
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', () => {
|
||||
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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+10
@@ -108,3 +108,13 @@
|
||||
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;
|
||||
}
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user