Drop unnecessary type arguments in the isolated declarations quick fix (

#59665) Co-authored-by: Jake Bailey <[email protected]> Co-authored-by: Nathan Shively-Sanders <[email protected]>
microsoft · Sep 16, 2024 · 8230bc6 · 8230bc6
1 parent 52eaa7b
commit 8230bc6
Show file tree

Hide file tree

Showing 10 changed files with 167 additions and 4 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -1641,6 +1641,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         getBaseTypeOfLiteralType,
         getWidenedType,
         getWidenedLiteralType,
+        fillMissingTypeArguments,
         getTypeFromTypeNode: nodeIn => {
             const node = getParseTreeNode(nodeIn, isTypeNode);
             return node ? getTypeFromTypeNode(node) : errorType;

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5421,6 +5421,7 @@ export interface TypeChecker {
     /** @internal */ isTypeParameterPossiblyReferenced(tp: TypeParameter, node: Node): boolean;
     /** @internal */ typeHasCallOrConstructSignatures(type: Type): boolean;
     /** @internal */ getSymbolFlags(symbol: Symbol): SymbolFlags;
+    /** @internal */ fillMissingTypeArguments(typeArguments: readonly Type[], typeParameters: readonly TypeParameter[] | undefined, minTypeArgumentCount: number, isJavaScriptImplicitAny: boolean): Type[];
 }
 
 /** @internal */

diff --git a/src/services/codefixes/fixMissingTypeAnnotationOnExports.ts b/src/services/codefixes/fixMissingTypeAnnotationOnExports.ts
@@ -4,8 +4,9 @@ import {
     createImportAdder,
     eachDiagnostic,
     registerCodeFix,
+    typeNodeToAutoImportableTypeNode,
     typePredicateToAutoImportableTypeNode,
-    typeToAutoImportableTypeNode,
+    typeToMinimizedReferenceType,
 } from "../_namespaces/ts.codefix.js";
 import {
     ArrayBindingPattern,
@@ -1096,9 +1097,9 @@ function withContext<T>(
         return emptyInferenceResult;
     }
 
-    function typeToTypeNode(type: Type, enclosingDeclaration: Node, flags = NodeBuilderFlags.None) {
+    function typeToTypeNode(type: Type, enclosingDeclaration: Node, flags = NodeBuilderFlags.None): TypeNode | undefined {
         let isTruncated = false;
-        const result = typeToAutoImportableTypeNode(typeChecker, importAdder, type, enclosingDeclaration, scriptTarget, declarationEmitNodeBuilderFlags | flags, declarationEmitInternalNodeBuilderFlags, {
+        const minimizedTypeNode = typeToMinimizedReferenceType(typeChecker, type, enclosingDeclaration, declarationEmitNodeBuilderFlags | flags, declarationEmitInternalNodeBuilderFlags, {
             moduleResolverHost: program,
             trackSymbol() {
                 return true;
@@ -1107,6 +1108,10 @@ function withContext<T>(
                 isTruncated = true;
             },
         });
+        if (!minimizedTypeNode) {
+            return undefined;
+        }
+        const result = typeNodeToAutoImportableTypeNode(minimizedTypeNode, importAdder, scriptTarget);
         return isTruncated ? factory.createKeywordTypeNode(SyntaxKind.AnyKeyword) : result;
     }
 

diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts
@@ -23,6 +23,7 @@ import {
     flatMap,
     FunctionDeclaration,
     FunctionExpression,
+    GenericType,
     GetAccessorDeclaration,
     getAllAccessorDeclarations,
     getCheckFlags,
@@ -59,6 +60,7 @@ import {
     isSetAccessorDeclaration,
     isStringLiteral,
     isTypeNode,
+    isTypeReferenceNode,
     isTypeUsableAsPropertyName,
     isYieldExpression,
     LanguageServiceHost,
@@ -595,7 +597,15 @@ function createTypeParameterName(index: number) {
 
 /** @internal */
 export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, internalFlags?: InternalNodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
-    let typeNode = checker.typeToTypeNode(type, contextNode, flags, internalFlags, tracker);
+    const typeNode = checker.typeToTypeNode(type, contextNode, flags, internalFlags, tracker);
+    if (!typeNode) {
+        return undefined;
+    }
+    return typeNodeToAutoImportableTypeNode(typeNode, importAdder, scriptTarget);
+}
+
+/** @internal */
+export function typeNodeToAutoImportableTypeNode(typeNode: TypeNode, importAdder: ImportAdder, scriptTarget: ScriptTarget): TypeNode | undefined {
     if (typeNode && isImportTypeNode(typeNode)) {
         const importableReference = tryGetAutoImportableReferenceFromTypeNode(typeNode, scriptTarget);
         if (importableReference) {
@@ -608,6 +618,40 @@ export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder:
     return getSynthesizedDeepClone(typeNode);
 }
 
+function endOfRequiredTypeParameters(checker: TypeChecker, type: GenericType): number {
+    Debug.assert(type.typeArguments);
+    const fullTypeArguments = type.typeArguments;
+    const target = type.target;
+    for (let cutoff = 0; cutoff < fullTypeArguments.length; cutoff++) {
+        const typeArguments = fullTypeArguments.slice(0, cutoff);
+        const filledIn = checker.fillMissingTypeArguments(typeArguments, target.typeParameters, cutoff, /*isJavaScriptImplicitAny*/ false);
+        if (filledIn.every((fill, i) => fill === fullTypeArguments[i])) {
+            return cutoff;
+        }
+    }
+    // If we make it all the way here, all the type arguments are required.
+    return fullTypeArguments.length;
+}
+
+/** @internal */
+export function typeToMinimizedReferenceType(checker: TypeChecker, type: Type, contextNode: Node | undefined, flags?: NodeBuilderFlags, internalFlags?: InternalNodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
+    let typeNode = checker.typeToTypeNode(type, contextNode, flags, internalFlags, tracker);
+    if (!typeNode) {
+        return undefined;
+    }
+    if (isTypeReferenceNode(typeNode)) {
+        const genericType = type as GenericType;
+        if (genericType.typeArguments && typeNode.typeArguments) {
+            const cutoff = endOfRequiredTypeParameters(checker, genericType);
+            if (cutoff < typeNode.typeArguments.length) {
+                const newTypeArguments = factory.createNodeArray(typeNode.typeArguments.slice(0, cutoff));
+                typeNode = factory.updateTypeReferenceNode(typeNode, typeNode.typeName, newTypeArguments);
+            }
+        }
+    }
+    return typeNode;
+}
+
 /** @internal */
 export function typePredicateToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, typePredicate: TypePredicate, contextNode: Node | undefined, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, internalFlags?: InternalNodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
     let typePredicateNode = checker.typePredicateToTypePredicateNode(typePredicate, contextNode, flags, internalFlags, tracker);

diff --git a/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports50-generics-with-default.ts b/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports50-generics-with-default.ts
@@ -0,0 +1,15 @@
+/// <reference path='fourslash.ts'/>
+
+// @isolatedDeclarations: true
+// @declaration: true
+// @lib: es2015
+////let x: Iterator<number>;
+////export const y = x;
+
+verify.codeFix({
+    description: "Add annotation of type 'Iterator<number>'",
+    index: 0,
+    newFileContent:
+`let x: Iterator<number>;
+export const y: Iterator<number> = x;`,
+});
diff --git a/...sh/codeFixMissingTypeAnnotationOnExports51-slightly-more-complex-generics-with-default.ts b/...sh/codeFixMissingTypeAnnotationOnExports51-slightly-more-complex-generics-with-default.ts
@@ -0,0 +1,19 @@
+/// <reference path='fourslash.ts'/>
+
+// @isolatedDeclarations: true
+// @declaration: true
+
+////export interface Foo<T, U = T[]> {}
+////export function foo(x: Foo<string>) {
+////    return x;
+////}
+
+verify.codeFix({
+    description: "Add return type 'Foo<string>'",
+    index: 0,
+    newFileContent:
+`export interface Foo<T, U = T[]> {}
+export function foo(x: Foo<string>): Foo<string> {
+    return x;
+}`,
+});
diff --git a/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports52-generics-oversimplification.ts b/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports52-generics-oversimplification.ts
@@ -0,0 +1,23 @@
+/// <reference path='fourslash.ts'/>
+
+// In the abstract, we might prefer the inferred return type annotation to
+// be identical to the parameter type (with 2 type parameters).
+// Our current heuristic to avoid overly complex types in this case creates
+// "overly simple" types, but this tradeoff seems reasonable.
+
+// @isolatedDeclarations: true
+// @declaration: true
+////export interface Foo<T, U = T[]> {}
+////export function foo(x: Foo<string, string[]>) {
+////    return x;
+////}
+
+verify.codeFix({
+    description: "Add return type 'Foo<string>'",
+    index: 0,
+    newFileContent:
+`export interface Foo<T, U = T[]> {}
+export function foo(x: Foo<string, string[]>): Foo<string> {
+    return x;
+}`,
+});
diff --git a/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports53-nested-generic-types.ts b/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports53-nested-generic-types.ts
@@ -0,0 +1,21 @@
+/// <reference path='fourslash.ts'/>
+
+// Our current heursitic to avoid overly verbose generic types
+// doesn't handle generic types nested inside other types.
+
+// @isolatedDeclarations: true
+// @declaration: true
+////export interface Foo<T, U = T[]> {}
+////export function foo(x: Map<number, Foo<string>>) {
+////    return x;
+////}
+
+verify.codeFix({
+    description: "Add return type 'Map<number, Foo<string, string[]>>'",
+    index: 0,
+    newFileContent:
+`export interface Foo<T, U = T[]> {}
+export function foo(x: Map<number, Foo<string>>): Map<number, Foo<string, string[]>> {
+    return x;
+}`,
+});
diff --git a/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports54-generator-generics.ts b/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports54-generator-generics.ts
@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts'/>
+
+// @isolatedDeclarations: true
+// @declaration: true
+// @lib: es2015
+//// export function foo(x: Generator<number>) {
+////     return x;
+//// }
+
+verify.codeFix({
+    description: "Add return type 'Generator<number>'",
+    index: 0,
+    newFileContent:
+`export function foo(x: Generator<number>): Generator<number> {
+    return x;
+}`
+});
diff --git a/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports55-generator-return.ts b/tests/cases/fourslash/codeFixMissingTypeAnnotationOnExports55-generator-return.ts
@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts'/>
+
+// @isolatedDeclarations: true
+// @declaration: true
+// @lib: es2015
+//// export function *foo() {
+////     yield 5;
+//// }
+
+verify.codeFix({
+    description: "Add return type 'Generator<number, void, unknown>'",
+    index: 0,
+    newFileContent:
+`export function *foo(): Generator<number, void, unknown> {
+    yield 5;
+}`
+});