From f44bec4ed048207b58e4d489c3ba399309b76d42 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 25 Oct 2024 18:35:09 +0100 Subject: [PATCH] feat(lsp): "typescript.preferences.preferTypeOnlyAutoImports" setting (#26546) --- cli/lsp/config.rs | 5 +++++ cli/lsp/tsc.rs | 7 +++++- cli/tsc/99_main_compiler.js | 39 +++++----------------------------- tests/integration/lsp_tests.rs | 22 +++++++++++++------ 4 files changed, 32 insertions(+), 41 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 74f3583d68..3ffc9e657a 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -439,6 +439,8 @@ pub struct LanguagePreferences { pub use_aliases_for_renames: bool, #[serde(default)] pub quote_style: QuoteStyle, + #[serde(default)] + pub prefer_type_only_auto_imports: bool, } impl Default for LanguagePreferences { @@ -449,6 +451,7 @@ impl Default for LanguagePreferences { auto_import_file_exclude_patterns: vec![], use_aliases_for_renames: true, quote_style: Default::default(), + prefer_type_only_auto_imports: false, } } } @@ -2251,6 +2254,7 @@ mod tests { auto_import_file_exclude_patterns: vec![], use_aliases_for_renames: true, quote_style: QuoteStyle::Auto, + prefer_type_only_auto_imports: false, }, suggest: CompletionSettings { complete_function_calls: false, @@ -2296,6 +2300,7 @@ mod tests { auto_import_file_exclude_patterns: vec![], use_aliases_for_renames: true, quote_style: QuoteStyle::Auto, + prefer_type_only_auto_imports: false, }, suggest: CompletionSettings { complete_function_calls: false, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 0bc7d1600d..0f31d7dd3b 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4952,6 +4952,8 @@ pub struct UserPreferences { pub auto_import_file_exclude_patterns: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub interactive_inlay_hints: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_type_only_auto_imports: Option, } impl UserPreferences { @@ -5074,6 +5076,9 @@ impl UserPreferences { } else { Some(language_settings.preferences.quote_style) }, + prefer_type_only_auto_imports: Some( + language_settings.preferences.prefer_type_only_auto_imports, + ), ..base_preferences } } @@ -6215,7 +6220,7 @@ mod tests { let change = changes.text_changes.first().unwrap(); assert_eq!( change.new_text, - "import type { someLongVariable } from './b.ts'\n" + "import { someLongVariable } from './b.ts'\n" ); } diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index 719f2b9824..e3a0bee597 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -516,7 +516,6 @@ delete Object.prototype.__proto__; /** @typedef {{ * ls: ts.LanguageService & { [k:string]: any }, * compilerOptions: ts.CompilerOptions, - * forceEnabledVerbatimModuleSyntax: boolean, * }} LanguageServiceEntry */ /** @type {{ unscoped: LanguageServiceEntry, byScope: Map }} */ const languageServiceEntries = { @@ -1026,7 +1025,7 @@ delete Object.prototype.__proto__; : ts.sortAndDeduplicateDiagnostics( checkFiles.map((s) => program.getSemanticDiagnostics(s)).flat(), )), - ].filter(filterMapDiagnostic.bind(null, false)); + ].filter(filterMapDiagnostic); // emit the tsbuildinfo file // @ts-ignore: emitBuildInfo is not exposed (https://github.com/microsoft/TypeScript/issues/49871) @@ -1041,28 +1040,11 @@ delete Object.prototype.__proto__; debug("<<< exec stop"); } - /** - * @param {boolean} isLsp - * @param {ts.Diagnostic} diagnostic - */ - function filterMapDiagnostic(isLsp, diagnostic) { + /** @param {ts.Diagnostic} diagnostic */ + function filterMapDiagnostic(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; @@ -1159,12 +1141,10 @@ 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()"); @@ -1230,17 +1210,8 @@ 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, - forceEnabledVerbatimModuleSyntax, - }); + newByScope.set(scope, { ls, compilerOptions }); languageServiceEntries.byScope.delete(scope); } for (const oldEntry of languageServiceEntries.byScope.values()) { @@ -1305,7 +1276,7 @@ delete Object.prototype.__proto__; ...ls.getSemanticDiagnostics(specifier), ...ls.getSuggestionDiagnostics(specifier), ...ls.getSyntacticDiagnostics(specifier), - ].filter(filterMapDiagnostic.bind(null, true))); + ].filter(filterMapDiagnostic)); } return respond(id, diagnosticMap); } catch (e) { diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 35a412b288..79e6dc5c48 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -5955,7 +5955,7 @@ fn lsp_jsr_code_action_missing_declaration() { "character": 6, }, }, - "newText": "import type { ReturnType } from \"jsr:@denotest/types-file/types\";\n", + "newText": "import { ReturnType } from \"jsr:@denotest/types-file/types\";\n", }, { "range": { @@ -6469,6 +6469,16 @@ fn lsp_code_actions_imports() { let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); client.initialize_default(); + client.change_configuration(json!({ + "deno": { + "enable": true, + }, + "typescript": { + "preferences": { + "preferTypeOnlyAutoImports": true, + }, + }, + })); client.did_open(json!({ "textDocument": { "uri": "file:///a/file00.ts", @@ -6768,7 +6778,7 @@ fn lsp_code_actions_imports_dts() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 }, }, - "newText": "import type { SomeType } from \"./decl.d.ts\";\n", + "newText": "import { SomeType } from \"./decl.d.ts\";\n", }], }], }, @@ -7199,7 +7209,7 @@ fn lsp_code_actions_imports_respects_fmt_config() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } }, - "newText": "import type { DuckConfigOptions } from './file01.ts'\n" + "newText": "import { DuckConfigOptions } from './file01.ts'\n" }] }] } @@ -7252,7 +7262,7 @@ fn lsp_code_actions_imports_respects_fmt_config() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 } }, - "newText": "import type { DuckConfigOptions } from './file01.ts'\n" + "newText": "import { DuckConfigOptions } from './file01.ts'\n" }] }] }, @@ -7352,7 +7362,7 @@ fn lsp_quote_style_from_workspace_settings() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 }, }, - "newText": "import type { DuckConfigOptions } from './file01.ts';\n", + "newText": "import { DuckConfigOptions } from './file01.ts';\n", }], }], }, @@ -7396,7 +7406,7 @@ fn lsp_quote_style_from_workspace_settings() { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 0 }, }, - "newText": "import type { DuckConfigOptions } from \"./file01.ts\";\n", + "newText": "import { DuckConfigOptions } from \"./file01.ts\";\n", }], }], },