Skip to content

Commit

Permalink
Avoid the double-symbol trick for enums
Browse files Browse the repository at this point in the history
  • Loading branch information
elibarzilay committed Aug 7, 2020
1 parent bae111f commit 6eb3daa
Show file tree
Hide file tree
Showing 16 changed files with 105 additions and 46 deletions.
3 changes: 2 additions & 1 deletion src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,8 @@ 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 (isJSDocTypeAlias(node)) Debug.assert(isInJSFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypeAlias(node)) {
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext))
|| (isJSDocTypeAlias(node) && (isJSDocTypedefTag(node) || !container.locals))) {
if (!container.locals || (hasSyntacticModifier(node, ModifierFlags.Default) && !getDeclarationName(node))) {
return declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes); // No local symbol for an unnamed default!
}
Expand Down
25 changes: 25 additions & 0 deletions tests/baselines/reference/callbackCrossModule.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
tests/cases/conformance/jsdoc/use.js(1,30): error TS2694: Namespace 'C' has no exported member 'Con'.


==== tests/cases/conformance/jsdoc/mod1.js (0 errors) ====
/** @callback Con - some kind of continuation
* @param {object | undefined} error
* @return {any} I don't even know what this should return
*/
module.exports = C
function C() {
this.p = 1
}

==== tests/cases/conformance/jsdoc/use.js (1 errors) ====
/** @param {import('./mod1').Con} k */
~~~
!!! error TS2694: Namespace 'C' has no exported member 'Con'.
function f(k) {
if (1 === 2 - 1) {
// I guess basic math works!
}
return k({ ok: true})
}


6 changes: 3 additions & 3 deletions tests/baselines/reference/callbackCrossModule.types
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ function C() {
=== tests/cases/conformance/jsdoc/use.js ===
/** @param {import('./mod1').Con} k */
function f(k) {
>f : (k: import('./mod1').Con) => any
>k : import("tests/cases/conformance/jsdoc/mod1").Con
>f : (k: any) => any
>k : any

if (1 === 2 - 1) {
>1 === 2 - 1 : boolean
Expand All @@ -38,7 +38,7 @@ function f(k) {
}
return k({ ok: true})
>k({ ok: true}) : any
>k : import("tests/cases/conformance/jsdoc/mod1").Con
>k : any
>{ ok: true} : { ok: boolean; }
>ok : boolean
>true : true
Expand Down
21 changes: 21 additions & 0 deletions tests/baselines/reference/callbackTagNamespace.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
tests/cases/conformance/jsdoc/namespaced.js(8,5): error TS8030: The type of a function declaration must match the function's signature.
tests/cases/conformance/jsdoc/namespaced.js(8,22): error TS2694: Namespace 'NS.Nested' has no exported member 'Inner'.


==== tests/cases/conformance/jsdoc/namespaced.js (2 errors) ====
/**
* @callback NS.Nested.Inner
* @param {Object} space - spaaaaaaaaace
* @param {Object} peace - peaaaaaaaaace
* @return {string | number}
*/
var x = 1;
/** @type {NS.Nested.Inner} */
~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS8030: The type of a function declaration must match the function's signature.
~~~~~
!!! error TS2694: Namespace 'NS.Nested' has no exported member 'Inner'.
function f(space, peace) {
return '1'
}

2 changes: 1 addition & 1 deletion tests/baselines/reference/callbackTagNamespace.types
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var x = 1;

/** @type {NS.Nested.Inner} */
function f(space, peace) {
>f : (space: any, peace: any) => string | number
>f : (space: any, peace: any) => string
>space : any
>peace : any

Expand Down
25 changes: 25 additions & 0 deletions tests/baselines/reference/enumTagImported.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
tests/cases/conformance/jsdoc/value.js(3,7): error TS2322: Type 'string' is not assignable to type '{ ADD: string; REMOVE: string; }'.


==== tests/cases/conformance/jsdoc/type.js (0 errors) ====
/** @typedef {import("./mod1").TestEnum} TE */
/** @type {TE} */
const test = 'add'
/** @type {import("./mod1").TestEnum} */
const tost = 'remove'

==== tests/cases/conformance/jsdoc/value.js (1 errors) ====
import { TestEnum } from "./mod1"
/** @type {TestEnum} */
const tist = TestEnum.ADD
~~~~
!!! error TS2322: Type 'string' is not assignable to type '{ ADD: string; REMOVE: string; }'.


==== tests/cases/conformance/jsdoc/mod1.js (0 errors) ====
/** @enum {string} */
export const TestEnum = {
ADD: 'add',
REMOVE: 'remove'
}

2 changes: 1 addition & 1 deletion tests/baselines/reference/enumTagImported.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const tist = TestEnum.ADD
=== tests/cases/conformance/jsdoc/mod1.js ===
/** @enum {string} */
export const TestEnum = {
>TestEnum : Symbol(TestEnum, Decl(mod1.js, 1, 12), Decl(mod1.js, 0, 4))
>TestEnum : Symbol(TestEnum, Decl(mod1.js, 1, 12))

ADD: 'add',
>ADD : Symbol(ADD, Decl(mod1.js, 1, 25))
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/enumTagImported.types
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
/** @typedef {import("./mod1").TestEnum} TE */
/** @type {TE} */
const test = 'add'
>test : string
>test : any
>'add' : "add"

/** @type {import("./mod1").TestEnum} */
const tost = 'remove'
>tost : string
>tost : any
>'remove' : "remove"

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

/** @type {TestEnum} */
const tist = TestEnum.ADD
>tist : string
>tist : { ADD: string; REMOVE: string; }
>TestEnum.ADD : string
>TestEnum : { ADD: string; REMOVE: string; }
>ADD : string
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/enumTagOnExports.symbols
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
=== tests/cases/conformance/jsdoc/enumTagOnExports.js ===
/** @enum {number} */
exports.a = {};
>exports.a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8), Decl(enumTagOnExports.js, 0, 4))
>exports : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8), Decl(enumTagOnExports.js, 0, 4))
>a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8), Decl(enumTagOnExports.js, 0, 4))
>exports.a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8))
>exports : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8))
>a : Symbol(a, Decl(enumTagOnExports.js, 0, 0), Decl(enumTagOnExports.js, 1, 8))

