Improvements to generateTypes (#28458)

* Improvements to generateTypes

* createProperty only if necessary

Author: Andy

src/compiler/inspectValue.ts

@@ -26,6 +26,7 @@ namespace ts {
     }
     export interface ValueInfoObject extends ValueInfoBase {
         readonly kind: ValueKind.Object;
+        readonly hasNontrivialPrototype: boolean;
         readonly members: ReadonlyArray<ValueInfo>;
     }
 
@@ -63,7 +64,9 @@ namespace ts {
                     const builtin = getBuiltinType(name, value as object, recurser);
                     if (builtin !== undefined) return builtin;
                     const entries = getEntriesOfObject(value as object);
-                    return { kind: ValueKind.Object, name, members: flatMap(entries, ({ key, value }) => getValueInfo(key, value, recurser)) };
+                    const hasNontrivialPrototype = Object.getPrototypeOf(value) !== Object.prototype;
+                    const members = flatMap(entries, ({ key, value }) => getValueInfo(key, value, recurser));
+                    return { kind: ValueKind.Object, name, hasNontrivialPrototype, members };
                 }
                 return { kind: ValueKind.Const, name, typeName: isNullOrUndefined(value) ? "any" : typeof value };
             },

src/services/codefixes/generateTypes.ts

@@ -36,13 +36,15 @@ namespace ts {
             case ValueKind.FunctionOrClass:
                 return [...exportEquals(), ...functionOrClassToStatements(modifiers, name, info)];
             case ValueKind.Object:
-                const { members } = info;
-                if (kind === OutputKind.ExportEquals) {
-                    return flatMap(members, v => toStatements(v, OutputKind.NamedExport));
-                }
-                if (members.some(m => m.kind === ValueKind.FunctionOrClass)) {
-                    // If some member is a function, use a namespace so it gets a FunctionDeclaration or ClassDeclaration.
-                    return [...exportDefault(), createNamespace(modifiers, name, flatMap(members, toNamespaceMemberStatements))];
+                const { members, hasNontrivialPrototype } = info;
+                if (!hasNontrivialPrototype) {
+                    if (kind === OutputKind.ExportEquals) {
+                        return flatMap(members, v => toStatements(v, OutputKind.NamedExport));
+                    }
+                    if (members.some(m => m.kind === ValueKind.FunctionOrClass)) {
+                        // If some member is a function, use a namespace so it gets a FunctionDeclaration or ClassDeclaration.
+                        return [...exportDefault(), createNamespace(modifiers, name, flatMap(members, toNamespaceMemberStatements))];
+                    }
                 }
                 // falls through
             case ValueKind.Const:
@@ -62,10 +64,20 @@ namespace ts {
     function functionOrClassToStatements(modifiers: Modifiers, name: string, { source, prototypeMembers, namespaceMembers }: ValueInfoFunctionOrClass): ReadonlyArray<Statement> {
         const fnAst = parseClassOrFunctionBody(source);
         const { parameters, returnType } = fnAst === undefined ? { parameters: emptyArray, returnType: anyType() } : getParametersAndReturnType(fnAst);
-        const instanceProperties = typeof fnAst === "object" ? getConstructorFunctionInstanceProperties(fnAst) : emptyArray;
+        const protoOrInstanceMembers = createMap<MethodDeclaration | PropertyDeclaration>();
+        if (typeof fnAst === "object") getConstructorFunctionInstanceProperties(fnAst, protoOrInstanceMembers);
+        for (const p of prototypeMembers) {
+            // ignore non-functions on the prototype
+            if (p.kind === ValueKind.FunctionOrClass) {
+                const m = tryGetMethod(p);
+                if (m) {
+                    protoOrInstanceMembers.set(p.name, m);
+                }
+            }
+        }
 
         const classStaticMembers: ClassElement[] | undefined =
-            instanceProperties.length !== 0 || prototypeMembers.length !== 0 || fnAst === undefined || typeof fnAst !== "number" && fnAst.kind === SyntaxKind.Constructor ? [] : undefined;
+            protoOrInstanceMembers.size !== 0 || fnAst === undefined || typeof fnAst !== "number" && fnAst.kind === SyntaxKind.Constructor ? [] : undefined;
 
         const namespaceStatements = flatMap(namespaceMembers, info => {
             if (!isValidIdentifier(info.name)) return undefined;
@@ -91,6 +103,9 @@ namespace ts {
                                 return undefined;
                             }
                         }
+                        break;
+                    default:
+                        Debug.assertNever(info);
                 }
             }
             return toStatements(info, OutputKind.NamespaceMember);
@@ -106,9 +121,7 @@ namespace ts {
                 [
                     ...classStaticMembers,
                     ...(parameters.length ? [createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, parameters, /*body*/ undefined)] : emptyArray),
-                    ...instanceProperties,
-                    // ignore non-functions on the prototype
-                    ...mapDefined(prototypeMembers, info => info.kind === ValueKind.FunctionOrClass ? tryGetMethod(info) : undefined),
+                    ...arrayFrom(protoOrInstanceMembers.values()),
                 ])
             : createFunctionDeclaration(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, name, /*typeParameters*/ undefined, parameters, returnType, /*body*/ undefined);
         return [decl, ...(namespaceStatements.length === 0 ? emptyArray : [createNamespace(modifiers && modifiers.map(m => getSynthesizedDeepClone(m)), name, namespaceStatements)])];
@@ -150,16 +163,16 @@ namespace ts {
     }
 
     // Parses assignments to "this.x" in the constructor into class property declarations
-    function getConstructorFunctionInstanceProperties(fnAst: FunctionOrConstructorNode): ReadonlyArray<PropertyDeclaration> {
-        const members: PropertyDeclaration[] = [];
+    function getConstructorFunctionInstanceProperties(fnAst: FunctionOrConstructorNode, members: Map<MethodDeclaration | PropertyDeclaration>): void {
         forEachOwnNodeOfFunction(fnAst, node => {
             if (isAssignmentExpression(node, /*excludeCompoundAssignment*/ true) &&
                 isPropertyAccessExpression(node.left) && node.left.expression.kind === SyntaxKind.ThisKeyword) {
                 const name = node.left.name.text;
-                if (!isJsPrivate(name)) members.push(createProperty(/*decorators*/ undefined, /*modifiers*/ undefined, name, /*questionOrExclamationToken*/ undefined, anyType(), /*initializer*/ undefined));
+                if (!isJsPrivate(name)) {
+                    getOrUpdate(members, name, () => createProperty(/*decorators*/ undefined, /*modifiers*/ undefined, name, /*questionOrExclamationToken*/ undefined, anyType(), /*initializer*/ undefined));
+                }
             }
         });
-        return members;
     }
 
     interface ParametersAndReturnType { readonly parameters: ReadonlyArray<ParameterDeclaration>; readonly returnType: TypeNode; }

tests/cases/fourslash/generateTypes_classes.ts

@@ -111,4 +111,39 @@ declare class example {
 `,
 },
 
+{
+    // No duplicate instance members
+    value: (() => {
+        class C {
+            constructor() {
+                (this as any).x = 0;
+                (this as any).x = 1;
+                (this as any).m = 0;
+            }
+            m() {}
+        }
+        return C;
+    })(),
+    output:
+`export = example;
+declare class example {
+    x: any;
+    m(): void;
+}
+`,
+},
+
+{
+    // nontrivial prototype marks something as an instance
+    value: (() => {
+        const obj = Object.create({});
+        obj.m = function() { this.x = 0; }
+        return { obj };
+    })(),
+    output:
+`export const obj: {
+    m: Function;
+};
+`,
+},
 );