From d4a06251c54fc004e189469e493b1261be200300 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 17 Sep 2024 18:28:51 +0100 Subject: [PATCH] feat(lsp): auto-import types with 'import type' (#25662) --- cli/lsp/tsc.rs | 2 +- cli/tsc/99_main_compiler.js | 39 +++++++++++++++++++--- tests/integration/lsp_tests.rs | 61 ++++++---------------------------- 3 files changed, 46 insertions(+), 56 deletions(-) diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 0decef3960..6d3e1317b1 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -6154,7 +6154,7 @@ mod tests { let change = changes.text_changes.first().unwrap(); assert_eq!( change.new_text, - "import { someLongVariable } from './b.ts'\n" + "import type { someLongVariable } from './b.ts'\n" ); } diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index 7fac8cd351..c5769168fa 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -516,6 +516,7 @@ delete Object.prototype.__proto__; /** @typedef {{ * ls: ts.LanguageService & { [k:string]: any }, * compilerOptions: ts.CompilerOptions, + * forceEnabledVerbatimModuleSyntax: boolean, * }} LanguageServiceEntry */ /** @type {{ unscoped: LanguageServiceEntry, byScope: Map }} */ const languageServiceEntries = { @@ -1025,7 +1026,7 @@ delete Object.prototype.__proto__; : ts.sortAndDeduplicateDiagnostics( checkFiles.map((s) => program.getSemanticDiagnostics(s)).flat(), )), - ].filter(filterMapDiagnostic); + ].filter(filterMapDiagnostic.bind(null, false)); // emit the tsbuildinfo file // @ts-ignore: emitBuildInfo is not exposed (https://github.com/microsoft/TypeScript/issues/49871) @@ -1040,11 +1041,28 @@ delete Object.prototype.__proto__; debug("<<< exec stop"); } - /** @param {ts.Diagnostic} diagnostic */ - function filterMapDiagnostic(diagnostic) { + /** + * @param {boolean} isLsp + * @param {ts.Diagnostic} diagnostic + */ + function filterMapDiagnostic(isLsp, diagnostic) { if (IGNORED_DIAGNOSTICS.includes(diagnostic.code)) { return false; } + if (isLsp) { + // TS1484: `...` is a type and must be imported using a type-only import when 'verbatimModuleSyntax' is enabled. + // We force-enable `verbatimModuleSyntax` in the LSP so the `type` + // modifier is used when auto-importing types. But we don't want this + // diagnostic unless it was explicitly enabled by the user. + if (diagnostic.code == 1484) { + const entry = (lastRequestScope + ? languageServiceEntries.byScope.get(lastRequestScope) + : null) ?? languageServiceEntries.unscoped; + if (entry.forceEnabledVerbatimModuleSyntax) { + return false; + } + } + } // make the diagnostic for using an `export =` in an es module a warning if (diagnostic.code === 1203) { diagnostic.category = ts.DiagnosticCategory.Warning; @@ -1140,10 +1158,12 @@ delete Object.prototype.__proto__; "strict": true, "target": "esnext", "useDefineForClassFields": true, + "verbatimModuleSyntax": true, "jsx": "react", "jsxFactory": "React.createElement", "jsxFragmentFactory": "React.Fragment", }), + forceEnabledVerbatimModuleSyntax: true, }; setLogDebug(enableDebugLogging, "TSLS"); debug("serverInit()"); @@ -1209,8 +1229,17 @@ delete Object.prototype.__proto__; const ls = oldEntry ? oldEntry.ls : ts.createLanguageService(host, documentRegistry); + let forceEnabledVerbatimModuleSyntax = false; + if (!config["verbatimModuleSyntax"]) { + config["verbatimModuleSyntax"] = true; + forceEnabledVerbatimModuleSyntax = true; + } const compilerOptions = lspTsConfigToCompilerOptions(config); - newByScope.set(scope, { ls, compilerOptions }); + newByScope.set(scope, { + ls, + compilerOptions, + forceEnabledVerbatimModuleSyntax, + }); languageServiceEntries.byScope.delete(scope); } for (const oldEntry of languageServiceEntries.byScope.values()) { @@ -1275,7 +1304,7 @@ delete Object.prototype.__proto__; ...ls.getSemanticDiagnostics(specifier), ...ls.getSuggestionDiagnostics(specifier), ...ls.getSyntacticDiagnostics(specifier), - ].filter(filterMapDiagnostic)); + ].filter(filterMapDiagnostic.bind(null, true))); } return respond(id, diagnosticMap); } catch (e) { diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index b6cc71ec62..095cbee98e 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -5659,7 +5659,7 @@ fn lsp_jsr_code_action_missing_declaration() { "character": 6, }, }, - "newText": "import { ReturnType } from \"jsr:@denotest/types-file/types\";\n", + "newText": "import type { ReturnType } from \"jsr:@denotest/types-file/types\";\n", }, { "range": { @@ -6150,7 +6150,7 @@ export class DuckConfig { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } }, - "newText": "import { DuckConfigOptions } from \"./file02.ts\";\n\n" + "newText": "import type { DuckConfigOptions } from \"./file02.ts\";\n\n" }] }] } @@ -6266,7 +6266,7 @@ export class DuckConfig { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } }, - "newText": "import { DuckConfig } from \"./file01.ts\";\nimport { DuckConfigOptions } from \"./file02.ts\";\n\n" + "newText": "import { DuckConfig } from \"./file01.ts\";\nimport type { DuckConfigOptions } from \"./file02.ts\";\n\n" }] }] }, @@ -6343,7 +6343,7 @@ fn lsp_code_actions_imports_dts() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 }, }, - "newText": "import { SomeType } from \"./decl.d.ts\";\n", + "newText": "import type { SomeType } from \"./decl.d.ts\";\n", }], }], }, @@ -6663,7 +6663,7 @@ fn lsp_code_actions_imports_respects_fmt_config() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } }, - "newText": "import { DuckConfigOptions } from './file01.ts'\n" + "newText": "import type { DuckConfigOptions } from './file01.ts'\n" }] }] } @@ -6716,7 +6716,7 @@ fn lsp_code_actions_imports_respects_fmt_config() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } }, - "newText": "import { DuckConfigOptions } from './file01.ts'\n" + "newText": "import type { DuckConfigOptions } from './file01.ts'\n" }] }] }, @@ -6816,7 +6816,7 @@ fn lsp_quote_style_from_workspace_settings() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 }, }, - "newText": "import { DuckConfigOptions } from './file01.ts';\n", + "newText": "import type { DuckConfigOptions } from './file01.ts';\n", }], }], }, @@ -6860,7 +6860,7 @@ fn lsp_quote_style_from_workspace_settings() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 }, }, - "newText": "import { DuckConfigOptions } from \"./file01.ts\";\n", + "newText": "import type { DuckConfigOptions } from \"./file01.ts\";\n", }], }], }, @@ -7246,12 +7246,12 @@ fn lsp_completions_auto_import() { "uri": "file:///a/file.ts", "languageId": "typescript", "version": 1, - "text": "export {};\n\n", + "text": "const result = add(1, 2);\n", } })); let list = client.get_completion_list( "file:///a/file.ts", - (2, 0), + (0, 18), json!({ "triggerKind": 1 }), ); assert!(!list.is_incomplete); @@ -7259,46 +7259,7 @@ fn lsp_completions_auto_import() { let Some(item) = item else { panic!("completions items missing 'add' symbol"); }; - let mut item_value = serde_json::to_value(item).unwrap(); - item_value["data"]["tsc"]["data"]["exportMapKey"] = - serde_json::Value::String("".to_string()); - - let req = json!({ - "label": "add", - "labelDetails": { - "description": "./🦕.ts", - }, - "kind": 3, - "sortText": "￿16_0", - "commitCharacters": [ - ".", - ",", - ";", - "(" - ], - "data": { - "tsc": { - "specifier": "file:///a/file.ts", - "position": 12, - "name": "add", - "source": "./%F0%9F%A6%95.ts", - "specifierRewrite": [ - "./%F0%9F%A6%95.ts", - "./🦕.ts", - ], - "data": { - "exportName": "add", - "exportMapKey": "", - "moduleSpecifier": "./%F0%9F%A6%95.ts", - "fileName": "file:///a/%F0%9F%A6%95.ts" - }, - "useCodeSnippet": false - } - } - }); - assert_eq!(item_value, req); - - let res = client.write_request("completionItem/resolve", req); + let res = client.write_request("completionItem/resolve", json!(item)); assert_eq!( res, json!({