Skip to content

Commit

Permalink
feat(prefer-const): add rule (#933)
Browse files Browse the repository at this point in the history
Co-authored-by: baseballyama <xydybaseball@gmail.com>
  • Loading branch information
mikededo and baseballyama authored Dec 30, 2024
1 parent 117e60d commit 71eca84
Show file tree
Hide file tree
Showing 13 changed files with 228 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/green-squids-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'eslint-plugin-svelte': minor
---

Add `prefer-const` rule
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ These rules relate to better ways of doing things to help you avoid problems:
| [svelte/no-unused-class-name](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-class-name/) | disallow the use of a class in the template without a corresponding style | |
| [svelte/no-unused-svelte-ignore](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: |
| [svelte/no-useless-mustaches](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: |
| [svelte/prefer-const](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/) | Require `const` declarations for variables that are never reassigned after declared | :wrench: |
| [svelte/prefer-destructured-store-props](https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
| [svelte/require-each-key](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-each-key/) | require keyed `{#each}` block | |
| [svelte/require-event-dispatcher-types](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-event-dispatcher-types/) | require type parameters for `createEventDispatcher` | |
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ These rules relate to better ways of doing things to help you avoid problems:
| [svelte/no-unused-class-name](./rules/no-unused-class-name.md) | disallow the use of a class in the template without a corresponding style | |
| [svelte/no-unused-svelte-ignore](./rules/no-unused-svelte-ignore.md) | disallow unused svelte-ignore comments | :star: |
| [svelte/no-useless-mustaches](./rules/no-useless-mustaches.md) | disallow unnecessary mustache interpolations | :wrench: |
| [svelte/prefer-const](./rules/prefer-const.md) | Require `const` declarations for variables that are never reassigned after declared | :wrench: |
| [svelte/prefer-destructured-store-props](./rules/prefer-destructured-store-props.md) | destructure values from object stores for better change tracking & fewer redraws | :bulb: |
| [svelte/require-each-key](./rules/require-each-key.md) | require keyed `{#each}` block | |
| [svelte/require-event-dispatcher-types](./rules/require-event-dispatcher-types.md) | require type parameters for `createEventDispatcher` | |
Expand Down
69 changes: 69 additions & 0 deletions docs/rules/prefer-const.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/prefer-const'
description: 'Require `const` declarations for variables that are never reassigned after declared'
---

# svelte/prefer-const

> Require `const` declarations for variables that are never reassigned after declared
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

## :book: Rule Details

This rule reports the same as the base ESLint `prefer-const` rule, except that ignores Svelte reactive values such as `$derived` and `$props`. If this rule is active, make sure to disable the base `prefer-const` rule, as it will conflict with this rule.

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/prefer-const: "error" */
// ✓ GOOD
const { a, b } = $props();
let c = $state('');
let d = $derived(a * 2);
let e = $derived.by(() => b * 2);
// ✗ BAD
let obj = { a, b };
let g = $state(0);
let h = $state({ count: 1 });
</script>
<input bind:value={c} />
<input bind:value={h.count} />
```

## :wrench: Options

```json
{
"svelte/prefer-const": [
"error",
{
"destructuring": "any",
"ignoreReadonly": true
}
]
}
```

- `destructuring`: The kind of the way to address variables in destructuring. There are 2 values:
- `any` (default): if any variables in destructuring should be const, this rule warns for those variables.
- `all`: if all variables in destructuring should be const, this rule warns the variables. Otherwise, ignores them.
- `ignoreReadonly`: If `true`, this rule will ignore variables that are read between the declaration and the _first_ assignment.

## :books: Further Reading

- See [ESLint `prefer-const` rule](https://eslint.org/docs/latest/rules/prefer-const) for more information about the base rule.

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/prefer-const.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts)

<sup>Taken with ❤️ [from ESLint core](https://eslint.org/docs/rules/prefer-const)</sup>
10 changes: 10 additions & 0 deletions packages/eslint-plugin-svelte/src/rule-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ export interface RuleOptions {
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-class-directive/
*/
'svelte/prefer-class-directive'?: Linter.RuleEntry<SveltePreferClassDirective>
/**
* Require `const` declarations for variables that are never reassigned after declared
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-const/
*/
'svelte/prefer-const'?: Linter.RuleEntry<SveltePreferConst>
/**
* destructure values from object stores for better change tracking & fewer redraws
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/prefer-destructured-store-props/
Expand Down Expand Up @@ -485,6 +490,11 @@ type SvelteNoUselessMustaches = []|[{
type SveltePreferClassDirective = []|[{
prefer?: ("always" | "empty")
}]
// ----- svelte/prefer-const -----
type SveltePreferConst = []|[{
destructuring?: ("any" | "all")
ignoreReadBeforeAssign?: boolean
}]
// ----- svelte/shorthand-attribute -----
type SvelteShorthandAttribute = []|[{
prefer?: ("always" | "never")
Expand Down
81 changes: 81 additions & 0 deletions packages/eslint-plugin-svelte/src/rules/prefer-const.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import type { TSESTree } from '@typescript-eslint/types';

import { createRule } from '../utils/index.js';
import { defineWrapperListener, getCoreRule } from '../utils/eslint-core.js';

const coreRule = getCoreRule('prefer-const');

/**
* Finds and returns the callee of a declaration node within variable declarations or object patterns.
*/
function findDeclarationCallee(node: TSESTree.Expression) {
const { parent } = node;
if (parent.type === 'VariableDeclarator' && parent.init?.type === 'CallExpression') {
return parent.init.callee;
}

return null;
}

/**
* Determines if a declaration should be skipped in the const preference analysis.
* Specifically checks for Svelte's state management utilities ($props, $derived).
*/
function shouldSkipDeclaration(declaration: TSESTree.Expression | null) {
if (!declaration) {
return false;
}

const callee = findDeclarationCallee(declaration);
if (!callee) {
return false;
}

if (callee.type === 'Identifier' && ['$props', '$derived'].includes(callee.name)) {
return true;
}

if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') {
return false;
}

if (
callee.object.name === '$derived' &&
callee.property.type === 'Identifier' &&
callee.property.name === 'by'
) {
return true;
}

return false;
}

export default createRule('prefer-const', {
meta: {
...coreRule.meta,
docs: {
description: coreRule.meta.docs.description,
category: 'Best Practices',
recommended: false,
extensionRule: 'prefer-const'
}
},
create(context) {
return defineWrapperListener(coreRule, context, {
createListenerProxy(coreListener) {
return {
...coreListener,
VariableDeclaration(node) {
for (const decl of node.declarations) {
if (shouldSkipDeclaration(decl.init)) {
return;
}
}

coreListener.VariableDeclaration?.(node);
}
};
}
});
}
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin-svelte/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ export interface SourceCode {

getLines(): string[];

getDeclaredVariables(node: TSESTree.Node): Variable[];

getAllComments(): AST.Comment[];

getComments(node: NodeOrToken): {
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin-svelte/src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import noUnusedClassName from '../rules/no-unused-class-name.js';
import noUnusedSvelteIgnore from '../rules/no-unused-svelte-ignore.js';
import noUselessMustaches from '../rules/no-useless-mustaches.js';
import preferClassDirective from '../rules/prefer-class-directive.js';
import preferConst from '../rules/prefer-const.js';
import preferDestructuredStoreProps from '../rules/prefer-destructured-store-props.js';
import preferStyleDirective from '../rules/prefer-style-directive.js';
import requireEachKey from '../rules/require-each-key.js';
Expand Down Expand Up @@ -120,6 +121,7 @@ export const rules = [
noUnusedSvelteIgnore,
noUselessMustaches,
preferClassDirective,
preferConst,
preferDestructuredStoreProps,
preferStyleDirective,
requireEachKey,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
- message: "'zero' is never reassigned. Use 'const' instead."
line: 3
column: 6
suggestions: null
- message: "'state' is never reassigned. Use 'const' instead."
line: 4
column: 6
suggestions: null
- message: "'raw' is never reassigned. Use 'const' instead."
line: 5
column: 6
suggestions: null
- message: "'doubled' is never reassigned. Use 'const' instead."
line: 6
column: 6
suggestions: null
- message: "'calculated' is never reassigned. Use 'const' instead."
line: 8
column: 6
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
let { prop1, prop2 } = $props();
let zero = 0;
let state = $state(0);
let raw = $state.raw(0);
let doubled = state * 2;
let derived = $derived(state * 2);
let calculated = calc();
let derivedBy = $derived.by(calc());
let noInit;
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
let { prop1, prop2 } = $props();
const zero = 0;
const state = $state(0);
const raw = $state.raw(0);
const doubled = state * 2;
let derived = $derived(state * 2);
const calculated = calc();
let derivedBy = $derived.by(calc());
let noInit;
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script>
const a = {};
</script>
12 changes: 12 additions & 0 deletions packages/eslint-plugin-svelte/tests/src/rules/prefer-const.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { RuleTester } from '../../utils/eslint-compat';
import rule from '../../../src/rules/prefer-const';
import { loadTestCases } from '../../utils/utils';

const tester = new RuleTester({
languageOptions: {
ecmaVersion: 2020,
sourceType: 'module'
},
});

tester.run('prefer-const', rule as any, loadTestCases('prefer-const'));

0 comments on commit 71eca84

Please sign in to comment.