From 25bafe2977e182d407e8eda154489e3fdabc71f2 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Thu, 28 Jul 2022 23:55:51 -0700 Subject: [PATCH 1/5] wip: add no reactive literals rule --- docs/rules/no-reactive-literals.md | 32 +++++++++ src/rules/no-reactive-literals.ts | 69 +++++++++++++++++++ src/utils/rules.ts | 2 + .../invalid/test01-errors.json | 17 +++++ .../invalid/test01-input.svelte | 6 ++ .../invalid/test01-output.svelte | 6 ++ .../valid/test01-input.svelte | 6 ++ tests/src/rules/no-reactive-literals.ts | 12 ++++ 8 files changed, 150 insertions(+) create mode 100644 docs/rules/no-reactive-literals.md create mode 100644 src/rules/no-reactive-literals.ts create mode 100644 tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json create mode 100644 tests/fixtures/rules/no-reactive-literals/invalid/test01-input.svelte create mode 100644 tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte create mode 100644 tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte create mode 100644 tests/src/rules/no-reactive-literals.ts diff --git a/docs/rules/no-reactive-literals.md b/docs/rules/no-reactive-literals.md new file mode 100644 index 000000000..ce24cd8cf --- /dev/null +++ b/docs/rules/no-reactive-literals.md @@ -0,0 +1,32 @@ +# (svelte/no-reactive-literals) + +> Don't assign literal values in reactive statements + +## :book: Rule Details + +This rule reports on any assignment of a static, unchanging value within a reactive statement because it's not necessary. + + + + + +```svelte + +``` + + +## :wrench: Options + +Nothing + +## :mag: Implementation + +- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/no-reactive-literals.ts) +- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/no-reactive-literals.ts) diff --git a/src/rules/no-reactive-literals.ts b/src/rules/no-reactive-literals.ts new file mode 100644 index 000000000..70c284a96 --- /dev/null +++ b/src/rules/no-reactive-literals.ts @@ -0,0 +1,69 @@ +import type { TSESTree } from "@typescript-eslint/types" +import { createRule } from "../utils" + +const labeledStatementBase = `SvelteReactiveStatement > ExpressionStatement > AssignmentExpression` + +export default createRule("no-reactive-literals", { + meta: { + docs: { + description: "Don't assign literal values in reactive statements", + category: "Stylistic Issues", + recommended: false, + conflictWithPrettier: false, + }, + fixable: "code", + schema: [], + messages: { + noReactiveLiterals: `Do not assign literal values inside reactive statements unless absolutely necessary.`, + }, + type: "suggestion", + }, + create(context) { + /** + * Reusable method for multiple types of nodes that should warn + * + * @param node TSESTree.AssignmentExpression the node that was found + * @returns void + */ + function warn(node: TSESTree.AssignmentExpression) { + // Move upwards to include the entire reactive statement + const parent = node.parent?.parent + + if (!parent) { + return false + } + + const source = context.getSourceCode() + + return context.report({ + node: parent, + loc: parent.loc, + messageId: "noReactiveLiterals", + + fix(fixer) { + return [ + // Insert "let" + whatever was in there + fixer.insertTextBefore(parent, `let ${source.getText(node)}`), + + // Remove the original reactive statement + fixer.remove(parent), + ] + }, + }) + } + + return { + // $: foo = "foo"; + // $: foo = 1; + [`${labeledStatementBase}[right.type="Literal"]`]: warn, + + // $: foo = []; + [`${labeledStatementBase}[right.type="ArrayExpression"][right.elements.length=0]`]: + warn, + + // $: foo = {}; + [`${labeledStatementBase}[right.type="ObjectExpression"][right.properties.length=0]`]: + warn, + } + }, +}) diff --git a/src/utils/rules.ts b/src/utils/rules.ts index 8a2b068fc..c08ae0f1d 100644 --- a/src/utils/rules.ts +++ b/src/utils/rules.ts @@ -15,6 +15,7 @@ import noDynamicSlotName from "../rules/no-dynamic-slot-name" import noInnerDeclarations from "../rules/no-inner-declarations" import noNotFunctionHandler from "../rules/no-not-function-handler" import noObjectInTextMustaches from "../rules/no-object-in-text-mustaches" +import noReactiveLiterals from "../rules/no-reactive-literals" import noShorthandStylePropertyOverrides from "../rules/no-shorthand-style-property-overrides" import noSpacesAroundEqualSignsInAttribute from "../rules/no-spaces-around-equal-signs-in-attribute" import noTargetBlank from "../rules/no-target-blank" @@ -47,6 +48,7 @@ export const rules = [ noInnerDeclarations, noNotFunctionHandler, noObjectInTextMustaches, + noReactiveLiterals, noShorthandStylePropertyOverrides, noSpacesAroundEqualSignsInAttribute, noTargetBlank, diff --git a/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json b/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json new file mode 100644 index 000000000..cb52a2f99 --- /dev/null +++ b/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json @@ -0,0 +1,17 @@ +[ + { + "message": "Do not assign literal values inside reactive statements unless absolutely necessary.", + "line": 3, + "column": 5 + }, + { + "message": "Do not assign literal values inside reactive statements unless absolutely necessary.", + "line": 4, + "column": 5 + }, + { + "message": "Do not assign literal values inside reactive statements unless absolutely necessary.", + "line": 5, + "column": 5 + } +] diff --git a/tests/fixtures/rules/no-reactive-literals/invalid/test01-input.svelte b/tests/fixtures/rules/no-reactive-literals/invalid/test01-input.svelte new file mode 100644 index 000000000..0267fe56b --- /dev/null +++ b/tests/fixtures/rules/no-reactive-literals/invalid/test01-input.svelte @@ -0,0 +1,6 @@ + + diff --git a/tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte b/tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte new file mode 100644 index 000000000..8f3b11054 --- /dev/null +++ b/tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte @@ -0,0 +1,6 @@ + + diff --git a/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte b/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte new file mode 100644 index 000000000..8f0034dd5 --- /dev/null +++ b/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte @@ -0,0 +1,6 @@ + + diff --git a/tests/src/rules/no-reactive-literals.ts b/tests/src/rules/no-reactive-literals.ts new file mode 100644 index 000000000..368cefc17 --- /dev/null +++ b/tests/src/rules/no-reactive-literals.ts @@ -0,0 +1,12 @@ +import { RuleTester } from "eslint" +import rule from "../../../src/rules/no-reactive-literals" +import { loadTestCases } from "../../utils/utils" + +const tester = new RuleTester({ + parserOptions: { + ecmaVersion: 2020, + sourceType: "module", + }, +}) + +tester.run("no-reactive-literals", rule as any, loadTestCases("no-reactive-literals")) From 2e524100e491bbfac8719347f7b6293b3727ef0d Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sat, 30 Jul 2022 00:01:44 -0700 Subject: [PATCH 2/5] feat: add snapshot support for suggestions --- src/types.ts | 2 ++ tests/utils/utils.ts | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/types.ts b/src/types.ts index 4a8b32452..fc14c2701 100644 --- a/src/types.ts +++ b/src/types.ts @@ -69,6 +69,7 @@ export interface RuleMetaData { } messages: { [messageId: string]: string } fixable?: "code" | "whitespace" + hasSuggestions?: boolean schema: JSONSchema4 | JSONSchema4[] deprecated?: boolean replacedBy?: string[] @@ -98,6 +99,7 @@ export interface PartialRuleMetaData { ) messages: { [messageId: string]: string } fixable?: "code" | "whitespace" + hasSuggestions?: boolean schema: JSONSchema4 | JSONSchema4[] deprecated?: boolean replacedBy?: string[] diff --git a/tests/utils/utils.ts b/tests/utils/utils.ts index 56b70694d..644ff8ee7 100644 --- a/tests/utils/utils.ts +++ b/tests/utils/utils.ts @@ -130,6 +130,7 @@ export function loadTestCases( throw new Error(`Empty code: ${test.filename}`) } } + return { valid, invalid, @@ -155,6 +156,14 @@ function* itrListupInput(rootDir: string): IterableIterator { } } +// Necessary because of this: +// https://github.com/eslint/eslint/issues/14936#issuecomment-906746754 +function applySuggestion(code: string, suggestion: Linter.LintSuggestion) { + const { fix } = suggestion + + return `${code.slice(0, fix.range[0])}${fix.text}${code.slice(fix.range[1])}` +} + function writeFixtures( ruleName: string, inputFile: string, @@ -184,6 +193,7 @@ function writeFixtures( }, config.filename, ) + if (force || !fs.existsSync(errorFile)) { fs.writeFileSync( errorFile, @@ -192,6 +202,14 @@ function writeFixtures( message: m.message, line: m.line, column: m.column, + suggestions: m.suggestions + ? m.suggestions.map((s) => ({ + desc: s.desc, + messageId: s.messageId, + // Need to have this be the *fixed* output, not just the fix content or anything + output: applySuggestion(config.code, s), + })) + : null, })), null, 2, From 7108869669cb6c93e12af7f51743a23653411a51 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sat, 30 Jul 2022 00:02:25 -0700 Subject: [PATCH 3/5] wip: feedback & test updates --- src/rules/no-reactive-literals.ts | 92 +++++++++---------- .../invalid/test01-errors.json | 27 +++++- .../invalid/test01-output.svelte | 6 -- tests/src/rules/no-reactive-literals.ts | 14 ++- 4 files changed, 76 insertions(+), 63 deletions(-) delete mode 100644 tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte diff --git a/src/rules/no-reactive-literals.ts b/src/rules/no-reactive-literals.ts index 70c284a96..ba69034a1 100644 --- a/src/rules/no-reactive-literals.ts +++ b/src/rules/no-reactive-literals.ts @@ -1,69 +1,63 @@ import type { TSESTree } from "@typescript-eslint/types" import { createRule } from "../utils" -const labeledStatementBase = `SvelteReactiveStatement > ExpressionStatement > AssignmentExpression` - export default createRule("no-reactive-literals", { meta: { docs: { description: "Don't assign literal values in reactive statements", - category: "Stylistic Issues", + category: "Best Practices", recommended: false, - conflictWithPrettier: false, }, - fixable: "code", + hasSuggestions: true, schema: [], messages: { noReactiveLiterals: `Do not assign literal values inside reactive statements unless absolutely necessary.`, + fixReactiveLiteral: `Move the literal out of the reactive statement into an assignment`, }, type: "suggestion", }, create(context) { - /** - * Reusable method for multiple types of nodes that should warn - * - * @param node TSESTree.AssignmentExpression the node that was found - * @returns void - */ - function warn(node: TSESTree.AssignmentExpression) { - // Move upwards to include the entire reactive statement - const parent = node.parent?.parent - - if (!parent) { - return false - } - - const source = context.getSourceCode() - - return context.report({ - node: parent, - loc: parent.loc, - messageId: "noReactiveLiterals", - - fix(fixer) { - return [ - // Insert "let" + whatever was in there - fixer.insertTextBefore(parent, `let ${source.getText(node)}`), - - // Remove the original reactive statement - fixer.remove(parent), - ] - }, - }) - } - return { - // $: foo = "foo"; - // $: foo = 1; - [`${labeledStatementBase}[right.type="Literal"]`]: warn, - - // $: foo = []; - [`${labeledStatementBase}[right.type="ArrayExpression"][right.elements.length=0]`]: - warn, - - // $: foo = {}; - [`${labeledStatementBase}[right.type="ObjectExpression"][right.properties.length=0]`]: - warn, + [`SvelteReactiveStatement > ExpressionStatement > AssignmentExpression${[ + // $: foo = "foo"; + // $: foo = 1; + `[right.type="Literal"]`, + + // $: foo = []; + `[right.type="ArrayExpression"][right.elements.length=0]`, + + // $: foo = {}; + `[right.type="ObjectExpression"][right.properties.length=0]`, + ].join(",")}`](node: TSESTree.AssignmentExpression) { + // Move upwards to include the entire reactive statement + const parent = node.parent?.parent + + if (!parent) { + return false + } + + const source = context.getSourceCode() + + return context.report({ + node: parent, + loc: parent.loc, + messageId: "noReactiveLiterals", + suggest: [ + { + messageId: "fixReactiveLiteral", + fix(fixer) { + return [ + // Insert "let" + whatever was in there + fixer.insertTextBefore(parent, `let ${source.getText(node)}`), + + // Remove the original reactive statement + fixer.remove(parent), + ] + }, + }, + ], + }) + }, } }, }) diff --git a/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json b/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json index cb52a2f99..c09a9bba6 100644 --- a/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json +++ b/tests/fixtures/rules/no-reactive-literals/invalid/test01-errors.json @@ -2,16 +2,37 @@ { "message": "Do not assign literal values inside reactive statements unless absolutely necessary.", "line": 3, - "column": 5 + "column": 5, + "suggestions": [ + { + "desc": "Move the literal out of the reactive statement into an assignment", + "messageId": "fixReactiveLiteral", + "output": "\n\n" + } + ] }, { "message": "Do not assign literal values inside reactive statements unless absolutely necessary.", "line": 4, - "column": 5 + "column": 5, + "suggestions": [ + { + "desc": "Move the literal out of the reactive statement into an assignment", + "messageId": "fixReactiveLiteral", + "output": "\n\n" + } + ] }, { "message": "Do not assign literal values inside reactive statements unless absolutely necessary.", "line": 5, - "column": 5 + "column": 5, + "suggestions": [ + { + "desc": "Move the literal out of the reactive statement into an assignment", + "messageId": "fixReactiveLiteral", + "output": "\n\n" + } + ] } ] diff --git a/tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte b/tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte deleted file mode 100644 index 8f3b11054..000000000 --- a/tests/fixtures/rules/no-reactive-literals/invalid/test01-output.svelte +++ /dev/null @@ -1,6 +0,0 @@ - - diff --git a/tests/src/rules/no-reactive-literals.ts b/tests/src/rules/no-reactive-literals.ts index 368cefc17..d78739107 100644 --- a/tests/src/rules/no-reactive-literals.ts +++ b/tests/src/rules/no-reactive-literals.ts @@ -3,10 +3,14 @@ import rule from "../../../src/rules/no-reactive-literals" import { loadTestCases } from "../../utils/utils" const tester = new RuleTester({ - parserOptions: { - ecmaVersion: 2020, - sourceType: "module", - }, + parserOptions: { + ecmaVersion: 2020, + sourceType: "module", + }, }) -tester.run("no-reactive-literals", rule as any, loadTestCases("no-reactive-literals")) +tester.run( + "no-reactive-literals", + rule as any, + loadTestCases("no-reactive-literals"), +) From 6feae94f59b0b7fca63bf3e35aabd829144bef9d Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sat, 30 Jul 2022 14:00:38 -0700 Subject: [PATCH 4/5] chore: fix lint issue --- .../rules/no-reactive-literals/valid/test01-input.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte b/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte index 8f0034dd5..67e41d3e0 100644 --- a/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte +++ b/tests/fixtures/rules/no-reactive-literals/valid/test01-input.svelte @@ -1,6 +1,6 @@ From 3b1435cee8bd168848b61644f334cd34e2684684 Mon Sep 17 00:00:00 2001 From: Pat Cavit Date: Sat, 30 Jul 2022 19:18:08 -0700 Subject: [PATCH 5/5] docs: yarn update output --- README.md | 1 + docs/rules.md | 1 + docs/rules/no-reactive-literals.md | 12 +++++++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3f346b4a6..5dbdf553f 100644 --- a/README.md +++ b/README.md @@ -281,6 +281,7 @@ These rules relate to better ways of doing things to help you avoid problems: |:--------|:------------|:---| | [svelte/button-has-type](https://ota-meshi.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute | | | [svelte/no-at-debug-tags](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: | +| [svelte/no-reactive-literals](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | Don't assign literal values in reactive statements | | | [svelte/no-unused-svelte-ignore](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-unused-svelte-ignore/) | disallow unused svelte-ignore comments | :star: | | [svelte/no-useless-mustaches](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-useless-mustaches/) | disallow unnecessary mustache interpolations | :wrench: | | [svelte/require-optimized-style-attribute](https://ota-meshi.github.io/eslint-plugin-svelte/rules/require-optimized-style-attribute/) | require style attributes that can be optimized | | diff --git a/docs/rules.md b/docs/rules.md index 900cdcc55..9581c794f 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -41,6 +41,7 @@ These rules relate to better ways of doing things to help you avoid problems: |:--------|:------------|:---| | [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | | | [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: | +| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | Don't assign literal values in reactive statements | | | [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/require-optimized-style-attribute](./rules/require-optimized-style-attribute.md) | require style attributes that can be optimized | | diff --git a/docs/rules/no-reactive-literals.md b/docs/rules/no-reactive-literals.md index ce24cd8cf..37a297d42 100644 --- a/docs/rules/no-reactive-literals.md +++ b/docs/rules/no-reactive-literals.md @@ -1,7 +1,16 @@ -# (svelte/no-reactive-literals) +--- +pageClass: "rule-details" +sidebarDepth: 0 +title: "svelte/no-reactive-literals" +description: "Don't assign literal values in reactive statements" +--- + +# svelte/no-reactive-literals > Don't assign literal values in reactive statements +- :exclamation: **_This rule has not been released yet._** + ## :book: Rule Details This rule reports on any assignment of a static, unchanging value within a reactive statement because it's not necessary. @@ -20,6 +29,7 @@ This rule reports on any assignment of a static, unchanging value within a react $: foo = "bar"; ``` + ## :wrench: Options