Skip to content

[Fix]: improve algorithm to check if a variable is coming from the pragma #2706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 57 additions & 2 deletions lib/util/Components.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {<variable>} from 'react'
if (
latestDef.parent
&& latestDef.parent.type === 'ImportDeclaration'
&& latestDef.parent.source.value === pragma.toLocaleLowerCase()
) {
return true;
}
}
}
return false;
},
Expand Down
12 changes: 11 additions & 1 deletion lib/util/variable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
185 changes: 185 additions & 0 deletions tests/lib/rules/no-multi-comp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div>
<div>Counter</div>
</div>
);
}
}
`,
parser: parsers.BABEL_ESLINT
}],

invalid: [{
Expand Down Expand Up @@ -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 <div></div>;
});
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
`,
options: [{
ignoreStateless: false
}],
errors: [{
message: 'Declare only one React component per file',
line: 6
}]
}, {
code: `
const memo = React.memo;
const HelloComponent = (props) => {
return <div></div>;
};
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
`,
options: [{
ignoreStateless: false
}],
errors: [{
message: 'Declare only one React component per file',
line: 6
}]
}, {
code: `
const {forwardRef} = React;
const HelloComponent = (0, (props) => {
return <div></div>;
});
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
`,
options: [{
ignoreStateless: false
}],
errors: [{
message: 'Declare only one React component per file',
line: 6
}]
}, {
code: `
const {memo} = React;
const HelloComponent = (0, (props) => {
return <div></div>;
});
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
`,
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 <div></div>;
});
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
`,
options: [{
ignoreStateless: false
}],
errors: [{
message: 'Declare only one React component per file',
line: 6
}]
}, {
code: `
import {forwardRef} from 'react';
const HelloComponent = (0, (props) => {
return <div></div>;
});
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
`,
options: [{
ignoreStateless: false
}],
errors: [{
message: 'Declare only one React component per file',
line: 6
}]
}, {
code: `
const { memo } = require('react');
const HelloComponent = (0, (props) => {
return <div></div>;
});
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
`,
options: [{
ignoreStateless: false
}],
errors: [{
message: 'Declare only one React component per file',
line: 6
}]
}, {
code: `
const {forwardRef} = require('react');
const HelloComponent = (0, (props) => {
return <div></div>;
});
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
`,
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 <div></div>;
});
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
`,
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 <div></div>;
});
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
`,
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 <div ref={ref}>{text}</div>;
})
const Label = memo(() => <Text />);
`,
settings: {
react: {
pragma: 'Foo'
}
},
errors: [{
message: 'Declare only one React component per file'
}]
}]
});
27 changes: 27 additions & 0 deletions tests/util/variable.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
});