From 9ab07e013e87ce27dd97294ac5980ea16b94fd8b Mon Sep 17 00:00:00 2001 From: Joshua Stiefer Date: Sat, 11 Nov 2017 14:35:48 -0700 Subject: [PATCH 1/3] Add checkContextTypes option to forbid-prop-types --- docs/rules/forbid-prop-types.md | 6 +- lib/rules/forbid-prop-types.js | 20 +- lib/util/props.js | 16 ++ tests/lib/rules/forbid-prop-types.js | 395 +++++++++++++++++++++++++++ 4 files changed, 431 insertions(+), 6 deletions(-) diff --git a/docs/rules/forbid-prop-types.md b/docs/rules/forbid-prop-types.md index 951285ce3b..03ff60b580 100644 --- a/docs/rules/forbid-prop-types.md +++ b/docs/rules/forbid-prop-types.md @@ -44,7 +44,7 @@ class Component extends React.Component { ```js ... -"react/forbid-prop-types": [, { "forbid": [] }] +"react/forbid-prop-types": [, { "forbid": [], checkContextTypes: }] ... ``` @@ -52,6 +52,10 @@ class Component extends React.Component { An array of strings, with the names of `PropTypes` keys that are forbidden. The default value for this option is `['any', 'array', 'object']`. +### `checkContextTypes` + +Whether or not to check `contextTypes` for forbidden prop types. The default value is false. + ## When not to use This rule is a formatting/documenting preference and not following it won't negatively affect the quality of your code. This rule encourages prop types that more specifically document their usage. diff --git a/lib/rules/forbid-prop-types.js b/lib/rules/forbid-prop-types.js index 6d03b21566..91f280cc00 100644 --- a/lib/rules/forbid-prop-types.js +++ b/lib/rules/forbid-prop-types.js @@ -32,6 +32,9 @@ module.exports = { items: { type: 'string' } + }, + checkContextTypes: { + type: 'boolean' } }, additionalProperties: true @@ -40,14 +43,21 @@ module.exports = { create: function(context) { const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); + const configuration = context.options[0] || {}; + const checkContextTypes = configuration.checkContextTypes || false; function isForbidden(type) { - const configuration = context.options[0] || {}; - const forbid = configuration.forbid || DEFAULTS; return forbid.indexOf(type) >= 0; } + function shouldCheckContextTypes(node) { + if (checkContextTypes && propsUtil.isContextTypesDeclaration(node)) { + return true; + } + return false; + } + /** * Find a variable by name in the current scope. * @param {string} name Name of the variable to look for. @@ -132,14 +142,14 @@ module.exports = { return { ClassProperty: function(node) { - if (!propsUtil.isPropTypesDeclaration(node)) { + if (!propsUtil.isPropTypesDeclaration(node) && !shouldCheckContextTypes(node)) { return; } checkNode(node.value); }, MemberExpression: function(node) { - if (!propsUtil.isPropTypesDeclaration(node)) { + if (!propsUtil.isPropTypesDeclaration(node) && !shouldCheckContextTypes(node)) { return; } @@ -152,7 +162,7 @@ module.exports = { return; } - if (!propsUtil.isPropTypesDeclaration(property)) { + if (!propsUtil.isPropTypesDeclaration(property) && !shouldCheckContextTypes(property)) { return; } if (property.value.type === 'ObjectExpression') { diff --git a/lib/util/props.js b/lib/util/props.js index f2433978bf..42ade33ce0 100644 --- a/lib/util/props.js +++ b/lib/util/props.js @@ -20,6 +20,21 @@ function isPropTypesDeclaration(node) { return astUtil.getPropertyName(node) === 'propTypes'; } +/** + * Checks if the node passed in looks like a contextTypes declaration. + * @param {ASTNode} node The node to check. + * @returns {Boolean} `true` if the node is a contextTypes declaration, `false` if not + */ +function isContextTypesDeclaration(node) { + if (node && node.type === 'ClassProperty') { + // Flow support + if (node.typeAnnotation && node.key.name === 'context') { + return true; + } + } + return astUtil.getPropertyName(node) === 'contextTypes'; +} + /** * Checks if the Identifier node passed in looks like a defaultProps declaration. * @param {ASTNode} node The node to check. Must be an Identifier node. @@ -41,6 +56,7 @@ function isRequiredPropType(propTypeExpression) { module.exports = { isPropTypesDeclaration: isPropTypesDeclaration, + isContextTypesDeclaration: isContextTypesDeclaration, isDefaultPropsDeclaration: isDefaultPropsDeclaration, isRequiredPropType: isRequiredPropType }; diff --git a/tests/lib/rules/forbid-prop-types.js b/tests/lib/rules/forbid-prop-types.js index e6c7eb3b8a..6d73ed76a2 100644 --- a/tests/lib/rules/forbid-prop-types.js +++ b/tests/lib/rules/forbid-prop-types.js @@ -190,6 +190,167 @@ ruleTester.run('forbid-prop-types', rule, { '}' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'var First = createReactClass({', + ' contextTypes: externalPropTypes,', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkContextTypes: true}] + }, { + code: [ + 'var First = createReactClass({', + ' contextTypes: {', + ' s: PropTypes.string,', + ' n: PropTypes.number,', + ' i: PropTypes.instanceOf,', + ' b: PropTypes.bool', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkContextTypes: true}] + }, { + code: [ + 'var First = createReactClass({', + ' contextTypes: {', + ' a: PropTypes.array', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'object'], + checkContextTypes: true + }] + }, { + code: [ + 'var First = createReactClass({', + ' contextTypes: {', + ' o: PropTypes.object', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'array'], + checkContextTypes: true + }] + }, { + code: [ + 'var First = createReactClass({', + ' contextTypes: {', + ' o: PropTypes.object,', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'array'], + checkContextTypes: true + }] + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.contextTypes = {', + ' a: PropTypes.string,', + ' b: PropTypes.string', + '};', + 'First.contextTypes.justforcheck = PropTypes.string;' + ].join('\n'), + options: [{checkContextTypes: true}] + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.contextTypes = {', + ' elem: PropTypes.instanceOf(HTMLElement)', + '};' + ].join('\n'), + options: [{checkContextTypes: true}] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello
;', + ' }', + '}', + 'Hello.contextTypes = {', + ' "aria-controls": PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint', + options: [{checkContextTypes: true}] + }, { + // Invalid code, should not be validated + code: [ + 'class Component extends React.Component {', + ' contextTypes: {', + ' a: PropTypes.any,', + ' c: PropTypes.any,', + ' b: PropTypes.any', + ' };', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{checkContextTypes: true}] + }, { + code: [ + 'var Hello = createReactClass({', + ' render: function() {', + ' let { a, ...b } = obj;', + ' let c = { ...d };', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkContextTypes: true}] + }, { + code: [ + 'var Hello = createReactClass({', + ' contextTypes: {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkContextTypes: true}] + }, { + // Proptypes declared with a spread property + code: [ + 'class Test extends react.component {', + ' static contextTypes = {', + ' intl: React.contextTypes.number,', + ' ...contextTypes', + ' };', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{checkContextTypes: true}] }], invalid: [{ @@ -480,5 +641,239 @@ ruleTester.run('forbid-prop-types', rule, { forbid: ['object'] }], errors: 1 + }, { + code: [ + 'var First = createReactClass({', + ' contextTypes: {', + ' a: PropTypes.any', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'class Foo extends Component {', + ' static contextTypes = {', + ' a: PropTypes.any', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: [{checkContextTypes: true}], + parser: 'babel-eslint', + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'class Foo extends Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Foo.contextTypes = {', + ' a: PropTypes.any', + '};' + ].join('\n'), + options: [{checkContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 7, + column: 3, + type: 'Property' + }] + }, { + code: [ + 'function Foo(props) {', + ' return
;', + '}', + 'Foo.contextTypes = {', + ' a: PropTypes.any', + '};' + ].join('\n'), + options: [{checkContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 5, + column: 3, + type: 'Property' + }] + }, { + code: [ + 'const Foo = (props) => {', + ' return
;', + '};', + 'Foo.contextTypes = {', + ' a: PropTypes.any', + '};' + ].join('\n'), + options: [{checkContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 5, + column: 3, + type: 'Property' + }] + }, { + code: [ + 'class Component extends React.Component {', + ' static contextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + ' });', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: 2, + options: [{checkContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'class Component extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Component.contextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + '});' + ].join('\n'), + errors: 2, + options: [{checkContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'function Component(props) {', + ' return
;', + '}', + 'Component.contextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + '});' + ].join('\n'), + errors: 2, + options: [{checkContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'const Component = (props) => {', + ' return
;', + '};', + 'Component.contextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + '});' + ].join('\n'), + errors: 2, + options: [{checkContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'var Hello = createReactClass({', + ' contextTypes: {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkContextTypes: true + }], + errors: 1 + }, { + code: [ + 'class Component extends React.Component {', + ' static contextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{ + forbid: ['instanceOf'], + checkContextTypes: true + }], + errors: 1 + }, { + code: [ + 'class Component extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Component.contextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkContextTypes: true + }], + errors: 1 + }, { + code: [ + 'function Component(props) {', + ' return
;', + '}', + 'Component.contextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkContextTypes: true + }], + errors: 1 + }, { + code: [ + 'const Component = (props) => {', + ' return
;', + '};', + 'Component.contextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkContextTypes: true + }], + errors: 1 }] }); From da82f2b6cbc73a26bd23087cba78cba8c89cba18 Mon Sep 17 00:00:00 2001 From: Joshua Stiefer Date: Sat, 11 Nov 2017 15:56:18 -0700 Subject: [PATCH 2/3] Add checkChildContextTypes for forbid-prop-types --- docs/rules/forbid-prop-types.md | 6 +- lib/rules/forbid-prop-types.js | 29 +- lib/util/props.js | 10 + tests/lib/rules/forbid-prop-types.js | 423 ++++++++++++++++++++++++++- 4 files changed, 450 insertions(+), 18 deletions(-) diff --git a/docs/rules/forbid-prop-types.md b/docs/rules/forbid-prop-types.md index 03ff60b580..3784eff356 100644 --- a/docs/rules/forbid-prop-types.md +++ b/docs/rules/forbid-prop-types.md @@ -44,7 +44,7 @@ class Component extends React.Component { ```js ... -"react/forbid-prop-types": [, { "forbid": [], checkContextTypes: }] +"react/forbid-prop-types": [, { "forbid": [], checkContextTypes: , checkChildContextTypes: }] ... ``` @@ -56,6 +56,10 @@ An array of strings, with the names of `PropTypes` keys that are forbidden. The Whether or not to check `contextTypes` for forbidden prop types. The default value is false. +### `checkChildContextTypes` + +Whether or not to check `childContextTypes` for forbidden prop types. The default value is false. + ## When not to use This rule is a formatting/documenting preference and not following it won't negatively affect the quality of your code. This rule encourages prop types that more specifically document their usage. diff --git a/lib/rules/forbid-prop-types.js b/lib/rules/forbid-prop-types.js index 91f280cc00..b99dfc3a59 100644 --- a/lib/rules/forbid-prop-types.js +++ b/lib/rules/forbid-prop-types.js @@ -35,6 +35,9 @@ module.exports = { }, checkContextTypes: { type: 'boolean' + }, + checkChildContextTypes: { + type: 'boolean' } }, additionalProperties: true @@ -45,6 +48,7 @@ module.exports = { const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); const configuration = context.options[0] || {}; const checkContextTypes = configuration.checkContextTypes || false; + const checkChildContextTypes = configuration.checkChildContextTypes || false; function isForbidden(type) { const forbid = configuration.forbid || DEFAULTS; @@ -58,6 +62,13 @@ module.exports = { return false; } + function shouldCheckChildContextTypes(node) { + if (checkChildContextTypes && propsUtil.isChildContextTypesDeclaration(node)) { + return true; + } + return false; + } + /** * Find a variable by name in the current scope. * @param {string} name Name of the variable to look for. @@ -142,14 +153,22 @@ module.exports = { return { ClassProperty: function(node) { - if (!propsUtil.isPropTypesDeclaration(node) && !shouldCheckContextTypes(node)) { + if ( + !propsUtil.isPropTypesDeclaration(node) && + !shouldCheckContextTypes(node) && + !shouldCheckChildContextTypes(node) + ) { return; } checkNode(node.value); }, MemberExpression: function(node) { - if (!propsUtil.isPropTypesDeclaration(node) && !shouldCheckContextTypes(node)) { + if ( + !propsUtil.isPropTypesDeclaration(node) && + !shouldCheckContextTypes(node) && + !shouldCheckChildContextTypes(node) + ) { return; } @@ -162,7 +181,11 @@ module.exports = { return; } - if (!propsUtil.isPropTypesDeclaration(property) && !shouldCheckContextTypes(property)) { + if ( + !propsUtil.isPropTypesDeclaration(property) && + !shouldCheckContextTypes(property) && + !shouldCheckChildContextTypes(property) + ) { return; } if (property.value.type === 'ObjectExpression') { diff --git a/lib/util/props.js b/lib/util/props.js index 42ade33ce0..f90648ccf2 100644 --- a/lib/util/props.js +++ b/lib/util/props.js @@ -35,6 +35,15 @@ function isContextTypesDeclaration(node) { return astUtil.getPropertyName(node) === 'contextTypes'; } +/** + * Checks if the node passed in looks like a childContextTypes declaration. + * @param {ASTNode} node The node to check. + * @returns {Boolean} `true` if the node is a childContextTypes declaration, `false` if not + */ +function isChildContextTypesDeclaration(node) { + return astUtil.getPropertyName(node) === 'childContextTypes'; +} + /** * Checks if the Identifier node passed in looks like a defaultProps declaration. * @param {ASTNode} node The node to check. Must be an Identifier node. @@ -57,6 +66,7 @@ function isRequiredPropType(propTypeExpression) { module.exports = { isPropTypesDeclaration: isPropTypesDeclaration, isContextTypesDeclaration: isContextTypesDeclaration, + isChildContextTypesDeclaration: isChildContextTypesDeclaration, isDefaultPropsDeclaration: isDefaultPropsDeclaration, isRequiredPropType: isRequiredPropType }; diff --git a/tests/lib/rules/forbid-prop-types.js b/tests/lib/rules/forbid-prop-types.js index 6d73ed76a2..30a3ebf340 100644 --- a/tests/lib/rules/forbid-prop-types.js +++ b/tests/lib/rules/forbid-prop-types.js @@ -193,7 +193,7 @@ ruleTester.run('forbid-prop-types', rule, { }, { code: [ 'var First = createReactClass({', - ' contextTypes: externalPropTypes,', + ' childContextTypes: externalPropTypes,', ' render: function() {', ' return
;', ' }', @@ -203,7 +203,7 @@ ruleTester.run('forbid-prop-types', rule, { }, { code: [ 'var First = createReactClass({', - ' contextTypes: {', + ' childContextTypes: {', ' s: PropTypes.string,', ' n: PropTypes.number,', ' i: PropTypes.instanceOf,', @@ -218,7 +218,7 @@ ruleTester.run('forbid-prop-types', rule, { }, { code: [ 'var First = createReactClass({', - ' contextTypes: {', + ' childContextTypes: {', ' a: PropTypes.array', ' },', ' render: function() {', @@ -233,7 +233,7 @@ ruleTester.run('forbid-prop-types', rule, { }, { code: [ 'var First = createReactClass({', - ' contextTypes: {', + ' childContextTypes: {', ' o: PropTypes.object', ' },', ' render: function() {', @@ -248,7 +248,7 @@ ruleTester.run('forbid-prop-types', rule, { }, { code: [ 'var First = createReactClass({', - ' contextTypes: {', + ' childContextTypes: {', ' o: PropTypes.object,', ' },', ' render: function() {', @@ -267,11 +267,11 @@ ruleTester.run('forbid-prop-types', rule, { ' return
;', ' }', '}', - 'First.contextTypes = {', + 'First.childContextTypes = {', ' a: PropTypes.string,', ' b: PropTypes.string', '};', - 'First.contextTypes.justforcheck = PropTypes.string;' + 'First.childContextTypes.justforcheck = PropTypes.string;' ].join('\n'), options: [{checkContextTypes: true}] }, { @@ -281,7 +281,7 @@ ruleTester.run('forbid-prop-types', rule, { ' return
;', ' }', '}', - 'First.contextTypes = {', + 'First.childContextTypes = {', ' elem: PropTypes.instanceOf(HTMLElement)', '};' ].join('\n'), @@ -293,7 +293,7 @@ ruleTester.run('forbid-prop-types', rule, { ' return
Hello
;', ' }', '}', - 'Hello.contextTypes = {', + 'Hello.childContextTypes = {', ' "aria-controls": PropTypes.string', '};' ].join('\n'), @@ -303,7 +303,7 @@ ruleTester.run('forbid-prop-types', rule, { // Invalid code, should not be validated code: [ 'class Component extends React.Component {', - ' contextTypes: {', + ' childContextTypes: {', ' a: PropTypes.any,', ' c: PropTypes.any,', ' b: PropTypes.any', @@ -329,7 +329,7 @@ ruleTester.run('forbid-prop-types', rule, { }, { code: [ 'var Hello = createReactClass({', - ' contextTypes: {', + ' childContextTypes: {', ' retailer: PropTypes.instanceOf(Map).isRequired,', ' requestRetailer: PropTypes.func.isRequired', ' },', @@ -343,14 +343,175 @@ ruleTester.run('forbid-prop-types', rule, { // Proptypes declared with a spread property code: [ 'class Test extends react.component {', - ' static contextTypes = {', - ' intl: React.contextTypes.number,', - ' ...contextTypes', + ' static childContextTypes = {', + ' intl: React.childContextTypes.number,', + ' ...childContextTypes', ' };', '}' ].join('\n'), parser: 'babel-eslint', options: [{checkContextTypes: true}] + }, { + code: [ + 'var First = createReactClass({', + ' childContextTypes: externalPropTypes,', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkChildContextTypes: true}] + }, { + code: [ + 'var First = createReactClass({', + ' childContextTypes: {', + ' s: PropTypes.string,', + ' n: PropTypes.number,', + ' i: PropTypes.instanceOf,', + ' b: PropTypes.bool', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkChildContextTypes: true}] + }, { + code: [ + 'var First = createReactClass({', + ' childContextTypes: {', + ' a: PropTypes.array', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'object'], + checkChildContextTypes: true + }] + }, { + code: [ + 'var First = createReactClass({', + ' childContextTypes: {', + ' o: PropTypes.object', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'array'], + checkChildContextTypes: true + }] + }, { + code: [ + 'var First = createReactClass({', + ' childContextTypes: {', + ' o: PropTypes.object,', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'array'], + checkChildContextTypes: true + }] + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.childContextTypes = {', + ' a: PropTypes.string,', + ' b: PropTypes.string', + '};', + 'First.childContextTypes.justforcheck = PropTypes.string;' + ].join('\n'), + options: [{checkChildContextTypes: true}] + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.childContextTypes = {', + ' elem: PropTypes.instanceOf(HTMLElement)', + '};' + ].join('\n'), + options: [{checkChildContextTypes: true}] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello
;', + ' }', + '}', + 'Hello.childContextTypes = {', + ' "aria-controls": PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint', + options: [{checkChildContextTypes: true}] + }, { + // Invalid code, should not be validated + code: [ + 'class Component extends React.Component {', + ' childContextTypes: {', + ' a: PropTypes.any,', + ' c: PropTypes.any,', + ' b: PropTypes.any', + ' };', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{checkChildContextTypes: true}] + }, { + code: [ + 'var Hello = createReactClass({', + ' render: function() {', + ' let { a, ...b } = obj;', + ' let c = { ...d };', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkChildContextTypes: true}] + }, { + code: [ + 'var Hello = createReactClass({', + ' childContextTypes: {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkChildContextTypes: true}] + }, { + // Proptypes declared with a spread property + code: [ + 'class Test extends react.component {', + ' static childContextTypes = {', + ' intl: React.childContextTypes.number,', + ' ...childContextTypes', + ' };', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{checkChildContextTypes: true}] }], invalid: [{ @@ -875,5 +1036,239 @@ ruleTester.run('forbid-prop-types', rule, { checkContextTypes: true }], errors: 1 + }, { + code: [ + 'var First = createReactClass({', + ' childContextTypes: {', + ' a: PropTypes.any', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{checkChildContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'class Foo extends Component {', + ' static childContextTypes = {', + ' a: PropTypes.any', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: [{checkChildContextTypes: true}], + parser: 'babel-eslint', + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'class Foo extends Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Foo.childContextTypes = {', + ' a: PropTypes.any', + '};' + ].join('\n'), + options: [{checkChildContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 7, + column: 3, + type: 'Property' + }] + }, { + code: [ + 'function Foo(props) {', + ' return
;', + '}', + 'Foo.childContextTypes = {', + ' a: PropTypes.any', + '};' + ].join('\n'), + options: [{checkChildContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 5, + column: 3, + type: 'Property' + }] + }, { + code: [ + 'const Foo = (props) => {', + ' return
;', + '};', + 'Foo.childContextTypes = {', + ' a: PropTypes.any', + '};' + ].join('\n'), + options: [{checkChildContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 5, + column: 3, + type: 'Property' + }] + }, { + code: [ + 'class Component extends React.Component {', + ' static childContextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + ' });', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: 2, + options: [{checkChildContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'class Component extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Component.childContextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + '});' + ].join('\n'), + errors: 2, + options: [{checkChildContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'function Component(props) {', + ' return
;', + '}', + 'Component.childContextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + '});' + ].join('\n'), + errors: 2, + options: [{checkChildContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'const Component = (props) => {', + ' return
;', + '};', + 'Component.childContextTypes = forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + '});' + ].join('\n'), + errors: 2, + options: [{checkChildContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } + }, { + code: [ + 'var Hello = createReactClass({', + ' childContextTypes: {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkChildContextTypes: true + }], + errors: 1 + }, { + code: [ + 'class Component extends React.Component {', + ' static childContextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{ + forbid: ['instanceOf'], + checkChildContextTypes: true + }], + errors: 1 + }, { + code: [ + 'class Component extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Component.childContextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkChildContextTypes: true + }], + errors: 1 + }, { + code: [ + 'function Component(props) {', + ' return
;', + '}', + 'Component.childContextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkChildContextTypes: true + }], + errors: 1 + }, { + code: [ + 'const Component = (props) => {', + ' return
;', + '};', + 'Component.childContextTypes = {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkChildContextTypes: true + }], + errors: 1 }] }); From 0ab4b82c280a9e9d8374a13846b25ae2e3a210ba Mon Sep 17 00:00:00 2001 From: Joshua Stiefer Date: Sat, 11 Nov 2017 16:33:44 -0700 Subject: [PATCH 3/3] Support getters in forbid-prop-types --- lib/rules/forbid-prop-types.js | 17 +++ lib/util/Components.js | 20 +--- lib/util/ast.js | 27 ++++- tests/lib/rules/forbid-prop-types.js | 170 ++++++++++++++++++++++++++- 4 files changed, 213 insertions(+), 21 deletions(-) diff --git a/lib/rules/forbid-prop-types.js b/lib/rules/forbid-prop-types.js index b99dfc3a59..c2ec47c737 100644 --- a/lib/rules/forbid-prop-types.js +++ b/lib/rules/forbid-prop-types.js @@ -5,6 +5,7 @@ const variableUtil = require('../util/variable'); const propsUtil = require('../util/props'); +const astUtil = require('../util/ast'); // ------------------------------------------------------------------------------ // Constants @@ -175,6 +176,22 @@ module.exports = { checkNode(node.parent.right); }, + MethodDefinition: function(node) { + if ( + !propsUtil.isPropTypesDeclaration(node) && + !shouldCheckContextTypes(node) && + !shouldCheckChildContextTypes(node) + ) { + return; + } + + const returnStatement = astUtil.findReturnStatement(node); + + if (returnStatement && returnStatement.argument) { + checkNode(returnStatement.argument); + } + }, + ObjectExpression: function(node) { node.properties.forEach(property => { if (!property.key) { diff --git a/lib/util/Components.js b/lib/util/Components.js index b25bcce3f6..5e78e664fb 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -9,6 +9,7 @@ const util = require('util'); const doctrine = require('doctrine'); const variableUtil = require('./variable'); const pragmaUtil = require('./pragma'); +const astUtil = require('./ast'); const usedPropTypesAreEquivalent = (propA, propB) => { if (propA.name === propB.name) { @@ -356,24 +357,7 @@ function componentRule(rule, context) { * * @param {ASTNode} ASTnode The AST node being checked */ - findReturnStatement: function(node) { - if ( - (!node.value || !node.value.body || !node.value.body.body) && - (!node.body || !node.body.body) - ) { - return false; - } - - const bodyNodes = (node.value ? node.value.body.body : node.body.body); - - let i = bodyNodes.length - 1; - for (; i >= 0; i--) { - if (bodyNodes[i].type === 'ReturnStatement') { - return bodyNodes[i]; - } - } - return false; - }, + findReturnStatement: astUtil.findReturnStatement, /** * Get the parent component node from the current scope diff --git a/lib/util/ast.js b/lib/util/ast.js index 2ab5cf4ba2..dc9ce7704f 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -35,7 +35,32 @@ function getComponentProperties(node) { } } +/** + * Find a return statment in the current node + * + * @param {ASTNode} ASTnode The AST node being checked + */ +function findReturnStatement(node) { + if ( + (!node.value || !node.value.body || !node.value.body.body) && + (!node.body || !node.body.body) + ) { + return false; + } + + const bodyNodes = (node.value ? node.value.body.body : node.body.body); + + let i = bodyNodes.length - 1; + for (; i >= 0; i--) { + if (bodyNodes[i].type === 'ReturnStatement') { + return bodyNodes[i]; + } + } + return false; +} + module.exports = { getPropertyName: getPropertyName, - getComponentProperties: getComponentProperties + getComponentProperties: getComponentProperties, + findReturnStatement: findReturnStatement }; diff --git a/tests/lib/rules/forbid-prop-types.js b/tests/lib/rules/forbid-prop-types.js index 30a3ebf340..f79703d3de 100644 --- a/tests/lib/rules/forbid-prop-types.js +++ b/tests/lib/rules/forbid-prop-types.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- @@ -190,6 +188,18 @@ ruleTester.run('forbid-prop-types', rule, { '}' ].join('\n'), parser: 'babel-eslint' + }, { + // Proptypes declared with a spread property + code: [ + 'class Test extends react.component {', + ' static get propTypes() {', + ' return {', + ' intl: React.propTypes.number,', + ' ...propTypes', + ' };', + ' };', + '}' + ].join('\n') }, { code: [ 'var First = createReactClass({', @@ -351,6 +361,19 @@ ruleTester.run('forbid-prop-types', rule, { ].join('\n'), parser: 'babel-eslint', options: [{checkContextTypes: true}] + }, { + // Proptypes declared with a spread property + code: [ + 'class Test extends react.component {', + ' static get childContextTypes() {', + ' return {', + ' intl: React.childContextTypes.number,', + ' ...childContextTypes', + ' };', + ' };', + '}' + ].join('\n'), + options: [{checkContextTypes: true}] }, { code: [ 'var First = createReactClass({', @@ -512,6 +535,19 @@ ruleTester.run('forbid-prop-types', rule, { ].join('\n'), parser: 'babel-eslint', options: [{checkChildContextTypes: true}] + }, { + // Proptypes declared with a spread property + code: [ + 'class Test extends react.component {', + ' static get childContextTypes() {', + ' return {', + ' intl: React.childContextTypes.number,', + ' ...childContextTypes', + ' };', + ' };', + '}' + ].join('\n'), + options: [{checkChildContextTypes: true}] }], invalid: [{ @@ -753,6 +789,21 @@ ruleTester.run('forbid-prop-types', rule, { ].join('\n'), parser: 'babel-eslint', errors: 2 + }, { + code: [ + 'class Component extends React.Component {', + ' static get propTypes() {', + ' return {', + ' a: PropTypes.array,', + ' o: PropTypes.object', + ' };', + ' };', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + errors: 2 }, { code: [ 'class Component extends React.Component {', @@ -770,6 +821,24 @@ ruleTester.run('forbid-prop-types', rule, { settings: { propWrapperFunctions: ['forbidExtraProps'] } + }, { + code: [ + 'class Component extends React.Component {', + ' static get propTypes() {', + ' return forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + ' });', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + errors: 2, + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } }, { code: [ 'var Hello = createReactClass({', @@ -839,6 +908,26 @@ ruleTester.run('forbid-prop-types', rule, { column: 5, type: 'Property' }] + }, { + code: [ + 'class Foo extends Component {', + ' static get contextTypes() {', + ' return {', + ' a: PropTypes.any', + ' };', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: [{checkContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 4, + column: 7, + type: 'Property' + }] }, { code: [ 'class Foo extends Component {', @@ -907,6 +996,25 @@ ruleTester.run('forbid-prop-types', rule, { settings: { propWrapperFunctions: ['forbidExtraProps'] } + }, { + code: [ + 'class Component extends React.Component {', + ' static get contextTypes() {', + ' return forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + ' });', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + errors: 2, + options: [{checkContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } }, { code: [ 'class Component extends React.Component {', @@ -989,6 +1097,25 @@ ruleTester.run('forbid-prop-types', rule, { checkContextTypes: true }], errors: 1 + }, { + code: [ + 'class Component extends React.Component {', + ' static get contextTypes() {', + ' return {', + ' retailer: PropTypes.instanceOf(Map).isRequired,', + ' requestRetailer: PropTypes.func.isRequired', + ' };', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: [{ + forbid: ['instanceOf'], + checkContextTypes: true + }], + errors: 1 }, { code: [ 'class Component extends React.Component {', @@ -1073,6 +1200,26 @@ ruleTester.run('forbid-prop-types', rule, { column: 5, type: 'Property' }] + }, { + code: [ + 'class Foo extends Component {', + ' static get childContextTypes() {', + ' return {', + ' a: PropTypes.any', + ' };', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: [{checkChildContextTypes: true}], + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 4, + column: 7, + type: 'Property' + }] }, { code: [ 'class Foo extends Component {', @@ -1141,6 +1288,25 @@ ruleTester.run('forbid-prop-types', rule, { settings: { propWrapperFunctions: ['forbidExtraProps'] } + }, { + code: [ + 'class Component extends React.Component {', + ' static get childContextTypes() {', + ' return forbidExtraProps({', + ' a: PropTypes.array,', + ' o: PropTypes.object', + ' });', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + errors: 2, + options: [{checkChildContextTypes: true}], + settings: { + propWrapperFunctions: ['forbidExtraProps'] + } }, { code: [ 'class Component extends React.Component {',