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

This function types #6739

Merged
merged 46 commits into from Apr 7, 2016

Conversation

Projects
None yet
8 participants
@sandersn
Copy link
Member

sandersn commented Jan 29, 2016

Implements the proposal at #6018 and finishes the work described at #3694. See the bottom of this description for a tutorial and usage recommendations for this new feature.

This change adds checking for this types in functions. With this-function types, you can prevent the use of methods as free functions. And this types allow you to make constructor functions that return a real type rather than any. A number of other common javascript patterns can be given types or even inferred with --strictThis turned on.

Correctly prevent references to this when assigning callbacks

interface Callbacks {
  property: number;
  onClick(this: void, e: Event): void;
}
function handleClick(this:void, e: Event) {
  console.log(this.property); // error, 'void' has no member 'property'
}
class C {
  property: string
  tryHandleClick(this: this, e: Event): void {
    console.log(this.property); // OK, 'C' has member 'property'
  }
  actuallyHandleClick(this: void, e: Event): void {
    console.log(this.property); // error, 'void' has no member 'property'
  }
}
let c: C;
let callbacks: Callbacks;
callbacks.onClick = handleClick; // OK, this: void for both
callbacks.onClick = c.tryHandleClick; // Error, 'C' is not assignable to 'void'
callbacks.onClick = c.actuallyHandleClick; // OK, this: void for both

Note that callback functions are prevented from referring to members of this because they declare that this: void in order to be assignable to onClick.

Also note that the less common case of callback methods still allows functions to be assigned to these properties:

class Callbacks {
  m: number;
  callback: (this: this, n: number) => number;
}
function f(this: void, n: number) {
  return n;
}
let c: Callbacks;
c.callback = f; // OK, because f does not refer to any properties of `this`.

You can even require a different this for methods that will be used as a callback:

interface Callbacks {
  property: number;
  onClick(this: Callbacks, e: Event): void;
}
class C {
  property: string
  tryHandleClick(this: Callbacks, e: Event): void {
    console.log(this.property); // OK, 'C' has member 'property'
  }
}
let c: C;
let callbacks: Callbacks;
callbacks.onClick = c.tryHandleClick;

Contextual typing of methods and functions in object literals

Now when an object literal declares that it is of some type, this is also contextually typed inside the object literal.

interface I {
  n: number;
  method(m: number): number;
  callback: (m: number) => number;
}
let o: I = {
  n: 12,
  method(m) {
    return this.n + m; // OK, `this: I` from context
  },
  callback = m => m + 1, // OK, `this: void` from context
}

Build object literals from existing functions

You can also define functions alone and then later build an object literal from them. If the functions specify the this type, then the compiler will check references to this in the function's body. And it will also check that the function's this is assignable to the this of the object literal's type.

interface I {
  n: number;
  method: (m: number) => number;
}
function futureMethod(this: I, m: number) {
  return this.n + m;
}
let o: I = { n: 12, method: futureMethod };

Defining a function this way also requires that it cannot be called free:

futureMethod(12); // error, 'void' is not assignable to 'I'
o.futureMethod(12); // ok, 'o' is of type 'I'

The type of functions when used as constructors is now known.

When you use new with a function, you can now declare the type of the object that will be constructed with this. Previously, the type was always any. The compiler will also check that your assignments to this are correct in the body of the function.

function Symbol(this: Symbol, flags: SymbolFlags, name: string) {
  this.flags = flags;
  this.name = name;
  this.declarations = undefined;
}
let s = new Symbol(flags, "core"); // s: Symbol

strictThisChecks flag default types

UPDATE: --strictThisChecks removed

Because of performance concerns and lack of information on how people will use this types, we decided to remove --strictThisChecks for this release. I left the original explanation below.

--noImplicitThis

--noImplicitThis makes it an error to use this that is implicitly any inside of a function:

function F(x: number, y: number) {
    this.x = x; // this: any, so anything is legal
    this.z = y; // missed horrible typo
}
let f = new F(); // also, f: any  <-- that's sad :(

Add an annotation to fix this:

interface F { 
    x: number;
    y: number 
}
function F(this: F, x: number, y: number) {
    this.x = x;
    this.y = y;
}
let f = new F(12, 13); // f: F <-- hooray! :)

Previous explanation of strictThisChecks

For backward compatibility, this types default to any if not specified. However, if --strictThis is specified, then functions will default this: void and methods will default this: this. This removes the need for most of the annotations in the previous examples. For example, an interface can declare a function with this: void by using function syntax and a method with this: this by using method syntax:

interface I {
  f: (n: number) => number; // this: void
  m(n: number): number; // this: this
}
function g(n: number) { // this: void
  return n;
}
class C {
  private special: number = 12;
  m(n: number): number { return n + this.special; }
}

let i: I;
let c: C;
i.f = g; // ok, this: void
i.f = c.m; // error, 'void' is not assignable to 'C' (missing member 'm')
i.m = c.m; // error, 'I' is not assignable to 'C' (missing member 'special')

This breaks a lot of existing code, but is easy to write for future code.

How to upgrade code to --strictThis

When you switch on --strictThis you'll see a lot of errors with calling a method as if it were a function:

interface Object {
  method(n: number): void;
}
const f = object.method; 
f(12); // ERROR, this should be 'Object' not 'void'

You can fix the usage or the interface definition:

// Fix usage:
const fix1 = n => object.method(n);
correct(12); // OK, lambda captures the object and doesn't require 'this'
f.call(object, 12); // alternate fix: use Function.call

/// OR ///
// Fix definition:
interface Object {
  method(this: void, n: number): void;
}
// alternate fix: use function syntax to implicitly set this: void
interface Object {
  method: (n: number) => void;
}
const f = object.method;
f(12); // OK, this is void

Designing new, strict-this code

To be able to switch --strictThis on, the style that you write interfaces needs to change. You need to consider how people will use your interfaces. If they will treat your interface's functions as methods on an object, you should use the normal method declaration syntax. If they will treat them as a callback or some other kind of free function, you should declare them using the function property syntax. For example:

interface Extractor {
  extract(input: string): Row[]; // method style
}

interface Callbacks {
  callback: (e: Event) => void; // function property style
}

A safe default is to use the method declaration syntax. I discuss the tradeoffs below.

OO programming style

If you are writing pure OO Typescript/ES6, then you don't need to change much to work with --strictThis.

  • OO interfaces should have this: this
  • The only exception should be callbacks, which should have this: void.

Fortunately, these are the default types for the method and function syntax, respectively, so you don't have to write anything much different:

interface Extractor {
    extract(input: string): Row[];
    // the default (this: this) makes this the same as writing:
    // extract(this: this, input: string): Item[];

}
class XmlExtractor implements Extractor {
    extract(input: string): Row[] {
        // read from Xml into your Row objects
        // you can call private methods, etc, like before.
    }
    private helperMethod(sub: string): Row {
        // ...
    }
}

Now you can't assign extract to a function by mistake:

const xml = new XmlExtractor();
otherObject.onCall = xml.extract; // error: types of this are not compatible
let ex = xml.extract; // ok ...
ex('<row>...</row>'); // error, this: void is not compatible with this: XmlExtractor
otherObject.onExtract = ex; // error this: void is not compatible with this: XmlExtractor

And if you implement an interface, your methods get the right this-type regardless of what syntax you use:

interface Callbacks {
  callback: (e: Event) => void;
}
class XmlExtractor implements Extractor, Callbacks {
  extract(input: string): Row[] {
    // here, this: XmlExtractor
  }
  callback(e: Event) {
    // here, this: void
    this.extract(e.data); // error! 'this: void' has no method 'extract'
  }
}
otherObject.onExtract = xml.extract // error
otherObject.onExtract = xml.callback // ok!

How can I use a method as a callback?

You may have noticed that XmlExtractor.callback isn't that useful as a method since you can't actually refer to any other methods. The solution is the same as you use today: wrap the method call inside a lambda:

class XmlExtractor implements Extractors, Callbacks {
  extract(input: string): Row[] { ... }
  callback = e => this.extract(e.data); // ok, => doesn't capture 'this'
}

This formulation is OK because lambda (=>) doesn't bind this, so this comes from the class instead of from the implementing function.

Functional style

If you are writing your code in functional style, you probably use interfaces to describe records of functions. You can still declare the functions using the method style and build instances of the interface using an object literal of your functions:

interface Compiler {
  parse(program: string): Node;
  bind(tree: Node): Map<Symbol>;
  check(tree: Node): Diagnostic[];
}
function parse(program: string) {
  // code inside does not refer to this
}
// etc ...
let compiler: Compiler = { parse, bind, check }

But this prevents users from pulling these functions off of compiler and using them individually:

let tree = compiler.parse("console.log('hello')");
let parse = compiler.parse;
parse("console.log('goodbye')"); // error, this is 'void' which is not assignable to 'Compiler'

This is safe because of course they are free functions that do not require access to this. If you want enable this usage, you need to use the function property declaration syntax for the interface:

interface Compiler {
  parse: (program: string) => Node;
  bind: (tree: Node) => Map<Symbol>;
  check: (tree: Node) => Diagnostic[];
}
function parse(program: string) {
  // code inside doesn't refer to this
}
let compiler: Compiler = { parse, bind, check }
let parse = compiler.parse;
parse("console.log('goodbye')");

How can I implement an OO-style interface with nothing but functions?

If you want to implement an OO-style interface using only functions, you'll need to declare the this type in order to have access to that interface's members inside the function body.

function extract(this: Extractor, input: string): Row[] {
  // code here can refer to members of Extractor
}

Then you can create an object literal that contains that function:

let extractor: Extractor = {
  extract
}

This is essentially the code you would write today, but now usages of this are checked. For even more convenience you can use the contextual typing that comes from writing a function inside an object literal:

let extract: Extractor = {
  function extract(input: string): Row[] {
    // code here can refer to members of Extractor
  }
}

But at this point you might as well write a new class that implements Extractor.

Interfacing with JavaScript

The problem with interfacing to JavaScript is that it may use any or all of the above styles. A good default for writing types for Javascript code is the method declaration style. This makes implementing the interface easier. On the other hand, it prevents users of the interface from pulling functions off of the interface in order to save them as variables or to use them as callbacks.

This is the main reason that DefinitelyTyped definitions can't automatically compiled with --strictThis: it's impossible to predict which style of usage is the desired one for a currently-unmarked interface:

interface WholeClass {
  method(): void; // probably a method?
}
interface Record {
  func(x: string): number; // probably a function?
}

sandersn added some commits Jan 29, 2016

Parse this type using parameter syntax
Syntax is the same as a normal parameter:

```ts
function f(this: void, x: number) {
}
```
Check this type in functions.
If `this` is not provided, it defaults to `void` for functions and `this`
for methods. The rules for checking are similar to parameter checking, but
there's still quite a bit of duplication for this implementation.
Add overloads for Function.apply/call/bind
The new overloads use this types to specify the return type of these
functions as well as the type of `thisArg`.
Update baselines
1. Display of `this` changes for quick info.
2. The type of Function.call/apply/bind is more precise.
@sandersn

This comment has been minimized.

Copy link
Member

sandersn commented Feb 1, 2016

@ahejlsberg and @DanielRosenwasser, I believe you were both interested in this.

@@ -1297,6 +1297,9 @@ namespace ts {
// as other properties in the object literal. So we use SymbolFlags.PropertyExcludes
// so that it will conflict with any other object literal members with the same
// name.
if (options.strictThis) {
seenThisKeyword = true;
}

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

So any interface containing a method is now generic. Probably unavoidable, but we should get some data on what it means for performance.

This comment has been minimized.

@sandersn

sandersn Feb 3, 2016

Member

I ran some numbers this afternoon and didn't see a big change. Could be misreading the results though.

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Just to recap your findings: On the Monaco project this ends up adding as much as 10% to the check time for an overall impact of up to 5%.

writePunctuation(writer, SyntaxKind.OpenParenToken);
const useThisType = thisType && thisType.symbol;

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Why the check for thisType.symbol?

This comment has been minimized.

@sandersn

sandersn Feb 5, 2016

Member

voidType doesn't have a symbol, which avoids displaying (this: void, ...) for every single function, especially those that weren't annotated in the first place.

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 5, 2016

Member

There are other types that don't have a symbol, e.g. union and intersection types. If you want to exclude the void type then you should check for that.

@@ -3525,12 +3533,13 @@ namespace ts {
resolveObjectTypeMembers(type, source, typeParameters, typeArguments);
}

function createSignature(declaration: SignatureDeclaration, typeParameters: TypeParameter[], parameters: Symbol[],
function createSignature(declaration: SignatureDeclaration, typeParameters: TypeParameter[], parameters: Symbol[], thisType: Type,

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Shouldn't thisType come before parameters?

This comment has been minimized.

@sandersn

sandersn Feb 3, 2016

Member

fixed

@@ -4092,7 +4106,15 @@ namespace ts {
const resolvedSymbol = resolveName(param, paramSymbol.name, SymbolFlags.Value, undefined, undefined);
paramSymbol = resolvedSymbol;
}
parameters.push(paramSymbol);
if (paramSymbol.name === "this") {

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Parser allows this parameters in any position, so here I would change to if (i === firstParamIndex && paramSymbol.name === "this") and then add const firstParamIndex = isJSConstructSignature ? 1: 0 above the for loop.

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Actually, since we don't care about JSConstructSignature case, you should just say if (i === 0 && paramSymbol.name === "this").

parameters.push(paramSymbol);
if (paramSymbol.name === "this") {
thisType = param.type && getTypeOfSymbol(paramSymbol);
if (i !== 0 || declaration.kind === SyntaxKind.Constructor) {

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

This check seems out of place here. It should be done in checkParameter.

if (paramSymbol.name === "this") {
thisType = param.type && getTypeOfSymbol(paramSymbol);
if (i !== 0 || declaration.kind === SyntaxKind.Constructor) {
error(param, Diagnostics.this_cannot_be_referenced_in_current_location);

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

I would have one error message that says 'this' parameter must be the first parameter and another that says A constructor cannot have a 'this' parameter.

This comment has been minimized.

@sandersn
@@ -4092,7 +4106,15 @@ namespace ts {
const resolvedSymbol = resolveName(param, paramSymbol.name, SymbolFlags.Value, undefined, undefined);
paramSymbol = resolvedSymbol;
}
parameters.push(paramSymbol);
if (paramSymbol.name === "this") {
thisType = param.type && getTypeOfSymbol(paramSymbol);

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

So, if the programmer omits a type annotation on a this parameter, you will think there was no this parameter and get your parameter count adjustments wrong below. You need to track the presence of a this parameter separately from its type. When the type annotation is missing you should use unknownType. Also, you can just use getTypeFromTypeNode(param.type) instead of getTypeOfSymbol.

This comment has been minimized.

@sandersn
@@ -4100,7 +4122,7 @@ namespace ts {

if (param.initializer || param.questionToken || param.dotDotDotToken) {
if (minArgumentCount < 0) {
minArgumentCount = i;
minArgumentCount = i - (thisType ? 1 : 0);

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

See my comments about parameter count adjustments above.

This comment has been minimized.

@sandersn
else if ((declaration.kind === SyntaxKind.MethodDeclaration || declaration.kind === SyntaxKind.MethodSignature)
&& (isClassLike(declaration.parent) || declaration.parent.kind === SyntaxKind.InterfaceDeclaration)) {
thisType = declaration.flags & NodeFlags.Static ?
getWidenedType(checkExpression((<ClassLikeDeclaration>declaration.parent).name)) :

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Yikes, this checkExpression call looks all wrong. The name of a class declaration isn't an expression. You need to use getTypeOfSymbol(getSymbolOfNode(declaration.parent)).

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Also, why are you widening the type?

This comment has been minimized.

@sandersn

sandersn Feb 3, 2016

Member

I tried getTypeOfSymbol(getSymbolOfNode(... first. According to my notes, I had trouble getting typeof C vs C in the static case.

But it looks like I was wrong. It works now. I'm pretty sure some other change I made was wrong at the time.

}
if (!thisType && compilerOptions.strictThis) {
if (declaration.kind === SyntaxKind.FunctionDeclaration
|| declaration.kind === SyntaxKind.CallSignature

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Elsewhere we don't use this style of putting || at the beginning of the line.

This comment has been minimized.

@sandersn

sandersn Feb 3, 2016

Member

oops, it's a habit left over from non-ASI languages. Fixed.

@@ -5182,7 +5220,14 @@ namespace ts {
}

function isContextSensitiveFunctionLikeDeclaration(node: FunctionLikeDeclaration) {
return !node.typeParameters && node.parameters.length && !forEach(node.parameters, p => p.type);
if (compilerOptions.strictThis) {

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Why this check? It doesn't matter if were in strict-this mode, we always want to ignore this parameters for purposes of this function.

This comment has been minimized.

@sandersn

sandersn Feb 3, 2016

Member

I thought that in loose-this mode, a function with all arg types specified and no this is not contextually typed. But thinking about it a second time, I don't think that's true:

interface I {
  y: number;
  m(this: I, x: number);
}
let o: I = {
  m(x: number) {
    return x + this./**/
  }
}

It would in fact be nice if we would contextually type this so that we could offer y as a completion. I'll get rid of everything except the updated check.

This comment has been minimized.

@sandersn
return !node.typeParameters && node.parameters.length && !forEach(node.parameters, p => p.type);
if (compilerOptions.strictThis) {
return !node.typeParameters &&
(!forEach(node.parameters, p => p.type)

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Maybe add a hasParameterWithTypeAnnotation function and do the appropriate compensation for this parameters in there.

const t = target.thisType || anyType;
if (s !== voidType) {
// void sources are assignable to anything.
let related = compareTypes(getApparentType(t), getApparentType(s), reportErrors);

This comment has been minimized.

@ahejlsberg

ahejlsberg Feb 3, 2016

Member

Why are you comparing apparent types?

This comment has been minimized.

@sandersn

sandersn Feb 5, 2016

Member

Same reason I used getWidenedType elsewhere. I'll get rid of the calls since they are wrong.

@Gaelan

This comment has been minimized.

Copy link

Gaelan commented Apr 4, 2016

Not that #7097 is fixed, can strictThisChecks be readded?

@DanielRosenwasser

This comment has been minimized.

Copy link
Member

DanielRosenwasser commented Apr 4, 2016

@Gaelan I believe we concluded at our design meeting that we were going to hold off on --strictThis, partially for perf reasons, partially because alongside readonly and non-nullable types, this was going to cause a lot of issues for the community in updating .d.ts files in a compatible manner. Check out #7689.

@Gaelan

This comment has been minimized.

Copy link

Gaelan commented Apr 4, 2016

@sandersn

This comment has been minimized.

Copy link
Member

sandersn commented Apr 7, 2016

@DanielRosenwasser I added a contextual typing test as in (2) with an implicit any on a this parameter. Note that the types are not quite right — the this parameter is never contextually typed, just this expressions in the function body. This is because getSignatureFromDeclaration stores thisType eagerly -- not thisSymbol like the rest of the compiler. I discussed this with @ahejlsberg and we decided it was OK for now since it simplifies the code a lot. I might change it later.

The effect is that this:

o.someMethod = function(this, m) { return this.n + m };

has these types:

o.someMethod: (this: O, m: number) => number // source of context
function(this: any, m: number) => number // this: any, but returns number
this: O // inside the body, this is contextually typed
this.n: number
m: number
return this.n +  m // correctly returns number

Note that the thisType: any is not contagious because it's not actually used -- for this.n, checkThisExpression uses the contextual type instead, which is this: O.

@sandersn sandersn merged commit 3704ad7 into master Apr 7, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@sandersn sandersn deleted the this-function-types branch Apr 7, 2016

basarat added a commit to TypeStrong/atom-typescript that referenced this pull request Apr 8, 2016

@robotlolita robotlolita referenced this pull request Apr 15, 2017

Open

typescript typings #65

@avivcarmis

This comment has been minimized.

Copy link

avivcarmis commented Sep 14, 2017

This feature is exactly what I'm after, I see it was merged but i can't find any docs regarding, and i saw here that it was not working well.
Is there any update? Can it be found in some newer version? Will it be added in the future?

@sandersn

This comment has been minimized.

Copy link
Member

sandersn commented Sep 14, 2017

https://www.typescriptlang.org/docs/handbook/functions.html has the documentation near the bottom of the page.

@mhegazy mhegazy referenced this pull request Jun 4, 2018

Closed

Fixing undefined 'this' in Javascript #24636

4 of 4 tasks complete

@Microsoft Microsoft locked and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.