Skip to content

Commit 39a980b

Browse files
elibarzilayelibarzilay
committed
Avoid the double-symbol trick for enums
Fixes #33575.
1 parent bae111f commit 39a980b

14 files changed

+118
-41
lines changed

‎src/compiler/binder.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ namespace ts {
541541
}
542542

543543
function declareModuleMember(node: Declaration, symbolFlags: SymbolFlags, symbolExcludes: SymbolFlags): Symbol {
544-
const hasExportModifier = getCombinedModifierFlags(node) & ModifierFlags.Export;
544+
const hasExportModifier = (getCombinedModifierFlags(node) & ModifierFlags.Export) || (isJSDocTypeAlias(node) && !!(node as JSDocTypedefTag).name);
545545
if (symbolFlags & SymbolFlags.Alias) {
546546
if (node.kind === SyntaxKind.ExportSpecifier || (node.kind === SyntaxKind.ImportEqualsDeclaration && hasExportModifier)) {
547547
return declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes);
@@ -567,7 +567,7 @@ namespace ts {
567567
// and this case is specially handled. Module augmentations should only be merged with original module definition
568568
// and should never be merged directly with other augmentation, and the latter case would be possible if automatic merge is allowed.
569569
if (isJSDocTypeAlias(node)) Debug.assert(isInJSFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
570-
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypeAlias(node)) {
570+
if (!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) {
571571
if (!container.locals || (hasSyntacticModifier(node, ModifierFlags.Default) && !getDeclarationName(node))) {
572572
return declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes); // No local symbol for an unnamed default!
573573
}
@@ -578,7 +578,10 @@ namespace ts {
578578
return local;
579579
}
580580
else {
581-
return declareSymbol(container.locals!, /*parent*/ undefined, node, symbolFlags, symbolExcludes);
581+
if (!container.locals) {
582+
container.locals = createSymbolTable();
583+
}
584+
return declareSymbol(container.locals, /*parent*/ undefined, node, symbolFlags, symbolExcludes);
582585
}
583586
}
584587
}

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

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
tests/cases/conformance/jsdoc/value.js(3,7): error TS2322: Type 'string' is not assignable to type '{ ADD: string; REMOVE: string; }'.
2+
3+
4+
==== tests/cases/conformance/jsdoc/type.js (0 errors) ====
5+
/** @typedef {import("./mod1").TestEnum} TE */
6+
/** @type {TE} */
7+
const test = 'add'
8+
/** @type {import("./mod1").TestEnum} */
9+
const tost = 'remove'
10+
11+
==== tests/cases/conformance/jsdoc/value.js (1 errors) ====
12+
import { TestEnum } from "./mod1"
13+
/** @type {TestEnum} */
14+
const tist = TestEnum.ADD
15+
~~~~
16+
!!! error TS2322: Type 'string' is not assignable to type '{ ADD: string; REMOVE: string; }'.
17+
18+
19+
==== tests/cases/conformance/jsdoc/mod1.js (0 errors) ====
20+
/** @enum {string} */
21+
export const TestEnum = {
22+
ADD: 'add',
23+
REMOVE: 'remove'
24+
}
25+

‎tests/baselines/reference/enumTagImported.symbols

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const tist = TestEnum.ADD
2323
=== tests/cases/conformance/jsdoc/mod1.js ===
2424
/** @enum {string} */
2525
export const TestEnum = {
26-
>TestEnum : Symbol(TestEnum, Decl(mod1.js, 1, 12), Decl(mod1.js, 0, 4))
26+
>TestEnum : Symbol(TestEnum, Decl(mod1.js, 1, 12))
2727

2828
ADD: 'add',
2929
>ADD : Symbol(ADD, Decl(mod1.js, 1, 25))

‎tests/baselines/reference/enumTagImported.types

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
/** @typedef {import("./mod1").TestEnum} TE */
33
/** @type {TE} */
44
const test = 'add'
5-
>test : string
5+
>test : any
66
>'add' : "add"
77

88
/** @type {import("./mod1").TestEnum} */
99
const tost = 'remove'
10-
>tost : string
10+
>tost : any
1111
>'remove' : "remove"
1212

1313
=== tests/cases/conformance/jsdoc/value.js ===
@@ -16,7 +16,7 @@ import { TestEnum } from "./mod1"
1616

1717
/** @type {TestEnum} */
1818
const tist = TestEnum.ADD
19-
>tist : string
19+
>tist : { ADD: string; REMOVE: string; }
2020
>TestEnum.ADD : string
2121
>TestEnum : { ADD: string; REMOVE: string; }
2222
>ADD : string

‎tests/baselines/reference/enumTagOnExports.symbols

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
=== tests/cases/conformance/jsdoc/enumTagOnExports.js ===
22
/** @enum {number} */
33
exports.a = {};
4-
>exports.a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8), Decl(enumTagOnExports.js, 0, 4))
5-
>exports : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8), Decl(enumTagOnExports.js, 0, 4))
6-
>a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8), Decl(enumTagOnExports.js, 0, 4))
4+
>exports.a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8))
5+
>exports : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8))
6+
>a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8))
77

