Skip to content

Commit 7b684c5

Browse files
andrewbranchandrewbranch
committed
Audit all resolveSymbol and resolveName calls
1 parent d199f2c commit 7b684c5

36 files changed

+680
-64
lines changed

‎src/compiler/checker.ts

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,12 +3248,25 @@ namespace ts {
32483248
*/
32493249
function getAllSymbolFlags(symbol: Symbol): SymbolFlags | undefined {
32503250
let flags = symbol.flags;
3251+
let seenSymbols;
32513252
while (symbol.flags & SymbolFlags.Alias) {
3252-
symbol = resolveAlias(symbol);
3253-
if (symbol === unknownSymbol) {
3253+
const target = resolveAlias(symbol);
3254+
if (target === unknownSymbol) {
32543255
return undefined;
32553256
}
3256-
flags |= symbol.flags;
3257+
3258+
// Optimization - avoid creating `seenSymbols` Set in most cases
3259+
if (target === symbol) {
3260+
break;
3261+
}
3262+
else if (target.flags & SymbolFlags.Alias && !seenSymbols) {
3263+
seenSymbols = new Set([symbol, target]);
3264+
}
3265+
else if (seenSymbols?.has(target)) {
3266+
break;
3267+
}
3268+
flags |= target.flags;
3269+
symbol = target;
32573270
}
32583271
return flags;
32593272
}
@@ -3350,8 +3363,8 @@ namespace ts {
33503363
// This way a chain of imports can be elided if ultimately the final input is only used in a type
33513364
// position.
33523365
if (isInternalModuleImportEqualsDeclaration(node)) {
3353-
const target = resolveSymbol(symbol);
3354-
if (target === unknownSymbol || target.flags & SymbolFlags.Value) {
3366+
const targetFlags = getAllSymbolFlags(resolveSymbol(symbol));
3367+
if (targetFlags === undefined || targetFlags & SymbolFlags.Value) {
33553368
// import foo = <symbol>
33563369
checkExpressionCached(node.moduleReference as Expression);
33573370
}
@@ -4277,7 +4290,7 @@ namespace ts {
42774290
function symbolIsValue(symbol: Symbol, includeTypeOnlyMembers?: boolean): boolean {
42784291
return !!(
42794292
symbol.flags & SymbolFlags.Value ||
4280-
symbol.flags & SymbolFlags.Alias && resolveAlias(symbol).flags & SymbolFlags.Value && (includeTypeOnlyMembers || !getTypeOnlyAliasDeclaration(symbol)));
4293+
symbol.flags & SymbolFlags.Alias && (getAllSymbolFlags(symbol) ?? -1) & SymbolFlags.Value && (includeTypeOnlyMembers || !getTypeOnlyAliasDeclaration(symbol)));
42814294
}
42824295

42834296
function findConstructorDeclaration(node: ClassLikeDeclaration): ConstructorDeclaration | undefined {
@@ -4572,7 +4585,7 @@ namespace ts {
45724585

45734586
// Qualify if the symbol from symbol table has same meaning as expected
45744587
symbolFromSymbolTable = (symbolFromSymbolTable.flags & SymbolFlags.Alias && !getDeclarationOfKind(symbolFromSymbolTable, SyntaxKind.ExportSpecifier)) ? resolveAlias(symbolFromSymbolTable) : symbolFromSymbolTable;
4575-
if (symbolFromSymbolTable.flags & meaning) {
4588+
if ((getAllSymbolFlags(symbolFromSymbolTable) ?? unknownSymbol.flags) & meaning) {
45764589
qualify = true;
45774590
return true;
45784591
}
@@ -7535,7 +7548,7 @@ namespace ts {
75357548
}
75367549

75377550
function isTypeOnlyNamespace(symbol: Symbol) {
7538-
return every(getNamespaceMembersForSerialization(symbol), m => !(resolveSymbol(m).flags & SymbolFlags.Value));
7551+
return every(getNamespaceMembersForSerialization(symbol), m => !((getAllSymbolFlags(resolveSymbol(m)) ?? unknownSymbol.flags) & SymbolFlags.Value));
75397552
}
75407553

75417554
function serializeModule(symbol: Symbol, symbolName: string, modifierFlags: ModifierFlags) {
@@ -10013,7 +10026,7 @@ namespace ts {
1001310026
links.type = exportSymbol?.declarations && isDuplicatedCommonJSExport(exportSymbol.declarations) && symbol.declarations!.length ? getFlowTypeFromCommonJSExport(exportSymbol)
1001410027
: isDuplicatedCommonJSExport(symbol.declarations) ? autoType
1001510028
: declaredType ? declaredType
10016-
: targetSymbol.flags & SymbolFlags.Value ? getTypeOfSymbol(targetSymbol)
10029+
: (getAllSymbolFlags(targetSymbol) ?? -1) & SymbolFlags.Value ? getTypeOfSymbol(targetSymbol)
1001710030
: errorType;
1001810031
}
1001910032
return links.type;
@@ -32350,7 +32363,7 @@ namespace ts {
3235032363
if (symbol && symbol.flags & SymbolFlags.Alias) {
3235132364
symbol = resolveAlias(symbol);
3235232365
}
32353-
return !!(symbol && (symbol.flags & SymbolFlags.Enum) && getEnumKind(symbol) === EnumKind.Literal);
32366+
return !!(symbol && ((getAllSymbolFlags(symbol) ?? unknownSymbol.flags) & SymbolFlags.Enum) && getEnumKind(symbol) === EnumKind.Literal);
3235432367
}
3235532368
return false;
3235632369
}
@@ -41460,14 +41473,15 @@ namespace ts {
4146041473
if (node.moduleReference.kind !== SyntaxKind.ExternalModuleReference) {
4146141474
const target = resolveAlias(getSymbolOfNode(node));
4146241475
if (target !== unknownSymbol) {
41463-
if (target.flags & SymbolFlags.Value) {
41476+
const targetFlags = getAllSymbolFlags(target) ?? unknownSymbol.flags;
41477+
if (targetFlags & SymbolFlags.Value) {
4146441478
// Target is a value symbol, check that it is not hidden by a local declaration with the same name
4146541479
const moduleName = getFirstIdentifier(node.moduleReference);
4146641480
if (!(resolveEntityName(moduleName, SymbolFlags.Value | SymbolFlags.Namespace)!.flags & SymbolFlags.Namespace)) {
4146741481
error(moduleName, Diagnostics.Module_0_is_hidden_by_a_local_declaration_with_the_same_name, declarationNameToString(moduleName));
4146841482
}
4146941483
}
41470-
if (target.flags & SymbolFlags.Type) {
41484+
if (targetFlags & SymbolFlags.Type) {
4147141485
checkTypeNameIsReserved(node.name, Diagnostics.Import_name_cannot_be_0);
4147241486
}
4147341487
}
@@ -41618,7 +41632,7 @@ namespace ts {
4161841632
markExportAsReferenced(node);
4161941633
}
4162041634
const target = symbol && (symbol.flags & SymbolFlags.Alias ? resolveAlias(symbol) : symbol);
41621-
if (!target || target === unknownSymbol || target.flags & SymbolFlags.Value) {
41635+
if (!target || (getAllSymbolFlags(target) ?? -1) & SymbolFlags.Value) {
4162241636
checkExpressionCached(node.propertyName || node.name);
4162341637
}
4162441638
}
@@ -41670,7 +41684,7 @@ namespace ts {
4167041684
markAliasReferenced(sym, id);
4167141685
// If not a value, we're interpreting the identifier as a type export, along the lines of (`export { Id as default }`)
4167241686
const target = sym.flags & SymbolFlags.Alias ? resolveAlias(sym) : sym;
41673-
if (target === unknownSymbol || target.flags & SymbolFlags.Value) {
41687+
if ((getAllSymbolFlags(target) ?? -1) & SymbolFlags.Value) {
4167441688
// However if it is a value, we need to check it's being used correctly
4167541689
checkExpressionCached(node.expression);
4167641690
}
@@ -43099,7 +43113,7 @@ namespace ts {
4309943113

4310043114
function isValue(s: Symbol): boolean {
4310143115
s = resolveSymbol(s);
43102-
return s && !!(s.flags & SymbolFlags.Value);
43116+
return s && !!((getAllSymbolFlags(s) ?? unknownSymbol.flags) & SymbolFlags.Value);
4310343117
}
4310443118
}
4310543119

‎tests/baselines/reference/allowImportClausesToMergeWithTypes.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ b_1["default"];
4444
//// [index.js]
4545
"use strict";
4646
exports.__esModule = true;
47+
var a_1 = require("./a");
4748
var x = { x: "" };
4849
a_1["default"];
4950
var b_1 = require("./b");

‎tests/baselines/reference/allowImportClausesToMergeWithTypes.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export { zzz as default };
3030

3131
=== tests/cases/compiler/index.ts ===
3232
import zzz from "./a";
33-
>zzz : any
33+
>zzz : 123
3434

3535
const x: zzz = { x: "" };
3636
>x : zzz
@@ -39,7 +39,7 @@ const x: zzz = { x: "" };
3939
>"" : ""
4040

4141
zzz;
42-
>zzz : any
42+
>zzz : 123
4343

4444
import originalZZZ from "./b";
4545
>originalZZZ : 123

‎tests/baselines/reference/exportSpecifierForAGlobal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,4 @@ exports.f = f;
2424

2525
//// [b.d.ts]
2626
export { X };
27-
export declare function f(): X;
27+
export declare function f(): globalThis.X;

‎tests/baselines/reference/exportSpecifierForAGlobal.types

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ declare class X { }
44

55
=== tests/cases/compiler/b.ts ===
66
export {X};
7-
>X : typeof X
7+
>X : typeof globalThis.X
88

99
export function f() {
10-
>f : () => X
10+
>f : () => globalThis.X
1111

1212
var x: X;
13-
>x : X
13+
>x : globalThis.X
1414

1515
return x;
16-
>x : X
16+
>x : globalThis.X
1717
}
1818

‎tests/baselines/reference/exportSpecifierReferencingOuterDeclaration1.symbols

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@ declare module "m" {
1212
export function foo(): X.bar;
1313
>foo : Symbol(foo, Decl(exportSpecifierReferencingOuterDeclaration1.ts, 2, 17))
1414
>X : Symbol(X, Decl(exportSpecifierReferencingOuterDeclaration1.ts, 0, 0))
15-
>bar : Symbol(X.bar, Decl(exportSpecifierReferencingOuterDeclaration1.ts, 0, 18))
15+
>bar : Symbol(globalThis.X.bar, Decl(exportSpecifierReferencingOuterDeclaration1.ts, 0, 18))
1616
}

‎tests/baselines/reference/exportSpecifierReferencingOuterDeclaration2.symbols

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ export { X };
1010
export declare function foo(): X.bar;
1111
>foo : Symbol(foo, Decl(exportSpecifierReferencingOuterDeclaration2_B.ts, 0, 13))
1212
>X : Symbol(X, Decl(exportSpecifierReferencingOuterDeclaration2_A.ts, 0, 0))
13-
>bar : Symbol(X.bar, Decl(exportSpecifierReferencingOuterDeclaration2_A.ts, 0, 18))
13+
>bar : Symbol(globalThis.X.bar, Decl(exportSpecifierReferencingOuterDeclaration2_A.ts, 0, 18))
1414

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
tests/cases/conformance/constEnums/merge.ts(1,10): error TS2440: Import declaration conflicts with local declaration of 'Enum'.
2+
3+
4+
==== tests/cases/conformance/constEnums/enum.ts (0 errors) ====
5+
export const enum Enum {
6+
One = 1,
7+
}
8+
9+
==== tests/cases/conformance/constEnums/merge.ts (1 errors) ====
10+
import { Enum } from "./enum";
11+
~~~~
12+
!!! error TS2440: Import declaration conflicts with local declaration of 'Enum'.
13+
namespace Enum {
14+
export type Foo = number;
15+
}
16+
export { Enum };
17+
18+
==== tests/cases/conformance/constEnums/index.ts (0 errors) ====
19+
import { Enum } from "./merge";
20+
Enum.One;
21+

‎tests/baselines/reference/importElisionConstEnumMerge1.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//// [tests/cases/conformance/constEnums/importElisionConstEnumMerge1.ts] ////
2+
3+
//// [enum.ts]
4+
export const enum Enum {
5+
One = 1,
6+
}
7+
8+
//// [merge.ts]
9+
import { Enum } from "./enum";
10+
namespace Enum {
11+
export type Foo = number;
12+
}
13+
export { Enum };
14+
15+
//// [index.ts]
16+
import { Enum } from "./merge";
17+
Enum.One;
18+
19+
20+
//// [enum.js]
21+
"use strict";
22+
exports.__esModule = true;
23+
//// [merge.js]
24+
"use strict";
25+
exports.__esModule = true;
26+
exports.Enum = void 0;
27+
//// [index.js]
28+
"use strict";
29+
exports.__esModule = true;
30+
var merge_1 = require("./merge");
31+
1 /* Enum.One */;

‎tests/baselines/reference/importElisionConstEnumMerge1.symbols

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
=== tests/cases/conformance/constEnums/enum.ts ===
2+
export const enum Enum {
3+
>Enum : Symbol(Enum, Decl(enum.ts, 0, 0))
4+
5+
One = 1,
6+
>One : Symbol(Enum.One, Decl(enum.ts, 0, 24))
7+
}
8+
9+
=== tests/cases/conformance/constEnums/merge.ts ===
10+
import { Enum } from "./enum";
11+
>Enum : Symbol(Enum, Decl(merge.ts, 0, 8), Decl(merge.ts, 0, 30))
12+
13+
namespace Enum {
14+
>Enum : Symbol(Enum, Decl(merge.ts, 0, 8), Decl(merge.ts, 0, 30))
15+
16+
export type Foo = number;
17+
>Foo : Symbol(Foo, Decl(merge.ts, 1, 16))
18+
}
19+
export { Enum };
20+
>Enum : Symbol(Enum, Decl(merge.ts, 4, 8))
21+
22+
=== tests/cases/conformance/constEnums/index.ts ===
23+
import { Enum } from "./merge";
24+
>Enum : Symbol(Enum, Decl(index.ts, 0, 8))
25+
26+
Enum.One;
27+
>Enum.One : Symbol(Enum.One, Decl(enum.ts, 0, 24))
28+
>Enum : Symbol(Enum, Decl(index.ts, 0, 8))
29+
>One : Symbol(Enum.One, Decl(enum.ts, 0, 24))
30+

‎tests/baselines/reference/importElisionConstEnumMerge1.types

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/conformance/constEnums/enum.ts ===
2+
export const enum Enum {
3+
>Enum : Enum
4+
5+
One = 1,
6+
>One : Enum.One
7+
>1 : 1
8+
}
9+
10+
=== tests/cases/conformance/constEnums/merge.ts ===
11+
import { Enum } from "./enum";
12+
>Enum : typeof Enum
13+
14+
namespace Enum {
15+
export type Foo = number;
16+
>Foo : number
17+
}
18+
export { Enum };
19+
>Enum : typeof Enum
20+
21+
=== tests/cases/conformance/constEnums/index.ts ===
22+
import { Enum } from "./merge";
23+
>Enum : typeof import("tests/cases/conformance/constEnums/enum").Enum
24+
25+
Enum.One;
26+
>Enum.One : import("tests/cases/conformance/constEnums/enum").Enum
27+
>Enum : typeof import("tests/cases/conformance/constEnums/enum").Enum
28+
>One : import("tests/cases/conformance/constEnums/enum").Enum
29+

‎tests/baselines/reference/jsDeclarationsImportAliasExposedWithinNamespace.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export {myTypes};
2626

2727
=== tests/cases/conformance/jsdoc/declarations/file2.js ===
2828
import {myTypes} from './file.js';
29-
>myTypes : any
29+
>myTypes : { [x: string]: any; }
3030

3131
/**
3232
* @namespace testFnTypes

‎tests/baselines/reference/noCrashOnImportShadowing.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,15 @@ exports.zzz = 123;
3434
//// [a.js]
3535
"use strict";
3636
exports.__esModule = true;
37+
exports.B = void 0;
3738
var B = require("./b");
39+
exports.B = B;
3840
var x = { x: "" };
3941
B.zzz;
4042
//// [index.js]
4143
"use strict";
4244
exports.__esModule = true;
45+
var a_1 = require("./a");
4346
var x = { x: "" };
4447
a_1.B.zzz;
4548
var OriginalB = require("./b");

‎tests/baselines/reference/noCrashOnImportShadowing.symbols

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ const x: B = { x: "" };
3636
>x : Symbol(x, Decl(index.ts, 2, 14))
3737

3838
B.zzz;
39+
>B.zzz : Symbol(OriginalB.zzz, Decl(b.ts, 0, 12))
3940
>B : Symbol(B, Decl(index.ts, 0, 8))
41+
>zzz : Symbol(OriginalB.zzz, Decl(b.ts, 0, 12))
4042

4143
import * as OriginalB from "./b";
4244
>OriginalB : Symbol(OriginalB, Decl(index.ts, 5, 6))

‎tests/baselines/reference/noCrashOnImportShadowing.types

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ B.zzz;
2424
>zzz : 123
2525

2626
export { B };
27-
>B : any
27+
>B : typeof B
2828

2929
=== tests/cases/compiler/index.ts ===
3030
import { B } from "./a";
31-
>B : any
31+
>B : typeof OriginalB
3232

3333
const x: B = { x: "" };
3434
>x : B
@@ -37,9 +37,9 @@ const x: B = { x: "" };
3737
>"" : ""
3838

3939
B.zzz;
40-
>B.zzz : any
41-
>B : any
42-
>zzz : any
40+
>B.zzz : 123
41+
>B : typeof OriginalB
42+
>zzz : 123
4343

4444
import * as OriginalB from "./b";
4545
>OriginalB : typeof OriginalB

0 commit comments

Comments
 (0)