diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 846faa54df..39b91954ed 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -13,6 +13,7 @@ const variable = require('../util/variable'); const annotations = require('../util/annotations'); const versionUtil = require('../util/version'); const propsUtil = require('../util/props'); +const astUtil = require('../util/ast'); // ------------------------------------------------------------------------------ // Constants @@ -21,7 +22,6 @@ const propsUtil = require('../util/props'); const DIRECT_PROPS_REGEX = /^props\s*(\.|\[)/; const DIRECT_NEXT_PROPS_REGEX = /^nextProps\s*(\.|\[)/; const DIRECT_PREV_PROPS_REGEX = /^prevProps\s*(\.|\[)/; -const LIFE_CYCLE_METHODS = ['componentWillReceiveProps', 'shouldComponentUpdate', 'componentWillUpdate', 'componentDidUpdate']; // ------------------------------------------------------------------------------ // Rule Definition @@ -82,24 +82,6 @@ module.exports = { return value; } - /** - * Check if we are in a lifecycle method - * @return {boolean} true if we are in a class constructor, false if not - **/ - function inLifeCycleMethod() { - let scope = context.getScope(); - while (scope) { - if ( - scope.block && scope.block.parent && - scope.block.parent.key && - LIFE_CYCLE_METHODS.indexOf(scope.block.parent.key.name) >= 0 - ) { - return true; - } - scope = scope.upper; - } - return false; - } /** * Check if the current node is in a setState updater method @@ -155,7 +137,7 @@ module.exports = { || isPropArgumentInSetStateUpdater(node)) ); const isStatelessFunctionUsage = node.object.name === 'props'; - return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod(); + return isClassUsage || isStatelessFunctionUsage || astUtil.inLifeCycleMethod(context); } /** @@ -241,40 +223,6 @@ module.exports = { ); } - /** - * Returns true if the given node is a React Component lifecycle method - * @param {ASTNode} node The AST node being checked. - * @return {Boolean} True if the node is a lifecycle method - */ - function isNodeALifeCycleMethod(node) { - const nodeKeyName = (node.key || {}).name; - return ( - node.kind === 'constructor' || - nodeKeyName === 'componentWillReceiveProps' || - nodeKeyName === 'shouldComponentUpdate' || - nodeKeyName === 'componentWillUpdate' || - nodeKeyName === 'componentDidUpdate' - ); - } - - /** - * Returns true if the given node is inside a React Component lifecycle - * method. - * @param {ASTNode} node The AST node being checked. - * @return {Boolean} True if the node is inside a lifecycle method - */ - function isInLifeCycleMethod(node) { - if (node.type === 'MethodDefinition' && isNodeALifeCycleMethod(node)) { - return true; - } - - if (node.parent) { - return isInLifeCycleMethod(node.parent); - } - - return false; - } - /** * Checks if a prop init name matches common naming patterns * @param {ASTNode} node The AST node being checked. @@ -553,21 +501,6 @@ module.exports = { } } - /** - * Check if we are in a class constructor - * @return {boolean} true if we are in a class constructor, false if not - */ - function inConstructor() { - let scope = context.getScope(); - while (scope) { - if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') { - return true; - } - scope = scope.upper; - } - return false; - } - /** * Retrieve the name of a property node * @param {ASTNode} node The AST node with the property. @@ -579,10 +512,11 @@ module.exports = { const isDirectPrevProp = DIRECT_PREV_PROPS_REGEX.test(sourceCode.getText(node)); const isDirectSetStateProp = isPropArgumentInSetStateUpdater(node); const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); - const isNotInConstructor = !inConstructor(node); - const isNotInLifeCycleMethod = !inLifeCycleMethod(); + const isNotInLifeCycleMethod = !astUtil.inLifeCycleMethod(context); + const isNotInConstructor = !astUtil.inConstructor(context); const isNotInSetStateUpdater = !inSetStateUpdater(); - if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp) + if ( + (isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp) && isInClassComponent && isNotInConstructor && isNotInLifeCycleMethod @@ -668,10 +602,12 @@ module.exports = { ) ); // let {firstname} = props - const genericDestructuring = isPropAttributeName(node) && ( - utils.getParentStatelessComponent() || - isInLifeCycleMethod(node) - ); + const genericDestructuring = + isPropAttributeName(node) && ( + utils.getParentStatelessComponent() || + astUtil.inConstructor(context) || + astUtil.inLifeCycleMethod(context) + ); if (thisDestructuring) { properties = node.id.properties[i].value.properties; @@ -992,7 +928,8 @@ module.exports = { // let {firstname} = props const statelessDestructuring = destructuring && isPropAttributeName(node) && ( utils.getParentStatelessComponent() || - isInLifeCycleMethod(node) + astUtil.inConstructor(context) || + astUtil.inLifeCycleMethod(context) ); if (!thisDestructuring && !statelessDestructuring) { @@ -1058,7 +995,7 @@ module.exports = { ObjectPattern: function(node) { // If the object pattern is a destructured props object in a lifecycle // method -- mark it for used props. - if (isNodeALifeCycleMethod(node.parent.parent)) { + if (astUtil.inConstructor(context) || astUtil.inLifeCycleMethod(context)) { node.properties.forEach((property, i) => { if (i === 0) { markPropTypesAsUsed(node.parent); diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 7638959db0..42fac50a69 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -13,6 +13,7 @@ const variable = require('../util/variable'); const annotations = require('../util/annotations'); const versionUtil = require('../util/version'); const propsUtil = require('../util/props'); +const astUtil = require('../util/ast'); // ------------------------------------------------------------------------------ // Constants @@ -86,39 +87,6 @@ module.exports = { return value; } - /** - * Check if we are in a class constructor - * @return {boolean} true if we are in a class constructor, false if not - */ - function inConstructor() { - let scope = context.getScope(); - while (scope) { - if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') { - return true; - } - scope = scope.upper; - } - return false; - } - - /** - * Check if we are in a class constructor - * @return {boolean} true if we are in a class constructor, false if not - */ - function inComponentWillReceiveProps() { - let scope = context.getScope(); - while (scope) { - if ( - scope.block && scope.block.parent && - scope.block.parent.key && scope.block.parent.key.name === 'componentWillReceiveProps' - ) { - return true; - } - scope = scope.upper; - } - return false; - } - /** * Checks if a prop is being assigned a value props.bar = 'bar' * @param {ASTNode} node The AST node being checked. @@ -144,7 +112,7 @@ module.exports = { node.object.type === 'ThisExpression' && node.property.name === 'props' ); const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node); - const isNextPropsUsage = node.object.name === 'nextProps' && inComponentWillReceiveProps(); + const isNextPropsUsage = node.object.name === 'nextProps' && astUtil.inLifeCycleMethod(context); return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage; } @@ -534,9 +502,14 @@ module.exports = { function getPropertyName(node) { const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); - const isNotInConstructor = !inConstructor(); - const isNotInComponentWillReceiveProps = !inComponentWillReceiveProps(); - if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps) { + const isNotInConstructor = !astUtil.inConstructor(context); + const isNotInLifecycleMethod = !astUtil.inLifeCycleMethod(context); + if ( + isDirectProp && + isInClassComponent && + isNotInConstructor && + isNotInLifecycleMethod + ) { return void 0; } if (!isDirectProp) { @@ -614,9 +587,11 @@ module.exports = { ); // let {firstname} = props const directDestructuring = - PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) - ; + PROPS_REGEX.test(node.init.name) && ( + utils.getParentStatelessComponent() || + astUtil.inConstructor(context) || + astUtil.inLifeCycleMethod(context) + ); if (thisDestructuring) { properties = node.id.properties[i].value.properties; @@ -649,7 +624,7 @@ module.exports = { name: name, allNames: allNames, node: ( - !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ? + !isDirectProp && !astUtil.inConstructor(context) && !astUtil.inLifeCycleMethod(context) ? node.parent.property : node.property ) @@ -967,7 +942,7 @@ module.exports = { const directDestructuring = destructuring && PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) + (utils.getParentStatelessComponent() || astUtil.inConstructor(context) || astUtil.inLifeCycleMethod(context)) ; if (!thisDestructuring && !directDestructuring) { diff --git a/lib/util/ast.js b/lib/util/ast.js index dc9ce7704f..f63661d390 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -3,6 +3,8 @@ */ 'use strict'; +const LIFE_CYCLE_METHODS = ['componentWillReceiveProps', 'shouldComponentUpdate', 'componentWillUpdate', 'componentDidUpdate']; + /** * Get properties name * @param {Object} node - Property. @@ -59,8 +61,49 @@ function findReturnStatement(node) { return false; } +/** + * Check if we are in a lifecycle method + * @return {boolean} true if we are in a lifecycle method, false if not + **/ +function inLifeCycleMethod(context) { + let scope = context.getScope(); + while (scope) { + if ( + scope.block && + scope.block.parent && + scope.block.parent.key && + LIFE_CYCLE_METHODS.indexOf(scope.block.parent.key.name) >= 0 + ) { + return true; + } + scope = scope.upper; + } + return false; +} + +/** + * Check if we are in a class constructor + * @return {boolean} true if we are in a class constructor, false if not + */ +function inConstructor(context) { + let scope = context.getScope(); + while (scope) { + if ( + scope.block && + scope.block.parent && + scope.block.parent.kind === 'constructor' + ) { + return true; + } + scope = scope.upper; + } + return false; +} + module.exports = { + findReturnStatement: findReturnStatement, getPropertyName: getPropertyName, getComponentProperties: getComponentProperties, - findReturnStatement: findReturnStatement + inConstructor: inConstructor, + inLifeCycleMethod: inLifeCycleMethod }; diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index 7321ff3afc..f3047263a8 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -3241,6 +3241,23 @@ ruleTester.run('no-unused-prop-types', rule, { line: 3, column: 13 }] + }, { + code: [ + 'class Hello extends Component {', + ' static propTypes = {', + ' unused: PropTypes.bool', + ' }', + ' constructor(props) {', + ' this.state = { something: props.something }', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'unused\' PropType is defined but prop is never used', + line: 3, + column: 13 + }] }, { code: [ 'class Hello extends Component {', diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 9bc4223e6f..15a22c838e 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -1677,7 +1677,7 @@ ruleTester.run('prop-types', rule, { class Bar extends React.Component { props: Props; - + render() { return