diff --git a/.changeset/clear-mugs-rush.md b/.changeset/clear-mugs-rush.md new file mode 100644 index 000000000..83198ba36 --- /dev/null +++ b/.changeset/clear-mugs-rush.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-svelte': minor +--- + +Adds a suggestion to the `require-store-callbacks-use-set-param` rule to automatically rename or add function parameters. diff --git a/README.md b/README.md index 3308ebfc6..c379a3d82 100644 --- a/README.md +++ b/README.md @@ -272,7 +272,7 @@ These rules relate to possible syntax or logic errors in Svelte code: | [svelte/no-shorthand-style-property-overrides](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-shorthand-style-property-overrides/) | disallow shorthand style properties that override related longhand properties | :star: | | [svelte/no-store-async](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-store-async/) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | :star: | | [svelte/no-unknown-style-directive-property](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-unknown-style-directive-property/) | disallow unknown `style:property` | :star: | -| [svelte/require-store-callbacks-use-set-param](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | | +| [svelte/require-store-callbacks-use-set-param](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-callbacks-use-set-param/) | store callbacks must use `set` param | :bulb: | | [svelte/require-store-reactive-access](https://sveltejs.github.io/eslint-plugin-svelte/rules/require-store-reactive-access/) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :star::wrench: | | [svelte/valid-compile](https://sveltejs.github.io/eslint-plugin-svelte/rules/valid-compile/) | disallow warnings when compiling. | | | [svelte/valid-style-parse](https://sveltejs.github.io/eslint-plugin-svelte/rules/valid-style-parse/) | require valid style element parsing | | diff --git a/docs/rules.md b/docs/rules.md index 5a47ddea8..df2ca5ec4 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -29,7 +29,7 @@ These rules relate to possible syntax or logic errors in Svelte code: | [svelte/no-shorthand-style-property-overrides](./rules/no-shorthand-style-property-overrides.md) | disallow shorthand style properties that override related longhand properties | :star: | | [svelte/no-store-async](./rules/no-store-async.md) | disallow using async/await inside svelte stores because it causes issues with the auto-unsubscribing features | :star: | | [svelte/no-unknown-style-directive-property](./rules/no-unknown-style-directive-property.md) | disallow unknown `style:property` | :star: | -| [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | | +| [svelte/require-store-callbacks-use-set-param](./rules/require-store-callbacks-use-set-param.md) | store callbacks must use `set` param | :bulb: | | [svelte/require-store-reactive-access](./rules/require-store-reactive-access.md) | disallow to use of the store itself as an operand. Need to use $ prefix or get function. | :star::wrench: | | [svelte/valid-compile](./rules/valid-compile.md) | disallow warnings when compiling. | | | [svelte/valid-style-parse](./rules/valid-style-parse.md) | require valid style element parsing | | diff --git a/docs/rules/require-store-callbacks-use-set-param.md b/docs/rules/require-store-callbacks-use-set-param.md index 4995fa495..dbe0f5945 100644 --- a/docs/rules/require-store-callbacks-use-set-param.md +++ b/docs/rules/require-store-callbacks-use-set-param.md @@ -10,6 +10,8 @@ since: 'v2.12.0' > store callbacks must use `set` param +- :bulb: Some problems reported by this rule are manually fixable by editor [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). + ## :book: Rule Details This rule disallows if `readable` / `writable` store's setter function doesn't use `set` parameter.
diff --git a/packages/eslint-plugin-svelte/src/rules/derived-has-same-inputs-outputs.ts b/packages/eslint-plugin-svelte/src/rules/derived-has-same-inputs-outputs.ts index 30f86aa31..319c49d95 100644 --- a/packages/eslint-plugin-svelte/src/rules/derived-has-same-inputs-outputs.ts +++ b/packages/eslint-plugin-svelte/src/rules/derived-has-same-inputs-outputs.ts @@ -3,34 +3,7 @@ import type { Variable } from '@typescript-eslint/scope-manager'; import { createRule } from '../utils/index.js'; import type { RuleContext, RuleFixer } from '../types.js'; import { extractStoreReferences } from './reference-helpers/svelte-store.js'; -import { getScope } from '../utils/ast-utils.js'; - -function findVariableForName( - context: RuleContext, - node: TSESTree.Node, - name: string, - expectedName: string -): { hasConflict: boolean; variable: Variable | null } { - const scope = getScope(context, node); - let variable: Variable | null = null; - - for (const ref of scope.references) { - if (ref.identifier.name === expectedName) { - return { hasConflict: true, variable: null }; - } - } - - for (const v of scope.variables) { - if (v.name === expectedName) { - return { hasConflict: true, variable: null }; - } - if (v.name === name) { - variable = v; - } - } - - return { hasConflict: false, variable }; -} +import { findVariableForReplacement } from '../utils/ast-utils.js'; function createFixer(node: TSESTree.Node, variable: Variable | null, name: string) { return function* fix(fixer: RuleFixer) { @@ -92,7 +65,7 @@ export default createRule('derived-has-same-inputs-outputs', { if (fnParam.type !== 'Identifier') return; const expectedName = `$${args.name}`; if (expectedName !== fnParam.name) { - const { hasConflict, variable } = findVariableForName( + const { hasConflict, variable } = findVariableForReplacement( context, fn.body, fnParam.name, @@ -136,7 +109,7 @@ export default createRule('derived-has-same-inputs-outputs', { if (element && element.type === 'Identifier' && argName) { const expectedName = `$${argName}`; if (expectedName !== element.name) { - const { hasConflict, variable } = findVariableForName( + const { hasConflict, variable } = findVariableForReplacement( context, fn.body, element.name, diff --git a/packages/eslint-plugin-svelte/src/rules/require-store-callbacks-use-set-param.ts b/packages/eslint-plugin-svelte/src/rules/require-store-callbacks-use-set-param.ts index fc12fdfee..6d138c88a 100644 --- a/packages/eslint-plugin-svelte/src/rules/require-store-callbacks-use-set-param.ts +++ b/packages/eslint-plugin-svelte/src/rules/require-store-callbacks-use-set-param.ts @@ -1,5 +1,8 @@ +import { findVariableForReplacement } from '../utils/ast-utils.js'; +import type { SuggestionReportDescriptor } from '../types.js'; import { createRule } from '../utils/index.js'; import { extractStoreReferences } from './reference-helpers/svelte-store.js'; +import { getSourceCode } from '../utils/compat.js'; export default createRule('require-store-callbacks-use-set-param', { meta: { @@ -8,9 +11,12 @@ export default createRule('require-store-callbacks-use-set-param', { category: 'Possible Errors', recommended: false }, + hasSuggestions: true, schema: [], messages: { - unexpected: 'Store callbacks must use `set` param.' + unexpected: 'Store callbacks must use `set` param.', + updateParam: 'Rename parameter from {{oldName}} to `set`.', + addParam: 'Add a `set` parameter.' }, type: 'suggestion' }, @@ -24,10 +30,48 @@ export default createRule('require-store-callbacks-use-set-param', { } const param = fn.params[0]; if (!param || (param.type === 'Identifier' && param.name !== 'set')) { + const { hasConflict, variable } = findVariableForReplacement( + context, + fn.body, + param ? param.name : null, + 'set' + ); + const suggest: SuggestionReportDescriptor[] = []; + if (!hasConflict) { + if (param) { + suggest.push({ + messageId: 'updateParam', + data: { oldName: param.name }, + *fix(fixer) { + yield fixer.replaceText(param, 'set'); + + if (variable) { + for (const ref of variable.references) { + yield fixer.replaceText(ref.identifier, 'set'); + } + } + } + }); + } else { + const token = getSourceCode(context).getTokenBefore(fn.body, { + filter: (token) => token.type === 'Punctuator' && token.value === '(', + includeComments: false + }); + if (token) { + suggest.push({ + messageId: 'addParam', + fix(fixer) { + return fixer.insertTextAfter(token, 'set'); + } + }); + } + } + } context.report({ node: fn, loc: fn.loc, - messageId: 'unexpected' + messageId: 'unexpected', + suggest }); } } diff --git a/packages/eslint-plugin-svelte/src/utils/ast-utils.ts b/packages/eslint-plugin-svelte/src/utils/ast-utils.ts index 1b804baef..02c5fff66 100644 --- a/packages/eslint-plugin-svelte/src/utils/ast-utils.ts +++ b/packages/eslint-plugin-svelte/src/utils/ast-utils.ts @@ -685,3 +685,37 @@ function getSimpleNameFromNode( return getSourceCode(context).getText(node); } + +/** + * Finds the variable for a given name in the specified node's scope. + * Also determines if the replacement name is already in use. + * + * If the `name` is set to null, this assumes you're adding a new variable + * and reports if it is already in use. + */ +export function findVariableForReplacement( + context: RuleContext, + node: TSESTree.Node, + name: string | null, + replacementName: string +): { hasConflict: boolean; variable: Variable | null } { + const scope = getScope(context, node); + let variable: Variable | null = null; + + for (const ref of scope.references) { + if (ref.identifier.name === replacementName) { + return { hasConflict: true, variable: null }; + } + } + + for (const v of scope.variables) { + if (v.name === replacementName) { + return { hasConflict: true, variable: null }; + } + if (v.name === name) { + variable = v; + } + } + + return { hasConflict: false, variable }; +} diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-errors.yaml index 74e27379c..2f0e15716 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-errors.yaml @@ -1,16 +1,198 @@ - message: Store callbacks must use `set` param. line: 4 column: 18 - suggestions: null + suggestions: + - desc: Add a `set` parameter. + messageId: addParam + output: | + - message: Store callbacks must use `set` param. line: 5 column: 18 - suggestions: null + suggestions: + - desc: Rename parameter from foo to `set`. + messageId: updateParam + output: | + - message: Store callbacks must use `set` param. line: 7 column: 18 - suggestions: null + suggestions: + - desc: Add a `set` parameter. + messageId: addParam + output: | + - message: Store callbacks must use `set` param. line: 8 column: 18 + suggestions: + - desc: Rename parameter from foo to `set`. + messageId: updateParam + output: | + +- message: Store callbacks must use `set` param. + line: 10 + column: 18 + suggestions: + - desc: Rename parameter from foo to `set`. + messageId: updateParam + output: | + +- message: Store callbacks must use `set` param. + line: 21 + column: 18 + suggestions: null +- message: Store callbacks must use `set` param. + line: 27 + column: 19 suggestions: null diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-input.svelte b/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-input.svelte index a41201205..e89cab34f 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-input.svelte +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test01-input.svelte @@ -6,4 +6,26 @@ writable(false, () => true); writable(false, (foo) => true); + + readable(false, (foo) => { + foo; + insideACallback(() => { + foo; + }); + conflictingName(() => { + const foo = 303; + foo; + }); + }); + + readable(false, () => { + const set = 303; + }); + + insideACallback(() => { + const set = 303; + readable(false, () => { + set; + }); + }); diff --git a/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test02-errors.yaml b/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test02-errors.yaml index 74e27379c..6f2236ad9 100644 --- a/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test02-errors.yaml +++ b/packages/eslint-plugin-svelte/tests/fixtures/rules/require-store-callbacks-use-set-param/invalid/test02-errors.yaml @@ -1,16 +1,64 @@ - message: Store callbacks must use `set` param. line: 4 column: 18 - suggestions: null + suggestions: + - desc: Add a `set` parameter. + messageId: addParam + output: | + - message: Store callbacks must use `set` param. line: 5 column: 18 - suggestions: null + suggestions: + - desc: Rename parameter from foo to `set`. + messageId: updateParam + output: | + - message: Store callbacks must use `set` param. line: 7 column: 18 - suggestions: null + suggestions: + - desc: Add a `set` parameter. + messageId: addParam + output: | + - message: Store callbacks must use `set` param. line: 8 column: 18 - suggestions: null + suggestions: + - desc: Rename parameter from foo to `set`. + messageId: updateParam + output: | +