diff --git a/docs/analyze.md b/docs/analyze.md index fdee69f..e85fb0a 100644 --- a/docs/analyze.md +++ b/docs/analyze.md @@ -32,6 +32,9 @@ another package or local module is not treated as an Askr API. - `askr/parse-error` reports malformed source before other results can be considered complete. +- `askr/no-hardcoded-theme-token` reports `--ak-*` token names in runtime + JavaScript and TypeScript string, template, and JSX attribute literals. The + exact `@askrjs/themes` workspace is exempt because it owns those declarations. - `askr/stable-render-call` enforces stable top-level calls for state, derived values, selectors, resources, lifecycle operations, actions, queries, and mutations where the AST establishes a component render context. diff --git a/docs/superpowers/plans/2026-07-30-hardcoded-theme-token-rule.md b/docs/superpowers/plans/2026-07-30-hardcoded-theme-token-rule.md new file mode 100644 index 0000000..cd60ec5 --- /dev/null +++ b/docs/superpowers/plans/2026-07-30-hardcoded-theme-token-rule.md @@ -0,0 +1,333 @@ +# Hardcoded Theme Token Analyzer Rule Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `askr/no-hardcoded-theme-token`, a warning-only analyzer rule that reports literal `--ak-*` references in runtime JS/TS while exempting the exact `@askrjs/themes` workspace. + +**Architecture:** Extend the existing monolithic rule registry with one syntax-only rule. It walks each already-selected TypeScript source file, extracts string and template literal segments, deduplicates nodes, and emits diagnostics through the existing helper; project exclusions remain in source discovery, and the token-owner exemption uses `context.workspace.name`. + +**Tech Stack:** TypeScript 6 compiler API, Vitest, Node.js filesystem fixtures, Vite Plus project scripts. + +--- + +## File Map + +- Modify `src/analyze/rules.ts`: define literal-segment extraction, implement the rule, and register it in `ANALYZE_RULES`. +- Modify `tests/analyze.rules.test.ts`: add focused rule behavior, exclusion, and workspace-identity regression tests. +- Modify `docs/analyze.md`: document the new correctness rule and its exact exemption. + +### Task 1: Detect hardcoded theme-token literals + +**Files:** +- Modify: `tests/analyze.rules.test.ts` in the `describe("analyzer rules")` block +- Modify: `src/analyze/rules.ts` near the shared `visit` helper and before `frameworkConfigRule` + +- [ ] **Step 1: Write the failing literal-detection test** + +Add this test to `tests/analyze.rules.test.ts`: + +```ts +it("reports hardcoded theme tokens in runtime literal segments", async () => { + const root = await fixture({ + "src/tokens.tsx": ` + declare const dynamicToken: string; + declare const suffix: string; + const plain = "var(--ak-color-fg-muted)"; + const staticTemplate = \`--ak-surface-raised\`; + const interpolated = \`var(--ak-\${suffix}) and --ak-border-subtle\`; + const indirect = dynamicToken; + // --ak-comment-only + export const view =
; + `, + }); + + const found = (await diagnostics(root)).filter( + (entry) => entry.ruleId === "askr/no-hardcoded-theme-token", + ); + + expect(found).toHaveLength(5); + expect(found.every((entry) => entry.file === "src/tokens.tsx")).toBe(true); + expect(found.every((entry) => entry.category === "correctness")).toBe(true); + expect(found.every((entry) => entry.severity === "warning")).toBe(true); + expect(found.every((entry) => entry.fix === undefined)).toBe(true); + expect(found.map((entry) => entry.line)).toEqual([4, 5, 6, 6, 9]); +}); +``` + +This covers an ordinary string, a no-substitution template, both static segments +of an interpolated template, a JSX string attribute, and negative cases for a +comment and a non-literal identifier. + +- [ ] **Step 2: Run the focused test and verify it fails** + +Run: + +```bash +rtk npm test -- tests/analyze.rules.test.ts -t "reports hardcoded theme tokens in runtime literal segments" +``` + +Expected: FAIL because `found` has length `0` instead of `5`. + +- [ ] **Step 3: Add the literal-segment extractor and rule** + +Add this helper near the existing `visit` helper in `src/analyze/rules.ts`: + +```ts +type RuntimeLiteralSegment = ts.StringLiteralLike | ts.TemplateLiteralLikeNode; + +function runtimeLiteralSegments(node: ts.Node): readonly RuntimeLiteralSegment[] { + if (ts.isStringLiteralLike(node)) return [node]; + if (ts.isTemplateExpression(node)) { + return [node.head, ...node.templateSpans.map((span) => span.literal)]; + } + return []; +} +``` + +Add this rule before `frameworkConfigRule`: + +```ts +const hardcodedThemeTokenRule: AnalyzeRule = { + id: "askr/no-hardcoded-theme-token", + category: "correctness", + severity: "warning", + description: "Runtime source must not hardcode Askr theme token names.", + analyze(context) { + const diagnostics: AnalyzeDiagnostic[] = []; + const reported = new Set(); + + for (const sourceFile of context.sourceFiles) { + visit(sourceFile, (node) => { + for (const segment of runtimeLiteralSegments(node)) { + if (reported.has(segment) || !segment.text.includes("--ak-")) continue; + reported.add(segment); + diagnostics.push( + diagnostic( + context, + segment, + this, + "Runtime code hardcodes an Askr theme token.", + "Move the token mapping to theme CSS and select it with a semantic class or data-* attribute.", + ), + ); + } + }); + } + + return diagnostics; + }, +}; +``` + +Register it in `ANALYZE_RULES` by inserting this exact entry immediately after +`parseErrorRule` and before `stableRenderRule`: + +```ts + hardcodedThemeTokenRule, +``` + +The `Set` prevents duplicate reports if TypeScript traverses a template segment +both through its `TemplateExpression` and as a child node. + +- [ ] **Step 4: Run the focused test and verify it passes** + +Run: + +```bash +rtk npm test -- tests/analyze.rules.test.ts -t "reports hardcoded theme tokens in runtime literal segments" +``` + +Expected: PASS with five diagnostics from `src/tokens.tsx`. + +- [ ] **Step 5: Run the complete analyzer rule test file** + +Run: + +```bash +rtk npm test -- tests/analyze.rules.test.ts +``` + +Expected: PASS; existing analyzer rules remain unchanged. + +- [ ] **Step 6: Commit the detection behavior** + +```bash +rtk git add src/analyze/rules.ts tests/analyze.rules.test.ts +rtk git commit -m "feat(analyze): report hardcoded theme tokens" +``` + +### Task 2: Respect configured exclusions and the token-owner workspace + +**Files:** +- Modify: `tests/analyze.rules.test.ts` in the `describe("analyzer rules")` block +- Modify: `src/analyze/rules.ts` in `hardcodedThemeTokenRule.analyze` + +- [ ] **Step 1: Write the failing exemption and exclusion test** + +Add this test to `tests/analyze.rules.test.ts`: + +```ts +it("honors theme-token exclusions and exempts only the exact theme owner", async () => { + const excludedRoot = await fixture( + { + "src/app.ts": 'export const token = "--ak-included";', + "vendor/ignored.ts": 'export const token = "--ak-excluded";', + }, + { + manifest: { + name: "fixture", + askr: { analyze: { exclude: ["vendor/**"] } }, + }, + }, + ); + const ownerRoot = await fixture( + { "src/theme.ts": 'export const token = "--ak-owned";' }, + { manifest: { name: "@askrjs/themes" } }, + ); + const lookalikeRoot = await fixture( + { "src/theme.ts": 'export const token = "--ak-not-owned";' }, + { manifest: { name: "@askrjs/themes-preview" } }, + ); + + const ruleFindings = async (root: string) => + (await diagnostics(root)).filter( + (entry) => entry.ruleId === "askr/no-hardcoded-theme-token", + ); + + await expect(ruleFindings(excludedRoot)).resolves.toEqual([ + expect.objectContaining({ file: "src/app.ts" }), + ]); + await expect(ruleFindings(ownerRoot)).resolves.toEqual([]); + await expect(ruleFindings(lookalikeRoot)).resolves.toEqual([ + expect.objectContaining({ file: "src/theme.ts" }), + ]); +}); +``` + +- [ ] **Step 2: Run the focused test and verify the exact-owner assertion fails** + +Run: + +```bash +rtk npm test -- tests/analyze.rules.test.ts -t "honors theme-token exclusions and exempts only the exact theme owner" +``` + +Expected: FAIL because `@askrjs/themes` still receives one diagnostic. The +configured exclusion assertion should already pass because source discovery +omits `vendor/ignored.ts`. + +- [ ] **Step 3: Add the exact workspace-name exemption** + +At the start of `hardcodedThemeTokenRule.analyze`, add: + +```ts +analyze(context) { + if (context.workspace.name === "@askrjs/themes") return []; + + const diagnostics: AnalyzeDiagnostic[] = []; + // existing literal walk remains unchanged +``` + +Do not use substring, path, dependency, or scope matching; only the exact +manifest-derived workspace name owns the token namespace. + +- [ ] **Step 4: Run the focused test and verify it passes** + +Run: + +```bash +rtk npm test -- tests/analyze.rules.test.ts -t "honors theme-token exclusions and exempts only the exact theme owner" +``` + +Expected: PASS: one included-file diagnostic, no owner diagnostic, and one +lookalike-package diagnostic. + +- [ ] **Step 5: Run all analyzer tests** + +Run: + +```bash +rtk npm test -- tests/analyze.rules.test.ts tests/analyze.cli.test.ts +``` + +Expected: PASS. + +- [ ] **Step 6: Commit the exemption behavior** + +```bash +rtk git add src/analyze/rules.ts tests/analyze.rules.test.ts +rtk git commit -m "test(analyze): cover theme token rule boundaries" +``` + +### Task 3: Document and verify the complete rule + +**Files:** +- Modify: `docs/analyze.md` under `### Correctness` + +- [ ] **Step 1: Add the rule to the public analyzer catalog** + +Insert this bullet after `askr/parse-error` in `docs/analyze.md`: + +```markdown +- `askr/no-hardcoded-theme-token` reports `--ak-*` token names in runtime + JavaScript and TypeScript string, template, and JSX attribute literals. The + exact `@askrjs/themes` workspace is exempt because it owns those declarations. +``` + +- [ ] **Step 2: Run formatting and inspect any mechanical changes** + +Run: + +```bash +rtk npm run fmt +rtk git diff --check +rtk git status --short +``` + +Expected: formatting succeeds, `git diff --check` prints nothing, and only the +intended rule, tests, documentation, and plan/spec files differ from `main`. + +- [ ] **Step 3: Run the repository's complete quality gate** + +Run: + +```bash +rtk npm run check +``` + +Expected: lint, TypeScript type checking, coverage tests, build, publint, and +package dry-run all pass. + +- [ ] **Step 4: Commit documentation or formatter changes** + +```bash +rtk git add docs/analyze.md src/analyze/rules.ts tests/analyze.rules.test.ts +rtk git commit -m "docs(analyze): describe theme token diagnostics" +``` + +If formatting made no source/test changes, the commit contains only +`docs/analyze.md`. + +- [ ] **Step 5: Verify the final branch state and issue coverage** + +Run: + +```bash +rtk git status --short --branch +rtk git log --oneline origin/main..HEAD +rtk git diff --check origin/main...HEAD +rtk gh pr list --repo askrjs/askr-cli --state all --search "48 in:body" --json number,title,state,url +``` + +Expected: a clean branch, the design and implementation commits listed, no +whitespace errors, and no competing pull request for issue #48. + +- [ ] **Step 6: Push and open the pull request** + +```bash +rtk git push -u origin fix/48-hardcoded-theme-token-rule +rtk gh pr create --repo askrjs/askr-cli --base main --head lntutor:fix/48-hardcoded-theme-token-rule --title "feat(analyze): report hardcoded theme tokens" --body $'## Summary\n- add askr/no-hardcoded-theme-token for runtime JS/TS literals\n- respect configured source exclusions and exempt only @askrjs/themes\n- document the rule and cover strings, templates, JSX, comments, exclusions, and workspace identity\n\n## Verification\n- npm run check\n\nCloses #48' +``` + +Expected: GitHub returns a new PR URL created after the contribution-goal +baseline. diff --git a/docs/superpowers/specs/2026-07-30-hardcoded-theme-token-rule-design.md b/docs/superpowers/specs/2026-07-30-hardcoded-theme-token-rule-design.md new file mode 100644 index 0000000..1acb6f8 --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-hardcoded-theme-token-rule-design.md @@ -0,0 +1,99 @@ +# Hardcoded Theme Token Analyzer Rule Design + +## Goal + +Add an `askr analyze` rule that reports direct references to Askr CSS custom +properties (`--ak-*`) in runtime JavaScript and TypeScript. The rule should +guide users toward semantic classes or `data-*` attributes without changing +source code automatically. + +## Rule Contract + +- ID: `askr/no-hardcoded-theme-token` +- Category: `correctness` +- Default severity: `warning` +- Autofix: none +- Message: runtime code should not name Askr theme tokens directly; move the + token mapping to theme CSS and select it through a semantic class or a + `data-*` attribute. + +The rule applies to `.js`, `.jsx`, `.ts`, and `.tsx` files already selected by +the analyzer. Existing configured `exclude` globs continue to control source +discovery and require no rule-specific matching logic. + +## Detection Architecture + +Implement the behavior as a dedicated rule in the existing analyzer rule +registry. The rule walks the TypeScript AST and inspects literal-like runtime +values rather than searching raw source text. This keeps comments out of scope +and gives each diagnostic an exact source location. + +The visitor reports a node when its literal text contains `--ak-`: + +- string literals; +- no-substitution template literals; +- template head, middle, and tail segments in interpolated templates; and +- JSX attribute values written as string literals. + +Each AST node is reported at most once. The rule does not evaluate identifiers, +concatenations, function results, or other value flow. It also does not inspect +comments, CSS files, or arbitrary raw source text. + +## Theme-Owner Exemption + +The analyzer should skip this rule when the analyzed workspace manifest has the +exact package name `@askrjs/themes`. This is the narrow token-owner exemption +requested by the issue. Package paths or names that merely contain `theme` are +not exempt, which avoids suppressing diagnostics in ordinary applications. + +The exemption uses `context.workspace.name`, which is already available to +every analyzer rule from the discovered workspace manifest. No new analyzer +context or project-discovery behavior is required. + +## Data Flow + +1. Existing project discovery selects runtime JS/TS files and applies configured + exclusions. +2. Project analysis reads the workspace manifest identity. +3. If the package name is `@askrjs/themes`, this rule returns no diagnostics. +4. Otherwise, the rule visits relevant literal nodes in each selected source + file. +5. A literal segment containing `--ak-` produces one warning at that segment's + source location. +6. The existing analyzer reporting pipeline formats and emits the diagnostic. + +## Error Handling and Boundaries + +Missing or malformed optional manifest metadata must not crash analysis. When +the package cannot be identified as the exact token owner, the rule runs +normally. Existing parser and project-discovery error behavior remains +unchanged. + +There is deliberately no autofix: selecting the correct semantic class or +state attribute requires application-specific intent. + +## Tests + +Add analyzer CLI integration coverage for: + +- an ordinary string containing an Askr token; +- a no-substitution template literal; +- interpolated templates, including offending static segments around an + expression; +- a JSX string attribute; +- multiple offending literals producing distinct diagnostics; +- comments and identifier-based/non-literal values producing no diagnostics; +- files omitted by configured `exclude` globs; +- an exact `@askrjs/themes` package producing no diagnostics; and +- a similarly named non-owner package still producing diagnostics. + +Run the repository's complete `npm run check` gate before opening the pull +request. + +## Alternatives Rejected + +Raw-source regular-expression scanning is smaller but would report comments and +can overlap AST-derived findings. Type-checker or value-flow analysis could find +dynamically assembled tokens, but it adds complexity and false-positive risk +beyond issue #48's syntactic scope. AST literal inspection is precise and fits +the analyzer's existing rule model. diff --git a/src/analyze/rules.ts b/src/analyze/rules.ts index 6e9c04e..236f560 100644 --- a/src/analyze/rules.ts +++ b/src/analyze/rules.ts @@ -167,6 +167,19 @@ function visit(sourceFile: ts.SourceFile, callback: (node: ts.Node) => void): vo walk(sourceFile); } +function runtimeLiteralText(node: ts.Node): string | null { + if ( + ts.isStringLiteral(node) || + ts.isNoSubstitutionTemplateLiteral(node) || + ts.isTemplateHead(node) || + ts.isTemplateMiddle(node) || + ts.isTemplateTail(node) + ) { + return node.text; + } + return null; +} + function containingFunction(node: ts.Node): ts.SignatureDeclaration | null { for (let current = node.parent; current; current = current.parent) { if (ts.isFunctionLike(current)) return current; @@ -2812,26 +2825,40 @@ const hardcodedThemeTokenRule: AnalyzeRule = { id: "askr/no-hardcoded-theme-token", category: "correctness", severity: "warning", - description: "Runtime UI literals should use semantic theme tokens.", + description: "Runtime UI literals should use semantic theme styling.", analyze(context) { - if (["@askrjs/askr", "@askrjs/themes"].includes(packageName(context.workspace.manifest))) - return []; + const workspacePackage = packageName(context.workspace.manifest); + const ownsThemeTokens = workspacePackage === "@askrjs/themes"; + const mayHardcodeColors = ["@askrjs/askr", "@askrjs/themes"].includes(workspacePackage); const diagnostics: AnalyzeDiagnostic[] = []; const color = /(?:#[0-9a-f]{3,8}\b|\brgba?\s*\(|\bhsla?\s*\()/i; for (const sourceFile of context.sourceFiles) { - if (/(?:^|[./_-])(?:test|spec)\.[cm]?[jt]sx?$/.test(sourceFile.fileName)) continue; - if (!color.test(sourceFile.text)) continue; + const checkColors = + !mayHardcodeColors && + !/(?:^|[./_-])(?:test|spec)\.[cm]?[jt]sx?$/.test(sourceFile.fileName) && + color.test(sourceFile.text); + const checkTokens = !ownsThemeTokens && sourceFile.text.includes("--ak-"); + if (!checkColors && !checkTokens) continue; visit(sourceFile, (node) => { - if ( - (ts.isStringLiteralLike(node) || ts.isNoSubstitutionTemplateLiteral(node)) && - color.test(node.text) - ) { + const text = runtimeLiteralText(node); + if (text === null) return; + if (checkTokens && text.includes("--ak-")) { + diagnostics.push( + diagnostic( + context, + node, + this, + "Runtime code names an Askr theme token directly; use a semantic class or data-* attribute instead.", + "Move the token mapping to theme CSS and select it through a semantic class or data-* attribute.", + ), + ); + } else if (checkColors && color.test(text)) { diagnostics.push( diagnostic( context, node, this, - `Runtime UI literal '${node.text}' hardcodes a color instead of a theme token.`, + `Runtime UI literal '${text}' hardcodes a color instead of a theme token.`, "Use a semantic prop, theme variable, or design-system class.", ), ); diff --git a/templates/ssr/src/pages/example.tsx b/templates/ssr/src/pages/example.tsx index 354678d..a02841c 100644 --- a/templates/ssr/src/pages/example.tsx +++ b/templates/ssr/src/pages/example.tsx @@ -92,7 +92,7 @@ export default function Example() {

