diff --git a/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts b/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts index c606592..0629f1d 100644 --- a/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts +++ b/packages/mcp-server/src/mcp/autofixers/add-autofixers-issues.test.ts @@ -117,6 +117,36 @@ describe('add_autofixers_issues', () => { ); }); }); + + it('should add a suggestion when calling a function inside an effect', () => { + const content = run_autofixers_on_code(` + `); + + expect(content.suggestions.length).toBeGreaterThanOrEqual(1); + expect(content.suggestions).toContain( + `You are calling the function \`fetch_data\` inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + }); + + it('should add a suggestion when calling a function inside an effect (with non identifier callee)', () => { + const content = run_autofixers_on_code(` + `); + + expect(content.suggestions.length).toBeGreaterThanOrEqual(1); + expect(content.suggestions).toContain( + `You are calling a function inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + }); }); with_possible_inits('($init)', ({ init }) => { diff --git a/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts b/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts index e2e4894..eb1cc59 100644 --- a/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts +++ b/packages/mcp-server/src/mcp/autofixers/visitors/assign-in-effect.ts @@ -1,4 +1,10 @@ -import type { AssignmentExpression, Identifier, Node, UpdateExpression } from 'estree'; +import type { + AssignmentExpression, + CallExpression, + Identifier, + Node, + UpdateExpression, +} from 'estree'; import type { Autofixer, AutofixerState } from './index.js'; import { left_most_id } from '../ast/utils.js'; import type { AST } from 'svelte-eslint-parser'; @@ -27,9 +33,9 @@ function run_if_in_effect( } } -function visitor( +function assign_or_update_visitor( node: UpdateExpression | AssignmentExpression, - { state, path }: Context, + { state, path, next }: Context, ) { run_if_in_effect(path, state, () => { function check_if_stateful_id(id: Identifier) { @@ -58,9 +64,25 @@ function visitor( } } }); + next(); +} + +function call_expression_visitor( + node: CallExpression, + { state, path, next }: Context, +) { + run_if_in_effect(path, state, () => { + const function_name = + node.callee.type === 'Identifier' ? `the function \`${node.callee.name}\`` : 'a function'; + state.output.suggestions.push( + `You are calling ${function_name} inside an $effect. Please check if the function is reassigning a stateful variable because that's considered malpractice and check if it could use \`$derived\` instead. Ignore this suggestion if you are sure this function is not assigning any stateful variable or if you can't check if it does.`, + ); + }); + next(); } export const assign_in_effect: Autofixer = { - UpdateExpression: visitor, - AssignmentExpression: visitor, + UpdateExpression: assign_or_update_visitor, + AssignmentExpression: assign_or_update_visitor, + CallExpression: call_expression_visitor, };