Skip to content

Commit e3e767b

Browse files
jzabalaljharb
authored andcommitted
[Fix]: improve algorithm to check if a variable is coming from the pragma
1 parent c57cc31 commit e3e767b

File tree

4 files changed

+280
-3
lines changed

4 files changed

+280
-3
lines changed

lib/util/Components.js

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,63 @@ function componentRule(rule, context) {
307307
const variables = variableUtil.variablesInScope(context);
308308
const variableInScope = variableUtil.getVariable(variables, variable);
309309
if (variableInScope) {
310-
const map = variableInScope.scope.set;
311-
return map.has(pragma);
310+
const latestDef = variableUtil.getLatestVariableDefinition(variableInScope);
311+
if (latestDef) {
312+
// check if latest definition is a variable declaration: 'variable = value'
313+
if (latestDef.node.type === 'VariableDeclarator' && latestDef.node.init) {
314+
// check for: 'variable = pragma.variable'
315+
if (
316+
latestDef.node.init.type === 'MemberExpression'
317+
&& latestDef.node.init.object.type === 'Identifier'
318+
&& latestDef.node.init.object.name === pragma
319+
) {
320+
return true;
321+
}
322+
// check for: '{variable} = pragma'
323+
if (
324+
latestDef.node.init.type === 'Identifier'
325+
&& latestDef.node.init.name === pragma
326+
) {
327+
return true;
328+
}
329+
330+
// "require('react')"
331+
let requireExpression = null;
332+
333+
// get "require('react')" from: "{variable} = require('react')"
334+
if (latestDef.node.init.type === 'CallExpression') {
335+
requireExpression = latestDef.node.init;
336+
}
337+
// get "require('react')" from: "variable = require('react').variable"
338+
if (
339+
!requireExpression
340+
&& latestDef.node.init.type === 'MemberExpression'
341+
&& latestDef.node.init.object.type === 'CallExpression'
342+
) {
343+
requireExpression = latestDef.node.init.object;
344+
}
345+
346+
// check proper require.
347+
if (
348+
requireExpression.callee.name === 'require'
349+
&& requireExpression.arguments[0]
350+
&& requireExpression.arguments[0].value === pragma.toLocaleLowerCase()
351+
) {
352+
return true;
353+
}
354+
355+
return false;
356+
}
357+
358+
// latest definition is an import declaration: import {<variable>} from 'react'
359+
if (
360+
latestDef.parent
361+
&& latestDef.parent.type === 'ImportDeclaration'
362+
&& latestDef.parent.source.value === pragma.toLocaleLowerCase()
363+
) {
364+
return true;
365+
}
366+
}
312367
}
313368
return false;
314369
},

lib/util/variable.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,19 @@ function findVariableByName(context, name) {
7272
return variable.defs[0].node.init;
7373
}
7474

75+
/**
76+
* Returns the latest definition of the variable.
77+
* @param {Object} variable
78+
* @returns {Object | undefined} The latest variable definition or undefined.
79+
*/
80+
function getLatestVariableDefinition(variable) {
81+
return variable.defs[variable.defs.length - 1];
82+
}
83+
7584
module.exports = {
7685
findVariable,
7786
findVariableByName,
7887
getVariable,
79-
variablesInScope
88+
variablesInScope,
89+
getLatestVariableDefinition
8090
};

tests/lib/rules/no-multi-comp.js

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,25 @@ ruleTester.run('no-multi-comp', rule, {
221221
options: [{
222222
ignoreStateless: false
223223
}]
224+
}, {
225+
code: `
226+
import React from 'react';
227+
function memo() {
228+
var outOfScope = "hello"
229+
return null;
230+
}
231+
class ComponentY extends React.Component {
232+
memoCities = memo((cities) => cities.map((v) => ({ label: v })));
233+
render() {
234+
return (
235+
<div>
236+
<div>Counter</div>
237+
</div>
238+
);
239+
}
240+
}
241+
`,
242+
parser: parsers.BABEL_ESLINT
224243
}],
225244

