Skip to content

Commit 0c06bb1

Browse files
golopotgolopot
committed
[New] prop-types: handle variables defined as props
1 parent 7d036cf commit 0c06bb1

File tree

3 files changed

+218
-38
lines changed

3 files changed

+218
-38
lines changed

‎lib/util/usedPropTypes.js

+82-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,46 @@ const ast = require('./ast');
1515
const LIFE_CYCLE_METHODS = ['componentWillReceiveProps', 'shouldComponentUpdate', 'componentWillUpdate', 'componentDidUpdate'];
1616
const ASYNC_SAFE_LIFE_CYCLE_METHODS = ['getDerivedStateFromProps', 'getSnapshotBeforeUpdate', 'UNSAFE_componentWillReceiveProps', 'UNSAFE_componentWillUpdate'];
1717

18+
function createPropVariables() {
19+
/** @type {Map<string, string[]>} Maps the variable to its definition. `props.a.b` is stored as `['a', 'b']` */
20+
let propVariables = new Map();
21+
let hasBeenWritten = false;
22+
const stack = [{propVariables, hasBeenWritten}];
23+
return {
24+
pushScope() {
25+
// popVariables is not copied until first write.
26+
stack.push({propVariables, hasBeenWritten: false});
27+
},
28+
popScope() {
29+
stack.pop();
30+
propVariables = stack[stack.length - 1].propVariables;
31+
hasBeenWritten = stack[stack.length - 1].hasBeenWritten;
32+
},
33+
/**
34+
* Add a variable name to the current scope
35+
* @param {string} name
36+
* @param {string[]} allNames Example: `props.a.b` should be formatted as `['a', 'b']`
37+
*/
38+
set(name, allNames) {
39+
if (!hasBeenWritten) {
40+
// copy on write
41+
propVariables = new Map(propVariables);
42+
stack[stack.length - 1].propVariables = propVariables;
43+
stack[stack.length - 1].hasBeenWritten = true;
44+
}
45+
return propVariables.set(name, allNames);
46+
},
47+
/**
48+
* Get the definition of a variable.
49+
* @param {string} name
50+
* @returns {string[]} Example: `props.a.b` is represented by `['a', 'b']`
51+
*/
52+
get(name) {
53+
return propVariables.get(name);
54+
}
55+
};
56+
}
57+
1858
/**
1959
* Checks if the string is one of `props`, `nextProps`, or `prevProps`
2060
* @param {string} name The AST node being checked.
@@ -230,6 +270,8 @@ function isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafe
230270
module.exports = function usedPropTypesInstructions(context, components, utils) {
231271
const checkAsyncSafeLifeCycles = versionUtil.testReactVersion(context, '16.3.0');
232272

273+
const propVariables = createPropVariables();
274+
233275
/**
234276
* Mark a prop type as used
235277
* @param {ASTNode} node The AST node being marked.
@@ -261,6 +303,14 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
261303
node.parent.id.parent = node.parent; // patch for bug in eslint@4 in which ObjectPattern has no parent
262304
markPropTypesAsUsed(node.parent.id, allNames);
263305
}
306+
307+
// const a = props.a
308+
if (
309+
node.parent.type === 'VariableDeclarator' &&
310+
node.parent.id.type === 'Identifier'
311+
) {
312+
propVariables.set(node.parent.id.name, allNames);
313+
}
264314
// Do not mark computed props as used.
265315
type = name !== '__COMPUTED_PROP__' ? 'direct' : null;
266316
}
@@ -314,6 +364,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
314364
const propName = ast.getKeyValue(context, properties[k]);
315365

316366
if (propName) {
367+
propVariables.set(propName, parentNames.concat([propName]));
317368
usedPropTypes.push({
318369
allNames: parentNames.concat([propName]),
319370
name: propName,
@@ -371,6 +422,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
371422
* FunctionDeclaration, or FunctionExpression
372423
*/
373424
function handleFunctionLikeExpressions(node) {
425+
propVariables.pushScope();
374426
handleSetStateUpdater(node);
375427
markDestructuredFunctionArgumentsAsUsed(node);
376428
}
@@ -392,6 +444,11 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
392444

