From e92a08fd8f1c8b0c77fad06b8f22c8606f9ccc82 Mon Sep 17 00:00:00 2001 From: Andrew <0coming.soon@gmail.com> Date: Mon, 1 Apr 2019 02:19:21 +0500 Subject: [PATCH] Fixed decorated methods declared after usage (#813) (#852) --- src/reactThisBindingIssueRule.ts | 16 +++++++++++----- src/tests/ReactThisBindingIssueRuleTests.ts | 5 +++++ ...ueWithDecoratorDeclaredAfterUsage-passing.tsx | 12 ++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 test-data/ReactThisBinding/ReactThisBindingIssueWithDecoratorDeclaredAfterUsage-passing.tsx diff --git a/src/reactThisBindingIssueRule.ts b/src/reactThisBindingIssueRule.ts index 41e7e235c..689342fb3 100644 --- a/src/reactThisBindingIssueRule.ts +++ b/src/reactThisBindingIssueRule.ts @@ -244,13 +244,20 @@ function walk(ctx: Lint.WalkContext) { ); } + function visitClassMember(node: ts.Node) { + if (tsutils.isConstructorDeclaration(node)) { + boundListeners = getSelfBoundListeners(node); + } else if (tsutils.isMethodDeclaration(node)) { + if (isMethodBoundWithDecorators(node, ctx.options.allowedDecorators)) { + boundListeners = boundListeners.add('this.' + node.name.getText()); + } + } + } + function cb(node: ts.Node): void { if (tsutils.isMethodDeclaration(node)) { // reset variable scope when we encounter a method. Start tracking variables that are instantiated // in scope so we can make sure new function instances are not passed as JSX attributes - if (isMethodBoundWithDecorators(node, ctx.options.allowedDecorators)) { - boundListeners = boundListeners.add('this.' + node.name.getText()); - } scope = new Scope(undefined); ts.forEachChild(node, cb); scope = undefined; @@ -289,8 +296,7 @@ function walk(ctx: Lint.WalkContext) { declaredMethods.add('this.' + methodName); } ); - } else if (tsutils.isConstructorDeclaration(node)) { - boundListeners = getSelfBoundListeners(node); + node.members.forEach(visitClassMember); } else if (tsutils.isJsxElement(node)) { visitJsxOpeningElement(node.openingElement); // a normal JSX element has-a OpeningElement } else if (tsutils.isJsxSelfClosingElement(node)) { diff --git a/src/tests/ReactThisBindingIssueRuleTests.ts b/src/tests/ReactThisBindingIssueRuleTests.ts index e23fce9fb..550719cd4 100644 --- a/src/tests/ReactThisBindingIssueRuleTests.ts +++ b/src/tests/ReactThisBindingIssueRuleTests.ts @@ -12,6 +12,11 @@ describe('reactThisBindingIssueRule', (): void => { TestHelper.assertNoViolationWithOptions(ruleName, [true, { 'bind-decorators': ['autobind'] }], fileWithDecorator); }); + it('should pass even if a decorated method declared after usage', (): void => { + const file: string = 'test-data/ReactThisBinding/ReactThisBindingIssueWithDecoratorDeclaredAfterUsage-passing.tsx'; + TestHelper.assertNoViolationWithOptions(ruleName, [true, { 'bind-decorators': ['autobind'] }], file); + }); + it('should fail if decorator is not whitelisted in config', () => { const fileWithDecorator: string = 'test-data/ReactThisBinding/ReactThisBindingIssueWithDecorator-passing.tsx'; const expectedMsg = `A class method is passed as a JSX attribute without having the 'this' reference bound: `; diff --git a/test-data/ReactThisBinding/ReactThisBindingIssueWithDecoratorDeclaredAfterUsage-passing.tsx b/test-data/ReactThisBinding/ReactThisBindingIssueWithDecoratorDeclaredAfterUsage-passing.tsx new file mode 100644 index 000000000..4f208f49b --- /dev/null +++ b/test-data/ReactThisBinding/ReactThisBindingIssueWithDecoratorDeclaredAfterUsage-passing.tsx @@ -0,0 +1,12 @@ +import React = require('react'); + +declare const autobind: MethodDecorator; + +export class MyComponent extends React.Component { + public render() { + return
; + } + + @autobind + private listener1(): void {} +}