Feature: Disallow 'this' in non-arrow closure functions

[bparadie]

A mistake, that is easy to make, is the accidental use of this in functions and anonymous functions, i.e.

$( "#target" ).click(function() {
  this.onClick();
});

A common pattern to avoid that problem is using a proxy for this instead of this in functions and anonymous functions, i.e.:

var self = myObjectWithOnClick;
$( "#target" ).click(function() {
  self.onClick();
});

Using this in member function of classes (methods) are of course allowed.

Please consider adding a rule for disallowing 'this' in functions for tslint.

[awerlang]

There are legal use cases like this one:

var obj = {threshold: 10};
var filtered = [12, 5, 8, 130, 44].filter(function isBigEnough(element) {
  return element >= this.threshold;
}, obj);

And one of the reasons to use typescript is taking advantage of arrow functions:

$( "#target" ).click(() => {
  this.onClick();
});

[bparadie]

@awerlang Yes, there are use cases like the one you are pointing out where using this makes sense in closure functions. I am a developer who chooses to never use this in closure functions and I would love to see those flagged by tslint if I do so.

I am not sure whether I am getting your point with your 2nd example. The arrow function does not change the semantics of this. The correct arrow function version would be:

var self = myObjectWithOnClick;
$( "#target" ).click(() => {
  self.onClick();
})

[awerlang]

Actually the arrow function notation takes this from the surrouding context always, it doesn't care how it is invoked.

export class Controller {

  constructor(el) {
    $( el ).click(() => {
      this.onClick();
    });
  }

  onClick() {
    alert("clicked!");
  }
}

var ctrl = new Controller("#target");

[bparadie]

@awerlang You are right, I stand corrected!

I just went to http://www.typescriptlang.org/Playground/ and compiled your example.
This is what I got:

define(["require", "exports"], function (require, exports) {
    var Controller = (function () {
        function Controller(el) {
            var _this = this;
            $(el).click(function () {
                _this.onClick();
            });
        }
        Controller.prototype.onClick = function () {
            alert("clicked!");
        };
        return Controller;
    })();
    exports.Controller = Controller;
    var ctrl = new Controller("#target");
});

The compiler clearly picked up this in the arrow function and replaced it with _this.
I didn't know about that language feature and I am delighted!

Circling back to my original feature request... What I am hearing you say between the lines is: "If you are concerned about incorrect usage of this in closure functions then you should switch to arrow functions".

I would agree to that, but then the next thing I would do is close this issue and open a new one with the title:

Feature: Disallow 'this' in non-arrow closure functions

or even more radical:

Feature: Disallow non-arrow closure functions

The case I want myself protected against is using this in closure functions where this is associated with the caller of the function and not the instance of the method. I am willing to use arrow-functions, but then I still would love to see tslint warning me if I used this in non-arrow-functions. Alternatively I would be willing to give up non-arrow closure functions entirely. For that case I would love to get a warning from tslint if I did use a non-arrow function.

[awerlang]

I would agree to "Feature: Disallow 'this' in non-arrow closure functions".

If one were to disallow non-arrow functions, then it would become impossible to declare anonymous constructor functions, because this on an array function would reference something probably not intended. Of course I prefer to make a class instead, but I don't know about others.

[bparadie]

ok, I am changing the title from

Feature: Disallow 'this' in functions

to

Feature: Disallow 'this' in non-arrow closure functions

Thanks for your input!

[devoto13]

I believe TypeScript 2 with this feature this parameters and --noImplicitThis (see Compiler Options) makes this rule obsolete.

With --noImplicitThis TypeScript will error on usage of this in non-arrow functions, unless this is explicitly specified. Furthermore specifying this will give you code completion for its properties.

See example:

class User {
    constructor(public name: string) {
    }

    onReady(callback: () => void) {
        callback.apply(this);
    }
}

let user = new User("John");
user.onReady(function () {
    console.log(this.name);
});

TypeScript will report an error on usage of this.name with --noImplicitAny enabled.

But if you specify this in declaration like so:

    onReady(callback: (this: User) => void) {

It will not error out and work correctly.

@bparadie so with --noImplicityAny and not specifying this in the callback declarations you should get exactly the behaviour you want.

[ajafff]

Closing this feature request. With the added support of contextual this, this-parameters and --noImplicitThis in the compiler, there's no longer the need for such a lint rule.
The above comment by @devoto13 explains it very well.