From c6264d01e5f858022ea99cf368adb626ff7b19af Mon Sep 17 00:00:00 2001 From: Joshua Stiefer Date: Mon, 2 Jan 2017 00:29:14 -0700 Subject: [PATCH 1/3] Check for createElement being called from React Fixes #996. This will prevent document.createElement triggering a false positive for react/display-name. --- lib/util/Components.js | 2 ++ tests/lib/rules/display-name.js | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/lib/util/Components.js b/lib/util/Components.js index 37816a3bd4..7ed953330e 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -258,6 +258,8 @@ function componentRule(rule, context) { var returnsReactCreateElement = node[property] && node[property].callee && + node[property].callee.object && + node[property].callee.object.name === 'React' && node[property].callee.property && node[property].callee.property.name === 'createElement' ; diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 28ef63d7fa..921302bca3 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -378,6 +378,13 @@ ruleTester.run('display-name', rule, { ')' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'module.exports = {', + ' createElement: tagName => document.createElement(tagName)', + '};' + ].join('\n'), + parser: 'babel-eslint' }], invalid: [{ From 47405b412b83f06b3f462f9d17fa6001c035f0ad Mon Sep 17 00:00:00 2001 From: Joshua Stiefer Date: Mon, 2 Jan 2017 03:11:43 -0700 Subject: [PATCH 2/3] Add check for destructured createElement Due to the previous change of checking for `createElement` being called on React, this left out the use case of a destructured import of `createElement`. --- lib/util/Components.js | 13 +++++++++++++ lib/util/variable.js | 20 ++++++++++++++++++++ tests/lib/rules/display-name.js | 17 +++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/lib/util/Components.js b/lib/util/Components.js index 7ed953330e..51f0416595 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -255,7 +255,20 @@ function componentRule(rule, context) { node[property] && node[property].type === 'JSXElement' ; + var destructuredReactCreateElement = function () { + var variables = variableUtil.variablesInScope(context); + var variable = variableUtil.getVariable(variables, 'createElement'); + if (variable !== null) { + var map = variable.scope.set; + // eslint-disable-next-line no-undef + if (map instanceof Map && map.has('React')) { + return true; + } + } + return false; + }; var returnsReactCreateElement = + destructuredReactCreateElement() || node[property] && node[property].callee && node[property].callee.object && diff --git a/lib/util/variable.js b/lib/util/variable.js index 81ae18d465..5cbc0df55b 100644 --- a/lib/util/variable.js +++ b/lib/util/variable.js @@ -23,6 +23,25 @@ function findVariable(variables, name) { return false; } +/** + * Find and return a particular variable in a list + * @param {Array} variables The variables list. + * @param {Array} name The name of the variable to search. + * @returns {Object} Variable if the variable was found, null if not. + */ +function getVariable(variables, name) { + var i; + var len; + + for (i = 0, len = variables.length; i < len; i++) { + if (variables[i].name === name) { + return variables[i]; + } + } + + return null; +} + /** * List all variable in a given scope * @@ -52,5 +71,6 @@ function variablesInScope(context) { module.exports = { findVariable: findVariable, + getVariable: getVariable, variablesInScope: variablesInScope }; diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 921302bca3..139c09d5cb 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -385,6 +385,12 @@ ruleTester.run('display-name', rule, { '};' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'const { createElement } = document;', + 'createElement("a");' + ].join('\n'), + parser: 'babel-eslint' }], invalid: [{ @@ -555,5 +561,16 @@ ruleTester.run('display-name', rule, { errors: [{ message: 'Component definition is missing display name' }] + }, { + code: [ + 'import React, { createElement } from "react";', + 'export default (props) => {', + ' return createElement("div", {}, "hello");', + '};' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Component definition is missing display name' + }] }] }); From 9b884740b1ca3ba06ebfe29389f6cf52e0231a66 Mon Sep 17 00:00:00 2001 From: Joshua Stiefer Date: Mon, 2 Jan 2017 11:44:32 -0700 Subject: [PATCH 3/3] Clean up variable utils and add more tests --- lib/util/Components.js | 5 ++--- lib/util/variable.js | 26 ++++++-------------------- tests/lib/rules/display-name.js | 24 ++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index 51f0416595..a71884b731 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -258,10 +258,9 @@ function componentRule(rule, context) { var destructuredReactCreateElement = function () { var variables = variableUtil.variablesInScope(context); var variable = variableUtil.getVariable(variables, 'createElement'); - if (variable !== null) { + if (variable) { var map = variable.scope.set; - // eslint-disable-next-line no-undef - if (map instanceof Map && map.has('React')) { + if (map.has('React')) { return true; } } diff --git a/lib/util/variable.js b/lib/util/variable.js index 5cbc0df55b..6ec67c686e 100644 --- a/lib/util/variable.js +++ b/lib/util/variable.js @@ -11,16 +11,9 @@ * @returns {Boolean} True if the variable was found, false if not. */ function findVariable(variables, name) { - var i; - var len; - - for (i = 0, len = variables.length; i < len; i++) { - if (variables[i].name === name) { - return true; - } - } - - return false; + return variables.some(function (variable) { + return variable.name === name; + }); } /** @@ -30,16 +23,9 @@ function findVariable(variables, name) { * @returns {Object} Variable if the variable was found, null if not. */ function getVariable(variables, name) { - var i; - var len; - - for (i = 0, len = variables.length; i < len; i++) { - if (variables[i].name === name) { - return variables[i]; - } - } - - return null; + return variables.find(function (variable) { + return variable.name === name; + }); } /** diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 139c09d5cb..3f8db25185 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -572,5 +572,29 @@ ruleTester.run('display-name', rule, { errors: [{ message: 'Component definition is missing display name' }] + }, { + code: [ + 'import React from "react";', + 'const { createElement } = React;', + 'export default (props) => {', + ' return createElement("div", {}, "hello");', + '};' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Component definition is missing display name' + }] + }, { + code: [ + 'import React from "react";', + 'const createElement = React.createElement;', + 'export default (props) => {', + ' return createElement("div", {}, "hello");', + '};' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Component definition is missing display name' + }] }] });