226245
invalid: [{
@@ -376,5 +395,171 @@ ruleTester.run('no-multi-comp', rule, {
376395
message: 'Declare only one React component per file',
377396
line: 5
378397
}]
398+
}, {
399+
code: `
400+
const forwardRef = React.forwardRef;
401+
const HelloComponent = (0, (props) => {
402+
return <div></div>;
403+
});
404+
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
405+
`,
406+
options: [{
407+
ignoreStateless: false
408+
}],
409+
errors: [{
410+
message: 'Declare only one React component per file',
411+
line: 6
412+
}]
413+
}, {
414+
code: `
415+
const memo = React.memo;
416+
const HelloComponent = (props) => {
417+
return <div></div>;
418+
};
419+
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
420+
`,
421+
options: [{
422+
ignoreStateless: false
423+
}],
424+
errors: [{
425+
message: 'Declare only one React component per file',
426+
line: 6
427+
}]
428+
}, {
429+
code: `
430+
const {forwardRef} = React;
431+
const HelloComponent = (0, (props) => {
432+
return <div></div>;
433+
});
434+
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
435+
`,
436+
options: [{
437+
ignoreStateless: false
438+
}],
439+
errors: [{
440+
message: 'Declare only one React component per file',
441+
line: 6
442+
}]
443+
}, {
444+
code: `
445+
const {memo} = React;
446+
const HelloComponent = (0, (props) => {
447+
return <div></div>;
448+
});
449+
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
450+
`,
451+
options: [{
452+
ignoreStateless: false
453+
}],
454+
errors: [{
455+
message: 'Declare only one React component per file',
456+
line: 6
457+
}]
458+
}, {
459+
code: `
460+
import React, { memo } from 'react';
461+
const HelloComponent = (0, (props) => {
462+
return <div></div>;
463+
});
464+
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
465+
`,
466+
options: [{
467+
ignoreStateless: false
468+
}],
469+
errors: [{
470+
message: 'Declare only one React component per file',
471+
line: 6
472+
}]
473+
}, {
474+
code: `
475+
import {forwardRef} from 'react';
476+
const HelloComponent = (0, (props) => {
477+
return <div></div>;
478+
});
479+
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
480+
`,
481+
options: [{
482+
ignoreStateless: false
483+
}],
484+
errors: [{
485+
message: 'Declare only one React component per file',
486+
line: 6
487+
}]
488+
}, {
489+
code: `
490+
const { memo } = require('react');
491+
const HelloComponent = (0, (props) => {
492+
return <div></div>;
493+
});
494+
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
495+
`,
496+
options: [{
497+
ignoreStateless: false
498+
}],
499+
errors: [{
500+
message: 'Declare only one React component per file',
501+
line: 6
502+
}]
503+
}, {
504+
code: `
505+
const {forwardRef} = require('react');
506+
const HelloComponent = (0, (props) => {
507+
return <div></div>;
508+
});
509+
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
510+
`,
511+
options: [{
512+
ignoreStateless: false
513+
}],
514+
errors: [{
515+
message: 'Declare only one React component per file',
516+
line: 6
517+
}]
518+
}, {
519+
code: `
520+
const forwardRef = require('react').forwardRef;
521+
const HelloComponent = (0, (props) => {
522+
return <div></div>;
523+
});
524+
const HelloComponent2 = forwardRef((props, ref) => <HelloComponent></HelloComponent>);
525+
`,
526+
options: [{
527+
ignoreStateless: false
528+
}],
529+
errors: [{
530+
message: 'Declare only one React component per file',
531+
line: 6
532+
}]
533+
}, {
534+
code: `
535+
const memo = require('react').memo;
536+
const HelloComponent = (0, (props) => {
537+
return <div></div>;
538+
});
539+
const HelloComponent2 = memo((props) => <HelloComponent></HelloComponent>);
540+
`,
541+
options: [{
542+
ignoreStateless: false
543+
}],
544+
errors: [{
545+
message: 'Declare only one React component per file',
546+
line: 6
547+
}]
548+
}, {
549+
code: `
550+
import Foo, { memo, forwardRef } from 'foo';
551+
const Text = forwardRef(({ text }, ref) => {
552+
return <div ref={ref}>{text}</div>;
553+
})
554+
const Label = memo(() => <Text />);
555+
`,
556+
settings: {
557+
react: {
558+
pragma: 'Foo'
559+
}
560+
},
561+
errors: [{
562+
message: 'Declare only one React component per file'
563+
}]
379564
}]
380565
});

tests/util/variable.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
5+
const getLatestVariableDefinition = require('../../lib/util/variable').getLatestVariableDefinition;
6+
7+
describe('variable', () => {
8+
describe('getLatestVariableDefinition', () => {
9+
it('should return undefined for empty definitions', () => {
10+
const variable = {
11+
defs: []
12+
};
13+
assert.equal(getLatestVariableDefinition(variable), undefined);
14+
});
15+
16+
it('should return the latest definition', () => {
17+
const variable = {
18+
defs: [
19+
'one',
20+
'two',
21+
'latest'
22+
]
23+
};
24+
assert.equal(getLatestVariableDefinition(variable), 'latest');
25+
});
26+
});
27+
});

0 commit comments

Comments
 (0)