From 13806f1bc99817c06223a2b799bdc1125d208290 Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Sat, 7 Jan 2017 20:49:41 +0000 Subject: [PATCH 1/5] Add support for variable reference to sort-prop-types (fixes #622) --- lib/rules/sort-prop-types.js | 35 +++++++++++++++++++++++++----- tests/lib/rules/sort-prop-types.js | 27 +++++++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/lib/rules/sort-prop-types.js b/lib/rules/sort-prop-types.js index b3c04f5482..817e4123d7 100644 --- a/lib/rules/sort-prop-types.js +++ b/lib/rules/sort-prop-types.js @@ -3,6 +3,10 @@ */ 'use strict'; +var find = require('array.prototype.find'); + +var variableUtil = require('../util/variable'); + // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -150,11 +154,32 @@ module.exports = { }, MemberExpression: function(node) { - if (isPropTypesDeclaration(node.property)) { - var right = node.parent.right; - if (right && right.type === 'ObjectExpression') { - checkSorted(right.properties); - } + if (!isPropTypesDeclaration(node.property)) { + return; + } + var right = node.parent.right; + var declarations; + switch (right && right.type) { + case 'ObjectExpression': + declarations = right.properties; + break; + case 'Identifier': + var variable = find(variableUtil.variablesInScope(context), function (item) { + return item.name === right.name; + }); + if ( + !variable || !variable.defs[0] || + !variable.defs[0].node.init || !variable.defs[0].node.init.properties + ) { + break; + } + declarations = variable.defs[0].node.init.properties; + break; + default: + break; + } + if (declarations) { + checkSorted(declarations); } }, diff --git a/tests/lib/rules/sort-prop-types.js b/tests/lib/rules/sort-prop-types.js index d18b8aa144..fd1fe835ba 100644 --- a/tests/lib/rules/sort-prop-types.js +++ b/tests/lib/rules/sort-prop-types.js @@ -323,6 +323,15 @@ ruleTester.run('sort-prop-types', rule, { '}' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'const propTypes = require(\'./externalPropTypes\')', + 'const TextFieldLabel = (props) => {', + ' return
;', + '};', + 'TextFieldLabel.propTypes = propTypes;' + ].join('\n'), + parserOptions: parserOptions }], invalid: [{ @@ -624,5 +633,23 @@ ruleTester.run('sort-prop-types', rule, { column: 5, type: 'Property' }] + }, { + code: [ + 'const propTypes = {', + ' b: PropTypes.string,', + ' a: PropTypes.string,', + '};', + 'const TextFieldLabel = (props) => {', + ' return
;', + '};', + 'TextFieldLabel.propTypes = propTypes;' + ].join('\n'), + parserOptions: parserOptions, + errors: [{ + message: ERROR_MESSAGE, + line: 3, + column: 3, + type: 'Property' + }] }] }); From bad47d6a005a5738f25eecc0ab55e1d8181cd0e6 Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Sun, 8 Jan 2017 19:28:47 +0000 Subject: [PATCH 2/5] Fix prop-types to correcly assign props to components (fixes #991) --- lib/rules/prop-types.js | 2 +- tests/lib/rules/prop-types.js | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index e64aaef39a..2e34672fce 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -586,7 +586,7 @@ module.exports = { } var component = components.get(utils.getParentComponent()); - var usedPropTypes = component && component.usedPropTypes || []; + var usedPropTypes = (component && component.usedPropTypes || []).slice(); switch (type) { case 'direct': diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 30dd356af3..fd143f4398 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -1415,6 +1415,19 @@ ruleTester.run('prop-types', rule, { jsx: true } } + }, { + code: [ + 'export class Example extends Component {', + ' static propTypes = {', + ' onDelete: React.PropTypes.func.isRequired', + ' }', + ' handleDeleteConfirm = () => {', + ' this.props.onDelete();', + ' };', + ' handleSubmit = async ({certificate, key}) => {};', + '}' + ].join('\n'), + parser: 'babel-eslint' } ], From 0fcf32170025e8ea08b7018e694f11008a2027be Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Sun, 8 Jan 2017 19:48:43 +0000 Subject: [PATCH 3/5] Update CHANGELOG and bump version --- CHANGELOG.md | 17 +++++++++++++++++ package.json | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 182b07cc7d..334beb2f92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,23 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). +## [6.9.0] - 2016-01-08 +### Added +* Add support for variable reference to [`sort-prop-types`][] ([#622][]) + +### Fixed +* Fix Node.js 0.10 support ([#1000][] @ljharb) +* Fix [`prop-types`][] to correctly assign props to components ([#991][]) + +### Changed +* Documentation improvements ([#995][] @rutsky) + +[6.9.0]: https://github.com/yannickcr/eslint-plugin-react/compare/v6.8.0...v6.9.0 +[#622]: https://github.com/yannickcr/eslint-plugin-react/issues/622 +[#1000]: https://github.com/yannickcr/eslint-plugin-react/pull/1000 +[#991]: https://github.com/yannickcr/eslint-plugin-react/issues/991 +[#995]: https://github.com/yannickcr/eslint-plugin-react/pull/995 + ## [6.8.0] - 2016-12-05 ### Added * Add [`no-array-index-key`][] rule ([#978][] @lencioni) diff --git a/package.json b/package.json index 3264fcf9c1..46fd913bc2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-react", - "version": "6.8.0", + "version": "6.9.0", "author": "Yannick Croissant ", "description": "React specific linting rules for ESLint", "main": "index.js", From a33db3be709b6177355f960003f72eef3368f372 Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Tue, 10 Jan 2017 00:58:42 +0100 Subject: [PATCH 4/5] Add support for nextProps to prop-types (fixes #814) --- lib/rules/prop-types.js | 76 +++++++++++++++++++++++------------ tests/lib/rules/prop-types.js | 41 +++++++++++++++++++ 2 files changed, 91 insertions(+), 26 deletions(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 2e34672fce..e232990c64 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -15,7 +15,8 @@ var annotations = require('../util/annotations'); // Constants // ------------------------------------------------------------------------------ -var DIRECT_PROPS_REGEX = /^props\s*(\.|\[)/; +var PROPS_REGEX = /^(props|nextProps)$/; +var DIRECT_PROPS_REGEX = /^(props|nextProps)\s*(\.|\[)/; // ------------------------------------------------------------------------------ // Rule Definition @@ -81,6 +82,39 @@ 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() { + var 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() { + var 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 we are using a prop * @param {ASTNode} node The AST node being checked. @@ -92,7 +126,8 @@ module.exports = { node.object.type === 'ThisExpression' && node.property.name === 'props' ); var isStatelessFunctionUsage = node.object.name === 'props'; - return isClassUsage || isStatelessFunctionUsage; + var isNextPropsUsage = node.object.name === 'nextProps' && inComponentWillReceiveProps(); + return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage; } /** @@ -464,21 +499,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() { - var 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. @@ -487,8 +507,9 @@ module.exports = { function getPropertyName(node) { var isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); var isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); - var isNotInConstructor = !inConstructor(node); - if (isDirectProp && isInClassComponent && isNotInConstructor) { + var isNotInConstructor = !inConstructor(); + var isNotInComponentWillReceiveProps = !inComponentWillReceiveProps(); + if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps) { return void 0; } if (!isDirectProp) { @@ -561,13 +582,13 @@ module.exports = { // let {props: {firstname}} = this var thisDestructuring = ( !hasSpreadOperator(node.id.properties[i]) && - (node.id.properties[i].key.name === 'props' || node.id.properties[i].key.value === 'props') && + (PROPS_REGEX.test(node.id.properties[i].key.name) || PROPS_REGEX.test(node.id.properties[i].key.value)) && node.id.properties[i].value.type === 'ObjectPattern' ); // let {firstname} = props var directDestructuring = - node.init.name === 'props' && - (utils.getParentStatelessComponent() || inConstructor()) + PROPS_REGEX.test(node.init.name) && + (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) ; if (thisDestructuring) { @@ -600,7 +621,10 @@ module.exports = { usedPropTypes.push({ name: name, allNames: allNames, - node: !isDirectProp && !inConstructor(node) ? node.parent.property : node.property + node: ( + !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ? node.parent.property : + node.property + ) }); break; case 'destructuring': @@ -612,7 +636,7 @@ module.exports = { var currentNode = node; allNames = []; - while (currentNode.property && currentNode.property.name !== 'props') { + while (currentNode.property && !PROPS_REGEX.test(currentNode.property.name)) { allNames.unshift(currentNode.property.name); currentNode = currentNode.object; } @@ -813,8 +837,8 @@ module.exports = { // let {firstname} = props var directDestructuring = destructuring && - node.init.name === 'props' && - (utils.getParentStatelessComponent() || inConstructor()) + PROPS_REGEX.test(node.init.name) && + (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) ; if (!thisDestructuring && !directDestructuring) { diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index fd143f4398..16bfae17ba 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -2575,6 +2575,47 @@ ruleTester.run('prop-types', rule, { errors: [ {message: '\'foo\' is missing in props validation'} ] + }, { + code: [ + 'class Hello extends Component {', + ' static propTypes = {', + ' bar: PropTypes.func', + ' }', + ' componentWillReceiveProps(nextProps) {', + ' if (nextProps.foo) {', + ' return;', + ' }', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [ + {message: '\'foo\' is missing in props validation'} + ] + }, { + code: [ + 'class Hello extends Component {', + ' static propTypes = {', + ' bar: PropTypes.func', + ' }', + ' componentWillReceiveProps(nextProps) {', + ' const {foo} = nextProps;', + ' if (foo) {', + ' return;', + ' }', + ' }', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [ + {message: '\'foo\' is missing in props validation'} + ] } ] }); From 0e8a1be4c15e444339c4111476a3112f4205d873 Mon Sep 17 00:00:00 2001 From: Neil Kistner Date: Tue, 10 Jan 2017 18:22:50 -0600 Subject: [PATCH 5/5] Fix `require-default-props` rule when using Flow type from assignment --- lib/rules/require-default-props.js | 7 ++++- tests/lib/rules/require-default-props.js | 33 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 550ac4b8a8..ea9c69757c 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -141,7 +141,12 @@ module.exports = { switch (node.typeAnnotation.type) { case 'GenericTypeAnnotation': var annotation = resolveGenericTypeAnnotation(node.typeAnnotation); - properties = annotation ? annotation.properties : []; + + if (annotation && annotation.id) { + annotation = findVariableByName(annotation.id.name); + } + + properties = annotation ? (annotation.properties || []) : []; break; case 'UnionTypeAnnotation': diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js index 991913e88b..57d918be37 100644 --- a/tests/lib/rules/require-default-props.js +++ b/tests/lib/rules/require-default-props.js @@ -676,6 +676,39 @@ ruleTester.run('require-default-props', rule, { '}' ].join('\n'), parser: 'babel-eslint' + }, + { + code: [ + 'import type ImportedProps from "fake";', + 'type Props = ImportedProps;', + 'function Hello(props: Props) {', + ' return
Hello {props.name.firstname}
;', + '}' + ].join('\n'), + parser: 'babel-eslint' + }, + // don't error when variable is not in scope + { + code: [ + 'import type { ImportedType } from "fake";', + 'type Props = ImportedType;', + 'function Hello(props: Props) {', + ' return
Hello {props.name.firstname}
;', + '}' + ].join('\n'), + parser: 'babel-eslint' + }, + // make sure error is not thrown with multiple assignments + { + code: [ + 'import type ImportedProps from "fake";', + 'type NestedProps = ImportedProps;', + 'type Props = NestedProps;', + 'function Hello(props: Props) {', + ' return
Hello {props.name.firstname}
;', + '}' + ].join('\n'), + parser: 'babel-eslint' } ],