Reactive state driving UI updates in real time.

-
+
setBold((b) => !b)}> Bold diff --git a/templates/ssr/src/styles.css b/templates/ssr/src/styles.css index 001555e..d7714a9 100644 --- a/templates/ssr/src/styles.css +++ b/templates/ssr/src/styles.css @@ -127,6 +127,13 @@ code { margin-bottom: var(--ak-space-md); } +.showcase-controls { + display: flex; + align-items: center; + gap: var(--ak-space-md); + margin-bottom: var(--ak-space-md); +} + /* Hero buttons */ .hero-actions { display: flex; diff --git a/templates/startkit/src/components/app-header.tsx b/templates/startkit/src/components/app-header.tsx index 1bca567..c651dd1 100644 --- a/templates/startkit/src/components/app-header.tsx +++ b/templates/startkit/src/components/app-header.tsx @@ -24,9 +24,9 @@ export default function AppHeader() { return (
diff --git a/templates/startkit/src/components/page-header.tsx b/templates/startkit/src/components/page-header.tsx index 120a1e0..82f14e7 100644 --- a/templates/startkit/src/components/page-header.tsx +++ b/templates/startkit/src/components/page-header.tsx @@ -10,7 +10,6 @@ export default function PageHeader(props: { class="page-header" align="center" justify="between" - gap="var(--ak-space-lg)" wrap="wrap" >
diff --git a/templates/startkit/src/pages/workspace/accounts/index.tsx b/templates/startkit/src/pages/workspace/accounts/index.tsx index d635802..cde148c 100644 --- a/templates/startkit/src/pages/workspace/accounts/index.tsx +++ b/templates/startkit/src/pages/workspace/accounts/index.tsx @@ -147,7 +147,7 @@ export default function AccountsPage() { errorText={accountsResource.error?.message ?? null} /> - +