Repository: denoland/deno
Pull request: feat(lsp): "typescript.preferences.preferTypeOnlyAutoImports" setting

Closes https://github.com/denoland/vscode_deno/issues/1197.

This reverts the hack implemented in #25662. It turns out there is a setting for this.

Note: this behaviour would no longer be enabled by default. Users relying on it should add `"typescript.preferences.preferTypeOnlyAutoImports": true` to their editor settings. cc @lucacasonato @marvinhagemeister 

Changed files:
filename: cli/lsp/config.rs
patch: @@ -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,

filename: cli/lsp/tsc.rs
patch: @@ -4952,6 +4952,8 @@ pub struct UserPreferences {
   pub auto_import_file_exclude_patterns: Option<Vec<String>>,
   #[serde(skip_serializing_if = "Option::is_none")]
   pub interactive_inlay_hints: Option<bool>,
+  #[serde(skip_serializing_if = "Option::is_none")]
+  pub prefer_type_only_auto_imports: Option<bool>,
 }
 
 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"
     );
   }
 

filename: cli/tsc/99_main_compiler.js
patch: @@ -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<string, LanguageServiceEntry> }} */
   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) {

filename: tests/integration/lsp_tests.rs
patch: @@ -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",
           }],
         }],
       },