Reuse input nodes where possible when serializing jsdoc implements clauses (#41783)

weswigham · web-flow · commit 69143ecc5b92 · 2020-12-08T12:13:37.000-08:00
* Reuse input nodes where possible when serializing jsdoc implements clauses

* Whitespace changes, per PR feedback
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -5860,6 +5860,32 @@ namespace ts {
                 return typeToTypeNodeHelper(type, context);
             }
 
+            function trackExistingEntityName<T extends EntityNameOrEntityNameExpression>(node: T, context: NodeBuilderContext, includePrivateSymbol?: (s: Symbol) => void) {
+                let introducesError = false;
+                const leftmost = getFirstIdentifier(node);
+                if (isInJSFile(node) && (isExportsIdentifier(leftmost) || isModuleExportsAccessExpression(leftmost.parent) || (isQualifiedName(leftmost.parent) && isModuleIdentifier(leftmost.parent.left) && isExportsIdentifier(leftmost.parent.right)))) {
+                    introducesError = true;
+                    return { introducesError, node };
+                }
+                const sym = resolveEntityName(leftmost, SymbolFlags.All, /*ignoreErrors*/ true, /*dontResolveALias*/ true);
+                if (sym) {
+                    if (isSymbolAccessible(sym, context.enclosingDeclaration, SymbolFlags.All, /*shouldComputeAliasesToMakeVisible*/ false).accessibility !== SymbolAccessibility.Accessible) {
+                        introducesError = true;
+                    }
+                    else {
+                        context.tracker?.trackSymbol?.(sym, context.enclosingDeclaration, SymbolFlags.All);
+                        includePrivateSymbol?.(sym);
+                    }
+                    if (isIdentifier(node)) {
+                        const name = sym.flags & SymbolFlags.TypeParameter ? typeParameterToName(getDeclaredTypeOfSymbol(sym), context) : factory.cloneNode(node);
+                        name.symbol = sym; // for quickinfo, which uses identifier symbol information
+                        return { introducesError, node: setEmitFlags(setOriginalNode(name, node), EmitFlags.NoAsciiEscaping) };
+                    }
+                }
+
+                return { introducesError, node };
+            }
+
             function serializeExistingTypeNode(context: NodeBuilderContext, existing: TypeNode, includePrivateSymbol?: (s: Symbol) => void, bundled?: boolean) {
                 if (cancellationToken && cancellationToken.throwIfCancellationRequested) {
                     cancellationToken.throwIfCancellationRequested();
@@ -5983,25 +6009,10 @@ namespace ts {
                     }
 
                     if (isEntityName(node) || isEntityNameExpression(node)) {
-                        const leftmost = getFirstIdentifier(node);
-                        if (isInJSFile(node) && (isExportsIdentifier(leftmost) || isModuleExportsAccessExpression(leftmost.parent) || (isQualifiedName(leftmost.parent) && isModuleIdentifier(leftmost.parent.left) && isExportsIdentifier(leftmost.parent.right)))) {
-                            hadError = true;
-                            return node;
-                        }
-                        const sym = resolveEntityName(leftmost, SymbolFlags.All, /*ignoreErrors*/ true, /*dontResolveALias*/ true);
-                        if (sym) {
-                            if (isSymbolAccessible(sym, context.enclosingDeclaration, SymbolFlags.All, /*shouldComputeAliasesToMakeVisible*/ false).accessibility !== SymbolAccessibility.Accessible) {
-                                hadError = true;
-                            }
-                            else {
-                                context.tracker?.trackSymbol?.(sym, context.enclosingDeclaration, SymbolFlags.All);
-                                includePrivateSymbol?.(sym);
-                            }
-                            if (isIdentifier(node)) {
-                                const name = sym.flags & SymbolFlags.TypeParameter ? typeParameterToName(getDeclaredTypeOfSymbol(sym), context) : factory.cloneNode(node);
-                                name.symbol = sym; // for quickinfo, which uses identifier symbol information
-                                return setEmitFlags(setOriginalNode(name, node), EmitFlags.NoAsciiEscaping);
-                            }
+                        const { introducesError, node: result } = trackExistingEntityName(node, context, includePrivateSymbol);
+                        hadError = hadError || introducesError;
+                        if (result !== node) {
+                            return result;
                         }
                     }
 
@@ -6732,12 +6743,50 @@ namespace ts {
                         !(p.flags & SymbolFlags.Prototype || p.escapedName === "prototype" || p.valueDeclaration && getEffectiveModifierFlags(p.valueDeclaration) & ModifierFlags.Static && isClassLike(p.valueDeclaration.parent));
                 }
 
+                function sanitizeJSDocImplements(clauses: readonly ExpressionWithTypeArguments[]): ExpressionWithTypeArguments[] | undefined {
+                    const result = mapDefined(clauses, e => {
+                        const oldEnclosing = context.enclosingDeclaration;
+                        context.enclosingDeclaration = e;
+                        let expr = e.expression;
+                        if (isEntityNameExpression(expr)) {
+                            if (isIdentifier(expr) && idText(expr) === "") {
+                                return cleanup(/*result*/ undefined); // Empty heritage clause, should be an error, but prefer emitting no heritage clauses to reemitting the empty one
+                            }
+                            let introducesError: boolean;
+                            ({ introducesError, node: expr } = trackExistingEntityName(expr, context, includePrivateSymbol));
+                            if (introducesError) {
+                                return cleanup(/*result*/ undefined);
+                            }
+                        }
+                        return cleanup(factory.createExpressionWithTypeArguments(expr,
+                            map(e.typeArguments, a =>
+                                serializeExistingTypeNode(context, a, includePrivateSymbol, bundled)
+                                || typeToTypeNodeHelper(getTypeFromTypeNode(a), context)
+                            )
+                        ));
+
+                        function cleanup<T>(result: T): T {
+                            context.enclosingDeclaration = oldEnclosing;
+                            return result;
+                        }
+                    });
+                    if (result.length === clauses.length) {
+                        return result;
+                    }
+                    return undefined;
+                }
+
                 function serializeAsClass(symbol: Symbol, localName: string, modifierFlags: ModifierFlags) {
+                    const originalDecl = find(symbol.declarations, isClassLike);
+                    const oldEnclosing = context.enclosingDeclaration;
+                    context.enclosingDeclaration = originalDecl || oldEnclosing;
                     const localParams = getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol);
                     const typeParamDecls = map(localParams, p => typeParameterToDeclaration(p, context));
                     const classType = getDeclaredTypeOfClassOrInterface(symbol);
                     const baseTypes = getBaseTypes(classType);
-                    const implementsExpressions = mapDefined(getImplementsTypes(classType), serializeImplementedType);
+                    const originalImplements = originalDecl && getEffectiveImplementsTypeNodes(originalDecl);
+                    const implementsExpressions = originalImplements && sanitizeJSDocImplements(originalImplements)
+                        || mapDefined(getImplementsTypes(classType), serializeImplementedType);
                     const staticType = getTypeOfSymbol(symbol);
                     const isClass = !!staticType.symbol?.valueDeclaration && isClassLike(staticType.symbol.valueDeclaration);
                     const staticBaseType = isClass
@@ -6790,6 +6839,7 @@ namespace ts {
                         [factory.createConstructorDeclaration(/*decorators*/ undefined, factory.createModifiersFromModifierFlags(ModifierFlags.Private), [], /*body*/ undefined)] :
                         serializeSignatures(SignatureKind.Construct, staticType, staticBaseType, SyntaxKind.Constructor) as ConstructorDeclaration[];
                     const indexSignatures = serializeIndexSignatures(classType, baseTypes[0]);
+                    context.enclosingDeclaration = oldEnclosing;
                     addResult(setTextRange(factory.createClassDeclaration(
                         /*decorators*/ undefined,
                         /*modifiers*/ undefined,
diff --git a/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.js b/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.js
@@ -0,0 +1,71 @@
+//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassImplementsGenericsSerialization.ts] ////
+
+//// [interface.ts]
+export interface Encoder<T> {
+    encode(value: T): Uint8Array
+}
+//// [lib.js]
+/**
+ * @template T
+ * @implements {IEncoder<T>}
+ */
+export class Encoder {
+    /**
+     * @param {T} value 
+     */
+    encode(value) {
+        return new Uint8Array(0)
+    }
+}
+
+
+/**
+ * @template T
+ * @typedef {import('./interface').Encoder<T>} IEncoder
+ */
+
+//// [interface.js]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+//// [lib.js]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.Encoder = void 0;
+/**
+ * @template T
+ * @implements {IEncoder<T>}
+ */
+var Encoder = /** @class */ (function () {
+    function Encoder() {
+    }
+    /**
+     * @param {T} value
+     */
+    Encoder.prototype.encode = function (value) {
+        return new Uint8Array(0);
+    };
+    return Encoder;
+}());
+exports.Encoder = Encoder;
+/**
+ * @template T
+ * @typedef {import('./interface').Encoder<T>} IEncoder
+ */ 
+
+
+//// [interface.d.ts]
+export interface Encoder<T> {
+    encode(value: T): Uint8Array;
+}
+//// [lib.d.ts]
+/**
+ * @template T
+ * @implements {IEncoder<T>}
+ */
+export class Encoder<T> implements IEncoder<T> {
+    /**
+     * @param {T} value
+     */
+    encode(value: T): Uint8Array;
+}
+export type IEncoder<T> = import("./interface").Encoder<T>;
diff --git a/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.symbols b/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.symbols
@@ -0,0 +1,36 @@
+=== tests/cases/conformance/jsdoc/declarations/interface.ts ===
+export interface Encoder<T> {
+>Encoder : Symbol(Encoder, Decl(interface.ts, 0, 0))
+>T : Symbol(T, Decl(interface.ts, 0, 25))
+
+    encode(value: T): Uint8Array
+>encode : Symbol(Encoder.encode, Decl(interface.ts, 0, 29))
+>value : Symbol(value, Decl(interface.ts, 1, 11))
+>T : Symbol(T, Decl(interface.ts, 0, 25))
+>Uint8Array : Symbol(Uint8Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
+}
+=== tests/cases/conformance/jsdoc/declarations/lib.js ===
+/**
+ * @template T
+ * @implements {IEncoder<T>}
+ */
+export class Encoder {
+>Encoder : Symbol(Encoder, Decl(lib.js, 0, 0))
+
+    /**
+     * @param {T} value 
+     */
+    encode(value) {
+>encode : Symbol(Encoder.encode, Decl(lib.js, 4, 22))
+>value : Symbol(value, Decl(lib.js, 8, 11))
+
+        return new Uint8Array(0)
+>Uint8Array : Symbol(Uint8Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
+    }
+}
+
+
+/**
+ * @template T
+ * @typedef {import('./interface').Encoder<T>} IEncoder
+ */
diff --git a/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.types b/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.types
@@ -0,0 +1,33 @@
+=== tests/cases/conformance/jsdoc/declarations/interface.ts ===
+export interface Encoder<T> {
+    encode(value: T): Uint8Array
+>encode : (value: T) => Uint8Array
+>value : T
+}
+=== tests/cases/conformance/jsdoc/declarations/lib.js ===
+/**
+ * @template T
+ * @implements {IEncoder<T>}
+ */
+export class Encoder {
+>Encoder : Encoder<T>
+
+    /**
+     * @param {T} value 
+     */
+    encode(value) {
+>encode : (value: T) => Uint8Array
+>value : T
+
+        return new Uint8Array(0)
+>new Uint8Array(0) : Uint8Array
+>Uint8Array : Uint8ArrayConstructor
+>0 : 0
+    }
+}
+
+
+/**
+ * @template T
+ * @typedef {import('./interface').Encoder<T>} IEncoder
+ */
diff --git a/tests/baselines/reference/jsDeclarationsClassesErr.js b/tests/baselines/reference/jsDeclarationsClassesErr.js
@@ -222,11 +222,11 @@ exports.CC = CC;
 
 
 //// [index.d.ts]
-export class M<T_1> {
-    field: T_1;
+export class M<T> {
+    field: T;
 }
-export class N<U_1> extends M<U_1> {
-    other: U_1;
+export class N<U> extends M<U> {
+    other: U;
 }
 export class O {
     [idx: string]: string;
diff --git a/tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassImplementsGenericsSerialization.ts b/tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassImplementsGenericsSerialization.ts
@@ -0,0 +1,28 @@
+// @allowJs: true
+// @checkJs: true
+// @target: es5
+// @outDir: ./out
+// @declaration: true
+// @filename: interface.ts
+export interface Encoder<T> {
+    encode(value: T): Uint8Array
+}
+// @filename: lib.js
+/**
+ * @template T
+ * @implements {IEncoder<T>}
+ */
+export class Encoder {
+    /**
+     * @param {T} value 
+     */
+    encode(value) {
+        return new Uint8Array(0)
+    }
+}
+
+
+/**
+ * @template T
+ * @typedef {import('./interface').Encoder<T>} IEncoder
+ */

Original file line number	Diff line number	Diff line change
`@@ -222,11 +222,11 @@ exports.CC = CC;`
`222`	`222`
`223`	`223`
`224`	`224`	`//// [index.d.ts]`
`225`		`-export class M<T_1> {`
`226`		`- field: T_1;`
	`225`	`+export class M<T> {`
	`226`	`+ field: T;`
`227`	`227`	`}`
`228`		`-export class N<U_1> extends M<U_1> {`
`229`		`- other: U_1;`
	`228`	`+export class N<U> extends M<U> {`
	`229`	`+ other: U;`
`230`	`230`	`}`
`231`	`231`	`export class O {`
`232`	`232`	`[idx: string]: string;`