Add user preference for preferring type-only auto imports (#56090)

andrewbranch · web-flow · commit aef4e468d9bd · 2023-10-20T14:52:18.000-07:00
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -10014,6 +10014,7 @@ export interface UserPreferences {
     readonly interactiveInlayHints?: boolean;
     readonly allowRenameOfImportPath?: boolean;
     readonly autoImportFileExcludePatterns?: string[];
+    readonly preferTypeOnlyAutoImports?: boolean;
     readonly organizeImportsIgnoreCase?: "auto" | boolean;
     readonly organizeImportsCollation?: "ordinal" | "unicode";
     readonly organizeImportsLocale?: string;
diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts
@@ -3347,12 +3347,16 @@ export class TestState {
         this.verifyRangeIs(expectedText, includeWhiteSpace);
     }
 
-    public verifyCodeFixAll({ fixId, fixAllDescription, newFileContent, commands: expectedCommands }: FourSlashInterface.VerifyCodeFixAllOptions): void {
-        const fixWithId = ts.find(this.getCodeFixes(this.activeFile.fileName), a => a.fixId === fixId);
+    public verifyCodeFixAll({ fixId, fixAllDescription, newFileContent, commands: expectedCommands, preferences }: FourSlashInterface.VerifyCodeFixAllOptions): void {
+        if (this.testType === FourSlashTestType.Server && preferences) {
+            this.configure(preferences);
+        }
+
+        const fixWithId = ts.find(this.getCodeFixes(this.activeFile.fileName, /*errorCode*/ undefined, preferences), a => a.fixId === fixId);
         ts.Debug.assert(fixWithId !== undefined, "No available code fix has the expected id. Fix All is not available if there is only one potentially fixable diagnostic present.", () => `Expected '${fixId}'. Available actions:\n${ts.mapDefined(this.getCodeFixes(this.activeFile.fileName), a => `${a.fixName} (${a.fixId || "no fix id"})`).join("\n")}`);
         ts.Debug.assertEqual(fixWithId.fixAllDescription, fixAllDescription);
 
-        const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings, ts.emptyOptions);
+        const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings, preferences || ts.emptyOptions);
         assert.deepEqual<readonly {}[] | undefined>(commands, expectedCommands);
         this.verifyNewContent({ newFileContent }, changes);
     }
diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts
@@ -1869,6 +1869,7 @@ export interface VerifyCodeFixAllOptions {
     fixAllDescription: string;
     newFileContent: NewFileContent;
     commands: readonly {}[];
+    preferences?: ts.UserPreferences;
 }
 
 export interface VerifyRefactorOptions {
diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
@@ -389,6 +389,7 @@ function createImportAdderWorker(sourceFile: SourceFile, program: Program, useAu
                 namedImports && arrayFrom(namedImports.entries(), ([name, addAsTypeOnly]) => ({ addAsTypeOnly, name })),
                 namespaceLikeImport,
                 compilerOptions,
+                preferences,
             );
             newDeclarations = combine(newDeclarations, declarations);
         });
@@ -1348,6 +1349,7 @@ function codeActionForFixWorker(
                     namedImports,
                     namespaceLikeImport,
                     program.getCompilerOptions(),
+                    preferences,
                 ),
                 /*blankLineBetween*/ true,
                 preferences,
