Skip to content

Always export typedefs #23723

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -176,6 +177,7 @@ namespace ts {
bind(file);
file.symbolCount = symbolCount;
file.classifiableNames = classifiableNames;
delayedBindJSDocTypedefTag();
}

file = undefined;
Expand All @@ -186,6 +188,7 @@ namespace ts {
thisParentContainer = undefined;
blockScopeContainer = undefined;
lastContainer = undefined;
delayedTypedefs = undefined;
seenThisKeyword = false;
currentFlow = undefined;
currentBreakTarget = undefined;
Expand Down Expand Up @@ -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!
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure i understand why this is related ot the original change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that typedefs are exported, there's a possibility that their names collide, such that there is a difference between

let x: import("./a.js").Foo
// and
import a = require('./a.js')
var foo = new Foo()

In other words, the type reference Foo might refer to the typedef, whereas the value referenced in new Foo might refer to the class. There should be an error to prevent this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see. thanks.

SymbolFlags.Property | SymbolFlags.ExportValue | SymbolFlags.Class :
SymbolFlags.Property | SymbolFlags.ExportValue;
declareSymbol(symbol.exports, symbol, lhs, flags, SymbolFlags.None);
}
}

Expand Down
50 changes: 42 additions & 8 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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:
Expand All @@ -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";
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTypedefNoCrash.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -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))

70 changes: 70 additions & 0 deletions tests/baselines/reference/typedefCrossModule.symbols
Original file line number Diff line number Diff line change
@@ -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))



88 changes: 88 additions & 0 deletions tests/baselines/reference/typedefCrossModule.types
Original file line number Diff line number Diff line change
@@ -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; }



Loading