Skip to content

Bind toplevel this assignments as global declarations #22891

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
8 changes: 7 additions & 1 deletion src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2364,7 +2364,13 @@ namespace ts {
break;
case SyntaxKind.SourceFile:
// this.foo assignment in a source file
// Do not bind. It would be nice to support this someday though.
// Bind this property in the global namespace or in the exports if in commonjs
if ((thisContainer as SourceFile).commonJsModuleIndicator) {
declareSymbol(file.symbol.exports, file.symbol, node, SymbolFlags.Property | SymbolFlags.ExportValue, SymbolFlags.None);
}
else {
declareSymbolAndAddToSymbolTable(node, SymbolFlags.FunctionScopedVariable, SymbolFlags.FunctionScopedVariableExcludes);
}
break;

default:
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13960,6 +13960,11 @@ namespace ts {
if (type && type !== unknownType) {
return getFlowTypeOfReference(node, type);
}
if (isSourceFile(container)) {
// look up in the source file's locals or exports
const parent = getSymbolOfNode(container);
return createAnonymousType(parent, container.commonJsModuleIndicator ? parent.exports : globals, emptyArray, emptyArray, createIndexInfo(anyType, /*isReadonly*/ false), undefined);
Copy link

@ghost ghost Mar 27, 2018

Choose a reason for hiding this comment

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

createAnonymousType looks expensive if we call it on every this expression not in a class/function. Don't we already get a type for the source file when doing import a = require("source-file");?
Also, we should maintain a type for globals; that would fix #14052. But until then createAnonymousType(globals) will be really expensive if there are 1000 globals, and there might be! Might be better to leave that case as a // TODO: GH#14052 and leave as any for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am also not sure i understand the need for the indexer.. why not just return a "global" type?

Copy link
Member Author

@sandersn sandersn Mar 28, 2018

Choose a reason for hiding this comment

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

@mhegazy Do you want an error on any unknown toplevel this access in Javascript? I don't think we should be stricter than Typescript here.

@Andy-MS I'll take a look and see what I can do. A global type seems like a good idea.

I don't think the current code is a real performance problem, though, for a couple of reasons:

  1. It only happens a couple of times per project, even in large code bases like chrome-devtools-frontend. Compare to checkObjectLiteral, which creates a fresh anonymous type for every object literal in a code base.
  2. createAnonymousType just saves a pointer to the globals symbol table (which does in fact have hundreds of entries just from JS built-ins). The property symbols themselves cache their own resolved type if it's requested.

}
}
}

Expand Down
24 changes: 24 additions & 0 deletions tests/baselines/reference/topLevelThisAssignment.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
tests/cases/conformance/salsa/a.js(4,1): error TS2304: Cannot find name 'unknown'.
tests/cases/conformance/salsa/b.js(3,1): error TS2304: Cannot find name 'unknown'.


==== tests/cases/conformance/salsa/a.js (1 errors) ====
this.a = 10;
this.a;
a;
unknown;
~~~~~~~
!!! error TS2304: Cannot find name 'unknown'.
this.unknown;

// also, improved types for this-prefixed globals like eval:
this.eval('hi');

==== tests/cases/conformance/salsa/b.js (1 errors) ====
this.a;
a;
unknown;
~~~~~~~
!!! error TS2304: Cannot find name 'unknown'.
this.unknown;

13 changes: 13 additions & 0 deletions tests/baselines/reference/topLevelThisAssignment.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,28 @@
this.a = 10;
this.a;
a;
unknown;
this.unknown;

// also, improved types for this-prefixed globals like eval:
this.eval('hi');

//// [b.js]
this.a;
a;
unknown;
this.unknown;


//// [output.js]
this.a = 10;
this.a;
a;
unknown;
this.unknown;
// also, improved types for this-prefixed globals like eval:
this.eval('hi');
this.a;
a;
unknown;
this.unknown;
35 changes: 28 additions & 7 deletions tests/baselines/reference/topLevelThisAssignment.symbols
Original file line number Diff line number Diff line change
@@ -1,10 +1,31 @@
=== tests/cases/conformance/salsa/a.js ===
this.a = 10;
No type information for this code.this.a;
No type information for this code.a;
No type information for this code.
No type information for this code.=== tests/cases/conformance/salsa/b.js ===
>this.a : Symbol(a, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 0, 0))

this.a;
No type information for this code.a;
No type information for this code.
No type information for this code.
>this.a : Symbol(a, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 0, 0))

a;
>a : Symbol(a, Decl(a.js, 0, 0))

unknown;
this.unknown;

// also, improved types for this-prefixed globals like eval:
this.eval('hi');
>this.eval : Symbol(eval, Decl(lib.d.ts, --, --))
>eval : Symbol(eval, Decl(lib.d.ts, --, --))

=== tests/cases/conformance/salsa/b.js ===
this.a;
>this.a : Symbol(a, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 0, 0))

a;
>a : Symbol(a, Decl(a.js, 0, 0))

unknown;
this.unknown;

46 changes: 35 additions & 11 deletions tests/baselines/reference/topLevelThisAssignment.types
Original file line number Diff line number Diff line change
@@ -1,25 +1,49 @@
=== tests/cases/conformance/salsa/a.js ===
this.a = 10;
>this.a = 10 : 10
>this.a : any
>this : any
>a : any
>this.a : number
>this : { [x: string]: any; eval(x: string): any; parseInt(s: string, radix?: number): number; parseFloat(string: string): number; isNaN(number: number): boolean; isFinite(number: number): boolean; decodeURI(encodedURI: string): string; decodeURIComponent(encodedURIComponent: string): string; encodeURI(uri: string): string; encodeURIComponent(uriComponent: string): string; escape(string: string): string; unescape(string: string): string; readonly NaN: number; readonly Infinity: number; readonly Object: ObjectConstructor; readonly Function: FunctionConstructor; readonly String: StringConstructor; readonly Boolean: BooleanConstructor; readonly Number: NumberConstructor; readonly Math: Math; readonly Date: DateConstructor; readonly RegExp: RegExpConstructor; readonly Error: ErrorConstructor; readonly EvalError: EvalErrorConstructor; readonly RangeError: RangeErrorConstructor; readonly ReferenceError: ReferenceErrorConstructor; readonly SyntaxError: SyntaxErrorConstructor; readonly TypeError: TypeErrorConstructor; readonly URIError: URIErrorConstructor; readonly JSON: JSON; readonly Array: ArrayConstructor; readonly ArrayBuffer: ArrayBufferConstructor; readonly DataView: DataViewConstructor; readonly Int8Array: Int8ArrayConstructor; readonly Uint8Array: Uint8ArrayConstructor; readonly Uint8ClampedArray: Uint8ClampedArrayConstructor; readonly Int16Array: Int16ArrayConstructor; readonly Uint16Array: Uint16ArrayConstructor; readonly Int32Array: Int32ArrayConstructor; readonly Uint32Array: Uint32ArrayConstructor; readonly Float32Array: Float32ArrayConstructor; readonly Float64Array: Float64ArrayConstructor; Intl: typeof Intl; a: number; undefined: undefined; }
>a : number
>10 : 10

this.a;
>this.a : any
>this : any
>a : any
>this.a : number
>this : { [x: string]: any; eval(x: string): any; parseInt(s: string, radix?: number): number; parseFloat(string: string): number; isNaN(number: number): boolean; isFinite(number: number): boolean; decodeURI(encodedURI: string): string; decodeURIComponent(encodedURIComponent: string): string; encodeURI(uri: string): string; encodeURIComponent(uriComponent: string): string; escape(string: string): string; unescape(string: string): string; readonly NaN: number; readonly Infinity: number; readonly Object: ObjectConstructor; readonly Function: FunctionConstructor; readonly String: StringConstructor; readonly Boolean: BooleanConstructor; readonly Number: NumberConstructor; readonly Math: Math; readonly Date: DateConstructor; readonly RegExp: RegExpConstructor; readonly Error: ErrorConstructor; readonly EvalError: EvalErrorConstructor; readonly RangeError: RangeErrorConstructor; readonly ReferenceError: ReferenceErrorConstructor; readonly SyntaxError: SyntaxErrorConstructor; readonly TypeError: TypeErrorConstructor; readonly URIError: URIErrorConstructor; readonly JSON: JSON; readonly Array: ArrayConstructor; readonly ArrayBuffer: ArrayBufferConstructor; readonly DataView: DataViewConstructor; readonly Int8Array: Int8ArrayConstructor; readonly Uint8Array: Uint8ArrayConstructor; readonly Uint8ClampedArray: Uint8ClampedArrayConstructor; readonly Int16Array: Int16ArrayConstructor; readonly Uint16Array: Uint16ArrayConstructor; readonly Int32Array: Int32ArrayConstructor; readonly Uint32Array: Uint32ArrayConstructor; readonly Float32Array: Float32ArrayConstructor; readonly Float64Array: Float64ArrayConstructor; Intl: typeof Intl; a: number; undefined: undefined; }
>a : number

a;
>a : any
>a : number

unknown;
>unknown : any

this.unknown;
>this.unknown : any
>this : { [x: string]: any; eval(x: string): any; parseInt(s: string, radix?: number): number; parseFloat(string: string): number; isNaN(number: number): boolean; isFinite(number: number): boolean; decodeURI(encodedURI: string): string; decodeURIComponent(encodedURIComponent: string): string; encodeURI(uri: string): string; encodeURIComponent(uriComponent: string): string; escape(string: string): string; unescape(string: string): string; readonly NaN: number; readonly Infinity: number; readonly Object: ObjectConstructor; readonly Function: FunctionConstructor; readonly String: StringConstructor; readonly Boolean: BooleanConstructor; readonly Number: NumberConstructor; readonly Math: Math; readonly Date: DateConstructor; readonly RegExp: RegExpConstructor; readonly Error: ErrorConstructor; readonly EvalError: EvalErrorConstructor; readonly RangeError: RangeErrorConstructor; readonly ReferenceError: ReferenceErrorConstructor; readonly SyntaxError: SyntaxErrorConstructor; readonly TypeError: TypeErrorConstructor; readonly URIError: URIErrorConstructor; readonly JSON: JSON; readonly Array: ArrayConstructor; readonly ArrayBuffer: ArrayBufferConstructor; readonly DataView: DataViewConstructor; readonly Int8Array: Int8ArrayConstructor; readonly Uint8Array: Uint8ArrayConstructor; readonly Uint8ClampedArray: Uint8ClampedArrayConstructor; readonly Int16Array: Int16ArrayConstructor; readonly Uint16Array: Uint16ArrayConstructor; readonly Int32Array: Int32ArrayConstructor; readonly Uint32Array: Uint32ArrayConstructor; readonly Float32Array: Float32ArrayConstructor; readonly Float64Array: Float64ArrayConstructor; Intl: typeof Intl; a: number; undefined: undefined; }
>unknown : any

// also, improved types for this-prefixed globals like eval:
this.eval('hi');
>this.eval('hi') : any
>this.eval : (x: string) => any
>this : { [x: string]: any; eval(x: string): any; parseInt(s: string, radix?: number): number; parseFloat(string: string): number; isNaN(number: number): boolean; isFinite(number: number): boolean; decodeURI(encodedURI: string): string; decodeURIComponent(encodedURIComponent: string): string; encodeURI(uri: string): string; encodeURIComponent(uriComponent: string): string; escape(string: string): string; unescape(string: string): string; readonly NaN: number; readonly Infinity: number; readonly Object: ObjectConstructor; readonly Function: FunctionConstructor; readonly String: StringConstructor; readonly Boolean: BooleanConstructor; readonly Number: NumberConstructor; readonly Math: Math; readonly Date: DateConstructor; readonly RegExp: RegExpConstructor; readonly Error: ErrorConstructor; readonly EvalError: EvalErrorConstructor; readonly RangeError: RangeErrorConstructor; readonly ReferenceError: ReferenceErrorConstructor; readonly SyntaxError: SyntaxErrorConstructor; readonly TypeError: TypeErrorConstructor; readonly URIError: URIErrorConstructor; readonly JSON: JSON; readonly Array: ArrayConstructor; readonly ArrayBuffer: ArrayBufferConstructor; readonly DataView: DataViewConstructor; readonly Int8Array: Int8ArrayConstructor; readonly Uint8Array: Uint8ArrayConstructor; readonly Uint8ClampedArray: Uint8ClampedArrayConstructor; readonly Int16Array: Int16ArrayConstructor; readonly Uint16Array: Uint16ArrayConstructor; readonly Int32Array: Int32ArrayConstructor; readonly Uint32Array: Uint32ArrayConstructor; readonly Float32Array: Float32ArrayConstructor; readonly Float64Array: Float64ArrayConstructor; Intl: typeof Intl; a: number; undefined: undefined; }
>eval : (x: string) => any
>'hi' : "hi"