/** @enum {string} */
module.exports.b = {};
>module.exports.b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15), Decl(enumTagOnExports.js, 3, 4))
>module.exports : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15), Decl(enumTagOnExports.js, 3, 4))
>module.exports.b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15))
>module.exports : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15))
>module : Symbol(module, Decl(enumTagOnExports.js, 1, 15))
>exports : Symbol("tests/cases/conformance/jsdoc/enumTagOnExports", Decl(enumTagOnExports.js, 0, 0))
>b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15), Decl(enumTagOnExports.js, 3, 4))
>b : Symbol(b, Decl(enumTagOnExports.js, 1, 15), Decl(enumTagOnExports.js, 4, 15))

4 changes: 2 additions & 2 deletions tests/baselines/reference/exportedAliasedEnumTag.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ var middlewarify = module.exports = {};

/** @enum */
middlewarify.Type = {
>middlewarify.Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13), Decl(exportedAliasedEnumTag.js, 2, 4))
>middlewarify.Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13))
>middlewarify : Symbol(middlewarify, Decl(exportedAliasedEnumTag.js, 0, 3))
>Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13), Decl(exportedAliasedEnumTag.js, 2, 4))
>Type : Symbol(Type, Decl(exportedAliasedEnumTag.js, 0, 39), Decl(exportedAliasedEnumTag.js, 3, 13))

BEFORE: 'before'
>BEFORE : Symbol(BEFORE, Decl(exportedAliasedEnumTag.js, 3, 21))
Expand Down
7 changes: 4 additions & 3 deletions tests/baselines/reference/jsDeclarationsEnumTag.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,21 +111,22 @@ exports.ff = ff;
export function consume(t: Target, s: Second, f: Fs): void;
/** @param {string} s */
export function ff(s: string): any;
export type Target = string;
export namespace Target {
const START: string;
const MIDDLE: string;
const END: string;
const OK_I_GUESS: number;
}
export type Second = number;
export namespace Second {
const OK: number;
const FINE: number;
}
export type Fs = (arg0: number) => number;
export namespace Fs {
function ADD1(n: any): any;
function ID(n: any): any;
function SUB1(n: any): number;
}
type Target = string;
type Second = number;
type Fs = (arg0: number) => number;
export {};
12 changes: 6 additions & 6 deletions tests/baselines/reference/jsDeclarationsEnumTag.symbols
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
=== tests/cases/conformance/jsdoc/declarations/index.js ===
/** @enum {string} */
export const Target = {
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
>Target : Symbol(Target, Decl(index.js, 1, 12))

START: "start",
>START : Symbol(START, Decl(index.js, 1, 23))
Expand All @@ -18,7 +18,7 @@ export const Target = {
}
/** @enum number */
export const Second = {
>Second : Symbol(Second, Decl(index.js, 9, 12), Decl(index.js, 8, 4))
>Second : Symbol(Second, Decl(index.js, 9, 12))

OK: 1,
>OK : Symbol(OK, Decl(index.js, 9, 23))
Expand All @@ -29,7 +29,7 @@ export const Second = {
}
/** @enum {function(number): number} */
export const Fs = {
>Fs : Symbol(Fs, Decl(index.js, 15, 12), Decl(index.js, 14, 4))
>Fs : Symbol(Fs, Decl(index.js, 15, 12))

ADD1: n => n + 1,
>ADD1 : Symbol(ADD1, Decl(index.js, 15, 19))
Expand Down Expand Up @@ -77,7 +77,7 @@ export function consume(t,s,f) {
var v = Target.START
>v : Symbol(v, Decl(index.js, 34, 7))
>Target.START : Symbol(START, Decl(index.js, 1, 23))
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
>Target : Symbol(Target, Decl(index.js, 1, 12))
>START : Symbol(START, Decl(index.js, 1, 23))

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

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

return null
}
else {
return Target[s]
>Target : Symbol(Target, Decl(index.js, 1, 12), Decl(index.js, 0, 4))
>Target : Symbol(Target, Decl(index.js, 1, 12))
>s : Symbol(s, Decl(index.js, 38, 19))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ declare class MyClass {
method(): void;
}
declare namespace MyClass {
export { staticMethod, staticProperty, DoneCB };
function staticMethod(): void;
const staticProperty: number;
}
declare function staticMethod(): void;
declare var staticProperty: number;
/**
* Callback to be invoked when test execution is complete.
*/
type DoneCB = (failures: number) => any;
8 changes: 0 additions & 8 deletions tests/baselines/reference/jsDeclarationsTypeAliases.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,9 @@ var LocalThing = /** @class */ (function () {

//// [index.d.ts]
export type PropName = string | number | symbol;
/**
* Callback
*/
export type NumberToStringCb = (a: number) => string;
export type MixinName<T> = T & {
name: string;
};
/**
* Identity function
*/
export type Identity<T> = (x: T) => T;
//// [mixed.d.ts]
export type SomeType = number | {
x: string;
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/jsEnumTagOnObjectFrozen.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ const Thing = Object.freeze({
});

exports.Thing = Thing;
>exports.Thing : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
>exports : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
>Thing : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
>exports.Thing : Symbol(Thing, Decl(index.js, 4, 3))
>exports : Symbol(Thing, Decl(index.js, 4, 3))
>Thing : Symbol(Thing, Decl(index.js, 4, 3))
>Thing : Symbol(Thing, Decl(index.js, 1, 5), Decl(index.js, 0, 4))

/**
Expand Down
3 changes: 1 addition & 2 deletions tests/cases/fourslash/quickInfoJSExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
//// export { test/**/String };

verify.quickInfoAt("",
`type testString = string
(alias) type testString = any
`(alias) type testString = string
(alias) const testString: {
one: string;
two: string;
Expand Down

0 comments on commit 6eb3daa

Please sign in to comment.