From 0eb53a39a6841e53ba1ce7b87b8004e7da64eaa6 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Wed, 23 Sep 2015 09:27:16 -0700 Subject: [PATCH] add forbid-prop-types rule that by default forbids any, array, and object --- docs/rules/forbid-prop-types.md | 57 ++++ index.js | 6 +- lib/rules/forbid-prop-types.js | 116 ++++++++ tests/lib/rules/forbid-prop-types.js | 425 +++++++++++++++++++++++++++ 4 files changed, 602 insertions(+), 2 deletions(-) create mode 100644 docs/rules/forbid-prop-types.md create mode 100644 lib/rules/forbid-prop-types.js create mode 100644 tests/lib/rules/forbid-prop-types.js diff --git a/docs/rules/forbid-prop-types.md b/docs/rules/forbid-prop-types.md new file mode 100644 index 0000000000..efac6526a0 --- /dev/null +++ b/docs/rules/forbid-prop-types.md @@ -0,0 +1,57 @@ +# Forbid certain propTypes (forbid-prop-types) + +By default this rule prevents vague prop types with more specific alternatives available (`any`, `array`, `object`), but any prop type can be disabled if desired. The defaults are chosen because they have obvious replacements. `any` should be replaced with, well, anything. `array` and `object` can be replaced with `arrayOf` and `shape`, respectively. + +## Rule Details + +This rule checks all JSX components and verifies that no forbidden propsTypes are used. +This rule is off by default. + +The following patterns are considered warnings: + +```js +var Component = React.createClass({ + propTypes: { + a: React.PropTypes.any, + r: React.PropTypes.array, + o: React.PropTypes.object + }, +... +}); + +class Component extends React.Component { + ... +} +Component.propTypes = { + a: React.PropTypes.any, + r: React.PropTypes.array, + o: React.PropTypes.object +}; + +class Component extends React.Component { + static propTypes = { + a: React.PropTypes.any, + r: React.PropTypes.array, + o: React.PropTypes.object + } + render() { + return
; + } +} +``` + +## Rule Options + +```js +... +"forbid-prop-types": [, { "forbid": [] }] +... +``` + +### `forbid` + +An array of strings, with the names of React.PropType keys that are forbidden. + +## 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/index.js b/index.js index c654d5988f..192dd60f25 100644 --- a/index.js +++ b/index.js @@ -28,7 +28,8 @@ module.exports = { 'jsx-no-literals': require('./lib/rules/jsx-no-literals'), 'jsx-indent-props': require('./lib/rules/jsx-indent-props'), 'jsx-closing-bracket-location': require('./lib/rules/jsx-closing-bracket-location'), - 'no-direct-mutation-state': require('./lib/rules/no-direct-mutation-state') + 'no-direct-mutation-state': require('./lib/rules/no-direct-mutation-state'), + 'forbid-prop-types': require('./lib/rules/forbid-prop-types') }, rulesConfig: { 'jsx-uses-react': 0, @@ -57,6 +58,7 @@ module.exports = { 'jsx-no-literals': 0, 'jsx-indent-props': 0, 'jsx-closing-bracket-location': 0, - 'no-direct-mutation-state': 0 + 'no-direct-mutation-state': 0, + 'forbid-prop-types': 0 } }; diff --git a/lib/rules/forbid-prop-types.js b/lib/rules/forbid-prop-types.js new file mode 100644 index 0000000000..e655d55839 --- /dev/null +++ b/lib/rules/forbid-prop-types.js @@ -0,0 +1,116 @@ +/** + * @fileoverview Forbid certain propTypes + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Constants +// ------------------------------------------------------------------------------ + +var DEFAULTS = ['any', 'array', 'object']; + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = function(context) { + + function isForbidden(type) { + var configuration = context.options[0] || {}; + + var forbid = configuration.forbid || DEFAULTS; + return forbid.indexOf(type) >= 0; + } + + /** + * Checks if node is `propTypes` declaration + * @param {ASTNode} node The AST node being checked. + * @returns {Boolean} True if node is `propTypes` declaration, false if not. + */ + function isPropTypesDeclaration(node) { + + // Special case for class properties + // (babel-eslint does not expose property name so we have to rely on tokens) + if (node.type === 'ClassProperty') { + var tokens = context.getFirstTokens(node, 2); + if (tokens[0].value === 'propTypes' || tokens[1].value === 'propTypes') { + return true; + } + return false; + } + + return Boolean( + node && + node.name === 'propTypes' + ); + } + + + /** + * Checks if propTypes declarations are forbidden + * @param {Array} declarations The array of AST nodes being checked. + * @returns {void} + */ + function checkForbidden(declarations) { + declarations.forEach(function(declaration) { + if ( + declaration.value.type === 'MemberExpression' && + declaration.value.property && + declaration.value.property.name && + declaration.value.property.name === 'isRequired' + ) { + declaration.value = declaration.value.object; + } + + if (isForbidden(declaration.value.property.name)) { + context.report(declaration, 'Prop type `' + declaration.value.property.name + '` is forbidden'); + } + }); + } + + return { + ClassProperty: function(node) { + if (isPropTypesDeclaration(node) && node.value && node.value.type === 'ObjectExpression') { + checkForbidden(node.value.properties); + } + }, + + MemberExpression: function(node) { + if (isPropTypesDeclaration(node.property)) { + var right = node.parent.right; + if (right && right.type === 'ObjectExpression') { + checkForbidden(right.properties); + } + } + }, + + ObjectExpression: function(node) { + node.properties.forEach(function(property) { + if (!property.key) { + return; + } + + if (!isPropTypesDeclaration(property.key)) { + return; + } + if (property.value.type === 'ObjectExpression') { + checkForbidden(property.value.properties); + } + }); + } + + }; +}; + +module.exports.schema = [{ + type: 'object', + properties: { + forbid: { + type: 'array', + items: { + type: 'string' + } + } + }, + additionalProperties: true +}]; diff --git a/tests/lib/rules/forbid-prop-types.js b/tests/lib/rules/forbid-prop-types.js new file mode 100644 index 0000000000..42caff7151 --- /dev/null +++ b/tests/lib/rules/forbid-prop-types.js @@ -0,0 +1,425 @@ +/** + * @fileoverview Tests for forbid-prop-types + */ +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +var rule = require('../../../lib/rules/forbid-prop-types'); +var RuleTester = require('eslint').RuleTester; + +require('babel-eslint'); + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +var ANY_ERROR_MESSAGE = 'Prop type `any` is forbidden'; +var ARRAY_ERROR_MESSAGE = 'Prop type `array` is forbidden'; +var NUMBER_ERROR_MESSAGE = 'Prop type `number` is forbidden'; +var OBJECT_ERROR_MESSAGE = 'Prop type `object` is forbidden'; + +var ruleTester = new RuleTester(); +ruleTester.run('forbid-prop-types', rule, { + + valid: [{ + code: [ + 'var First = React.createClass({', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + } + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: externalPropTypes,', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + } + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' s: React.PropTypes.string,', + ' n: React.PropTypes.number,', + ' i: React.PropTypes.instanceOf,', + ' b: React.PropTypes.bool', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + } + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.array', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'object'] + }], + ecmaFeatures: { + jsx: true + } + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' o: React.PropTypes.object', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'array'] + }], + ecmaFeatures: { + jsx: true + } + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' o: React.PropTypes.object,', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + forbid: ['any', 'array'] + }], + ecmaFeatures: { + jsx: true + } + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.propTypes = {', + ' a: React.PropTypes.string,', + ' b: React.PropTypes.string', + '};', + 'First.propTypes.justforcheck = React.PropTypes.string;' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "aria-controls": React.PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint' + }, { + // Invalid code, should not be validated + code: [ + 'class Component extends React.Component {', + ' propTypes: {', + ' a: React.PropTypes.any,', + ' c: React.PropTypes.any,', + ' b: React.PropTypes.any', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint' + }, { + code: [ + 'var Hello = React.createClass({', + ' render: function() {', + ' let { a, ...b } = obj;', + ' let c = { ...d };', + ' return
;', + ' }', + '});' + ].join('\n'), + env: { + es6: true + }, + ecmaFeatures: { + experimentalObjectRestSpread: true, + jsx: true + } + }], + + invalid: [{ + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.any', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' n: React.PropTypes.number', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: NUMBER_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }], + options: [{ + forbid: ['number'] + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.any.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: ANY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.array', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: ARRAY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.array.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: ARRAY_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.object', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: OBJECT_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.object.isRequired', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: [{ + message: OBJECT_ERROR_MESSAGE, + line: 3, + column: 5, + type: 'Property' + }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.array,', + ' o: React.PropTypes.object', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: 2 + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' s: React.PropTypes.shape({,', + ' o: React.PropTypes.object', + ' })', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: 1 + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.array', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});', + 'var Second = React.createClass({', + ' propTypes: {', + ' o: React.PropTypes.object', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: true + }, + errors: 2 + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.propTypes = {', + ' a: React.PropTypes.array,', + ' o: React.PropTypes.object', + '};', + 'class Second extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'Second.propTypes = {', + ' a: React.PropTypes.array,', + ' o: React.PropTypes.object', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: 4 + }, { + code: [ + 'class Component extends React.Component {', + ' static propTypes = {', + ' a: React.PropTypes.array,', + ' o: React.PropTypes.object', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: 2 + }] +});