Skip to content

Commit 8365d29

Browse files
ljharbljharb
authored
Merge pull request #1356 from dustinsoftware/issue1352
Fix handling of identifiers
2 parents 60aed2e + 3c76731 commit 8365d29

File tree

8 files changed

+191
-83
lines changed

8 files changed

+191
-83
lines changed

‎lib/rules/default-props-match-prop-types.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ module.exports = {
119119
propWrapperFunctions.has(node.callee.name) &&
120120
node.arguments && node.arguments[0]
121121
) {
122-
return node.arguments[0];
122+
return resolveNodeValue(node.arguments[0]);
123123
}
124124
return node;
125125
}

‎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
}

‎lib/rules/require-default-props.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ module.exports = {
109109
propWrapperFunctions.has(node.callee.name) &&
110110
node.arguments && node.arguments[0]
111111
) {
112-
return node.arguments[0];
112+
return resolveNodeValue(node.arguments[0]);
113113
}
114114

115115
return node;

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

Lines changed: 44 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,25 @@ module.exports = {
7878
return getValueName(node) === 'isRequired';
7979
}
8080

81+
/**
82+
* Find a variable by name in the current scope.
83+
* @param {string} name Name of the variable to look for.
84+
* @returns {ASTNode|null} Return null if the variable could not be found, ASTNode otherwise.
85+
*/
86+
function findVariableByName(name) {
87+
const variable = variableUtil.variablesInScope(context).find(item => item.name === name);
88+
89+
if (!variable || !variable.defs[0] || !variable.defs[0].node) {
90+
return null;
91+
}
92+
93+
if (variable.defs[0].node.type === 'TypeAlias') {
94+
return variable.defs[0].node.right;
95+
}
96+
97+
return variable.defs[0].node.init;
98+
}
99+
81100
/**
82101
* Checks if propTypes declarations are sorted
83102
* @param {Array} declarations The array of AST nodes being checked.
@@ -143,62 +162,42 @@ module.exports = {
143162
}, declarations[0]);
144163
}
145164

165+
function checkNode(node) {
166+
switch (node && node.type) {
167+
case 'ObjectExpression':
168+
checkSorted(node.properties);
169+
break;
170+
case 'Identifier':
171+
const propTypesObject = findVariableByName(node.name);
172+
if (propTypesObject && propTypesObject.properties) {
173+
checkSorted(propTypesObject.properties);
174+
}
175+
break;
176+
case 'CallExpression':
177+
const innerNode = node.arguments && node.arguments[0];
178+
if (propWrapperFunctions.has(node.callee.name) && innerNode) {
179+
checkNode(innerNode);
180+
}
181+
break;
182+
default:
183+
break;
184+
}
185+
}
186+
146187
return {
147188
ClassProperty: function(node) {
148189
if (!isPropTypesDeclaration(node)) {
149190
return;
150191
}
151-
switch (node.value && node.value.type) {
152-
case 'ObjectExpression':
153-
checkSorted(node.value.properties);
154-
break;
155-
case 'CallExpression':
156-
if (
157-
propWrapperFunctions.has(node.value.callee.name) &&
158-
node.value.arguments && node.value.arguments[0]
159-
) {
160-
checkSorted(node.value.arguments[0].properties);
161-
}
162-
break;
163-
default:
164-
break;
165-
}
192+
checkNode(node.value);
166193
},
167194

168195
MemberExpression: function(node) {
169196
if (!isPropTypesDeclaration(node.property)) {
170197
return;
171198
}
172-
const right = node.parent.right;
173-
let declarations;
174-
switch (right && right.type) {
175-
case 'CallExpression':
176-
if (
177-
propWrapperFunctions.has(right.callee.name) &&
178-
right.arguments && right.arguments[0]
179-
) {
180-
declarations = right.arguments[0].properties;
181-
}
182-
break;
183-
case 'ObjectExpression':
184-
declarations = right.properties;
185-
break;
186-
case 'Identifier':
187-
const variable = variableUtil.variablesInScope(context).find(item => item.name === right.name);
188-
if (
189-
!variable || !variable.defs[0] ||
190-
!variable.defs[0].node.init || !variable.defs[0].node.init.properties
191-
) {
192-
break;
193-
}
194-
declarations = variable.defs[0].node.init.properties;
195-
break;
196-
default:
197-
break;
198-
}
199-
if (declarations) {
200-
checkSorted(declarations);
201-
}
199+
200+
checkNode(node.parent.right);
202201
},
203202

204203
ObjectExpression: function(node) {

‎tests/lib/rules/default-props-match-prop-types.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,29 @@ ruleTester.run('default-props-match-prop-types', rule, {
783783
column: 3
784784
}]
785785
},
786+
{
787+
code: [
788+
'function MyStatelessComponent({ foo, bar }) {',
789+
' return <div>{foo}{bar}</div>;',
790+
'}',
791+
'const propTypes = {',
792+
' foo: React.PropTypes.string,',
793+
' bar: React.PropTypes.string.isRequired',
794+
'};',
795+
'MyStatelessComponent.propTypes = forbidExtraProps(propTypes);',
796+
'MyStatelessComponent.defaultProps = {',
797+
' baz: "baz"',
798+
'};'
799+
].join('\n'),
800+
settings: {
801+
propWrapperFunctions: ['forbidExtraProps']
802+
},
803+
errors: [{
804+
message: 'defaultProp "baz" has no corresponding propTypes declaration.',
805+
line: 10,
806+
column: 3
807+
}]
808+
},
786809
{
787810
code: [
788811
'function MyStatelessComponent({ foo, bar }) {',

‎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
}

‎tests/lib/rules/require-default-props.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,26 @@ ruleTester.run('require-default-props', rule, {
776776
propWrapperFunctions: ['forbidExtraProps']
777777
}
778778
},
779+
{
780+
code: [
781+
'function MyStatelessComponent({ foo, bar }) {',
782+
' return <div>{foo}{bar}</div>;',
783+
'}',
784+
'const propTypes = {',
785+
' foo: PropTypes.string,',
786+
' bar: PropTypes.string.isRequired',
787+
'};',
788+
'MyStatelessComponent.propTypes = forbidExtraProps(propTypes);'
789+
].join('\n'),
790+
errors: [{
791+
message: 'propType "foo" is not required, but has no corresponding defaultProp declaration.',
792+
line: 5,
793+
column: 3
794+
}],
795+
settings: {
796+
propWrapperFunctions: ['forbidExtraProps']
797+
}
798+
},
779799
{
780800
code: [
781801
'function MyStatelessComponent({ foo, bar }) {',

0 commit comments

Comments
 (0)