From 1ecb622532fdb8e649cfcb559867f41c07c28134 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 15 Jun 2018 21:27:03 -0700 Subject: [PATCH 01/49] Don't depend on state parameter name in no-unused-state Resolves #1759 --- lib/rules/no-unused-state.js | 43 ++++++++++--- tests/lib/rules/no-unused-state.js | 98 ++++++++++++++++++++++++++++-- 2 files changed, 127 insertions(+), 14 deletions(-) diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js index 4d8348419c..7d771fe303 100644 --- a/lib/rules/no-unused-state.js +++ b/lib/rules/no-unused-state.js @@ -77,7 +77,38 @@ module.exports = { // JSX attributes), then this is again set to null. let classInfo = null; - // Returns true if the given node is possibly a reference to `this.state`, `prevState` or `nextState`. + function isStateParameterReference(node) { + const classMethods = [ + 'shouldComponentUpdate', + 'componentWillUpdate', + 'UNSAFE_componentWillUpdate', + 'getSnapshotBeforeUpdate', + 'componentDidUpdate' + ]; + + let scope = context.getScope(); + while (scope) { + const parent = scope.block && scope.block.parent; + if ( + parent && + parent.type === 'MethodDefinition' && ( + parent.static && parent.key.name === 'getDerivedStateFromProps' || + classMethods.indexOf(parent.key.name !== -1) + ) && + parent.value.type === 'FunctionExpression' && + parent.value.params[1] && + parent.value.params[1].name === node.name + ) { + return true; + } + scope = scope.upper; + } + + return false; + } + + // Returns true if the given node is possibly a reference to `this.state` or the state parameter of + // a lifecycle method. function isStateReference(node) { node = uncast(node); @@ -91,15 +122,7 @@ module.exports = { classInfo.aliases && classInfo.aliases.has(node.name); - const isPrevStateReference = - node.type === 'Identifier' && - node.name === 'prevState'; - - const isNextStateReference = - node.type === 'Identifier' && - node.name === 'nextState'; - - return isDirectStateReference || isAliasedStateReference || isPrevStateReference || isNextStateReference; + return isDirectStateReference || isAliasedStateReference || isStateParameterReference(node); } // Takes an ObjectExpression node and adds all named Property nodes to the diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js index e26244be56..8261ae708c 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -492,15 +492,15 @@ eslintTester.run('no-unused-state', rule, { parser: 'babel-eslint' }, { - code: `class ESLintExample extends Component { + code: `class GetDerivedStateFromPropsTest extends Component { constructor(props) { super(props); this.state = { id: 123, }; } - static getDerivedStateFromProps(nextProps, prevState) { - if (prevState.id === nextProps.id) { + static getDerivedStateFromProps(nextProps, otherState) { + if (otherState.id === nextProps.id) { return { selected: true, }; @@ -516,7 +516,29 @@ eslintTester.run('no-unused-state', rule, { parser: 'babel-eslint' }, { - code: `class ESLintExample extends Component { + code: `class ComponentDidUpdateTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123, + }; + } + + componentDidUpdate(someProps, someState) { + if (someState.id === someProps.id) { + doStuff(); + } + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + parser: 'babel-eslint' + }, + { + code: `class ShouldComponentUpdateTest extends Component { constructor(props) { super(props); this.state = { @@ -533,6 +555,27 @@ eslintTester.run('no-unused-state', rule, { } }`, parser: 'babel-eslint' + }, + { + code: `class NestedScopesTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123, + }; + } + shouldComponentUpdate(nextProps, nextState) { + return (function() { + return nextState.id === nextProps.id; + })(); + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + parser: 'babel-eslint' } ], @@ -824,6 +867,53 @@ eslintTester.run('no-unused-state', rule, { }`, errors: getErrorMessages(['bar']), parser: 'babel-eslint' + }, + { + code: `class FakePrevStateVariableTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123, + foo: 456 + }; + } + + componentDidUpdate(someProps, someState) { + if (someState.id === someProps.id) { + const prevState = { foo: 789 }; + console.log(prevState.foo); + } + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + errors: getErrorMessages(['foo']), + parser: 'babel-eslint' + }, + { + code: `class MissingStateParameterTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123 + }; + } + + componentDidUpdate(someProps) { + const prevState = { id: 456 }; + console.log(prevState.id); + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + errors: getErrorMessages(['id']), + parser: 'babel-eslint' } ] }); From fb5c87f4db13d1bab2df43c1376474d8ead3beb4 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 4 Aug 2018 23:52:46 -0700 Subject: [PATCH 02/49] Additional tests for prop-types and no-unused-prop-types --- lib/rules/no-unused-prop-types.js | 8 +- lib/rules/prop-types.js | 2 +- lib/util/propTypes.js | 10 +- tests/lib/rules/no-unused-prop-types.js | 258 +++++++++++++++++++++--- tests/lib/rules/prop-types.js | 92 +++++++++ 5 files changed, 332 insertions(+), 38 deletions(-) diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 64de1fbd25..3948d67398 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -146,7 +146,7 @@ module.exports = { function mustBeValidated(component) { return Boolean( component && - !component.ignorePropsValidation + !component.ignoreUnusedPropTypesValidation ); } @@ -397,7 +397,7 @@ module.exports = { const component = components.get(utils.getParentComponent()); const usedPropTypes = component && component.usedPropTypes || []; - let ignorePropsValidation = component && component.ignorePropsValidation || false; + let ignoreUnusedPropTypesValidation = component && component.ignoreUnusedPropTypesValidation || false; switch (type) { case 'direct': @@ -414,7 +414,7 @@ module.exports = { case 'destructuring': for (let k = 0, l = (properties || []).length; k < l; k++) { if (hasSpreadOperator(properties[k]) || properties[k].computed) { - ignorePropsValidation = true; + ignoreUnusedPropTypesValidation = true; break; } const propName = getKeyValue(properties[k]); @@ -441,7 +441,7 @@ module.exports = { components.set(component ? component.node : node, { usedPropTypes: usedPropTypes, - ignorePropsValidation: ignorePropsValidation + ignoreUnusedPropTypesValidation: ignoreUnusedPropTypesValidation }); } diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 239c58bcec..c35e811bb4 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -191,7 +191,7 @@ module.exports = { return true; } // Consider every children as declared - if (propType.children === true) { + if (propType.children === true || propType.containsSpread) { return true; } if (propType.acceptedProperties) { diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index 88be549151..d58af934de 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -143,10 +143,10 @@ module.exports = function propTypesInstructions(context, components, utils) { } }); - // nested object type spread means we need to ignore/accept everything in this object - if (containsObjectTypeSpread) { - return {}; - } + // Mark if this shape has spread. We will know to consider all props from this shape as having propTypes, + // but still have the ability to detect unused children of this shape. + shapeTypeDefinition.containsSpread = containsObjectTypeSpread; + return shapeTypeDefinition; }, @@ -669,7 +669,7 @@ module.exports = function propTypesInstructions(context, components, utils) { JSXSpreadAttribute: function(node) { const component = components.get(utils.getParentComponent()); components.set(component ? component.node : node, { - ignorePropsValidation: true + ignoreUnusedPropTypesValidation: true }); }, diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index 61bff0fee9..afa4b5a94b 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -1801,34 +1801,6 @@ ruleTester.run('no-unused-prop-types', rule, { ' bar: PropTypes.bool', '};' ].join('\n') - }, { - code: [ - 'type Person = {', - ' ...data,', - ' lastname: string', - '};', - 'class Hello extends React.Component {', - ' props: Person;', - ' render () {', - ' return
Hello {this.props.firstname}
;', - ' }', - '}' - ].join('\n'), - parser: 'babel-eslint' - }, { - code: [ - 'type Person = {|', - ' ...data,', - ' lastname: string', - '|};', - 'class Hello extends React.Component {', - ' props: Person;', - ' render () {', - ' return
Hello {this.props.firstname}
;', - ' }', - '}' - ].join('\n'), - parser: 'babel-eslint' }, { // The next two test cases are related to: https://github.com/yannickcr/eslint-plugin-react/issues/1183 code: [ @@ -2470,6 +2442,20 @@ ruleTester.run('no-unused-prop-types', rule, { '}' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'const foo = {};', + 'class Hello extends React.Component {', + ' render() {', + ' const {firstname, lastname} = this.props.name;', + ' return
{firstname} {lastname}
;', + ' }', + '}', + 'Hello.propTypes = {', + ' name: PropTypes.shape(foo)', + '};' + ].join('\n'), + parser: 'babel-eslint' }, { // issue #933 code: [ @@ -2913,6 +2899,23 @@ ruleTester.run('no-unused-prop-types', rule, { } `, parser: 'babel-eslint' + }, { + code: [ + 'import type {BasePerson} from \'./types\'', + 'type Props = {', + ' person: {', + ' ...$Exact,', + ' lastname: string', + ' }', + '};', + 'class Hello extends React.Component {', + ' props: Props;', + ' render () {', + ' return
Hello {this.props.person.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', } ], @@ -4475,6 +4478,48 @@ ruleTester.run('no-unused-prop-types', rule, { errors: [{ message: '\'lastname\' PropType is defined but prop is never used' }] + }, { + code: ` + type Person = string; + class Hello extends React.Component<{ person: Person }> { + render () { + return
; + } + } + `, + settings: {react: {flowVersion: '0.53'}}, + errors: [{ + message: '\'person\' PropType is defined but prop is never used' + }], + parser: 'babel-eslint' + }, { + code: ` + type Person = string; + class Hello extends React.Component { + render () { + return
; + } + } + `, + settings: {react: {flowVersion: '0.52'}}, + errors: [{ + message: '\'person\' PropType is defined but prop is never used' + }], + parser: 'babel-eslint' + }, { + code: ` + function higherOrderComponent() { + return class extends React.Component

{ + render() { + return

; + } + } + } + `, + errors: [{ + message: '\'foo\' PropType is defined but prop is never used' + }], + parser: 'babel-eslint' }, { // issue #1506 code: [ @@ -4665,6 +4710,163 @@ ruleTester.run('no-unused-prop-types', rule, { }, { message: '\'a.b.c\' PropType is defined but prop is never used' }] + }, { + code: ` + type Props = { foo: string } + function higherOrderComponent() { + return class extends React.Component { + render() { + return
; + } + } + } + `, + parser: 'babel-eslint', + errors: [{ + message: '\'foo\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'type Person = {', + ' ...data,', + ' lastname: string', + '};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'type Person = {|', + ' ...data,', + ' lastname: string', + '|};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'type Person = {', + ' ...$Exact,', + ' lastname: string', + '};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'import type {Data} from \'./Data\'', + 'type Person = {', + ' ...Data,', + ' lastname: string', + '};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.bar}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'import type {Data} from \'some-libdef-like-flow-typed-provides\'', + 'type Person = {', + ' ...Data,', + ' lastname: string', + '};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.bar}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'type Person = {', + ' ...data,', + ' lastname: string', + '};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'type Person = {|', + ' ...data,', + ' lastname: string', + '|};', + 'class Hello extends React.Component {', + ' props: Person;', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] + }, { + code: [ + 'import type {BasePerson} from \'./types\'', + 'type Props = {', + ' person: {', + ' ...$Exact,', + ' lastname: string', + ' }', + '};', + 'class Hello extends React.Component {', + ' props: Props;', + ' render () {', + ' return
Hello {this.props.person.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [{skipShapeProps: false}], + errors: [{ + message: '\'person.lastname\' PropType is defined but prop is never used' + }] } /* , { diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index a879f05434..bdc492f262 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -342,6 +342,51 @@ ruleTester.run('prop-types', rule, { ' ])', '};' ].join('\n') + }, { + code: ` + class Component extends React.Component { + render() { + return
{this.props.foo.baz}
; + } + } + Component.propTypes = { + foo: PropTypes.oneOfType([ + PropTypes.shape({ + bar: PropTypes.string + }), + PropTypes.shape({ + baz: PropTypes.string + }) + ]) + }; + ` + }, { + code: ` + class Component extends React.Component { + render() { + return
{this.props.foo.baz}
; + } + } + Component.propTypes = { + foo: PropTypes.oneOfType([ + PropTypes.shape({ + bar: PropTypes.string + }), + PropTypes.instanceOf(Baz) + ]) + }; + ` + }, { + code: ` + class Component extends React.Component { + render() { + return
{this.props.foo.baz}
; + } + } + Component.propTypes = { + foo: PropTypes.oneOf(['bar', 'baz']) + }; + ` }, { code: [ 'class Hello extends React.Component {', @@ -478,6 +523,20 @@ ruleTester.run('prop-types', rule, { '};' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'const foo = {};', + 'class Hello extends React.Component {', + ' render() {', + ' const {firstname, lastname} = this.props.name;', + ' return
{firstname} {lastname}
;', + ' }', + '}', + 'Hello.propTypes = {', + ' name: PropTypes.shape(foo)', + '};' + ].join('\n'), + parser: 'babel-eslint' }, { code: [ 'class Hello extends React.Component {', @@ -1209,6 +1268,20 @@ ruleTester.run('prop-types', rule, { '} & FieldProps' ].join('\n'), parser: 'babel-eslint' + }, { + // Impossible intersection type + code: ` + import React from 'react'; + type Props = string & { + fullname: string + }; + class Test extends React.PureComponent { + render() { + return
Hello {this.props.fullname}
+ } + } + `, + parser: 'babel-eslint' }, { code: [ 'Card.propTypes = {', @@ -3760,6 +3833,25 @@ ruleTester.run('prop-types', rule, { message: '\'bad\' is missing in props validation' }], parser: 'babel-eslint' + }, + { + code: ` + class Component extends React.Component { + render() { + return
{this.props.foo.baz}
; + } + } + Component.propTypes = { + foo: PropTypes.oneOfType([ + PropTypes.shape({ + bar: PropTypes.string + }) + ]) + }; + `, + errors: [{ + message: '\'foo.baz\' is missing in props validation' + }] } ] }); From e5bf7da85a25eed225485be2845c03c4c5a28f98 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 14 Aug 2018 23:47:31 -0700 Subject: [PATCH 03/49] Fix lint --- tests/lib/rules/no-unused-prop-types.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index afa4b5a94b..6f6012dfc2 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -2915,7 +2915,7 @@ ruleTester.run('no-unused-prop-types', rule, { ' }', '}' ].join('\n'), - parser: 'babel-eslint', + parser: 'babel-eslint' } ], From 2d285120dbcb4e9002ebf74a97cf4b3c437ee63c Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 15 Aug 2018 10:21:59 -0700 Subject: [PATCH 04/49] Add propTypes tests --- tests/lib/rules/no-unused-prop-types.js | 50 +++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index 6f6012dfc2..e29b4888eb 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -2916,6 +2916,21 @@ ruleTester.run('no-unused-prop-types', rule, { '}' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'import BasePerson from \'./types\'', + 'class Hello extends React.Component {', + ' render () {', + ' return
Hello {this.props.person.firstname}
;', + ' }', + '}', + 'Hello.propTypes = {', + ' person: ProTypes.shape({', + ' ...BasePerson,', + ' lastname: PropTypes.string', + ' })', + '};' + ].join('\n') } ], @@ -4846,6 +4861,22 @@ ruleTester.run('no-unused-prop-types', rule, { errors: [{ message: '\'lastname\' PropType is defined but prop is never used' }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}', + 'Hello.propTypes = {', + ' ...BasePerson,', + ' lastname: PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'lastname\' PropType is defined but prop is never used' + }] }, { code: [ 'import type {BasePerson} from \'./types\'', @@ -4867,6 +4898,25 @@ ruleTester.run('no-unused-prop-types', rule, { errors: [{ message: '\'person.lastname\' PropType is defined but prop is never used' }] + }, { + code: [ + 'import BasePerson from \'./types\'', + 'class Hello extends React.Component {', + ' render () {', + ' return
Hello {this.props.person.firstname}
;', + ' }', + '}', + 'Hello.propTypes = {', + ' person: ProTypes.shape({', + ' ...BasePerson,', + ' lastname: PropTypes.string', + ' })', + '};' + ].join('\n'), + options: [{skipShapeProps: false}], + errors: [{ + message: '\'person.lastname\' PropType is defined but prop is never used' + }] } /* , { From 72a71b302c5f36ee3dd440da43737afc21cc311e Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Tue, 21 Aug 2018 13:27:24 -0700 Subject: [PATCH 05/49] =?UTF-8?q?[Fix]=20`jsx-no-target-blank`:=20don?= =?UTF-8?q?=E2=80=99t=20crash=20when=20there=E2=80=99s=20no=20value?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1949. --- lib/rules/jsx-no-target-blank.js | 4 +++- tests/lib/rules/jsx-no-target-blank.js | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js index e4492874e0..daee012551 100644 --- a/lib/rules/jsx-no-target-blank.js +++ b/lib/rules/jsx-no-target-blank.js @@ -11,7 +11,9 @@ const docsUrl = require('../util/docsUrl'); // ------------------------------------------------------------------------------ function isTargetBlank(attr) { - return attr.name.name === 'target' && + return attr.name && + attr.name.name === 'target' && + attr.value && attr.value.type === 'Literal' && attr.value.value.toLowerCase() === '_blank'; } diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js index 4fbbb1e0d3..eb90cbd6e3 100644 --- a/tests/lib/rules/jsx-no-target-blank.js +++ b/tests/lib/rules/jsx-no-target-blank.js @@ -33,6 +33,7 @@ ruleTester.run('jsx-no-target-blank', rule, { valid: [ {code: ''}, {code: ''}, + {code: ''}, {code: ''}, {code: ''}, {code: 's'}, From e2483659a150bc9b9fe8de80174ca63170b15d6b Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 21 Aug 2018 21:29:48 -0700 Subject: [PATCH 06/49] Support shorthand fragments in Components --- lib/util/Components.js | 7 ++++--- lib/util/jsx.js | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index 1859600f35..5961eaae60 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -11,6 +11,7 @@ const variableUtil = require('./variable'); const pragmaUtil = require('./pragma'); const astUtil = require('./ast'); const propTypes = require('./propTypes'); +const jsxUtil = require('./jsx'); function getId(node) { return node && node.range.join(':'); @@ -349,12 +350,12 @@ function componentRule(rule, context) { const returnsConditionalJSXConsequent = node[property] && node[property].type === 'ConditionalExpression' && - node[property].consequent.type === 'JSXElement' + jsxUtil.isJSX(node[property].consequent) ; const returnsConditionalJSXAlternate = node[property] && node[property].type === 'ConditionalExpression' && - node[property].alternate.type === 'JSXElement' + jsxUtil.isJSX(node[property].alternate) ; const returnsConditionalJSX = strict ? @@ -363,7 +364,7 @@ function componentRule(rule, context) { const returnsJSX = node[property] && - node[property].type === 'JSXElement' + jsxUtil.isJSX(node[property]) ; const returnsReactCreateElement = this.isReactCreateElement(node[property]); diff --git a/lib/util/jsx.js b/lib/util/jsx.js index e6c3e007d6..a1484ef8bb 100644 --- a/lib/util/jsx.js +++ b/lib/util/jsx.js @@ -9,7 +9,7 @@ const COMPAT_TAG_REGEX = /^[a-z]|\-/; /** * Checks if a node represents a DOM element. - * @param {String} node - JSXOpeningElement to check. + * @param {object} node - JSXOpeningElement to check. * @returns {boolean} Whether or not the node corresponds to a DOM element. */ function isDOMComponent(node) { @@ -25,6 +25,16 @@ function isDOMComponent(node) { return COMPAT_TAG_REGEX.test(name); } +/** + * Checks if a node represents a JSX element or fragment. + * @param {object} node - node to check. + * @returns {boolean} Whether or not the node if a JSX element or fragment. + */ +function isJSX(node) { + return ['JSXElement', 'JSXFragment'].indexOf(node.type) >= 0; +} + module.exports = { - isDOMComponent: isDOMComponent + isDOMComponent: isDOMComponent, + isJSX: isJSX }; From 6cf535ad66556dfaa34381bbf63be27b41ee1109 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 21 Aug 2018 22:27:24 -0700 Subject: [PATCH 07/49] `jsx-curly-brace-presence`: support shorthand fragments --- lib/rules/jsx-curly-brace-presence.js | 20 +++++++++----------- tests/lib/rules/jsx-curly-brace-presence.js | 11 +++++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lib/rules/jsx-curly-brace-presence.js b/lib/rules/jsx-curly-brace-presence.js index 88f6fa175a..b609ed2fb4 100644 --- a/lib/rules/jsx-curly-brace-presence.js +++ b/lib/rules/jsx-curly-brace-presence.js @@ -6,6 +6,7 @@ 'use strict'; const docsUrl = require('../util/docsUrl'); +const jsxUtil = require('../util/jsx'); // ------------------------------------------------------------------------------ // Constants @@ -168,13 +169,12 @@ module.exports = { function lintUnnecessaryCurly(JSXExpressionNode) { const expression = JSXExpressionNode.expression; const expressionType = expression.type; - const parentType = JSXExpressionNode.parent.type; if ( (expressionType === 'Literal' || expressionType === 'JSXText') && typeof expression.value === 'string' && !needToEscapeCharacterForJSX(expression.raw) && ( - parentType === 'JSXElement' || + jsxUtil.isJSX(JSXExpressionNode.parent) || !containsQuoteCharacters(expression.value) ) ) { @@ -183,7 +183,7 @@ module.exports = { expressionType === 'TemplateLiteral' && expression.expressions.length === 0 && !needToEscapeCharacterForJSX(expression.quasis[0].value.raw) && ( - parentType === 'JSXElement' || + jsxUtil.isJSX(JSXExpressionNode.parent) || !containsQuoteCharacters(expression.quasis[0].value.cooked) ) ) { @@ -191,24 +191,22 @@ module.exports = { } } - function areRuleConditionsSatisfied(parentType, config, ruleCondition) { + function areRuleConditionsSatisfied(parent, config, ruleCondition) { return ( - parentType === 'JSXAttribute' && + parent.type === 'JSXAttribute' && typeof config.props === 'string' && config.props === ruleCondition ) || ( - parentType === 'JSXElement' && + jsxUtil.isJSX(parent) && typeof config.children === 'string' && config.children === ruleCondition ); } function shouldCheckForUnnecessaryCurly(parent, config) { - const parentType = parent.type; - // If there are more than one JSX child, there is no need to check for // unnecessary curly braces. - if (parentType === 'JSXElement' && parent.children.length !== 1) { + if (jsxUtil.isJSX(parent) && parent.children.length !== 1) { return false; } @@ -220,7 +218,7 @@ module.exports = { return false; } - return areRuleConditionsSatisfied(parentType, config, OPTION_NEVER); + return areRuleConditionsSatisfied(parent, config, OPTION_NEVER); } function shouldCheckForMissingCurly(parent, config) { @@ -232,7 +230,7 @@ module.exports = { return false; } - return areRuleConditionsSatisfied(parent.type, config, OPTION_ALWAYS); + return areRuleConditionsSatisfied(parent, config, OPTION_ALWAYS); } // -------------------------------------------------------------------------- diff --git a/tests/lib/rules/jsx-curly-brace-presence.js b/tests/lib/rules/jsx-curly-brace-presence.js index 59d6563c7d..bfdf422831 100644 --- a/tests/lib/rules/jsx-curly-brace-presence.js +++ b/tests/lib/rules/jsx-curly-brace-presence.js @@ -32,6 +32,10 @@ ruleTester.run('jsx-curly-brace-presence', rule, { { code: 'foo' }, + { + code: '<>foo', + parser: 'babel-eslint' + }, { code: 'foo', options: [{props: 'never'}] @@ -259,6 +263,13 @@ ruleTester.run('jsx-curly-brace-presence', rule, { options: [{children: 'never'}], errors: [{message: unnecessaryCurlyMessage}] }, + { + code: '<>{`foo`}', + output: '<>foo', + parser: 'babel-eslint', + options: [{children: 'never'}], + errors: [{message: unnecessaryCurlyMessage}] + }, { code: `{'foo'}`, output: 'foo', From 74d4251dfa2d5de4691f54ef75e425d1e129ed12 Mon Sep 17 00:00:00 2001 From: Bruno Coelho Date: Wed, 22 Aug 2018 11:05:58 +0100 Subject: [PATCH 08/49] Update jsx-no-target-blank.md --- docs/rules/jsx-no-target-blank.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/jsx-no-target-blank.md b/docs/rules/jsx-no-target-blank.md index c6664d56dd..be20205c2a 100644 --- a/docs/rules/jsx-no-target-blank.md +++ b/docs/rules/jsx-no-target-blank.md @@ -8,7 +8,7 @@ This rules requires that you accompany `target='_blank'` attributes with `rel='n ## Rule Details -This rule aims to prevent user generated links from creating security vulerabilities by requiring +This rule aims to prevent user generated links from creating security vulnerabilities by requiring `rel='noreferrer noopener'` for external links, and optionally any dynamically generated links. ## Rule Options From 2165858534cb1f80a0cda5083450b9aabfe5441a Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Thu, 23 Aug 2018 22:22:50 -0700 Subject: [PATCH 09/49] `jsx-curly-spacing`: support shorthand fragments --- lib/rules/jsx-curly-spacing.js | 1 + tests/lib/rules/jsx-curly-spacing.js | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/rules/jsx-curly-spacing.js b/lib/rules/jsx-curly-spacing.js index 81f11c81b2..f1395e5d07 100644 --- a/lib/rules/jsx-curly-spacing.js +++ b/lib/rules/jsx-curly-spacing.js @@ -331,6 +331,7 @@ module.exports = { break; case 'JSXElement': + case 'JSXFragment': config = childrenConfig; break; diff --git a/tests/lib/rules/jsx-curly-spacing.js b/tests/lib/rules/jsx-curly-spacing.js index 1447b7a487..a76150672a 100644 --- a/tests/lib/rules/jsx-curly-spacing.js +++ b/tests/lib/rules/jsx-curly-spacing.js @@ -677,6 +677,9 @@ ruleTester.run('jsx-curly-spacing', rule, { '`}' ].join('\n'), options: [{children: {when: 'never', allowMultiline: false}}] + }, { + code: '<>{bar} {baz};', + parser: 'babel-eslint' }], invalid: [{ @@ -738,6 +741,16 @@ ruleTester.run('jsx-curly-spacing', rule, { }, { message: 'There should be no space before \'}\'' }] + }, { + code: '<>{ bar };', + output: '<>{bar};', + parser: 'babel-eslint', + options: [{children: true}], + errors: [{ + message: 'There should be no space after \'{\'' + }, { + message: 'There should be no space before \'}\'' + }] }, { code: '{ { bar: true, baz: true } };', output: '{{ bar: true, baz: true }};', From ac7f3f5128e7bf10e1bb99dc172a45fe7580fe84 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Thu, 23 Aug 2018 22:28:34 -0700 Subject: [PATCH 10/49] `jsx-filename-extension`: support shorthand fragments --- lib/rules/jsx-filename-extension.js | 47 ++++++++++++----------- tests/lib/rules/jsx-filename-extension.js | 38 +++++++++++++++--- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/lib/rules/jsx-filename-extension.js b/lib/rules/jsx-filename-extension.js index fecfa88e64..7e7a7c1439 100644 --- a/lib/rules/jsx-filename-extension.js +++ b/lib/rules/jsx-filename-extension.js @@ -43,38 +43,41 @@ module.exports = { }, create: function(context) { + let invalidExtension; + let invalidNode; + function getExtensionsConfig() { return context.options[0] && context.options[0].extensions || DEFAULTS.extensions; } - let invalidExtension; - let invalidNode; + function handleJSX(node) { + const filename = context.getFilename(); + if (filename === '') { + return; + } - // -------------------------------------------------------------------------- - // Public - // -------------------------------------------------------------------------- + if (invalidNode) { + return; + } - return { - JSXElement: function(node) { - const filename = context.getFilename(); - if (filename === '') { - return; - } + const allowedExtensions = getExtensionsConfig(); + const isAllowedExtension = allowedExtensions.some(extension => filename.slice(-extension.length) === extension); - if (invalidNode) { - return; - } + if (isAllowedExtension) { + return; + } - const allowedExtensions = getExtensionsConfig(); - const isAllowedExtension = allowedExtensions.some(extension => filename.slice(-extension.length) === extension); + invalidNode = node; + invalidExtension = path.extname(filename); + } - if (isAllowedExtension) { - return; - } + // -------------------------------------------------------------------------- + // Public + // -------------------------------------------------------------------------- - invalidNode = node; - invalidExtension = path.extname(filename); - }, + return { + JSXElement: handleJSX, + JSXFragment: handleJSX, 'Program:exit': function() { if (!invalidNode) { diff --git a/tests/lib/rules/jsx-filename-extension.js b/tests/lib/rules/jsx-filename-extension.js index 02a2a616a3..485ddfc9c3 100644 --- a/tests/lib/rules/jsx-filename-extension.js +++ b/tests/lib/rules/jsx-filename-extension.js @@ -23,7 +23,8 @@ const parserOptions = { // Code Snippets // ------------------------------------------------------------------------------ -const withJSX = 'module.exports = function MyComponent() { return
\n
\n
; }'; +const withJSXElement = 'module.exports = function MyComponent() { return
\n
\n
; }'; +const withJSXFragment = 'module.exports = function MyComponent() { return <>\n; }'; const withoutJSX = 'module.exports = {}'; // ------------------------------------------------------------------------------ @@ -36,29 +37,54 @@ ruleTester.run('jsx-filename-extension', rule, { valid: [ { filename: '', - code: withJSX + code: withJSXElement }, { filename: 'MyComponent.jsx', - code: withJSX + code: withJSXElement }, { filename: 'MyComponent.js', options: [{extensions: ['.js', '.jsx']}], - code: withJSX + code: withJSXElement }, { filename: 'notAComponent.js', code: withoutJSX + }, { + filename: '', + code: withJSXFragment, + parser: 'babel-eslint' + }, + { + filename: 'MyComponent.jsx', + code: withJSXFragment, + parser: 'babel-eslint' + }, { + filename: 'MyComponent.js', + options: [{extensions: ['.js', '.jsx']}], + code: withJSXFragment, + parser: 'babel-eslint' } ], invalid: [ { filename: 'MyComponent.js', - code: withJSX, + code: withJSXElement, + errors: [{message: 'JSX not allowed in files with extension \'.js\''}] + }, { + filename: 'MyComponent.jsx', + code: withJSXElement, + options: [{extensions: ['.js']}], + errors: [{message: 'JSX not allowed in files with extension \'.jsx\''}] + }, { + filename: 'MyComponent.js', + code: withJSXFragment, + parser: 'babel-eslint', errors: [{message: 'JSX not allowed in files with extension \'.js\''}] }, { filename: 'MyComponent.jsx', - code: withJSX, + code: withJSXFragment, + parser: 'babel-eslint', options: [{extensions: ['.js']}], errors: [{message: 'JSX not allowed in files with extension \'.jsx\''}] } From d537c06b333068250427dc3b41eef59d286e9752 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 24 Aug 2018 17:56:58 -0700 Subject: [PATCH 11/49] `jsx-max-depth`: support shorthand fragments --- lib/rules/jsx-max-depth.js | 41 ++++++++++++------------ tests/lib/rules/jsx-max-depth.js | 53 ++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/lib/rules/jsx-max-depth.js b/lib/rules/jsx-max-depth.js index 9341a6d970..02de21ef03 100644 --- a/lib/rules/jsx-max-depth.js +++ b/lib/rules/jsx-max-depth.js @@ -6,6 +6,7 @@ const has = require('has'); const variableUtil = require('../util/variable'); +const jsxUtil = require('../util/jsx'); const docsUrl = require('../util/docsUrl'); // ------------------------------------------------------------------------------ @@ -39,16 +40,12 @@ module.exports = { const option = context.options[0] || {}; const maxDepth = has(option, 'max') ? option.max : DEFAULT_DEPTH; - function isJSXElement(node) { - return node.type === 'JSXElement'; - } - function isExpression(node) { return node.type === 'JSXExpressionContainer'; } function hasJSX(node) { - return isJSXElement(node) || isExpression(node) && isJSXElement(node.expression); + return jsxUtil.isJSX(node) || isExpression(node) && jsxUtil.isJSX(node.expression); } function isLeaf(node) { @@ -60,9 +57,9 @@ module.exports = { function getDepth(node) { let count = 0; - while (isJSXElement(node.parent) || isExpression(node.parent)) { + while (jsxUtil.isJSX(node.parent) || isExpression(node.parent)) { node = node.parent; - if (isJSXElement(node)) { + if (jsxUtil.isJSX(node)) { count++; } } @@ -82,7 +79,7 @@ module.exports = { }); } - function findJSXElement(variables, name) { + function findJSXElementOrFragment(variables, name) { function find(refs) { let i = refs.length; @@ -90,10 +87,10 @@ module.exports = { if (has(refs[i], 'writeExpr')) { const writeExpr = refs[i].writeExpr; - return isJSXElement(writeExpr) + return jsxUtil.isJSX(writeExpr) && writeExpr || writeExpr.type === 'Identifier' - && findJSXElement(variables, writeExpr.name); + && findJSXElementOrFragment(variables, writeExpr.name); } } @@ -119,24 +116,28 @@ module.exports = { }); } + function handleJSX(node) { + if (!isLeaf(node)) { + return; + } + + const depth = getDepth(node); + if (depth > maxDepth) { + report(node, depth); + } + } + return { - JSXElement: function(node) { - if (!isLeaf(node)) { - return; - } + JSXElement: handleJSX, + JSXFragment: handleJSX, - const depth = getDepth(node); - if (depth > maxDepth) { - report(node, depth); - } - }, JSXExpressionContainer: function(node) { if (node.expression.type !== 'Identifier') { return; } const variables = variableUtil.variablesInScope(context); - const element = findJSXElement(variables, node.expression.name); + const element = findJSXElementOrFragment(variables, node.expression.name); if (element) { const baseDepth = getDepth(node); diff --git a/tests/lib/rules/jsx-max-depth.js b/tests/lib/rules/jsx-max-depth.js index 0b2ceb1a9c..2519b54dbe 100644 --- a/tests/lib/rules/jsx-max-depth.js +++ b/tests/lib/rules/jsx-max-depth.js @@ -61,6 +61,26 @@ ruleTester.run('jsx-max-depth', rule, { }, { code: 'const foo = (x) =>
{x}
;', options: [{max: 2}] + }, { + code: [ + '<>' + ].join('\n'), + parser: 'babel-eslint' + }, { + code: [ + '<>', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint', + options: [{max: 1}] + }, { + code: [ + 'const x = <>x;', + '<>{x}' + ].join('\n'), + parser: 'babel-eslint', + options: [{max: 2}] }], invalid: [{ @@ -121,6 +141,39 @@ ruleTester.run('jsx-max-depth', rule, { '{
}', '
' ].join('\n'), + parser: 'babel-eslint', errors: [{message: 'Expected the depth of nested jsx elements to be <= 2, but found 3.'}] + }, { + code: [ + '<>', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint', + options: [{max: 0}], + errors: [{message: 'Expected the depth of nested jsx elements to be <= 0, but found 1.'}] + }, { + code: [ + '<>', + ' <>', + ' ', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint', + options: [{max: 1}], + errors: [{message: 'Expected the depth of nested jsx elements to be <= 1, but found 2.'}] + }, { + code: [ + 'const x = <>;', + 'let y = x;', + '<>{x}-{y}' + ].join('\n'), + parser: 'babel-eslint', + options: [{max: 1}], + errors: [ + {message: 'Expected the depth of nested jsx elements to be <= 1, but found 2.'}, + {message: 'Expected the depth of nested jsx elements to be <= 1, but found 2.'} + ] }] }); From 83eb3072f00c386c3649a2db599bcb546e3a68e5 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 25 Aug 2018 23:11:18 -0700 Subject: [PATCH 12/49] `jsx-one-expression-per-line`: support shorthand fragments --- lib/rules/jsx-one-expression-per-line.js | 269 +++++++++--------- .../lib/rules/jsx-one-expression-per-line.js | 58 ++++ 2 files changed, 194 insertions(+), 133 deletions(-) diff --git a/lib/rules/jsx-one-expression-per-line.js b/lib/rules/jsx-one-expression-per-line.js index cc14ad8323..b9bacdacee 100644 --- a/lib/rules/jsx-one-expression-per-line.js +++ b/lib/rules/jsx-one-expression-per-line.js @@ -50,171 +50,174 @@ module.exports = { return n.openingElement ? n.openingElement.name.name : sourceCode.getText(n).replace(/\n/g, ''); } - return { - JSXElement: function (node) { - const children = node.children; - - if (!children || !children.length) { - return; - } + function handleJSX(node) { + const children = node.children; - const openingElement = node.openingElement; - const closingElement = node.closingElement; - const openingElementStartLine = openingElement.loc.start.line; - const openingElementEndLine = openingElement.loc.end.line; - const closingElementStartLine = closingElement.loc.start.line; - const closingElementEndLine = closingElement.loc.end.line; + if (!children || !children.length) { + return; + } - if (children.length === 1) { - const child = children[0]; + const openingElement = node.openingElement || node.openingFragment; + const closingElement = node.closingElement || node.closingFragment; + const openingElementStartLine = openingElement.loc.start.line; + const openingElementEndLine = openingElement.loc.end.line; + const closingElementStartLine = closingElement.loc.start.line; + const closingElementEndLine = closingElement.loc.end.line; + + if (children.length === 1) { + const child = children[0]; + if ( + openingElementStartLine === openingElementEndLine && + openingElementEndLine === closingElementStartLine && + closingElementStartLine === closingElementEndLine && + closingElementEndLine === child.loc.start.line && + child.loc.start.line === child.loc.end.line + ) { if ( - openingElementStartLine === openingElementEndLine && - openingElementEndLine === closingElementStartLine && - closingElementStartLine === closingElementEndLine && - closingElementEndLine === child.loc.start.line && - child.loc.start.line === child.loc.end.line + options.allow === 'single-child' || + options.allow === 'literal' && (child.type === 'Literal' || child.type === 'JSXText') ) { - if ( - options.allow === 'single-child' || - options.allow === 'literal' && (child.type === 'Literal' || child.type === 'JSXText') - ) { - return; - } + return; } } + } - const childrenGroupedByLine = {}; - const fixDetailsByNode = {}; + const childrenGroupedByLine = {}; + const fixDetailsByNode = {}; - children.forEach(child => { - let countNewLinesBeforeContent = 0; - let countNewLinesAfterContent = 0; + children.forEach(child => { + let countNewLinesBeforeContent = 0; + let countNewLinesAfterContent = 0; - if (child.type === 'Literal' || child.type === 'JSXText') { - if (/^\s*$/.test(child.raw)) { - return; - } + if (child.type === 'Literal' || child.type === 'JSXText') { + if (/^\s*$/.test(child.raw)) { + return; + } + + countNewLinesBeforeContent = (child.raw.match(/^ *\n/g) || []).length; + countNewLinesAfterContent = (child.raw.match(/\n *$/g) || []).length; + } + + const startLine = child.loc.start.line + countNewLinesBeforeContent; + const endLine = child.loc.end.line - countNewLinesAfterContent; - countNewLinesBeforeContent = (child.raw.match(/^ *\n/g) || []).length; - countNewLinesAfterContent = (child.raw.match(/\n *$/g) || []).length; + if (startLine === endLine) { + if (!childrenGroupedByLine[startLine]) { + childrenGroupedByLine[startLine] = []; } + childrenGroupedByLine[startLine].push(child); + } else { + if (!childrenGroupedByLine[startLine]) { + childrenGroupedByLine[startLine] = []; + } + childrenGroupedByLine[startLine].push(child); + if (!childrenGroupedByLine[endLine]) { + childrenGroupedByLine[endLine] = []; + } + childrenGroupedByLine[endLine].push(child); + } + }); - const startLine = child.loc.start.line + countNewLinesBeforeContent; - const endLine = child.loc.end.line - countNewLinesAfterContent; + Object.keys(childrenGroupedByLine).forEach(_line => { + const line = parseInt(_line, 10); + const firstIndex = 0; + const lastIndex = childrenGroupedByLine[line].length - 1; - if (startLine === endLine) { - if (!childrenGroupedByLine[startLine]) { - childrenGroupedByLine[startLine] = []; + childrenGroupedByLine[line].forEach((child, i) => { + let prevChild; + let nextChild; + + if (i === firstIndex) { + if (line === openingElementEndLine) { + prevChild = openingElement; } - childrenGroupedByLine[startLine].push(child); } else { - if (!childrenGroupedByLine[startLine]) { - childrenGroupedByLine[startLine] = []; - } - childrenGroupedByLine[startLine].push(child); - if (!childrenGroupedByLine[endLine]) { - childrenGroupedByLine[endLine] = []; - } - childrenGroupedByLine[endLine].push(child); + prevChild = childrenGroupedByLine[line][i - 1]; } - }); - - Object.keys(childrenGroupedByLine).forEach(_line => { - const line = parseInt(_line, 10); - const firstIndex = 0; - const lastIndex = childrenGroupedByLine[line].length - 1; - - childrenGroupedByLine[line].forEach((child, i) => { - let prevChild; - let nextChild; - - if (i === firstIndex) { - if (line === openingElementEndLine) { - prevChild = openingElement; - } - } else { - prevChild = childrenGroupedByLine[line][i - 1]; - } - if (i === lastIndex) { - if (line === closingElementStartLine) { - nextChild = closingElement; - } - } else { - // We don't need to append a trailing because the next child will prepend a leading. - // nextChild = childrenGroupedByLine[line][i + 1]; + if (i === lastIndex) { + if (line === closingElementStartLine) { + nextChild = closingElement; } + } else { + // We don't need to append a trailing because the next child will prepend a leading. + // nextChild = childrenGroupedByLine[line][i + 1]; + } - function spaceBetweenPrev () { - return ((prevChild.type === 'Literal' || prevChild.type === 'JSXText') && / $/.test(prevChild.raw)) || - ((child.type === 'Literal' || child.type === 'JSXText') && /^ /.test(child.raw)) || - sourceCode.isSpaceBetweenTokens(prevChild, child); - } + function spaceBetweenPrev () { + return ((prevChild.type === 'Literal' || prevChild.type === 'JSXText') && / $/.test(prevChild.raw)) || + ((child.type === 'Literal' || child.type === 'JSXText') && /^ /.test(child.raw)) || + sourceCode.isSpaceBetweenTokens(prevChild, child); + } - function spaceBetweenNext () { - return ((nextChild.type === 'Literal' || nextChild.type === 'JSXText') && /^ /.test(nextChild.raw)) || - ((child.type === 'Literal' || child.type === 'JSXText') && / $/.test(child.raw)) || - sourceCode.isSpaceBetweenTokens(child, nextChild); - } + function spaceBetweenNext () { + return ((nextChild.type === 'Literal' || nextChild.type === 'JSXText') && /^ /.test(nextChild.raw)) || + ((child.type === 'Literal' || child.type === 'JSXText') && / $/.test(child.raw)) || + sourceCode.isSpaceBetweenTokens(child, nextChild); + } - if (!prevChild && !nextChild) { - return; - } + if (!prevChild && !nextChild) { + return; + } - const source = sourceCode.getText(child); - const leadingSpace = !!(prevChild && spaceBetweenPrev()); - const trailingSpace = !!(nextChild && spaceBetweenNext()); - const leadingNewLine = !!prevChild; - const trailingNewLine = !!nextChild; + const source = sourceCode.getText(child); + const leadingSpace = !!(prevChild && spaceBetweenPrev()); + const trailingSpace = !!(nextChild && spaceBetweenNext()); + const leadingNewLine = !!prevChild; + const trailingNewLine = !!nextChild; - const key = nodeKey(child); + const key = nodeKey(child); - if (!fixDetailsByNode[key]) { - fixDetailsByNode[key] = { - node: child, - source: source, - descriptor: nodeDescriptor(child) - }; - } + if (!fixDetailsByNode[key]) { + fixDetailsByNode[key] = { + node: child, + source: source, + descriptor: nodeDescriptor(child) + }; + } - if (leadingSpace) { - fixDetailsByNode[key].leadingSpace = true; - } - if (leadingNewLine) { - fixDetailsByNode[key].leadingNewLine = true; - } - if (trailingNewLine) { - fixDetailsByNode[key].trailingNewLine = true; - } - if (trailingSpace) { - fixDetailsByNode[key].trailingSpace = true; - } - }); + if (leadingSpace) { + fixDetailsByNode[key].leadingSpace = true; + } + if (leadingNewLine) { + fixDetailsByNode[key].leadingNewLine = true; + } + if (trailingNewLine) { + fixDetailsByNode[key].trailingNewLine = true; + } + if (trailingSpace) { + fixDetailsByNode[key].trailingSpace = true; + } }); + }); - Object.keys(fixDetailsByNode).forEach(key => { - const details = fixDetailsByNode[key]; + Object.keys(fixDetailsByNode).forEach(key => { + const details = fixDetailsByNode[key]; - const nodeToReport = details.node; - const descriptor = details.descriptor; - const source = details.source.replace(/(^ +| +(?=\n)*$)/g, ''); + const nodeToReport = details.node; + const descriptor = details.descriptor; + const source = details.source.replace(/(^ +| +(?=\n)*$)/g, ''); - const leadingSpaceString = details.leadingSpace ? '\n{\' \'}' : ''; - const trailingSpaceString = details.trailingSpace ? '{\' \'}\n' : ''; - const leadingNewLineString = details.leadingNewLine ? '\n' : ''; - const trailingNewLineString = details.trailingNewLine ? '\n' : ''; + const leadingSpaceString = details.leadingSpace ? '\n{\' \'}' : ''; + const trailingSpaceString = details.trailingSpace ? '{\' \'}\n' : ''; + const leadingNewLineString = details.leadingNewLine ? '\n' : ''; + const trailingNewLineString = details.trailingNewLine ? '\n' : ''; - const replaceText = `${leadingSpaceString}${leadingNewLineString}${source}${trailingNewLineString}${trailingSpaceString}`; + const replaceText = `${leadingSpaceString}${leadingNewLineString}${source}${trailingNewLineString}${trailingSpaceString}`; - context.report({ - node: nodeToReport, - message: `\`${descriptor}\` must be placed on a new line`, - fix: function (fixer) { - return fixer.replaceText(nodeToReport, replaceText); - } - }); + context.report({ + node: nodeToReport, + message: `\`${descriptor}\` must be placed on a new line`, + fix: function (fixer) { + return fixer.replaceText(nodeToReport, replaceText); + } }); - } + }); + } + + return { + JSXElement: handleJSX, + JSXFragment: handleJSX }; } }; diff --git a/tests/lib/rules/jsx-one-expression-per-line.js b/tests/lib/rules/jsx-one-expression-per-line.js index 0ca57b8882..9e653e8542 100644 --- a/tests/lib/rules/jsx-one-expression-per-line.js +++ b/tests/lib/rules/jsx-one-expression-per-line.js @@ -102,6 +102,24 @@ ruleTester.run('jsx-one-expression-per-line', rule, { }, { code: '', options: [{allow: 'single-child'}] + }, { + code: '<>', + parser: 'babel-eslint' + }, { + code: [ + '<>', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint' + }, { + code: [ + '<>', + ' ', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint' }], invalid: [{ @@ -949,5 +967,45 @@ ruleTester.run('jsx-one-expression-per-line', rule, { '' ].join('\n'), errors: [{message: '`foobar` must be placed on a new line'}] + }, { + code: '<>{"foo"}', + output: [ + '<>', + '{"foo"}', + '' + ].join('\n'), + errors: [{message: '`{"foo"}` must be placed on a new line'}], + parser: 'babel-eslint', + parserOptions: parserOptions + }, { + code: [ + '', + ' <>', + '' + ].join('\n'), + output: [ + '', + ' ', + '<>', + '' + ].join('\n'), + errors: [{message: '`<>` must be placed on a new line'}], + parser: 'babel-eslint', + parserOptions: parserOptions + }, { + code: [ + '<', + '>', + '' + ].join('\n'), + output: [ + '<', + '>', + '', + '' + ].join('\n'), + errors: [{message: '`Foo` must be placed on a new line'}], + parser: 'babel-eslint', + parserOptions: parserOptions }] }); From bace62ccc0f08e4b8e47afea1ed65e9d85b6a709 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sun, 26 Aug 2018 23:24:46 -0700 Subject: [PATCH 13/49] `no-unescaped-entities`: support shorthand fragments --- lib/rules/no-unescaped-entities.js | 3 +- tests/lib/rules/no-unescaped-entities.js | 61 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-unescaped-entities.js b/lib/rules/no-unescaped-entities.js index db945e859a..1fa88d1f5e 100644 --- a/lib/rules/no-unescaped-entities.js +++ b/lib/rules/no-unescaped-entities.js @@ -5,6 +5,7 @@ 'use strict'; const docsUrl = require('../util/docsUrl'); +const jsxUtil = require('../util/jsx'); // ------------------------------------------------------------------------------ // Rule Definition @@ -72,7 +73,7 @@ module.exports = { return { 'Literal, JSXText': function(node) { - if (node.parent.type === 'JSXElement') { + if (jsxUtil.isJSX(node.parent)) { reportInvalidEntity(node); } } diff --git a/tests/lib/rules/no-unescaped-entities.js b/tests/lib/rules/no-unescaped-entities.js index 61e3a9e0a4..aff01dca5b 100644 --- a/tests/lib/rules/no-unescaped-entities.js +++ b/tests/lib/rules/no-unescaped-entities.js @@ -71,6 +71,35 @@ ruleTester.run('no-unescaped-entities', rule, { }, }); ` + }, + { + code: ` + var Hello = createReactClass({ + render: function() { + return <>Here is some text!; + } + }); + `, + parser: 'babel-eslint' + }, { + code: ` + var Hello = createReactClass({ + render: function() { + return <>I’ve escaped some entities: > < &; + } + }); + `, + parser: 'babel-eslint' + }, + { + code: ` + var Hello = createReactClass({ + render: function() { + return <>{">" + "<" + "&" + '"'}; + }, + }); + `, + parser: 'babel-eslint' } ], @@ -84,6 +113,16 @@ ruleTester.run('no-unescaped-entities', rule, { }); `, errors: [{message: 'HTML entities must be escaped.'}] + }, { + code: ` + var Hello = createReactClass({ + render: function() { + return <>>; + } + }); + `, + parser: 'babel-eslint', + errors: [{message: 'HTML entities must be escaped.'}] }, { code: ` var Hello = createReactClass({ @@ -95,6 +134,18 @@ ruleTester.run('no-unescaped-entities', rule, { }); `, errors: [{message: 'HTML entities must be escaped.'}] + }, { + code: ` + var Hello = createReactClass({ + render: function() { + return <>first line is ok + so is second + and here are some bad entities: > + } + }); + `, + parser: 'babel-eslint', + errors: [{message: 'HTML entities must be escaped.'}] }, { code: ` var Hello = createReactClass({ @@ -126,6 +177,16 @@ ruleTester.run('no-unescaped-entities', rule, { }); `, errors: [{message: 'HTML entities must be escaped.'}] + }, { + code: ` + var Hello = createReactClass({ + render: function() { + return <>{"Unbalanced braces"}}; + } + }); + `, + parser: 'babel-eslint', + errors: [{message: 'HTML entities must be escaped.'}] } ] }); From 2899e57bc147641e2642f8d096d2ab612a32232a Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Mon, 27 Aug 2018 08:39:44 -0700 Subject: [PATCH 14/49] `jsx-wrap-multilines`: support shorthand fragments --- lib/rules/jsx-wrap-multilines.js | 3 +- tests/lib/rules/jsx-wrap-multilines.js | 425 +++++++++++++++++++++++++ 2 files changed, 427 insertions(+), 1 deletion(-) diff --git a/lib/rules/jsx-wrap-multilines.js b/lib/rules/jsx-wrap-multilines.js index ca9b980d62..32d3d08113 100644 --- a/lib/rules/jsx-wrap-multilines.js +++ b/lib/rules/jsx-wrap-multilines.js @@ -6,6 +6,7 @@ const has = require('has'); const docsUrl = require('../util/docsUrl'); +const jsxUtil = require('../util/jsx'); // ------------------------------------------------------------------------------ // Constants @@ -122,7 +123,7 @@ module.exports = { } function check(node, type) { - if (!node || node.type !== 'JSXElement') { + if (!node || !jsxUtil.isJSX(node)) { return; } diff --git a/tests/lib/rules/jsx-wrap-multilines.js b/tests/lib/rules/jsx-wrap-multilines.js index 450ee782e6..0351df1831 100644 --- a/tests/lib/rules/jsx-wrap-multilines.js +++ b/tests/lib/rules/jsx-wrap-multilines.js @@ -44,6 +44,16 @@ const RETURN_PAREN = ` }); `; +const RETURN_PAREN_FRAGMENT = ` + var Hello = createReactClass({ + render: function() { + return (<> +

Hello {this.props.name}

+ ); + } + }); +`; + const RETURN_NO_PAREN = ` var Hello = createReactClass({ render: function() { @@ -54,6 +64,16 @@ const RETURN_NO_PAREN = ` }); `; +const RETURN_NO_PAREN_FRAGMENT = ` + var Hello = createReactClass({ + render: function() { + return <> +

Hello {this.props.name}

+ ; + } + }); +`; + const RETURN_PAREN_NEW_LINE = ` var Hello = createReactClass({ render: function() { @@ -66,8 +86,30 @@ const RETURN_PAREN_NEW_LINE = ` }); `; +const RETURN_PAREN_NEW_LINE_FRAGMENT = ` + var Hello = createReactClass({ + render: function() { + return ( + <> +

Hello {this.props.name}

+ + ); + } + }); +`; + +const RETURN_SINGLE_LINE_FRAGMENT = ` + var Hello = createReactClass({ + render: function() { + return <>Hello {this.props.name}; + } + }); +`; + const DECLARATION_TERNARY_SINGLE_LINE = 'var hello = foo ?

Hello

:

Hi

;'; +const DECLARATION_TERNARY_SINGLE_LINE_FRAGMENT = 'var hello = foo ? <>Hello : <>Hi;'; + const DECLARATION_TERNARY_PAREN = ` var hello = foo ? (

Hello

@@ -76,6 +118,14 @@ const DECLARATION_TERNARY_PAREN = `
); `; +const DECLARATION_TERNARY_PAREN_FRAGMENT = ` + var hello = foo ? (<> +

Hello

+ ) : (<> +

Hi

+ ); +`; + const DECLARATION_TERNARY_NO_PAREN = ` var hello = foo ?

Hello

@@ -84,6 +134,14 @@ const DECLARATION_TERNARY_NO_PAREN = `
; `; +const DECLARATION_TERNARY_NO_PAREN_FRAGMENT = ` + var hello = foo ? <> +

Hello

+ : <> +

Hi

+ ; +`; + const DECLARATION_TERNARY_PAREN_NEW_LINE = ` var hello = foo ? (
@@ -107,6 +165,15 @@ const ASSIGNMENT_TERNARY_PAREN = `
); `; +const ASSIGNMENT_TERNARY_PAREN_FRAGMENT = ` + var hello; + hello = foo ? (<> +

Hello

+ ) : (<> +

Hi

+ ); +`; + const ASSIGNMENT_TERNARY_NO_PAREN = ` var hello; hello = foo ?
@@ -116,6 +183,15 @@ const ASSIGNMENT_TERNARY_NO_PAREN = `
; `; +const ASSIGNMENT_TERNARY_NO_PAREN_FRAGMENT = ` + var hello; + hello = foo ? <> +

Hello

+ : <> +

Hi

+ ; +`; + const ASSIGNMENT_TERNARY_PAREN_NEW_LINE = ` var hello; hello = foo ? ( @@ -137,12 +213,24 @@ const DECLARATION_PAREN = `
); `; +const DECLARATION_PAREN_FRAGMENT = ` + var hello = (<> +

Hello

+ ); +`; + const DECLARATION_NO_PAREN = ` var hello =

Hello

; `; +const DECLARATION_NO_PAREN_FRAGMENT = ` + var hello = <> +

Hello

+ ; +`; + const DECLARATION_PAREN_NEW_LINE = ` var hello = (
@@ -160,6 +248,13 @@ const ASSIGNMENT_PAREN = `
); `; +const ASSIGNMENT_PAREN_FRAGMENT = ` + var hello; + hello = (<> +

Hello

+ ); +`; + const ASSIGNMENT_NO_PAREN = ` var hello; hello =
@@ -167,6 +262,13 @@ const ASSIGNMENT_NO_PAREN = `
; `; +const ASSIGNMENT_NO_PAREN_FRAGMENT = ` + var hello; + hello = <> +

Hello

+ ; +`; + const ASSIGNMENT_PAREN_NEW_LINE = ` var hello; hello = ( @@ -184,12 +286,24 @@ const ARROW_PAREN = `
); `; +const ARROW_PAREN_FRAGMENT = ` + var hello = () => (<> +

Hello

+ ); +`; + const ARROW_NO_PAREN = ` var hello = () =>

Hello

; `; +const ARROW_NO_PAREN_FRAGMENT = ` + var hello = () => <> +

Hello

+ ; +`; + const ARROW_PAREN_NEW_LINE = ` var hello = () => (
@@ -208,6 +322,14 @@ const CONDITION_PAREN = `
`; +const CONDITION_PAREN_FRAGMENT = ` +
+ {foo ? (<> +

Hello

+ ) : null} +
+`; + const CONDITION_NO_PAREN = `
{foo ?
@@ -216,6 +338,14 @@ const CONDITION_NO_PAREN = `
`; +const CONDITION_NO_PAREN_FRAGMENT = ` +
+ {foo ? <> +

Hello

+ : null} +
+`; + const CONDITION_PAREN_NEW_LINE = `
{foo ? ( @@ -238,6 +368,16 @@ const LOGICAL_PAREN = `
`; +const LOGICAL_PAREN_FRAGMENT = ` +
+ {foo && + (<> +

Hello World

+ ) + } +
+`; + const LOGICAL_NO_PAREN = `
{foo && @@ -248,6 +388,16 @@ const LOGICAL_NO_PAREN = `
`; +const LOGICAL_NO_PAREN_FRAGMENT = ` +
+ {foo && + <> +

Hello World

+ + } +
+`; + const LOGICAL_PAREN_NEW_LINE_AUTOFIX = `
{foo && ( @@ -258,6 +408,16 @@ const LOGICAL_PAREN_NEW_LINE_AUTOFIX = `
`; +const LOGICAL_PAREN_NEW_LINE_AUTOFIX_FRAGMENT = ` +
+ {foo && ( +<> +

Hello World

+ +)} +
+`; + const LOGICAL_PAREN_NEW_LINE = `
{foo && ( @@ -280,6 +440,16 @@ const ATTR_PAREN = `
`; +const ATTR_PAREN_FRAGMENT = ` +
+

Hello

+ ) + }> +

Hello

+
+`; + const ATTR_NO_PAREN = `
@@ -290,6 +460,16 @@ const ATTR_NO_PAREN = `
`; +const ATTR_NO_PAREN_FRAGMENT = ` +
+

Hello

+ + }> +

Hello

+
+`; + const ATTR_PAREN_NEW_LINE = `
@@ -310,6 +490,16 @@ const ATTR_PAREN_NEW_LINE_AUTOFIX = `
`; +const ATTR_PAREN_NEW_LINE_AUTOFIX_FRAGMENT = ` +
+

Hello

+ +)}> +

Hello

+
+`; + function addNewLineSymbols(code) { return code.replace(/\(\)/g, '>\n)'); } @@ -324,11 +514,21 @@ ruleTester.run('jsx-wrap-multilines', rule, { valid: [ { code: RETURN_SINGLE_LINE + }, { + code: RETURN_SINGLE_LINE_FRAGMENT, + parser: 'babel-eslint' }, { code: RETURN_PAREN + }, { + code: RETURN_PAREN, + parser: 'babel-eslint' }, { code: RETURN_SINGLE_LINE, options: [{return: true}] + }, { + code: RETURN_SINGLE_LINE_FRAGMENT, + parser: 'babel-eslint', + options: [{return: true}] }, { code: RETURN_PAREN, options: [{return: true}] @@ -340,11 +540,18 @@ ruleTester.run('jsx-wrap-multilines', rule, { options: [{return: false}] }, { code: DECLARATION_TERNARY_SINGLE_LINE + }, { + code: DECLARATION_TERNARY_SINGLE_LINE_FRAGMENT, + parser: 'babel-eslint' }, { code: DECLARATION_TERNARY_PAREN }, { code: DECLARATION_TERNARY_SINGLE_LINE, options: [{declaration: true}] + }, { + code: DECLARATION_TERNARY_SINGLE_LINE, + parser: 'babel-eslint', + options: [{declaration: true}] }, { code: DECLARATION_TERNARY_PAREN, options: [{declaration: true}] @@ -374,6 +581,9 @@ ruleTester.run('jsx-wrap-multilines', rule, { code: DECLARATION_SINGLE_LINE }, { code: DECLARATION_PAREN + }, { + code: DECLARATION_PAREN_FRAGMENT, + parser: 'babel-eslint' }, { code: DECLARATION_SINGLE_LINE, options: [{declaration: true}] @@ -383,6 +593,10 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: DECLARATION_NO_PAREN, options: [{declaration: 'ignore'}] + }, { + code: DECLARATION_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + options: [{declaration: 'ignore'}] }, { code: DECLARATION_NO_PAREN, options: [{declaration: false}] @@ -394,28 +608,46 @@ ruleTester.run('jsx-wrap-multilines', rule, { options: [{declaration: false}] }, { code: ASSIGNMENT_PAREN + }, { + code: ASSIGNMENT_PAREN_FRAGMENT, + parser: 'babel-eslint' }, { code: ASSIGNMENT_PAREN, options: [{assignment: true}] }, { code: ASSIGNMENT_NO_PAREN, options: [{assignment: 'ignore'}] + }, { + code: ASSIGNMENT_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + options: [{assignment: 'ignore'}] }, { code: ASSIGNMENT_NO_PAREN, options: [{assignment: false}] }, { code: ARROW_PAREN + }, { + code: ARROW_PAREN_FRAGMENT, + parser: 'babel-eslint' }, { code: ARROW_SINGLE_LINE }, { code: ARROW_PAREN, options: [{arrow: true}] + }, { + code: ARROW_PAREN, + parser: 'babel-eslint', + options: [{arrow: true}] }, { code: ARROW_SINGLE_LINE, options: [{arrow: true}] }, { code: ARROW_NO_PAREN, options: [{arrow: 'ignore'}] + }, { + code: ARROW_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + options: [{arrow: 'ignore'}] }, { code: ARROW_NO_PAREN, options: [{arrow: false}] @@ -429,6 +661,10 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: CONDITION_PAREN, options: [{condition: true}] + }, { + code: CONDITION_PAREN_FRAGMENT, + parser: 'babel-eslint', + options: [{condition: true}] }, { code: LOGICAL_SINGLE_LINE }, { @@ -436,6 +672,10 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: LOGICAL_PAREN, options: [{logical: true}] + }, { + code: LOGICAL_PAREN_FRAGMENT, + parser: 'babel-eslint', + options: [{logical: true}] }, { code: ATTR_SINGLE_LINE }, { @@ -443,9 +683,17 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: ATTR_PAREN, options: [{prop: true}] + }, { + code: ATTR_PAREN_FRAGMENT, + parser: 'babel-eslint', + options: [{prop: true}] }, { code: RETURN_PAREN_NEW_LINE, options: [{return: 'parens-new-line'}] + }, { + code: RETURN_PAREN_NEW_LINE_FRAGMENT, + parser: 'babel-eslint', + options: [{return: 'parens-new-line'}] }, { code: DECLARATION_TERNARY_PAREN_NEW_LINE, options: [{declaration: 'parens-new-line'}] @@ -478,11 +726,22 @@ ruleTester.run('jsx-wrap-multilines', rule, { code: RETURN_NO_PAREN, output: RETURN_PAREN, errors: [{message: MISSING_PARENS}] + }, { + code: RETURN_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: RETURN_PAREN_FRAGMENT, + errors: [{message: MISSING_PARENS}] }, { code: RETURN_NO_PAREN, output: RETURN_PAREN, options: [{return: true}], errors: [{message: MISSING_PARENS}] + }, { + code: RETURN_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: RETURN_PAREN_FRAGMENT, + options: [{return: true}], + errors: [{message: MISSING_PARENS}] }, { code: DECLARATION_TERNARY_NO_PAREN, output: DECLARATION_TERNARY_PAREN, @@ -490,6 +749,14 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: MISSING_PARENS}, {message: MISSING_PARENS} ] + }, { + code: DECLARATION_TERNARY_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: DECLARATION_TERNARY_PAREN_FRAGMENT, + errors: [ + {message: MISSING_PARENS}, + {message: MISSING_PARENS} + ] }, { code: DECLARATION_TERNARY_NO_PAREN, output: DECLARATION_TERNARY_PAREN, @@ -498,6 +765,15 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: MISSING_PARENS}, {message: MISSING_PARENS} ] + }, { + code: DECLARATION_TERNARY_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: DECLARATION_TERNARY_PAREN_FRAGMENT, + options: [{declaration: true}], + errors: [ + {message: MISSING_PARENS}, + {message: MISSING_PARENS} + ] }, { code: ASSIGNMENT_TERNARY_NO_PAREN, output: ASSIGNMENT_TERNARY_PAREN, @@ -505,6 +781,14 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: MISSING_PARENS}, {message: MISSING_PARENS} ] + }, { + code: ASSIGNMENT_TERNARY_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: ASSIGNMENT_TERNARY_PAREN_FRAGMENT, + errors: [ + {message: MISSING_PARENS}, + {message: MISSING_PARENS} + ] }, { code: ASSIGNMENT_TERNARY_NO_PAREN, output: ASSIGNMENT_TERNARY_PAREN, @@ -513,10 +797,24 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: MISSING_PARENS}, {message: MISSING_PARENS} ] + }, { + code: ASSIGNMENT_TERNARY_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: ASSIGNMENT_TERNARY_PAREN_FRAGMENT, + options: [{assignment: true}], + errors: [ + {message: MISSING_PARENS}, + {message: MISSING_PARENS} + ] }, { code: DECLARATION_NO_PAREN, output: DECLARATION_PAREN, errors: [{message: MISSING_PARENS}] + }, { + code: DECLARATION_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: DECLARATION_PAREN_FRAGMENT, + errors: [{message: MISSING_PARENS}] }, { code: DECLARATION_NO_PAREN, output: DECLARATION_PAREN, @@ -526,6 +824,11 @@ ruleTester.run('jsx-wrap-multilines', rule, { code: ASSIGNMENT_NO_PAREN, output: ASSIGNMENT_PAREN, errors: [{message: MISSING_PARENS}] + }, { + code: ASSIGNMENT_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: ASSIGNMENT_PAREN_FRAGMENT, + errors: [{message: MISSING_PARENS}] }, { code: ASSIGNMENT_NO_PAREN, output: ASSIGNMENT_PAREN, @@ -535,6 +838,11 @@ ruleTester.run('jsx-wrap-multilines', rule, { code: ARROW_NO_PAREN, output: ARROW_PAREN, errors: [{message: MISSING_PARENS}] + }, { + code: ARROW_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: ARROW_PAREN_FRAGMENT, + errors: [{message: MISSING_PARENS}] }, { code: ARROW_NO_PAREN, output: ARROW_PAREN, @@ -545,6 +853,12 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: CONDITION_PAREN, options: [{condition: 'parens'}], errors: [{message: MISSING_PARENS}] + }, { + code: CONDITION_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: CONDITION_PAREN_FRAGMENT, + options: [{condition: 'parens'}], + errors: [{message: MISSING_PARENS}] }, { code: CONDITION_NO_PAREN, output: CONDITION_PAREN, @@ -555,6 +869,12 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: LOGICAL_PAREN, options: [{logical: 'parens'}], errors: [{message: MISSING_PARENS}] + }, { + code: LOGICAL_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: LOGICAL_PAREN_FRAGMENT, + options: [{logical: 'parens'}], + errors: [{message: MISSING_PARENS}] }, { code: LOGICAL_NO_PAREN, output: LOGICAL_PAREN, @@ -565,6 +885,12 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: ATTR_PAREN, options: [{prop: 'parens'}], errors: [{message: MISSING_PARENS}] + }, { + code: ATTR_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: ATTR_PAREN_FRAGMENT, + options: [{prop: 'parens'}], + errors: [{message: MISSING_PARENS}] }, { code: ATTR_NO_PAREN, output: ATTR_PAREN, @@ -575,11 +901,23 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: addNewLineSymbols(RETURN_PAREN), options: [{return: 'parens-new-line'}], errors: [{message: MISSING_PARENS}] + }, { + code: RETURN_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(RETURN_PAREN_FRAGMENT), + options: [{return: 'parens-new-line'}], + errors: [{message: MISSING_PARENS}] }, { code: RETURN_PAREN, output: addNewLineSymbols(RETURN_PAREN), options: [{return: 'parens-new-line'}], errors: [{message: PARENS_NEW_LINES}] + }, { + code: RETURN_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(RETURN_PAREN_FRAGMENT), + options: [{return: 'parens-new-line'}], + errors: [{message: PARENS_NEW_LINES}] }, { code: DECLARATION_TERNARY_NO_PAREN, output: addNewLineSymbols(DECLARATION_TERNARY_PAREN), @@ -588,6 +926,24 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: MISSING_PARENS}, {message: MISSING_PARENS} ] + }, { + code: DECLARATION_TERNARY_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(DECLARATION_TERNARY_PAREN_FRAGMENT), + options: [{declaration: 'parens-new-line'}], + errors: [ + {message: MISSING_PARENS}, + {message: MISSING_PARENS} + ] + }, { + code: DECLARATION_TERNARY_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(DECLARATION_TERNARY_PAREN_FRAGMENT), + options: [{declaration: 'parens-new-line'}], + errors: [ + {message: PARENS_NEW_LINES}, + {message: PARENS_NEW_LINES} + ] }, { code: DECLARATION_TERNARY_PAREN, output: addNewLineSymbols(DECLARATION_TERNARY_PAREN), @@ -596,6 +952,15 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: PARENS_NEW_LINES}, {message: PARENS_NEW_LINES} ] + }, { + code: DECLARATION_TERNARY_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(DECLARATION_TERNARY_PAREN_FRAGMENT), + options: [{declaration: 'parens-new-line'}], + errors: [ + {message: PARENS_NEW_LINES}, + {message: PARENS_NEW_LINES} + ] }, { code: ASSIGNMENT_TERNARY_NO_PAREN, output: addNewLineSymbols(ASSIGNMENT_TERNARY_PAREN), @@ -604,6 +969,15 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: MISSING_PARENS}, {message: MISSING_PARENS} ] + }, { + code: ASSIGNMENT_TERNARY_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(ASSIGNMENT_TERNARY_PAREN_FRAGMENT), + options: [{assignment: 'parens-new-line'}], + errors: [ + {message: MISSING_PARENS}, + {message: MISSING_PARENS} + ] }, { code: ASSIGNMENT_TERNARY_PAREN, output: addNewLineSymbols(ASSIGNMENT_TERNARY_PAREN), @@ -612,6 +986,15 @@ ruleTester.run('jsx-wrap-multilines', rule, { {message: PARENS_NEW_LINES}, {message: PARENS_NEW_LINES} ] + }, { + code: ASSIGNMENT_TERNARY_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(ASSIGNMENT_TERNARY_PAREN_FRAGMENT), + options: [{assignment: 'parens-new-line'}], + errors: [ + {message: PARENS_NEW_LINES}, + {message: PARENS_NEW_LINES} + ] }, { code: DECLARATION_NO_PAREN, output: addNewLineSymbols(DECLARATION_PAREN), @@ -637,21 +1020,45 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: addNewLineSymbols(ARROW_PAREN), options: [{arrow: 'parens-new-line'}], errors: [{message: PARENS_NEW_LINES}] + }, { + code: ARROW_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(ARROW_PAREN_FRAGMENT), + options: [{arrow: 'parens-new-line'}], + errors: [{message: PARENS_NEW_LINES}] }, { code: ARROW_NO_PAREN, output: addNewLineSymbols(ARROW_PAREN), options: [{arrow: 'parens-new-line'}], errors: [{message: MISSING_PARENS}] + }, { + code: ARROW_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(ARROW_PAREN_FRAGMENT), + options: [{arrow: 'parens-new-line'}], + errors: [{message: MISSING_PARENS}] }, { code: CONDITION_PAREN, output: addNewLineSymbols(CONDITION_PAREN), options: [{condition: 'parens-new-line'}], errors: [{message: PARENS_NEW_LINES}] + }, { + code: CONDITION_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(CONDITION_PAREN_FRAGMENT), + options: [{condition: 'parens-new-line'}], + errors: [{message: PARENS_NEW_LINES}] }, { code: CONDITION_NO_PAREN, output: addNewLineSymbols(CONDITION_PAREN), options: [{condition: 'parens-new-line'}], errors: [{message: MISSING_PARENS}] + }, { + code: CONDITION_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(CONDITION_PAREN_FRAGMENT), + options: [{condition: 'parens-new-line'}], + errors: [{message: MISSING_PARENS}] }, { code: LOGICAL_PAREN, output: addNewLineSymbols(LOGICAL_PAREN), @@ -662,15 +1069,33 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: LOGICAL_PAREN_NEW_LINE_AUTOFIX, options: [{logical: 'parens-new-line'}], errors: [{message: MISSING_PARENS}] + }, { + code: LOGICAL_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: LOGICAL_PAREN_NEW_LINE_AUTOFIX_FRAGMENT, + options: [{logical: 'parens-new-line'}], + errors: [{message: MISSING_PARENS}] }, { code: ATTR_PAREN, output: addNewLineSymbols(ATTR_PAREN), options: [{prop: 'parens-new-line'}], errors: [{message: PARENS_NEW_LINES}] + }, { + code: ATTR_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: addNewLineSymbols(ATTR_PAREN_FRAGMENT), + options: [{prop: 'parens-new-line'}], + errors: [{message: PARENS_NEW_LINES}] }, { code: ATTR_NO_PAREN, output: ATTR_PAREN_NEW_LINE_AUTOFIX, options: [{prop: 'parens-new-line'}], errors: [{message: MISSING_PARENS}] + }, { + code: ATTR_NO_PAREN_FRAGMENT, + parser: 'babel-eslint', + output: ATTR_PAREN_NEW_LINE_AUTOFIX_FRAGMENT, + options: [{prop: 'parens-new-line'}], + errors: [{message: MISSING_PARENS}] }] }); From 0a68c285ee185ac7d3ad5760ad143307d500b41e Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Mon, 27 Aug 2018 08:46:38 -0700 Subject: [PATCH 15/49] Test for #1694 --- tests/lib/rules/prop-types.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index bdc492f262..4cbe55fa26 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -3852,6 +3852,21 @@ ruleTester.run('prop-types', rule, { errors: [{ message: '\'foo.baz\' is missing in props validation' }] + }, + { + code: ` + const ForAttendees = ({ page }) => ( + <> +
{page}
+ + ); + + export default ForAttendees; + `, + parser: 'babel-eslint', + errors: [{ + message: '\'page\' is missing in props validation' + }] } ] }); From 128b38ffc47bc49bb535335616bc4f5ffef233c3 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 4 Sep 2018 19:46:33 -0700 Subject: [PATCH 16/49] `jsx-closing-tag-location`: support shorthand fragments --- lib/rules/jsx-closing-tag-location.js | 71 +++++++++++---------- tests/lib/rules/jsx-closing-tag-location.js | 37 +++++++++++ 2 files changed, 74 insertions(+), 34 deletions(-) diff --git a/lib/rules/jsx-closing-tag-location.js b/lib/rules/jsx-closing-tag-location.js index 81c1f77686..244d3072f5 100644 --- a/lib/rules/jsx-closing-tag-location.js +++ b/lib/rules/jsx-closing-tag-location.js @@ -22,45 +22,48 @@ module.exports = { }, create: function(context) { - return { - JSXClosingElement: function(node) { - if (!node.parent) { - return; - } - - const opening = node.parent.openingElement; - if (opening.loc.start.line === node.loc.start.line) { - return; - } + function handleClosingElement(node) { + if (!node.parent) { + return; + } - if (opening.loc.start.column === node.loc.start.column) { - return; - } + const opening = node.parent.openingElement || node.parent.openingFragment; + if (opening.loc.start.line === node.loc.start.line) { + return; + } - let message; - if (!astUtil.isNodeFirstInLine(context, node)) { - message = 'Closing tag of a multiline JSX expression must be on its own line.'; - } else { - message = 'Expected closing tag to match indentation of opening.'; - } + if (opening.loc.start.column === node.loc.start.column) { + return; + } - context.report({ - node: node, - loc: node.loc, - message, - fix: function(fixer) { - const indent = Array(opening.loc.start.column + 1).join(' '); - if (astUtil.isNodeFirstInLine(context, node)) { - return fixer.replaceTextRange( - [node.range[0] - node.loc.start.column, node.range[0]], - indent - ); - } + let message; + if (!astUtil.isNodeFirstInLine(context, node)) { + message = 'Closing tag of a multiline JSX expression must be on its own line.'; + } else { + message = 'Expected closing tag to match indentation of opening.'; + } - return fixer.insertTextBefore(node, `\n${indent}`); + context.report({ + node: node, + loc: node.loc, + message, + fix: function(fixer) { + const indent = Array(opening.loc.start.column + 1).join(' '); + if (astUtil.isNodeFirstInLine(context, node)) { + return fixer.replaceTextRange( + [node.range[0] - node.loc.start.column, node.range[0]], + indent + ); } - }); - } + + return fixer.insertTextBefore(node, `\n${indent}`); + } + }); + } + + return { + JSXClosingElement: handleClosingElement, + JSXClosingFragment: handleClosingElement }; } }; diff --git a/tests/lib/rules/jsx-closing-tag-location.js b/tests/lib/rules/jsx-closing-tag-location.js index 5dda91f58f..6899bfd669 100644 --- a/tests/lib/rules/jsx-closing-tag-location.js +++ b/tests/lib/rules/jsx-closing-tag-location.js @@ -36,6 +36,18 @@ ruleTester.run('jsx-closing-tag-location', rule, { code: ` foo ` + }, { + code: ` + <> + foo + + `, + parser: 'babel-eslint' + }, { + code: ` + <>foo + `, + parser: 'babel-eslint' }], invalid: [{ @@ -61,5 +73,30 @@ ruleTester.run('jsx-closing-tag-location', rule, { `, errors: MESSAGE_OWN_LINE + }, { + code: ` + <> + foo + + `, + parser: 'babel-eslint', + output: ` + <> + foo + + `, + errors: MESSAGE_MATCH_INDENTATION + }, { + code: ` + <> + foo + `, + parser: 'babel-eslint', + output: ` + <> + foo + + `, + errors: MESSAGE_OWN_LINE }] }); From 18898e1c41ff0d9f2f8e1c93267b6cda91328765 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 4 Sep 2018 19:53:21 -0700 Subject: [PATCH 17/49] `jsx-uses-react`: support shorthand fragments --- lib/rules/jsx-uses-react.js | 10 +++++----- tests/lib/rules/jsx-uses-react.js | 11 +++++++++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/rules/jsx-uses-react.js b/lib/rules/jsx-uses-react.js index 51ba2fc320..ffafc4049d 100644 --- a/lib/rules/jsx-uses-react.js +++ b/lib/rules/jsx-uses-react.js @@ -25,16 +25,16 @@ module.exports = { create: function(context) { const pragma = pragmaUtil.getFromContext(context); + function handleOpeningElement() { + context.markVariableAsUsed(pragma); + } // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- return { - - JSXOpeningElement: function() { - context.markVariableAsUsed(pragma); - } - + JSXOpeningElement: handleOpeningElement, + JSXOpeningFragment: handleOpeningElement }; } }; diff --git a/tests/lib/rules/jsx-uses-react.js b/tests/lib/rules/jsx-uses-react.js index 1cb516ba18..d8874e91d6 100644 --- a/tests/lib/rules/jsx-uses-react.js +++ b/tests/lib/rules/jsx-uses-react.js @@ -39,7 +39,8 @@ ruleTester.run('no-unused-vars', rule, { {code: '/*eslint jsx-uses-react:1*/ var React;
;'}, {code: '/*eslint jsx-uses-react:1*/ var React; (function () {
})();'}, {code: '/*eslint jsx-uses-react:1*/ /** @jsx Foo */ var Foo;
;'}, - {code: '/*eslint jsx-uses-react:1*/ var Foo;
;', settings: settings} + {code: '/*eslint jsx-uses-react:1*/ var Foo;
;', settings: settings}, + {code: '/*eslint jsx-uses-react:1*/ var React; <>;', parser: 'babel-eslint'} ], invalid: [{ code: '/*eslint jsx-uses-react:1*/ var React;', @@ -49,6 +50,12 @@ ruleTester.run('no-unused-vars', rule, { errors: [{message: '\'React\' is defined but never used.'}] }, { code: '/*eslint jsx-uses-react:1*/ var React;
;', - errors: [{message: '\'React\' is defined but never used.'}], settings: settings + errors: [{message: '\'React\' is defined but never used.'}], + settings: settings + }, { + code: '/*eslint jsx-uses-react:1*/ var React; <>;', + parser: 'babel-eslint', + errors: [{message: '\'React\' is defined but never used.'}], + settings: settings }] }); From dd00d8aabad7ccbc17bbb7859406dd4aa8fb1601 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 7 Sep 2018 13:18:58 -0700 Subject: [PATCH 18/49] `jsx-indent`: support shorthand fragments --- lib/rules/jsx-indent.js | 77 +++--- tests/lib/rules/jsx-indent.js | 491 ++++++++++++++++++++++++++++++++++ 2 files changed, 532 insertions(+), 36 deletions(-) diff --git a/lib/rules/jsx-indent.js b/lib/rules/jsx-indent.js index cbb9dac84e..1ba0d75cdc 100644 --- a/lib/rules/jsx-indent.js +++ b/lib/rules/jsx-indent.js @@ -205,43 +205,48 @@ module.exports = { } } - return { - JSXOpeningElement: function(node) { - let prevToken = sourceCode.getTokenBefore(node); - if (!prevToken) { - return; - } - // Use the parent in a list or an array - if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') { - prevToken = sourceCode.getNodeByRangeIndex(prevToken.range[0]); - prevToken = prevToken.type === 'Literal' || prevToken.type === 'JSXText' ? prevToken.parent : prevToken; - // Use the first non-punctuator token in a conditional expression - } else if (prevToken.type === 'Punctuator' && prevToken.value === ':') { - do { - prevToken = sourceCode.getTokenBefore(prevToken); - } while (prevToken.type === 'Punctuator'); - prevToken = sourceCode.getNodeByRangeIndex(prevToken.range[0]); - while (prevToken.parent && prevToken.parent.type !== 'ConditionalExpression') { - prevToken = prevToken.parent; - } - } - prevToken = prevToken.type === 'JSXExpressionContainer' ? prevToken.expression : prevToken; - - const parentElementIndent = getNodeIndent(prevToken); - const indent = ( - prevToken.loc.start.line === node.loc.start.line || - isRightInLogicalExp(node) || - isAlternateInConditionalExp(node) - ) ? 0 : indentSize; - checkNodesIndent(node, parentElementIndent + indent); - }, - JSXClosingElement: function(node) { - if (!node.parent) { - return; + function handleOpeningElement(node) { + let prevToken = sourceCode.getTokenBefore(node); + if (!prevToken) { + return; + } + // Use the parent in a list or an array + if (prevToken.type === 'JSXText' || prevToken.type === 'Punctuator' && prevToken.value === ',') { + prevToken = sourceCode.getNodeByRangeIndex(prevToken.range[0]); + prevToken = prevToken.type === 'Literal' || prevToken.type === 'JSXText' ? prevToken.parent : prevToken; + // Use the first non-punctuator token in a conditional expression + } else if (prevToken.type === 'Punctuator' && prevToken.value === ':') { + do { + prevToken = sourceCode.getTokenBefore(prevToken); + } while (prevToken.type === 'Punctuator' && prevToken.value !== '/'); + prevToken = sourceCode.getNodeByRangeIndex(prevToken.range[0]); + while (prevToken.parent && prevToken.parent.type !== 'ConditionalExpression') { + prevToken = prevToken.parent; } - const peerElementIndent = getNodeIndent(node.parent.openingElement); - checkNodesIndent(node, peerElementIndent); - }, + } + prevToken = prevToken.type === 'JSXExpressionContainer' ? prevToken.expression : prevToken; + const parentElementIndent = getNodeIndent(prevToken); + const indent = ( + prevToken.loc.start.line === node.loc.start.line || + isRightInLogicalExp(node) || + isAlternateInConditionalExp(node) + ) ? 0 : indentSize; + checkNodesIndent(node, parentElementIndent + indent); + } + + function handleClosingElement(node) { + if (!node.parent) { + return; + } + const peerElementIndent = getNodeIndent(node.parent.openingElement || node.parent.openingFragment); + checkNodesIndent(node, peerElementIndent); + } + + return { + JSXOpeningElement: handleOpeningElement, + JSXOpeningFragment: handleOpeningElement, + JSXClosingElement: handleClosingElement, + JSXClosingFragment: handleClosingElement, JSXExpressionContainer: function(node) { if (!node.parent) { return; diff --git a/tests/lib/rules/jsx-indent.js b/tests/lib/rules/jsx-indent.js index 520e2b7147..fc44dd4a5a 100644 --- a/tests/lib/rules/jsx-indent.js +++ b/tests/lib/rules/jsx-indent.js @@ -29,11 +29,22 @@ ruleTester.run('jsx-indent', rule, { code: [ '' ].join('\n') + }, { + code: [ + '<>' + ].join('\n'), + parser: 'babel-eslint' }, { code: [ '', '' ].join('\n') + }, { + code: [ + '<>', + '' + ].join('\n'), + parser: 'babel-eslint' }, { code: [ '', @@ -41,6 +52,22 @@ ruleTester.run('jsx-indent', rule, { '' ].join('\n'), options: [2] + }, { + code: [ + '', + ' <>', + '' + ].join('\n'), + parser: 'babel-eslint', + options: [2] + }, { + code: [ + '<>', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ '', @@ -71,6 +98,16 @@ ruleTester.run('jsx-indent', rule, { '}' ].join('\n'), options: [2] + }, { + code: [ + 'function App() {', + ' return ', + ' <>', + ' ;', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ 'function App() {', @@ -80,6 +117,16 @@ ruleTester.run('jsx-indent', rule, { '}' ].join('\n'), options: [2] + }, { + code: [ + 'function App() {', + ' return (', + ' <>', + ' );', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ 'function App() {', @@ -91,6 +138,18 @@ ruleTester.run('jsx-indent', rule, { '}' ].join('\n'), options: [2] + }, { + code: [ + 'function App() {', + ' return (', + ' ', + ' <>', + ' ', + ' );', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ 'it(', @@ -102,6 +161,18 @@ ruleTester.run('jsx-indent', rule, { ')' ].join('\n'), options: [2] + }, { + code: [ + 'it(', + ' (', + '
', + ' <>', + '
', + ' )', + ')' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ 'it(', @@ -132,6 +203,17 @@ ruleTester.run('jsx-indent', rule, { '}' ].join('\n'), options: [2] + }, { + code: [ + '{', + ' head.title &&', + ' <>', + ' {head.title}', + ' ', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ '{', @@ -171,6 +253,15 @@ ruleTester.run('jsx-indent', rule, { ']' ].join('\n'), options: [2] + }, { + code: [ + '[', + ' <>,', + ' <>', + ']' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: [ '
', @@ -193,6 +284,18 @@ ruleTester.run('jsx-indent', rule, { ' }', '
' ].join('\n') + }, { + code: [ + '
', + ' {foo &&', + ' [', + ' <>,', + ' <>', + ' ]', + ' }', + '
' + ].join('\n'), + parser: 'babel-eslint' }, { // Literals indentation is not touched code: [ @@ -203,6 +306,16 @@ ruleTester.run('jsx-indent', rule, { 'bar
', '
' ].join('\n') + }, { + code: [ + '<>', + 'bar <>', + ' bar', + ' bar {foo}', + 'bar ', + '' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon at the end of the first expression) @@ -211,6 +324,13 @@ ruleTester.run('jsx-indent', rule, { ' :', ' ' ].join('\n') + }, { + code: [ + 'foo ?', + ' <> :', + ' <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon at the start of the second expression) @@ -219,6 +339,13 @@ ruleTester.run('jsx-indent', rule, { ' ', ' : ' ].join('\n') + }, { + code: [ + 'foo ?', + ' <>', + ' : <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon on its own line) @@ -228,6 +355,14 @@ ruleTester.run('jsx-indent', rule, { ':', ' ' ].join('\n') + }, { + code: [ + 'foo ?', + ' <>', + ':', + ' <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (multiline JSX, colon on its own line) @@ -249,6 +384,12 @@ ruleTester.run('jsx-indent', rule, { 'foo ? :', '' ].join('\n') + }, { + code: [ + 'foo ? <> :', + '<>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (first expression on test line, colon at the start of the second expression) @@ -256,6 +397,12 @@ ruleTester.run('jsx-indent', rule, { 'foo ? ', ': ' ].join('\n') + }, { + code: [ + 'foo ? <>', + ': <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (first expression on test line, colon on its own line) @@ -264,6 +411,13 @@ ruleTester.run('jsx-indent', rule, { ':', '' ].join('\n') + }, { + code: [ + 'foo ? <>', + ':', + '<>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon at the end of the first expression, parenthesized first expression) @@ -273,6 +427,14 @@ ruleTester.run('jsx-indent', rule, { ') :', ' ' ].join('\n') + }, { + code: [ + 'foo ? (', + ' <>', + ') :', + ' <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon at the start of the second expression, parenthesized first expression) @@ -282,6 +444,14 @@ ruleTester.run('jsx-indent', rule, { ')', ' : ' ].join('\n') + }, { + code: [ + 'foo ? (', + ' <>', + ')', + ' : <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon on its own line, parenthesized first expression) @@ -292,6 +462,15 @@ ruleTester.run('jsx-indent', rule, { ':', ' ' ].join('\n') + }, { + code: [ + 'foo ? (', + ' <>', + ')', + ':', + ' <>' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon at the end of the first expression, parenthesized second expression) @@ -301,6 +480,14 @@ ruleTester.run('jsx-indent', rule, { ' ', ' )' ].join('\n') + }, { + code: [ + 'foo ?', + ' <> : (', + ' <>', + ' )' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon on its own line, parenthesized second expression) @@ -311,6 +498,15 @@ ruleTester.run('jsx-indent', rule, { ' ', ')' ].join('\n') + }, { + code: [ + 'foo ?', + ' <>', + ': (', + ' <>', + ')' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon indented on its own line, parenthesized second expression) @@ -321,6 +517,15 @@ ruleTester.run('jsx-indent', rule, { ' ', ' )' ].join('\n') + }, { + code: [ + 'foo ?', + ' <>', + ' : (', + ' <>', + ' )' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon at the end of the first expression, both expression parenthesized) @@ -331,6 +536,15 @@ ruleTester.run('jsx-indent', rule, { ' ', ')' ].join('\n') + }, { + code: [ + 'foo ? (', + ' <>', + ') : (', + ' <>', + ')' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon on its own line, both expression parenthesized) @@ -342,6 +556,16 @@ ruleTester.run('jsx-indent', rule, { ' ', ')' ].join('\n') + }, { + code: [ + 'foo ? (', + ' <>', + ')', + ': (', + ' <>', + ')' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (colon on its own line, both expression parenthesized) @@ -354,6 +578,17 @@ ruleTester.run('jsx-indent', rule, { ' ', ')' ].join('\n') + }, { + code: [ + 'foo ? (', + ' <>', + ')', + ':', + '(', + ' <>', + ')' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (first expression on test line, colon at the end of the first expression, parenthesized second expression) @@ -362,6 +597,13 @@ ruleTester.run('jsx-indent', rule, { ' ', ')' ].join('\n') + }, { + code: [ + 'foo ? <> : (', + ' <>', + ')' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (first expression on test line, colon at the start of the second expression, parenthesized second expression) @@ -369,6 +611,12 @@ ruleTester.run('jsx-indent', rule, { 'foo ? ', ': ()' ].join('\n') + }, { + code: [ + 'foo ? <>', + ': (<>)' + ].join('\n'), + parser: 'babel-eslint' }, { // Multiline ternary // (first expression on test line, colon on its own line, parenthesized second expression) @@ -378,6 +626,14 @@ ruleTester.run('jsx-indent', rule, { ' ', ')' ].join('\n') + }, { + code: [ + 'foo ? <>', + ': (', + ' <>', + ')' + ].join('\n'), + parser: 'babel-eslint' }, { code: [ '', @@ -416,6 +672,21 @@ ruleTester.run('jsx-indent', rule, { '}' ].join('\n'), options: [2] + }, { + code: [ + 'function foo() {', + ' ', + ' {condition ?', + ' :', + ' <>', + ' }', + ' ', + '}' + ].join('\n'), + parser: 'babel-eslint', + options: [2] }, { code: ` class Test extends React.Component { @@ -430,6 +701,21 @@ ruleTester.run('jsx-indent', rule, { } `, options: [2] + }, { + code: ` + class Test extends React.Component { + render() { + return ( + <> + <> + <> + + ); + } + } + `, + parser: 'babel-eslint', + options: [2] }], invalid: [{ @@ -444,6 +730,32 @@ ruleTester.run('jsx-indent', rule, { '' ].join('\n'), errors: [{message: 'Expected indentation of 4 space characters but found 2.'}] + }, { + code: [ + '', + ' <>', + '' + ].join('\n'), + parser: 'babel-eslint', + output: [ + '', + ' <>', + '' + ].join('\n'), + errors: [{message: 'Expected indentation of 4 space characters but found 2.'}] + }, { + code: [ + '<>', + ' ', + '' + ].join('\n'), + parser: 'babel-eslint', + output: [ + '<>', + ' ', + '' + ].join('\n'), + errors: [{message: 'Expected indentation of 4 space characters but found 2.'}] }, { code: [ '', @@ -635,6 +947,24 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 2 space characters but found 4.'} ] + }, { + code: [ + '[', + '
,', + ' <>', + ']' + ].join('\n'), + parser: 'babel-eslint', + output: [ + '[', + '
,', + ' <>', + ']' + ].join('\n'), + options: [2], + errors: [ + {message: 'Expected indentation of 2 space characters but found 4.'} + ] }, { code: [ '\n', @@ -729,6 +1059,21 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 4 space characters but found 0.'} ] + }, { + code: [ + 'foo ?', + ' :', + '<>' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ?', + ' :', + ' <>' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { // Multiline ternary // (colon on its own line) @@ -761,6 +1106,23 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 0 space characters but found 4.'} ] + }, { + code: [ + 'foo ?', + ' ', + ':', + '<>' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ?', + ' ', + ':', + ' <>' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { // Multiline ternary // (first expression on test line, colon on its own line) @@ -795,6 +1157,23 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 4 space characters but found 0.'} ] + }, { + code: [ + 'foo ? (', + ' ', + ') :', + '<>' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ? (', + ' ', + ') :', + ' <>' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { // Multiline ternary // (colon on its own line, parenthesized first expression) @@ -833,6 +1212,23 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 8 space characters but found 4.'} ] + }, { + code: [ + 'foo ?', + ' : (', + ' <>', + ' )' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ?', + ' : (', + ' <>', + ' )' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 8 space characters but found 4.'} + ] }, { // Multiline ternary // (colon on its own line, parenthesized second expression) @@ -873,6 +1269,25 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 8 space characters but found 4.'} ] + }, { + code: [ + 'foo ?', + ' ', + ' : (', + ' <>', + ' )' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ?', + ' ', + ' : (', + ' <>', + ' )' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 8 space characters but found 4.'} + ] }, { // Multiline ternary // (colon at the end of the first expression, both expression parenthesized) @@ -894,6 +1309,26 @@ ruleTester.run('jsx-indent', rule, { {message: 'Expected indentation of 4 space characters but found 0.'}, {message: 'Expected indentation of 4 space characters but found 0.'} ] + }, { + code: [ + 'foo ? (', + '<>', + ') : (', + '<>', + ')' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ? (', + ' <>', + ') : (', + ' <>', + ')' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'}, + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { // Multiline ternary // (colon on its own line, both expression parenthesized) @@ -942,6 +1377,30 @@ ruleTester.run('jsx-indent', rule, { {message: 'Expected indentation of 4 space characters but found 0.'}, {message: 'Expected indentation of 4 space characters but found 0.'} ] + }, { + code: [ + 'foo ? (', + '<>', + ')', + ':', + '(', + '<>', + ')' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ? (', + ' <>', + ')', + ':', + '(', + ' <>', + ')' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'}, + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { // Multiline ternary // (first expression on test line, colon at the end of the first expression, parenthesized second expression) @@ -958,6 +1417,21 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 4 space characters but found 0.'} ] + }, { + code: [ + 'foo ? : (', + '<>', + ')' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ? : (', + ' <>', + ')' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { // Multiline ternary // (first expression on test line, colon on its own line, parenthesized second expression) @@ -976,6 +1450,23 @@ ruleTester.run('jsx-indent', rule, { errors: [ {message: 'Expected indentation of 4 space characters but found 0.'} ] + }, { + code: [ + 'foo ? ', + ': (', + '<>', + ')' + ].join('\n'), + parser: 'babel-eslint', + output: [ + 'foo ? ', + ': (', + ' <>', + ')' + ].join('\n'), + errors: [ + {message: 'Expected indentation of 4 space characters but found 0.'} + ] }, { code: [ '

', From 083adf86582f5d320073b9d9f012b741a1ff3af0 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 7 Sep 2018 17:37:15 -0700 Subject: [PATCH 19/49] `jsx-child-element-spacing`: support shorthand fragments --- lib/rules/jsx-child-element-spacing.js | 61 ++++++++++---------- tests/lib/rules/jsx-child-element-spacing.js | 22 +++++++ 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/lib/rules/jsx-child-element-spacing.js b/lib/rules/jsx-child-element-spacing.js index d6c03c59bf..cda5af590c 100644 --- a/lib/rules/jsx-child-element-spacing.js +++ b/lib/rules/jsx-child-element-spacing.js @@ -68,39 +68,42 @@ module.exports = { INLINE_ELEMENTS.has(elementName(node)) ); + const handleJSX = node => { + let lastChild = null; + let child = null; + (node.children.concat([null])).forEach(nextChild => { + if ( + (lastChild || nextChild) && + (!lastChild || isInlineElement(lastChild)) && + (child && (child.type === 'Literal' || child.type === 'JSXText')) && + (!nextChild || isInlineElement(nextChild)) && + true + ) { + if (lastChild && child.value.match(TEXT_FOLLOWING_ELEMENT_PATTERN)) { + context.report({ + node: lastChild, + loc: lastChild.loc.end, + message: `Ambiguous spacing after previous element ${elementName(lastChild)}` + }); + } else if (nextChild && child.value.match(TEXT_PRECEDING_ELEMENT_PATTERN)) { + context.report({ + node: nextChild, + loc: nextChild.loc.start, + message: `Ambiguous spacing before next element ${elementName(nextChild)}` + }); + } + } + lastChild = child; + child = nextChild; + }); + }; + const TEXT_FOLLOWING_ELEMENT_PATTERN = /^\s*\n\s*\S/; const TEXT_PRECEDING_ELEMENT_PATTERN = /\S\s*\n\s*$/; return { - JSXElement: function(node) { - let lastChild = null; - let child = null; - (node.children.concat([null])).forEach(nextChild => { - if ( - (lastChild || nextChild) && - (!lastChild || isInlineElement(lastChild)) && - (child && (child.type === 'Literal' || child.type === 'JSXText')) && - (!nextChild || isInlineElement(nextChild)) && - true - ) { - if (lastChild && child.value.match(TEXT_FOLLOWING_ELEMENT_PATTERN)) { - context.report({ - node: lastChild, - loc: lastChild.loc.end, - message: `Ambiguous spacing after previous element ${elementName(lastChild)}` - }); - } else if (nextChild && child.value.match(TEXT_PRECEDING_ELEMENT_PATTERN)) { - context.report({ - node: nextChild, - loc: nextChild.loc.start, - message: `Ambiguous spacing before next element ${elementName(nextChild)}` - }); - } - } - lastChild = child; - child = nextChild; - }); - } + JSXElement: handleJSX, + JSXFragment: handleJSX }; } }; diff --git a/tests/lib/rules/jsx-child-element-spacing.js b/tests/lib/rules/jsx-child-element-spacing.js index fdd19149fb..51c837b18a 100644 --- a/tests/lib/rules/jsx-child-element-spacing.js +++ b/tests/lib/rules/jsx-child-element-spacing.js @@ -17,6 +17,13 @@ ruleTester.run('jsx-child-element-spacing', rule, { foo ` + }, { + code: ` + <> + foo + + `, + parser: 'babel-eslint' }, { code: ` @@ -146,6 +153,21 @@ ruleTester.run('jsx-child-element-spacing', rule, { ] }, { code: ` +<> + foo + bar + + `, + parser: 'babel-eslint', + errors: [ + { + message: 'Ambiguous spacing before next element a', + line: 4, + column: 3 + } + ] + }, { + code: ` bar baz From 3d0580ff39eb5e6e524d5fe2fd51a594c7ca7317 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 7 Sep 2018 18:02:01 -0700 Subject: [PATCH 20/49] `jsx-no-comment-textnodes`: add tests for shorthand fragments --- tests/lib/rules/jsx-no-comment-textnodes.js | 35 +++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/lib/rules/jsx-no-comment-textnodes.js b/tests/lib/rules/jsx-no-comment-textnodes.js index 5691d1aaf1..5126038b8f 100644 --- a/tests/lib/rules/jsx-no-comment-textnodes.js +++ b/tests/lib/rules/jsx-no-comment-textnodes.js @@ -40,6 +40,19 @@ ruleTester.run('jsx-no-comment-textnodes', rule, { } `, parser: 'babel-eslint' + }, { + code: ` + class Comp1 extends Component { + render() { + return ( + <> + {/* valid */} + + ); + } + } + `, + parser: 'babel-eslint' }, { code: ` class Comp1 extends Component { @@ -125,6 +138,18 @@ ruleTester.run('jsx-no-comment-textnodes', rule, { `, parser: 'babel-eslint' }, + { + code: ` + + `, + parser: 'babel-eslint' + }, + { + code: ` + <> + `, + parser: 'babel-eslint' + }, { code: ` @@ -158,6 +183,16 @@ ruleTester.run('jsx-no-comment-textnodes', rule, { `, parser: 'babel-eslint', errors: [{message: 'Comments inside children section of tag should be placed inside braces'}] + }, { + code: ` + class Comp1 extends Component { + render() { + return (<>// invalid); + } + } + `, + parser: 'babel-eslint', + errors: [{message: 'Comments inside children section of tag should be placed inside braces'}] }, { code: ` class Comp1 extends Component { From c1b6da0bb9b06f5f8b6e9b37eb23eb570235f9c1 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 7 Sep 2018 18:04:20 -0700 Subject: [PATCH 21/49] `jsx-no-literals`: add tests for shorthand fragments --- tests/lib/rules/jsx-no-literals.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/lib/rules/jsx-no-literals.js b/tests/lib/rules/jsx-no-literals.js index 869466c265..7edac90594 100644 --- a/tests/lib/rules/jsx-no-literals.js +++ b/tests/lib/rules/jsx-no-literals.js @@ -41,6 +41,19 @@ ruleTester.run('jsx-no-literals', rule, { } `, parser: 'babel-eslint' + }, { + code: ` + class Comp1 extends Component { + render() { + return ( + <> + {'asdjfl'} + + ); + } + } + `, + parser: 'babel-eslint' }, { code: ` class Comp1 extends Component { @@ -189,6 +202,16 @@ ruleTester.run('jsx-no-literals', rule, { `, parser: 'babel-eslint', errors: [{message: 'Missing JSX expression container around literal string'}] + }, { + code: ` + class Comp1 extends Component { + render() { + return (<>test); + } + } + `, + parser: 'babel-eslint', + errors: [{message: 'Missing JSX expression container around literal string'}] }, { code: ` class Comp1 extends Component { From 8215be35c270f539db0884032e901a7589578f24 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 7 Sep 2018 18:21:58 -0700 Subject: [PATCH 22/49] Fix linting error --- lib/rules/jsx-child-element-spacing.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/jsx-child-element-spacing.js b/lib/rules/jsx-child-element-spacing.js index cda5af590c..271423d970 100644 --- a/lib/rules/jsx-child-element-spacing.js +++ b/lib/rules/jsx-child-element-spacing.js @@ -56,6 +56,9 @@ module.exports = { ] }, create: function (context) { + const TEXT_FOLLOWING_ELEMENT_PATTERN = /^\s*\n\s*\S/; + const TEXT_PRECEDING_ELEMENT_PATTERN = /\S\s*\n\s*$/; + const elementName = node => ( node.openingElement && node.openingElement.name && @@ -98,9 +101,6 @@ module.exports = { }); }; - const TEXT_FOLLOWING_ELEMENT_PATTERN = /^\s*\n\s*\S/; - const TEXT_PRECEDING_ELEMENT_PATTERN = /\S\s*\n\s*$/; - return { JSXElement: handleJSX, JSXFragment: handleJSX From f0e2e1e364f27be89e771dda237b6a422c839be6 Mon Sep 17 00:00:00 2001 From: ametreniuc Date: Mon, 10 Sep 2018 19:07:19 +0300 Subject: [PATCH 23/49] [Fix] `sort-prop-types`: fix string property order Fixes #1976. , --- lib/rules/sort-prop-types.js | 3 +++ tests/lib/rules/sort-prop-types.js | 34 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/lib/rules/sort-prop-types.js b/lib/rules/sort-prop-types.js index da94a641db..a2e2cf22a5 100644 --- a/lib/rules/sort-prop-types.js +++ b/lib/rules/sort-prop-types.js @@ -57,6 +57,9 @@ module.exports = { const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); function getKey(node) { + if (node.key && node.key.value) { + return node.key.value; + } return sourceCode.getText(node.key || node.argument); } diff --git a/tests/lib/rules/sort-prop-types.js b/tests/lib/rules/sort-prop-types.js index e83ccacd34..3f8d184626 100644 --- a/tests/lib/rules/sort-prop-types.js +++ b/tests/lib/rules/sort-prop-types.js @@ -1533,5 +1533,39 @@ ruleTester.run('sort-prop-types', rule, { ' }', '});' ].join('\n') + }, { + code: [ + 'var First = createReactClass({', + ' propTypes: {', + ' \'data-letter\': PropTypes.string,', + ' a: PropTypes.any,', + ' e: PropTypes.any', + ' },', + ' render: function() {', + ' return

;', + ' }', + '});' + ].join('\n'), + options: [{ + noSortAlphabetically: false + }], + errors: [{ + message: ERROR_MESSAGE, + line: 4, + column: 5, + type: 'Property' + }], + output: [ + 'var First = createReactClass({', + ' propTypes: {', + ' a: PropTypes.any,', + ' \'data-letter\': PropTypes.string,', + ' e: PropTypes.any', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n') }] }); From 089d54536714682e3c7b207d6382b8decc17584e Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Thu, 13 Sep 2018 23:16:59 -0700 Subject: [PATCH 24/49] [Fix] `destructuring-assignment`: handle nested props usage --- lib/rules/destructuring-assignment.js | 13 ++++++++++++- tests/lib/rules/destructuring-assignment.js | 11 +++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/rules/destructuring-assignment.js b/lib/rules/destructuring-assignment.js index 2e09440819..9f7d28255f 100644 --- a/lib/rules/destructuring-assignment.js +++ b/lib/rules/destructuring-assignment.js @@ -82,6 +82,17 @@ module.exports = { } } + function isInClassProperty(node) { + let curNode = node.parent; + while (curNode) { + if (curNode.type === 'ClassProperty') { + return true; + } + curNode = curNode.parent; + } + return false; + } + function handleClassUsage(node) { // this.props.Aprop || this.context.aProp || this.state.aState const isPropUsed = ( @@ -92,7 +103,7 @@ module.exports = { if ( isPropUsed && configuration === 'always' && - !(ignoreClassFields && node.parent.type === 'ClassProperty') + !(ignoreClassFields && isInClassProperty(node)) ) { context.report({ node: node, diff --git a/tests/lib/rules/destructuring-assignment.js b/tests/lib/rules/destructuring-assignment.js index 164a41ed0a..e4b80f6108 100644 --- a/tests/lib/rules/destructuring-assignment.js +++ b/tests/lib/rules/destructuring-assignment.js @@ -173,6 +173,17 @@ ruleTester.run('destructuring-assignment', rule, { `, options: ['always', {ignoreClassFields: true}], parser: 'babel-eslint' + }, { + code: [ + 'class Input extends React.Component {', + ' id = `${this.props.name}`;', + ' render() {', + ' return
;', + ' }', + '}' + ].join('\n'), + options: ['always', {ignoreClassFields: true}], + parser: 'babel-eslint' }], invalid: [{ From eae31a08cef54a0f051a4cd4917a3d24e97feb40 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 18 Sep 2018 10:39:16 +0100 Subject: [PATCH 25/49] add example of complete react/jsx-no-target-blank rule --- docs/rules/jsx-no-target-blank.md | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/docs/rules/jsx-no-target-blank.md b/docs/rules/jsx-no-target-blank.md index be20205c2a..8ce05daa48 100644 --- a/docs/rules/jsx-no-target-blank.md +++ b/docs/rules/jsx-no-target-blank.md @@ -15,27 +15,35 @@ This rule aims to prevent user generated links from creating security vulnerabil There are two main options for the rule: -* `{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default) -* `{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link +- `{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default) +- `{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link +```json +"react/jsx-no-target-blank": [, { enforceDynamicLinks: }] +``` + +- enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. +- enforce: optional string, defaults to "always" ### always (default) When {"enforceDynamicLinks": "always"} is set, the following patterns are considered errors: ```jsx -var Hello = -var Hello = +var Hello = ; +var Hello = ; ``` The following patterns are **not** considered errors: ```jsx -var Hello =

-var Hello =
-var Hello = -var Hello = -var Hello = +var Hello =

; +var Hello = ( + +); +var Hello = ; +var Hello = ; +var Hello = ; ``` ### never @@ -43,7 +51,7 @@ var Hello = When {"enforceDynamicLinks": "never"} is set, the following patterns are **not** considered errors: ```jsx -var Hello = +var Hello = ; ``` ## When Not To Use It From 658379a52fe566bb23cfce72bd4ab1541581b290 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 18 Sep 2018 10:56:25 +0100 Subject: [PATCH 26/49] undid editor prettifying opinions --- docs/rules/jsx-no-target-blank.md | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/docs/rules/jsx-no-target-blank.md b/docs/rules/jsx-no-target-blank.md index 8ce05daa48..438dc2f08a 100644 --- a/docs/rules/jsx-no-target-blank.md +++ b/docs/rules/jsx-no-target-blank.md @@ -15,35 +15,34 @@ This rule aims to prevent user generated links from creating security vulnerabil There are two main options for the rule: -- `{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default) -- `{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link +* `{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default) +* `{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link ```json "react/jsx-no-target-blank": [, { enforceDynamicLinks: }] ``` -- enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. -- enforce: optional string, defaults to "always" +* enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. +* enforce: optional string, defaults to "always" + ### always (default) When {"enforceDynamicLinks": "always"} is set, the following patterns are considered errors: ```jsx -var Hello = ; -var Hello = ; +var Hello = +var Hello = ``` The following patterns are **not** considered errors: ```jsx -var Hello =

; -var Hello = ( - -); -var Hello = ; -var Hello = ; -var Hello = ; +var Hello =

+var Hello = +var Hello = +var Hello = +var Hello = ``` ### never @@ -51,9 +50,9 @@ var Hello = ; When {"enforceDynamicLinks": "never"} is set, the following patterns are **not** considered errors: ```jsx -var Hello = ; +var Hello = ``` ## When Not To Use It -If you do not have any external links, you can disable this rule +If you do not have any external links, you can disable this rule \ No newline at end of file From c8855358ab62915d7202be146d4753fb83c820d0 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Wed, 19 Sep 2018 03:08:17 +0300 Subject: [PATCH 27/49] Fix `no-this-in-sfc` behavior for arrow functions Fixed #1967 --- lib/rules/no-this-in-sfc.js | 8 ++++---- lib/util/Components.js | 25 ++++++++++++++++++++++++- tests/lib/rules/no-this-in-sfc.js | 20 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-this-in-sfc.js b/lib/rules/no-this-in-sfc.js index 3883a36d69..8d715e7bae 100644 --- a/lib/rules/no-this-in-sfc.js +++ b/lib/rules/no-this-in-sfc.js @@ -29,11 +29,11 @@ module.exports = { create: Components.detect((context, components, utils) => ({ MemberExpression(node) { - const component = components.get(utils.getParentStatelessComponent()); - if (!component) { - return; - } if (node.object.type === 'ThisExpression') { + const component = components.get(utils.getParentStatelessComponent()); + if (!component) { + return; + } context.report({ node: node, message: ERROR_MESSAGE diff --git a/lib/util/Components.js b/lib/util/Components.js index 5961eaae60..4b55f88191 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -469,7 +469,13 @@ function componentRule(rule, context) { const node = scope.block; const isClass = node.type === 'ClassExpression'; const isFunction = /Function/.test(node.type); // Functions - const isMethod = node.parent && node.parent.type === 'MethodDefinition'; // Classes methods + const isArrowFunction = node.type === 'ArrowFunctionExpression'; + let functionScope = scope; + if (isArrowFunction) { + functionScope = utils.getParentFunctionScope(scope); + } + const methodNode = functionScope && functionScope.block.parent; + const isMethod = methodNode && methodNode.type === 'MethodDefinition'; // Classes methods const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.) // Attribute Expressions inside JSX Elements () const isJSXExpressionContainer = node.parent && node.parent.type === 'JSXExpressionContainer'; @@ -486,6 +492,23 @@ function componentRule(rule, context) { return null; }, + /** + * Get a parent scope created by a FunctionExpression or FunctionDeclaration + * @param {Scope} scope The child scope + * @returns {Scope} A parent function scope + */ + getParentFunctionScope(scope) { + scope = scope.upper; + while (scope) { + const type = scope.block.type; + if (type === 'FunctionExpression' || type === 'FunctionExpression') { + return scope; + } + scope = scope.upper; + } + return null; + }, + /** * Get the related component from a node * diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index 026b44dc1a..cae483ab62 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -99,6 +99,26 @@ ruleTester.run('no-this-in-sfc', rule, { code: 'const Foo = (props) => props.foo ? {props.bar} : null;' }, { code: 'const Foo = ({ foo, bar }) => foo ? {bar} : null;' + }, { + code: ` + class Foo { + bar() { + () => { + this.something(); + return null; + }; + } + }` + }, { + code: ` + class Foo { + bar() { + () => () => { + this.something(); + return null; + }; + } + }` }], invalid: [{ code: ` From 1a9d54c0856aef92db367ce5cc546c30728cb3fb Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Wed, 19 Sep 2018 19:57:03 +0300 Subject: [PATCH 28/49] Fixed typo, added more tests --- lib/util/Components.js | 2 +- tests/lib/rules/no-this-in-sfc.js | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index 4b55f88191..8ed7bbd2c3 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -501,7 +501,7 @@ function componentRule(rule, context) { scope = scope.upper; while (scope) { const type = scope.block.type; - if (type === 'FunctionExpression' || type === 'FunctionExpression') { + if (type === 'FunctionExpression' || type === 'FunctionDeclaration') { return scope; } scope = scope.upper; diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index cae483ab62..8812055136 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -185,5 +185,12 @@ ruleTester.run('no-this-in-sfc', rule, { return
{this.props.foo}
; }`, errors: [{message: ERROR_MESSAGE}, {message: ERROR_MESSAGE}] + }, { + code: ` + () => { + this.something(); + return null; + }`, + errors: [{message: ERROR_MESSAGE}] }] }); From ac78b513430bbb46c84846e6ea021880f86b36e2 Mon Sep 17 00:00:00 2001 From: atomcorp Date: Wed, 19 Sep 2018 23:10:21 +0100 Subject: [PATCH 29/49] quotes for json key, reordered information --- docs/rules/jsx-no-target-blank.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/docs/rules/jsx-no-target-blank.md b/docs/rules/jsx-no-target-blank.md index 438dc2f08a..dd0cdb4c9b 100644 --- a/docs/rules/jsx-no-target-blank.md +++ b/docs/rules/jsx-no-target-blank.md @@ -12,21 +12,17 @@ This rule aims to prevent user generated links from creating security vulnerabil `rel='noreferrer noopener'` for external links, and optionally any dynamically generated links. ## Rule Options - -There are two main options for the rule: - -* `{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default) -* `{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link - ```json -"react/jsx-no-target-blank": [, { enforceDynamicLinks: }] +... +"react/jsx-no-target-blank": [, { "enforceDynamicLinks": }] +... ``` * enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. -* enforce: optional string, defaults to "always" - +* enforce: optional string, 'always' or 'never' ### always (default) +`{"enforceDynamicLinks": "always"}` enforces the rule if the href is a dynamic link (default) When {"enforceDynamicLinks": "always"} is set, the following patterns are considered errors: @@ -47,6 +43,8 @@ var Hello = ### never +`{"enforceDynamicLinks": "never"}` does not enforce the rule if the href is a dynamic link + When {"enforceDynamicLinks": "never"} is set, the following patterns are **not** considered errors: ```jsx From e0d5821436b6d4b9f615a04113f01b854342cd4b Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Thu, 20 Sep 2018 03:18:38 +0300 Subject: [PATCH 30/49] Add a test to cover FunctionDeclaration type as a parent scope --- tests/lib/rules/no-this-in-sfc.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index 8812055136..63a849f840 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -192,5 +192,18 @@ ruleTester.run('no-this-in-sfc', rule, { return null; }`, errors: [{message: ERROR_MESSAGE}] + }, { + code: ` + class Foo { + bar() { + function Bar(){ + return () => { + this.something(); + return null; + } + } + } + }`, + errors: [{message: ERROR_MESSAGE}] }] }); From dd6f130a8c2c7b036b3fee2dff25af83a64861a7 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 14:18:22 -0700 Subject: [PATCH 31/49] Extract used prop types detection code --- lib/rules/no-unused-prop-types.js | 451 +------------------------- lib/rules/prop-types.js | 354 +------------------- lib/util/Components.js | 14 +- lib/util/usedPropTypes.js | 520 ++++++++++++++++++++++++++++++ 4 files changed, 533 insertions(+), 806 deletions(-) create mode 100644 lib/util/usedPropTypes.js diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 3948d67398..ed4681470a 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -9,20 +9,8 @@ const has = require('has'); const Components = require('../util/Components'); -const astUtil = require('../util/ast'); -const versionUtil = require('../util/version'); const docsUrl = require('../util/docsUrl'); -// ------------------------------------------------------------------------------ -// Constants -// ------------------------------------------------------------------------------ - -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']; -const ASYNC_SAFE_LIFE_CYCLE_METHODS = ['getDerivedStateFromProps', 'getSnapshotBeforeUpdate', 'UNSAFE_componentWillReceiveProps', 'UNSAFE_componentWillUpdate']; - // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -53,91 +41,11 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { - const sourceCode = context.getSourceCode(); - const checkAsyncSafeLifeCycles = versionUtil.testReactVersion(context, '16.3.0'); + create: Components.detect((context, components) => { const defaults = {skipShapeProps: true, customValidators: []}; const configuration = Object.assign({}, defaults, context.options[0] || {}); const UNUSED_MESSAGE = '\'{{name}}\' PropType is defined but prop is never used'; - /** - * 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) { - const name = scope.block.parent.key.name; - - if (LIFE_CYCLE_METHODS.indexOf(name) >= 0) { - return true; - } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(name) >= 0) { - return true; - } - } - scope = scope.upper; - } - return false; - } - - /** - * Check if the current node is in a setState updater method - * @return {boolean} true if we are in a setState updater, false if not - */ - function inSetStateUpdater() { - let scope = context.getScope(); - while (scope) { - if ( - scope.block && scope.block.parent - && scope.block.parent.type === 'CallExpression' - && scope.block.parent.callee.property - && scope.block.parent.callee.property.name === 'setState' - // Make sure we are in the updater not the callback - && scope.block.parent.arguments[0].start === scope.block.start - ) { - return true; - } - scope = scope.upper; - } - return false; - } - - function isPropArgumentInSetStateUpdater(node) { - let scope = context.getScope(); - while (scope) { - if ( - scope.block && scope.block.parent - && scope.block.parent.type === 'CallExpression' - && scope.block.parent.callee.property - && scope.block.parent.callee.property.name === 'setState' - // Make sure we are in the updater not the callback - && scope.block.parent.arguments[0].start === scope.block.start - && scope.block.parent.arguments[0].params - && scope.block.parent.arguments[0].params.length > 1 - ) { - return scope.block.parent.arguments[0].params[1].name === node.object.name; - } - scope = scope.upper; - } - return false; - } - - /** - * Checks if we are using a prop - * @param {ASTNode} node The AST node being checked. - * @returns {Boolean} True if we are using a prop, false if not. - */ - function isPropTypesUsage(node) { - const isClassUsage = ( - (utils.getParentES6Component() || utils.getParentES5Component()) && - ((node.object.type === 'ThisExpression' && node.property.name === 'props') - || isPropArgumentInSetStateUpdater(node)) - ); - const isStatelessFunctionUsage = node.object.name === 'props'; - return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod(); - } - /** * Checks if the component must be validated * @param {Object} component The component to process @@ -150,56 +58,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; - - if (node.kind === 'constructor') { - return true; - } else if (LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { - return true; - } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { - return true; - } - - return false; - } - - /** - * 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' || node.type === 'Property') && 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. - * @returns {Boolean} True if the prop name matches - */ - function isPropAttributeName (node) { - return ( - node.init.name === 'props' || - node.init.name === 'nextProps' || - node.init.name === 'prevProps' - ); - } - /** * Checks if a prop is used * @param {ASTNode} node The AST node being checked. @@ -222,229 +80,6 @@ module.exports = { return false; } - /** - * Checks if the prop has spread operator. - * @param {ASTNode} node The AST node being marked. - * @returns {Boolean} True if the prop has spread operator, false if not. - */ - function hasSpreadOperator(node) { - const tokens = sourceCode.getTokens(node); - return tokens.length && tokens[0].value === '...'; - } - - /** - * Removes quotes from around an identifier. - * @param {string} the identifier to strip - */ - function stripQuotes(string) { - return string.replace(/^\'|\'$/g, ''); - } - - /** - * Retrieve the name of a key node - * @param {ASTNode} node The AST node with the key. - * @return {string} the name of the key - */ - function getKeyValue(node) { - if (node.type === 'ObjectTypeProperty') { - const tokens = context.getFirstTokens(node, 2); - return (tokens[0].value === '+' || tokens[0].value === '-' - ? tokens[1].value - : stripQuotes(tokens[0].value) - ); - } - const key = node.key || node.argument; - return key.type === 'Identifier' ? key.name : key.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; - } - - /** - * Retrieve the name of a property node - * @param {ASTNode} node The AST node with the property. - * @return {string} the name of the property or undefined if not found - */ - function getPropertyName(node) { - const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); - const isDirectNextProp = DIRECT_NEXT_PROPS_REGEX.test(sourceCode.getText(node)); - 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 isNotInSetStateUpdater = !inSetStateUpdater(); - if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp) - && isInClassComponent - && isNotInConstructor - && isNotInLifeCycleMethod - && isNotInSetStateUpdater - ) { - return void 0; - } - if (!isDirectProp && !isDirectNextProp && !isDirectPrevProp && !isDirectSetStateProp) { - node = node.parent; - } - const property = node.property; - if (property) { - switch (property.type) { - case 'Identifier': - if (node.computed) { - return '__COMPUTED_PROP__'; - } - return property.name; - case 'MemberExpression': - return void 0; - case 'Literal': - // Accept computed properties that are literal strings - if (typeof property.value === 'string') { - return property.value; - } - // falls through - default: - if (node.computed) { - return '__COMPUTED_PROP__'; - } - break; - } - } - return void 0; - } - - /** - * Mark a prop type as used - * @param {ASTNode} node The AST node being marked. - */ - function markPropTypesAsUsed(node, parentNames) { - parentNames = parentNames || []; - let type; - let name; - let allNames; - let properties; - switch (node.type) { - case 'MemberExpression': - name = getPropertyName(node); - if (name) { - allNames = parentNames.concat(name); - if (node.parent.type === 'MemberExpression') { - markPropTypesAsUsed(node.parent, allNames); - } - // Do not mark computed props as used. - type = name !== '__COMPUTED_PROP__' ? 'direct' : null; - } else if ( - node.parent.id && - node.parent.id.properties && - node.parent.id.properties.length && - getKeyValue(node.parent.id.properties[0]) - ) { - type = 'destructuring'; - properties = node.parent.id.properties; - } - break; - case 'ArrowFunctionExpression': - case 'FunctionDeclaration': - case 'FunctionExpression': - if (node.params.length === 0) { - break; - } - type = 'destructuring'; - properties = node.params[0].properties; - if (inSetStateUpdater()) { - properties = node.params[1].properties; - } - break; - case 'VariableDeclarator': - for (let i = 0, j = node.id.properties.length; i < j; i++) { - // let {props: {firstname}} = this - const thisDestructuring = ( - node.id.properties[i].key && ( - (node.id.properties[i].key.name === 'props' || node.id.properties[i].key.value === 'props') && - node.id.properties[i].value.type === 'ObjectPattern' - ) - ); - // let {firstname} = props - const genericDestructuring = isPropAttributeName(node) && ( - utils.getParentStatelessComponent() || - isInLifeCycleMethod(node) - ); - - if (thisDestructuring) { - properties = node.id.properties[i].value.properties; - } else if (genericDestructuring) { - properties = node.id.properties; - } else { - continue; - } - type = 'destructuring'; - break; - } - break; - default: - throw new Error(`${node.type} ASTNodes are not handled by markPropTypesAsUsed`); - } - - const component = components.get(utils.getParentComponent()); - const usedPropTypes = component && component.usedPropTypes || []; - let ignoreUnusedPropTypesValidation = component && component.ignoreUnusedPropTypesValidation || false; - - switch (type) { - case 'direct': - // Ignore Object methods - if (Object.prototype[name]) { - break; - } - - usedPropTypes.push({ - name: name, - allNames: allNames - }); - break; - case 'destructuring': - for (let k = 0, l = (properties || []).length; k < l; k++) { - if (hasSpreadOperator(properties[k]) || properties[k].computed) { - ignoreUnusedPropTypesValidation = true; - break; - } - const propName = getKeyValue(properties[k]); - - let currentNode = node; - allNames = []; - while (currentNode.property && currentNode.property.name !== 'props') { - allNames.unshift(currentNode.property.name); - currentNode = currentNode.object; - } - allNames.push(propName); - - if (propName) { - usedPropTypes.push({ - allNames: allNames, - name: propName - }); - } - } - break; - default: - break; - } - - components.set(component ? component.node : node, { - usedPropTypes: usedPropTypes, - ignoreUnusedPropTypesValidation: ignoreUnusedPropTypesValidation - }); - } - /** * Used to recursively loop through each declared prop type * @param {Object} component The component to process @@ -490,94 +125,11 @@ module.exports = { reportUnusedPropType(component, component.declaredPropTypes); } - /** - * @param {ASTNode} node We expect either an ArrowFunctionExpression, - * FunctionDeclaration, or FunctionExpression - */ - function markDestructuredFunctionArgumentsAsUsed(node) { - const destructuring = node.params && node.params[0] && node.params[0].type === 'ObjectPattern'; - if (destructuring && components.get(node)) { - markPropTypesAsUsed(node); - } - } - - function handleSetStateUpdater(node) { - if (!node.params || node.params.length < 2 || !inSetStateUpdater()) { - return; - } - markPropTypesAsUsed(node); - } - - /** - * Handle both stateless functions and setState updater functions. - * @param {ASTNode} node We expect either an ArrowFunctionExpression, - * FunctionDeclaration, or FunctionExpression - */ - function handleFunctionLikeExpressions(node) { - handleSetStateUpdater(node); - markDestructuredFunctionArgumentsAsUsed(node); - } - - function handleCustomValidators(component) { - const propTypes = component.declaredPropTypes; - if (!propTypes) { - return; - } - - Object.keys(propTypes).forEach(key => { - const node = propTypes[key].node; - - if (astUtil.isFunctionLikeExpression(node)) { - markPropTypesAsUsed(node); - } - }); - } - // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- return { - VariableDeclarator: function(node) { - const destructuring = node.init && node.id && node.id.type === 'ObjectPattern'; - // let {props: {firstname}} = this - const thisDestructuring = destructuring && node.init.type === 'ThisExpression'; - // let {firstname} = props - const statelessDestructuring = destructuring && isPropAttributeName(node) && ( - utils.getParentStatelessComponent() || - isInLifeCycleMethod(node) - ); - - if (!thisDestructuring && !statelessDestructuring) { - return; - } - markPropTypesAsUsed(node); - }, - - FunctionDeclaration: handleFunctionLikeExpressions, - - ArrowFunctionExpression: handleFunctionLikeExpressions, - - FunctionExpression: handleFunctionLikeExpressions, - - MemberExpression: function(node) { - if (isPropTypesUsage(node)) { - markPropTypesAsUsed(node); - } - }, - - 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)) { - node.properties.forEach((property, i) => { - if (i === 0) { - markPropTypesAsUsed(node.parent); - } - }); - } - }, - 'Program:exit': function() { const list = components.list(); // Report undeclared proptypes for all classes @@ -585,7 +137,6 @@ module.exports = { if (!has(list, component) || !mustBeValidated(list[component])) { continue; } - handleCustomValidators(list[component]); reportUnusedPropTypes(list[component]); } } diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index c35e811bb4..235502766d 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -11,13 +11,6 @@ const has = require('has'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); -// ------------------------------------------------------------------------------ -// Constants -// ------------------------------------------------------------------------------ - -const PROPS_REGEX = /^(props|nextProps)$/; -const DIRECT_PROPS_REGEX = /^(props|nextProps)\s*(\.|\[)/; - // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -54,94 +47,13 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { - const sourceCode = context.getSourceCode(); + create: Components.detect((context, components) => { const configuration = context.options[0] || {}; const ignored = configuration.ignore || []; const skipUndeclared = configuration.skipUndeclared || false; const MISSING_MESSAGE = '\'{{name}}\' is missing in props validation'; - /** - * 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; - } - - /** - * Check if we are in a class constructor - * @return {boolean} true if we are in a class constructor, false if not - */ - function inShouldComponentUpdate() { - let scope = context.getScope(); - while (scope) { - if ( - scope.block && scope.block.parent && - scope.block.parent.key && scope.block.parent.key.name === 'shouldComponentUpdate' - ) { - 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. - * @returns {Boolean} - */ - - function isAssignmentToProp(node) { - return ( - node.parent && - node.parent.type === 'AssignmentExpression' && - node.parent.left === node - ); - } - - /** - * Checks if we are using a prop - * @param {ASTNode} node The AST node being checked. - * @returns {Boolean} True if we are using a prop, false if not. - */ - function isPropTypesUsage(node) { - const isClassUsage = ( - (utils.getParentES6Component() || utils.getParentES5Component()) && - node.object.type === 'ThisExpression' && node.property.name === 'props' - ); - const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node); - const isNextPropsUsage = node.object.name === 'nextProps' && (inComponentWillReceiveProps() || inShouldComponentUpdate()); - return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage; - } - /** * Checks if the prop is ignored * @param {String} name Name of the prop to check. @@ -247,214 +159,6 @@ module.exports = { return false; } - /** - * Checks if the prop has spread operator. - * @param {ASTNode} node The AST node being marked. - * @returns {Boolean} True if the prop has spread operator, false if not. - */ - function hasSpreadOperator(node) { - const tokens = sourceCode.getTokens(node); - return tokens.length && tokens[0].value === '...'; - } - - /** - * Removes quotes from around an identifier. - * @param {string} the identifier to strip - */ - function stripQuotes(string) { - return string.replace(/^\'|\'$/g, ''); - } - - /** - * Retrieve the name of a key node - * @param {ASTNode} node The AST node with the key. - * @return {string} the name of the key - */ - function getKeyValue(node) { - if (node.type === 'ObjectTypeProperty') { - const tokens = context.getFirstTokens(node, 2); - return (tokens[0].value === '+' || tokens[0].value === '-' - ? tokens[1].value - : stripQuotes(tokens[0].value) - ); - } - const key = node.key || node.argument; - return key.type === 'Identifier' ? key.name : key.value; - } - - /** - * Retrieve the name of a property node - * @param {ASTNode} node The AST node with the property. - * @return {string} the name of the property or undefined if not found - */ - function getPropertyName(node) { - const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); - const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); - const isNotInConstructor = !inConstructor(); - const isNotInComponentWillReceiveProps = !inComponentWillReceiveProps(); - const isNotInShouldComponentUpdate = !inShouldComponentUpdate(); - if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps - && isNotInShouldComponentUpdate) { - return void 0; - } - if (!isDirectProp) { - node = node.parent; - } - const property = node.property; - if (property) { - switch (property.type) { - case 'Identifier': - if (node.computed) { - return '__COMPUTED_PROP__'; - } - return property.name; - case 'MemberExpression': - return void 0; - case 'Literal': - // Accept computed properties that are literal strings - if (typeof property.value === 'string') { - return property.value; - } - // falls through - default: - if (node.computed) { - return '__COMPUTED_PROP__'; - } - break; - } - } - return void 0; - } - - /** - * Mark a prop type as used - * @param {ASTNode} node The AST node being marked. - */ - function markPropTypesAsUsed(node, parentNames) { - parentNames = parentNames || []; - let type; - let name; - let allNames; - let properties; - switch (node.type) { - case 'MemberExpression': - name = getPropertyName(node); - if (name) { - allNames = parentNames.concat(name); - if (node.parent.type === 'MemberExpression') { - markPropTypesAsUsed(node.parent, allNames); - } - // Do not mark computed props as used. - type = name !== '__COMPUTED_PROP__' ? 'direct' : null; - } else if ( - node.parent.id && - node.parent.id.properties && - node.parent.id.properties.length && - getKeyValue(node.parent.id.properties[0]) - ) { - type = 'destructuring'; - properties = node.parent.id.properties; - } - break; - case 'ArrowFunctionExpression': - case 'FunctionDeclaration': - case 'FunctionExpression': - type = 'destructuring'; - properties = node.params[0].properties; - break; - case 'MethodDefinition': - const destructuring = node.value && node.value.params && node.value.params[0] && node.value.params[0].type === 'ObjectPattern'; - if (destructuring) { - type = 'destructuring'; - properties = node.value.params[0].properties; - break; - } else { - return; - } - case 'VariableDeclarator': - for (let i = 0, j = node.id.properties.length; i < j; i++) { - // let {props: {firstname}} = this - const thisDestructuring = ( - !hasSpreadOperator(node.id.properties[i]) && - (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 - const directDestructuring = - PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) - ; - - if (thisDestructuring) { - properties = node.id.properties[i].value.properties; - } else if (directDestructuring) { - properties = node.id.properties; - } else { - continue; - } - type = 'destructuring'; - break; - } - break; - default: - throw new Error(`${node.type} ASTNodes are not handled by markPropTypesAsUsed`); - } - - const component = components.get(utils.getParentComponent()); - const usedPropTypes = (component && component.usedPropTypes || []).slice(); - - switch (type) { - case 'direct': - // Ignore Object methods - if (Object.prototype[name]) { - break; - } - - const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); - - usedPropTypes.push({ - name: name, - allNames: allNames, - node: ( - !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ? - node.parent.property : - node.property - ) - }); - break; - case 'destructuring': - for (let k = 0, l = properties.length; k < l; k++) { - if (hasSpreadOperator(properties[k]) || properties[k].computed) { - continue; - } - const propName = getKeyValue(properties[k]); - - let currentNode = node; - allNames = []; - while (currentNode.property && !PROPS_REGEX.test(currentNode.property.name)) { - allNames.unshift(currentNode.property.name); - currentNode = currentNode.object; - } - allNames.push(propName); - - if (propName) { - usedPropTypes.push({ - name: propName, - allNames: allNames, - node: properties[k] - }); - } - } - break; - default: - break; - } - - components.set(node, { - usedPropTypes: usedPropTypes - }); - } - /** * Reports undeclared proptypes for a given component * @param {Object} component The component to process @@ -478,67 +182,11 @@ module.exports = { } } - /** - * @param {ASTNode} node We expect either an ArrowFunctionExpression, - * FunctionDeclaration, or FunctionExpression - */ - function markDestructuredFunctionArgumentsAsUsed(node) { - const destructuring = node.params && node.params[0] && node.params[0].type === 'ObjectPattern'; - if (destructuring && components.get(node)) { - markPropTypesAsUsed(node); - } - } - // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- return { - VariableDeclarator: function(node) { - const destructuring = node.init && node.id && node.id.type === 'ObjectPattern'; - // let {props: {firstname}} = this - const thisDestructuring = destructuring && node.init.type === 'ThisExpression'; - // let {firstname} = props - const directDestructuring = - destructuring && - PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) - ; - - if (!thisDestructuring && !directDestructuring) { - return; - } - markPropTypesAsUsed(node); - }, - - FunctionDeclaration: markDestructuredFunctionArgumentsAsUsed, - - ArrowFunctionExpression: markDestructuredFunctionArgumentsAsUsed, - - FunctionExpression: function(node) { - if (node.parent.type === 'MethodDefinition') { - return; - } - markDestructuredFunctionArgumentsAsUsed(node); - }, - - MemberExpression: function(node) { - if (isPropTypesUsage(node)) { - markPropTypesAsUsed(node); - } - }, - - MethodDefinition: function(node) { - const destructuring = node.value && node.value.params && node.value.params[0] && node.value.params[0].type === 'ObjectPattern'; - if (node.key.name === 'componentWillReceiveProps' && destructuring) { - markPropTypesAsUsed(node); - } - - if (node.key.name === 'shouldComponentUpdate' && destructuring) { - markPropTypesAsUsed(node); - } - }, - 'Program:exit': function() { const list = components.list(); // Report undeclared proptypes for all classes diff --git a/lib/util/Components.js b/lib/util/Components.js index 8ed7bbd2c3..40f174441c 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -12,6 +12,7 @@ const pragmaUtil = require('./pragma'); const astUtil = require('./ast'); const propTypes = require('./propTypes'); const jsxUtil = require('./jsx'); +const usedPropTypesUtil = require('./usedPropTypes'); function getId(node) { return node && node.range.join(':'); @@ -37,6 +38,7 @@ function mergeUsedPropTypes(propsList, newPropsList) { propsToAdd.push(newProp); } }); + return propsList.concat(propsToAdd); } @@ -142,7 +144,8 @@ class Components { const newUsedProps = (this._list[i].usedPropTypes || []).filter(propType => !propType.node || propType.node.kind !== 'init'); const componentId = getId(component.node); - usedPropTypes[componentId] = (usedPropTypes[componentId] || []).concat(newUsedProps); + + usedPropTypes[componentId] = mergeUsedPropTypes(usedPropTypes[componentId] || [], newUsedProps); } } @@ -154,7 +157,7 @@ class Components { const id = getId(this._list[j].node); list[j] = this._list[j]; if (usedPropTypes[id]) { - list[j].usedPropTypes = (list[j].usedPropTypes || []).concat(usedPropTypes[id]); + list[j].usedPropTypes = mergeUsedPropTypes(list[j].usedPropTypes || [], usedPropTypes[id]); } } return list; @@ -720,7 +723,9 @@ function componentRule(rule, context) { const ruleInstructions = rule(context, components, utils); const updatedRuleInstructions = util._extend({}, ruleInstructions); const propTypesInstructions = propTypes(context, components, utils); - const allKeys = new Set(Object.keys(detectionInstructions).concat(Object.keys(propTypesInstructions))); + const usedPropTypesInstructions = usedPropTypesUtil(context, components, utils); + const allKeys = new Set(Object.keys(detectionInstructions).concat(Object.keys(propTypesInstructions)) + .concat(Object.keys(usedPropTypesInstructions))); allKeys.forEach(instruction => { updatedRuleInstructions[instruction] = function(node) { if (instruction in detectionInstructions) { @@ -729,6 +734,9 @@ function componentRule(rule, context) { if (instruction in propTypesInstructions) { propTypesInstructions[instruction](node); } + if (instruction in usedPropTypesInstructions) { + usedPropTypesInstructions[instruction](node); + } return ruleInstructions[instruction] ? ruleInstructions[instruction](node) : void 0; }; }); diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js new file mode 100644 index 0000000000..3eecf33acc --- /dev/null +++ b/lib/util/usedPropTypes.js @@ -0,0 +1,520 @@ +/** + * @fileoverview Common used propTypes detection functionality. + */ +'use strict'; + +const has = require('has'); +const astUtil = require('./ast'); +const versionUtil = require('./version'); + +// ------------------------------------------------------------------------------ +// Constants +// ------------------------------------------------------------------------------ + +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']; +const ASYNC_SAFE_LIFE_CYCLE_METHODS = ['getDerivedStateFromProps', 'getSnapshotBeforeUpdate', 'UNSAFE_componentWillReceiveProps', 'UNSAFE_componentWillUpdate']; + +/** + * Checks if a prop init name matches common naming patterns + * @param {ASTNode} node The AST node being checked. + * @returns {Boolean} True if the prop name matches + */ +function isPropAttributeName (node) { + return ( + node.init.name === 'props' || + node.init.name === 'nextProps' || + node.init.name === 'prevProps' + ); +} + +/** + * Checks if the component must be validated + * @param {Object} component The component to process + * @returns {Boolean} True if the component must be validated, false if not. + */ +function mustBeValidated(component) { + return Boolean( + component && + !component.ignorePropsValidation + ); +} + +module.exports = function usedPropTypesInstructions(context, components, utils) { + const sourceCode = context.getSourceCode(); + const checkAsyncSafeLifeCycles = versionUtil.testReactVersion(context, '16.3.0'); + + /** + * 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; + } + + /** + * 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) { + const name = scope.block.parent.key.name; + + if (LIFE_CYCLE_METHODS.indexOf(name) >= 0) { + return true; + } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(name) >= 0) { + return true; + } + } + scope = scope.upper; + } + return false; + } + + /** + * 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; + + if (node.kind === 'constructor') { + return true; + } else if (LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { + return true; + } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { + return true; + } + + return false; + } + + /** + * 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' || node.type === 'Property') && isNodeALifeCycleMethod(node)) { + return true; + } + + if (node.parent) { + return isInLifeCycleMethod(node.parent); + } + + return false; + } + + /** + * Check if the current node is in a setState updater method + * @return {boolean} true if we are in a setState updater, false if not + */ + function inSetStateUpdater() { + let scope = context.getScope(); + while (scope) { + if ( + scope.block && scope.block.parent + && scope.block.parent.type === 'CallExpression' + && scope.block.parent.callee.property + && scope.block.parent.callee.property.name === 'setState' + // Make sure we are in the updater not the callback + && scope.block.parent.arguments[0].start === scope.block.start + ) { + return true; + } + scope = scope.upper; + } + return false; + } + + function isPropArgumentInSetStateUpdater(node) { + let scope = context.getScope(); + while (scope) { + if ( + scope.block && scope.block.parent + && scope.block.parent.type === 'CallExpression' + && scope.block.parent.callee.property + && scope.block.parent.callee.property.name === 'setState' + // Make sure we are in the updater not the callback + && scope.block.parent.arguments[0].start === scope.block.start + && scope.block.parent.arguments[0].params + && scope.block.parent.arguments[0].params.length > 1 + ) { + return scope.block.parent.arguments[0].params[1].name === node.object.name; + } + scope = scope.upper; + } + return false; + } + + /** + * Checks if the prop has spread operator. + * @param {ASTNode} node The AST node being marked. + * @returns {Boolean} True if the prop has spread operator, false if not. + */ + function hasSpreadOperator(node) { + const tokens = sourceCode.getTokens(node); + return tokens.length && tokens[0].value === '...'; + } + + /** + * Removes quotes from around an identifier. + * @param {string} the identifier to strip + */ + function stripQuotes(string) { + return string.replace(/^\'|\'$/g, ''); + } + + /** + * Retrieve the name of a key node + * @param {ASTNode} node The AST node with the key. + * @return {string} the name of the key + */ + function getKeyValue(node) { + if (node.type === 'ObjectTypeProperty') { + const tokens = context.getFirstTokens(node, 2); + return (tokens[0].value === '+' || tokens[0].value === '-' + ? tokens[1].value + : stripQuotes(tokens[0].value) + ); + } + const key = node.key || node.argument; + return key.type === 'Identifier' ? key.name : key.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; + } + + /** + * Retrieve the name of a property node + * @param {ASTNode} node The AST node with the property. + * @return {string} the name of the property or undefined if not found + */ + function getPropertyName(node) { + const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); + const isDirectNextProp = DIRECT_NEXT_PROPS_REGEX.test(sourceCode.getText(node)); + 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 isNotInSetStateUpdater = !inSetStateUpdater(); + if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp) + && isInClassComponent + && isNotInConstructor + && isNotInLifeCycleMethod + && isNotInSetStateUpdater + ) { + return void 0; + } + if (!isDirectProp && !isDirectNextProp && !isDirectPrevProp && !isDirectSetStateProp) { + node = node.parent; + } + const property = node.property; + if (property) { + switch (property.type) { + case 'Identifier': + if (node.computed) { + return '__COMPUTED_PROP__'; + } + return property.name; + case 'MemberExpression': + return void 0; + case 'Literal': + // Accept computed properties that are literal strings + if (typeof property.value === 'string') { + return property.value; + } + // falls through + default: + if (node.computed) { + return '__COMPUTED_PROP__'; + } + break; + } + } + return void 0; + } + + /** + * Checks if a prop is being assigned a value props.bar = 'bar' + * @param {ASTNode} node The AST node being checked. + * @returns {Boolean} + */ + function isAssignmentToProp(node) { + return ( + node.parent && + node.parent.type === 'AssignmentExpression' && + node.parent.left === node + ); + } + + /** + * Checks if we are using a prop + * @param {ASTNode} node The AST node being checked. + * @returns {Boolean} True if we are using a prop, false if not. + */ + function isPropTypesUsage(node) { + const isClassUsage = ( + (utils.getParentES6Component() || utils.getParentES5Component()) && + ((node.object.type === 'ThisExpression' && node.property.name === 'props') + || isPropArgumentInSetStateUpdater(node)) + ); + const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node); + return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod(); + } + + /** + * Mark a prop type as used + * @param {ASTNode} node The AST node being marked. + */ + function markPropTypesAsUsed(node, parentNames) { + parentNames = parentNames || []; + let type; + let name; + let allNames; + let properties; + switch (node.type) { + case 'MemberExpression': + name = getPropertyName(node); + if (name) { + allNames = parentNames.concat(name); + if (node.parent.type === 'MemberExpression') { + markPropTypesAsUsed(node.parent, allNames); + } + // Do not mark computed props as used. + type = name !== '__COMPUTED_PROP__' ? 'direct' : null; + } else if ( + node.parent.id && + node.parent.id.properties && + node.parent.id.properties.length && + getKeyValue(node.parent.id.properties[0]) + ) { + type = 'destructuring'; + properties = node.parent.id.properties; + } + break; + case 'ArrowFunctionExpression': + case 'FunctionDeclaration': + case 'FunctionExpression': + if (node.params.length === 0) { + break; + } + type = 'destructuring'; + properties = node.params[0].properties; + if (inSetStateUpdater()) { + properties = node.params[1].properties; + } + break; + case 'VariableDeclarator': + for (let i = 0, j = node.id.properties.length; i < j; i++) { + // let {props: {firstname}} = this + const thisDestructuring = ( + node.id.properties[i].key && ( + (node.id.properties[i].key.name === 'props' || node.id.properties[i].key.value === 'props') && + node.id.properties[i].value.type === 'ObjectPattern' + ) + ); + // let {firstname} = props + const genericDestructuring = isPropAttributeName(node) && ( + utils.getParentStatelessComponent() || + isInLifeCycleMethod(node) + ); + + if (thisDestructuring) { + properties = node.id.properties[i].value.properties; + } else if (genericDestructuring) { + properties = node.id.properties; + } else { + continue; + } + type = 'destructuring'; + break; + } + break; + default: + throw new Error(`${node.type} ASTNodes are not handled by markPropTypesAsUsed`); + } + + const component = components.get(utils.getParentComponent()); + const usedPropTypes = component && component.usedPropTypes || []; + let ignoreUnusedPropTypesValidation = component && component.ignoreUnusedPropTypesValidation || false; + + switch (type) { + case 'direct': + // Ignore Object methods + if (Object.prototype[name]) { + break; + } + + const nodeSource = sourceCode.getText(node); + const isDirectProp = DIRECT_PROPS_REGEX.test(nodeSource) || DIRECT_NEXT_PROPS_REGEX.test(nodeSource); + usedPropTypes.push({ + name: name, + allNames: allNames, + node: ( + !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ? + node.parent.property : + node.property + ) + }); + break; + case 'destructuring': + for (let k = 0, l = (properties || []).length; k < l; k++) { + if (hasSpreadOperator(properties[k]) || properties[k].computed) { + ignoreUnusedPropTypesValidation = true; + break; + } + const propName = getKeyValue(properties[k]); + + let currentNode = node; + allNames = []; + while (currentNode.property && currentNode.property.name !== 'props') { + allNames.unshift(currentNode.property.name); + currentNode = currentNode.object; + } + allNames.push(propName); + if (propName) { + usedPropTypes.push({ + allNames: allNames, + name: propName, + node: properties[k] + }); + } + } + break; + default: + break; + } + + components.set(component ? component.node : node, { + usedPropTypes: usedPropTypes, + ignoreUnusedPropTypesValidation: ignoreUnusedPropTypesValidation + }); + } + + /** + * @param {ASTNode} node We expect either an ArrowFunctionExpression, + * FunctionDeclaration, or FunctionExpression + */ + function markDestructuredFunctionArgumentsAsUsed(node) { + const destructuring = node.params && node.params[0] && node.params[0].type === 'ObjectPattern'; + if (destructuring && components.get(node)) { + markPropTypesAsUsed(node); + } + } + + function handleSetStateUpdater(node) { + if (!node.params || node.params.length < 2 || !inSetStateUpdater()) { + return; + } + markPropTypesAsUsed(node); + } + + /** + * Handle both stateless functions and setState updater functions. + * @param {ASTNode} node We expect either an ArrowFunctionExpression, + * FunctionDeclaration, or FunctionExpression + */ + function handleFunctionLikeExpressions(node) { + handleSetStateUpdater(node); + markDestructuredFunctionArgumentsAsUsed(node); + } + + function handleCustomValidators(component) { + const propTypes = component.declaredPropTypes; + if (!propTypes) { + return; + } + + Object.keys(propTypes).forEach(key => { + const node = propTypes[key].node; + + if (astUtil.isFunctionLikeExpression(node)) { + markPropTypesAsUsed(node); + } + }); + } + + return { + VariableDeclarator: function(node) { + const destructuring = node.init && node.id && node.id.type === 'ObjectPattern'; + // let {props: {firstname}} = this + const thisDestructuring = destructuring && node.init.type === 'ThisExpression'; + // let {firstname} = props + const statelessDestructuring = destructuring && isPropAttributeName(node) && ( + utils.getParentStatelessComponent() || + isInLifeCycleMethod(node) + ); + + if (!thisDestructuring && !statelessDestructuring) { + return; + } + markPropTypesAsUsed(node); + }, + + FunctionDeclaration: handleFunctionLikeExpressions, + + ArrowFunctionExpression: handleFunctionLikeExpressions, + + FunctionExpression: handleFunctionLikeExpressions, + + MemberExpression: function(node) { + if (isPropTypesUsage(node)) { + markPropTypesAsUsed(node); + } + }, + + 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)) { + node.properties.forEach((property, i) => { + if (i === 0) { + markPropTypesAsUsed(node.parent); + } + }); + } + }, + + 'Program:exit': function() { + const list = components.list(); + + for (const component in list) { + if (!has(list, component) || !mustBeValidated(list[component])) { + continue; + } + handleCustomValidators(list[component]); + } + } + }; +}; From deae58e9fb4eb46d64d05c4b15cbfa186b9e7c0d Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 18 Aug 2018 22:59:31 -0700 Subject: [PATCH 32/49] Add test cases --- tests/lib/rules/prop-types.js | 82 +++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 4cbe55fa26..4b54a0531f 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -2030,6 +2030,44 @@ ruleTester.run('prop-types', rule, { Slider.propTypes = RcSlider.propTypes; ` + }, + { + code: ` + class Foo extends React.Component { + bar() { + this.setState((state, props) => ({ current: props.current })); + } + render() { + return
; + } + } + + Foo.propTypes = { + current: PropTypes.number.isRequired, + }; + ` + }, + { + code: ` + class Foo extends React.Component { + static getDerivedStateFromProps(props) { + const { foo } = props; + return { + foobar: foo + }; + } + + render() { + const { foobar } = this.state; + return
{foobar}
; + } + } + + Foo.propTypes = { + foo: PropTypes.func.isRequired, + }; + `, + settings: {react: {version: '16.3.0'}} } ], @@ -3851,6 +3889,50 @@ ruleTester.run('prop-types', rule, { `, errors: [{ message: '\'foo.baz\' is missing in props validation' + }], + }, + { + code: ` + class Foo extends React.Component { + bar() { + this.setState((state, props) => ({ current: props.current, bar: props.bar })); + } + render() { + return
; + } + } + + Foo.propTypes = { + current: PropTypes.number.isRequired, + }; + `, + errors: [{ + message: '\'bar\' is missing in props validation' + }] + }, + { + code: ` + class Foo extends React.Component { + static getDerivedStateFromProps(props) { + const { foo, bar } = props; + return { + foobar: foo + bar + }; + } + + render() { + const { foobar } = this.state; + return
{foobar}
; + } + } + + Foo.propTypes = { + foo: PropTypes.func.isRequired, + }; + `, + settings: {react: {version: '16.3.0'}}, + errors: [{ + message: '\'bar\' is missing in props validation' }] }, { From 3794588615f40e6f2089a9482d89d5a662c1808a Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Mon, 20 Aug 2018 11:55:30 -0700 Subject: [PATCH 33/49] Move used prop type flag setting to new helper --- lib/util/propTypes.js | 7 ------- lib/util/usedPropTypes.js | 7 +++++++ tests/lib/rules/prop-types.js | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index d58af934de..f0ca66ee8e 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -666,13 +666,6 @@ module.exports = function propTypesInstructions(context, components, utils) { } }, - JSXSpreadAttribute: function(node) { - const component = components.get(utils.getParentComponent()); - components.set(component ? component.node : node, { - ignoreUnusedPropTypesValidation: true - }); - }, - TypeAlias: function(node) { setInTypeScope(node.id.name, node.right); }, diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js index 3eecf33acc..dde379d898 100644 --- a/lib/util/usedPropTypes.js +++ b/lib/util/usedPropTypes.js @@ -488,6 +488,13 @@ module.exports = function usedPropTypesInstructions(context, components, utils) FunctionExpression: handleFunctionLikeExpressions, + JSXSpreadAttribute: function(node) { + const component = components.get(utils.getParentComponent()); + components.set(component ? component.node : node, { + ignoreUnusedPropTypesValidation: true + }); + }, + MemberExpression: function(node) { if (isPropTypesUsage(node)) { markPropTypesAsUsed(node); diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 4b54a0531f..25d65635d3 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -3889,7 +3889,7 @@ ruleTester.run('prop-types', rule, { `, errors: [{ message: '\'foo.baz\' is missing in props validation' - }], + }] }, { code: ` From cf43b2e71d445b73dc4f93b542faf42fe83affcb Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Tue, 21 Aug 2018 08:25:53 -0700 Subject: [PATCH 34/49] Replace for..in with Array.prototype methods --- lib/rules/boolean-prop-naming.js | 3 +-- lib/rules/default-props-match-prop-types.js | 9 ++----- lib/rules/display-name.js | 9 ++++--- lib/rules/no-multi-comp.js | 9 ++++--- lib/rules/no-set-state.js | 9 ++++--- lib/rules/no-unused-prop-types.js | 9 ++++--- lib/rules/prefer-stateless-function.js | 10 ++++---- lib/rules/prop-types.js | 9 ++++--- lib/rules/require-default-props.js | 11 +++------ lib/rules/require-optimization.js | 9 ++++--- lib/rules/require-render-return.js | 8 +++---- lib/rules/sort-comp.js | 7 ++---- lib/util/Components.js | 26 +++++++-------------- lib/util/usedPropTypes.js | 9 ++++--- 14 files changed, 52 insertions(+), 85 deletions(-) diff --git a/lib/rules/boolean-prop-naming.js b/lib/rules/boolean-prop-naming.js index f65b8101f4..f81fc14861 100644 --- a/lib/rules/boolean-prop-naming.js +++ b/lib/rules/boolean-prop-naming.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const propsUtil = require('../util/props'); const docsUrl = require('../util/docsUrl'); @@ -248,7 +247,7 @@ module.exports = { } } - if (!has(list, component) || (list[component].invalidProps || []).length) { + if ((list[component].invalidProps || []).length) { reportInvalidNaming(list[component]); } }); diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index 79723f18a8..203bd2db2d 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -5,7 +5,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const variableUtil = require('../util/variable'); const annotations = require('../util/annotations'); @@ -595,11 +594,7 @@ module.exports = { stack = null; const list = components.list(); - for (const component in list) { - if (!has(list, component)) { - continue; - } - + Object.keys(list).forEach(component => { // If no defaultProps could be found, we don't report anything. if (!list[component].defaultProps) { return; @@ -609,7 +604,7 @@ module.exports = { list[component].propTypes, list[component].defaultProps || {} ); - } + }); } }; }) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 4e40d3fcb7..9b1b25cc81 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const astUtil = require('../util/ast'); const docsUrl = require('../util/docsUrl'); @@ -216,12 +215,12 @@ module.exports = { 'Program:exit': function() { const list = components.list(); // Report missing display name for all components - for (const component in list) { - if (!has(list, component) || list[component].hasDisplayName) { - continue; + Object.keys(list).forEach(component => { + if (list[component].hasDisplayName) { + return; } reportMissingDisplayName(list[component]); - } + }); } }; }) diff --git a/lib/rules/no-multi-comp.js b/lib/rules/no-multi-comp.js index 4d6082d767..3c0cefa3e4 100644 --- a/lib/rules/no-multi-comp.js +++ b/lib/rules/no-multi-comp.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); @@ -61,15 +60,15 @@ module.exports = { const list = components.list(); let i = 0; - for (const component in list) { - if (!has(list, component) || isIgnored(list[component]) || ++i === 1) { - continue; + Object.keys(list).forEach(component => { + if (isIgnored(list[component]) || ++i === 1) { + return; } context.report({ node: list[component].node, message: MULTI_COMP_MESSAGE }); - } + }); } }; }) diff --git a/lib/rules/no-set-state.js b/lib/rules/no-set-state.js index 3b18b5c182..80890cdc16 100644 --- a/lib/rules/no-set-state.js +++ b/lib/rules/no-set-state.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); @@ -74,12 +73,12 @@ module.exports = { 'Program:exit': function() { const list = components.list(); - for (const component in list) { - if (!has(list, component) || isValid(list[component])) { - continue; + Object.keys(list).forEach(component => { + if (isValid(list[component])) { + return; } reportSetStateUsages(list[component]); - } + }); } }; }) diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index ed4681470a..2a218238d6 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -7,7 +7,6 @@ // As for exceptions for props.children or props.className (and alike) look at // https://github.com/yannickcr/eslint-plugin-react/issues/7 -const has = require('has'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); @@ -133,12 +132,12 @@ module.exports = { 'Program:exit': function() { const list = components.list(); // Report undeclared proptypes for all classes - for (const component in list) { - if (!has(list, component) || !mustBeValidated(list[component])) { - continue; + Object.keys(list).forEach(component => { + if (!mustBeValidated(list[component])) { + return; } reportUnusedPropTypes(list[component]); - } + }); } }; }) diff --git a/lib/rules/prefer-stateless-function.js b/lib/rules/prefer-stateless-function.js index 13f79193e5..4550cb54d1 100644 --- a/lib/rules/prefer-stateless-function.js +++ b/lib/rules/prefer-stateless-function.js @@ -6,7 +6,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const versionUtil = require('../util/version'); const astUtil = require('../util/ast'); @@ -357,9 +356,8 @@ module.exports = { 'Program:exit': function() { const list = components.list(); - for (const component in list) { + Object.keys(list).forEach(component => { if ( - !has(list, component) || hasOtherProperties(list[component].node) || list[component].useThis || list[component].useRef || @@ -368,17 +366,17 @@ module.exports = { list[component].useDecorators || (!utils.isES5Component(list[component].node) && !utils.isES6Component(list[component].node)) ) { - continue; + return; } if (list[component].hasSCU && list[component].usePropsOrContext) { - continue; + return; } context.report({ node: list[component].node, message: 'Component should be written as a pure function' }); - } + }); } }; }) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 235502766d..b6e4a82c6a 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -7,7 +7,6 @@ // As for exceptions for props.children or props.className (and alike) look at // https://github.com/yannickcr/eslint-plugin-react/issues/7 -const has = require('has'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); @@ -190,12 +189,12 @@ module.exports = { 'Program:exit': function() { const list = components.list(); // Report undeclared proptypes for all classes - for (const component in list) { - if (!has(list, component) || !mustBeValidated(list[component])) { - continue; + Object.keys(list).forEach(component => { + if (!mustBeValidated(list[component])) { + return; } reportUndeclaredPropTypes(list[component]); - } + }); } }; }) diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 0704e8e323..935564e632 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const variableUtil = require('../util/variable'); const annotations = require('../util/annotations'); @@ -626,21 +625,17 @@ module.exports = { stack = null; const list = components.list(); - for (const component in list) { - if (!has(list, component)) { - continue; - } - + Object.keys(list).forEach(component => { // If no propTypes could be found, we don't report anything. if (!list[component].propTypes) { - continue; + return; } reportPropTypesWithoutDefault( list[component].propTypes, list[component].defaultProps || {} ); - } + }); } }; }) diff --git a/lib/rules/require-optimization.js b/lib/rules/require-optimization.js index 670c7fc2ed..b9b8623f1b 100644 --- a/lib/rules/require-optimization.js +++ b/lib/rules/require-optimization.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); @@ -221,12 +220,12 @@ module.exports = { const list = components.list(); // Report missing shouldComponentUpdate for all components - for (const component in list) { - if (!has(list, component) || list[component].hasSCU) { - continue; + Object.keys(list).forEach(component => { + if (list[component].hasSCU) { + return; } reportMissingOptimization(list[component]); - } + }); } }; }) diff --git a/lib/rules/require-render-return.js b/lib/rules/require-render-return.js index 139465045e..0c20334482 100644 --- a/lib/rules/require-render-return.js +++ b/lib/rules/require-render-return.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const Components = require('../util/Components'); const astUtil = require('../util/ast'); const docsUrl = require('../util/docsUrl'); @@ -79,20 +78,19 @@ module.exports = { 'Program:exit': function() { const list = components.list(); - for (const component in list) { + Object.keys(list).forEach(component => { if ( - !has(list, component) || !hasRenderMethod(list[component].node) || list[component].hasReturnStatement || (!utils.isES5Component(list[component].node) && !utils.isES6Component(list[component].node)) ) { - continue; + return; } context.report({ node: list[component].node, message: 'Your render method should have return statement' }); - } + }); } }; }) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 4e2602a74e..3997021ad0 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -444,13 +444,10 @@ module.exports = { return { 'Program:exit': function() { const list = components.list(); - for (const component in list) { - if (!has(list, component)) { - continue; - } + Object.keys(list).forEach(component => { const properties = astUtil.getComponentProperties(list[component].node); checkPropsOrder(properties); - } + }); reportErrors(); } diff --git a/lib/util/Components.js b/lib/util/Components.js index 40f174441c..6c7b44304b 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -4,7 +4,6 @@ */ 'use strict'; -const has = require('has'); const util = require('util'); const doctrine = require('doctrine'); const variableUtil = require('./variable'); @@ -125,9 +124,9 @@ class Components { const usedPropTypes = {}; // Find props used in components for which we are not confident - for (const i in this._list) { - if (!has(this._list, i) || this._list[i].confidence >= 2) { - continue; + Object.keys(this._list).forEach(i => { + if (this._list[i].confidence >= 2) { + return; } let component = null; let node = null; @@ -147,19 +146,19 @@ class Components { usedPropTypes[componentId] = mergeUsedPropTypes(usedPropTypes[componentId] || [], newUsedProps); } - } + }); // Assign used props in not confident components to the parent component - for (const j in this._list) { - if (!has(this._list, j) || this._list[j].confidence < 2) { - continue; + Object.keys(this._list).forEach(j => { + if (this._list[j].confidence < 2) { + return; } const id = getId(this._list[j].node); list[j] = this._list[j]; if (usedPropTypes[id]) { list[j].usedPropTypes = mergeUsedPropTypes(list[j].usedPropTypes || [], usedPropTypes[id]); } - } + }); return list; } @@ -170,14 +169,7 @@ class Components { * @returns {Number} Components list length */ length() { - let length = 0; - for (const i in this._list) { - if (!has(this._list, i) || this._list[i].confidence < 2) { - continue; - } - length++; - } - return length; + return Object.keys(this._list).filter(i => this._list[i].confidence >= 2).length; } } diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js index dde379d898..269266096b 100644 --- a/lib/util/usedPropTypes.js +++ b/lib/util/usedPropTypes.js @@ -3,7 +3,6 @@ */ 'use strict'; -const has = require('has'); const astUtil = require('./ast'); const versionUtil = require('./version'); @@ -516,12 +515,12 @@ module.exports = function usedPropTypesInstructions(context, components, utils) 'Program:exit': function() { const list = components.list(); - for (const component in list) { - if (!has(list, component) || !mustBeValidated(list[component])) { - continue; + Object.keys(list).forEach(component => { + if (!mustBeValidated(list[component])) { + return; } handleCustomValidators(list[component]); - } + }); } }; }; From fd03bd79686efb6154861518ff39bb4ef1336d47 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 19 Sep 2018 21:32:41 -0700 Subject: [PATCH 35/49] Filter arrays instead of if conditions --- lib/rules/boolean-prop-naming.js | 2 +- lib/rules/display-name.js | 5 +---- lib/rules/no-multi-comp.js | 14 +++++------- lib/rules/no-set-state.js | 5 +---- lib/rules/no-unused-prop-types.js | 2 +- lib/rules/prop-types.js | 5 +---- lib/rules/require-default-props.js | 7 +----- lib/rules/require-optimization.js | 5 +---- lib/util/Components.js | 16 +++++--------- lib/util/usedPropTypes.js | 35 +++++++++++++----------------- 10 files changed, 34 insertions(+), 62 deletions(-) diff --git a/lib/rules/boolean-prop-naming.js b/lib/rules/boolean-prop-naming.js index f81fc14861..d1be3364aa 100644 --- a/lib/rules/boolean-prop-naming.js +++ b/lib/rules/boolean-prop-naming.js @@ -247,7 +247,7 @@ module.exports = { } } - if ((list[component].invalidProps || []).length) { + if (list[component].invalidProps && list[component].invalidProps.length > 0) { reportInvalidNaming(list[component]); } }); diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 9b1b25cc81..c1e02be3cd 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -215,10 +215,7 @@ module.exports = { 'Program:exit': function() { const list = components.list(); // Report missing display name for all components - Object.keys(list).forEach(component => { - if (list[component].hasDisplayName) { - return; - } + Object.keys(list).filter(component => !list[component].hasDisplayName).forEach(component => { reportMissingDisplayName(list[component]); }); } diff --git a/lib/rules/no-multi-comp.js b/lib/rules/no-multi-comp.js index 3c0cefa3e4..9bb61899d1 100644 --- a/lib/rules/no-multi-comp.js +++ b/lib/rules/no-multi-comp.js @@ -58,16 +58,14 @@ module.exports = { } const list = components.list(); - let i = 0; - Object.keys(list).forEach(component => { - if (isIgnored(list[component]) || ++i === 1) { - return; + Object.keys(list).filter(component => !isIgnored(list[component])).forEach((component, i) => { + if (i >= 1) { + context.report({ + node: list[component].node, + message: MULTI_COMP_MESSAGE + }); } - context.report({ - node: list[component].node, - message: MULTI_COMP_MESSAGE - }); }); } }; diff --git a/lib/rules/no-set-state.js b/lib/rules/no-set-state.js index 80890cdc16..f5dc97f699 100644 --- a/lib/rules/no-set-state.js +++ b/lib/rules/no-set-state.js @@ -73,10 +73,7 @@ module.exports = { 'Program:exit': function() { const list = components.list(); - Object.keys(list).forEach(component => { - if (isValid(list[component])) { - return; - } + Object.keys(list).filter(component => !isValid(list[component])).forEach(component => { reportSetStateUsages(list[component]); }); } diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 2a218238d6..0e80cfd179 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -132,7 +132,7 @@ module.exports = { 'Program:exit': function() { const list = components.list(); // Report undeclared proptypes for all classes - Object.keys(list).forEach(component => { + Object.keys(list).filter(component => mustBeValidated(list[component])).forEach(component => { if (!mustBeValidated(list[component])) { return; } diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index b6e4a82c6a..16eaf6ee24 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -189,10 +189,7 @@ module.exports = { 'Program:exit': function() { const list = components.list(); // Report undeclared proptypes for all classes - Object.keys(list).forEach(component => { - if (!mustBeValidated(list[component])) { - return; - } + Object.keys(list).filter(component => mustBeValidated(list[component])).forEach(component => { reportUndeclaredPropTypes(list[component]); }); } diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 935564e632..ea8b4b153b 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -625,12 +625,7 @@ module.exports = { stack = null; const list = components.list(); - Object.keys(list).forEach(component => { - // If no propTypes could be found, we don't report anything. - if (!list[component].propTypes) { - return; - } - + Object.keys(list).filter(component => list[component].propTypes).forEach(component => { reportPropTypesWithoutDefault( list[component].propTypes, list[component].defaultProps || {} diff --git a/lib/rules/require-optimization.js b/lib/rules/require-optimization.js index b9b8623f1b..e447bdcc5a 100644 --- a/lib/rules/require-optimization.js +++ b/lib/rules/require-optimization.js @@ -220,10 +220,7 @@ module.exports = { const list = components.list(); // Report missing shouldComponentUpdate for all components - Object.keys(list).forEach(component => { - if (list[component].hasSCU) { - return; - } + Object.keys(list).filter(component => !list[component].hasSCU).forEach(component => { reportMissingOptimization(list[component]); }); } diff --git a/lib/util/Components.js b/lib/util/Components.js index 6c7b44304b..b7e2a99a63 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -124,10 +124,7 @@ class Components { const usedPropTypes = {}; // Find props used in components for which we are not confident - Object.keys(this._list).forEach(i => { - if (this._list[i].confidence >= 2) { - return; - } + Object.keys(this._list).filter(i => this._list[i].confidence < 2).forEach(i => { let component = null; let node = null; node = this._list[i].node; @@ -149,10 +146,7 @@ class Components { }); // Assign used props in not confident components to the parent component - Object.keys(this._list).forEach(j => { - if (this._list[j].confidence < 2) { - return; - } + Object.keys(this._list).filter(j => this._list[j].confidence >= 2).forEach(j => { const id = getId(this._list[j].node); list[j] = this._list[j]; if (usedPropTypes[id]) { @@ -716,8 +710,10 @@ function componentRule(rule, context) { const updatedRuleInstructions = util._extend({}, ruleInstructions); const propTypesInstructions = propTypes(context, components, utils); const usedPropTypesInstructions = usedPropTypesUtil(context, components, utils); - const allKeys = new Set(Object.keys(detectionInstructions).concat(Object.keys(propTypesInstructions)) - .concat(Object.keys(usedPropTypesInstructions))); + const allKeys = new Set(Object.keys(detectionInstructions).concat( + Object.keys(propTypesInstructions), + Object.keys(usedPropTypesInstructions) + )); allKeys.forEach(instruction => { updatedRuleInstructions[instruction] = function(node) { if (instruction in detectionInstructions) { diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js index 269266096b..dd3becf331 100644 --- a/lib/util/usedPropTypes.js +++ b/lib/util/usedPropTypes.js @@ -35,10 +35,7 @@ function isPropAttributeName (node) { * @returns {Boolean} True if the component must be validated, false if not. */ function mustBeValidated(component) { - return Boolean( - component && - !component.ignorePropsValidation - ); + return !!(component && !component.ignorePropsValidation); } module.exports = function usedPropTypesInstructions(context, components, utils) { @@ -53,8 +50,10 @@ module.exports = function usedPropTypesInstructions(context, components, utils) let scope = context.getScope(); while (scope) { if ( - scope.block && scope.block.parent && - scope.block.parent.key && scope.block.parent.key.name === 'componentWillReceiveProps' + scope.block + && scope.block.parent + && scope.block.parent.key + && scope.block.parent.key.name === 'componentWillReceiveProps' ) { return true; } @@ -75,7 +74,8 @@ module.exports = function usedPropTypesInstructions(context, components, utils) if (LIFE_CYCLE_METHODS.indexOf(name) >= 0) { return true; - } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(name) >= 0) { + } + if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(name) >= 0) { return true; } } @@ -94,9 +94,11 @@ module.exports = function usedPropTypesInstructions(context, components, utils) if (node.kind === 'constructor') { return true; - } else if (LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { + } + if (LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { return true; - } else if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { + } + if (checkAsyncSafeLifeCycles && ASYNC_SAFE_LIFE_CYCLE_METHODS.indexOf(nodeKeyName) >= 0) { return true; } @@ -371,7 +373,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils) switch (type) { case 'direct': // Ignore Object methods - if (Object.prototype[name]) { + if (name in Object.prototype) { break; } @@ -503,22 +505,15 @@ module.exports = function usedPropTypesInstructions(context, components, utils) 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)) { - node.properties.forEach((property, i) => { - if (i === 0) { - markPropTypesAsUsed(node.parent); - } - }); + if (isNodeALifeCycleMethod(node.parent.parent) && node.properties.length > 0) { + markPropTypesAsUsed(node.parent); } }, 'Program:exit': function() { const list = components.list(); - Object.keys(list).forEach(component => { - if (!mustBeValidated(list[component])) { - return; - } + Object.keys(list).filter(component => mustBeValidated(list[component])).forEach(component => { handleCustomValidators(list[component]); }); } From efb5bd76a149c7a3ab46a79e9f41f78e186dd152 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 14:57:15 -0700 Subject: [PATCH 36/49] Extract defaultProps detection code from default-props-match-prop-types --- lib/rules/default-props-match-prop-types.js | 127 +--------- lib/util/Components.js | 13 +- lib/util/defaultProps.js | 257 ++++++++++++++++++++ 3 files changed, 278 insertions(+), 119 deletions(-) create mode 100644 lib/util/defaultProps.js diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index 203bd2db2d..10472dfee7 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -200,39 +200,6 @@ module.exports = { }); } - /** - * Extracts a DefaultProp from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object|string} Object representation of a defaultProp, to be consumed by - * `addDefaultPropsToComponent`, or string "unresolved", if the defaultProps - * from this ObjectExpression can't be resolved. - */ - function getDefaultPropsFromObjectExpression(objectExpression) { - const hasSpread = objectExpression.properties.find(property => property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement'); - - if (hasSpread) { - return 'unresolved'; - } - - return objectExpression.properties.map(defaultProp => ({ - name: defaultProp.key.name, - node: defaultProp - })); - } - - /** - * Marks a component's DefaultProps declaration as "unresolved". A component's DefaultProps is - * marked as "unresolved" if we cannot safely infer the values of its defaultProps declarations - * without risking false negatives. - * @param {Object} component The component to mark. - * @returns {void} - */ - function markDefaultPropsAsUnresolved(component) { - components.set(component.node, { - defaultProps: 'unresolved' - }); - } - /** * Adds propTypes to the component passed in. * @param {ASTNode} component The component to add the propTypes to. @@ -247,31 +214,6 @@ module.exports = { }); } - /** - * Adds defaultProps to the component passed in. - * @param {ASTNode} component The component to add the defaultProps to. - * @param {String[]|String} defaultProps defaultProps to add to the component or the string "unresolved" - * if this component has defaultProps that can't be resolved. - * @returns {void} - */ - function addDefaultPropsToComponent(component, defaultProps) { - // Early return if this component's defaultProps is already marked as "unresolved". - if (component.defaultProps === 'unresolved') { - return; - } - - if (defaultProps === 'unresolved') { - markDefaultPropsAsUnresolved(component); - return; - } - - const defaults = component.defaultProps || []; - - components.set(component.node, { - defaultProps: defaults.concat(defaultProps) - }); - } - /** * Tries to find a props type annotation in a stateless component. * @param {ASTNode} node The AST node to look for a props type annotation. @@ -352,9 +294,8 @@ module.exports = { return { MemberExpression: function(node) { const isPropType = propsUtil.isPropTypesDeclaration(node); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -375,21 +316,8 @@ module.exports = { // MyComponent.propTypes = myPropTypes; if (node.parent.type === 'AssignmentExpression') { const expression = resolveNodeValue(node.parent.right); - if (!expression || expression.type !== 'ObjectExpression') { - // If a value can't be found, we mark the defaultProps declaration as "unresolved", because - // we should ignore this component and not report any errors for it, to avoid false-positives - // with e.g. external defaultProps declarations. - if (isDefaultProp) { - markDefaultPropsAsUnresolved(component); - } - - return; - } - - if (isPropType) { + if (expression && expression.type === 'ObjectExpression') { addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); } return; @@ -399,20 +327,11 @@ module.exports = { // MyComponent.propTypes.baz = React.PropTypes.string; if (node.parent.type === 'MemberExpression' && node.parent.parent && node.parent.parent.type === 'AssignmentExpression') { - if (isPropType) { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } else { - addDefaultPropsToComponent(component, [{ - name: node.parent.property.name, - node: node.parent.parent - }]); - } - - return; + addPropTypesToComponent(component, [{ + name: node.parent.property.name, + isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), + node: node.parent.parent + }]); } }, @@ -438,9 +357,8 @@ module.exports = { } const isPropType = propsUtil.isPropTypesDeclaration(node); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -460,11 +378,7 @@ module.exports = { return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); }, // e.g.: @@ -495,9 +409,8 @@ module.exports = { const propName = astUtil.getPropertyName(node); const isPropType = propName === 'propTypes'; - const isDefaultProp = propName === 'defaultProps' || propName === 'getDefaultProps'; - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -512,11 +425,7 @@ module.exports = { return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); }, // e.g.: @@ -547,25 +456,11 @@ module.exports = { } const isPropType = propsUtil.isPropTypesDeclaration(property); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(property); - - if (!isPropType && !isDefaultProp) { - return; - } if (isPropType && property.value.type === 'ObjectExpression') { addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); return; } - - if (isDefaultProp && property.value.type === 'FunctionExpression') { - const returnStatement = utils.findReturnStatement(property); - if (!returnStatement || returnStatement.argument.type !== 'ObjectExpression') { - return; - } - - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(returnStatement.argument)); - } }); }, diff --git a/lib/util/Components.js b/lib/util/Components.js index b7e2a99a63..1854c8e6a7 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -9,9 +9,10 @@ const doctrine = require('doctrine'); const variableUtil = require('./variable'); const pragmaUtil = require('./pragma'); const astUtil = require('./ast'); -const propTypes = require('./propTypes'); +const propTypesUtil = require('./propTypes'); const jsxUtil = require('./jsx'); const usedPropTypesUtil = require('./usedPropTypes'); +const defaultPropsUtil = require('./defaultProps'); function getId(node) { return node && node.range.join(':'); @@ -708,12 +709,15 @@ function componentRule(rule, context) { // Update the provided rule instructions to add the component detection const ruleInstructions = rule(context, components, utils); const updatedRuleInstructions = util._extend({}, ruleInstructions); - const propTypesInstructions = propTypes(context, components, utils); + const propTypesInstructions = propTypesUtil(context, components, utils); const usedPropTypesInstructions = usedPropTypesUtil(context, components, utils); + const defaultPropsInstructions = defaultPropsUtil(context, components, utils); const allKeys = new Set(Object.keys(detectionInstructions).concat( Object.keys(propTypesInstructions), - Object.keys(usedPropTypesInstructions) + Object.keys(usedPropTypesInstructions), + Object.keys(defaultPropsInstructions) )); + allKeys.forEach(instruction => { updatedRuleInstructions[instruction] = function(node) { if (instruction in detectionInstructions) { @@ -725,6 +729,9 @@ function componentRule(rule, context) { if (instruction in usedPropTypesInstructions) { usedPropTypesInstructions[instruction](node); } + if (instruction in defaultPropsInstructions) { + defaultPropsInstructions[instruction](node); + } return ruleInstructions[instruction] ? ruleInstructions[instruction](node) : void 0; }; }); diff --git a/lib/util/defaultProps.js b/lib/util/defaultProps.js new file mode 100644 index 0000000000..00bef47f4c --- /dev/null +++ b/lib/util/defaultProps.js @@ -0,0 +1,257 @@ +/** + * @fileoverview Common defaultProps detection functionality. + */ +'use strict'; + +const astUtil = require('./ast'); +const propsUtil = require('./props'); +const variableUtil = require('./variable'); + +module.exports = function defaultPropsInstructions(context, components, utils) { + const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); + + /** + * Try to resolve the node passed in to a variable in the current scope. If the node passed in is not + * an Identifier, then the node is simply returned. + * @param {ASTNode} node The node to resolve. + * @returns {ASTNode|null} Return null if the value could not be resolved, ASTNode otherwise. + */ + function resolveNodeValue(node) { + if (node.type === 'Identifier') { + return variableUtil.findVariableByName(context, node.name); + } + if ( + node.type === 'CallExpression' && + propWrapperFunctions.has(node.callee.name) && + node.arguments && node.arguments[0] + ) { + return resolveNodeValue(node.arguments[0]); + } + return node; + } + + /** + * Extracts a DefaultProp from an ObjectExpression node. + * @param {ASTNode} objectExpression ObjectExpression node. + * @returns {Object|string} Object representation of a defaultProp, to be consumed by + * `addDefaultPropsToComponent`, or string "unresolved", if the defaultProps + * from this ObjectExpression can't be resolved. + */ + function getDefaultPropsFromObjectExpression(objectExpression) { + const hasSpread = objectExpression.properties.find(property => property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement'); + + if (hasSpread) { + return 'unresolved'; + } + + return objectExpression.properties.map(defaultProp => ({ + name: defaultProp.key.name, + node: defaultProp + })); + } + + /** + * Marks a component's DefaultProps declaration as "unresolved". A component's DefaultProps is + * marked as "unresolved" if we cannot safely infer the values of its defaultProps declarations + * without risking false negatives. + * @param {Object} component The component to mark. + * @returns {void} + */ + function markDefaultPropsAsUnresolved(component) { + components.set(component.node, { + defaultProps: 'unresolved' + }); + } + + /** + * Adds defaultProps to the component passed in. + * @param {ASTNode} component The component to add the defaultProps to. + * @param {String[]|String} defaultProps defaultProps to add to the component or the string "unresolved" + * if this component has defaultProps that can't be resolved. + * @returns {void} + */ + function addDefaultPropsToComponent(component, defaultProps) { + // Early return if this component's defaultProps is already marked as "unresolved". + if (component.defaultProps === 'unresolved') { + return; + } + + if (defaultProps === 'unresolved') { + markDefaultPropsAsUnresolved(component); + return; + } + + const defaults = component.defaultProps || []; + + components.set(component.node, { + defaultProps: defaults.concat(defaultProps) + }); + } + + return { + MemberExpression: function(node) { + const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); + + if (!isDefaultProp) { + return; + } + + // find component this defaultProps belongs to + const component = utils.getRelatedComponent(node); + if (!component) { + return; + } + + // e.g.: + // MyComponent.propTypes = { + // foo: React.PropTypes.string.isRequired, + // bar: React.PropTypes.string + // }; + // + // or: + // + // MyComponent.propTypes = myPropTypes; + if (node.parent.type === 'AssignmentExpression') { + const expression = resolveNodeValue(node.parent.right); + if (!expression || expression.type !== 'ObjectExpression') { + // If a value can't be found, we mark the defaultProps declaration as "unresolved", because + // we should ignore this component and not report any errors for it, to avoid false-positives + // with e.g. external defaultProps declarations. + if (isDefaultProp) { + markDefaultPropsAsUnresolved(component); + } + + return; + } + + addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); + + return; + } + + // e.g.: + // MyComponent.propTypes.baz = React.PropTypes.string; + if (node.parent.type === 'MemberExpression' && node.parent.parent && + node.parent.parent.type === 'AssignmentExpression') { + addDefaultPropsToComponent(component, [{ + name: node.parent.property.name, + node: node.parent.parent + }]); + } + }, + + // e.g.: + // class Hello extends React.Component { + // static get defaultProps() { + // return { + // name: 'Dean' + // }; + // } + // render() { + // return
Hello {this.props.name}
; + // } + // } + MethodDefinition: function(node) { + if (!node.static || node.kind !== 'get') { + return; + } + + if (!propsUtil.isDefaultPropsDeclaration(node)) { + return; + } + + // find component this propTypes/defaultProps belongs to + const component = components.get(utils.getParentES6Component()); + if (!component) { + return; + } + + const returnStatement = utils.findReturnStatement(node); + if (!returnStatement) { + return; + } + + const expression = resolveNodeValue(returnStatement.argument); + if (!expression || expression.type !== 'ObjectExpression') { + return; + } + + addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); + }, + + // e.g.: + // class Greeting extends React.Component { + // render() { + // return ( + //

Hello, {this.props.foo} {this.props.bar}

+ // ); + // } + // static defaultProps = { + // foo: 'bar', + // bar: 'baz' + // }; + // } + ClassProperty: function(node) { + if (!(node.static && node.value)) { + return; + } + + const propName = astUtil.getPropertyName(node); + const isDefaultProp = propName === 'defaultProps' || propName === 'getDefaultProps'; + + if (!isDefaultProp) { + return; + } + + // find component this propTypes/defaultProps belongs to + const component = components.get(utils.getParentES6Component()); + if (!component) { + return; + } + + const expression = resolveNodeValue(node.value); + if (!expression || expression.type !== 'ObjectExpression') { + return; + } + + addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); + }, + + // e.g.: + // React.createClass({ + // render: function() { + // return
{this.props.foo}
; + // }, + // getDefaultProps: function() { + // return { + // foo: 'default' + // }; + // } + // }); + ObjectExpression: function(node) { + // find component this propTypes/defaultProps belongs to + const component = utils.isES5Component(node) && components.get(node); + if (!component) { + return; + } + + // Search for the proptypes declaration + node.properties.forEach(property => { + if (property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement') { + return; + } + + const isDefaultProp = propsUtil.isDefaultPropsDeclaration(property); + + if (isDefaultProp && property.value.type === 'FunctionExpression') { + const returnStatement = utils.findReturnStatement(property); + if (!returnStatement || returnStatement.argument.type !== 'ObjectExpression') { + return; + } + + addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(returnStatement.argument)); + } + }); + } + }; +}; From ce9ba3420625bd93ae2dfe88d3c9732ce63b7dc0 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 15:16:28 -0700 Subject: [PATCH 37/49] Make require-default-props use defaultProps detection --- lib/rules/default-props-match-prop-types.js | 9 +- lib/rules/require-default-props.js | 124 ++------------------ lib/util/defaultProps.js | 13 +- 3 files changed, 26 insertions(+), 120 deletions(-) diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index 10472dfee7..fb9c082a26 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -264,8 +264,9 @@ module.exports = { return; } - defaultProps.forEach(defaultProp => { - const prop = propFromName(propTypes, defaultProp.name); + Object.keys(defaultProps).forEach(defaultPropName => { + const defaultProp = defaultProps[defaultPropName]; + const prop = propFromName(propTypes, defaultPropName); if (prop && (allowRequiredDefaults || !prop.isRequired)) { return; @@ -275,13 +276,13 @@ module.exports = { context.report( defaultProp.node, 'defaultProp "{{name}}" defined for isRequired propType.', - {name: defaultProp.name} + {name: defaultPropName} ); } else { context.report( defaultProp.node, 'defaultProp "{{name}}" has no corresponding propTypes declaration.', - {name: defaultProp.name} + {name: defaultPropName} ); } }); diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index ea8b4b153b..6b418cf86a 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -175,36 +175,6 @@ module.exports = { }); } - /** - * Extracts a DefaultProp from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object|string} Object representation of a defaultProp, to be consumed by - * `addDefaultPropsToComponent`, or string "unresolved", if the defaultProps - * from this ObjectExpression can't be resolved. - */ - function getDefaultPropsFromObjectExpression(objectExpression) { - const hasSpread = objectExpression.properties.find(property => property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement'); - - if (hasSpread) { - return 'unresolved'; - } - - return objectExpression.properties.map(property => sourceCode.getText(property.key).replace(QUOTES_REGEX, '')); - } - - /** - * Marks a component's DefaultProps declaration as "unresolved". A component's DefaultProps is - * marked as "unresolved" if we cannot safely infer the values of its defaultProps declarations - * without risking false negatives. - * @param {Object} component The component to mark. - * @returns {void} - */ - function markDefaultPropsAsUnresolved(component) { - components.set(component.node, { - defaultProps: 'unresolved' - }); - } - /** * Adds propTypes to the component passed in. * @param {ASTNode} component The component to add the propTypes to. @@ -219,35 +189,6 @@ module.exports = { }); } - /** - * Adds defaultProps to the component passed in. - * @param {ASTNode} component The component to add the defaultProps to. - * @param {String[]|String} defaultProps defaultProps to add to the component or the string "unresolved" - * if this component has defaultProps that can't be resolved. - * @returns {void} - */ - function addDefaultPropsToComponent(component, defaultProps) { - // Early return if this component's defaultProps is already marked as "unresolved". - if (component.defaultProps === 'unresolved') { - return; - } - - if (defaultProps === 'unresolved') { - markDefaultPropsAsUnresolved(component); - return; - } - - const defaults = component.defaultProps || {}; - - defaultProps.forEach(defaultProp => { - defaults[defaultProp] = true; - }); - - components.set(component.node, { - defaultProps: defaults - }); - } - /** * Tries to find a props type annotation in a stateless component. * @param {ASTNode} node The AST node to look for a props type annotation. @@ -369,9 +310,8 @@ module.exports = { return { MemberExpression: function(node) { const isPropType = propsUtil.isPropTypesDeclaration(node); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -393,39 +333,21 @@ module.exports = { if (node.parent.type === 'AssignmentExpression') { const expression = resolveNodeValue(node.parent.right); if (!expression || expression.type !== 'ObjectExpression') { - // If a value can't be found, we mark the defaultProps declaration as "unresolved", because - // we should ignore this component and not report any errors for it, to avoid false-positives - // with e.g. external defaultProps declarations. - if (isDefaultProp) { - markDefaultPropsAsUnresolved(component); - } - return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } - + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); return; } // e.g.: // MyComponent.propTypes.baz = PropTypes.string; if (node.parent.type === 'MemberExpression' && node.parent.parent.type === 'AssignmentExpression') { - if (isPropType) { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } else { - addDefaultPropsToComponent(component, [node.parent.property.name]); - } - - return; + addPropTypesToComponent(component, [{ + name: node.parent.property.name, + isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), + node: node.parent.parent + }]); } }, @@ -451,9 +373,8 @@ module.exports = { } const isPropType = propsUtil.isPropTypesDeclaration(node); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -473,11 +394,7 @@ module.exports = { return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); }, // e.g.: @@ -507,9 +424,8 @@ module.exports = { } const isPropType = astUtil.getPropertyName(node) === 'propTypes'; - const isDefaultProp = astUtil.getPropertyName(node) === 'defaultProps' || astUtil.getPropertyName(node) === 'getDefaultProps'; - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -524,11 +440,7 @@ module.exports = { return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); }, // e.g.: @@ -559,25 +471,11 @@ module.exports = { } const isPropType = propsUtil.isPropTypesDeclaration(property); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(property); - - if (!isPropType && !isDefaultProp) { - return; - } if (isPropType && property.value.type === 'ObjectExpression') { addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); return; } - - if (isDefaultProp && property.value.type === 'FunctionExpression') { - const returnStatement = utils.findReturnStatement(property); - if (!returnStatement || returnStatement.argument.type !== 'ObjectExpression') { - return; - } - - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(returnStatement.argument)); - } }); }, diff --git a/lib/util/defaultProps.js b/lib/util/defaultProps.js index 00bef47f4c..8546c5e654 100644 --- a/lib/util/defaultProps.js +++ b/lib/util/defaultProps.js @@ -7,7 +7,10 @@ const astUtil = require('./ast'); const propsUtil = require('./props'); const variableUtil = require('./variable'); +const QUOTES_REGEX = /^["']|["']$/g; + module.exports = function defaultPropsInstructions(context, components, utils) { + const sourceCode = context.getSourceCode(); const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); /** @@ -45,7 +48,7 @@ module.exports = function defaultPropsInstructions(context, components, utils) { } return objectExpression.properties.map(defaultProp => ({ - name: defaultProp.key.name, + name: sourceCode.getText(defaultProp.key).replace(QUOTES_REGEX, ''), node: defaultProp })); } @@ -81,10 +84,14 @@ module.exports = function defaultPropsInstructions(context, components, utils) { return; } - const defaults = component.defaultProps || []; + const defaults = component.defaultProps || {}; + const newDefaultProps = defaultProps.reduce((acc, prop) => { + acc[prop.name] = prop; + return acc; + }, Object.assign({}, defaults)); components.set(component.node, { - defaultProps: defaults.concat(defaultProps) + defaultProps: newDefaultProps }); } From b81e19a10ff8b076b8e13b4bd8217fd017e57ea8 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Mon, 20 Aug 2018 14:12:30 -0700 Subject: [PATCH 38/49] Use object.fromentries --- lib/util/defaultProps.js | 10 ++++++---- package.json | 1 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/util/defaultProps.js b/lib/util/defaultProps.js index 8546c5e654..32b0aec051 100644 --- a/lib/util/defaultProps.js +++ b/lib/util/defaultProps.js @@ -3,6 +3,7 @@ */ 'use strict'; +const fromEntries = require('object.fromentries'); const astUtil = require('./ast'); const propsUtil = require('./props'); const variableUtil = require('./variable'); @@ -85,10 +86,11 @@ module.exports = function defaultPropsInstructions(context, components, utils) { } const defaults = component.defaultProps || {}; - const newDefaultProps = defaultProps.reduce((acc, prop) => { - acc[prop.name] = prop; - return acc; - }, Object.assign({}, defaults)); + const newDefaultProps = Object.assign( + {}, + defaults, + fromEntries(defaultProps.map(prop => [prop.name, prop])) + ); components.set(component.node, { defaultProps: newDefaultProps diff --git a/package.json b/package.json index 8fb035558b..0337c577e4 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "doctrine": "^2.1.0", "has": "^1.0.3", "jsx-ast-utils": "^2.0.1", + "object.fromentries": "^2.0.0", "prop-types": "^15.6.2" }, "devDependencies": { From 68d63845635da3fedd6205b068cb33ff3d848c76 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Tue, 25 Sep 2018 02:00:40 +0300 Subject: [PATCH 39/49] Fix `no-this-in-sfc` for class properties Fixed #1960 --- lib/util/Components.js | 22 ++++++++++++---------- tests/lib/rules/no-this-in-sfc.js | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index b7e2a99a63..2ea7cc896f 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -456,15 +456,16 @@ function componentRule(rule, context) { let scope = context.getScope(); while (scope) { const node = scope.block; - const isClass = node.type === 'ClassExpression'; const isFunction = /Function/.test(node.type); // Functions const isArrowFunction = node.type === 'ArrowFunctionExpression'; - let functionScope = scope; + let enclosingScope = scope; if (isArrowFunction) { - functionScope = utils.getParentFunctionScope(scope); + enclosingScope = utils.getArrowFunctionScope(scope); } - const methodNode = functionScope && functionScope.block.parent; - const isMethod = methodNode && methodNode.type === 'MethodDefinition'; // Classes methods + const enclosingScopeType = enclosingScope && enclosingScope.block.type; + const enclosingScopeParent = enclosingScope && enclosingScope.block.parent; + const isClass = enclosingScopeType === 'ClassDeclaration' || enclosingScopeType === 'ClassExpression'; + const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.) // Attribute Expressions inside JSX Elements () const isJSXExpressionContainer = node.parent && node.parent.type === 'JSXExpressionContainer'; @@ -482,15 +483,16 @@ function componentRule(rule, context) { }, /** - * Get a parent scope created by a FunctionExpression or FunctionDeclaration - * @param {Scope} scope The child scope - * @returns {Scope} A parent function scope + * Get an enclosing scope used to find `this` value by an arrow function + * @param {Scope} scope Current scope + * @returns {Scope} An enclosing scope used by an arrow function */ - getParentFunctionScope(scope) { + getArrowFunctionScope(scope) { scope = scope.upper; while (scope) { const type = scope.block.type; - if (type === 'FunctionExpression' || type === 'FunctionDeclaration') { + if (type === 'FunctionExpression' || type === 'FunctionDeclaration' + || type === 'ClassDeclaration' || type === 'ClassExpression') { return scope; } scope = scope.upper; diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index 63a849f840..9c241cb8ed 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -16,6 +16,8 @@ const ERROR_MESSAGE = 'Stateless functional components should not use this'; const rule = require('../../../lib/rules/no-this-in-sfc'); const RuleTester = require('eslint').RuleTester; +require('babel-eslint'); + const parserOptions = { ecmaVersion: 2018, sourceType: 'module', @@ -119,6 +121,24 @@ ruleTester.run('no-this-in-sfc', rule, { }; } }` + }, { + code: ` + class Foo { + bar = () => { + this.something(); + return null; + }; + }`, + parser: 'babel-eslint' + }, { + code: ` + class Foo { + bar = () => () => { + this.something(); + return null; + }; + }`, + parser: 'babel-eslint' }], invalid: [{ code: ` From 70155b4f85c85165e157e5141053906c084f1398 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 16:31:27 -0700 Subject: [PATCH 40/49] Extract isRequired detection from default-props-match-prop-types --- lib/rules/default-props-match-prop-types.js | 416 +------------------- lib/util/propTypes.js | 23 +- 2 files changed, 24 insertions(+), 415 deletions(-) diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index fb9c082a26..6270d2042d 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -6,10 +6,6 @@ 'use strict'; const Components = require('../util/Components'); -const variableUtil = require('../util/variable'); -const annotations = require('../util/annotations'); -const astUtil = require('../util/ast'); -const propsUtil = require('../util/props'); const docsUrl = require('../util/docsUrl'); // ------------------------------------------------------------------------------ @@ -36,219 +32,9 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { + create: Components.detect((context, components) => { const configuration = context.options[0] || {}; const allowRequiredDefaults = configuration.allowRequiredDefaults || false; - const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); - // Used to track the type annotations in scope. - // Necessary because babel's scopes do not track type annotations. - let stack = null; - - /** - * Try to resolve the node passed in to a variable in the current scope. If the node passed in is not - * an Identifier, then the node is simply returned. - * @param {ASTNode} node The node to resolve. - * @returns {ASTNode|null} Return null if the value could not be resolved, ASTNode otherwise. - */ - function resolveNodeValue(node) { - if (node.type === 'Identifier') { - return variableUtil.findVariableByName(context, node.name); - } - if ( - node.type === 'CallExpression' && - propWrapperFunctions.has(node.callee.name) && - node.arguments && node.arguments[0] - ) { - return resolveNodeValue(node.arguments[0]); - } - return node; - } - - /** - * Helper for accessing the current scope in the stack. - * @param {string} key The name of the identifier to access. If omitted, returns the full scope. - * @param {ASTNode} value If provided sets the new value for the identifier. - * @returns {Object|ASTNode} Either the whole scope or the ASTNode associated with the given identifier. - */ - function typeScope(key, value) { - if (arguments.length === 0) { - return stack[stack.length - 1]; - } else if (arguments.length === 1) { - return stack[stack.length - 1][key]; - } - stack[stack.length - 1][key] = value; - return value; - } - - /** - * Tries to find the definition of a GenericTypeAnnotation in the current scope. - * @param {ASTNode} node The node GenericTypeAnnotation node to resolve. - * @return {ASTNode|null} Return null if definition cannot be found, ASTNode otherwise. - */ - function resolveGenericTypeAnnotation(node) { - if (node.type !== 'GenericTypeAnnotation' || node.id.type !== 'Identifier') { - return null; - } - - return variableUtil.findVariableByName(context, node.id.name) || typeScope(node.id.name); - } - - function resolveUnionTypeAnnotation(node) { - // Go through all the union and resolve any generic types. - return node.types.map(annotation => { - if (annotation.type === 'GenericTypeAnnotation') { - return resolveGenericTypeAnnotation(annotation); - } - - return annotation; - }); - } - - /** - * Extracts a PropType from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromObjectExpression(objectExpression) { - const props = objectExpression.properties.filter(property => property.type !== 'ExperimentalSpreadProperty' && property.type !== 'SpreadElement'); - - return props.map(property => ({ - name: property.key.name, - isRequired: propsUtil.isRequiredPropType(property.value), - node: property - })); - } - - /** - * Handles Props defined in IntersectionTypeAnnotation nodes - * e.g. type Props = PropsA & PropsB - * @param {ASTNode} intersectionTypeAnnotation ObjectExpression node. - * @returns {Object[]} - */ - function getPropertiesFromIntersectionTypeAnnotationNode(annotation) { - return annotation.types.reduce((properties, type) => { - annotation = resolveGenericTypeAnnotation(type); - - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - if (!annotation || !annotation.properties) { - return properties; - } - - return properties.concat(annotation.properties); - }, []); - } - - /** - * Extracts a PropType from a TypeAnnotation node. - * @param {ASTNode} node TypeAnnotation node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromTypeAnnotation(node) { - let properties = []; - - switch (node.typeAnnotation.type) { - case 'GenericTypeAnnotation': - let annotation = resolveGenericTypeAnnotation(node.typeAnnotation); - - if (annotation && annotation.type === 'IntersectionTypeAnnotation') { - properties = getPropertiesFromIntersectionTypeAnnotationNode(annotation); - } else { - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - properties = annotation ? (annotation.properties || []) : []; - } - - break; - - case 'UnionTypeAnnotation': - const union = resolveUnionTypeAnnotation(node.typeAnnotation); - properties = union.reduce((acc, curr) => { - if (!curr) { - return acc; - } - - return acc.concat(curr.properties); - }, []); - break; - - case 'ObjectTypeAnnotation': - properties = node.typeAnnotation.properties; - break; - - default: - properties = []; - break; - } - - const props = properties.filter(property => property.type === 'ObjectTypeProperty'); - - return props.map(property => { - // the `key` property is not present in ObjectTypeProperty nodes, so we need to get the key name manually. - const tokens = context.getFirstTokens(property, 1); - const name = tokens[0].value; - - return { - name: name, - isRequired: !property.optional, - node: property - }; - }); - } - - /** - * Adds propTypes to the component passed in. - * @param {ASTNode} component The component to add the propTypes to. - * @param {Object[]} propTypes propTypes to add to the component. - * @returns {void} - */ - function addPropTypesToComponent(component, propTypes) { - const props = component.propTypes || []; - - components.set(component.node, { - propTypes: props.concat(propTypes) - }); - } - - /** - * Tries to find a props type annotation in a stateless component. - * @param {ASTNode} node The AST node to look for a props type annotation. - * @return {void} - */ - function handleStatelessComponent(node) { - if (!node.params || !node.params.length || !annotations.isAnnotatedFunctionPropsDeclaration(node, context)) { - return; - } - - // find component this props annotation belongs to - const component = components.get(utils.getParentStatelessComponent()); - if (!component) { - return; - } - - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.params[0].typeAnnotation, context)); - } - - function handlePropTypeAnnotationClassProperty(node) { - // find component this props annotation belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.typeAnnotation, context)); - } - - function isPropTypeAnnotation(node) { - return (astUtil.getPropertyName(node) === 'props' && !!node.typeAnnotation); - } - - function propFromName(propTypes, name) { - return propTypes.find(prop => prop.name === name); - } /** * Reports all defaultProps passed in that don't have an appropriate propTypes counterpart. @@ -260,13 +46,13 @@ module.exports = { // If this defaultProps is "unresolved" or the propTypes is undefined, then we should ignore // this component and not report any errors for it, to avoid false-positives with e.g. // external defaultProps/propTypes declarations or spread operators. - if (defaultProps === 'unresolved' || !propTypes) { + if (defaultProps === 'unresolved' || !propTypes || Object.keys(propTypes).length === 0) { return; } Object.keys(defaultProps).forEach(defaultPropName => { const defaultProp = defaultProps[defaultPropName]; - const prop = propFromName(propTypes, defaultPropName); + const prop = propTypes[defaultPropName]; if (prop && (allowRequiredDefaults || !prop.isRequired)) { return; @@ -293,201 +79,7 @@ module.exports = { // -------------------------------------------------------------------------- return { - MemberExpression: function(node) { - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = utils.getRelatedComponent(node); - if (!component) { - return; - } - - // e.g.: - // MyComponent.propTypes = { - // foo: React.PropTypes.string.isRequired, - // bar: React.PropTypes.string - // }; - // - // or: - // - // MyComponent.propTypes = myPropTypes; - if (node.parent.type === 'AssignmentExpression') { - const expression = resolveNodeValue(node.parent.right); - if (expression && expression.type === 'ObjectExpression') { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } - - return; - } - - // e.g.: - // MyComponent.propTypes.baz = React.PropTypes.string; - if (node.parent.type === 'MemberExpression' && node.parent.parent && - node.parent.parent.type === 'AssignmentExpression') { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } - }, - - // e.g.: - // class Hello extends React.Component { - // static get propTypes() { - // return { - // name: React.PropTypes.string - // }; - // } - // static get defaultProps() { - // return { - // name: 'Dean' - // }; - // } - // render() { - // return
Hello {this.props.name}
; - // } - // } - MethodDefinition: function(node) { - if (!node.static || node.kind !== 'get') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const returnStatement = utils.findReturnStatement(node); - if (!returnStatement) { - return; - } - - const expression = resolveNodeValue(returnStatement.argument); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // class Greeting extends React.Component { - // render() { - // return ( - //

Hello, {this.props.foo} {this.props.bar}

- // ); - // } - // static propTypes = { - // foo: React.PropTypes.string, - // bar: React.PropTypes.string.isRequired - // }; - // } - ClassProperty: function(node) { - if (isPropTypeAnnotation(node)) { - handlePropTypeAnnotationClassProperty(node); - return; - } - - if (!node.static) { - return; - } - - if (!node.value) { - return; - } - - const propName = astUtil.getPropertyName(node); - const isPropType = propName === 'propTypes'; - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const expression = resolveNodeValue(node.value); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // React.createClass({ - // render: function() { - // return
{this.props.foo}
; - // }, - // propTypes: { - // foo: React.PropTypes.string.isRequired, - // }, - // getDefaultProps: function() { - // return { - // foo: 'default' - // }; - // } - // }); - ObjectExpression: function(node) { - // find component this propTypes/defaultProps belongs to - const component = utils.isES5Component(node) && components.get(node); - if (!component) { - return; - } - - // Search for the proptypes declaration - node.properties.forEach(property => { - if (property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(property); - - if (isPropType && property.value.type === 'ObjectExpression') { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); - return; - } - }); - }, - - TypeAlias: function(node) { - typeScope(node.id.name, node.right); - }, - - Program: function() { - stack = [{}]; - }, - - BlockStatement: function () { - stack.push(Object.create(typeScope())); - }, - - 'BlockStatement:exit': function () { - stack.pop(); - }, - - // Check for type annotations in stateless components - FunctionDeclaration: handleStatelessComponent, - ArrowFunctionExpression: handleStatelessComponent, - FunctionExpression: handleStatelessComponent, - 'Program:exit': function() { - stack = null; const list = components.list(); Object.keys(list).forEach(component => { @@ -497,7 +89,7 @@ module.exports = { } reportInvalidDefaultProps( - list[component].propTypes, + list[component].declaredPropTypes, list[component].defaultProps || {} ); }); diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index f0ca66ee8e..68c2641891 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -61,7 +61,7 @@ function iterateProperties(context, properties, fn) { const key = getKeyValue(context, node); const value = node.value; - fn(key, value); + fn(key, value, node); } } } @@ -139,6 +139,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.fullName = fullName; types.name = childKey; types.node = childValue; + types.isRequired = !childValue.optional; shapeTypeDefinition.children[childKey] = types; } }); @@ -209,6 +210,7 @@ module.exports = function propTypesInstructions(context, components, utils) { if (annotation.type === 'GenericTypeAnnotation' && getInTypeScope(annotation.id.name)) { return getInTypeScope(annotation.id.name); } + return annotation; } @@ -248,7 +250,7 @@ module.exports = function propTypesInstructions(context, components, utils) { function declarePropTypesForObjectTypeAnnotation(propTypes, declaredPropTypes) { let ignorePropsValidation = false; - iterateProperties(context, propTypes.properties, (key, value) => { + iterateProperties(context, propTypes.properties, (key, value, propNode) => { if (!value) { ignorePropsValidation = true; return; @@ -258,6 +260,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.fullName = key; types.name = key; types.node = value; + types.isRequired = !propNode.optional; declaredPropTypes[key] = types; }); @@ -444,6 +447,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.fullName = key; types.name = key; types.node = value; + types.isRequired = propsUtil.isRequiredPropType(value); declaredPropTypes[key] = types; }); break; @@ -482,6 +486,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.name = propTypes.property.name; types.fullName = [parentProp, propTypes.property.name].join('.'); types.node = propTypes.property; + types.isRequired = propsUtil.isRequiredPropType(propTypes.parent.right); curDeclaredPropTypes[propTypes.property.name] = types; } else { let isUsedInPropTypes = false; @@ -546,7 +551,19 @@ module.exports = function propTypesInstructions(context, components, utils) { if (!node.params || !node.params.length || !annotations.isAnnotatedFunctionPropsDeclaration(node, context)) { return; } - markPropTypesAsDeclared(node, resolveTypeAnnotation(node.params[0])); + + const param = node.params[0]; + if (param.typeAnnotation && param.typeAnnotation.typeAnnotation && param.typeAnnotation.typeAnnotation.type === 'UnionTypeAnnotation') { + param.typeAnnotation.typeAnnotation.types.forEach(annotation => { + if (annotation.type === 'GenericTypeAnnotation') { + markPropTypesAsDeclared(node, resolveTypeAnnotation(annotation)); + } else { + markPropTypesAsDeclared(node, annotation); + } + }); + } else { + markPropTypesAsDeclared(node, resolveTypeAnnotation(param)); + } } /** From 2e4f91afdc8a089936a2ff49e6b4c83cccb3432a Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 17:22:59 -0700 Subject: [PATCH 41/49] Make require-default-props use isRequired detection --- lib/rules/no-unused-prop-types.js | 2 +- lib/rules/require-default-props.js | 459 +---------------------------- lib/util/propTypes.js | 28 +- lib/util/usedPropTypes.js | 4 +- 4 files changed, 27 insertions(+), 466 deletions(-) diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 0e80cfd179..8aaf9f9d2f 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -103,7 +103,7 @@ module.exports = { if (prop.node && !isPropUsed(component, prop)) { context.report( - prop.node, + prop.node.value || prop.node, UNUSED_MESSAGE, { name: prop.fullName } diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 6b418cf86a..4daf0ba3ad 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -5,13 +5,8 @@ 'use strict'; const Components = require('../util/Components'); -const variableUtil = require('../util/variable'); -const annotations = require('../util/annotations'); -const astUtil = require('../util/ast'); -const propsUtil = require('../util/props'); const docsUrl = require('../util/docsUrl'); -const QUOTES_REGEX = /^["']|["']$/g; // ------------------------------------------------------------------------------ // Rule Definition @@ -36,191 +31,10 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { - const sourceCode = context.getSourceCode(); - const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); + create: Components.detect((context, components) => { const configuration = context.options[0] || {}; const forbidDefaultForRequired = configuration.forbidDefaultForRequired || false; - // Used to track the type annotations in scope. - // Necessary because babel's scopes do not track type annotations. - let stack = null; - /** - * Try to resolve the node passed in to a variable in the current scope. If the node passed in is not - * an Identifier, then the node is simply returned. - * @param {ASTNode} node The node to resolve. - * @returns {ASTNode|null} Return null if the value could not be resolved, ASTNode otherwise. - */ - function resolveNodeValue(node) { - if (node.type === 'Identifier') { - return variableUtil.findVariableByName(context, node.name); - } - if ( - node.type === 'CallExpression' && - propWrapperFunctions.has(node.callee.name) && - node.arguments && node.arguments[0] - ) { - return resolveNodeValue(node.arguments[0]); - } - - return node; - } - - /** - * Helper for accessing the current scope in the stack. - * @param {string} key The name of the identifier to access. If omitted, returns the full scope. - * @param {ASTNode} value If provided sets the new value for the identifier. - * @returns {Object|ASTNode} Either the whole scope or the ASTNode associated with the given identifier. - */ - function typeScope(key, value) { - if (arguments.length === 0) { - return stack[stack.length - 1]; - } else if (arguments.length === 1) { - return stack[stack.length - 1][key]; - } - stack[stack.length - 1][key] = value; - return value; - } - - /** - * Tries to find the definition of a GenericTypeAnnotation in the current scope. - * @param {ASTNode} node The node GenericTypeAnnotation node to resolve. - * @return {ASTNode|null} Return null if definition cannot be found, ASTNode otherwise. - */ - function resolveGenericTypeAnnotation(node) { - if (node.type !== 'GenericTypeAnnotation' || node.id.type !== 'Identifier') { - return null; - } - - return variableUtil.findVariableByName(context, node.id.name) || typeScope(node.id.name); - } - - function resolveUnionTypeAnnotation(node) { - // Go through all the union and resolve any generic types. - return node.types.map(annotation => { - if (annotation.type === 'GenericTypeAnnotation') { - return resolveGenericTypeAnnotation(annotation); - } - - return annotation; - }); - } - - /** - * Extracts a PropType from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromObjectExpression(objectExpression) { - const props = objectExpression.properties.filter(property => property.type !== 'ExperimentalSpreadProperty' && property.type !== 'SpreadElement'); - - return props.map(property => ({ - name: sourceCode.getText(property.key).replace(QUOTES_REGEX, ''), - isRequired: propsUtil.isRequiredPropType(property.value), - node: property - })); - } - - /** - * Extracts a PropType from a TypeAnnotation node. - * @param {ASTNode} node TypeAnnotation node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromTypeAnnotation(node) { - let properties; - - switch (node.typeAnnotation.type) { - case 'GenericTypeAnnotation': - let annotation = resolveGenericTypeAnnotation(node.typeAnnotation); - - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - properties = annotation ? (annotation.properties || []) : []; - break; - - case 'UnionTypeAnnotation': - const union = resolveUnionTypeAnnotation(node.typeAnnotation); - properties = union.reduce((acc, curr) => { - if (!curr) { - return acc; - } - - return acc.concat(curr.properties); - }, []); - break; - - case 'ObjectTypeAnnotation': - properties = node.typeAnnotation.properties; - break; - - default: - properties = []; - break; - } - - const props = properties.filter(property => property.type === 'ObjectTypeProperty'); - - return props.map(property => { - // the `key` property is not present in ObjectTypeProperty nodes, so we need to get the key name manually. - const tokens = context.getFirstTokens(property, 1); - const name = tokens[0].value; - - return { - name: name, - isRequired: !property.optional, - node: property - }; - }); - } - - /** - * Adds propTypes to the component passed in. - * @param {ASTNode} component The component to add the propTypes to. - * @param {Object[]} propTypes propTypes to add to the component. - * @returns {void} - */ - function addPropTypesToComponent(component, propTypes) { - const props = component.propTypes || []; - - components.set(component.node, { - propTypes: props.concat(propTypes) - }); - } - - /** - * Tries to find a props type annotation in a stateless component. - * @param {ASTNode} node The AST node to look for a props type annotation. - * @return {void} - */ - function handleStatelessComponent(node) { - if (!node.params || !node.params.length || !annotations.isAnnotatedFunctionPropsDeclaration(node, context)) { - return; - } - - // find component this props annotation belongs to - const component = components.get(utils.getParentStatelessComponent()); - if (!component) { - return; - } - - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.params[0].typeAnnotation, context)); - } - - function handlePropTypeAnnotationClassProperty(node) { - // find component this props annotation belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.typeAnnotation, context)); - } - - function isPropTypeAnnotation(node) { - return (astUtil.getPropertyName(node) === 'props' && !!node.typeAnnotation); - } /** * Reports all propTypes passed in that don't have a defaultProp counterpart. @@ -235,297 +49,42 @@ module.exports = { return; } - propTypes.forEach(prop => { + Object.keys(propTypes).forEach(propName => { + const prop = propTypes[propName]; if (prop.isRequired) { - if (forbidDefaultForRequired && defaultProps[prop.name]) { + if (forbidDefaultForRequired && defaultProps[propName]) { context.report( prop.node, 'propType "{{name}}" is required and should not have a defaultProp declaration.', - {name: prop.name} + {name: propName} ); } return; } - if (defaultProps[prop.name]) { + if (defaultProps[propName]) { return; } context.report( prop.node, 'propType "{{name}}" is not required, but has no corresponding defaultProp declaration.', - {name: prop.name} + {name: propName} ); }); } - /** - * Extracts a PropType from a TypeAnnotation contained in generic node. - * @param {ASTNode} node TypeAnnotation node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromGeneric(node) { - let annotation = resolveGenericTypeAnnotation(node); - - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - const properties = annotation ? (annotation.properties || []) : []; - - const props = properties.filter(property => property.type === 'ObjectTypeProperty'); - - return props.map(property => { - // the `key` property is not present in ObjectTypeProperty nodes, so we need to get the key name manually. - const tokens = context.getFirstTokens(property, 1); - const name = tokens[0].value; - - return { - name: name, - isRequired: !property.optional, - node: property - }; - }); - } - - function hasPropTypesAsGeneric(node) { - return node && node.parent && node.parent.type === 'ClassDeclaration'; - } - - function handlePropTypesAsGeneric(node) { - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - if (node.params[0]) { - addPropTypesToComponent(component, getPropTypesFromGeneric(node.params[0], context)); - } - } - // -------------------------------------------------------------------------- // Public API // -------------------------------------------------------------------------- return { - MemberExpression: function(node) { - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = utils.getRelatedComponent(node); - if (!component) { - return; - } - - // e.g.: - // MyComponent.propTypes = { - // foo: PropTypes.string.isRequired, - // bar: PropTypes.string - // }; - // - // or: - // - // MyComponent.propTypes = myPropTypes; - if (node.parent.type === 'AssignmentExpression') { - const expression = resolveNodeValue(node.parent.right); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - return; - } - - // e.g.: - // MyComponent.propTypes.baz = PropTypes.string; - if (node.parent.type === 'MemberExpression' && node.parent.parent.type === 'AssignmentExpression') { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } - }, - - // e.g.: - // class Hello extends React.Component { - // static get propTypes() { - // return { - // name: PropTypes.string - // }; - // } - // static get defaultProps() { - // return { - // name: 'Dean' - // }; - // } - // render() { - // return
Hello {this.props.name}
; - // } - // } - MethodDefinition: function(node) { - if (!node.static || node.kind !== 'get') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const returnStatement = utils.findReturnStatement(node); - if (!returnStatement) { - return; - } - - const expression = resolveNodeValue(returnStatement.argument); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // class Greeting extends React.Component { - // render() { - // return ( - //

Hello, {this.props.foo} {this.props.bar}

- // ); - // } - // static propTypes = { - // foo: PropTypes.string, - // bar: PropTypes.string.isRequired - // }; - // } - ClassProperty: function(node) { - if (isPropTypeAnnotation(node)) { - handlePropTypeAnnotationClassProperty(node); - return; - } - - if (!node.static) { - return; - } - - if (!node.value) { - return; - } - - const isPropType = astUtil.getPropertyName(node) === 'propTypes'; - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const expression = resolveNodeValue(node.value); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // createReactClass({ - // render: function() { - // return
{this.props.foo}
; - // }, - // propTypes: { - // foo: PropTypes.string.isRequired, - // }, - // getDefaultProps: function() { - // return { - // foo: 'default' - // }; - // } - // }); - ObjectExpression: function(node) { - // find component this propTypes/defaultProps belongs to - const component = utils.isES5Component(node) && components.get(node); - if (!component) { - return; - } - - // Search for the proptypes declaration - node.properties.forEach(property => { - if (property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(property); - - if (isPropType && property.value.type === 'ObjectExpression') { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); - return; - } - }); - }, - - TypeAlias: function(node) { - typeScope(node.id.name, node.right); - }, - - Program: function() { - stack = [{}]; - }, - - BlockStatement: function () { - stack.push(Object.create(typeScope())); - }, - - 'BlockStatement:exit': function () { - stack.pop(); - }, - - // e.g.: - // type HelloProps = { - // foo?: string - // }; - // class Hello extends React.Component { - // static defaultProps = { - // foo: 'default' - // } - // render() { - // return
{this.props.foo}
; - // } - // }; - TypeParameterInstantiation: function(node) { - if (hasPropTypesAsGeneric(node)) { - handlePropTypesAsGeneric(node); - return; - } - }, - - // Check for type annotations in stateless components - FunctionDeclaration: handleStatelessComponent, - ArrowFunctionExpression: handleStatelessComponent, - FunctionExpression: handleStatelessComponent, - 'Program:exit': function() { - stack = null; const list = components.list(); - Object.keys(list).filter(component => list[component].propTypes).forEach(component => { + Object.keys(list).filter(component => list[component].declaredPropTypes).forEach(component => { reportPropTypesWithoutDefault( - list[component].propTypes, + list[component].declaredPropTypes, list[component].defaultProps || {} ); }); diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index 68c2641891..d90f7d7713 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -130,7 +130,7 @@ module.exports = function propTypesInstructions(context, components, utils) { type: 'shape', children: {} }; - iterateProperties(context, annotation.properties, (childKey, childValue) => { + iterateProperties(context, annotation.properties, (childKey, childValue, propNode) => { const fullName = [parentName, childKey].join('.'); if (!childKey && !childValue) { containsObjectTypeSpread = true; @@ -138,7 +138,7 @@ module.exports = function propTypesInstructions(context, components, utils) { const types = buildTypeAnnotationDeclarationTypes(childValue, fullName, seen); types.fullName = fullName; types.name = childKey; - types.node = childValue; + types.node = propNode; types.isRequired = !childValue.optional; shapeTypeDefinition.children[childKey] = types; } @@ -259,7 +259,7 @@ module.exports = function propTypesInstructions(context, components, utils) { const types = buildTypeAnnotationDeclarationTypes(value, key); types.fullName = key; types.name = key; - types.node = value; + types.node = propNode; types.isRequired = !propNode.optional; declaredPropTypes[key] = types; }); @@ -352,13 +352,15 @@ module.exports = function propTypesInstructions(context, components, utils) { type: 'shape', children: {} }; - iterateProperties(context, argument.properties, (childKey, childValue) => { - const fullName = [parentName, childKey].join('.'); - const types = buildReactDeclarationTypes(childValue, fullName); - types.fullName = fullName; - types.name = childKey; - types.node = childValue; - shapeTypeDefinition.children[childKey] = types; + iterateProperties(context, argument.properties, (childKey, childValue, propNode) => { + if (childValue) { // skip spread propTypes + const fullName = [parentName, childKey].join('.'); + const types = buildReactDeclarationTypes(childValue, fullName); + types.fullName = fullName; + types.name = childKey; + types.node = propNode; + shapeTypeDefinition.children[childKey] = types; + } }); return shapeTypeDefinition; case 'arrayOf': @@ -438,7 +440,7 @@ module.exports = function propTypesInstructions(context, components, utils) { ignorePropsValidation = declarePropTypesForObjectTypeAnnotation(propTypes, declaredPropTypes); break; case 'ObjectExpression': - iterateProperties(context, propTypes.properties, (key, value) => { + iterateProperties(context, propTypes.properties, (key, value, propNode) => { if (!value) { ignorePropsValidation = true; return; @@ -446,7 +448,7 @@ module.exports = function propTypesInstructions(context, components, utils) { const types = buildReactDeclarationTypes(value, key); types.fullName = key; types.name = key; - types.node = value; + types.node = propNode; types.isRequired = propsUtil.isRequiredPropType(value); declaredPropTypes[key] = types; }); @@ -485,7 +487,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.name = propTypes.property.name; types.fullName = [parentProp, propTypes.property.name].join('.'); - types.node = propTypes.property; + types.node = propTypes.parent; types.isRequired = propsUtil.isRequiredPropType(propTypes.parent.right); curDeclaredPropTypes[propTypes.property.name] = types; } else { diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js index dd3becf331..caec9ad450 100644 --- a/lib/util/usedPropTypes.js +++ b/lib/util/usedPropTypes.js @@ -460,8 +460,8 @@ module.exports = function usedPropTypesInstructions(context, components, utils) Object.keys(propTypes).forEach(key => { const node = propTypes[key].node; - if (astUtil.isFunctionLikeExpression(node)) { - markPropTypesAsUsed(node); + if (node.value && astUtil.isFunctionLikeExpression(node.value)) { + markPropTypesAsUsed(node.value); } }); } From 4d1b833427ef90c79c1614342f8429732954ed61 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 17 Aug 2018 21:36:16 -0700 Subject: [PATCH 42/49] Add test for #1908 --- .../rules/default-props-match-prop-types.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/lib/rules/default-props-match-prop-types.js b/tests/lib/rules/default-props-match-prop-types.js index d8b65302af..4b06b93da3 100644 --- a/tests/lib/rules/default-props-match-prop-types.js +++ b/tests/lib/rules/default-props-match-prop-types.js @@ -1568,6 +1568,27 @@ ruleTester.run('default-props-match-prop-types', rule, { message: 'defaultProp "foo" defined for isRequired propType.' } ] + }, + { + code: ` + class SomeComponent extends React.Component { + render() { + return
; + } + } + SomeComponent.propTypes = { + "firstProperty": PropTypes.string.isRequired, + }; + + SomeComponent.defaultProps = { + "firstProperty": () => undefined + }; + `, + errors: [ + { + message: 'defaultProp "firstProperty" defined for isRequired propType.' + } + ] } ] }); From 5211a2ba63952255572c749c56b8ee0f7f8603a1 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 26 Sep 2018 20:36:01 -0700 Subject: [PATCH 43/49] Add test for #1865 --- tests/lib/rules/require-default-props.js | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js index de42f32a78..a1c0225def 100644 --- a/tests/lib/rules/require-default-props.js +++ b/tests/lib/rules/require-default-props.js @@ -834,6 +834,19 @@ ruleTester.run('require-default-props', rule, { ].join('\n'), parser: 'babel-eslint', options: [{forbidDefaultForRequired: true}] + }, { + code: ` + type Props = { + +name?: string, + }; + function Hello(props: Props) { + return
Hello {props.name}
; + } + Hello.defaultProps = { + name: 'foo' + }; + `, + parser: 'babel-eslint' } ], @@ -2174,6 +2187,20 @@ ruleTester.run('require-default-props', rule, { }, { message: 'propType "bar" is not required, but has no corresponding defaultProp declaration.' }] + }, + { + code: ` + type Props = { + +name?: string, + }; + function Hello(props: Props) { + return
Hello {props.name}
; + } + `, + parser: 'babel-eslint', + errors: [{ + message: 'propType "name" is not required, but has no corresponding defaultProp declaration.' + }] } ] }); From d8a615f5857b7627b629ae98bc734fca6bcb03d4 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 26 Sep 2018 20:47:06 -0700 Subject: [PATCH 44/49] Use filter instead of if/return --- lib/rules/default-props-match-prop-types.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index 6270d2042d..cf1597a311 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -82,12 +82,8 @@ module.exports = { 'Program:exit': function() { const list = components.list(); - Object.keys(list).forEach(component => { - // If no defaultProps could be found, we don't report anything. - if (!list[component].defaultProps) { - return; - } - + // If no defaultProps could be found, we don't report anything. + Object.keys(list).filter(component => list[component].defaultProps).forEach(component => { reportInvalidDefaultProps( list[component].declaredPropTypes, list[component].defaultProps || {} From 9cf86d18d11ae28c58554359c94909264986dc28 Mon Sep 17 00:00:00 2001 From: Oliver Joseph Ash Date: Thu, 27 Sep 2018 14:21:43 +0100 Subject: [PATCH 45/49] `display-name` docs: mention default `ignoreTranspilerName` value --- docs/rules/display-name.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/display-name.md b/docs/rules/display-name.md index ca249e6821..2b17597fdc 100644 --- a/docs/rules/display-name.md +++ b/docs/rules/display-name.md @@ -33,7 +33,7 @@ var Hello = createReactClass({ ... ``` -### `ignoreTranspilerName` +### `ignoreTranspilerName` (default: `false`) When `true` the rule will ignore the name set by the transpiler and require a `displayName` property in this case. From 08e9d0b85841d764635c78d026cc3842c523d8f4 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 01:13:51 +0300 Subject: [PATCH 46/49] Extract util functions to AST utils --- lib/util/Components.js | 9 +++------ lib/util/ast.js | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index 2ea7cc896f..eb24ee41b4 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -457,14 +457,13 @@ function componentRule(rule, context) { while (scope) { const node = scope.block; const isFunction = /Function/.test(node.type); // Functions - const isArrowFunction = node.type === 'ArrowFunctionExpression'; + const isArrowFunction = astUtil.isArrowFunction(node); let enclosingScope = scope; if (isArrowFunction) { enclosingScope = utils.getArrowFunctionScope(scope); } - const enclosingScopeType = enclosingScope && enclosingScope.block.type; const enclosingScopeParent = enclosingScope && enclosingScope.block.parent; - const isClass = enclosingScopeType === 'ClassDeclaration' || enclosingScopeType === 'ClassExpression'; + const isClass = enclosingScope && astUtil.isClass(enclosingScope.block); const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.) // Attribute Expressions inside JSX Elements () @@ -490,9 +489,7 @@ function componentRule(rule, context) { getArrowFunctionScope(scope) { scope = scope.upper; while (scope) { - const type = scope.block.type; - if (type === 'FunctionExpression' || type === 'FunctionDeclaration' - || type === 'ClassDeclaration' || type === 'ClassExpression') { + if (astUtil.isFunction(scope.block) || astUtil.isClass(scope.block)) { return scope; } scope = scope.upper; diff --git a/lib/util/ast.js b/lib/util/ast.js index 10fc434625..9af7e0c5ce 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -102,11 +102,41 @@ function isFunctionLikeExpression(node) { return node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression'; } +/** + * Checks if the node is a function. + * @param {Object} context The node to check + * @return {Boolean} true if it's a function + */ +function isFunction(node) { + return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration'; +} + +/** + * Checks if the node is an arrow function. + * @param {Object} context The node to check + * @return {Boolean} true if it's an arrow function + */ +function isArrowFunction(node) { + return node.type === 'ArrowFunctionExpression'; +} + +/** + * Checks if the node is a class. + * @param {Object} context The node to check + * @return {Boolean} true if it's a class + */ +function isClass(node) { + return node.type === 'ClassDeclaration' || node.type === 'ClassExpression'; +} + module.exports = { findReturnStatement: findReturnStatement, getPropertyName: getPropertyName, getPropertyNameNode: getPropertyNameNode, getComponentProperties: getComponentProperties, - isNodeFirstInLine: isNodeFirstInLine, - isFunctionLikeExpression: isFunctionLikeExpression + isArrowFunction: isArrowFunction, + isClass: isClass, + isFunction: isFunction, + isFunctionLikeExpression: isFunctionLikeExpression, + isNodeFirstInLine: isNodeFirstInLine }; From 7b37681804871c2e5aff1b9b63e72932fd330daf Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 15:56:18 +0300 Subject: [PATCH 47/49] Remove redundant require('babel-eslint') from tests --- tests/lib/rules/boolean-prop-naming.js | 2 -- tests/lib/rules/default-props-match-prop-types.js | 2 -- tests/lib/rules/destructuring-assignment.js | 2 -- tests/lib/rules/display-name.js | 2 -- tests/lib/rules/forbid-component-props.js | 2 -- tests/lib/rules/forbid-dom-props.js | 2 -- tests/lib/rules/forbid-foreign-prop-types.js | 2 -- tests/lib/rules/jsx-sort-default-props.js | 2 -- tests/lib/rules/jsx-uses-vars.js | 2 -- tests/lib/rules/no-deprecated.js | 2 -- tests/lib/rules/no-did-mount-set-state.js | 2 -- tests/lib/rules/no-did-update-set-state.js | 2 -- tests/lib/rules/no-direct-mutation-state.js | 2 -- tests/lib/rules/no-multi-comp.js | 2 -- tests/lib/rules/no-string-refs.js | 2 -- tests/lib/rules/no-unused-prop-types.js | 2 -- tests/lib/rules/no-will-update-set-state.js | 2 -- tests/lib/rules/prefer-es6-class.js | 2 -- tests/lib/rules/prop-types.js | 2 -- tests/lib/rules/require-default-props.js | 2 -- tests/lib/rules/require-render-return.js | 2 -- tests/lib/rules/sort-comp.js | 2 -- tests/lib/rules/sort-prop-types.js | 2 -- 23 files changed, 46 deletions(-) diff --git a/tests/lib/rules/boolean-prop-naming.js b/tests/lib/rules/boolean-prop-naming.js index 16f000a7a6..a542b5f0ef 100644 --- a/tests/lib/rules/boolean-prop-naming.js +++ b/tests/lib/rules/boolean-prop-naming.js @@ -11,8 +11,6 @@ const rule = require('../../../lib/rules/boolean-prop-naming'); const RuleTester = require('eslint').RuleTester; -require('babel-eslint'); - const parserOptions = { ecmaVersion: 2018, sourceType: 'module', diff --git a/tests/lib/rules/default-props-match-prop-types.js b/tests/lib/rules/default-props-match-prop-types.js index d8b65302af..430fb973d6 100644 --- a/tests/lib/rules/default-props-match-prop-types.js +++ b/tests/lib/rules/default-props-match-prop-types.js @@ -12,8 +12,6 @@ const rule = require('../../../lib/rules/default-props-match-prop-types'); const RuleTester = require('eslint').RuleTester; -require('babel-eslint'); - const parserOptions = { ecmaVersion: 2018, sourceType: 'module', diff --git a/tests/lib/rules/destructuring-assignment.js b/tests/lib/rules/destructuring-assignment.js index e4b80f6108..482dd8510b 100644 --- a/tests/lib/rules/destructuring-assignment.js +++ b/tests/lib/rules/destructuring-assignment.js @@ -7,8 +7,6 @@ const rule = require('../../../lib/rules/destructuring-assignment'); const RuleTester = require('eslint').RuleTester; -require('babel-eslint'); - const parserOptions = { ecmaVersion: 2018, sourceType: 'module', diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 9919867361..d27ede8b29 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -11,8 +11,6 @@ const rule = require('../../../lib/rules/display-name'); const RuleTester = require('eslint').RuleTester; -require('babel-eslint'); - const parserOptions = { ecmaVersion: 2018, sourceType: 'module', diff --git a/tests/lib/rules/forbid-component-props.js b/tests/lib/rules/forbid-component-props.js index 01ea4b8c33..29d625390e 100644 --- a/tests/lib/rules/forbid-component-props.js +++ b/tests/lib/rules/forbid-component-props.js @@ -18,8 +18,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- diff --git a/tests/lib/rules/forbid-dom-props.js b/tests/lib/rules/forbid-dom-props.js index ca02b49002..ba0f1366c3 100644 --- a/tests/lib/rules/forbid-dom-props.js +++ b/tests/lib/rules/forbid-dom-props.js @@ -18,8 +18,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- diff --git a/tests/lib/rules/forbid-foreign-prop-types.js b/tests/lib/rules/forbid-foreign-prop-types.js index bcf6c2df5a..2ab08f6dad 100644 --- a/tests/lib/rules/forbid-foreign-prop-types.js +++ b/tests/lib/rules/forbid-foreign-prop-types.js @@ -18,8 +18,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- diff --git a/tests/lib/rules/jsx-sort-default-props.js b/tests/lib/rules/jsx-sort-default-props.js index 508ffe80d5..4c804a10ba 100644 --- a/tests/lib/rules/jsx-sort-default-props.js +++ b/tests/lib/rules/jsx-sort-default-props.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- diff --git a/tests/lib/rules/jsx-uses-vars.js b/tests/lib/rules/jsx-uses-vars.js index 65fe6ec8ed..6c258de76f 100644 --- a/tests/lib/rules/jsx-uses-vars.js +++ b/tests/lib/rules/jsx-uses-vars.js @@ -22,8 +22,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- diff --git a/tests/lib/rules/no-deprecated.js b/tests/lib/rules/no-deprecated.js index 12ab519904..221b49bc01 100644 --- a/tests/lib/rules/no-deprecated.js +++ b/tests/lib/rules/no-deprecated.js @@ -21,8 +21,6 @@ const parserOptions = { } }; -require('babel-eslint'); - function errorMessage(oldMethod, version, newMethod, refs) { newMethod = newMethod ? `, use ${newMethod} instead` : ''; refs = refs ? `, see ${refs}` : ''; diff --git a/tests/lib/rules/no-did-mount-set-state.js b/tests/lib/rules/no-did-mount-set-state.js index ae03823018..1acd459d14 100644 --- a/tests/lib/rules/no-did-mount-set-state.js +++ b/tests/lib/rules/no-did-mount-set-state.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-did-update-set-state.js b/tests/lib/rules/no-did-update-set-state.js index dcf3c5ff05..c50e295765 100644 --- a/tests/lib/rules/no-did-update-set-state.js +++ b/tests/lib/rules/no-did-update-set-state.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-direct-mutation-state.js b/tests/lib/rules/no-direct-mutation-state.js index 168aa568e8..b84bf3c071 100644 --- a/tests/lib/rules/no-direct-mutation-state.js +++ b/tests/lib/rules/no-direct-mutation-state.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-multi-comp.js b/tests/lib/rules/no-multi-comp.js index 9f71cca02b..291fdd3741 100644 --- a/tests/lib/rules/no-multi-comp.js +++ b/tests/lib/rules/no-multi-comp.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-string-refs.js b/tests/lib/rules/no-string-refs.js index 41e2012ff5..a36aaa72a6 100644 --- a/tests/lib/rules/no-string-refs.js +++ b/tests/lib/rules/no-string-refs.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index e29b4888eb..5b180d5c60 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -25,8 +25,6 @@ const settings = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/no-will-update-set-state.js b/tests/lib/rules/no-will-update-set-state.js index c2bec1d685..08a746b05d 100644 --- a/tests/lib/rules/no-will-update-set-state.js +++ b/tests/lib/rules/no-will-update-set-state.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/prefer-es6-class.js b/tests/lib/rules/prefer-es6-class.js index ca22b722d4..d047817ce7 100644 --- a/tests/lib/rules/prefer-es6-class.js +++ b/tests/lib/rules/prefer-es6-class.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 25d65635d3..dfeb0c5cd3 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -25,8 +25,6 @@ const settings = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js index de42f32a78..6698e67ddb 100644 --- a/tests/lib/rules/require-default-props.js +++ b/tests/lib/rules/require-default-props.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - const ruleTester = new RuleTester({parserOptions}); // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/require-render-return.js b/tests/lib/rules/require-render-return.js index 286d5671d9..68590dd31a 100644 --- a/tests/lib/rules/require-render-return.js +++ b/tests/lib/rules/require-render-return.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index 8277b10ab3..d7e9b4f806 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -19,8 +19,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ diff --git a/tests/lib/rules/sort-prop-types.js b/tests/lib/rules/sort-prop-types.js index 3f8d184626..684444214f 100644 --- a/tests/lib/rules/sort-prop-types.js +++ b/tests/lib/rules/sort-prop-types.js @@ -18,8 +18,6 @@ const parserOptions = { } }; -require('babel-eslint'); - // ----------------------------------------------------------------------------- // Tests // ----------------------------------------------------------------------------- From fee8c360c5c8b59e9c06cc68cce1951b756e927f Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 16:08:08 +0300 Subject: [PATCH 48/49] Remove redundant `require('babel-eslint')` --- tests/lib/rules/no-this-in-sfc.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js index 9c241cb8ed..b5d87f0267 100644 --- a/tests/lib/rules/no-this-in-sfc.js +++ b/tests/lib/rules/no-this-in-sfc.js @@ -16,8 +16,6 @@ const ERROR_MESSAGE = 'Stateless functional components should not use this'; const rule = require('../../../lib/rules/no-this-in-sfc'); const RuleTester = require('eslint').RuleTester; -require('babel-eslint'); - const parserOptions = { ecmaVersion: 2018, sourceType: 'module', From e779b00821d35a9d5d52b3efdd1d0c09e418d63c Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Fri, 28 Sep 2018 16:40:47 +0300 Subject: [PATCH 49/49] Minor adjustments --- lib/util/Components.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/util/Components.js b/lib/util/Components.js index eb24ee41b4..15026089dc 100644 --- a/lib/util/Components.js +++ b/lib/util/Components.js @@ -458,10 +458,7 @@ function componentRule(rule, context) { const node = scope.block; const isFunction = /Function/.test(node.type); // Functions const isArrowFunction = astUtil.isArrowFunction(node); - let enclosingScope = scope; - if (isArrowFunction) { - enclosingScope = utils.getArrowFunctionScope(scope); - } + const enclosingScope = isArrowFunction ? utils.getArrowFunctionScope(scope) : scope; const enclosingScopeParent = enclosingScope && enclosingScope.block.parent; const isClass = enclosingScope && astUtil.isClass(enclosingScope.block); const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods