diff --git a/src/cli/commands/analyze.ts b/src/cli/commands/analyze.ts index ec478389..4f9fcf96 100644 --- a/src/cli/commands/analyze.ts +++ b/src/cli/commands/analyze.ts @@ -159,7 +159,7 @@ export function registerAnalyze(cli: CAC): void { // JSON output mode — only JSON goes to stdout; exit code still applies if (options.json) { - console.log(JSON.stringify(buildResultJson(file.name, result, scores), null, 2)); + console.log(JSON.stringify(buildResultJson(file.name, result, scores, { fileKey: file.fileKey }), null, 2)); if (scores.overall.grade === "F") { process.exitCode = 1; } diff --git a/src/cli/commands/implement.ts b/src/cli/commands/implement.ts index 995ec700..c3c6199f 100644 --- a/src/cli/commands/implement.ts +++ b/src/cli/commands/implement.ts @@ -68,7 +68,7 @@ export function registerImplement(cli: CAC): void { // 2. Analysis const result = analyzeFile(file); const scores = calculateScores(result); - const resultJson = buildResultJson(file.name, result, scores); + const resultJson = buildResultJson(file.name, result, scores, { fileKey: file.fileKey }); await writeFile(resolve(outputDir, "analysis.json"), JSON.stringify(resultJson, null, 2), "utf-8"); console.log(` analysis.json: ${result.issues.length} issues, grade ${scores.overall.grade}`); diff --git a/src/core/engine/scoring.test.ts b/src/core/engine/scoring.test.ts index 91bd5259..a2e3297f 100644 --- a/src/core/engine/scoring.test.ts +++ b/src/core/engine/scoring.test.ts @@ -355,6 +355,7 @@ describe("buildResultJson", () => { expect(json.nodeCount).toBe(100); expect(json.issueCount).toBe(3); expect(json.version).toBeDefined(); + expect(json.analyzedAt).toBeDefined(); expect(json.scores).toBeDefined(); expect(json.summary).toBeDefined(); expect(typeof json.summary).toBe("string"); @@ -373,4 +374,38 @@ describe("buildResultJson", () => { expect(issuesByRule["no-auto-layout"]).toBe(2); expect(issuesByRule["raw-color"]).toBe(1); }); + + it("includes detailed issues list with severity and node info", () => { + const result = makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking" }), + makeIssue({ ruleId: "raw-color", category: "token", severity: "missing-info" }), + ]); + const scores = calculateScores(result); + const json = buildResultJson("TestFile", result, scores); + const issues = json.issues as Array<{ ruleId: string; severity: string; nodeId: string; nodePath: string; message: string }>; + + expect(issues).toHaveLength(2); + expect(issues[0]).toMatchObject({ + ruleId: "no-auto-layout", + severity: "blocking", + nodeId: expect.any(String), + nodePath: expect.any(String), + message: expect.any(String), + }); + expect(issues[1]).toMatchObject({ + ruleId: "raw-color", + severity: "missing-info", + }); + }); + + it("includes fileKey when provided", () => { + const result = makeResult([]); + const scores = calculateScores(result); + + const withKey = buildResultJson("TestFile", result, scores, { fileKey: "abc123" }); + expect(withKey.fileKey).toBe("abc123"); + + const withoutKey = buildResultJson("TestFile", result, scores); + expect(withoutKey.fileKey).toBeUndefined(); + }); }); diff --git a/src/core/engine/scoring.ts b/src/core/engine/scoring.ts index e6732586..178bc7fa 100644 --- a/src/core/engine/scoring.ts +++ b/src/core/engine/scoring.ts @@ -363,6 +363,7 @@ export function buildResultJson( fileName: string, result: AnalysisResult, scores: ScoreReport, + options?: { fileKey?: string }, ): Record { const issuesByRule: Record = {}; for (const issue of result.issues) { @@ -370,8 +371,18 @@ export function buildResultJson( issuesByRule[id] = (issuesByRule[id] ?? 0) + 1; } + const issues = result.issues.map((issue) => ({ + ruleId: issue.violation.ruleId, + severity: issue.config.severity, + nodeId: issue.violation.nodeId, + nodePath: issue.violation.nodePath, + message: issue.violation.message, + })); + const json: Record = { version: VERSION, + analyzedAt: result.analyzedAt, + ...(options?.fileKey && { fileKey: options.fileKey }), fileName, nodeCount: result.nodeCount, maxDepth: result.maxDepth, @@ -381,6 +392,7 @@ export function buildResultJson( categories: scores.byCategory, }, issuesByRule, + issues, summary: formatScoreSummary(scores), }; diff --git a/src/core/rules/behavior/index.ts b/src/core/rules/behavior/index.ts index b3257de7..d2b0da71 100644 --- a/src/core/rules/behavior/index.ts +++ b/src/core/rules/behavior/index.ts @@ -41,7 +41,7 @@ const textTruncationUnhandledCheck: RuleCheckFn = (node, context) => { ruleId: textTruncationUnhandledDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" may need text truncation handling`, + message: `"${node.name}" has long text (${node.characters!.length} chars) in narrow container (${width}px) — set text truncation (ellipsis) or use HUG sizing`, }; } } @@ -132,7 +132,7 @@ const prototypeLinkInDesignCheck: RuleCheckFn = (node, context) => { ruleId: prototypeLinkInDesignDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" looks interactive but has no prototype interactions defined`, + message: `"${node.name}" looks interactive but has no prototype interactions — add prototype interactions or rename to clarify non-interactive intent`, }; }; @@ -182,7 +182,7 @@ const overflowBehaviorUnknownCheck: RuleCheckFn = (node, context) => { ruleId: overflowBehaviorUnknownDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has children overflowing bounds without explicit clip/scroll behavior — AI must guess overflow handling`, + message: `"${node.name}" has children overflowing bounds without explicit clip/scroll behavior — enable "Clip content" or set explicit scroll behavior`, }; }; @@ -226,7 +226,7 @@ const wrapBehaviorUnknownCheck: RuleCheckFn = (node, context) => { ruleId: wrapBehaviorUnknownDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has ${visibleChildren.length} horizontal children exceeding container width without wrap behavior — AI cannot determine if content should wrap or scroll`, + message: `"${node.name}" has ${visibleChildren.length} horizontal children exceeding container width without wrap behavior — set layoutWrap to WRAP or add horizontal scroll behavior`, }; }; diff --git a/src/core/rules/component/index.ts b/src/core/rules/component/index.ts index 872df6ae..01bcfebf 100644 --- a/src/core/rules/component/index.ts +++ b/src/core/rules/component/index.ts @@ -143,7 +143,7 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { ruleId: missingComponentDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `Component "${matchingComponent.name}" exists — use instances instead of repeated frames (${sameNameFrames.length} found)`, + message: `Component "${matchingComponent.name}" exists — use instances instead of repeated frames (${sameNameFrames.length} found) — replace frames with component instances`, }; } } @@ -160,7 +160,7 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { ruleId: missingComponentDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" appears ${sameNameFrames.length} times — consider making it a component`, + message: `"${node.name}" appears ${sameNameFrames.length} times — extract as a reusable component`, }; } } @@ -221,7 +221,7 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { ruleId: missingComponentDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" and ${count - 1} sibling frame(s) share the same internal structure — consider extracting a component`, + message: `"${node.name}" and ${count - 1} sibling frame(s) share the same internal structure — extract a shared component from the repeated structure`, }; } } @@ -257,7 +257,7 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { ruleId: missingComponentDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${componentName}" instance has style overrides (${overrides.join(", ")}) — use a variant instead of direct style changes`, + message: `"${componentName}" instance has style overrides (${overrides.join(", ")}) — create a new variant for this style combination`, }; } return null; @@ -301,7 +301,7 @@ const detachedInstanceCheck: RuleCheckFn = (node, context) => { ruleId: detachedInstanceDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" may be a detached instance of component "${component.name}"`, + message: `"${node.name}" may be a detached instance of component "${component.name}" — restore as an instance of "${component.name}" or create a new variant`, }; } } @@ -354,7 +354,7 @@ const missingComponentDescriptionCheck: RuleCheckFn = (node, context) => { ruleId: missingComponentDescriptionDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `Component "${componentMeta.name}" has no description. Descriptions help developers understand purpose and usage.`, + message: `Component "${componentMeta.name}" has no description — add usage guidelines in the component's description field`, }; }; @@ -401,7 +401,7 @@ const variantStructureMismatchCheck: RuleCheckFn = (node, context) => { ruleId: variantStructureMismatchDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has ${mismatchCount}/${totalVariants} variants with different child structures — AI cannot create a unified component template`, + message: `"${node.name}" has ${mismatchCount}/${totalVariants} variants with different child structures — unify variant structures using visibility toggles for optional elements`, }; }; diff --git a/src/core/rules/component/missing-component.test.ts b/src/core/rules/component/missing-component.test.ts index ba92fa46..72f09056 100644 --- a/src/core/rules/component/missing-component.test.ts +++ b/src/core/rules/component/missing-component.test.ts @@ -297,7 +297,7 @@ describe("missing-component — Stage 3: Structure-based repetition", () => { expect(result!.ruleId).toBe("missing-component"); expect(result!.message).toContain("Card A"); expect(result!.message).toContain("1 sibling frame(s)"); - expect(result!.message).toContain("consider extracting a component"); + expect(result!.message).toContain("extract a shared component"); }); it("only flags first matching sibling", () => { @@ -534,7 +534,7 @@ describe("missing-component — Stage 4: Instance style overrides", () => { expect(result).not.toBeNull(); expect(result!.message).toContain("Button"); expect(result!.message).toContain("fills"); - expect(result!.message).toContain("use a variant"); + expect(result!.message).toContain("create a new variant"); }); it("flags when instance has different cornerRadius from master", () => { diff --git a/src/core/rules/naming/index.ts b/src/core/rules/naming/index.ts index 5ba83083..b2e0c1e7 100644 --- a/src/core/rules/naming/index.ts +++ b/src/core/rules/naming/index.ts @@ -79,7 +79,7 @@ const defaultNameCheck: RuleCheckFn = (node, context) => { ruleId: defaultNameDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" is a default name - provide a meaningful name`, + message: `${node.type} "${node.name}" has a default name — rename to describe its purpose (e.g., "Header", "ProductCard")`, }; }; @@ -116,7 +116,7 @@ const nonSemanticNameCheck: RuleCheckFn = (node, context) => { ruleId: nonSemanticNameDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" is a non-semantic name - describe its purpose`, + message: `${node.type} "${node.name}" is a non-semantic name — rename to describe its role (e.g., "Divider", "Background")`, }; }; @@ -172,7 +172,7 @@ const inconsistentNamingConventionCheck: RuleCheckFn = (node, context) => { ruleId: inconsistentNamingConventionDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses ${nodeConvention} while siblings use ${dominantConvention}`, + message: `"${node.name}" uses ${nodeConvention} while siblings use ${dominantConvention} — rename to match ${dominantConvention} convention`, }; } @@ -207,7 +207,7 @@ const numericSuffixNameCheck: RuleCheckFn = (node, context) => { ruleId: numericSuffixNameDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has a numeric suffix - consider renaming`, + message: `"${node.name}" has a numeric suffix — remove suffix and extract as component, or rename to describe the difference`, }; }; @@ -241,7 +241,7 @@ const tooLongNameCheck: RuleCheckFn = (node, context, options) => { ruleId: tooLongNameDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name.substring(0, 30)}..." is ${node.name.length} chars (max: ${maxLength})`, + message: `"${node.name.substring(0, 30)}..." is ${node.name.length} chars — shorten to under ${maxLength} characters`, }; }; diff --git a/src/core/rules/structure/index.ts b/src/core/rules/structure/index.ts index b91c4f86..986f4d78 100644 --- a/src/core/rules/structure/index.ts +++ b/src/core/rules/structure/index.ts @@ -66,7 +66,7 @@ const noAutoLayoutCheck: RuleCheckFn = (node, context) => { ruleId: noAutoLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has overlapping children without Auto Layout — AI cannot determine intended layout`, + message: `"${node.name}" has overlapping children without Auto Layout — apply auto-layout to separate overlapping children`, }; } } @@ -84,7 +84,7 @@ const noAutoLayoutCheck: RuleCheckFn = (node, context) => { ruleId: noAutoLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has nested containers without layout hints — structure is unpredictable for AI`, + message: `"${node.name}" has nested containers without layout hints — apply auto-layout to organize nested containers`, }; } } @@ -93,11 +93,26 @@ const noAutoLayoutCheck: RuleCheckFn = (node, context) => { // Priority 3: Basic no-auto-layout check (FRAME only) if (node.type !== "FRAME") return null; + const childCount = node.children?.length ?? 0; + let directionHint = ""; + if (node.children && node.children.length >= 2) { + const boxes = node.children.filter(c => c.absoluteBoundingBox).map(c => c.absoluteBoundingBox!); + if (boxes.length >= 2) { + const yRange = Math.max(...boxes.map(b => b.y)) - Math.min(...boxes.map(b => b.y)); + const xRange = Math.max(...boxes.map(b => b.x)) - Math.min(...boxes.map(b => b.x)); + directionHint = yRange > xRange ? "VERTICAL" : "HORIZONTAL"; + } + } + + const arrangement = directionHint + ? ` (${childCount} children arranged ${directionHint.toLowerCase()}ly)` + : childCount > 0 ? ` (${childCount} children)` : ""; + return { ruleId: noAutoLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `Frame "${node.name}" has no Auto Layout`, + message: `Frame "${node.name}" has no auto-layout${arrangement}${directionHint ? ` — apply ${directionHint} auto-layout` : " — apply auto-layout"}`, }; }; @@ -155,7 +170,7 @@ const absolutePositionInAutoLayoutCheck: RuleCheckFn = (node, context) => { ruleId: absolutePositionInAutoLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses absolute positioning inside Auto Layout parent "${context.parent.name}". If intentional (badge, overlay, close button), rename to badge-*, overlay-*, close-* to suppress this warning.`, + message: `"${node.name}" uses absolute positioning inside Auto Layout parent "${context.parent.name}" — remove absolute positioning or restructure outside the auto-layout parent`, }; }; @@ -201,7 +216,7 @@ const fixedSizeInAutoLayoutCheck: RuleCheckFn = (node, context) => { ruleId: fixedSizeInAutoLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `Container "${node.name}" (${width}×${height}) uses fixed size on both axes inside Auto Layout. Consider HUG or FILL for at least one axis.`, + message: `Container "${node.name}" (${width}×${height}) uses fixed size on both axes inside auto-layout — set at least one axis to HUG or FILL`, }; } @@ -223,7 +238,7 @@ const fixedSizeInAutoLayoutCheck: RuleCheckFn = (node, context) => { ruleId: fixedSizeInAutoLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has fixed width inside Auto Layout`, + message: `"${node.name}" has fixed width (${width}px) inside auto-layout — set horizontal sizing to FILL`, }; } @@ -276,12 +291,14 @@ const missingSizeConstraintCheck: RuleCheckFn = (node, context) => { const effectiveMissingMin = missingMin && !skipMinCheck; const effectiveMissingMax = missingMax && !skipMaxCheck; + const currentWidth = node.absoluteBoundingBox ? `${node.absoluteBoundingBox.width}px` : "unknown"; + if (effectiveMissingMin && effectiveMissingMax) { return { ruleId: missingSizeConstraintDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses FILL width without min or max width constraints`, + message: `"${node.name}" uses FILL width (currently ${currentWidth}) without min or max constraints — add minWidth and/or maxWidth`, }; } @@ -290,7 +307,7 @@ const missingSizeConstraintCheck: RuleCheckFn = (node, context) => { ruleId: missingSizeConstraintDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses FILL width without a min-width constraint. It may collapse on narrow screens.`, + message: `"${node.name}" uses FILL width (currently ${currentWidth}) without min-width — add minWidth to prevent collapse on narrow screens`, }; } @@ -299,7 +316,7 @@ const missingSizeConstraintCheck: RuleCheckFn = (node, context) => { ruleId: missingSizeConstraintDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses FILL width without a max-width constraint. Content may stretch too wide on large screens.`, + message: `"${node.name}" uses FILL width (currently ${currentWidth}) without max-width — add maxWidth to prevent stretching on large screens`, }; } @@ -337,7 +354,7 @@ const missingResponsiveBehaviorCheck: RuleCheckFn = (node, context) => { ruleId: missingResponsiveBehaviorDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has no responsive behavior configured`, + message: `"${node.name}" has no responsive behavior configured — apply auto-layout or set constraints`, }; } @@ -369,7 +386,7 @@ const groupUsageCheck: RuleCheckFn = (node, context) => { ruleId: groupUsageDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" is a Group - consider converting to Frame with Auto Layout`, + message: `"${node.name}" is a Group — convert to Frame and apply auto-layout`, }; }; @@ -401,7 +418,7 @@ const deepNestingCheck: RuleCheckFn = (node, context, options) => { ruleId: deepNestingDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" is nested ${context.componentDepth} levels deep within its component (max: ${maxDepth})`, + message: `"${node.name}" is nested ${context.componentDepth} levels deep within its component (max: ${maxDepth}) — extract into a sub-component to reduce depth`, }; }; @@ -468,7 +485,7 @@ const zIndexDependentLayoutCheck: RuleCheckFn = (node, context) => { ruleId: zIndexDependentLayoutDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses layer stacking for layout (${significantOverlapCount} overlaps)`, + message: `"${node.name}" uses layer stacking for layout (${significantOverlapCount} overlaps) — restructure using auto-layout to express relationships explicitly`, }; } @@ -537,7 +554,7 @@ const unnecessaryNodeCheck: RuleCheckFn = (node, context) => { ruleId: unnecessaryNodeDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" is an empty frame`, + message: `"${node.name}" is an empty frame${node.absoluteBoundingBox ? ` (${node.absoluteBoundingBox.width}×${node.absoluteBoundingBox.height})` : ""} — remove or replace with auto-layout spacing`, }; } diff --git a/src/core/rules/structure/no-auto-layout.test.ts b/src/core/rules/structure/no-auto-layout.test.ts index e81747b2..128e60ff 100644 --- a/src/core/rules/structure/no-auto-layout.test.ts +++ b/src/core/rules/structure/no-auto-layout.test.ts @@ -46,7 +46,7 @@ describe("no-auto-layout", () => { expect(result).not.toBeNull(); expect(result!.ruleId).toBe("no-auto-layout"); expect(result!.message).toContain("Container"); - expect(result!.message).toContain("no Auto Layout"); + expect(result!.message).toContain("no auto-layout"); }); it("flags frame with layoutMode NONE that has children", () => { diff --git a/src/core/rules/token/index.ts b/src/core/rules/token/index.ts index ac3cb7d8..355d7f61 100644 --- a/src/core/rules/token/index.ts +++ b/src/core/rules/token/index.ts @@ -47,11 +47,13 @@ const rawColorCheck: RuleCheckFn = (node, context) => { for (const fill of node.fills) { const fillObj = fill as Record; if (fillObj["type"] === "SOLID" && fillObj["color"]) { + const c = fillObj["color"] as Record; + const hex = `#${Math.round((c["r"] ?? 0) * 255).toString(16).padStart(2, "0")}${Math.round((c["g"] ?? 0) * 255).toString(16).padStart(2, "0")}${Math.round((c["b"] ?? 0) * 255).toString(16).padStart(2, "0")}`.toUpperCase(); return { ruleId: rawColorDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses raw color without style or variable`, + message: `"${node.name}" uses raw fill color ${hex} without style or variable — bind to a color variable`, }; } } @@ -88,11 +90,20 @@ const rawFontCheck: RuleCheckFn = (node, context) => { return null; } + const fontParts: string[] = []; + const s = node.style; + if (s) { + if (s["fontFamily"]) fontParts.push(String(s["fontFamily"])); + if (s["fontSize"]) fontParts.push(`${s["fontSize"]}px`); + if (s["fontWeight"]) fontParts.push(String(s["fontWeight"])); + } + const fontDesc = fontParts.length > 0 ? ` (${fontParts.join(" ")})` : ""; + return { ruleId: rawFontDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has no text style applied`, + message: `"${node.name}" uses raw font${fontDesc} without text style — apply a text style`, }; }; @@ -115,7 +126,8 @@ const inconsistentSpacingDef: RuleDefinition = { }; const inconsistentSpacingCheck: RuleCheckFn = (node, context, options) => { - const gridBase = (options?.["gridBase"] as number) ?? getRuleOption("inconsistent-spacing", "gridBase", 4); + const configuredGridBase = (options?.["gridBase"] as number) ?? getRuleOption("inconsistent-spacing", "gridBase", 4); + const gridBase = Number.isFinite(configuredGridBase) && configuredGridBase > 0 ? configuredGridBase : 4; // Check padding values const paddings = [ @@ -131,7 +143,7 @@ const inconsistentSpacingCheck: RuleCheckFn = (node, context, options) => { ruleId: inconsistentSpacingDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has padding ${padding}px not on ${gridBase}pt grid`, + message: `"${node.name}" has padding ${padding}px not on ${gridBase}pt grid — round to nearest ${gridBase}pt multiple (${Math.round(padding / gridBase) * gridBase}px)`, }; } } @@ -143,7 +155,7 @@ const inconsistentSpacingCheck: RuleCheckFn = (node, context, options) => { ruleId: inconsistentSpacingDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has item spacing ${node.itemSpacing}px not on ${gridBase}pt grid`, + message: `"${node.name}" has item spacing ${node.itemSpacing}px not on ${gridBase}pt grid — round to nearest ${gridBase}pt multiple (${Math.round(node.itemSpacing / gridBase) * gridBase}px)`, }; } } @@ -170,7 +182,8 @@ const magicNumberSpacingDef: RuleDefinition = { }; const magicNumberSpacingCheck: RuleCheckFn = (node, context, options) => { - const gridBase = (options?.["gridBase"] as number) ?? getRuleOption("magic-number-spacing", "gridBase", 4); + const configuredGridBase = (options?.["gridBase"] as number) ?? getRuleOption("magic-number-spacing", "gridBase", 4); + const gridBase = Number.isFinite(configuredGridBase) && configuredGridBase > 0 ? configuredGridBase : 4; // Similar to inconsistent-spacing but focuses on finding "magic" numbers // Magic numbers are often odd values like 13, 17, 23, etc. @@ -192,7 +205,7 @@ const magicNumberSpacingCheck: RuleCheckFn = (node, context, options) => { ruleId: magicNumberSpacingDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses magic number spacing: ${spacing}px`, + message: `"${node.name}" uses magic number spacing: ${spacing}px — round to ${Math.round(spacing / gridBase) * gridBase}px (nearest ${gridBase}pt grid value)`, }; } } @@ -233,11 +246,18 @@ const rawShadowCheck: RuleCheckFn = (node, context) => { effectObj["type"] === "DROP_SHADOW" || effectObj["type"] === "INNER_SHADOW" ) { + const shadowType = effectObj["type"] === "DROP_SHADOW" ? "drop shadow" : "inner shadow"; + const offset = effectObj["offset"] as Record | undefined; + const radius = effectObj["radius"] as number | undefined; + const detailParts: string[] = []; + if (offset) detailParts.push(`offset ${Math.round(offset["x"] ?? 0)},${Math.round(offset["y"] ?? 0)}`); + if (radius !== undefined) detailParts.push(`blur ${Math.round(radius)}`); + const details = detailParts.length > 0 ? ` (${detailParts.join(" ")})` : ""; return { ruleId: rawShadowDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has shadow effect without effect style`, + message: `"${node.name}" has ${shadowType}${details} without effect style — apply an effect style`, }; } } @@ -274,7 +294,7 @@ const rawOpacityCheck: RuleCheckFn = (node, context) => { ruleId: rawOpacityDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" uses raw opacity (${Math.round(node.opacity * 100)}%) without a variable binding`, + message: `"${node.name}" uses raw opacity (${Math.round(node.opacity * 100)}%) without a variable binding — bind opacity to a variable`, }; }; @@ -338,11 +358,13 @@ const multipleFillColorsCheck: RuleCheckFn = (node, context, options) => { const dist = colorDistance(myColor, sibColor); // Flag if colors are similar but not identical (distance > 0 but within tolerance) if (dist > 0 && dist <= tolerance) { + const myHex = `#${myColor[0].toString(16).padStart(2, "0")}${myColor[1].toString(16).padStart(2, "0")}${myColor[2].toString(16).padStart(2, "0")}`.toUpperCase(); + const sibHex = `#${sibColor[0].toString(16).padStart(2, "0")}${sibColor[1].toString(16).padStart(2, "0")}${sibColor[2].toString(16).padStart(2, "0")}`.toUpperCase(); return { ruleId: multipleFillColorsDef.id, nodeId: node.id, nodePath: context.path.join(" > "), - message: `"${node.name}" has a near-duplicate fill color compared to sibling "${sibling.name}" (distance: ${Math.round(dist)})`, + message: `"${node.name}" (${myHex}) has near-duplicate fill compared to sibling "${sibling.name}" (${sibHex}) — consolidate to a single color token`, }; } } diff --git a/src/core/rules/token/raw-color.test.ts b/src/core/rules/token/raw-color.test.ts index f67c9b3a..37cec80f 100644 --- a/src/core/rules/token/raw-color.test.ts +++ b/src/core/rules/token/raw-color.test.ts @@ -51,7 +51,7 @@ describe("raw-color", () => { expect(result).not.toBeNull(); expect(result!.ruleId).toBe("raw-color"); expect(result!.message).toContain("RawBox"); - expect(result!.message).toContain("raw color"); + expect(result!.message).toContain("raw fill color"); }); it("returns null for gradient fills (non-SOLID)", () => { diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 81b49f35..bb42b053 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -157,7 +157,7 @@ IMPORTANT — Before calling this tool, check which data source is available: content: [ { type: "text" as const, - text: JSON.stringify(buildResultJson(file.name, result, scores), null, 2), + text: JSON.stringify(buildResultJson(file.name, result, scores, { fileKey: file.fileKey }), null, 2), }, ], };