Skip to content

Check module.exports #25732

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 18 commits into from
Jul 20, 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
4 changes: 4 additions & 0 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2072,6 +2072,10 @@ namespace ts {
if (isSpecialPropertyDeclaration(node as PropertyAccessExpression)) {
bindSpecialPropertyDeclaration(node as PropertyAccessExpression);
}
if (isInJavaScriptFile(node) && isModuleExportsPropertyAccessExpression(node as PropertyAccessExpression)) {
declareSymbol(container.locals!, /*parent*/ undefined, (node as PropertyAccessExpression).expression as Identifier,
SymbolFlags.FunctionScopedVariable | SymbolFlags.ModuleExports, SymbolFlags.FunctionScopedVariableExcludes);
}
break;
case SyntaxKind.BinaryExpression:
const specialKind = getSpecialPropertyAssignmentKind(node as BinaryExpression);
Expand Down
73 changes: 45 additions & 28 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ namespace ts {
undefinedSymbol.declarations = [];
const argumentsSymbol = createSymbol(SymbolFlags.Property, "arguments" as __String);
const requireSymbol = createSymbol(SymbolFlags.Property, "require" as __String);
const moduleSymbol = createSymbol(SymbolFlags.Property, "module" as __String);

/** This will be set during calls to `getResolvedSignature` where services determines an apparent number of arguments greater than what is actually provided. */
let apparentArgumentCount: number | undefined;
Expand Down Expand Up @@ -1422,10 +1421,6 @@ namespace ts {
if (isRequireCall(originalLocation.parent, /*checkArgumentIsStringLiteralLike*/ false)) {
return requireSymbol;
}
if (isIdentifier(originalLocation) && isPropertyAccessExpression(originalLocation.parent) &&
originalLocation.escapedText === "module" && originalLocation.parent.name.escapedText === "exports") {
return moduleSymbol;
}
}
}
if (!result) {
Expand Down Expand Up @@ -2410,7 +2405,7 @@ namespace ts {
function getExportsOfModuleWorker(moduleSymbol: Symbol): SymbolTable {
const visitedSymbols: Symbol[] = [];

// A module defined by an 'export=' consists on one export that needs to be resolved
// A module defined by an 'export=' consists of one export that needs to be resolved
moduleSymbol = resolveExternalModuleSymbol(moduleSymbol);

return visit(moduleSymbol) || emptySymbols;
Expand Down Expand Up @@ -2526,9 +2521,7 @@ namespace ts {
function getExportSymbolOfValueSymbolIfExported(symbol: Symbol): Symbol;
function getExportSymbolOfValueSymbolIfExported(symbol: Symbol | undefined): Symbol | undefined;
function getExportSymbolOfValueSymbolIfExported(symbol: Symbol | undefined): Symbol | undefined {
return symbol && (symbol.flags & SymbolFlags.ExportValue) !== 0
? getMergedSymbol(symbol.exportSymbol)
: symbol;
return getMergedSymbol(symbol && (symbol.flags & SymbolFlags.ExportValue) !== 0 ? symbol.exportSymbol : symbol);
}

function symbolIsValue(symbol: Symbol): boolean {
Expand Down Expand Up @@ -4710,7 +4703,7 @@ namespace ts {
return undefined;
}

function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol) {
function getWidenedTypeFromJSSpecialPropertyDeclarations(symbol: Symbol, resolvedSymbol?: Symbol) {
// function/class/{} assignments are fresh declarations, not property assignments, so only add prototype assignments
const specialDeclaration = getAssignedJavascriptInitializer(symbol.valueDeclaration);
if (specialDeclaration) {
Expand Down Expand Up @@ -4766,15 +4759,18 @@ namespace ts {
}
else if (!jsDocType && isBinaryExpression(expression)) {
// If we don't have an explicit JSDoc type, get the type from the expression.
let type = getWidenedLiteralType(checkExpressionCached(expression.right));
let type = resolvedSymbol ? getTypeOfSymbol(resolvedSymbol) : getWidenedLiteralType(checkExpressionCached(expression.right));

if (getObjectFlags(type) & ObjectFlags.Anonymous &&
if (type.flags & TypeFlags.Object &&
special === SpecialPropertyAssignmentKind.ModuleExports &&
symbol.escapedName === InternalSymbolName.ExportEquals) {
const exportedType = resolveStructuredTypeMembers(type as AnonymousType);
const exportedType = resolveStructuredTypeMembers(type as ObjectType);
const members = createSymbolTable();
copyEntries(exportedType.members, members);
symbol.exports!.forEach((s, name) => {
if (resolvedSymbol && !resolvedSymbol.exports) {
resolvedSymbol.exports = createSymbolTable();
}
(resolvedSymbol || symbol).exports!.forEach((s, name) => {
if (members.has(name)) {
const exportedMember = exportedType.members.get(name)!;
const union = createSymbol(s.flags | exportedMember.flags, name);
Expand Down Expand Up @@ -4984,9 +4980,15 @@ namespace ts {
return links.type = getTypeOfPrototypeProperty(symbol);
}
// CommonsJS require and module both have type any.
if (symbol === requireSymbol || symbol === moduleSymbol) {
if (symbol === requireSymbol) {
return links.type = anyType;
}
if (symbol.flags & SymbolFlags.ModuleExports) {
const fileSymbol = getSymbolOfNode(getSourceFileOfNode(symbol.valueDeclaration));
const members = createSymbolTable();
members.set("exports" as __String, fileSymbol);
return links.type = createAnonymousType(symbol, members, emptyArray, emptyArray, undefined, undefined);
}
// Handle catch clause variables
const declaration = symbol.valueDeclaration;
if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) {
Expand Down Expand Up @@ -5220,13 +5222,28 @@ namespace ts {
links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(symbol);
}
else {
const type = createObjectType(ObjectFlags.Anonymous, symbol);
if (symbol.flags & SymbolFlags.Class) {
const baseTypeVariable = getBaseTypeVariableOfClass(symbol);
links.type = baseTypeVariable ? getIntersectionType([type, baseTypeVariable]) : type;
if (symbol.flags & SymbolFlags.ValueModule && symbol.valueDeclaration && isSourceFile(symbol.valueDeclaration) && symbol.valueDeclaration.commonJsModuleIndicator) {
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
return errorType;
}
const resolvedModule = resolveExternalModuleSymbol(symbol);
if (resolvedModule !== symbol) {
const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!);
links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule);
}
if (!popTypeResolution()) {
links.type = reportCircularityError(symbol);
}
Copy link

Choose a reason for hiding this comment

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

Why not set links.type in this case?
This whole function might be neater if it just called getTypeOfFuncClassEnumModuleUncached which used return x instead of links.type = x. Then we would be sure to be setting the value and not forgetting some control flow path.

Copy link
Member Author

Choose a reason for hiding this comment

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

The original reason is that getTypeOfVariableOrParameterOrProperty does. I think that's because you can call getTypeOfSymbol in situations that are speculative, and you may successfully be able to get the type at a later time.

I tried this and nothing broke, even when I changed the equivalent line in getTypeOfVariableOrParameterOrProperty. So it looks like my guess is wrong. I think I will merge this for now and try that refactor separately, for both.

}
else {
links.type = strictNullChecks && symbol.flags & SymbolFlags.Optional ? getOptionalType(type) : type;
if (!links.type) {
const type = createObjectType(ObjectFlags.Anonymous, symbol);
if (symbol.flags & SymbolFlags.Class) {
const baseTypeVariable = getBaseTypeVariableOfClass(symbol);
links.type = baseTypeVariable ? getIntersectionType([type, baseTypeVariable]) : type;
}
else {
links.type = strictNullChecks && symbol.flags & SymbolFlags.Optional ? getOptionalType(type) : type;
}
}
}
}
Expand Down Expand Up @@ -15016,6 +15033,7 @@ namespace ts {
let flowContainer = getControlFlowContainer(node);
const isOuterVariable = flowContainer !== declarationContainer;
const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent);
const isModuleExports = symbol.flags & SymbolFlags.ModuleExports;
// When the control flow originates in a function expression or arrow function and we are referencing
// a const variable or parameter from an outer function, we extend the origin of the control flow
// analysis to include the immediately enclosing function.
Expand All @@ -15027,7 +15045,7 @@ namespace ts {
// We only look for uninitialized variables in strict null checking mode, and only when we can analyze
// the entire control flow graph from the variable's declaration (i.e. when the flow container and
// declaration container are the same).
const assumeInitialized = isParameter || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget ||
const assumeInitialized = isParameter || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget || isModuleExports ||
type !== autoType && type !== autoArrayType && (!strictNullChecks || (type.flags & TypeFlags.AnyOrUnknown) !== 0 ||
isInTypeQuery(node) || node.parent.kind === SyntaxKind.ExportSpecifier) ||
node.parent.kind === SyntaxKind.NonNullExpression ||
Expand Down Expand Up @@ -19295,7 +19313,9 @@ namespace ts {
if (node.expression.kind === SyntaxKind.SuperKeyword) {
const superType = checkSuperExpression(node.expression);
if (isTypeAny(superType)) {
forEach(node.arguments, checkExpresionNoReturn); // Still visit arguments so they get marked for visibility, etc
for (const arg of node.arguments) {
checkExpression(arg); // Still visit arguments so they get marked for visibility, etc
}
return anySignature;
}
if (superType !== errorType) {
Expand Down Expand Up @@ -21328,8 +21348,9 @@ namespace ts {

function isJSSpecialPropertyAssignment(special: SpecialPropertyAssignmentKind) {
switch (special) {
case SpecialPropertyAssignmentKind.ExportsProperty:
case SpecialPropertyAssignmentKind.ModuleExports:
return true;
case SpecialPropertyAssignmentKind.ExportsProperty:
case SpecialPropertyAssignmentKind.Property:
case SpecialPropertyAssignmentKind.Prototype:
case SpecialPropertyAssignmentKind.PrototypeProperty:
Expand Down Expand Up @@ -21643,10 +21664,6 @@ namespace ts {
return type;
}

function checkExpresionNoReturn(node: Expression) {
checkExpression(node);
}

// Checks an expression and returns its type. The contextualMapper parameter serves two purposes: When
// contextualMapper is not undefined and not equal to the identityMapper function object it indicates that the
// expression is being inferentially typed (section 4.15.2 in spec) and provides the type mapper to use in
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3398,6 +3398,7 @@ namespace ts {
Optional = 1 << 24, // Optional property
Transient = 1 << 25, // Transient symbol (created during type check)
JSContainer = 1 << 26, // Contains Javascript special declarations
ModuleExports = 1 << 27, // Symbol for CommonJS `module` of `module.exports`

/* @internal */
All = FunctionScopedVariable | BlockScopedVariable | Property | EnumMember | Function | Class | Interface | ConstEnum | RegularEnum | ValueModule | NamespaceModule | TypeLiteral
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,7 @@ declare namespace ts {
Optional = 16777216,
Transient = 33554432,
JSContainer = 67108864,
ModuleExports = 134217728,
Enum = 384,
Variable = 3,
Value = 67216319,
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,7 @@ declare namespace ts {
Optional = 16777216,
Transient = 33554432,
JSContainer = 67108864,
ModuleExports = 134217728,
Enum = 384,
Variable = 3,
Value = 67216319,
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/callbackCrossModule.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* @return {any} I don't even know what this should return
*/
module.exports = C
>module.exports : Symbol("tests/cases/conformance/jsdoc/mod1", Decl(mod1.js, 0, 0))
>module : Symbol(export=, Decl(mod1.js, 0, 0))
>exports : Symbol(export=, Decl(mod1.js, 0, 0))
>C : Symbol(C, Decl(mod1.js, 4, 18))
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/callbackCrossModule.types
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
*/
module.exports = C
>module.exports = C : typeof C
>module.exports : any
>module : any
>exports : any
>module.exports : typeof C
>module : { "tests/cases/conformance/jsdoc/mod1": typeof C; }
>exports : typeof C
>C : typeof C

function C() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export = Foo;
/** @typedef {(foo: Foo) => string} FooFun */

module.exports = /** @type {FooFun} */(void 0);
>module.exports : Symbol("tests/cases/compiler/something", Decl(something.js, 0, 0))
>module : Symbol(export=, Decl(something.js, 0, 0))
>exports : Symbol(export=, Decl(something.js, 0, 0))

Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ export = Foo;

module.exports = /** @type {FooFun} */(void 0);
>module.exports = /** @type {FooFun} */(void 0) : (foo: typeof Foo) => string
>module.exports : any
>module : any
>exports : any
>module.exports : (foo: typeof Foo) => string
>module : { "tests/cases/compiler/something": (foo: typeof Foo) => string; }
>exports : (foo: typeof Foo) => string
>(void 0) : (foo: typeof Foo) => string
>void 0 : undefined
>0 : 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ export function abc(a, b, c) { return 5; }
>c : Symbol(c, Decl(bug24934.js, 0, 25))

module.exports = { abc };
>module : Symbol(module)
>module.exports : Symbol("tests/cases/conformance/salsa/bug24934", Decl(bug24934.js, 0, 0))
>module : Symbol(module, Decl(bug24934.js, 0, 42))
>exports : Symbol("tests/cases/conformance/salsa/bug24934", Decl(bug24934.js, 0, 0))
>abc : Symbol(abc, Decl(bug24934.js, 1, 18))

=== tests/cases/conformance/salsa/use.js ===
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ export function abc(a, b, c) { return 5; }
>5 : 5

module.exports = { abc };
>module.exports = { abc } : { abc: (a: any, b: any, c: any) => number; }
>module.exports : any
>module : any
>exports : any
>module.exports = { abc } : typeof import("tests/cases/conformance/salsa/bug24934")
>module.exports : typeof import("tests/cases/conformance/salsa/bug24934")
>module : { "tests/cases/conformance/salsa/bug24934": typeof import("tests/cases/conformance/salsa/bug24934"); }
>exports : typeof import("tests/cases/conformance/salsa/bug24934")
>{ abc } : { abc: (a: any, b: any, c: any) => number; }
>abc : (a: any, b: any, c: any) => number

Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/constructorFunctions2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function A() { this.id = 1; }
>id : Symbol(A.id, Decl(other.js, 0, 14))

module.exports = A;
>module.exports : Symbol("tests/cases/conformance/salsa/other", Decl(other.js, 0, 0))
>module : Symbol(export=, Decl(other.js, 0, 29))
>exports : Symbol(export=, Decl(other.js, 0, 29))
>A : Symbol(A, Decl(other.js, 0, 0))
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/constructorFunctions2.types
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ function A() { this.id = 1; }

module.exports = A;
>module.exports = A : typeof A
>module.exports : any
>module : any
>exports : any
>module.exports : typeof A
>module : { "tests/cases/conformance/salsa/other": typeof A; }
>exports : typeof A
>A : typeof A

This file was deleted.

Loading