Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Extend no-invalid-this rule to disallow this in functions in methods #1179

Merged
merged 2 commits into from
Apr 29, 2016
Merged

Extend no-invalid-this rule to disallow this in functions in methods #1179

merged 2 commits into from
Apr 29, 2016

Conversation

inthemill
Copy link
Contributor

This PR extends the [no-invalid-this] with the option "in-function-in-method".
If it is enabled it complains about the usage of the this keyword inside of function expression within a method.
this in such function expressions is usually meant to refer the this outside of the functions scope.
This rule will not complain about this in arrow functions.

In our project, it happened often when migrating JS to TS. With an angular service written as

angular.module('a',[]).factory('Service', function(Helper, Enhancer){
return {
    getAllMapped: function(){
         return Helper.getAll()
             .map(function(item){
                  return Enhancer.enhance(item);
             })
    }
}
}); 

We often end up with

class ServiceClass {
    constructor(private Helper, private Enhancer){
    }

    public getAllMapped() {
         return this.Helper.getAll()
             .map(function(item){
                  return this.Enhancer.enhance(item);
             })
    }
}
angular.module('a',[]).service('Service', ServiceClass);

which will not work as this.Enhancer is not defined.

…thods

[no-invalid-this], when enabled with "in-function-in-method" , complains about the usage of the
`this` keyword inside of function expression within a method.
`this` in such function expressions is usually meant to refer the this outside of the functions scope.
 This rule will not complain about this in arrow functions
@@ -240,7 +240,9 @@ A sample configuration file with all options is available [here](https://github.
* `no-eval` disallows `eval` function invocations.
* `no-inferrable-types` disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean.
* `no-internal-module` disallows internal `module` (use `namespace` instead).
* `no-invalid-this` disallows using the `this` keyword outside of classes.
* `no-invalid-this` disallows using the `this` keyword in scopes where they are often misinterpreted
* `outside-of-class` disallows using the `this` keyword outside of classes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep this behavior on by default? For backwards-compat reasons, we don't want to have to make users add a new option to their rule to get the same behavior as before

@jkillian
Copy link
Contributor

Looks good to me logic-wise @inthemill! If you could just make the couple tweaks listed above about the rule's API and style things, we'll be able to merge this after.

- keeps behaviour if tslint.json isn't changed.
- improve readablity of option and test
- improve message, with hint to resolve issue
- fix style issue
@jkillian
Copy link
Contributor

Looks great, thanks @inthemill!

@jkillian jkillian merged commit 09b2821 into palantir:master Apr 29, 2016
tomduncalf pushed a commit to tomduncalf/tslint that referenced this pull request Jun 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants