From 7443a37fff3d2cfc77d9c4379e19bb0bc8266800 Mon Sep 17 00:00:00 2001 From: Ian Sutherland Date: Tue, 23 Jan 2018 14:56:58 -0700 Subject: [PATCH] [New] `forbid-foreign-prop-types`: Add `allowInPropTypes` option Fixes #1647. --- lib/rules/forbid-foreign-prop-types.js | 79 ++++++++++++++++---- tests/lib/rules/forbid-foreign-prop-types.js | 36 ++++++++- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/lib/rules/forbid-foreign-prop-types.js b/lib/rules/forbid-foreign-prop-types.js index 0ea1e1bd64..5e83f891be 100644 --- a/lib/rules/forbid-foreign-prop-types.js +++ b/lib/rules/forbid-foreign-prop-types.js @@ -6,15 +6,6 @@ const docsUrl = require('../util/docsUrl'); -// ------------------------------------------------------------------------------ -// Constants -// ------------------------------------------------------------------------------ - - -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ - module.exports = { meta: { docs: { @@ -22,10 +13,25 @@ module.exports = { category: 'Best Practices', recommended: false, url: docsUrl('forbid-foreign-prop-types') - } + }, + + schema: [ + { + type: 'object', + properties: { + allowInPropTypes: { + type: 'boolean' + } + }, + additionalProperties: false + } + ] }, create: function(context) { + const config = context.options[0] || {}; + const allowInPropTypes = config.allowInPropTypes || false; + // -------------------------------------------------------------------------- // Helpers // -------------------------------------------------------------------------- @@ -34,15 +40,56 @@ module.exports = { return node.parent.type === 'AssignmentExpression' && node.parent.left === node; } + function findParentAssignmentExpression(node) { + let parent = node.parent; + + while (parent && parent.type !== 'Program') { + if (parent.type === 'AssignmentExpression') { + return parent; + } + parent = parent.parent; + } + return null; + } + + function isAllowedAssignment(node) { + if (!allowInPropTypes) { + return false; + } + + const assignmentExpression = findParentAssignmentExpression(node); + + if ( + assignmentExpression && + assignmentExpression.left && + assignmentExpression.left.property && + assignmentExpression.left.property.name === 'propTypes' + ) { + return true; + } + return false; + } + return { MemberExpression: function(node) { - if (!node.computed && node.property && node.property.type === 'Identifier' && - node.property.name === 'propTypes' && !isLeftSideOfAssignment(node) || - node.property && node.property.type === 'Literal' && - node.property.value === 'propTypes' && !isLeftSideOfAssignment(node)) { + if ( + node.property && + ( + !node.computed && + node.property.type === 'Identifier' && + node.property.name === 'propTypes' && + !isLeftSideOfAssignment(node) && + !isAllowedAssignment(node) + ) || ( + node.property.type === 'Literal' && + node.property.value === 'propTypes' && + !isLeftSideOfAssignment(node) && + !isAllowedAssignment(node) + ) + ) { context.report({ node: node.property, - message: 'Using another component\'s propTypes is forbidden' + message: 'Using propTypes from another component is not safe because they may be removed in production builds' }); } }, @@ -53,7 +100,7 @@ module.exports = { if (propTypesNode) { context.report({ node: propTypesNode, - message: 'Using another component\'s propTypes is forbidden' + message: 'Using propTypes from another component is not safe because they may be removed in production builds' }); } } diff --git a/tests/lib/rules/forbid-foreign-prop-types.js b/tests/lib/rules/forbid-foreign-prop-types.js index b2c1c4a5de..8c081b2df6 100644 --- a/tests/lib/rules/forbid-foreign-prop-types.js +++ b/tests/lib/rules/forbid-foreign-prop-types.js @@ -25,7 +25,7 @@ require('babel-eslint'); // Tests // ----------------------------------------------------------------------------- -const ERROR_MESSAGE = 'Using another component\'s propTypes is forbidden'; +const ERROR_MESSAGE = 'Using propTypes from another component is not safe because they may be removed in production builds'; const ruleTester = new RuleTester({parserOptions}); ruleTester.run('forbid-foreign-prop-types', rule, { @@ -50,6 +50,21 @@ ruleTester.run('forbid-foreign-prop-types', rule, { code: 'Foo["propTypes"] = propTypes' }, { code: 'const propTypes = "bar"; Foo[propTypes];' + }, + { + code: ` + const Message = (props) => (
{props.message}
); + Message.propTypes = { + message: PropTypes.string + }; + const Hello = (props) => (Hello {props.name}); + Hello.propTypes = { + name: Message.propTypes.message + }; + `, + options: [{ + allowInPropTypes: true + }] }], invalid: [{ @@ -139,5 +154,24 @@ ruleTester.run('forbid-foreign-prop-types', rule, { message: ERROR_MESSAGE, type: 'Property' }] + }, + { + code: ` + const Message = (props) => (
{props.message}
); + Message.propTypes = { + message: PropTypes.string + }; + const Hello = (props) => (Hello {props.name}); + Hello.propTypes = { + name: Message.propTypes.message + }; + `, + options: [{ + allowInPropTypes: false + }], + errors: [{ + message: ERROR_MESSAGE, + type: 'Identifier' + }] }] });