88
/** @enum {string} */
99
module.exports.b = {};
10-
>module.exports.b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15), Decl(enumTagOnExports.js, 3, 4))
11-
>module.exports : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15), Decl(enumTagOnExports.js, 3, 4))
10+
>module.exports.b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15))
11+
>module.exports : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15))
1212
>module : Symbol(module, Decl(enumTagOnExports.js, 1, 15))
1313
>exports : Symbol("tests/cases/conformance/jsdoc/enumTagOnExports", Decl(enumTagOnExports.js, 0, 0))
14-
>b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15), Decl(enumTagOnExports.js, 3, 4))
14+
>b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15))
1515

‎tests/baselines/reference/exportedAliasedEnumTag.symbols

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ var middlewarify = module.exports = {};
77

88
/** @enum */
99
middlewarify.Type = {
10-
>middlewarify.Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13), Decl(exportedAliasedEnumTag.js, 2, 4))
10+
>middlewarify.Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13))
1111
>middlewarify : Symbol(middlewarify, Decl(exportedAliasedEnumTag.js, 0, 3))
12-
>Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13), Decl(exportedAliasedEnumTag.js, 2, 4))
12+
>Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13))
1313

1414
BEFORE: 'before'
1515
>BEFORE : Symbol(BEFORE, Decl(exportedAliasedEnumTag.js, 3, 21))

‎tests/baselines/reference/jsDeclarationsEnumTag.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,22 @@ exports.ff = ff;
111111
export function consume(t: Target, s: Second, f: Fs): void;
112112
/** @param {string} s */
113113
export function ff(s: string): any;
114-
export type Target = string;
115114
export namespace Target {
116115
const START: string;
117116
const MIDDLE: string;
118117
const END: string;
119118
const OK_I_GUESS: number;
120119
}
121-
export type Second = number;
122120
export namespace Second {
123121
const OK: number;
124122
const FINE: number;
125123
}
126-
export type Fs = (arg0: number) => number;
127124
export namespace Fs {
128125
function ADD1(n: any): any;
129126
function ID(n: any): any;
130127
function SUB1(n: any): number;
131128
}
129+
type Target = string;
130+
type Second = number;
131+
type Fs = (arg0: number) => number;
132+
export {};

‎tests/baselines/reference/jsDeclarationsEnumTag.symbols

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
=== tests/cases/conformance/jsdoc/declarations/index.js ===
22
/** @enum {string} */
33
export const Target = {
4-
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
4+
>Target : Symbol(Target, Decl(index.js, 1, 12))
55

66
START: "start",
77
>START : Symbol(START, Decl(index.js, 1, 23))
@@ -18,7 +18,7 @@ export const Target = {
1818
}
1919
/** @enum number */
2020
export const Second = {
21-
>Second : Symbol(Second, Decl(index.js, 9, 12), Decl(index.js, 8, 4))
21+
>Second : Symbol(Second, Decl(index.js, 9, 12))
2222

2323
OK: 1,
2424
>OK : Symbol(OK, Decl(index.js, 9, 23))
@@ -29,7 +29,7 @@ export const Second = {
2929
}
3030
/** @enum {function(number): number} */
3131
export const Fs = {
32-
>Fs : Symbol(Fs, Decl(index.js, 15, 12), Decl(index.js, 14, 4))
32+
>Fs : Symbol(Fs, Decl(index.js, 15, 12))
3333

3434
ADD1: n => n + 1,
3535
>ADD1 : Symbol(ADD1, Decl(index.js, 15, 19))
@@ -77,7 +77,7 @@ export function consume(t,s,f) {
7777
var v = Target.START
7878
>v : Symbol(v, Decl(index.js, 34, 7))
7979
>Target.START : Symbol(START, Decl(index.js, 1, 23))
80-
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
80+
>Target : Symbol(Target, Decl(index.js, 1, 12))
8181
>START : Symbol(START, Decl(index.js, 1, 23))
8282

8383
v = 'something else' // allowed, like Typescript's classic enums and unlike its string enums
@@ -90,14 +90,14 @@ export function ff(s) {
9090

9191
// element access with arbitrary string is an error only with noImplicitAny
9292
if (!Target[s]) {
93-
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
93+
>Target : Symbol(Target, Decl(index.js, 1, 12))
9494
>s : Symbol(s, Decl(index.js, 38, 19))
9595

9696
return null
9797
}
9898
else {
9999
return Target[s]
100-
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
100+
>Target : Symbol(Target, Decl(index.js, 1, 12))
101101
>s : Symbol(s, Decl(index.js, 38, 19))
102102
}
103103
}

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

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
tests/cases/compiler/index.js(8,21): error TS2345: Argument of type 'number' is not assignable to parameter of type '{ WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }'.
2+
tests/cases/compiler/index.js(20,5): error TS2322: Type 'string' is not assignable to type '{ x: number; }'.
3+
4+
5+
==== tests/cases/compiler/enumDef.js (0 errors) ====
6+
var Host = {};
7+
Host.UserMetrics = {};
8+
/** @enum {number} */
9+
Host.UserMetrics.Action = {
10+
WindowDocked: 1,
11+
WindowUndocked: 2,
12+
ScriptsBreakpointSet: 3,
13+
TimelineStarted: 4,
14+
};
15+
/**
16+
* @typedef {string} Host.UserMetrics.Bargh
17+
*/
18+
/**
19+
* @typedef {string}
20+
*/
21+
Host.UserMetrics.Blah = {
22+
x: 12
23+
}
24+
==== tests/cases/compiler/index.js (2 errors) ====
25+
var Other = {};
26+
Other.Cls = class {
27+
/**
28+
* @param {!Host.UserMetrics.Action} p
29+
*/
30+
method(p) {}
31+
usage() {
32+
this.method(Host.UserMetrics.Action.WindowDocked);
33+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
34+
!!! error TS2345: Argument of type 'number' is not assignable to parameter of type '{ WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }'.
35+
}
36+
}
37+
38+
/**
39+
* @type {Host.UserMetrics.Bargh}
40+
*/
41+
var x = "ok";
42+
43+
/**
44+
* @type {Host.UserMetrics.Blah}
45+
*/
46+
var y = "ok";
47+
~
48+
!!! error TS2322: Type 'string' is not assignable to type '{ x: number; }'.
49+

‎tests/baselines/reference/jsEnumCrossFileExport.symbols

+6-6
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ Host.UserMetrics = {};
99

1010
/** @enum {number} */
1111
Host.UserMetrics.Action = {
12-
>Host.UserMetrics.Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17), Decl(enumDef.js, 2, 4))
12+
>Host.UserMetrics.Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17))
1313
>Host.UserMetrics : Symbol(Host.UserMetrics, Decl(enumDef.js, 0, 14), Decl(enumDef.js, 3, 5), Decl(enumDef.js, 15, 5), Decl(enumDef.js, 10, 26))
1414
>Host : Symbol(Host, Decl(enumDef.js, 0, 3), Decl(enumDef.js, 0, 14), Decl(enumDef.js, 1, 22), Decl(enumDef.js, 8, 2), Decl(enumDef.js, 10, 21))
1515
>UserMetrics : Symbol(Host.UserMetrics, Decl(enumDef.js, 0, 14), Decl(enumDef.js, 3, 5), Decl(enumDef.js, 15, 5), Decl(enumDef.js, 10, 26))
16-
>Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17), Decl(enumDef.js, 2, 4))
16+
>Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17))
1717

1818
WindowDocked: 1,
1919
>WindowDocked : Symbol(WindowDocked, Decl(enumDef.js, 3, 27))
@@ -35,11 +35,11 @@ Host.UserMetrics.Action = {
3535
* @typedef {string}
3636
*/
3737
Host.UserMetrics.Blah = {
38-
>Host.UserMetrics.Blah : Symbol(Host.UserMetrics.Blah, Decl(enumDef.js, 8, 2), Decl(enumDef.js, 15, 17), Decl(enumDef.js, 13, 3))
38+
>Host.UserMetrics.Blah : Symbol(Host.UserMetrics.Blah, Decl(enumDef.js, 8, 2), Decl(enumDef.js, 15, 17))
3939
>Host.UserMetrics : Symbol(Host.UserMetrics, Decl(enumDef.js, 0, 14), Decl(enumDef.js, 3, 5), Decl(enumDef.js, 15, 5), Decl(enumDef.js, 10, 26))
4040
>Host : Symbol(Host, Decl(enumDef.js, 0, 3), Decl(enumDef.js, 0, 14), Decl(enumDef.js, 1, 22), Decl(enumDef.js, 8, 2), Decl(enumDef.js, 10, 21))
4141
>UserMetrics : Symbol(Host.UserMetrics, Decl(enumDef.js, 0, 14), Decl(enumDef.js, 3, 5), Decl(enumDef.js, 15, 5), Decl(enumDef.js, 10, 26))
42-
>Blah : Symbol(Host.UserMetrics.Blah, Decl(enumDef.js, 8, 2), Decl(enumDef.js, 15, 17), Decl(enumDef.js, 13, 3))
42+
>Blah : Symbol(Host.UserMetrics.Blah, Decl(enumDef.js, 8, 2), Decl(enumDef.js, 15, 17))
4343

4444
x: 12
4545
>x : Symbol(x, Decl(enumDef.js, 15, 25))
@@ -68,11 +68,11 @@ Other.Cls = class {
6868
>this : Symbol(Cls, Decl(index.js, 1, 11))
6969
>method : Symbol(Cls.method, Decl(index.js, 1, 19))
7070
>Host.UserMetrics.Action.WindowDocked : Symbol(WindowDocked, Decl(enumDef.js, 3, 27))
71-
>Host.UserMetrics.Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17), Decl(enumDef.js, 2, 4))
71+
>Host.UserMetrics.Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17))
7272
>Host.UserMetrics : Symbol(Host.UserMetrics, Decl(enumDef.js, 0, 14), Decl(enumDef.js, 3, 5), Decl(enumDef.js, 15, 5), Decl(enumDef.js, 10, 26))
7373
>Host : Symbol(Host, Decl(enumDef.js, 0, 3), Decl(enumDef.js, 0, 14), Decl(enumDef.js, 1, 22), Decl(enumDef.js, 8, 2), Decl(enumDef.js, 10, 21))
7474
>UserMetrics : Symbol(Host.UserMetrics, Decl(enumDef.js, 0, 14), Decl(enumDef.js, 3, 5), Decl(enumDef.js, 15, 5), Decl(enumDef.js, 10, 26))
75-
>Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17), Decl(enumDef.js, 2, 4))
75+
>Action : Symbol(Host.UserMetrics.Action, Decl(enumDef.js, 1, 22), Decl(enumDef.js, 3, 17))
7676
>WindowDocked : Symbol(WindowDocked, Decl(enumDef.js, 3, 27))
7777
}
7878
}

‎tests/baselines/reference/jsEnumCrossFileExport.types

+5-5
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,17 @@ Other.Cls = class {
7272
* @param {!Host.UserMetrics.Action} p
7373
*/
7474
method(p) {}
75-
>method : (p: Host.UserMetrics.Action) => void
76-
>p : number
75+
>method : (p: { WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }) => void
76+
>p : { WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }
7777

7878
usage() {
7979
>usage : () => void
8080

8181
this.method(Host.UserMetrics.Action.WindowDocked);
8282
>this.method(Host.UserMetrics.Action.WindowDocked) : void
83-
>this.method : (p: number) => void
83+
>this.method : (p: { WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }) => void
8484
>this : this
85-
>method : (p: number) => void
85+
>method : (p: { WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }) => void
8686
>Host.UserMetrics.Action.WindowDocked : number
8787
>Host.UserMetrics.Action : { WindowDocked: number; WindowUndocked: number; ScriptsBreakpointSet: number; TimelineStarted: number; }
8888
>Host.UserMetrics : typeof Host.UserMetrics
@@ -104,6 +104,6 @@ var x = "ok";
104104
* @type {Host.UserMetrics.Blah}
105105
*/
106106
var y = "ok";
107-
>y : string
107+
>y : { x: number; }
108108
>"ok" : "ok"
109109

‎tests/baselines/reference/jsEnumTagOnObjectFrozen.symbols

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ const Thing = Object.freeze({
5555
});
5656

5757
exports.Thing = Thing;
58-
>exports.Thing : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
59-
>exports : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
60-
>Thing : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
58+
>exports.Thing : Symbol(Thing, Decl(index.js, 4, 3))
59+
>exports : Symbol(Thing, Decl(index.js, 4, 3))
60+
>Thing : Symbol(Thing, Decl(index.js, 4, 3))
6161
>Thing : Symbol(Thing, Decl(index.js, 1, 5), Decl(index.js, 0, 4))
6262

6363
/**

‎tests/baselines/reference/jsdocTypedefNoCrash.symbols

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44
* }}
55
*/
66
export const foo = 5;
7-
>foo : Symbol(foo, Decl(export.js, 4, 12), Decl(export.js, 1, 3))
7+
>foo : Symbol(foo, Decl(export.js, 4, 12))
88

‎tests/cases/fourslash/quickInfoJSExport.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
//// export { test/**/String };
1616

1717
verify.quickInfoAt("",
18-
`type testString = string
19-
(alias) type testString = any
18+
`(alias) type testString = string
2019
(alias) const testString: {
2120
one: string;
2221
two: string;

0 commit comments

Comments
 (0)