Skip to content

Commit 9504e6b

Browse files
committed
feat: autofixer action -> attachment
1 parent 7086e8e commit 9504e6b

File tree

4 files changed

+187
-48
lines changed

4 files changed

+187
-48
lines changed

src/lib/mcp/autofixers/add-autofixers-issues.test.ts

Lines changed: 139 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -375,37 +375,38 @@ describe('add_autofixers_issues', () => {
375375
});
376376
});
377377

378-
describe('bind_this_attachment', () => {
379-
it('should add suggestions when using bind:this on an element', () => {
380-
const content = run_autofixers_on_code(`
378+
describe('suggest_attachments', () => {
379+
describe('bind:this', () => {
380+
it('should add suggestions when using bind:this on an element', () => {
381+
const content = run_autofixers_on_code(`
381382
<script>
382383
let a = $state();
383384
</script>
384385
385386
<a bind:this={a} />`);
386387

387-
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
388-
expect(content.suggestions).toContain(
389-
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
390-
);
391-
});
388+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
389+
expect(content.suggestions).toContain(
390+
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
391+
);
392+
});
392393

393-
it('should not add suggestions when using bind:this on a component', () => {
394-
const content = run_autofixers_on_code(`
394+
it('should not add suggestions when using bind:this on a component', () => {
395+
const content = run_autofixers_on_code(`
395396
<script>
396397
import Child from './Child.svelte';
397398
let a = $state();
398399
</script>
399400
400401
<Child bind:this={a} />`);
401402

402-
expect(content.suggestions).not.toContain(
403-
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
404-
);
405-
});
403+
expect(content.suggestions).not.toContain(
404+
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
405+
);
406+
});
406407

407-
it('should not add suggestions when using bind:this on a component nested in an element', () => {
408-
const content = run_autofixers_on_code(`
408+
it('should not add suggestions when using bind:this on a component nested in an element', () => {
409+
const content = run_autofixers_on_code(`
409410
<script>
410411
import Child from './Child.svelte';
411412
let a = $state();
@@ -415,26 +416,135 @@ describe('add_autofixers_issues', () => {
415416
<Child bind:this={a} />
416417
</div>`);
417418

418-
expect(content.suggestions).not.toContain(
419-
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
420-
);
421-
});
419+
expect(content.suggestions).not.toContain(
420+
'The usage of `bind:this` can often be replaced with an easier to read `action` or even better an `attachment`. Consider using the latter if possible.',
421+
);
422+
});
422423

423-
it('should add suggestions but not suggest attachments when using bind:this on an element and the desired svelte version is 4', () => {
424-
const content = run_autofixers_on_code(
425-
`
424+
it('should add suggestions but not suggest attachments when using bind:this on an element and the desired svelte version is 4', () => {
425+
const content = run_autofixers_on_code(
426+
`
426427
<script>
427428
let a;
428429
</script>
429430
430431
<a bind:this={a} />`,
431-
4,
432-
);
432+
4,
433+
);
433434

434-
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
435-
expect(content.suggestions).toContain(
436-
'The usage of `bind:this` can often be replaced with an easier to read `action`. Consider using the latter if possible.',
437-
);
435+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
436+
expect(content.suggestions).toContain(
437+
'The usage of `bind:this` can often be replaced with an easier to read `action`. Consider using the latter if possible.',
438+
);
439+
});
440+
});
441+
442+
describe('use:', () => {
443+
it('should add suggestions when using use: on an element and the action is declared as a function', () => {
444+
const content = run_autofixers_on_code(
445+
`<script>
446+
function my_action(node) {
447+
// do something with the node
448+
}
449+
</script>
450+
451+
<a use:my_action />`,
452+
);
453+
454+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
455+
expect(content.suggestions).toContain(
456+
'Consider using an `attachment` instead of an `action` for "my_action".',
457+
);
458+
});
459+
460+
it('should add suggestions when using use: on an element and the action is declared as a variable', () => {
461+
const content = run_autofixers_on_code(
462+
`<script>
463+
const my_action = (node) => {
464+
// do something with the node
465+
}
466+
</script>
467+
468+
<a use:my_action />`,
469+
);
470+
471+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
472+
expect(content.suggestions).toContain(
473+
'Consider using an `attachment` instead of an `action` for "my_action".',
474+
);
475+
});
476+
477+
it('should add suggestions when using use: on an element and the action is declared as an object', () => {
478+
const content = run_autofixers_on_code(
479+
`<script>
480+
const my_action = {
481+
action: (node) => {
482+
// do something with the node
483+
}
484+
};
485+
</script>
486+
487+
<a use:my_action.action />`,
488+
);
489+
490+
expect(content.suggestions.length).toBeGreaterThanOrEqual(1);
491+
expect(content.suggestions).toContain(
492+
'Consider using an `attachment` instead of an `action` for "my_action".',
493+
);
494+
});
495+
496+
it('should not add suggestions when using use: on an element and the desired svelte version is 4', () => {
497+
const content = run_autofixers_on_code(
498+
`<script>
499+
function my_action(node) {
500+
// do something with the node
501+
}
502+
</script>
503+
504+
<a use:my_action />`,
505+
4,
506+
);
507+
508+
expect(content.suggestions).not.toContain(
509+
'Consider using an `attachment` instead of an `action` for "my_action".',
510+
);
511+
});
512+
513+
it('should not add suggestions when using use: on an element and the action comes from an import', () => {
514+
const content = run_autofixers_on_code(
515+
`<script>
516+
import { my_action } from './actions.js';
517+
</script>
518+
519+
<a use:my_action />`,
520+
);
521+
522+
expect(content.suggestions).not.toContain(
523+
'Consider using an `attachment` instead of an `action` for "my_action".',
524+
);
525+
});
526+
527+
it('should not add suggestions when using use: on an element and the action comes from the props', () => {
528+
const content = run_autofixers_on_code(
529+
`<script>
530+
const { my_action } = $props();
531+
</script>
532+
533+
<a use:my_action />`,
534+
);
535+
536+
expect(content.suggestions).not.toContain(
537+
'Consider using an `attachment` instead of an `action` for "my_action".',
538+
);
539+
});
540+
541+
it('should not add suggestions when using use: on an element and the action comes from a global variable', () => {
542+
const content = run_autofixers_on_code(`<a use:my_action />`);
543+
544+
expect(content.suggestions).not.toContain(
545+
'Consider using an `attachment` instead of an `action` for "my_action".',
546+
);
547+
});
438548
});
439549
});
440550
});

src/lib/mcp/autofixers/visitors/bind-this-attachment.ts

Lines changed: 0 additions & 18 deletions
This file was deleted.

src/lib/mcp/autofixers/visitors/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,4 @@ export * from './set-or-update-state.js';
1616
export * from './imported-runes.js';
1717
export * from './derived-with-function.js';
1818
export * from './use-runes-instead-of-store.js';
19-
export * from './bind-this-attachment.js';
19+
export * from './suggest-attachments.js';
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import type { Identifier } from 'estree';
2+
import type { Autofixer } from '.';
3+
import { left_most_id } from '../ast/utils.js';
4+
5+
export const suggest_attachments: Autofixer = {
6+
SvelteDirective(node, { state, next, path }) {
7+
if (node.kind === 'Binding' && node.key.name.name === 'this') {
8+
const parent_element = path.findLast((p) => p.type === 'SvelteElement');
9+
if (parent_element?.kind === 'html' && parent_element.startTag.attributes.includes(node)) {
10+
let better_an_attachment = ` or even better an \`attachment\``;
11+
if (state.desired_svelte_version === 4) {
12+
better_an_attachment = ``;
13+
}
14+
state.output.suggestions.push(
15+
`The usage of \`bind:this\` can often be replaced with an easier to read \`action\`${better_an_attachment}. Consider using the latter if possible.`,
16+
);
17+
}
18+
} else if (node.kind === 'Action' && state.desired_svelte_version === 5) {
19+
let id: Identifier | null = null;
20+
if (node.key.name.type === 'Identifier') {
21+
id = node.key.name;
22+
} else if (node.key.name.type === 'MemberExpression') {
23+
id = left_most_id(node.key.name);
24+
}
25+
if (id) {
26+
const reference = state.parsed.find_reference_by_id(id);
27+
const definition = reference?.resolved?.defs[0];
28+
console.log(definition);
29+
if (
30+
definition &&
31+
(definition.type === 'Variable' ||
32+
!(definition.type === 'ImportBinding' || definition.type === 'Parameter')) &&
33+
!(
34+
definition.type === 'Variable' &&
35+
definition.node.init?.type === 'CallExpression' &&
36+
state.parsed.is_rune(definition.node.init, ['$props'])
37+
)
38+
) {
39+
state.output.suggestions.push(
40+
`Consider using an \`attachment\` instead of an \`action\` for "${id.name}".`,
41+
);
42+
}
43+
}
44+
}
45+
next();
46+
},
47+
};

0 commit comments

Comments
 (0)