mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-15 23:56:29 +03:00
Merge pull request #553 from pbakaus/fix/issue-547-shadow-token-context
Allow documented sidecar shadow colors in shadow contexts (#547)
This commit is contained in:
@@ -14,6 +14,10 @@ const FALLBACK_DIRS = ['.agents/context', 'docs'];
|
||||
// boundaries; `.impeccable` is our own project marker.
|
||||
const PROJECT_ROOT_MARKERS = ['.git', 'package.json', '.impeccable'];
|
||||
const COLOR_CHANNEL_TOLERANCE = 6;
|
||||
// Shadow blacks at different alphas are different tokens (0.28 vs 0.55 is the
|
||||
// difference between a documented shadow and drift), so shadow matching cannot
|
||||
// reuse the r/g/b-only channel tolerance.
|
||||
const SHADOW_ALPHA_TOLERANCE = 0.02;
|
||||
const RADIUS_TOLERANCE_PX = 0.5;
|
||||
const FONT_SIZE_TOLERANCE_PX = 0.5;
|
||||
const FONT_SIZE_LITERAL_RE = /^-?[\d.]+(?:px|rem)$/;
|
||||
@@ -474,6 +478,25 @@ function addSidecarRadii(out, sidecar) {
|
||||
}
|
||||
}
|
||||
|
||||
// Sidecar `extensions.shadows` entries ({ name, value, purpose }) carry the
|
||||
// documented shadow vocabulary that Stitch's frontmatter schema can't hold.
|
||||
// Their colors go into a separate allowlist — NOT allowedColorKeys — because a
|
||||
// shadow black is only documented *as a shadow*: feeding it into the general
|
||||
// color allowlist would legalize #000 as a page ground (alpha is dropped from
|
||||
// colorKey), which is the hole issue #547 warns against.
|
||||
function addSidecarShadows(out, sidecar) {
|
||||
const shadows = sidecar?.extensions?.shadows;
|
||||
if (!Array.isArray(shadows)) return;
|
||||
|
||||
for (const entry of shadows) {
|
||||
if (typeof entry?.value !== 'string') continue;
|
||||
for (const match of entry.value.matchAll(CSS_COLOR_RE)) {
|
||||
const parsed = parseDesignColor(match[0]);
|
||||
if (parsed) out.allowedShadowColors.push({ color: parsed });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeDesignSystem(input = {}) {
|
||||
const frontmatter = input.frontmatter || {};
|
||||
const sidecar = input.sidecar || null;
|
||||
@@ -486,6 +509,7 @@ function normalizeDesignSystem(input = {}) {
|
||||
allowedColorKeys: new Map(),
|
||||
allowedRadii: [],
|
||||
allowedFontSizes: [],
|
||||
allowedShadowColors: [],
|
||||
hasPillRadius: false,
|
||||
};
|
||||
|
||||
@@ -495,6 +519,7 @@ function normalizeDesignSystem(input = {}) {
|
||||
addSidecarColors(out, sidecar);
|
||||
addRoundedScale(out, frontmatter.rounded);
|
||||
addSidecarRadii(out, sidecar);
|
||||
addSidecarShadows(out, sidecar);
|
||||
|
||||
out.hasFonts = out.allowedFonts.size > 0;
|
||||
out.hasColors = out.allowedColorKeys.size > 0;
|
||||
@@ -614,6 +639,20 @@ function isAllowedColorRaw(raw, designSystem) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// A color is a documented shadow color only when both the r/g/b channels AND
|
||||
// the alpha match a sidecar shadow token's color. Alpha has to be compared
|
||||
// here because colorKey()/colorsClose() drop it, and a match on r/g/b alone
|
||||
// would let every black at every alpha through.
|
||||
function isAllowedShadowColorRaw(raw, designSystem) {
|
||||
if (!designSystem?.allowedShadowColors?.length) return false;
|
||||
const parsed = parseDesignColor(String(raw || '').trim().toLowerCase());
|
||||
if (!parsed) return false;
|
||||
return designSystem.allowedShadowColors.some(entry =>
|
||||
colorsClose(parsed, entry.color) &&
|
||||
Math.abs((parsed.a ?? 1) - (entry.color.a ?? 1)) <= SHADOW_ALPHA_TOLERANCE,
|
||||
);
|
||||
}
|
||||
|
||||
function isAllowedRadiusRaw(raw, designSystem) {
|
||||
if (!designSystem?.hasRadii) return true;
|
||||
const text = String(raw || '').trim().toLowerCase();
|
||||
@@ -691,6 +730,40 @@ function isProbablyColorLiteral(line, match) {
|
||||
return styleContext || cssFunctionContext || jsColorKeyContext;
|
||||
}
|
||||
|
||||
// One complete `${...}` template interpolation. Its content may carry paired
|
||||
// quoted strings (function arguments, ternary branches) and one level of
|
||||
// braces (an object-literal argument, itself allowing paired quotes). Deeper
|
||||
// nesting would need a parser, so the regex deliberately fails safe there:
|
||||
// the context check misses and the finding fires — a false positive a waiver
|
||||
// can silence, never a leak.
|
||||
const QUOTED_STRING_SRC = `"[^"]*"|'[^']*'`;
|
||||
const INTERPOLATION_SRC =
|
||||
`\\$\\{(?:${QUOTED_STRING_SRC}|\\{(?:${QUOTED_STRING_SRC}|[^{}"'\`])*\\}|[^{}"'\`])*\\}`;
|
||||
// The two shadow-context tails. Unlike jsColorKeyContext, the JS tail admits
|
||||
// commas: a multi-layer shadow string is comma-separated, and a later
|
||||
// property on the same line is still blocked because it sits past the
|
||||
// string's closing quote. Both tails admit complete interpolations; a bare
|
||||
// `}`, quote, or `;` still ends the context.
|
||||
const SHADOW_CSS_CONTEXT_RE = new RegExp(
|
||||
`(?:^|[{\\s;"'\`(,])(?:box-shadow|text-shadow)\\s*:\\s*(?:${INTERPOLATION_SRC}|[^;{}"'\`])*$`, 'i',
|
||||
);
|
||||
const SHADOW_JS_CONTEXT_RE = new RegExp(
|
||||
`(?:^|[,{]\\s*)(?:boxShadow|textShadow)\\s*[:=]\\s*["'\`]?(?:${INTERPOLATION_SRC}|[^"'\`}])*$`, 'i',
|
||||
);
|
||||
|
||||
// True when the color literal sits inside a box-shadow / text-shadow value —
|
||||
// the only contexts where a documented shadow color is legal. Anchored to the
|
||||
// end of `before` (no ; } { or quote in between) so a shadow property earlier
|
||||
// on the line can't leak the allowance into a later declaration. Kept separate
|
||||
// from isProbablyColorLiteral(), which stays a boolean for its existing call
|
||||
// sites and deliberately discards which property matched.
|
||||
function isShadowPropertyContext(line, match) {
|
||||
const index = match.index ?? -1;
|
||||
if (index < 0) return false;
|
||||
const before = line.slice(0, index);
|
||||
return SHADOW_CSS_CONTEXT_RE.test(before) || SHADOW_JS_CONTEXT_RE.test(before);
|
||||
}
|
||||
|
||||
function isInsideCssAttributeSelector(line, index) {
|
||||
if (index < 0) return false;
|
||||
const before = line.slice(0, index);
|
||||
@@ -824,6 +897,7 @@ function checkSourceDesignSystem(content, filePath, options = {}) {
|
||||
if (!isProbablyColorLiteral(line, match)) continue;
|
||||
const raw = cssColorLabel(match[0]);
|
||||
if (isAllowedColorRaw(raw, designSystem)) continue;
|
||||
if (isShadowPropertyContext(line, match) && isAllowedShadowColorRaw(raw, designSystem)) continue;
|
||||
findings.push(makeDesignFinding(
|
||||
'design-system-color',
|
||||
filePath,
|
||||
@@ -1038,6 +1112,7 @@ export {
|
||||
loadDesignSystemForCwd,
|
||||
isAllowedFont,
|
||||
isAllowedColorRaw,
|
||||
isAllowedShadowColorRaw,
|
||||
isAllowedRadiusRaw,
|
||||
isAllowedFontSizeRaw,
|
||||
checkSourceDesignSystem,
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
checkSourceDesignSystem,
|
||||
collectStaticDesignSystemFindings,
|
||||
isAllowedColorRaw,
|
||||
isAllowedShadowColorRaw,
|
||||
isAllowedFont,
|
||||
isAllowedRadiusRaw,
|
||||
isAllowedFontSizeRaw,
|
||||
@@ -282,6 +283,9 @@ rounded:
|
||||
roundedMeta: {
|
||||
lg: { canonical: '24px' },
|
||||
},
|
||||
shadows: [
|
||||
{ name: 'ambient-low', value: '0 4px 24px rgba(0,0,0,0.12)', purpose: 'Diffuse hover glow.' },
|
||||
],
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -296,6 +300,8 @@ rounded:
|
||||
assert.equal(isAllowedColorRaw('#d55a42', loaded), true);
|
||||
assert.equal(isAllowedRadiusRaw('80px', loaded), true);
|
||||
assert.equal(isAllowedRadiusRaw('24px', loaded), true);
|
||||
assert.equal(isAllowedShadowColorRaw('rgba(0, 0, 0, 0.12)', loaded), true);
|
||||
assert.equal(isAllowedShadowColorRaw('rgba(0, 0, 0, 0.5)', loaded), false);
|
||||
});
|
||||
|
||||
it('unescapes YAML-escaped quotes around multi-word font families (issue #428)', () => {
|
||||
@@ -474,6 +480,131 @@ const badge = { className: "text-[10px]" };
|
||||
});
|
||||
});
|
||||
|
||||
describe('sidecar shadow tokens (issue #547)', () => {
|
||||
// Mirrors the sidecar `extensions.shadows` schema from document.md Step 4b.
|
||||
function shadowDesignSystem() {
|
||||
return normalizeDesignSystem({
|
||||
frontmatter: {
|
||||
colors: { ink: '#241f1a', paper: '#f7f4ee' },
|
||||
},
|
||||
sidecar: {
|
||||
extensions: {
|
||||
shadows: [
|
||||
{
|
||||
name: 'outset',
|
||||
value: 'inset 0 1px 0 oklch(1 0 0 / 0.07), 0 1px 2px oklch(0 0 0 / 0.28), 0 4px 12px oklch(0 0 0 / 0.22)',
|
||||
purpose: 'Default card shadow.',
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
it('matches documented shadow colors on alpha, not just r/g/b', () => {
|
||||
const designSystem = shadowDesignSystem();
|
||||
assert.equal(isAllowedShadowColorRaw('oklch(0 0 0 / 0.28)', designSystem), true);
|
||||
assert.equal(isAllowedShadowColorRaw('rgba(0, 0, 0, 0.28)', designSystem), true);
|
||||
assert.equal(isAllowedShadowColorRaw('oklch(1 0 0 / 0.07)', designSystem), true);
|
||||
// Same black, undocumented alpha: the r/g/b channels alone must not match.
|
||||
assert.equal(isAllowedShadowColorRaw('oklch(0 0 0 / 55%)', designSystem), false);
|
||||
assert.equal(isAllowedShadowColorRaw('#000', designSystem), false);
|
||||
// Shadow tokens must not switch the general color rule's allowlist on.
|
||||
assert.equal(isAllowedColorRaw('oklch(0 0 0 / 0.28)', designSystem), false);
|
||||
});
|
||||
|
||||
it('allows documented shadow colors in shadow contexts only', () => {
|
||||
const designSystem = shadowDesignSystem();
|
||||
const findings = checkSourceDesignSystem(`
|
||||
.a { box-shadow: 0 1px 2px oklch(0 0 0 / 0.28); }
|
||||
.b { box-shadow: 0 20px 50px oklch(0 0 0 / 55%); }
|
||||
.c { background: #000; }
|
||||
.d { background: oklch(0 0 0 / 0.28); }
|
||||
.e { text-shadow: 0 1px 2px oklch(0 0 0 / 0.28); }
|
||||
.f { box-shadow: inset 0 1px 0 oklch(1 0 0 / 0.07), 0 4px 12px oklch(0 0 0 / 0.22); }
|
||||
const card = { boxShadow: "0 1px 2px rgba(0, 0, 0, 0.28)" };
|
||||
const layered = { boxShadow: "0 1px 2px rgba(0, 0, 0, 0.28), 0 4px 12px rgba(0, 0, 0, 0.22)" };
|
||||
const bad = { color: "rgba(0, 0, 0, 0.28)" };
|
||||
const leak = { boxShadow: "0 1px 2px rgba(0, 0, 0, 0.28)", color: "rgba(0, 0, 0, 0.28)" };
|
||||
`, '/tmp/shadows.css', { designSystem });
|
||||
const colors = findings.filter((item) => item.antipattern === 'design-system-color');
|
||||
|
||||
// .a, .e, .f, and both JS boxShadow strings (including the second layer
|
||||
// past the comma) pass; .b (undocumented alpha), .c (forbidden ground),
|
||||
// .d (documented alpha outside a shadow), and both JS color keys still
|
||||
// fire — the `leak` line proves the closing quote stops the shadow
|
||||
// context from reaching a later property. A fix that silences .d has
|
||||
// stopped discriminating between shadow usage and page grounds.
|
||||
assert.deepEqual(
|
||||
colors.map((item) => [item.line, item.ignoreValue]),
|
||||
[
|
||||
[3, 'oklch(0 0 0 / 55%)'],
|
||||
[4, '#000'],
|
||||
[5, 'oklch(0 0 0 / 0.28)'],
|
||||
[10, 'rgba(0, 0, 0, 0.28)'],
|
||||
[11, 'rgba(0, 0, 0, 0.28)'],
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
it('abstains from the color rule entirely when only shadows are documented', () => {
|
||||
// A shadows-only sidecar must not switch hasColors on: with no palette to
|
||||
// measure against, the engine abstains rather than guesses, same as every
|
||||
// other design-system rule.
|
||||
const designSystem = normalizeDesignSystem({
|
||||
sidecar: {
|
||||
extensions: {
|
||||
shadows: [{ name: 'outset', value: '0 1px 2px oklch(0 0 0 / 0.28)' }],
|
||||
},
|
||||
},
|
||||
});
|
||||
assert.equal(designSystem.hasColors, false);
|
||||
const findings = checkSourceDesignSystem(
|
||||
'.c { background: #000; }',
|
||||
'/tmp/shadows-only.css',
|
||||
{ designSystem },
|
||||
);
|
||||
assert.equal(findings.some((item) => item.antipattern === 'design-system-color'), false);
|
||||
});
|
||||
|
||||
it('keeps shadow context across template interpolations', () => {
|
||||
const designSystem = shadowDesignSystem();
|
||||
const findings = checkSourceDesignSystem(`
|
||||
const card = { boxShadow: \`0 \${offset}px 2px rgba(0, 0, 0, 0.28)\` };
|
||||
box-shadow: 0 1px \${blur}px rgba(0, 0, 0, 0.28);
|
||||
const fn = { boxShadow: \`0 \${getShadow('lg')} 2px rgba(0, 0, 0, 0.28)\` };
|
||||
const tern = { boxShadow: \`0 1px \${dark ? "4px" : "2px"} rgba(0, 0, 0, 0.28)\` };
|
||||
box-shadow: 0 \${theme('blur')} rgba(0, 0, 0, 0.28);
|
||||
const nested = { boxShadow: \`0 \${getOffset({ size: 2 })}px 2px rgba(0, 0, 0, 0.28)\` };
|
||||
const nestedQ = { boxShadow: \`0 \${getOffset({ size: 'lg' })}px rgba(0, 0, 0, 0.28)\` };
|
||||
const leak = { boxShadow: \`0 \${offset}px rgba(0, 0, 0, 0.28)\`, color: "rgba(0, 0, 0, 0.28)" };
|
||||
`, '/tmp/interpolated.js', { designSystem });
|
||||
const colors = findings.filter((item) => item.antipattern === 'design-system-color');
|
||||
|
||||
// The documented shadow color passes after a \${...} interpolation in
|
||||
// the JS template literal and the CSS-in-JS line, including
|
||||
// interpolations carrying quoted function arguments, ternary branches,
|
||||
// and one level of object-literal braces; the color key on the leak line
|
||||
// still fires because it sits past the template's closing backtick.
|
||||
assert.deepEqual(
|
||||
colors.map((item) => [item.line, item.ignoreValue]),
|
||||
[[9, 'rgba(0, 0, 0, 0.28)']],
|
||||
);
|
||||
});
|
||||
|
||||
it('does not allow a later declaration to inherit shadow context from earlier on the line', () => {
|
||||
const designSystem = shadowDesignSystem();
|
||||
const findings = checkSourceDesignSystem(
|
||||
'.x { box-shadow: 0 1px 2px oklch(0 0 0 / 0.28); background: oklch(0 0 0 / 0.28); }',
|
||||
'/tmp/one-line.css',
|
||||
{ designSystem },
|
||||
);
|
||||
const colors = findings.filter((item) => item.antipattern === 'design-system-color');
|
||||
assert.equal(colors.length, 1);
|
||||
assert.equal(colors[0].ignoreValue, 'oklch(0 0 0 / 0.28)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('collectStaticDesignSystemFindings()', () => {
|
||||
function makeElement(tagName, { text = '', attrs = {}, style = {}, parentElement = null } = {}) {
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user