Skip to content

Commit 0ab4b82

Browse files
jomastijomasti
committed
Support getters in forbid-prop-types
1 parent da82f2b commit 0ab4b82

File tree

4 files changed

+213
-21
lines changed

4 files changed

+213
-21
lines changed

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
const variableUtil = require('../util/variable');
77
const propsUtil = require('../util/props');
8+
const astUtil = require('../util/ast');
89

910
// ------------------------------------------------------------------------------
1011
// Constants
@@ -175,6 +176,22 @@ module.exports = {
175176
checkNode(node.parent.right);
176177
},
177178

179+
MethodDefinition: function(node) {
180+
if (
181+
!propsUtil.isPropTypesDeclaration(node) &&
182+
!shouldCheckContextTypes(node) &&
183+
!shouldCheckChildContextTypes(node)
184+
) {
185+
return;
186+
}
187+
188+
const returnStatement = astUtil.findReturnStatement(node);
189+
190+
if (returnStatement && returnStatement.argument) {
191+
checkNode(returnStatement.argument);
192+
}
193+
},
194+
178195
ObjectExpression: function(node) {
179196
node.properties.forEach(property => {
180197
if (!property.key) {

‎lib/util/Components.js

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const util = require('util');
99
const doctrine = require('doctrine');
1010
const variableUtil = require('./variable');
1111
const pragmaUtil = require('./pragma');
12+
const astUtil = require('./ast');
1213

1314
const usedPropTypesAreEquivalent = (propA, propB) => {
1415
if (propA.name === propB.name) {
@@ -356,24 +357,7 @@ function componentRule(rule, context) {
356357
*
357358
* @param {ASTNode} ASTnode The AST node being checked
358359
*/
359-
findReturnStatement: function(node) {
360-
if (
361-
(!node.value || !node.value.body || !node.value.body.body) &&
362-
(!node.body || !node.body.body)
363-
) {
364-
return false;
365-
}
366-
367-
const bodyNodes = (node.value ? node.value.body.body : node.body.body);
368-
369-
let i = bodyNodes.length - 1;
370-
for (; i >= 0; i--) {
371-
if (bodyNodes[i].type === 'ReturnStatement') {
372-
return bodyNodes[i];
373-
}
374-
}
375-
return false;
376-
},
360+
findReturnStatement: astUtil.findReturnStatement,
377361

378362
/**
379363
* Get the parent component node from the current scope

‎lib/util/ast.js

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,32 @@ function getComponentProperties(node) {
3535
}
3636
}
3737

38+
/**
39+
* Find a return statment in the current node
40+
*
41+
* @param {ASTNode} ASTnode The AST node being checked
42+
*/
43+
function findReturnStatement(node) {
44+
if (
45+
(!node.value || !node.value.body || !node.value.body.body) &&
46+
(!node.body || !node.body.body)
47+
) {
48+
return false;
49+
}
50+
51+
const bodyNodes = (node.value ? node.value.body.body : node.body.body);
52+
53+
let i = bodyNodes.length - 1;
54+
for (; i >= 0; i--) {
55+
if (bodyNodes[i].type === 'ReturnStatement') {
56+
return bodyNodes[i];
57+
}
58+
}
59+
return false;
60+
}
61+
3862
module.exports = {
3963
getPropertyName: getPropertyName,
40-
getComponentProperties: getComponentProperties
64+
getComponentProperties: getComponentProperties,
65+
findReturnStatement: findReturnStatement
4166
};

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

Lines changed: 168 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ const parserOptions = {
1919
}
2020
};
2121

22-
require('babel-eslint');
23-
2422
// -----------------------------------------------------------------------------
2523
// Tests
2624
// -----------------------------------------------------------------------------
@@ -190,6 +188,18 @@ ruleTester.run('forbid-prop-types', rule, {
190188
'}'
191189
].join('\n'),
192190
parser: 'babel-eslint'
191+
}, {
192+
// Proptypes declared with a spread property
193+
code: [
194+
'class Test extends react.component {',
195+
' static get propTypes() {',
196+
' return {',
197+
' intl: React.propTypes.number,',
198+
' ...propTypes',
199+
' };',
200+
' };',
201+
'}'
202+
].join('\n')
193203
}, {
194204
code: [
195205
'var First = createReactClass({',
@@ -351,6 +361,19 @@ ruleTester.run('forbid-prop-types', rule, {
351361
].join('\n'),
352362
parser: 'babel-eslint',
353363
options: [{checkContextTypes: true}]
364+
}, {
365+
// Proptypes declared with a spread property
366+
code: [
367+
'class Test extends react.component {',
368+
' static get childContextTypes() {',
369+
' return {',
370+
' intl: React.childContextTypes.number,',
371+
' ...childContextTypes',
372+
' };',
373+
' };',
374+
'}'
375+
].join('\n'),
376+
options: [{checkContextTypes: true}]
354377
}, {
355378
code: [
356379
'var First = createReactClass({',
@@ -512,6 +535,19 @@ ruleTester.run('forbid-prop-types', rule, {
512535
].join('\n'),
513536
parser: 'babel-eslint',
514537
options: [{checkChildContextTypes: true}]
538+
}, {
539+
// Proptypes declared with a spread property
540+
code: [
541+
'class Test extends react.component {',
542+
' static get childContextTypes() {',
543+
' return {',
544+
' intl: React.childContextTypes.number,',
545+
' ...childContextTypes',
546+
' };',
547+
' };',
548+
'}'
549+
].join('\n'),
550+
options: [{checkChildContextTypes: true}]
515551
}],
516552

517553
invalid: [{
@@ -753,6 +789,21 @@ ruleTester.run('forbid-prop-types', rule, {
753789
].join('\n'),
754790
parser: 'babel-eslint',
755791
errors: 2
792+
}, {
793+
code: [
794+
'class Component extends React.Component {',
795+
' static get propTypes() {',
796+
' return {',
797+
' a: PropTypes.array,',
798+
' o: PropTypes.object',
799+
' };',
800+
' };',
801+
' render() {',
802+
' return <div />;',
803+
' }',
804+
'}'
805+
].join('\n'),
806+
errors: 2
756807
}, {
757808
code: [
758809
'class Component extends React.Component {',
@@ -770,6 +821,24 @@ ruleTester.run('forbid-prop-types', rule, {
770821
settings: {
771822
propWrapperFunctions: ['forbidExtraProps']
772823
}
824+
}, {
825+
code: [
826+
'class Component extends React.Component {',
827+
' static get propTypes() {',
828+
' return forbidExtraProps({',
829+
' a: PropTypes.array,',
830+
' o: PropTypes.object',
831+
' });',
832+
' }',
833+
' render() {',
834+
' return <div />;',
835+
' }',
836+
'}'
837+
].join('\n'),
838+
errors: 2,
839+
settings: {
840+
propWrapperFunctions: ['forbidExtraProps']
841+
}
773842
}, {
774843
code: [
775844
'var Hello = createReactClass({',
@@ -839,6 +908,26 @@ ruleTester.run('forbid-prop-types', rule, {
839908
column: 5,
840909
type: 'Property'
841910
}]
911+
}, {
912+
code: [
913+
'class Foo extends Component {',
914+
' static get contextTypes() {',
915+
' return {',
916+
' a: PropTypes.any',
917+
' };',
918+
' }',
919+
' render() {',
920+
' return <div />;',
921+
' }',
922+
'}'
923+
].join('\n'),
924+
options: [{checkContextTypes: true}],
925+
errors: [{
926+
message: ANY_ERROR_MESSAGE,
927+
line: 4,
928+
column: 7,
929+
type: 'Property'
930+
}]
842931
}, {
843932
code: [
844933
'class Foo extends Component {',
@@ -907,6 +996,25 @@ ruleTester.run('forbid-prop-types', rule, {
907996
settings: {
908997
propWrapperFunctions: ['forbidExtraProps']
909998
}
999+
}, {
1000+
code: [
1001+
'class Component extends React.Component {',
1002+
' static get contextTypes() {',
1003+
' return forbidExtraProps({',
1004+
' a: PropTypes.array,',
1005+
' o: PropTypes.object',
1006+
' });',
1007+
' }',
1008+
' render() {',
1009+
' return <div />;',
1010+
' }',
1011+
'}'
1012+
].join('\n'),
1013+
errors: 2,
1014+
options: [{checkContextTypes: true}],
1015+
settings: {
1016+
propWrapperFunctions: ['forbidExtraProps']
1017+
}
9101018
}, {
9111019
code: [
9121020
'class Component extends React.Component {',
@@ -989,6 +1097,25 @@ ruleTester.run('forbid-prop-types', rule, {
9891097
checkContextTypes: true
9901098
}],
9911099
errors: 1
1100+
}, {
1101+
code: [
1102+
'class Component extends React.Component {',
1103+
' static get contextTypes() {',
1104+
' return {',
1105+
' retailer: PropTypes.instanceOf(Map).isRequired,',
1106+
' requestRetailer: PropTypes.func.isRequired',
1107+
' };',
1108+
' }',
1109+
' render() {',
1110+
' return <div />;',
1111+
' }',
1112+
'}'
1113+
].join('\n'),
1114+
options: [{
1115+
forbid: ['instanceOf'],
1116+
checkContextTypes: true
1117+
}],
1118+
errors: 1
9921119
}, {
9931120
code: [
9941121
'class Component extends React.Component {',
@@ -1073,6 +1200,26 @@ ruleTester.run('forbid-prop-types', rule, {
10731200
column: 5,
10741201
type: 'Property'
10751202
}]
1203+
}, {
1204+
code: [
1205+
'class Foo extends Component {',
1206+
' static get childContextTypes() {',
1207+
' return {',
1208+
' a: PropTypes.any',
1209+
' };',
1210+
' }',
1211+
' render() {',
1212+
' return <div />;',
1213+
' }',
1214+
'}'
1215+
].join('\n'),
1216+
options: [{checkChildContextTypes: true}],
1217+
errors: [{
1218+
message: ANY_ERROR_MESSAGE,
1219+
line: 4,
1220+
column: 7,
1221+
type: 'Property'
1222+
}]
10761223
}, {
10771224
code: [
10781225
'class Foo extends Component {',
@@ -1141,6 +1288,25 @@ ruleTester.run('forbid-prop-types', rule, {
11411288
settings: {
11421289
propWrapperFunctions: ['forbidExtraProps']
11431290
}
1291+
}, {
1292+
code: [
1293+
'class Component extends React.Component {',
1294+
' static get childContextTypes() {',
1295+
' return forbidExtraProps({',
1296+
' a: PropTypes.array,',
1297+
' o: PropTypes.object',
1298+
' });',
1299+
' }',
1300+
' render() {',
1301+
' return <div />;',
1302+
' }',
1303+
'}'
1304+
].join('\n'),
1305+
errors: 2,
1306+
options: [{checkChildContextTypes: true}],
1307+
settings: {
1308+
propWrapperFunctions: ['forbidExtraProps']
1309+
}
11441310
}, {
11451311
code: [
11461312
'class Component extends React.Component {',

0 commit comments

Comments
 (0)