fix: component import quick-fix with "did you mean" diagnostics (#2373)

jasonlyu123 · web-flow · commit 74098904045a · 2024-05-17T22:54:24.000+02:00
* fix: component import quick-fix when undeclared variable might be a typo

* fix: combine fix sometime affect next document synchronization

* fix-all as well

* wording
diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts
@@ -276,7 +276,8 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
             if (
                 diagnostic.range.start.line < 0 ||
                 diagnostic.range.end.line < 0 ||
-                diagnostic.code !== DiagnosticCode.CANNOT_FIND_NAME
+                (diagnostic.code !== DiagnosticCode.CANNOT_FIND_NAME &&
+                    diagnostic.code !== DiagnosticCode.CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y)
             ) {
                 continue;
             }
@@ -313,10 +314,13 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
         const virtualDoc = new Document(virtualUri, newText);
         virtualDoc.openedByClient = true;
         // let typescript know about the virtual document
-        await this.lsAndTsDocResolver.getSnapshot(virtualDoc);
+        // getLSAndTSDoc instead of getSnapshot so that project dirty state is correctly tracked by us
+        // otherwise, sometime the applied code fix might not be picked up by the language service
+        // because we think the project is still dirty and doesn't update the project version
+        await this.lsAndTsDocResolver.getLSAndTSDoc(virtualDoc);
 
         return {
-            virtualDoc: new Document(virtualUri, newText),
+            virtualDoc,
             insertedNames: names
         };
     }
@@ -559,7 +563,9 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
         const end = tsDoc.offsetAt(tsDoc.getGeneratedPosition(range.end));
         const errorCodes: number[] = context.diagnostics.map((diag) => Number(diag.code));
         const cannotFindNameDiagnostic = context.diagnostics.filter(
-            (diagnostic) => diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME
+            (diagnostic) =>
+                diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME ||
+                diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y
         );
 
         const formatCodeSettings = await this.configManager.getFormatCodeSettingsForFile(
@@ -579,9 +585,14 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
                       formatCodeSettings
                   )
                 : undefined;
-        codeFixes =
-            // either-or situation
-            codeFixes || [
+
+        // either-or situation when it's not a "did you mean" fix
+        if (
+            codeFixes === undefined ||
+            errorCodes.includes(DiagnosticCode.CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y)
+        ) {
+            codeFixes ??= [];
+            codeFixes = codeFixes.concat(
                 ...lang.getCodeFixesAtPosition(
                     tsDoc.filePath,
                     start,
@@ -599,7 +610,8 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
                     userPreferences,
                     formatCodeSettings
                 )
-            ];
+            );
+        }
 
         const snapshots = new SnapshotMap(this.lsAndTsDocResolver);
         snapshots.set(tsDoc.filePath, tsDoc);
@@ -847,7 +859,11 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
             if (codeFix.fixName === FIX_IMPORT_FIX_NAME) {
                 const allCannotFindNameDiagnostics = lang
                     .getSemanticDiagnostics(fileName)
-                    .filter((diagnostic) => diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME);
+                    .filter(
+                        (diagnostic) =>
+                            diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME ||
+                            diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y
+                    );
 
                 if (allCannotFindNameDiagnostics.length < 2) {
                     checkedFixIds.add(codeFix.fixId);
diff --git a/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts b/packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts
@@ -52,6 +52,7 @@ export enum DiagnosticCode {
     MISSING_PROP = 2741, // "Property '..' is missing in type '..' but required in type '..'."
     NO_OVERLOAD_MATCHES_CALL = 2769, // "No overload matches this call"
     CANNOT_FIND_NAME = 2304, // "Cannot find name 'xxx'"
+    CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y = 2552, // "Cannot find name '...' Did you mean '...'?"
     EXPECTED_N_ARGUMENTS = 2554, // Expected {0} arguments, but got {1}.
     DEPRECATED_SIGNATURE = 6387 // The signature '..' of '..' is deprecated
 }
diff --git a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts
@@ -20,6 +20,7 @@ import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDo
 import { __resetCache } from '../../../../src/plugins/typescript/service';
 import { pathToUrl } from '../../../../src/utils';
 import { recursiveServiceWarmup } from '../test-utils';
+import { DiagnosticCode } from '../../../../src/plugins/typescript/features/DiagnosticsProvider';
 
 const testDir = path.join(__dirname, '..');
 const indent = ' '.repeat(4);
@@ -64,7 +65,7 @@ describe('CodeActionsProvider', function () {
             uri: pathToUrl(filePath),
             text: harmonizeNewLines(ts.sys.readFile(filePath) || '')
         });
-        return { provider, document, docManager };
+        return { provider, document, docManager, lsAndTsDocResolver };
     }
 
     it('provides quickfix', async () => {
@@ -661,6 +662,81 @@ describe('CodeActionsProvider', function () {
         ]);
     });
 
+    it('provides quickfix for component import with "did you mean" diagnostics', async () => {
+        const { provider, document } = setup('codeaction-component-import.svelte');
+
+        const codeActions = await provider.getCodeActions(
+            document,
+            Range.create(Position.create(4, 1), Position.create(4, 6)),
+            {
+                diagnostics: [
+                    {
+                        code: 2552,
+                        message: "Cannot find name 'Empty'. Did you mean 'EMpty'?",
+                        range: Range.create(Position.create(4, 1), Position.create(4, 6)),
+                        source: 'ts'
+                    }
+                ],
+                only: [CodeActionKind.QuickFix]
+            }
+        );
+
+        (<TextDocumentEdit>codeActions[0]?.edit?.documentChanges?.[0])?.edits.forEach(
+            (edit) => (edit.newText = harmonizeNewLines(edit.newText))
+        );
+
+        assert.deepStrictEqual(codeActions, <CodeAction[]>[
+            {
+                edit: {
+                    documentChanges: [
+                        {
+                            edits: [
+                                {
+                                    newText: harmonizeNewLines(
+                                        `\n${indent}import Empty from "../empty.svelte";\n`
+                                    ),
+                                    range: {
+                                        end: Position.create(0, 18),
+                                        start: Position.create(0, 18)
+                                    }
+                                }
+                            ],
+                            textDocument: {
+                                uri: getUri('codeaction-component-import.svelte'),
+                                version: null
+                            }
+                        }
+                    ]
+                },
+                kind: 'quickfix',
+                title: 'Add import from "../empty.svelte"'
+            },
+            {
+                edit: {
+                    documentChanges: [
+                        {
+                            edits: [
+                                {
+                                    newText: 'EMpty',
+                                    range: {
+                                        end: Position.create(4, 6),
+                                        start: Position.create(4, 1)
+                                    }
+                                }
+                            ],
+                            textDocument: {
+                                uri: getUri('codeaction-component-import.svelte'),
+                                version: null
+                            }
+                        }
+                    ]
+                },
+                kind: 'quickfix',
+                title: "Change spelling to 'EMpty'"
+            }
+        ]);
+    });
+
     it('remove import inline with script tag', async () => {
         const { provider, document } = setup('remove-imports-inline.svelte');
 
@@ -863,13 +939,15 @@ describe('CodeActionsProvider', function () {
     });
 
     it('provide quick fix to fix all missing import component', async () => {
-        const { provider, document } = setup('codeaction-custom-fix-all-component.svelte');
+        const { provider, document, docManager, lsAndTsDocResolver } = setup(
+            'codeaction-custom-fix-all-component.svelte'
+        );
 
         const range = Range.create(Position.create(4, 1), Position.create(4, 15));
         const codeActions = await provider.getCodeActions(document, range, {
             diagnostics: [
                 {
-                    code: 2304,
+                    code: DiagnosticCode.CANNOT_FIND_NAME,
                     message: "Cannot find name 'FixAllImported'.",
                     range: range,
                     source: 'ts'
@@ -912,6 +990,73 @@ describe('CodeActionsProvider', function () {
                 }
             ]
         });
+
+        // fix-all has some "creative" workaround. Testing if it won't affect the document synchronization after applying the fix
+        docManager.updateDocument(
+            document,
+            resolvedFixAll.edit.documentChanges[0].edits.map((edit) => ({
+                range: edit.range,
+                text: edit.newText
+            }))
+        );
+
+        const { lang, tsDoc } = await lsAndTsDocResolver.getLSAndTSDoc(document);
+        const cannotFindNameDiagnostics = lang
+            .getSemanticDiagnostics(tsDoc.filePath)
+            .filter((diagnostic) => diagnostic.code === DiagnosticCode.CANNOT_FIND_NAME);
+        assert.strictEqual(cannotFindNameDiagnostics.length, 0);
+    });
+
+    it('provide quick fix to fix all missing import component with "did you mean" diagnostics', async () => {
+        const { provider, document } = setup('codeaction-custom-fix-all-component4.svelte');
+
+        const range = Range.create(Position.create(4, 1), Position.create(4, 15));
+        const codeActions = await provider.getCodeActions(document, range, {
+            diagnostics: [
+                {
+                    code: DiagnosticCode.CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y,
+                    message: "Cannot find name 'FixAllImported'. Did you mean 'FixAllImported3'?",
+                    range: range,
+                    source: 'ts'
+                }
+            ],
+            only: [CodeActionKind.QuickFix]
+        });
+
+        const fixAll = codeActions.find((action) => action.data);
+        const resolvedFixAll = await provider.resolveCodeAction(document, fixAll!);
+
+        (<TextDocumentEdit>resolvedFixAll?.edit?.documentChanges?.[0])?.edits.forEach(
+            (edit) => (edit.newText = harmonizeNewLines(edit.newText))
+        );
+
+        assert.deepStrictEqual(resolvedFixAll.edit, {
+            documentChanges: [
+                {
+                    edits: [
+                        {
+                            newText:
+                                `\n${indent}import FixAllImported from \"./importing/FixAllImported.svelte\";\n` +
+                                `${indent}import FixAllImported2 from \"./importing/FixAllImported2.svelte\";\n`,
+                            range: {
+                                start: {
+                                    character: 18,
+                                    line: 0
+                                },
+                                end: {
+                                    character: 18,
+                                    line: 0
+                                }
+                            }
+                        }
+                    ],
+                    textDocument: {
+                        uri: getUri('codeaction-custom-fix-all-component4.svelte'),
+                        version: null
+                    }
+                }
+            ]
+        });
     });
 
     it('provide quick fix to fix all missing import component without duplicate (script)', async () => {
diff --git a/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-component-import.svelte b/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-component-import.svelte
@@ -0,0 +1,5 @@
+<script lang="ts">
+    class EMpty {}
+</script>
+
+<Empty />
diff --git a/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-custom-fix-all-component4.svelte b/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-custom-fix-all-component4.svelte
@@ -0,0 +1,6 @@
+<script lang="ts">
+class FixAllImported3 {}
+</script>
+
+<FixAllImported />
+<FixAllImported2 />

Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@ export enum DiagnosticCode {`
`52`	`52`	`MISSING_PROP = 2741, // "Property '..' is missing in type '..' but required in type '..'."`
`53`	`53`	`NO_OVERLOAD_MATCHES_CALL = 2769, // "No overload matches this call"`
`54`	`54`	`CANNOT_FIND_NAME = 2304, // "Cannot find name 'xxx'"`
	`55`	`+ CANNOT_FIND_NAME_X_DID_YOU_MEAN_Y = 2552, // "Cannot find name '...' Did you mean '...'?"`
`55`	`56`	`EXPECTED_N_ARGUMENTS = 2554, // Expected {0} arguments, but got {1}.`
`56`	`57`	`DEPRECATED_SIGNATURE = 6387 // The signature '..' of '..' is deprecated`
`57`	`58`	`}`