From e3e767bd041988d9acb7713874c0632c68408347 Mon Sep 17 00:00:00 2001 From: Johnny Zabala Date: Thu, 9 Jul 2020 19:20:13 -0400 Subject: [PATCH] [Fix]: improve algorithm to check if a variable is coming from the pragma --- lib/util/Components.js | 59 +++++++++- lib/util/variable.js | 12 +- tests/lib/rules/no-multi-comp.js | 185 +++++++++++++++++++++++++++++++ tests/util/variable.js | 27 +++++ 4 files changed, 280 insertions(+), 3 deletions(-) create mode 100644 tests/util/variable.js diff --git a/lib/util/Components.js b/lib/util/Components.js index b5abdd36f2..deb26dda56 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -307,8 +307,63 @@ function componentRule(rule, context) { const variables = variableUtil.variablesInScope(context); const variableInScope = variableUtil.getVariable(variables, variable); if (variableInScope) { - const map = variableInScope.scope.set; - return map.has(pragma); + const latestDef = variableUtil.getLatestVariableDefinition(variableInScope); + if (latestDef) { + // check if latest definition is a variable declaration: 'variable = value' + if (latestDef.node.type === 'VariableDeclarator' && latestDef.node.init) { + // check for: 'variable = pragma.variable' + if ( + latestDef.node.init.type === 'MemberExpression' + && latestDef.node.init.object.type === 'Identifier' + && latestDef.node.init.object.name === pragma + ) { + return true; + } + // check for: '{variable} = pragma' + if ( + latestDef.node.init.type === 'Identifier' + && latestDef.node.init.name === pragma + ) { + return true; + } + + // "require('react')" + let requireExpression = null; + + // get "require('react')" from: "{variable} = require('react')" + if (latestDef.node.init.type === 'CallExpression') { + requireExpression = latestDef.node.init; + } + // get "require('react')" from: "variable = require('react').variable" + if ( + !requireExpression + && latestDef.node.init.type === 'MemberExpression' + && latestDef.node.init.object.type === 'CallExpression' + ) { + requireExpression = latestDef.node.init.object; + } + + // check proper require. + if ( + requireExpression.callee.name === 'require' + && requireExpression.arguments[0] + && requireExpression.arguments[0].value === pragma.toLocaleLowerCase() + ) { + return true; + } + + return false; + } + + // latest definition is an import declaration: import {} from 'react' + if ( + latestDef.parent + && latestDef.parent.type === 'ImportDeclaration' + && latestDef.parent.source.value === pragma.toLocaleLowerCase() + ) { + return true; + } + } } return false; }, diff --git a/lib/util/variable.js b/lib/util/variable.js index 0575c75d50..8e91e80a08 100644 --- a/lib/util/variable.js +++ b/lib/util/variable.js @@ -72,9 +72,19 @@ function findVariableByName(context, name) { return variable.defs[0].node.init; } +/** + * Returns the latest definition of the variable. + * @param {Object} variable + * @returns {Object | undefined} The latest variable definition or undefined. + */ +function getLatestVariableDefinition(variable) { + return variable.defs[variable.defs.length - 1]; +} + module.exports = { findVariable, findVariableByName, getVariable, - variablesInScope + variablesInScope, + getLatestVariableDefinition }; diff --git a/tests/lib/rules/no-multi-comp.js b/tests/lib/rules/no-multi-comp.js index a4bad0c10b..e10b554bb2 100644 --- a/tests/lib/rules/no-multi-comp.js +++ b/tests/lib/rules/no-multi-comp.js @@ -221,6 +221,25 @@ ruleTester.run('no-multi-comp', rule, { options: [{ ignoreStateless: false }] + }, { + code: ` + import React from 'react'; + function memo() { + var outOfScope = "hello" + return null; + } + class ComponentY extends React.Component { + memoCities = memo((cities) => cities.map((v) => ({ label: v }))); + render() { + return ( +
+
Counter
+
+ ); + } + } + `, + parser: parsers.BABEL_ESLINT }], invalid: [{ @@ -376,5 +395,171 @@ ruleTester.run('no-multi-comp', rule, { message: 'Declare only one React component per file', line: 5 }] + }, { + code: ` + const forwardRef = React.forwardRef; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = forwardRef((props, ref) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const memo = React.memo; + const HelloComponent = (props) => { + return
; + }; + const HelloComponent2 = memo((props) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const {forwardRef} = React; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = forwardRef((props, ref) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const {memo} = React; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = memo((props) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + import React, { memo } from 'react'; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = memo((props) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + import {forwardRef} from 'react'; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = forwardRef((props, ref) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const { memo } = require('react'); + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = memo((props) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const {forwardRef} = require('react'); + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = forwardRef((props, ref) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const forwardRef = require('react').forwardRef; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = forwardRef((props, ref) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + const memo = require('react').memo; + const HelloComponent = (0, (props) => { + return
; + }); + const HelloComponent2 = memo((props) => ); + `, + options: [{ + ignoreStateless: false + }], + errors: [{ + message: 'Declare only one React component per file', + line: 6 + }] + }, { + code: ` + import Foo, { memo, forwardRef } from 'foo'; + const Text = forwardRef(({ text }, ref) => { + return
{text}
; + }) + const Label = memo(() => ); + `, + settings: { + react: { + pragma: 'Foo' + } + }, + errors: [{ + message: 'Declare only one React component per file' + }] }] }); diff --git a/tests/util/variable.js b/tests/util/variable.js new file mode 100644 index 0000000000..433a8b1532 --- /dev/null +++ b/tests/util/variable.js @@ -0,0 +1,27 @@ +'use strict'; + +const assert = require('assert'); + +const getLatestVariableDefinition = require('../../lib/util/variable').getLatestVariableDefinition; + +describe('variable', () => { + describe('getLatestVariableDefinition', () => { + it('should return undefined for empty definitions', () => { + const variable = { + defs: [] + }; + assert.equal(getLatestVariableDefinition(variable), undefined); + }); + + it('should return the latest definition', () => { + const variable = { + defs: [ + 'one', + 'two', + 'latest' + ] + }; + assert.equal(getLatestVariableDefinition(variable), 'latest'); + }); + }); +});