=== tests/cases/conformance/salsa/b.js ===
this.a;
>this.a : any
>this : any
>a : any
>this.a : number
>this : { [x: string]: any; eval(x: string): any; parseInt(s: string, radix?: number): number; parseFloat(string: string): number; isNaN(number: number): boolean; isFinite(number: number): boolean; decodeURI(encodedURI: string): string; decodeURIComponent(encodedURIComponent: string): string; encodeURI(uri: string): string; encodeURIComponent(uriComponent: string): string; escape(string: string): string; unescape(string: string): string; readonly NaN: number; readonly Infinity: number; readonly Object: ObjectConstructor; readonly Function: FunctionConstructor; readonly String: StringConstructor; readonly Boolean: BooleanConstructor; readonly Number: NumberConstructor; readonly Math: Math; readonly Date: DateConstructor; readonly RegExp: RegExpConstructor; readonly Error: ErrorConstructor; readonly EvalError: EvalErrorConstructor; readonly RangeError: RangeErrorConstructor; readonly ReferenceError: ReferenceErrorConstructor; readonly SyntaxError: SyntaxErrorConstructor; readonly TypeError: TypeErrorConstructor; readonly URIError: URIErrorConstructor; readonly JSON: JSON; readonly Array: ArrayConstructor; readonly ArrayBuffer: ArrayBufferConstructor; readonly DataView: DataViewConstructor; readonly Int8Array: Int8ArrayConstructor; readonly Uint8Array: Uint8ArrayConstructor; readonly Uint8ClampedArray: Uint8ClampedArrayConstructor; readonly Int16Array: Int16ArrayConstructor; readonly Uint16Array: Uint16ArrayConstructor; readonly Int32Array: Int32ArrayConstructor; readonly Uint32Array: Uint32ArrayConstructor; readonly Float32Array: Float32ArrayConstructor; readonly Float64Array: Float64ArrayConstructor; Intl: typeof Intl; a: number; undefined: undefined; }
>a : number

a;
>a : any
>a : number

unknown;
>unknown : any

this.unknown;
>this.unknown : any
>this : { [x: string]: any; eval(x: string): any; parseInt(s: string, radix?: number): number; parseFloat(string: string): number; isNaN(number: number): boolean; isFinite(number: number): boolean; decodeURI(encodedURI: string): string; decodeURIComponent(encodedURIComponent: string): string; encodeURI(uri: string): string; encodeURIComponent(uriComponent: string): string; escape(string: string): string; unescape(string: string): string; readonly NaN: number; readonly Infinity: number; readonly Object: ObjectConstructor; readonly Function: FunctionConstructor; readonly String: StringConstructor; readonly Boolean: BooleanConstructor; readonly Number: NumberConstructor; readonly Math: Math; readonly Date: DateConstructor; readonly RegExp: RegExpConstructor; readonly Error: ErrorConstructor; readonly EvalError: EvalErrorConstructor; readonly RangeError: RangeErrorConstructor; readonly ReferenceError: ReferenceErrorConstructor; readonly SyntaxError: SyntaxErrorConstructor; readonly TypeError: TypeErrorConstructor; readonly URIError: URIErrorConstructor; readonly JSON: JSON; readonly Array: ArrayConstructor; readonly ArrayBuffer: ArrayBufferConstructor; readonly DataView: DataViewConstructor; readonly Int8Array: Int8ArrayConstructor; readonly Uint8Array: Uint8ArrayConstructor; readonly Uint8ClampedArray: Uint8ClampedArrayConstructor; readonly Int16Array: Int16ArrayConstructor; readonly Uint16Array: Uint16ArrayConstructor; readonly Int32Array: Int32ArrayConstructor; readonly Uint32Array: Uint32ArrayConstructor; readonly Float32Array: Float32ArrayConstructor; readonly Float64Array: Float64ArrayConstructor; Intl: typeof Intl; a: number; undefined: undefined; }
>unknown : any

20 changes: 20 additions & 0 deletions tests/baselines/reference/topLevelThisAssignment2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
tests/cases/conformance/salsa/mod.js(6,1): error TS2304: Cannot find name 'a'.


==== tests/cases/conformance/salsa/use.js (0 errors) ====
var mod = require('./mod')
mod.a;

==== tests/cases/conformance/salsa/decl.d.ts (0 errors) ====
declare var module: { exports: any };
declare function require(name: string): any;
==== tests/cases/conformance/salsa/mod.js (1 errors) ====
/// <reference path='./decl.d.ts' />
module.exports = {};
this.a = 10;
this.a; // ok
module.exports.a; // should be ok but doesn't have the right type
a; // error: not actually at top-level in a module
~
!!! error TS2304: Cannot find name 'a'.

44 changes: 44 additions & 0 deletions tests/baselines/reference/topLevelThisAssignment2.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
=== tests/cases/conformance/salsa/use.js ===
var mod = require('./mod')
>mod : Symbol(mod, Decl(use.js, 0, 3))
>require : Symbol(require, Decl(decl.d.ts, 0, 37))
>'./mod' : Symbol("tests/cases/conformance/salsa/mod", Decl(mod.js, 0, 0))

mod.a;
>mod.a : Symbol(a, Decl(mod.js, 1, 20))
>mod : Symbol(mod, Decl(use.js, 0, 3))
>a : Symbol(a, Decl(mod.js, 1, 20))

=== tests/cases/conformance/salsa/decl.d.ts ===
declare var module: { exports: any };
>module : Symbol(module, Decl(decl.d.ts, 0, 11))
>exports : Symbol(exports, Decl(decl.d.ts, 0, 21))

declare function require(name: string): any;
>require : Symbol(require, Decl(decl.d.ts, 0, 37))
>name : Symbol(name, Decl(decl.d.ts, 1, 25))

=== tests/cases/conformance/salsa/mod.js ===
/// <reference path='./decl.d.ts' />
module.exports = {};
>module.exports : Symbol(exports, Decl(decl.d.ts, 0, 21))
>module : Symbol(module, Decl(decl.d.ts, 0, 11))
>exports : Symbol(exports, Decl(decl.d.ts, 0, 21))

this.a = 10;
>this.a : Symbol(a, Decl(mod.js, 1, 20))
>this : Symbol("tests/cases/conformance/salsa/mod", Decl(mod.js, 0, 0))
>a : Symbol(a, Decl(mod.js, 1, 20))

this.a; // ok
>this.a : Symbol(a, Decl(mod.js, 1, 20))
>this : Symbol("tests/cases/conformance/salsa/mod", Decl(mod.js, 0, 0))
>a : Symbol(a, Decl(mod.js, 1, 20))

module.exports.a; // should be ok but doesn't have the right type
>module.exports : Symbol(exports, Decl(decl.d.ts, 0, 21))
>module : Symbol(module, Decl(decl.d.ts, 0, 11))
>exports : Symbol(exports, Decl(decl.d.ts, 0, 21))

a; // error: not actually at top-level in a module

52 changes: 52 additions & 0 deletions tests/baselines/reference/topLevelThisAssignment2.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
=== tests/cases/conformance/salsa/use.js ===
var mod = require('./mod')
>mod : typeof "tests/cases/conformance/salsa/mod"
>require('./mod') : typeof "tests/cases/conformance/salsa/mod"
>require : (name: string) => any
>'./mod' : "./mod"

mod.a;
>mod.a : number
>mod : typeof "tests/cases/conformance/salsa/mod"
>a : number

=== tests/cases/conformance/salsa/decl.d.ts ===
declare var module: { exports: any };
>module : { exports: any; }
>exports : any

declare function require(name: string): any;
>require : (name: string) => any
>name : string

=== tests/cases/conformance/salsa/mod.js ===
/// <reference path='./decl.d.ts' />
module.exports = {};
>module.exports = {} : { [x: string]: any; }
>module.exports : any
>module : { exports: any; }
>exports : any
>{} : { [x: string]: any; }

this.a = 10;
>this.a = 10 : 10
>this.a : number
>this : typeof "tests/cases/conformance/salsa/mod"
>a : number
>10 : 10

this.a; // ok
>this.a : number
>this : typeof "tests/cases/conformance/salsa/mod"
>a : number

module.exports.a; // should be ok but doesn't have the right type
>module.exports.a : any
>module.exports : any
>module : { exports: any; }
>exports : any
>a : any

a; // error: not actually at top-level in a module
>a : any

4 changes: 4 additions & 0 deletions tests/baselines/reference/typeFromPropertyAssignment9.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ min.nest = this.min.nest || function () { };
>min.nest : Symbol(nest, Decl(a.js, 29, 27))
>min : Symbol(min, Decl(a.js, 29, 3))
>nest : Symbol(nest, Decl(a.js, 29, 27))
>this.min.nest : Symbol(nest, Decl(a.js, 29, 27))
>this.min : Symbol(min, Decl(a.js, 29, 3))
>min : Symbol(min, Decl(a.js, 29, 3))
>nest : Symbol(nest, Decl(a.js, 29, 27))

min.nest.other = self.min.nest.other || class { };
>min.nest.other : Symbol((Anonymous function).other, Decl(a.js, 30, 44))
Expand Down
Loading