From f9dbecc8024482ca95297509c71ef312d3eaed71 Mon Sep 17 00:00:00 2001 From: Arshdeep singh Date: Sun, 28 Jun 2026 17:21:15 +0530 Subject: [PATCH] feat(linter): detect token name collisions Adds duplicate and collision detection to the model builder for nested and flat token keys, preventing silent overwrites in the symbol table. --- packages/cli/src/linter/model/handler.test.ts | 90 ++++++++++++++ packages/cli/src/linter/model/handler.ts | 117 +++++++++++------- 2 files changed, 159 insertions(+), 48 deletions(-) diff --git a/packages/cli/src/linter/model/handler.test.ts b/packages/cli/src/linter/model/handler.test.ts index 191e31ae..8ef3f0d0 100644 --- a/packages/cli/src/linter/model/handler.test.ts +++ b/packages/cli/src/linter/model/handler.test.ts @@ -131,6 +131,96 @@ describe('ModelHandler', () => { expect(result.designSystem.symbolTable.has('colors.theme.surface.background.base')).toBe(true); }); + it('emits diagnostic for duplicate token path in colors', () => { + const result = handler.execute(makeParsed({ + colors: { + 'utility-info': { + '50': '#111111', + }, + 'utility-info.50': '#222222', + }, + })); + const errors = result.findings.filter(f => f.severity === 'error'); + expect(errors.length).toBe(1); + expect(errors[0]!.path).toBe('colors.utility-info.50'); + expect(errors[0]!.message).toBe("Duplicate token path 'colors.utility-info.50' detected."); + }); + + it('emits diagnostic when grouped color token flattens to an existing token name', () => { + const result = handler.execute(makeParsed({ + colors: { + 'utility-info-50': '#111111', + 'utility-info': { + '50': '#222222', + } + }, + })); + const errors = result.findings.filter(f => f.severity === 'error'); + expect(errors.length).toBe(1); + expect(errors[0]!.path).toBe('colors.utility-info.50'); + expect(errors[0]!.message).toBe("Grouped colors token flattens to 'utility-info-50', which is already defined."); + }); + + it('emits diagnostic for duplicate token path in rounded', () => { + const result = handler.execute(makeParsed({ + rounded: { + 'button': { + 'lg': '8px', + }, + 'button.lg': '12px', + }, + })); + const errors = result.findings.filter(f => f.severity === 'error'); + expect(errors.length).toBe(1); + expect(errors[0]!.path).toBe('rounded.button.lg'); + expect(errors[0]!.message).toBe("Duplicate token path 'rounded.button.lg' detected."); + }); + + it('emits diagnostic when grouped rounded token flattens to an existing token name', () => { + const result = handler.execute(makeParsed({ + rounded: { + 'button-lg': '8px', + 'button': { + 'lg': '12px', + } + }, + })); + const errors = result.findings.filter(f => f.severity === 'error'); + expect(errors.length).toBe(1); + expect(errors[0]!.path).toBe('rounded.button.lg'); + expect(errors[0]!.message).toBe("Grouped rounded token flattens to 'button-lg', which is already defined."); + }); + + it('emits diagnostic for duplicate token path in spacing', () => { + const result = handler.execute(makeParsed({ + spacing: { + 'gutter': { + 's': '8px', + }, + 'gutter.s': '12px', + }, + })); + const errors = result.findings.filter(f => f.severity === 'error'); + expect(errors.length).toBe(1); + expect(errors[0]!.path).toBe('spacing.gutter.s'); + expect(errors[0]!.message).toBe("Duplicate token path 'spacing.gutter.s' detected."); + }); + + it('emits diagnostic when grouped spacing token flattens to an existing token name', () => { + const result = handler.execute(makeParsed({ + spacing: { + 'gutter-s': '8px', + 'gutter': { + 's': '12px', + } + }, + })); + const errors = result.findings.filter(f => f.severity === 'error'); + expect(errors.length).toBe(1); + expect(errors[0]!.path).toBe('spacing.gutter.s'); + expect(errors[0]!.message).toBe("Grouped spacing token flattens to 'gutter-s', which is already defined."); + }); + it('resolves standard CSS named colors and converts them to hex/sRGB', () => { const result = handler.execute(makeParsed({ colors: { c1: 'red', c2: 'transparent', c3: 'aliceblue' }, diff --git a/packages/cli/src/linter/model/handler.ts b/packages/cli/src/linter/model/handler.ts index bee269ff..053fa614 100644 --- a/packages/cli/src/linter/model/handler.ts +++ b/packages/cli/src/linter/model/handler.ts @@ -54,7 +54,10 @@ export class ModelHandler implements ModelSpec { // ── Phase 1: Resolve primitive tokens ────────────────────────── // Colors if (input.colors) { + const isCollision = buildCollisionGuard('colors', findings); forEachLeaf(input.colors, (name, raw) => { + if (isCollision(name)) return; + if (typeof raw === 'string' && isTokenReference(raw)) { // Store raw reference for later resolution symbolTable.set(`colors.${name}`, raw); @@ -85,7 +88,10 @@ export class ModelHandler implements ModelSpec { // Rounded if (input.rounded) { + const isCollision = buildCollisionGuard('rounded', findings); forEachLeaf(input.rounded, (name, raw) => { + if (isCollision(name)) return; + if (typeof raw === 'string') { if (isParseableDimension(raw)) { const resolved = parseDimension(raw); @@ -114,7 +120,10 @@ export class ModelHandler implements ModelSpec { // Spacing if (input.spacing) { + const isCollision = buildCollisionGuard('spacing', findings); forEachLeaf(input.spacing, (name, raw) => { + if (isCollision(name)) return; + if (isParseableDimension(raw)) { const resolved = parseDimension(raw); spacing.set(name, resolved); @@ -125,54 +134,27 @@ export class ModelHandler implements ModelSpec { }, '', 0, findings, 'spacing'); } - // ── Phase 2: Resolve chained color references ────────────────── - // Iterate color entries that are still raw references and resolve them - if (input.colors) { - forEachLeaf(input.colors, (name, raw) => { - if (typeof raw === 'string' && isTokenReference(raw)) { - const resolved = resolveReference(symbolTable, raw.slice(1, -1), new Set()); - if (resolved !== null && typeof resolved === 'object' && 'type' in resolved && resolved.type === 'color') { - colors.set(name, resolved as ResolvedColor); - symbolTable.set(`colors.${name}`, resolved); - } - } - }); - } - - // Resolve chained rounded references - if (input.rounded) { - forEachLeaf(input.rounded, (name, raw) => { - if (typeof raw === 'string' && isTokenReference(raw)) { - const resolved = resolveReference(symbolTable, raw.slice(1, -1), new Set()); - if ( - resolved !== null && - typeof resolved === 'object' && - 'type' in resolved && - resolved.type === 'dimension' - ) { - rounded.set(name, resolved as ResolvedDimension); - symbolTable.set(`rounded.${name}`, resolved); - } - } - }); - } - - // Resolve chained spacing references - if (input.spacing) { - forEachLeaf(input.spacing, (name, raw) => { - if (typeof raw === 'string' && isTokenReference(raw)) { - const resolved = resolveReference(symbolTable, raw.slice(1, -1), new Set()); - if ( - resolved !== null && - typeof resolved === 'object' && - 'type' in resolved && - resolved.type === 'dimension' - ) { - spacing.set(name, resolved as ResolvedDimension); - symbolTable.set(`spacing.${name}`, resolved); - } - } - }); + // ── Phase 2: Resolve chained token references ────────────────── + // Iterate the symbol table directly (not re-walking raw input) so that + // Phase 1 collision decisions are never overwritten. + for (const [key, value] of symbolTable) { + if (typeof value !== 'string' || !isTokenReference(value)) continue; + const resolved = resolveReference(symbolTable, value.slice(1, -1), new Set()); + if (resolved === null || typeof resolved !== 'object' || !('type' in resolved)) continue; + + if (key.startsWith('colors.') && resolved.type === 'color') { + const name = key.slice('colors.'.length); + colors.set(name, resolved as ResolvedColor); + symbolTable.set(key, resolved); + } else if (key.startsWith('rounded.') && resolved.type === 'dimension') { + const name = key.slice('rounded.'.length); + rounded.set(name, resolved as ResolvedDimension); + symbolTable.set(key, resolved); + } else if (key.startsWith('spacing.') && resolved.type === 'dimension') { + const name = key.slice('spacing.'.length); + spacing.set(name, resolved as ResolvedDimension); + symbolTable.set(key, resolved); + } } // ── Phase 3: Build components ────────────────────────────────── @@ -265,6 +247,45 @@ export class ModelHandler implements ModelSpec { // ── Pure utility functions ───────────────────────────────────────── +/** + * Returns a predicate that detects token name collisions within a single + * token category (colors, rounded, spacing). Call once per category; the + * returned function tracks state via closure. + * + * Returns true (and pushes a finding) when the candidate name collides with + * an already-registered key, so callers can skip it with a simple `if + * (isCollision(name)) return;`. + */ +function buildCollisionGuard( + category: string, + findings: Finding[], +): (name: string) => boolean { + const seenKeys = new Set(); + const seenNormalized = new Map(); + return (name: string): boolean => { + const normalized = name.replace(/\./g, '-'); + if (seenKeys.has(name)) { + findings.push({ + severity: 'error', + path: `${category}.${name}`, + message: `Duplicate token path '${category}.${name}' detected.`, + }); + return true; + } + if (seenNormalized.has(normalized)) { + findings.push({ + severity: 'error', + path: `${category}.${name}`, + message: `Grouped ${category} token flattens to '${normalized}', which is already defined.`, + }); + return true; + } + seenKeys.add(name); + seenNormalized.set(normalized, name); + return false; + }; +} + /** * Parse a CSS color string into a ResolvedColor with RGB + WCAG luminance. */