393445
return {
394446
VariableDeclarator(node) {
447+
// let props = this.props
448+
if (isThisDotProps(node.init) && isInClassComponent(utils) && node.id.type === 'Identifier') {
449+
propVariables.set(node.id.name, []);
450+
}
451+
395452
// Only handles destructuring
396453
if (node.id.type !== 'ObjectPattern') {
397454
return;
@@ -400,14 +457,19 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
400457
// let {props: {firstname}} = this
401458
const propsProperty = node.id.properties.find(property => (
402459
property.key &&
403-
(property.key.name === 'props' || property.key.value === 'props') &&
404-
property.value.type === 'ObjectPattern'
460+
(property.key.name === 'props' || property.key.value === 'props')
405461
));
406-
if (propsProperty && node.init.type === 'ThisExpression') {
462+
if (node.init.type === 'ThisExpression' && propsProperty && propsProperty.value.type === 'ObjectPattern') {
407463
markPropTypesAsUsed(propsProperty.value);
408464
return;
409465
}
410466

467+
// let {props} = this
468+
if (node.init.type === 'ThisExpression' && propsProperty && propsProperty.value.name === 'props') {
469+
propVariables.set('props', []);
470+
return;
471+
}
472+
411473
// let {firstname} = props
412474
if (
413475
isCommonVariableNameForProps(node.init.name) &&
@@ -420,6 +482,12 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
420482
// let {firstname} = this.props
421483
if (isThisDotProps(node.init) && isInClassComponent(utils)) {
422484
markPropTypesAsUsed(node.id);
485+
return;
486+
}
487+
488+
// let {firstname} = thing, where thing is defined by const thing = this.props.**.*
489+
if (propVariables.get(node.init.name)) {
490+
markPropTypesAsUsed(node, propVariables.get(node.init.name));
423491
}
424492
},
425493

@@ -429,6 +497,12 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
429497

430498
FunctionExpression: handleFunctionLikeExpressions,
431499

500+
'FunctionDeclaration:exit': propVariables.popScope,
501+
502+
'ArrowFunctionExpression:exit': propVariables.popScope,
503+
504+
'FunctionExpression:exit': propVariables.popScope,
505+
432506
JSXSpreadAttribute(node) {
433507
const component = components.get(utils.getParentComponent());
434508
components.set(component ? component.node : node, {
@@ -439,6 +513,11 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
439513
MemberExpression(node) {
440514
if (isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafeLifeCycles)) {
441515
markPropTypesAsUsed(node);
516+
return;
517+
}
518+
519+
if (propVariables.get(node.object.name)) {
520+
markPropTypesAsUsed(node, propVariables.get(node.object.name));
442521
}
443522
},
444523

‎tests/lib/rules/no-unused-prop-types.js

+53-21
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,59 @@ ruleTester.run('no-unused-prop-types', rule, {
479479
'};'
480480
].join('\n'),
481481
parser: parsers.BABEL_ESLINT
482+
}, {
483+
code: `
484+
function Foo({ a }) {
485+
return <>{ a.b }</>
486+
}
487+
Foo.propTypes = {
488+
a: PropTypes.shape({
489+
b: PropType.string,
490+
})
491+
}
492+
`,
493+
options: [{skipShapeProps: false}],
494+
parser: parsers.BABEL_ESLINT
495+
}, {
496+
// Destructured assignment with Shape propTypes with skipShapeProps off issue #816
497+
code: `
498+
class Thing extends React.Component {
499+
static propTypes = {
500+
i18n: PropTypes.shape({
501+
gettext: PropTypes.func,
502+
}),
503+
}
504+
505+
render() {
506+
const { i18n } = this.props;
507+
return (
508+
<p>{i18n.gettext('Some Text')}</p>
509+
);
510+
}
511+
}
512+
`,
513+
parser: parsers.BABEL_ESLINT,
514+
options: [{skipShapeProps: false}]
515+
},
516+
{
517+
code: `
518+
class Thing extends React.Component {
519+
static propTypes = {
520+
a: PropTypes.shape({
521+
b: PropTypes.string,
522+
}),
523+
}
524+
525+
render() {
526+
const { a } = this.props;
527+
return (
528+
<p>{ a.b }</p>
529+
);
530+
}
531+
}
532+
`,
533+
parser: parsers.BABEL_ESLINT,
534+
options: [{skipShapeProps: false}]
482535
}, {
483536
code: [
484537
'var Hello = createReactClass({',
@@ -4472,27 +4525,6 @@ ruleTester.run('no-unused-prop-types', rule, {
44724525
}, {
44734526
message: '\'prop2.*\' PropType is defined but prop is never used'
44744527
}]
4475-
}, {
4476-
// Destructured assignment with Shape propTypes with skipShapeProps off issue #816
4477-
code: [
4478-
'export default class NavigationButton extends React.Component {',
4479-
' static propTypes = {',
4480-
' route: PropTypes.shape({',
4481-
' getBarTintColor: PropTypes.func.isRequired,',
4482-
' }).isRequired,',
4483-
' };',
4484-
4485-
' renderTitle() {',
4486-
' const { route } = this.props;',
4487-
' return <Title tintColor={route.getBarTintColor()}>TITLE</Title>;',
4488-
' }',
4489-
'}'
4490-
].join('\n'),
4491-
parser: parsers.BABEL_ESLINT,
4492-
options: [{skipShapeProps: false}],
4493-
errors: [{
4494-
message: '\'route.getBarTintColor\' PropType is defined but prop is never used'
4495-
}]
44964528
}, {
44974529
code: [
44984530
// issue #1097

‎tests/lib/rules/prop-types.js

+83-14
Original file line numberDiff line numberDiff line change
@@ -870,17 +870,6 @@ ruleTester.run('prop-types', rule, {
870870
'};'
871871
].join('\n'),
872872
parser: parsers.BABEL_ESLINT
873-
}, {
874-
// Reassigned props are ignored
875-
code: [
876-
'export class Hello extends Component {',
877-
' render() {',
878-
' const props = this.props;',
879-
' return <div>Hello {props.name.firstname} {props[\'name\'].lastname}</div>',
880-
' }',
881-
'}'
882-
].join('\n'),
883-
parser: parsers.BABEL_ESLINT
884873
}, {
885874
code: [
886875
'export default function FooBar(props) {',
@@ -2453,6 +2442,82 @@ ruleTester.run('prop-types', rule, {
24532442
{message: "'foo' is missing in props validation"},
24542443
{message: "'foo.bar' is missing in props validation"}
24552444
]
2445+
},
2446+
{
2447+
code: `
2448+
function Foo({ a }) {
2449+
return <p>{ a.nope }</p>
2450+
}
2451+
2452+
Foo.propTypes = {
2453+
a: PropTypes.shape({
2454+
_: PropType.string,
2455+
})
2456+
}
2457+
`,
2458+
errors: [
2459+
{message: "'a.nope' is missing in props validation"}
2460+
]
2461+
},
2462+
{
2463+
code: `
2464+
function Foo(props) {
2465+
const { a } = props
2466+
return <p>{ a.nope }</p>
2467+
}
2468+
2469+
Foo.propTypes = {
2470+
a: PropTypes.shape({
2471+
_: PropType.string,
2472+
})
2473+
}
2474+
`,
2475+
errors: [
2476+
{message: "'a.nope' is missing in props validation"}
2477+
]
2478+
},
2479+
{
2480+
code: `
2481+
function Foo(props) {
2482+
const a = props.a
2483+
return <p>{ a.nope }</p>
2484+
}
2485+
2486+
Foo.propTypes = {
2487+
a: PropTypes.shape({
2488+
_: PropType.string,
2489+
})
2490+
}
2491+
`,
2492+
errors: [
2493+
{message: "'a.nope' is missing in props validation"}
2494+
]
2495+
},
2496+
{
2497+
code: `
2498+
class Foo extends Component {
2499+
render() {
2500+
const props = this.props
2501+
return <div>{props.cat}</div>
2502+
}
2503+
}
2504+
`,
2505+
errors: [
2506+
{message: "'cat' is missing in props validation"}
2507+
]
2508+
},
2509+
{
2510+
code: `
2511+
class Foo extends Component {
2512+
render() {
2513+
const {props} = this
2514+
return <div>{props.cat}</div>
2515+
}
2516+
}
2517+
`,
2518+
errors: [
2519+
{message: "'cat' is missing in props validation"}
2520+
]
24562521
}, {
24572522
code: [
24582523
'class Hello extends React.Component {',
@@ -3364,6 +3429,8 @@ ruleTester.run('prop-types', rule, {
33643429
].join('\n'),
33653430
errors: [{
33663431
message: '\'names\' is missing in props validation'
3432+
}, {
3433+
message: '\'names.map\' is missing in props validation'
33673434
}]
33683435
}, {
33693436
code: [
@@ -4521,9 +4588,11 @@ ruleTester.run('prop-types', rule, {
45214588
}
45224589
`,
45234590
parser: parsers.BABEL_ESLINT,
4524-
errors: [{
4525-
message: '\'a\' is missing in props validation'
4526-
}]
4591+
errors: [
4592+
{message: '\'a\' is missing in props validation'},
4593+
{message: '\'a.b\' is missing in props validation'},
4594+
{message: '\'a.b.c\' is missing in props validation'}
4595+
]
45274596
},
45284597
{
45294598
code: `

0 commit comments

Comments
 (0)