feat(core): add CORRECT005 — flag mutation of a non-bindable $props

.changeset/correct005-prop-mutation.md

@@ -0,0 +1,6 @@
+---
+'@svelte-vitals/core': minor
+'svelte-vitals': minor
+---
+
+Add CORRECT005: flag mutation of a non-`$bindable` prop destructured from `$props()` (member writes, `delete`, or a mutating method call like `.push()`). Plain reassignment of the prop itself is not flagged — Svelte's docs explicitly sanction that pattern for ephemeral state; only mutation is prohibited. Catches a class of bug the compiler never reports: mutating a plain-object prop is a silent no-op, and mutating a reactive-state-proxy prop only warns at runtime if that code path is exercised.

AGENTS.md

@@ -40,7 +40,7 @@ CI (`.github/workflows/ci.yml`) runs four jobs: `lint`, `check` (build + typeche
   - `fix(cli): make --diff/--staged work when the project is not at the git repo root`
   - `test(cli): pin behavior for malformed .svelte files in both passes`
   - Other prefixes in use: `feat(vite):`, `docs:`, `chore:`.
-- **Adding a rule**: create `packages/core/src/rules/<category>/xxxNNN-slug.ts`, then register it in `packages/core/src/rules/index.ts` in all three places: the import, the `allRules` array, and the re-export block. Note: for historical reasons performance rules are split across `rules/perf/` (PERF001–008) and `rules/performance/` (PERF009–010) — check both when looking for an existing PERF rule.
+- **Adding a rule**: create `packages/core/src/rules/<category>/xxxNNN-slug.ts`, then register it in **four** places: `packages/core/src/rules/index.ts` (the import, the `allRules` array, and the re-export block) _and_ `packages/core/src/index.ts`'s own `export { ... } from './rules/index.js'` list, which duplicates the same names. TypeScript won't catch a missed spot in the fourth place (it's a plain re-export list), so grep for the previous rule's id after adding a new one. Add rule docs under `docs/src/content/docs/rules/<id>.md` (en) and `docs/src/content/docs/ja/rules/<id>.md` (ja) — `packages/cli/test/docs-links.test.ts` fails the build if either is missing. Note: for historical reasons performance rules are split across `rules/perf/` (PERF001–008) and `rules/performance/` (PERF009–010) — check both when looking for an existing PERF rule.
 - **Tests**: vitest, per-package `test/` directories; fixtures live under `test/fixtures/`.
 
 ## Design docs

docs/src/content/docs/guides/cli.md

@@ -131,7 +131,7 @@ An unknown category or a negative/non-numeric value is an error (exit `2`).
 
 For one intentional occurrence that `--ignore` would silence project-wide, add a
 `svelte-vitals-disable-next-line` comment on the line directly above it. Works for
-any component-scoped rule (Correctness, Security, Architecture, Performance): CORRECT001–004,
+any component-scoped rule (Correctness, Security, Architecture, Performance): CORRECT001–005,
 SEC001–002, ARCH001–002, PERF009–010.
 
 ```svelte

docs/src/content/docs/ja/guides/cli.md

@@ -129,7 +129,7 @@ svelte-vitals --weights seo=2,performance=1
 
 ### 特定の指摘だけをインラインで抑制する
 
-`--ignore` はプロジェクト全体でルールを無効にしますが、意図的な1箇所だけを黙らせたい場合は、対象行の直前に `svelte-vitals-disable-next-line` コメントを書きます。コンポーネントスコープの全ルール（Correctness、Security、Architecture、Performance）に対応: CORRECT001–004、SEC001–002、ARCH001–002、PERF009–010。
+`--ignore` はプロジェクト全体でルールを無効にしますが、意図的な1箇所だけを黙らせたい場合は、対象行の直前に `svelte-vitals-disable-next-line` コメントを書きます。コンポーネントスコープの全ルール（Correctness、Security、Architecture、Performance）に対応: CORRECT001–005、SEC001–002、ARCH001–002、PERF009–010。
 
 ```svelte
 <script>

docs/src/content/docs/ja/rules/correct005.md

@@ -0,0 +1,44 @@
+---
+title: CORRECT005 · 非 bindable prop の変異
+description: $bindable を宣言していない $props() の値を変異させてはいけません。
+---
+
+**重大度:** warning · **カテゴリ:** correctness
+
+## チェック内容
+
+`$props()` から分割代入された値のうち `$bindable` を宣言していないものへの変異を検出します — メンバー書き込み(`user.name = …`、`obj.count += 1`)、`delete obj.x`、変異メソッド呼び出し(`items.push(…)`、`arr.splice(…)`、`map.set(…)` など)です。`...rest` で受けたバインディングも対象になります — rest props は個別に `$bindable` を宣言できないためです。prop 自体への単純な再代入(`count = 5`)は対象外です — Svelte の公式ドキュメントは一時的な状態保持のための再代入を明示的に許容しており、禁止されているのは変異のみです。コンポーネントのスクリプトとテンプレートを静的(CLI)解析します。
+
+prop と同名の関数パラメータや `{#each ... as x}` のループ変数を変異させても検出対象にはなりません — そのバインディングは prop をシャドーイングしており、もはや prop 自体ではないためです。それ以外の形のシャドーイング(ブロックスコープの `let`/`const` による再宣言、`{#snippet}`/`{:then}`/`{:catch}` のバインディング)は追跡しておらず、理論上は誤検出につながり得ます — これは意図的に部分的な緩和策であり、完全なスコープ解決ではありません。
+
+## なぜ重要か
+
+Svelte の公式ドキュメントは明確に「`$bindable` でない限り prop を変異させてはいけない」と述べています。コンパイラが捕まえない失敗モードが3つあります:
+
+- **プレーンオブジェクト**の prop を変異させても、オブジェクトが state proxy でないため**無言で何も起きません**(開発時の警告すら出ません)。
+- **リアクティブな state proxy** の prop を変異させると動作はしますが、`ownership_invalid_mutation` という開発時警告が出ます — ただしそれは**そのコードパスが実際に実行された場合のみ**です。
+- 使用中の**フォールバック値**もプレーンオブジェクトと同様に振る舞い、変異は反映されません。
+
+静的解析であれば、コードパスが実行される前のレビュー・CI の時点でこの3つすべてを捕まえられます。
+
+## 修正方法
+
+```svelte
+<script>
+  let { user } = $props();
+
+  // prop を直接変異させる代わりに:
+  function rename(name) {
+    user.name = name; // 何も起きないか、ownership_invalid_mutation 警告が出る
+  }
+
+  // 変異前にクローンする:
+  function rename(name) {
+    const next = { ...user, name };
+    // next を使うか、変更を親に持ち上げる
+  }
+
+  // 親子で共有すべきなら bindable にする:
+  let { user = $bindable() } = $props();
+</script>
+```

docs/src/content/docs/rules/correct005.md

@@ -0,0 +1,44 @@
+---
+title: CORRECT005 · Mutated non-bindable prop
+description: Don't mutate a prop from $props() unless it is declared $bindable.
+---
+
+**Severity:** warning · **Category:** correctness
+
+## What it checks
+
+Flags a mutation of a value destructured from `$props()` that is not declared `$bindable`: a member write (`user.name = …`, `obj.count += 1`), `delete obj.x`, or a call to a mutating method (`items.push(…)`, `arr.splice(…)`, `map.set(…)`, …). A `...rest` binding is tracked too — rest props can never be individually declared `$bindable`. Plain reassignment of the prop itself (`count = 5`) is **not** flagged — Svelte's docs explicitly sanction temporary reassignment for unsaved ephemeral state; only mutation is prohibited. Checked by static (CLI) analysis of the component script and template.
+
+A function parameter or `{#each ... as x}` loop variable that reuses the prop's name is not flagged when mutated — that binding shadows the prop, so it isn't the prop at all. Other forms of shadowing (a block-scoped `let`/`const` redeclaration, `{#snippet}`/`{:then}`/`{:catch}` bindings) are not tracked and could in principle produce a false positive; this is a deliberately partial mitigation, not full scope resolution.
+
+## Why it matters
+
+Svelte's docs say plainly: "don't mutate props" unless they are `$bindable`. Three failure modes, none caught by the compiler:
+
+- A **plain-object** prop mutation is a silent no-op — the object isn't a state proxy, so not even the dev-time warning fires.
+- A **reactive-state-proxy** prop mutation works, but triggers the `ownership_invalid_mutation` dev warning — only if that code path is actually exercised at runtime.
+- A **fallback value** in use behaves like a plain object — mutation has no effect.
+
+Static analysis catches all three at review/CI time, before the code path has to run.
+
+## How to fix
+
+```svelte
+<script>
+  let { user } = $props();
+
+  // Instead of mutating the prop directly:
+  function rename(name) {
+    user.name = name; // no-op or ownership_invalid_mutation warning
+  }
+
+  // Clone before mutating:
+  function rename(name) {
+    const next = { ...user, name };
+    // ...use `next`, or lift the change to the parent
+  }
+
+  // Or make it bindable, if the parent and child should share it:
+  let { user = $bindable() } = $props();
+</script>
+```

packages/cli/test/malformed-svelte.test.ts

@@ -46,6 +46,7 @@ describe('collectComponentFacts: malformed .svelte files (component path)', () =
       imports: [],
       namespaceImports: [],
       constableStates: [],
+      mutatedProps: [],
       suppressions: []
     });
 
@@ -102,6 +103,7 @@ describe('collectComponentFacts: malformed .svelte files (component path)', () =
       imports: [],
       namespaceImports: [],
       constableStates: [],
+      mutatedProps: [],
       suppressions: []
     });
     expect(byFile.get(goodPath)!.loc).toBeGreaterThan(0);

