Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,11 @@ Override score, severity, or enable/disable individual rules:
| `missing-interaction-state` | -3 | missing-info |
| `missing-prototype` | -3 | missing-info |

**Minor (3 rules)** — naming issues with negligible impact (ΔV < 2%)
**Minor (4 rules)** — naming issues with negligible impact (ΔV < 2%)

| Rule ID | Default Score | Default Severity |
|---------|--------------|-----------------|
| `non-standard-naming` | -1 | suggestion |
| `default-name` | -1 | suggestion |
| `non-semantic-name` | -1 | suggestion |
| `inconsistent-naming-convention` | -1 | suggestion |
Expand Down
1 change: 1 addition & 0 deletions src/core/contracts/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export type RuleId =
| "missing-interaction-state"
| "missing-prototype"
// Minor — naming issues with negligible impact (ΔV < 2%)
| "non-standard-naming"
| "default-name"
| "non-semantic-name"
| "inconsistent-naming-convention";
Expand Down
2 changes: 1 addition & 1 deletion src/core/engine/scoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ describe("calculateGrade (via calculateScores)", () => {
"code-quality": ["missing-component", "detached-instance", "variant-structure-mismatch", "deep-nesting"],
"token-management": ["raw-value", "irregular-spacing"],
"interaction": ["missing-interaction-state", "missing-prototype"],
"minor": ["default-name", "non-semantic-name", "inconsistent-naming-convention"],
"minor": ["non-standard-naming", "default-name", "non-semantic-name", "inconsistent-naming-convention"],
};

for (const cat of categories) {
Expand Down
59 changes: 57 additions & 2 deletions src/core/rules/naming/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { RuleCheckFn, RuleDefinition } from "../../contracts/rule.js";
import { defineRule } from "../rule-registry.js";
import { defaultNameMsg, getDefaultNameSubType, nonSemanticNameMsg, inconsistentNamingMsg } from "../rule-messages.js";
import { isExcludedName, isDefaultName, isNonSemanticName } from "../node-semantics.js";
import { defaultNameMsg, getDefaultNameSubType, nonSemanticNameMsg, inconsistentNamingMsg, nonStandardNamingMsg } from "../rule-messages.js";
import { isExcludedName, isDefaultName, isNonSemanticName, STANDARD_STATE_NAMES, STATE_NAME_SUGGESTIONS, STATE_LIKE_PATTERN } from "../node-semantics.js";

function detectNamingConvention(name: string): string | null {
if (/^[a-z]+(-[a-z]+)*$/.test(name)) return "kebab-case";
Expand Down Expand Up @@ -141,3 +141,58 @@ export const inconsistentNamingConvention = defineRule({
check: inconsistentNamingConventionCheck,
});

// ============================================
// non-standard-naming
// ============================================

const nonStandardNamingDef: RuleDefinition = {
id: "non-standard-naming",
name: "Non-Standard Naming",
category: "minor",
why: "Non-standard state names prevent interaction rules from detecting state variants — AI cannot generate correct :hover/:active/:disabled styles",
impact: "Interaction state detection fails, resulting in static UI with no state transitions",
fix: "Use platform-standard state names: hover, active, pressed, selected, highlighted, disabled, focus, focused",
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const nonStandardNamingCheck: RuleCheckFn = (node, context) => {
// Only check COMPONENT_SET (variant container)
if (node.type !== "COMPONENT_SET") return null;
if (!node.componentPropertyDefinitions) return null;

for (const prop of Object.values(node.componentPropertyDefinitions)) {
const p = prop as Record<string, unknown>;
if (p["type"] !== "VARIANT") continue;
const options = p["variantOptions"];
if (!Array.isArray(options)) continue;

for (const opt of options) {
if (typeof opt !== "string") continue;
const lower = opt.toLowerCase().trim();

// Skip if it's a standard name
if (STANDARD_STATE_NAMES.has(lower)) continue;

// Check if it matches a known non-standard state name
if (STATE_LIKE_PATTERN.test(opt)) {
const suggestion = STATE_NAME_SUGGESTIONS[lower];
if (suggestion) {
return {
ruleId: nonStandardNamingDef.id,
subType: "state-name" as const,
nodeId: node.id,
nodePath: context.path.join(" > "),
message: nonStandardNamingMsg.stateName(node.name, opt, suggestion),
};
}
}
}
}

return null;
};
Comment on lines +157 to +192

@coderabbitai coderabbitai Bot Mar 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard against undefined node.name in message construction.

node.name is used at line 184 without a guard. While COMPONENT_SET nodes typically have names, the AnalysisNode type allows name to be undefined. Add an early return or provide a fallback.

Suggested fix
 const nonStandardNamingCheck: RuleCheckFn = (node, context) => {
   // Only check COMPONENT_SET (variant container)
   if (node.type !== "COMPONENT_SET") return null;
+  if (!node.name) return null;
   if (!node.componentPropertyDefinitions) return null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/rules/naming/index.ts` around lines 157 - 192, The rule
nonStandardNamingCheck uses node.name when building the message (via
nonStandardNamingMsg.stateName) but AnalysisNode.name may be undefined; update
nonStandardNamingCheck to guard for missing names by returning null early if
node.name is falsy or by supplying a safe fallback (e.g., "<unnamed>" or
node.id) when calling nonStandardNamingMsg.stateName so the message construction
never receives undefined; ensure the change is applied where the object with
ruleId nonStandardNamingDef.id and subType "state-name" is created.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping as-is. AnalysisNode.name is z.string() (required) in the Zod schema at src/core/contracts/figma-node.ts:75. It cannot be undefined at runtime.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify AnalysisNode.name field definition in the Zod schema
rg -n "name" src/core/contracts/figma-node.ts -C2

Length of output: 554


@let-sunny, confirmed — Line 75 of src/core/contracts/figma-node.ts shows name: z.string() with no .optional() or .nullable(), so node.name is always a defined string by the time any rule check function is invoked. My comment was a false positive. Thanks for the clarification!


✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 162
File: src/core/rules/naming/index.ts:157-192
Timestamp: 2026-03-29T03:02:35.383Z
Learning: In let-sunny/canicode, `AnalysisNode.name` is declared as `name: z.string()` (required, non-optional) at line 75 of `src/core/contracts/figma-node.ts`. It is guaranteed to be a defined string at runtime for all nodes reaching rule check functions. Do not flag `node.name` usage in rule files as potentially undefined.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 158
File: src/core/rules/interaction/index.ts:64-75
Timestamp: 2026-03-29T01:43:45.901Z
Learning: In let-sunny/canicode, `hasStateInVariantProps()` in `src/core/rules/interaction/index.ts` intentionally scans ALL `VARIANT`-typed `componentPropertyDefinitions` (not just ones keyed as `"State"`) because Figma variant axis names are freeform — designers use `State`, `Status`, `Mode`, `Type`, or localized names. If any variant option value matches the state regex pattern, the state is considered present. Do not flag this as a false-positive risk.

Learnt from: let-sunny
Repo: let-sunny/canicode PR: 161
File: src/core/rules/node-semantics.ts:157-160
Timestamp: 2026-03-29T02:11:22.940Z
Learning: In let-sunny/canicode, `isExcludedName()` from `src/core/rules/node-semantics.ts` is intentionally NOT used in interaction rules. Interaction detection uses `getInteractiveType()` (via `INTERACTIVE_PATTERNS`) as an independent classification path. `isExcludedName` only affects: naming rules (`src/core/rules/naming/index.ts`), auto-layout exemptions (`src/core/rules/rule-exceptions.ts`), and orchestrator-level node filtering (`src/agents/orchestrator.ts`). Do not flag `cta`, `filter`, or other terms in `EXCLUDED_NAME_PATTERN` as suppressing interactive control analysis.

Learnt from: let-sunny
Repo: let-sunny/canicode PR: 161
File: src/core/rules/node-semantics.ts:102-104
Timestamp: 2026-03-29T02:24:54.672Z
Learning: In let-sunny/canicode (`src/core/rules/node-semantics.ts`), `isInteractiveNode()` is intentionally narrow: it returns `true` only for state-variant interactive elements (button, link, tab, input, toggle) detected via `getInteractiveType()`. Overlay and carousel nodes are a separate semantic category called "behavior triggers" — they require prototype interactions but do NOT have hover/focus/disabled state variants. Do not suggest broadening `isInteractiveNode` to include overlay/carousel; the three categories (state-variant interactive, overlay, carousel) are siblings, not a hierarchy.

Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Constants must use SCREAMING_SNAKE_CASE naming convention

Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/token/index.ts:36-135
Timestamp: 2026-03-29T00:24:15.976Z
Learning: In let-sunny/canicode rule implementations (e.g., RuleCheckFn in src/core/rules/token/index.ts and other rule files), follow the engine contract: `RuleCheckFn` must return `RuleViolation | null`, meaning the engine supports only a single violation per node per rule. If `rawValueCheck` (or similar logic) returns as soon as it finds the first matching subtype, treat that as intentional and do not change it to accumulate multiple violations unless the engine contract is updated (tracked in issue `#155`).

Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/structure/index.ts:296-304
Timestamp: 2026-03-29T00:24:13.455Z
Learning: In let-sunny/canicode, the `non-layout-container` rule in `src/core/rules/structure/index.ts` intentionally flags non-empty `SECTION` nodes (`children.length > 0`) because Figma `SECTION` is not a layout container by design — using it structurally with children is treated as semantic misuse. This predicate is considered sufficient and intentional; do not flag it as too broad.

Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/rule-config.ts : `no-auto-layout` is the single highest-impact rule with score -10 — empirically validated via ablation experiments

Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to **/*.{ts,tsx} : Types and Interfaces must use PascalCase naming convention

Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/cli/**/*.ts : If no `node-id` is provided to CLI, print a warning

Learnt from: let-sunny
Repo: let-sunny/canicode PR: 93
File: src/core/rules/rule-config.ts:76-80
Timestamp: 2026-03-26T01:28:57.785Z
Learning: In the let-sunny/canicode repo, `src/core/rules/rule-config.ts` is automatically adjusted by a nightly calibration pipeline. Do NOT suggest adding inline comments to this file for calibration rationale — the change evidence is tracked in PR descriptions, commit messages, and `data/calibration-evidence.json` instead. Inline comments would create clutter as the file is frequently auto-modified.

Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/**/*.ts : All rule scores, severity, and thresholds are managed in `rules/rule-config.ts` — rule logic and score config must be intentionally separated

Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/rule-config.ts : Rule scores can be tuned in `rule-config.ts` without touching rule logic


export const nonStandardNaming = defineRule({
definition: nonStandardNamingDef,
check: nonStandardNamingCheck,
});

94 changes: 94 additions & 0 deletions src/core/rules/naming/non-standard-naming.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { makeNode, makeFile, makeContext } from "../test-helpers.js";
import { nonStandardNaming } from "./index.js";

describe("non-standard-naming", () => {
it("has correct rule definition metadata", () => {
const def = nonStandardNaming.definition;
expect(def.id).toBe("non-standard-naming");
expect(def.category).toBe("minor");
});

it("flags non-standard state name 'Clicked'", () => {
const node = makeNode({
id: "1:1",
name: "Button",
type: "COMPONENT_SET",
componentPropertyDefinitions: {
"State": { type: "VARIANT", variantOptions: ["Default", "Clicked", "Disabled"] },
},
});
const ctx = makeContext();
const result = nonStandardNaming.check(node, ctx);

expect(result).not.toBeNull();
expect(result!.subType).toBe("state-name");
expect(result!.message).toContain("Clicked");
expect(result!.message).toContain("pressed");
});

it("flags 'Inactive' → suggests 'disabled'", () => {
const node = makeNode({
id: "1:1",
name: "Toggle",
type: "COMPONENT_SET",
componentPropertyDefinitions: {
"State": { type: "VARIANT", variantOptions: ["On", "Inactive"] },
},
});
const ctx = makeContext();
const result = nonStandardNaming.check(node, ctx);

expect(result).not.toBeNull();
expect(result!.message).toContain("On");
expect(result!.message).toContain("active");
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

it("passes when all state names are standard", () => {
const node = makeNode({
id: "1:1",
name: "Button",
type: "COMPONENT_SET",
componentPropertyDefinitions: {
"State": { type: "VARIANT", variantOptions: ["Default", "Hover", "Pressed", "Disabled"] },
},
});
const ctx = makeContext();
expect(nonStandardNaming.check(node, ctx)).toBeNull();
});

it("passes for non-state variant properties", () => {
const node = makeNode({
id: "1:1",
name: "Button",
type: "COMPONENT_SET",
componentPropertyDefinitions: {
"Size": { type: "VARIANT", variantOptions: ["Small", "Medium", "Large"] },
},
});
const ctx = makeContext();
expect(nonStandardNaming.check(node, ctx)).toBeNull();
});

it("skips non-COMPONENT_SET nodes", () => {
const node = makeNode({
id: "1:1",
name: "Button",
type: "INSTANCE",
});
const ctx = makeContext();
expect(nonStandardNaming.check(node, ctx)).toBeNull();
});

it("accepts platform-standard names (pressed, selected, highlighted, focused)", () => {
const node = makeNode({
id: "1:1",
name: "Nav Item",
type: "COMPONENT_SET",
componentPropertyDefinitions: {
"State": { type: "VARIANT", variantOptions: ["Default", "Pressed", "Selected", "Highlighted", "Focused"] },
},
});
const ctx = makeContext();
expect(nonStandardNaming.check(node, ctx)).toBeNull();
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
45 changes: 45 additions & 0 deletions src/core/rules/node-semantics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,51 @@ export function isStatefulComponent(node: AnalysisNode): boolean {
return getStatefulComponentType(node) !== null;
}

/**
* Standard state names accepted across web + mobile platforms.
* Used by missing-interaction-state (to detect presence) and
* non-standard-naming (to flag non-standard names).
*/
/**
* Standard state names across web + mobile platforms.
*
* Sources:
* - CSS: :hover, :active, :focus, :disabled, :enabled — https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Selectors/Pseudo-classes
* - Material Design 3: Hovered, Focused, Pressed, Dragged — https://m3.material.io/foundations/interaction/states
* - UIKit: highlighted, selected, disabled, focused — https://developer.apple.com/documentation/uikit/uicontrol/state
*/
Comment thread
coderabbitai[bot] marked this conversation as resolved.
export const STANDARD_STATE_NAMES = new Set([
// CSS pseudo-classes (web)
"default", "hover", "active", "focus", "focused", "disabled", "enabled",
// Material Design 3 (Android)
"pressed", "dragged",
// UIKit (iOS)
"highlighted",
// Common
"selected",
]);

/**
* Patterns that look like state names but aren't in the standard set.
* Maps common non-standard names to their standard equivalent for suggestions.
*/
export const STATE_NAME_SUGGESTIONS: Record<string, string> = {
on: "active",
off: "disabled",
clicked: "pressed",
tapped: "pressed",
inactive: "disabled",
normal: "default",
rest: "default",
hovered: "hover",
activated: "active",
checked: "selected",
unchecked: "default",
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/** Pattern to detect state-like variant option names (broad match) */
export const STATE_LIKE_PATTERN = /\b(on|off|clicked|tapped|inactive|normal|rest|hovered|activated|checked|unchecked)\b/i;

// ── Overlay / Carousel patterns ──────────────────────────────────────────────

/** Elements that open on top of current view */
Expand Down
6 changes: 6 additions & 0 deletions src/core/rules/rule-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const RULE_ID_CATEGORY: Record<RuleId, Category> = {
"missing-interaction-state": "interaction",
"missing-prototype": "interaction",
// Minor
"non-standard-naming": "minor",
"default-name": "minor",
"non-semantic-name": "minor",
"inconsistent-naming-convention": "minor",
Expand Down Expand Up @@ -135,6 +136,11 @@ export const RULE_CONFIGS: Record<RuleId, RuleConfig> = {
},

// ── Minor ──
"non-standard-naming": {
severity: "suggestion",
score: -1,
enabled: true,
},
"default-name": {
severity: "suggestion",
score: -1,
Expand Down
9 changes: 9 additions & 0 deletions src/core/rules/rule-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ export const detachedInstanceMsg = (name: string, componentName: string) =>
export const variantStructureMismatchMsg = (name: string, mismatchCount: number, totalVariants: number) =>
`"${name}" has ${mismatchCount}/${totalVariants} variants with different child structures — unify variant structures using visibility toggles for optional elements`;

// ── non-standard-naming ──────────────────────────────────────────────────────

export type NonStandardNamingSubType = "state-name";

export const nonStandardNamingMsg = {
stateName: (name: string, nonStandard: string, suggestion: string) =>
`"${name}" has non-standard state name "${nonStandard}" — use "${suggestion}" for dev-friendly naming`,
};

// ── default-name ─────────────────────────────────────────────────────────────

export type DefaultNameSubType = "frame" | "group" | "vector" | "shape" | "text" | "image" | "component" | "instance";
Expand Down
Loading