@@ -1505,7 +1507,7 @@ function doAddExistingFix(
         const newSpecifiers = stableSort(
             namedImports.map(namedImport =>
                 factory.createImportSpecifier(
-                    (!clause.isTypeOnly || promoteFromTypeOnly) && needsTypeOnly(namedImport),
+                    (!clause.isTypeOnly || promoteFromTypeOnly) && shouldUseTypeOnly(namedImport, preferences),
                     /*propertyName*/ undefined,
                     factory.createIdentifier(namedImport.name),
                 )
@@ -1604,32 +1606,37 @@ function needsTypeOnly({ addAsTypeOnly }: { addAsTypeOnly: AddAsTypeOnly; }): bo
     return addAsTypeOnly === AddAsTypeOnly.Required;
 }
 
+function shouldUseTypeOnly(info: { addAsTypeOnly: AddAsTypeOnly; }, preferences: UserPreferences): boolean {
+    return needsTypeOnly(info) || !!preferences.preferTypeOnlyAutoImports && info.addAsTypeOnly !== AddAsTypeOnly.NotAllowed;
+}
+
 function getNewImports(
     moduleSpecifier: string,
     quotePreference: QuotePreference,
     defaultImport: Import | undefined,
     namedImports: readonly Import[] | undefined,
     namespaceLikeImport: Import & { importKind: ImportKind.CommonJS | ImportKind.Namespace; } | undefined,
     compilerOptions: CompilerOptions,
+    preferences: UserPreferences,
 ): AnyImportSyntax | readonly AnyImportSyntax[] {
     const quotedModuleSpecifier = makeStringLiteral(moduleSpecifier, quotePreference);
     let statements: AnyImportSyntax | readonly AnyImportSyntax[] | undefined;
     if (defaultImport !== undefined || namedImports?.length) {
         // `verbatimModuleSyntax` should prefer top-level `import type` -
         // even though it's not an error, it would add unnecessary runtime emit.
         const topLevelTypeOnly = (!defaultImport || needsTypeOnly(defaultImport)) && every(namedImports, needsTypeOnly) ||
-            compilerOptions.verbatimModuleSyntax &&
+            (compilerOptions.verbatimModuleSyntax || preferences.preferTypeOnlyAutoImports) &&
                 defaultImport?.addAsTypeOnly !== AddAsTypeOnly.NotAllowed &&
                 !some(namedImports, i => i.addAsTypeOnly === AddAsTypeOnly.NotAllowed);
         statements = combine(
             statements,
             makeImport(
                 defaultImport && factory.createIdentifier(defaultImport.name),
-                namedImports?.map(({ addAsTypeOnly, name }) =>
+                namedImports?.map(namedImport =>
                     factory.createImportSpecifier(
-                        !topLevelTypeOnly && addAsTypeOnly === AddAsTypeOnly.Required,
+                        !topLevelTypeOnly && shouldUseTypeOnly(namedImport, preferences),
                         /*propertyName*/ undefined,
-                        factory.createIdentifier(name),
+                        factory.createIdentifier(namedImport.name),
                     )
                 ),
                 moduleSpecifier,
@@ -1643,14 +1650,14 @@ function getNewImports(
         const declaration = namespaceLikeImport.importKind === ImportKind.CommonJS
             ? factory.createImportEqualsDeclaration(
                 /*modifiers*/ undefined,
-                needsTypeOnly(namespaceLikeImport),
+                shouldUseTypeOnly(namespaceLikeImport, preferences),
                 factory.createIdentifier(namespaceLikeImport.name),
                 factory.createExternalModuleReference(quotedModuleSpecifier),
             )
             : factory.createImportDeclaration(
                 /*modifiers*/ undefined,
                 factory.createImportClause(
-                    needsTypeOnly(namespaceLikeImport),
+                    shouldUseTypeOnly(namespaceLikeImport, preferences),
                     /*name*/ undefined,
                     factory.createNamespaceImport(factory.createIdentifier(namespaceLikeImport.name)),
                 ),
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -8742,6 +8742,7 @@ declare namespace ts {
         readonly interactiveInlayHints?: boolean;
         readonly allowRenameOfImportPath?: boolean;
         readonly autoImportFileExcludePatterns?: string[];
+        readonly preferTypeOnlyAutoImports?: boolean;
         readonly organizeImportsIgnoreCase?: "auto" | boolean;
         readonly organizeImportsCollation?: "ordinal" | "unicode";
         readonly organizeImportsLocale?: string;
diff --git a/tests/cases/fourslash/autoImportTypeOnlyPreferred3.ts b/tests/cases/fourslash/autoImportTypeOnlyPreferred3.ts
@@ -0,0 +1,71 @@
+// @module: esnext
+// @moduleResolution: bundler
+
+// @Filename: /a.ts
+//// export class A {}
+//// export class B {}
+
+// @Filename: /b.ts
+//// let x: A/*b*/;
+
+// @Filename: /c.ts
+//// import { A } from "./a";
+//// new A();
+//// let x: B/*c*/;
+
+// @Filename: /d.ts
+//// new A();
+//// let x: B;
+
+// @Filename: /ns.ts
+//// export * as default from "./a";
+
+// @Filename: /e.ts
+//// let x: /*e*/ns.A;
+
+goTo.marker("b");
+verify.importFixAtPosition([
+`import type { A } from "./a";
+
+let x: A;`],
+  /*errorCode*/ undefined,
+  {
+    preferTypeOnlyAutoImports: true,
+  }
+);
+
+goTo.marker("c");
+verify.importFixAtPosition([
+`import { A, type B } from "./a";
+new A();
+let x: B;`],
+  /*errorCode*/ undefined,
+  {
+    preferTypeOnlyAutoImports: true,
+  }
+);
+
+goTo.file("/d.ts");
+verify.codeFixAll({
+  fixId: "fixMissingImport",
+  fixAllDescription: "Add all missing imports",
+  newFileContent:
+`import { A, type B } from "./a";
+
+new A();
+let x: B;`,
+  preferences: {
+    preferTypeOnlyAutoImports: true,
+  },
+});
+
+goTo.marker("e");
+verify.importFixAtPosition([
+`import type ns from "./ns";
+
+let x: ns.A;`],
+  /*errorCode*/ undefined,
+  {
+    preferTypeOnlyAutoImports: true,
+  }
+);
diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts
@@ -363,7 +363,7 @@ declare namespace FourSlashInterface {
         docCommentTemplateAt(markerName: string | FourSlashInterface.Marker, expectedOffset: number, expectedText: string, options?: VerifyDocCommentTemplateOptions): void;
         noDocCommentTemplateAt(markerName: string | FourSlashInterface.Marker): void;
         rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void;
-        codeFixAll(options: { fixId: string, fixAllDescription: string, newFileContent: NewFileContent, commands?: {}[] }): void;
+        codeFixAll(options: { fixId: string, fixAllDescription: string, newFileContent: NewFileContent, commands?: {}[], preferences?: UserPreferences }): void;
         fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: FormatCodeOptions): void;
         rangeIs(expectedText: string, includeWhiteSpace?: boolean): void;
         fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: FormatCodeOptions): void;
@@ -663,6 +663,7 @@ declare namespace FourSlashInterface {
         readonly providePrefixAndSuffixTextForRename?: boolean;
         readonly allowRenameOfImportPath?: boolean;
         readonly autoImportFileExcludePatterns?: readonly string[];
+        readonly preferTypeOnlyAutoImports?: boolean;
         readonly organizeImportsIgnoreCase?: "auto" | boolean;
         readonly organizeImportsCollation?: "unicode" | "ordinal";
         readonly organizeImportsLocale?: string;

Original file line number	Diff line number	Diff line change
`@@ -1869,6 +1869,7 @@ export interface VerifyCodeFixAllOptions {`
`1869`	`1869`	`fixAllDescription: string;`
`1870`	`1870`	`newFileContent: NewFileContent;`
`1871`	`1871`	`commands: readonly {}[];`
	`1872`	`+ preferences?: ts.UserPreferences;`
`1872`	`1873`	`}`
`1873`	`1874`
`1874`	`1875`	`export interface VerifyRefactorOptions {`