packages/cli/test/suppression-e2e.test.ts

@@ -24,6 +24,7 @@ const comp = (over: Partial<ComponentFacts>): ComponentFacts => ({
   imports: [],
   namespaceImports: [],
   constableStates: [],
+  mutatedProps: [],
   suppressions: [],
   ...over
 });

packages/core/src/component-collect.ts

@@ -20,6 +20,7 @@ export function emptyComponentFacts(file: string): ComponentFacts {
     imports: [],
     namespaceImports: [],
     constableStates: [],
+    mutatedProps: [],
     suppressions: []
   };
 }

packages/core/src/component-parse.ts

@@ -311,6 +311,138 @@ function isPropsCall(node: Node): boolean {
   return node?.type === 'CallExpression' && node.callee?.type === 'Identifier' && node.callee.name === '$props';
 }
 
+/** Whether a CallExpression is a `$bindable(...)` call (a destructured prop's default value). */
+function isBindableCall(node: Node): boolean {
+  return node?.type === 'CallExpression' && node.callee?.type === 'Identifier' && node.callee.name === '$bindable';
+}
+
+/**
+ * Local identifier names bound to a non-`$bindable` prop from `$props()` (CORRECT005):
+ * plain and renamed destructured names, and the `...rest` binding (rest props can never
+ * be individually declared `$bindable` — that requires a per-prop destructuring default).
+ * A prop initialized with `$bindable(...)` is excluded — mutating it is the intended
+ * contract. `let props = $props()` (no destructuring) tracks `props` itself, since none
+ * of its fields can be `$bindable` either. Returns an empty set when `$props()` appears
+ * more than once, or a destructuring shape is ambiguous (nested pattern) — conservative,
+ * to avoid false positives rather than chase every shape.
+ */
+function collectNonBindableProps(program: Node): Set<string> {
+  const names = new Set<string>();
+  let seen = 0;
+  let ambiguous = false;
+  walkEstree(program, (n) => {
+    if (n.type !== 'VariableDeclarator' || !n.init || !isPropsCall(n.init)) return;
+    seen++;
+    if (n.id?.type === 'Identifier') {
+      names.add(n.id.name);
+      return;
+    }
+    if (n.id?.type !== 'ObjectPattern' || !Array.isArray(n.id.properties)) {
+      ambiguous = true;
+      return;
+    }
+    for (const p of n.id.properties) {
+      if (p?.type === 'RestElement') {
+        addBoundNames(p.argument, names);
+      } else if (p?.type === 'Property') {
+        if (p.value?.type === 'AssignmentPattern') {
+          if (!isBindableCall(p.value.right) && p.value.left?.type === 'Identifier') names.add(p.value.left.name);
+        } else if (p.value?.type === 'Identifier') {
+          names.add(p.value.name);
+        }
+        // A nested destructuring pattern (`{ a: { b } }`) is skipped conservatively.
+      }
+    }
+  });
+  return ambiguous || seen > 1 ? new Set() : names;
+}
+
+/** Mutating array/Set/Map methods — a call to one of these on a non-bindable prop mutates it (CORRECT005). */
+const MUTATING_METHODS = new Set([
+  'push',
+  'pop',
+  'shift',
+  'unshift',
+  'splice',
+  'sort',
+  'reverse',
+  'copyWithin',
+  'fill',
+  'set',
+  'add',
+  'delete',
+  'clear'
+]);
+
+/**
+ * Flag mutations of a non-`$bindable` prop (CORRECT005): a member-expression write
+ * (`prop.x = …`, `prop.x += …`, `prop.x++`), `delete prop.x`, or a call to a mutating
+ * method on the prop (`prop.push(...)`). Plain reassignment of the prop identifier
+ * itself (`prop = 5`) is NOT flagged — Svelte's docs explicitly sanction temporary
+ * reassignment for ephemeral state; only mutation is prohibited. Run over the instance
+ * program AND the template fragment (inline handlers can mutate props in the template).
+ */
+function collectPropMutations(
+  root: Node,
+  propNames: Set<string>,
+  source: string,
+  acc: { name: string; line: number }[]
+): void {
+  if (propNames.size === 0) return;
+
+  // Scope-aware guard (review): a prop name can be shadowed by a nested function's
+  // parameter (`function process(items) { items.push(x) }`) or a template `{#each}`
+  // block's loop variable (`{#each other as items}`) — either introduces an unrelated
+  // binding with the same name, and mutating IT is not a prop mutation. We track only
+  // these two binding sources (the two realistic ways a short prop-like name gets
+  // reused in a Svelte component); full lexical scope resolution (block-scoped
+  // let/const redeclaration, {#snippet}/{:then}/{:catch} bindings) is not attempted —
+  // this is a known, deliberately partial mitigation, not exhaustive shadow tracking.
+  function visit(node: Node, shadowed: Set<string>): void {
+    if (Array.isArray(node)) {
+      for (const child of node) visit(child, shadowed);
+      return;
+    }
+    if (!node || typeof node !== 'object' || typeof node.type !== 'string') return;
+
+    let scope = shadowed;
+    if (
+      node.type === 'FunctionDeclaration' ||
+      node.type === 'FunctionExpression' ||
+      node.type === 'ArrowFunctionExpression'
+    ) {
+      const introduced = new Set<string>();
+      for (const p of node.params ?? []) addBoundNames(p, introduced);
+      if (introduced.size > 0) scope = new Set([...shadowed, ...introduced]);
+    } else if (node.type === 'EachBlock' && node.context) {
+      const introduced = new Set<string>();
+      addBoundNames(node.context, introduced);
+      if (introduced.size > 0) scope = new Set([...shadowed, ...introduced]);
+    }
+
+    const flag = (r: string | undefined, at: Node) => {
+      if (r && propNames.has(r) && !scope.has(r)) acc.push({ name: r, line: lineOf(source, at.start) });
+    };
+    if (node.type === 'AssignmentExpression' && node.left?.type === 'MemberExpression') {
+      flag(rootObjectName(node.left), node);
+    } else if (node.type === 'UpdateExpression' && node.argument?.type === 'MemberExpression') {
+      flag(rootObjectName(node.argument), node);
+    } else if (node.type === 'UnaryExpression' && node.operator === 'delete') {
+      flag(rootObjectName(node.argument), node);
+    } else if (node.type === 'CallExpression' && node.callee?.type === 'MemberExpression') {
+      const method = node.callee.property?.type === 'Identifier' ? node.callee.property.name : undefined;
+      if (method && MUTATING_METHODS.has(method)) flag(rootObjectName(node.callee.object), node);
+    }
+
+    for (const key of Object.keys(node)) {
+      if (key === 'type' || key === 'start' || key === 'end' || key === 'loc' || key === 'range') continue;
+      visit(node[key], scope);
+    }
+  }
+
+  visit(root, new Set());
+}
+
 /** Named props destructured from `$props()`, or 0 when unknowable (ARCH002). */
 function countProps(program: Node): number {
   let count = 0;
@@ -396,6 +528,7 @@ export function parseComponentFacts(
   imports: string[];
   namespaceImports: { source: string; line: number }[];
   constableStates: { name: string; line: number }[];
+  mutatedProps: { name: string; line: number }[];
   suppressions: SuppressionDirective[];
 } {
   const ast = parse(source, { modern: true, filename }) as Node;
@@ -417,12 +550,16 @@ export function parseComponentFacts(
 
   const effects: EffectFact[] = [];
   const constableStates: { name: string; line: number }[] = [];
+  const mutatedProps: { name: string; line: number }[] = [];
   let propCount = 0;
   const program = ast.instance?.content;
   if (program) {
     collectImportSources(program, imports);
     collectNamespaceImports(program, source, namespaceImports);
     propCount = countProps(program);
+    const nonBindableProps = collectNonBindableProps(program);
+    collectPropMutations(program, nonBindableProps, source, mutatedProps);
+    if (ast.fragment) collectPropMutations(ast.fragment, nonBindableProps, source, mutatedProps);
     const stateNames = new Set<string>();
     const reactiveNames = new Set<string>();
     const stateDecls: { name: string; line: number }[] = [];
@@ -465,6 +602,7 @@ export function parseComponentFacts(
     imports,
     namespaceImports,
     constableStates,
+    mutatedProps,
     suppressions
   };
 }

packages/core/src/component.ts

@@ -56,6 +56,8 @@ export interface ComponentFacts {
   namespaceImports: { source: string; line: number }[];
   /** `$state` declarations never written or escaped anywhere in the component — candidates for const (CORRECT004). */
   constableStates: { name: string; line: number }[];
+  /** Mutations of a non-`$bindable` prop from `$props()` — member writes, `delete`, or a mutating method call (CORRECT005). */
+  mutatedProps: { name: string; line: number }[];
   /** Inline `svelte-vitals-disable-next-line` directives found in this file's source — component-rule escape hatch (issue #92). Optional: absent is equivalent to no directives, so existing external constructors of `ComponentFacts` are unaffected. */
   suppressions?: SuppressionDirective[];
 }

packages/core/src/index.ts

@@ -85,6 +85,7 @@ export {
   correct002EffectDerived,
   correct003EffectAsOnMount,
   correct004UnmutatedState,
+  correct005PropMutation,
   sec001Html,
   sec002JavascriptUrl,
   arch001ComponentSize,

packages/core/src/rules/correctness/correct005-prop-mutation.ts

@@ -0,0 +1,18 @@
+import { componentRule } from '../component-rule.js';
+
+export const correct005PropMutation = componentRule({
+  id: 'CORRECT005',
+  title: 'Mutated non-bindable prop',
+  category: 'correctness',
+  label: 'Prop mutation',
+  recommendation:
+    'Clone the value before mutating it, communicate the change via a callback prop, or declare the prop $bindable if the parent and child should share it.',
+  rationale:
+    "Svelte's docs say plainly: don't mutate props unless they are $bindable. A plain-object prop mutation is a silent no-op (the object isn't a state proxy); a reactive-state-proxy prop mutation works but triggers the ownership_invalid_mutation dev warning only when that code path actually runs. Neither is caught by the compiler, so this rule catches both statically.",
+  applies: (c) => c.mutatedProps.length > 0,
+  bad: (c) =>
+    c.mutatedProps.map((m) => ({
+      line: m.line,
+      message: `Prop "${m.name}" is mutated, but it is not declared $bindable`
+    }))
+});