Skip to content

feat: add set or update stateful variables autofixer & imported runes autofixer #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 18, 2025
Merged

feat: add set or update stateful variables autofixer & imported runes autofixer #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72e89af
feat: add `set` or `update` stateful variables autofixer
paoloricciuti Sep 18, 2025
4c8cadd
feat: add imported runes autofixer
paoloricciuti Sep 18, 2025
13719d0
feat: add derived with functions suggestions
paoloricciuti Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export const base_runes = [
'$state',
'$effect',
'$derived',
'$inspect',
'$props',
'$bindable',
'$host',
] as const;

export const nested_runes = [
'$state.raw',
'$state.snapshot',
'$effect.pre',
'$effect.tracking',
'$effect.pending',
'$effect.root',
'$derived.by',
'$inspect.trace',
'$props.id',
] as const;

export const runes = [...base_runes, ...nested_runes] as const;
294 changes: 264 additions & 30 deletions src/lib/mcp/autofixers/add-autofixers-issues.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,42 @@
import { describe, expect, it } from 'vitest';
import { add_autofixers_issues } from './add-autofixers-issues';
import { add_autofixers_issues } from './add-autofixers-issues.js';
import { base_runes } from '../../constants.js';

const dollarless_runes = base_runes.map((r) => ({ rune: r.replace('$', '') }));

function run_autofixers_on_code(code: string, desired_svelte_version = 5) {
const content = { issues: [], suggestions: [] };
add_autofixers_issues(content, code, desired_svelte_version);
return content;
}

describe('add_autofixers_issues', () => {
describe('assign_in_effect', () => {
it('should add suggestions when assigning to a stateful variable inside an effect', () => {
const content = { issues: [], suggestions: [] };
const code = `
it(`should add suggestions when assigning to a stateful variable inside an effect`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state(0);
$effect(() => {
count = 43;
});
</script>`;
add_autofixers_issues(content, code, 5);
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
);
});

it('should add a suggestion for each variable assigned within an effect', () => {
const content = { issues: [], suggestions: [] };
const code = `
it(`should add a suggestion for each variable assigned within an effect`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state(0);
const count2 = $state(0);
$effect(() => {
count = 43;
count2 = 44;
});
</script>`;
add_autofixers_issues(content, code, 5);
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(2);
expect(content.suggestions).toContain(
Expand All @@ -41,73 +46,302 @@ describe('add_autofixers_issues', () => {
'The stateful variable "count2" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
);
});
it('should not add a suggestion for variables that are not assigned within an effect', () => {
const content = { issues: [], suggestions: [] };
const code = `
it(`should not add a suggestion for variables that are not assigned within an effect`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state(0);
</script>

<button onclick={() => count = 43}>Increment</button>
`;
add_autofixers_issues(content, code, 5);
`);

expect(content.suggestions).not.toContain(
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
);
});

it("should not add a suggestions for variables that are assigned within an effect but aren't stateful", () => {
const content = { issues: [], suggestions: [] };
const code = `
const content = run_autofixers_on_code(`
<script>
const count = 0;

$effect(() => {
count = 43;
});
</script>`;
add_autofixers_issues(content, code, 5);
</script>`);

expect(content.suggestions).not.toContain(
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
);
});

it('should add a suggestion for variables that are assigned within an effect with an update', () => {
const content = { issues: [], suggestions: [] };
const code = `
it(`should add a suggestion for variables that are assigned within an effect with an update`, () => {
const content = run_autofixers_on_code(`
<script>
let count = $state(0);

$effect(() => {
count++;
});
</script>
`;
add_autofixers_issues(content, code, 5);
`);

expect(content.suggestions).toContain(
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
);
});

it('should add a suggestion for variables that are mutated within an effect', () => {
const content = { issues: [], suggestions: [] };
const code = `
it(`should add a suggestion for variables that are mutated within an effect`, () => {
const content = run_autofixers_on_code(`
<script>
let count = $state({ value: 0 });

$effect(() => {
count.value = 42;
});
</script>
`;
add_autofixers_issues(content, code, 5);
`);

expect(content.suggestions).toContain(
'The stateful variable "count" is assigned inside an $effect which is generally consider a malpractice. Consider using $derived if possible.',
);
});
});

describe.each([{ method: 'set' }, { method: 'update' }])(
'set_or_update_state ($method)',
({ method }) => {
it(`should add suggestions when using .${method}() on a stateful variable with a literal init`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state(0);
function update_count() {
count.${method}(43);
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them.`,
);
});

it(`should add suggestions when using .${method}() on a stateful variable with an array init`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state([0]);
function update_count() {
count.${method}([1]);
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them.`,
);
});

it(`should add suggestions when using .${method}() on a stateful variable with conditional if it's not sure if the method could actually be present on the variable ($state({}))`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state({ value: 0 });
function update_count() {
count.${method}({ value: 43 });
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
);
});

it(`should add suggestions when using .${method}() on a stateful variable with conditional if it's not sure if the method could actually be present on the variable ($state(new Class()))`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state(new Class());
function update_count() {
count.${method}(new Class());
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
);
});

it(`should add suggestions when using .${method}() on a stateful variable with conditional if it's not sure if the method could actually be present on the variable ($state(variable_name))`, () => {
const content = run_autofixers_on_code(`
<script>
const { init } = $props();
const count = $state(init);
function update_count() {
count.${method}(43);
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
);
});

it(`should not add suggestions when using .${method} on a stateful variable if it's not a method call`, () => {
const content = run_autofixers_on_code(`
<script>
const count = $state({});
function update_count() {
console.log(count.${method});
}
</script>`);

expect(content.suggestions).not.toContain(
`You are trying to update the stateful variable "count" using "${method}". stateful variables should be updated with a normal assignment/mutation, do not use methods to update them. However I can't verify if "count" is a state variable of an object or a class with a "${method}" method on it. Please verify that before updating the code to use a normal assignment`,
);
});
},
);

describe('imported_runes', () => {
describe.each([{ source: 'svelte' }, { source: 'svelte/runes' }])(
'from "$source"',
({ source }) => {
describe.each(dollarless_runes)('single import ($rune)', ({ rune }) => {
it(`should add suggestions when importing '${rune}' from '${source}'`, () => {
const content = run_autofixers_on_code(`
<script>
import { ${rune} } from '${source}';
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are importing "${rune}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${rune}" directly.`,
);
});

it(`should add suggestions when importing "${rune}" as the default export from '${source}'`, () => {
const content = run_autofixers_on_code(`
<script>
import ${rune} from '${source}';
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are importing "${rune}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${rune}" directly.`,
);
});

it(`should add suggestions when importing '${rune}' as the namespace export from '${source}'`, () => {
const content = run_autofixers_on_code(`
<script>
import * as ${rune} from '${source}';
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
`You are importing "${rune}" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$${rune}" directly.`,
);
});
});

it(`should add suggestions when importing multiple runes from '${source}'`, () => {
const content = run_autofixers_on_code(`
<script>
import { onMount, state, effect } from '${source}';
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(2);
expect(content.suggestions).toContain(
`You are importing "state" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$state" directly.`,
);
expect(content.suggestions).toContain(
`You are importing "effect" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$effect" directly.`,
);
});

it(`should not add suggestions when importing other identifiers from '${source}'`, () => {
const content = run_autofixers_on_code(`
<script>
import { onMount } from '${source}';
</script>`);

expect(content.suggestions).not.toContain(
`You are importing "onMount" from "${source}". This is not necessary, all runes are globally available. Please remove this import and use "$onMount" directly.`,
);
});
},
);
});

describe('derived_with_function', () => {
it(`should add suggestions when using a function as the first argument to $derived`, () => {
const content = run_autofixers_on_code(`
<script>
const value = $derived(() => {
return 43;
});
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
'You are passing a function to $derived when declaring "value" but $derived expects an expression. You can use $derived.by instead.',
);
});

it(`should add suggestions when using a function as the first argument to $derived in classes`, () => {
const content = run_autofixers_on_code(`
<script>
class Double {
value = $derived(() => 43);
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
'You are passing a function to $derived when declaring "value" but $derived expects an expression. You can use $derived.by instead.',
);
});

it(`should add suggestions when using a function as the first argument to $derived in classes constructors`, () => {
const content = run_autofixers_on_code(`
<script>
class Double {
value;

constructor(){
this.value = $derived(function() { return 44; });
}
}
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
'You are passing a function to $derived when declaring "value" but $derived expects an expression. You can use $derived.by instead.',
);
});

it(`should add suggestions when using a function as the first argument to $derived without the declaring part if it's not an identifier`, () => {
const content = run_autofixers_on_code(`
<script>
const { destructured } = $derived(() => 43);
</script>`);

expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
expect(content.suggestions).toContain(
'You are passing a function to $derived but $derived expects an expression. You can use $derived.by instead.',
);
});

it(`should add suggestions when using a function as the first argument to $derived.by`, () => {
const content = run_autofixers_on_code(`
<script>
const { destructured } = $derived.by(() => 43);
</script>`);

expect(content.suggestions).not.toContain(
'You are passing a function to $derived but $derived expects an expression. You can use $derived.by instead.',
);
});
});
});
Loading