Skip to content

Commit 69e5201

Browse files
author
Dustin Masters
committed
Fix handling of identifiers in forbid-prop-types.
This fixes incorrect behavior when propTypes were stored in a variable and then referenced later. #1352
1 parent dae6574 commit 69e5201

File tree

2 files changed

+57
-36
lines changed

2 files changed

+57
-36
lines changed

lib/rules/forbid-prop-types.js

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
*/
44
'use strict';
55

6+
const variableUtil = require('../util/variable');
7+
68
// ------------------------------------------------------------------------------
79
// Constants
810
// ------------------------------------------------------------------------------
@@ -67,14 +69,33 @@ module.exports = {
6769
);
6870
}
6971

72+
/**
73+
* Find a variable by name in the current scope.
74+
* @param {string} name Name of the variable to look for.
75+
* @returns {ASTNode|null} Return null if the variable could not be found, ASTNode otherwise.
76+
*/
77+
function findVariableByName(name) {
78+
const variable = variableUtil.variablesInScope(context).find(item => item.name === name);
79+
80+
if (!variable || !variable.defs[0] || !variable.defs[0].node) {
81+
return null;
82+
}
83+
84+
if (variable.defs[0].node.type === 'TypeAlias') {
85+
return variable.defs[0].node.right;
86+
}
87+
88+
return variable.defs[0].node.init;
89+
}
90+
7091

7192
/**
7293
* Checks if propTypes declarations are forbidden
7394
* @param {Array} declarations The array of AST nodes being checked.
7495
* @returns {void}
7596
*/
76-
function checkForbidden(declarations) {
77-
(declarations || []).forEach(declaration => {
97+
function checkProperties(declarations) {
98+
declarations.forEach(declaration => {
7899
if (declaration.type !== 'Property') {
79100
return;
80101
}
@@ -108,49 +129,42 @@ module.exports = {
108129
});
109130
}
110131

132+
function checkNode(node) {
133+
switch (node && node.type) {
134+
case 'ObjectExpression':
135+
checkProperties(node.properties);
136+
break;
137+
case 'Identifier':
138+
const propTypesObject = findVariableByName(node.name);
139+
if (propTypesObject && propTypesObject.properties) {
140+
checkProperties(propTypesObject.properties);
141+
}
142+
break;
143+
case 'CallExpression':
144+
const innerNode = node.arguments && node.arguments[0];
145+
if (propWrapperFunctions.has(node.callee.name) && innerNode) {
146+
checkNode(innerNode);
147+
}
148+
break;
149+
default:
150+
break;
151+
}
152+
}
153+
111154
return {
112155
ClassProperty: function(node) {
113156
if (!isPropTypesDeclaration(node)) {
114157
return;
115158
}
116-
switch (node.value && node.value.type) {
117-
case 'ObjectExpression':
118-
checkForbidden(node.value.properties);
119-
break;
120-
case 'CallExpression':
121-
if (
122-
propWrapperFunctions.has(node.value.callee.name) &&
123-
node.value.arguments && node.value.arguments[0]
124-
) {
125-
checkForbidden(node.value.arguments[0].properties);
126-
}
127-
break;
128-
default:
129-
break;
130-
}
159+
checkNode(node.value);
131160
},
132161

133162
MemberExpression: function(node) {
134163
if (!isPropTypesDeclaration(node.property)) {
135164
return;
136165
}
137166

138-
const right = node.parent.right;
139-
switch (right && right.type) {
140-
case 'ObjectExpression':
141-
checkForbidden(right.properties);
142-
break;
143-
case 'CallExpression':
144-
if (
145-
propWrapperFunctions.has(right.callee.name) &&
146-
right.arguments && right.arguments[0]
147-
) {
148-
checkForbidden(right.arguments[0].properties);
149-
}
150-
break;
151-
default:
152-
break;
153-
}
167+
checkNode(node.parent.right);
154168
},
155169

156170
ObjectExpression: function(node) {
@@ -163,7 +177,7 @@ module.exports = {
163177
return;
164178
}
165179
if (property.value.type === 'ObjectExpression') {
166-
checkForbidden(property.value.properties);
180+
checkProperties(property.value.properties);
167181
}
168182
});
169183
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,15 +398,22 @@ ruleTester.run('forbid-prop-types', rule, {
398398
settings: {
399399
propWrapperFunctions: ['forbidExtraProps']
400400
}
401+
}, {
402+
code: [
403+
'import { forbidExtraProps } from "airbnb-prop-types";',
404+
'export const propTypes = {dpm: PropTypes.any};',
405+
'export default function Component() {}',
406+
'Component.propTypes = propTypes;'
407+
].join('\n'),
408+
errors: [{message: ANY_ERROR_MESSAGE}]
401409
}, {
402410
code: [
403411
'import { forbidExtraProps } from "airbnb-prop-types";',
404412
'export const propTypes = {a: PropTypes.any};',
405413
'export default function Component() {}',
406414
'Component.propTypes = forbidExtraProps(propTypes);'
407415
].join('\n'),
408-
// errors: [{message: ANY_ERROR_MESSAGE}], // TODO: make this pass
409-
errors: [],
416+
errors: [{message: ANY_ERROR_MESSAGE}],
410417
settings: {
411418
propWrapperFunctions: ['forbidExtraProps']
412419
}

0 commit comments

Comments
 (0)