Always export typedefs (microsoft#23723)

* Always export typedefs

This actually just required deleting a check in declareModuleMembers
and checking for external AND commonjs modules in a couple of places.

However, while experimenting with this feature, I discovered that even
previously-exported typedefs would only be exported if they came after a
commonjs export node. So I added a commonjs check to the pass in the
parser. It will not catch nested module.exports, but it will catch
top-level assignments.

The new test tests both changes.

* Post-bind typedef instead of pre-checking for commonjs

* Duplicate identifier errors

* Fix class type reference resolution+update baselines

* Move to a type-based check for duplicate identifiers

Author: sandersn

src/compiler/binder.ts

@@ -118,6 +118,7 @@ namespace ts {
         let thisParentContainer: Node; // Container one level up
         let blockScopeContainer: Node;
         let lastContainer: Node;
+        let delayedTypedefs: { typedef: JSDocTypedefTag, container: Node, lastContainer: Node, blockScopeContainer: Node, parent: Node }[];
         let seenThisKeyword: boolean;
 
         // state used by control flow analysis
@@ -176,6 +177,7 @@ namespace ts {
                 bind(file);
                 file.symbolCount = symbolCount;
                 file.classifiableNames = classifiableNames;
+                delayedBindJSDocTypedefTag();
             }
 
             file = undefined;
@@ -186,6 +188,7 @@ namespace ts {
             thisParentContainer = undefined;
             blockScopeContainer = undefined;
             lastContainer = undefined;
+            delayedTypedefs = undefined;
             seenThisKeyword = false;
             currentFlow = undefined;
             currentBreakTarget = undefined;
@@ -450,8 +453,7 @@ namespace ts {
                 //       and this case is specially handled. Module augmentations should only be merged with original module definition
                 //       and should never be merged directly with other augmentation, and the latter case would be possible if automatic merge is allowed.
                 if (node.kind === SyntaxKind.JSDocTypedefTag) Debug.assert(isInJavaScriptFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
-                const isJSDocTypedefInJSDocNamespace = isJSDocTypedefTag(node) && node.name && node.name.kind === SyntaxKind.Identifier && node.name.isInJSDocNamespace;
-                if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypedefInJSDocNamespace) {
+                if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypedefTag(node)) {
                     if (hasModifier(node, ModifierFlags.Default) && !getDeclarationName(node)) {
                         return declareSymbol(container.symbol.exports, container.symbol, node, symbolFlags, symbolExcludes); // No local symbol for an unnamed default!
                     }
@@ -1727,7 +1729,7 @@ namespace ts {
                     declareModuleMember(node, symbolFlags, symbolExcludes);
                     break;
                 case SyntaxKind.SourceFile:
-                    if (isExternalModule(<SourceFile>container)) {
+                    if (isExternalOrCommonJsModule(<SourceFile>container)) {
                         declareModuleMember(node, symbolFlags, symbolExcludes);
                         break;
                     }
@@ -1745,6 +1747,24 @@ namespace ts {
             bindBlockScopedDeclaration(node, SymbolFlags.BlockScopedVariable, SymbolFlags.BlockScopedVariableExcludes);
         }
 
+        function delayedBindJSDocTypedefTag() {
+            if (!delayedTypedefs) {
+                return;
+            }
+            const saveContainer = container;
+            const saveLastContainer = lastContainer;
+            const saveBlockScopeContainer = blockScopeContainer;
+            const saveParent = parent;
+            for (const delay of delayedTypedefs) {
+                ({ container, lastContainer, blockScopeContainer, parent } = delay);
+                bindBlockScopedDeclaration(delay.typedef, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
+            }
+            container = saveContainer;
+            lastContainer = saveLastContainer;
+            blockScopeContainer = saveBlockScopeContainer;
+            parent = saveParent;
+        }
+
         // The binder visits every node in the syntax tree so it is a convenient place to perform a single localized
         // check for reserved words used as identifiers in strict mode code.
         function checkStrictModeIdentifier(node: Identifier) {
@@ -2194,7 +2214,7 @@ namespace ts {
                 case SyntaxKind.JSDocTypedefTag: {
                     const { fullName } = node as JSDocTypedefTag;
                     if (!fullName || fullName.kind === SyntaxKind.Identifier) {
-                        return bindBlockScopedDeclaration(<Declaration>node, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
+                        (delayedTypedefs || (delayedTypedefs = [])).push({ typedef: node as JSDocTypedefTag, container, lastContainer, blockScopeContainer, parent });
                     }
                     break;
                 }
@@ -2304,7 +2324,10 @@ namespace ts {
                 return s;
             });
             if (symbol) {
-                declareSymbol(symbol.exports, symbol, lhs, SymbolFlags.Property | SymbolFlags.ExportValue, SymbolFlags.None);
+                const flags = isClassExpression(node.right) ?
+                    SymbolFlags.Property | SymbolFlags.ExportValue | SymbolFlags.Class :
+                    SymbolFlags.Property | SymbolFlags.ExportValue;
+                declareSymbol(symbol.exports, symbol, lhs, flags, SymbolFlags.None);
             }
         }
 

src/compiler/checker.ts

@@ -7626,23 +7626,38 @@ namespace ts {
                 return unknownType;
             }
 
-            // A jsdoc TypeReference may have resolved to a value (as opposed to a type). If
-            // the symbol is a constructor function, return the inferred class type; otherwise,
-            // the type of this reference is just the type of the value we resolved to.
+            const jsdocType = getJSDocTypeReference(node, symbol, typeArguments);
+            if (jsdocType) {
+                return jsdocType;
+            }
+
+            // Resolve the type reference as a Type for the purpose of reporting errors.
+            resolveTypeReferenceName(getTypeReferenceName(node), SymbolFlags.Type);
+            return getTypeOfSymbol(symbol);
+        }
+
+        /**
+         * A jsdoc TypeReference may have resolved to a value (as opposed to a type). If
+         * the symbol is a constructor function, return the inferred class type; otherwise,
+         * the type of this reference is just the type of the value we resolved to.
+         */
+        function getJSDocTypeReference(node: NodeWithTypeArguments, symbol: Symbol, typeArguments: Type[]): Type | undefined {
             const assignedType = getAssignedClassType(symbol);
             const valueType = getTypeOfSymbol(symbol);
-            const referenceType = valueType.symbol && !isInferredClassType(valueType) && getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments);
+            const referenceType = valueType.symbol && valueType.symbol !== symbol && !isInferredClassType(valueType) && getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments);
             if (referenceType || assignedType) {
                 return referenceType && assignedType ? getIntersectionType([assignedType, referenceType]) : referenceType || assignedType;
             }
-
-            // Resolve the type reference as a Type for the purpose of reporting errors.
-            resolveTypeReferenceName(getTypeReferenceName(node), SymbolFlags.Type);
-            return valueType;
         }
 
         function getTypeReferenceTypeWorker(node: NodeWithTypeArguments, symbol: Symbol, typeArguments: Type[]): Type | undefined {
             if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
+                if (symbol.valueDeclaration && isBinaryExpression(symbol.valueDeclaration.parent)) {
+                    const jsdocType = getJSDocTypeReference(node, symbol, typeArguments);
+                    if (jsdocType) {
+                        return jsdocType;
+                    }
+                }
                 return getTypeFromClassOrInterfaceReference(node, symbol, typeArguments);
             }
 
@@ -20031,6 +20046,7 @@ namespace ts {
                         getUnionType([removeDefinitelyFalsyTypes(leftType), rightType], UnionReduction.Subtype) :
                         leftType;
                 case SyntaxKind.EqualsToken:
+                    checkSpecialAssignment(left, right);
                     checkAssignmentOperator(rightType);
                     return getRegularTypeOfObjectLiteral(rightType);
                 case SyntaxKind.CommaToken:
@@ -20040,6 +20056,24 @@ namespace ts {
                     return rightType;
             }
 
+            function checkSpecialAssignment(left: Node, right: Expression) {
+                const special = getSpecialPropertyAssignmentKind(left.parent as BinaryExpression);
+                if (special === SpecialPropertyAssignmentKind.ModuleExports) {
+                    const rightType = checkExpression(right, checkMode);
+                    for (const prop of getPropertiesOfObjectType(rightType)) {
+                        const propType = getTypeOfSymbol(prop);
+                        if (propType.symbol && propType.symbol.flags & SymbolFlags.Class) {
+                            const name = prop.escapedName;
+                            const symbol = resolveName(prop.valueDeclaration, name, SymbolFlags.Type, undefined, name, /*isUse*/ false);
+                            if (symbol) {
+                                grammarErrorOnNode(symbol.declarations[0], Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name));
+                                return grammarErrorOnNode(prop.valueDeclaration, Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name));
+                            }
+                        }
+                    }
+                }
+            }
+
             function isEvalNode(node: Expression) {
                 return node.kind === SyntaxKind.Identifier && (node as Identifier).escapedText === "eval";
             }

src/harness/harness.ts

@@ -217,7 +217,7 @@ namespace Utils {
             for (const childName in node) {
                 if (childName === "parent" || childName === "nextContainer" || childName === "modifiers" || childName === "externalModuleIndicator" ||
                     // for now ignore jsdoc comments
-                    childName === "jsDocComment" || childName === "checkJsDirective") {
+                    childName === "jsDocComment" || childName === "checkJsDirective" || childName === "commonJsModuleIndicator") {
                     continue;
                 }
                 const child = (<any>node)[childName];

tests/baselines/reference/jsdocTypedefNoCrash.symbols

@@ -4,5 +4,5 @@
  * }}
  */
 export const foo = 5;
->foo : Symbol(foo, Decl(export.js, 4, 12))
+>foo : Symbol(foo, Decl(export.js, 4, 12), Decl(export.js, 1, 3))
 

tests/baselines/reference/typedefCrossModule.symbols

@@ -0,0 +1,70 @@
+=== tests/cases/conformance/jsdoc/commonjs.d.ts ===
+declare var module: { exports: any};
+>module : Symbol(module, Decl(commonjs.d.ts, 0, 11))
+>exports : Symbol(exports, Decl(commonjs.d.ts, 0, 21))
+
+=== tests/cases/conformance/jsdoc/mod1.js ===
+/// <reference path="./commonjs.d.ts"/>
+/** @typedef {{ type: "a", x: 1 }} A */
+/** @typedef {{ type: "b", y: 1 }} B */
+/** @typedef {A | B} Both */
+module.exports = C
+>module.exports : Symbol(exports, Decl(commonjs.d.ts, 0, 21))
+>module : Symbol(export=, Decl(mod1.js, 0, 0))
+>exports : Symbol(export=, Decl(mod1.js, 0, 0))
+>C : Symbol(C, Decl(mod1.js, 4, 18))
+
+function C() {
+>C : Symbol(C, Decl(mod1.js, 4, 18))
+
+    this.p = 1
+>p : Symbol(C.p, Decl(mod1.js, 5, 14))
+}
+
+=== tests/cases/conformance/jsdoc/mod2.js ===
+/// <reference path="./commonjs.d.ts"/>
+/** @typedef {{ type: "a", x: 1 }} A */
+/** @typedef {{ type: "b", y: 1 }} B */
+/** @typedef {A | B} Both */
+
+export function C() {
+>C : Symbol(C, Decl(mod2.js, 0, 0))
+
+    this.p = 1
+>p : Symbol(C.p, Decl(mod2.js, 5, 21))
+}
+
+=== tests/cases/conformance/jsdoc/mod3.js ===
+/// <reference path="./commonjs.d.ts"/>
+/** @typedef {{ type: "a", x: 1 }} A */
+/** @typedef {{ type: "b", y: 1 }} B */
+/** @typedef {A | B} Both */
+
+exports.C = function() {
+>exports.C : Symbol(C, Decl(mod3.js, 0, 0))
+>exports : Symbol(C, Decl(mod3.js, 0, 0))
+>C : Symbol(C, Decl(mod3.js, 0, 0))
+
+    this.p = 1
+>p : Symbol(C.p, Decl(mod3.js, 5, 24))
+}
+
+=== tests/cases/conformance/jsdoc/use.js ===
+/** @type {import('./mod1').Both} */
+var both1 = { type: 'a', x: 1 };
+>both1 : Symbol(both1, Decl(use.js, 1, 3))
+>type : Symbol(type, Decl(use.js, 1, 13))
+>x : Symbol(x, Decl(use.js, 1, 24))
+
+/** @type {import('./mod2').Both} */
+var both2 = both1;
+>both2 : Symbol(both2, Decl(use.js, 3, 3))
+>both1 : Symbol(both1, Decl(use.js, 1, 3))
+
+/** @type {import('./mod3').Both} */
+var both3 = both2;
+>both3 : Symbol(both3, Decl(use.js, 5, 3))
+>both2 : Symbol(both2, Decl(use.js, 3, 3))
+
+
+

tests/baselines/reference/typedefCrossModule.types

@@ -0,0 +1,88 @@
+=== tests/cases/conformance/jsdoc/commonjs.d.ts ===
+declare var module: { exports: any};
+>module : { exports: any; }
+>exports : any
+
+=== tests/cases/conformance/jsdoc/mod1.js ===
+/// <reference path="./commonjs.d.ts"/>
+/** @typedef {{ type: "a", x: 1 }} A */
+/** @typedef {{ type: "b", y: 1 }} B */
+/** @typedef {A | B} Both */
+module.exports = C
+>module.exports = C : typeof C
+>module.exports : any
+>module : { exports: any; }
+>exports : any
+>C : typeof C
+
+function C() {
+>C : typeof C
+
+    this.p = 1
+>this.p = 1 : 1
+>this.p : any
+>this : any
+>p : any
+>1 : 1
+}
+
+=== tests/cases/conformance/jsdoc/mod2.js ===
+/// <reference path="./commonjs.d.ts"/>
+/** @typedef {{ type: "a", x: 1 }} A */
+/** @typedef {{ type: "b", y: 1 }} B */
+/** @typedef {A | B} Both */
+
+export function C() {
+>C : typeof C
+
+    this.p = 1
+>this.p = 1 : 1
+>this.p : any
+>this : any
+>p : any
+>1 : 1
+}
+
+=== tests/cases/conformance/jsdoc/mod3.js ===
+/// <reference path="./commonjs.d.ts"/>
+/** @typedef {{ type: "a", x: 1 }} A */
+/** @typedef {{ type: "b", y: 1 }} B */
+/** @typedef {A | B} Both */
+
+exports.C = function() {
+>exports.C = function() {    this.p = 1} : typeof C
+>exports.C : typeof C
+>exports : typeof import("tests/cases/conformance/jsdoc/mod3")
+>C : typeof C
+>function() {    this.p = 1} : typeof C
+
+    this.p = 1
+>this.p = 1 : 1
+>this.p : any
+>this : any
+>p : any
+>1 : 1
+}
+
+=== tests/cases/conformance/jsdoc/use.js ===
+/** @type {import('./mod1').Both} */
+var both1 = { type: 'a', x: 1 };
+>both1 : { type: "a"; x: 1; } | { type: "b"; y: 1; }
+>{ type: 'a', x: 1 } : { type: "a"; x: 1; }
+>type : "a"
+>'a' : "a"
+>x : 1
+>1 : 1
+
+/** @type {import('./mod2').Both} */
+var both2 = both1;
+>both2 : { type: "a"; x: 1; } | { type: "b"; y: 1; }
+>both1 : { type: "a"; x: 1; }
+
+/** @type {import('./mod3').Both} */
+var both3 = both2;
+>both3 : { type: "a"; x: 1; } | { type: "b"; y: 1; }
+>both2 : { type: "a"; x: 1; }
+
+
+