Skip to content

Commit 31d599a

Browse files
sandersnsandersn
authored
Check module.exports (microsoft#25732)
* Revert "Revert "Explicitly typed special assignments are context sensitive (microsoft#25619)"" This reverts commit 16676f2. * Revert "Revert "Explicitly typed prototype assignments are context sensitive (microsoft#25688)"" This reverts commit ff8c30d. * Initial, wasteful, solution It burns a check flags. Probably necessary, but perhaps not. I haven't accepted baselines, but they are a bit questionable. I'm not sure the synthetic type is right, because I expected to see { "exports": typeof import("x") } but instead see { "x": typeof import("x") }. * Update some baselines * module.exports= always uses lhs type Conflicts between exports property assignments and exports assignments should get a union type instead of an error. * Fix lint and accept good user baselines * Add tests based on user tests. These currently fail. * Fix all but 1 of user test bugs found by typing module.exports Still quite messy and full of notes * Follow merged symbols+allow any object type This allows exports like `module.exports = new EE` to have properties added to them. Still messy, but I'm going to run user tests and look for regressions. * Update improved user baselines * Fix infinite recursion when checking module.exports * Fix bogus use-before-def error getExportSymbolOfValueSymbolIfExported should always merge its returned symbol, whether it's symbol.exportSymbol or just symbol. * Update user test baselines * Cleanup * More small cleanup * Bind `module` of `module.exports` as a special symbol Previously it was also special, but created during name resolution in the checker. It made sense when there was only one special symbol for all files, but now there is one `module` symbol per file.
1 parent fedcd3a commit 31d599a

File tree

90 files changed

+1943
-1597
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

90 files changed

+1943
-1597
lines changed

‎src/compiler/binder.ts

+4
Original file line numberDiff line numberDiff line change
@@ -2072,6 +2072,10 @@ namespace ts {
20722072
if (isSpecialPropertyDeclaration(node as PropertyAccessExpression)) {
20732073
bindSpecialPropertyDeclaration(node as PropertyAccessExpression);
20742074
}
2075+
if (isInJavaScriptFile(node) && isModuleExportsPropertyAccessExpression(node as PropertyAccessExpression)) {
2076+
declareSymbol(container.locals!, /*parent*/ undefined, (node as PropertyAccessExpression).expression as Identifier,
2077+
SymbolFlags.FunctionScopedVariable | SymbolFlags.ModuleExports, SymbolFlags.FunctionScopedVariableExcludes);
2078+
}
20752079
break;
20762080
case SyntaxKind.BinaryExpression:
20772081
const specialKind = getSpecialPropertyAssignmentKind(node as BinaryExpression);

‎src/compiler/checker.ts

+45-28
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ namespace ts {
7676
undefinedSymbol.declarations = [];
7777
const argumentsSymbol = createSymbol(SymbolFlags.Property, "arguments" as __String);
7878
const requireSymbol = createSymbol(SymbolFlags.Property, "require" as __String);
79-
const moduleSymbol = createSymbol(SymbolFlags.Property, "module" as __String);
8079

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

2413-
// A module defined by an 'export=' consists on one export that needs to be resolved
2408+
// A module defined by an 'export=' consists of one export that needs to be resolved
24142409
moduleSymbol = resolveExternalModuleSymbol(moduleSymbol);
24152410

24162411
return visit(moduleSymbol) || emptySymbols;
@@ -2526,9 +2521,7 @@ namespace ts {
25262521
function getExportSymbolOfValueSymbolIfExported(symbol: Symbol): Symbol;
25272522
function getExportSymbolOfValueSymbolIfExported(symbol: Symbol | undefined): Symbol | undefined;
25282523
function getExportSymbolOfValueSymbolIfExported(symbol: Symbol | undefined): Symbol | undefined {
2529-
return symbol && (symbol.flags & SymbolFlags.ExportValue) !== 0
2530-
? getMergedSymbol(symbol.exportSymbol)
2531-
: symbol;
2524+
return getMergedSymbol(symbol && (symbol.flags & SymbolFlags.ExportValue) !== 0 ? symbol.exportSymbol : symbol);
25322525
}
25332526

25342527
function symbolIsValue(symbol: Symbol): boolean {
@@ -4710,7 +4703,7 @@ namespace ts {
47104703
return undefined;
47114704
}
47124705

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

4771-
if (getObjectFlags(type) & ObjectFlags.Anonymous &&
4764+
if (type.flags & TypeFlags.Object &&
47724765
special === SpecialPropertyAssignmentKind.ModuleExports &&
47734766
symbol.escapedName === InternalSymbolName.ExportEquals) {
4774-
const exportedType = resolveStructuredTypeMembers(type as AnonymousType);
4767+
const exportedType = resolveStructuredTypeMembers(type as ObjectType);
47754768
const members = createSymbolTable();
47764769
copyEntries(exportedType.members, members);
4777-
symbol.exports!.forEach((s, name) => {
4770+
if (resolvedSymbol && !resolvedSymbol.exports) {
4771+
resolvedSymbol.exports = createSymbolTable();
4772+
}
4773+
(resolvedSymbol || symbol).exports!.forEach((s, name) => {
47784774
if (members.has(name)) {
47794775
const exportedMember = exportedType.members.get(name)!;
47804776
const union = createSymbol(s.flags | exportedMember.flags, name);
@@ -4984,9 +4980,15 @@ namespace ts {
49844980
return links.type = getTypeOfPrototypeProperty(symbol);
49854981
}
49864982
// CommonsJS require and module both have type any.
4987-
if (symbol === requireSymbol || symbol === moduleSymbol) {
4983+
if (symbol === requireSymbol) {
49884984
return links.type = anyType;
49894985
}
4986+
if (symbol.flags & SymbolFlags.ModuleExports) {
4987+
const fileSymbol = getSymbolOfNode(getSourceFileOfNode(symbol.valueDeclaration));
4988+
const members = createSymbolTable();
4989+
members.set("exports" as __String, fileSymbol);
4990+
return links.type = createAnonymousType(symbol, members, emptyArray, emptyArray, undefined, undefined);
4991+
}
49904992
// Handle catch clause variables
49914993
const declaration = symbol.valueDeclaration;
49924994
if (isCatchClauseVariableDeclarationOrBindingElement(declaration)) {
@@ -5220,13 +5222,28 @@ namespace ts {
52205222
links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(symbol);
52215223
}
52225224
else {
5223-
const type = createObjectType(ObjectFlags.Anonymous, symbol);
5224-
if (symbol.flags & SymbolFlags.Class) {
5225-
const baseTypeVariable = getBaseTypeVariableOfClass(symbol);
5226-
links.type = baseTypeVariable ? getIntersectionType([type, baseTypeVariable]) : type;
5225+
if (symbol.flags & SymbolFlags.ValueModule && symbol.valueDeclaration && isSourceFile(symbol.valueDeclaration) && symbol.valueDeclaration.commonJsModuleIndicator) {
5226+
if (!pushTypeResolution(symbol, TypeSystemPropertyName.Type)) {
5227+
return errorType;
5228+
}
5229+
const resolvedModule = resolveExternalModuleSymbol(symbol);
5230+
if (resolvedModule !== symbol) {
5231+
const exportEquals = getMergedSymbol(symbol.exports!.get(InternalSymbolName.ExportEquals)!);
5232+
links.type = getWidenedTypeFromJSSpecialPropertyDeclarations(exportEquals, exportEquals === resolvedModule ? undefined : resolvedModule);
5233+
}
5234+
if (!popTypeResolution()) {
5235+
links.type = reportCircularityError(symbol);
5236+
}
52275237
}
5228-
else {
5229-
links.type = strictNullChecks && symbol.flags & SymbolFlags.Optional ? getOptionalType(type) : type;
5238+
if (!links.type) {
5239+
const type = createObjectType(ObjectFlags.Anonymous, symbol);
5240+
if (symbol.flags & SymbolFlags.Class) {
5241+
const baseTypeVariable = getBaseTypeVariableOfClass(symbol);
5242+
links.type = baseTypeVariable ? getIntersectionType([type, baseTypeVariable]) : type;
5243+
}
5244+
else {
5245+
links.type = strictNullChecks && symbol.flags & SymbolFlags.Optional ? getOptionalType(type) : type;
5246+
}
52305247
}
52315248
}
52325249
}
@@ -15016,6 +15033,7 @@ namespace ts {
1501615033
let flowContainer = getControlFlowContainer(node);
1501715034
const isOuterVariable = flowContainer !== declarationContainer;
1501815035
const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent);
15036+
const isModuleExports = symbol.flags & SymbolFlags.ModuleExports;
1501915037
// When the control flow originates in a function expression or arrow function and we are referencing
1502015038
// a const variable or parameter from an outer function, we extend the origin of the control flow
1502115039
// analysis to include the immediately enclosing function.
@@ -15027,7 +15045,7 @@ namespace ts {
1502715045
// We only look for uninitialized variables in strict null checking mode, and only when we can analyze
1502815046
// the entire control flow graph from the variable's declaration (i.e. when the flow container and
1502915047
// declaration container are the same).
15030-
const assumeInitialized = isParameter || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget ||
15048+
const assumeInitialized = isParameter || isAlias || isOuterVariable || isSpreadDestructuringAssignmentTarget || isModuleExports ||
1503115049
type !== autoType && type !== autoArrayType && (!strictNullChecks || (type.flags & TypeFlags.AnyOrUnknown) !== 0 ||
1503215050
isInTypeQuery(node) || node.parent.kind === SyntaxKind.ExportSpecifier) ||
1503315051
node.parent.kind === SyntaxKind.NonNullExpression ||
@@ -19295,7 +19313,9 @@ namespace ts {
1929519313
if (node.expression.kind === SyntaxKind.SuperKeyword) {
1929619314
const superType = checkSuperExpression(node.expression);
1929719315
if (isTypeAny(superType)) {
19298-
forEach(node.arguments, checkExpresionNoReturn); // Still visit arguments so they get marked for visibility, etc
19316+
for (const arg of node.arguments) {
19317+
checkExpression(arg); // Still visit arguments so they get marked for visibility, etc
19318+
}
1929919319
return anySignature;
1930019320
}
1930119321
if (superType !== errorType) {
@@ -21328,8 +21348,9 @@ namespace ts {
2132821348

2132921349
function isJSSpecialPropertyAssignment(special: SpecialPropertyAssignmentKind) {
2133021350
switch (special) {
21331-
case SpecialPropertyAssignmentKind.ExportsProperty:
2133221351
case SpecialPropertyAssignmentKind.ModuleExports:
21352+
return true;
21353+
case SpecialPropertyAssignmentKind.ExportsProperty:
2133321354
case SpecialPropertyAssignmentKind.Property:
2133421355
case SpecialPropertyAssignmentKind.Prototype:
2133521356
case SpecialPropertyAssignmentKind.PrototypeProperty:
@@ -21643,10 +21664,6 @@ namespace ts {
2164321664
return type;
2164421665
}
2164521666

21646-
function checkExpresionNoReturn(node: Expression) {
21647-
checkExpression(node);
21648-
}
21649-
2165021667
// Checks an expression and returns its type. The contextualMapper parameter serves two purposes: When
2165121668
// contextualMapper is not undefined and not equal to the identityMapper function object it indicates that the
2165221669
// expression is being inferentially typed (section 4.15.2 in spec) and provides the type mapper to use in

‎src/compiler/types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -3398,6 +3398,7 @@ namespace ts {
33983398
Optional = 1 << 24, // Optional property
33993399
Transient = 1 << 25, // Transient symbol (created during type check)
34003400
JSContainer = 1 << 26, // Contains Javascript special declarations
3401+
ModuleExports = 1 << 27, // Symbol for CommonJS `module` of `module.exports`
34013402

34023403
/* @internal */
34033404
All = FunctionScopedVariable | BlockScopedVariable | Property | EnumMember | Function | Class | Interface | ConstEnum | RegularEnum | ValueModule | NamespaceModule | TypeLiteral

‎tests/baselines/reference/api/tsserverlibrary.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2049,6 +2049,7 @@ declare namespace ts {
20492049
Optional = 16777216,
20502050
Transient = 33554432,
20512051
JSContainer = 67108864,
2052+
ModuleExports = 134217728,
20522053
Enum = 384,
20532054
Variable = 3,
20542055
Value = 67216319,

‎tests/baselines/reference/api/typescript.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2049,6 +2049,7 @@ declare namespace ts {
20492049
Optional = 16777216,
20502050
Transient = 33554432,
20512051
JSContainer = 67108864,
2052+
ModuleExports = 134217728,
20522053
Enum = 384,
20532054
Variable = 3,
20542055
Value = 67216319,

‎tests/baselines/reference/callbackCrossModule.symbols

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* @return {any} I don't even know what this should return
55
*/
66
module.exports = C
7+
>module.exports : Symbol("tests/cases/conformance/jsdoc/mod1", Decl(mod1.js, 0, 0))
78
>module : Symbol(export=, Decl(mod1.js, 0, 0))
89
>exports : Symbol(export=, Decl(mod1.js, 0, 0))
910
>C : Symbol(C, Decl(mod1.js, 4, 18))

‎tests/baselines/reference/callbackCrossModule.types

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
*/
66
module.exports = C
77
>module.exports = C : typeof C
8-
>module.exports : any
9-
>module : any
10-
>exports : any
8+
>module.exports : typeof C
9+
>module : { "tests/cases/conformance/jsdoc/mod1": typeof C; }
10+
>exports : typeof C
1111
>C : typeof C
1212

1313
function C() {

‎tests/baselines/reference/checkJsTypeDefNoUnusedLocalMarked.symbols

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export = Foo;
2222
/** @typedef {(foo: Foo) => string} FooFun */
2323

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

‎tests/baselines/reference/checkJsTypeDefNoUnusedLocalMarked.types

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ export = Foo;
2323

2424
module.exports = /** @type {FooFun} */(void 0);
2525
>module.exports = /** @type {FooFun} */(void 0) : (foo: typeof Foo) => string
26-
>module.exports : any
27-
>module : any
28-
>exports : any
26+
>module.exports : (foo: typeof Foo) => string
27+
>module : { "tests/cases/compiler/something": (foo: typeof Foo) => string; }
28+
>exports : (foo: typeof Foo) => string
2929
>(void 0) : (foo: typeof Foo) => string
3030
>void 0 : undefined
3131
>0 : 0

‎tests/baselines/reference/conflictingCommonJSES2015Exports.symbols

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ export function abc(a, b, c) { return 5; }
66
>c : Symbol(c, Decl(bug24934.js, 0, 25))
77

88
module.exports = { abc };
9-
>module : Symbol(module)
9+
>module.exports : Symbol("tests/cases/conformance/salsa/bug24934", Decl(bug24934.js, 0, 0))
10+
>module : Symbol(module, Decl(bug24934.js, 0, 42))
11+
>exports : Symbol("tests/cases/conformance/salsa/bug24934", Decl(bug24934.js, 0, 0))
1012
>abc : Symbol(abc, Decl(bug24934.js, 1, 18))
1113

1214
=== tests/cases/conformance/salsa/use.js ===

‎tests/baselines/reference/conflictingCommonJSES2015Exports.types

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ export function abc(a, b, c) { return 5; }
77
>5 : 5
88

99
module.exports = { abc };
10-
>module.exports = { abc } : { abc: (a: any, b: any, c: any) => number; }
11-
>module.exports : any
12-
>module : any
13-
>exports : any
10+
>module.exports = { abc } : typeof import("tests/cases/conformance/salsa/bug24934")
11+
>module.exports : typeof import("tests/cases/conformance/salsa/bug24934")
12+
>module : { "tests/cases/conformance/salsa/bug24934": typeof import("tests/cases/conformance/salsa/bug24934"); }
13+
>exports : typeof import("tests/cases/conformance/salsa/bug24934")
1414
>{ abc } : { abc: (a: any, b: any, c: any) => number; }
1515
>abc : (a: any, b: any, c: any) => number
1616

‎tests/baselines/reference/constructorFunctions2.symbols

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ function A() { this.id = 1; }
5252
>id : Symbol(A.id, Decl(other.js, 0, 14))
5353

5454
module.exports = A;
55+
>module.exports : Symbol("tests/cases/conformance/salsa/other", Decl(other.js, 0, 0))
5556
>module : Symbol(export=, Decl(other.js, 0, 29))
5657
>exports : Symbol(export=, Decl(other.js, 0, 29))
5758
>A : Symbol(A, Decl(other.js, 0, 0))

‎tests/baselines/reference/constructorFunctions2.types

+3-3
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ function A() { this.id = 1; }
7070

7171
module.exports = A;
7272
>module.exports = A : typeof A
73-
>module.exports : any
74-
>module : any
75-
>exports : any
73+
>module.exports : typeof A
74+
>module : { "tests/cases/conformance/salsa/other": typeof A; }
75+
>exports : typeof A
7676
>A : typeof A
7777

‎tests/baselines/reference/contextualTypedSpecialAssignment.errors.txt

-89
This file was deleted.

0 commit comments

Comments
 (0)