From 68d63845635da3fedd6205b068cb33ff3d848c76 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Tue, 25 Sep 2018 02:00:40 +0300 Subject: [PATCH 1/4] Fix `no-this-in-sfc` for class properties Fixed #1960 --- lib/util/Components.js | 22 ++++++++++++---------- tests/lib/rules/no-this-in-sfc.js | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index b7e2a99a63..2ea7cc896f 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -456,15 +456,16 @@ function componentRule(rule, context) { let scope = context.getScope(); while (scope) { const node = scope.block; - const isClass = node.type === 'ClassExpression'; const isFunction = /Function/.test(node.type); // Functions const isArrowFunction = node.type === 'ArrowFunctionExpression'; - let functionScope = scope; + let enclosingScope = scope; if (isArrowFunction) { - functionScope = utils.getParentFunctionScope(scope); + enclosingScope = utils.getArrowFunctionScope(scope); } - const methodNode = functionScope && functionScope.block.parent; - const isMethod = methodNode && methodNode.type === 'MethodDefinition'; // Classes methods + const enclosingScopeType = enclosingScope && enclosingScope.block.type; + const enclosingScopeParent = enclosingScope && enclosingScope.block.parent; + const isClass = enclosingScopeType === 'ClassDeclaration' || enclosingScopeType === 'ClassExpression'; + const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.) // Attribute Expressions inside JSX Elements () const isJSXExpressionContainer = node.parent && node.parent.type === 'JSXExpressionContainer'; @@ -482,15 +483,16 @@ function componentRule(rule, context) { }, /** - * Get a parent scope created by a FunctionExpression or FunctionDeclaration - * @param {Scope} scope The child scope - * @returns {Scope} A parent function scope + * Get an enclosing scope used to find `this` value by an arrow function + * @param {Scope} scope Current scope + * @returns {Scope} An enclosing scope used by an arrow function */ - getParentFunctionScope(scope) { + getArrowFunctionScope(scope) { scope = scope.upper; while (scope) { const type = scope.block.type; - if (type === 'FunctionExpression' || type === 'FunctionDeclaration') { + if (type === 'FunctionExpression' || type === 'FunctionDeclaration' + || type === 'ClassDeclaration' || type === 'ClassExpression') { return scope; } scope = scope.upper; diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index 63a849f840..9c241cb8ed 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -16,6 +16,8 @@ const ERROR_MESSAGE = 'Stateless functional components should not use this'; const rule = require('../../../lib/rules/no-this-in-sfc'); const RuleTester = require('eslint').RuleTester; +require('babel-eslint'); + const parserOptions = { ecmaVersion: 2018, sourceType: 'module', @@ -119,6 +121,24 @@ ruleTester.run('no-this-in-sfc', rule, { }; } }` + }, { + code: ` + class Foo { + bar = () => { + this.something(); + return null; + }; + }`, + parser: 'babel-eslint' + }, { + code: ` + class Foo { + bar = () => () => { + this.something(); + return null; + }; + }`, + parser: 'babel-eslint' }], invalid: [{ code: ` From 08e9d0b85841d764635c78d026cc3842c523d8f4 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 01:13:51 +0300 Subject: [PATCH 2/4] Extract util functions to AST utils --- lib/util/Components.js | 9 +++------ lib/util/ast.js | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index 2ea7cc896f..eb24ee41b4 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -457,14 +457,13 @@ function componentRule(rule, context) { while (scope) { const node = scope.block; const isFunction = /Function/.test(node.type); // Functions - const isArrowFunction = node.type === 'ArrowFunctionExpression'; + const isArrowFunction = astUtil.isArrowFunction(node); let enclosingScope = scope; if (isArrowFunction) { enclosingScope = utils.getArrowFunctionScope(scope); } - const enclosingScopeType = enclosingScope && enclosingScope.block.type; const enclosingScopeParent = enclosingScope && enclosingScope.block.parent; - const isClass = enclosingScopeType === 'ClassDeclaration' || enclosingScopeType === 'ClassExpression'; + const isClass = enclosingScope && astUtil.isClass(enclosingScope.block); const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.) // Attribute Expressions inside JSX Elements () @@ -490,9 +489,7 @@ function componentRule(rule, context) { getArrowFunctionScope(scope) { scope = scope.upper; while (scope) { - const type = scope.block.type; - if (type === 'FunctionExpression' || type === 'FunctionDeclaration' - || type === 'ClassDeclaration' || type === 'ClassExpression') { + if (astUtil.isFunction(scope.block) || astUtil.isClass(scope.block)) { return scope; } scope = scope.upper; diff --git a/lib/util/ast.js b/lib/util/ast.js index 10fc434625..9af7e0c5ce 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -102,11 +102,41 @@ function isFunctionLikeExpression(node) { return node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression'; } +/** + * Checks if the node is a function. + * @param {Object} context The node to check + * @return {Boolean} true if it's a function + */ +function isFunction(node) { + return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration'; +} + +/** + * Checks if the node is an arrow function. + * @param {Object} context The node to check + * @return {Boolean} true if it's an arrow function + */ +function isArrowFunction(node) { + return node.type === 'ArrowFunctionExpression'; +} + +/** + * Checks if the node is a class. + * @param {Object} context The node to check + * @return {Boolean} true if it's a class + */ +function isClass(node) { + return node.type === 'ClassDeclaration' || node.type === 'ClassExpression'; +} + module.exports = { findReturnStatement: findReturnStatement, getPropertyName: getPropertyName, getPropertyNameNode: getPropertyNameNode, getComponentProperties: getComponentProperties, - isNodeFirstInLine: isNodeFirstInLine, - isFunctionLikeExpression: isFunctionLikeExpression + isArrowFunction: isArrowFunction, + isClass: isClass, + isFunction: isFunction, + isFunctionLikeExpression: isFunctionLikeExpression, + isNodeFirstInLine: isNodeFirstInLine }; From fee8c360c5c8b59e9c06cc68cce1951b756e927f Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 16:08:08 +0300 Subject: [PATCH 3/4] Remove redundant `require('babel-eslint')` --- tests/lib/rules/no-this-in-sfc.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index 9c241cb8ed..b5d87f0267 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -16,8 +16,6 @@ const ERROR_MESSAGE = 'Stateless functional components should not use this'; const rule = require('../../../lib/rules/no-this-in-sfc'); const RuleTester = require('eslint').RuleTester; -require('babel-eslint'); - const parserOptions = { ecmaVersion: 2018, sourceType: 'module', From e779b00821d35a9d5d52b3efdd1d0c09e418d63c Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 16:40:47 +0300 Subject: [PATCH 4/4] Minor adjustments --- lib/util/Components.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index eb24ee41b4..15026089dc 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -458,10 +458,7 @@ function componentRule(rule, context) { const node = scope.block; const isFunction = /Function/.test(node.type); // Functions const isArrowFunction = astUtil.isArrowFunction(node); - let enclosingScope = scope; - if (isArrowFunction) { - enclosingScope = utils.getArrowFunctionScope(scope); - } + const enclosingScope = isArrowFunction ? utils.getArrowFunctionScope(scope) : scope; const enclosingScopeParent = enclosingScope && enclosingScope.block.parent; const isClass = enclosingScope && astUtil.isClass(enclosingScope.block); const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods