Skip to content

Commit

Permalink
no-invalid-this: false positive for method syntax
Browse files Browse the repository at this point in the history
* Keep track of evaluation context: look to the closest parent function
  and decide if it is allowed to refer to `this`
* Fix a false negative in the tests from palantir#3267
  • Loading branch information
amacleay committed Jul 12, 2018
1 parent 9d7db47 commit 8167440
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 18 deletions.
61 changes: 43 additions & 18 deletions src/rules/noInvalidThisRule.ts
Expand Up @@ -59,39 +59,64 @@ export class Rule extends Lint.Rules.AbstractRule {

function walk(ctx: Lint.WalkContext<boolean>): void {
const { sourceFile, options: checkFuncInMethod } = ctx;

const enum parentType {
None,
Class,
ClassMethod,
BoundRegularFunction,
UnboundRegularFunction,
}

function thisIsAllowed(parent: parentType): boolean {
return [parentType.ClassMethod, parentType.BoundRegularFunction]
.some((t) => parent === t);
}
let currentParent: parentType = parentType.None;
let inClass = false;
let inFunctionInClass = false;

ts.forEachChild(sourceFile, function cb(node: ts.Node) {
const originalParent = currentParent;
const originalInClass = inClass;
switch (node.kind) {
case ts.SyntaxKind.ClassDeclaration:
case ts.SyntaxKind.ClassExpression:
if (!inClass) {
inClass = true;
ts.forEachChild(node, cb);
inClass = false;
return;
}
break;
inClass = true;
currentParent = parentType.Class;
ts.forEachChild(node, cb);
currentParent = originalParent;
inClass = originalInClass;
return;

case ts.SyntaxKind.MethodDeclaration:
case ts.SyntaxKind.GetAccessor:
case ts.SyntaxKind.SetAccessor:
case ts.SyntaxKind.Constructor:
case ts.SyntaxKind.PropertyDeclaration:
case ts.SyntaxKind.FunctionDeclaration:
case ts.SyntaxKind.FunctionExpression:
if ((node as ts.FunctionLikeDeclaration).parameters.some(isThisParameter)) {
if (currentParent === parentType.Class) {
currentParent = parentType.ClassMethod;
ts.forEachChild(node, cb);
currentParent = originalParent;
return;
}
if (inClass) {
inFunctionInClass = true;
} else {
currentParent
= (node as ts.FunctionLikeDeclaration).parameters.some(isThisParameter)
? parentType.BoundRegularFunction
: parentType.UnboundRegularFunction;
ts.forEachChild(node, cb);
inFunctionInClass = false;
currentParent = originalParent;
return;
}
break;

case ts.SyntaxKind.ThisKeyword:
if (!inClass) {
ctx.addFailureAtNode(node, Rule.FAILURE_STRING_OUTSIDE);
} else if (checkFuncInMethod && inFunctionInClass) {
ctx.addFailureAtNode(node, Rule.FAILURE_STRING_INSIDE);
if (!thisIsAllowed(currentParent)) {
if (!inClass) {
ctx.addFailureAtNode(node, Rule.FAILURE_STRING_OUTSIDE);
} else if (checkFuncInMethod) {
ctx.addFailureAtNode(node, Rule.FAILURE_STRING_INSIDE);
}
}
return;
}
Expand Down
40 changes: 40 additions & 0 deletions test/rules/no-invalid-this/enabled/test.ts.lint
@@ -1,3 +1,23 @@
function setSomeProperty(this: { someProperty: number }) {
this.someProperty = 7;
}

function bindProperty(this: { someProperty: number }, fn: Function) {
return fn.bind(this);
}

const objectLiteralStyle = {
bindProperty: function(this: { someProperty: number }, fn: Function) {
return fn.bind(this);
},
};

const objectMethodStyle = {
bindProperty(this: { someProperty: number }, fn: Function) {
return fn.bind(this);
},
};

export function f<T>(this: Observable<T>): Observable<T> {
const nestedFunction = function(this) => {
console.log(this)
Expand All @@ -10,6 +30,7 @@ class ContextualThisClass {
let nestedFunction: (this: number[]) => number[] = function(this) {
[3,4].forEach(function(nr){
console.log(this);
~~~~ [the "this" keyword is disallowed in function bodies inside class methods, use arrow functions instead]
});
return this.map(i=>i);
};
Expand Down Expand Up @@ -67,6 +88,25 @@ class AClass {
[3,4].forEach(badFunction);
let badFunction = nr => console.log(this.x === nr);
}

public fMethod() {
const objectLiteralStyle = {
bindProperty: function(this: { someProperty: number }, fn: Function) {
return fn.bind(this);
},
};

const objectMethodStyle = {
bindProperty(this: { someProperty: number }, fn: Function) {
return fn.bind(this);
},
};

return {
objectLiteralStyle,
objectMethodStyle,
};
}
}

const AClassExpression = class {
Expand Down

0 comments on commit 8167440

Please sign in to comment.