diff --git a/docs/rules/destructuring-assignment.md b/docs/rules/destructuring-assignment.md index def9567bcd..186d0f5247 100644 --- a/docs/rules/destructuring-assignment.md +++ b/docs/rules/destructuring-assignment.md @@ -5,6 +5,16 @@ Rule can be set to either of `always` or `never`; "react/destructuring-assignment": [, 'always'] ``` +It also accepts an object as the first option for more granular control for different component types. + +```js +"react/destructuring-assignment": [, { + SFC: 'always', + class: 'always', + createClass: 'always', +}] +``` + ## Rule Details By default rule is set to `always` enforce destructuring assignment. The following patterns are considered warnings: diff --git a/lib/rules/destructuring-assignment.js b/lib/rules/destructuring-assignment.js index 45c2af91f7..3ac57f9770 100644 --- a/lib/rules/destructuring-assignment.js +++ b/lib/rules/destructuring-assignment.js @@ -7,6 +7,41 @@ const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); const DEFAULT_OPTION = 'always'; +const SCHEMA = { + type: 'string', + enum: [ + 'always', + 'never' + ] +}; + +function isAssignmentToProp(node) { + return ( + node.parent && + node.parent.type === 'AssignmentExpression' && + node.parent.left === node + ); +} + +/** + * Check if the node is in a render method of the component. + * + * @param {Object} node - The node. + * @param {Object} component - The component. + * @returns {Boolean} - Returns whether or not the node is in a render method. + */ +function isInRenderMethod(node, component) { + if (node === component) { + return false; + } + + if (node.type === 'MethodDefinition' || node.type === 'Property') { + // Check if the method is named render and that it's not a static method + return !node.static && node.key.name === 'render'; + } + + return isInRenderMethod(node.parent, component); +} module.exports = { meta: { @@ -17,64 +52,88 @@ module.exports = { url: docsUrl('destructuring-assignment') }, schema: [{ - type: 'string', - enum: [ - 'always', - 'never' - ] + oneOf: [SCHEMA, { + type: 'object', + properties: { + SFC: SCHEMA, + class: SCHEMA, + createClass: SCHEMA + } + }] }] }, create: Components.detect((context, components, utils) => { - const configuration = context.options[0] || DEFAULT_OPTION; - + function getConfiguration(componentType) { + if (typeof context.options[0] === 'object') { + return context.options[0][componentType] || DEFAULT_OPTION; + } - /** - * 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 - ); + return context.options[0] || DEFAULT_OPTION; } + /** * @param {ASTNode} node We expect either an ArrowFunctionExpression, * FunctionDeclaration, or FunctionExpression */ function handleStatelessComponent(node) { - const destructuringProps = node.params && node.params[0] && node.params[0].type === 'ObjectPattern'; - const destructuringContext = node.params && node.params[1] && node.params[1].type === 'ObjectPattern'; + const hasProps = node.params && node.params[0]; + const isDestructuringProps = hasProps && node.params[0].type === 'ObjectPattern'; + const hasContext = node.params && node.params[1]; + const isDestructuringContext = hasContext && node.params[1].type === 'ObjectPattern'; - if (destructuringProps && components.get(node) && configuration === 'never') { - context.report({ - node: node, - message: 'Must never use destructuring props assignment in SFC argument' - }); - } else if (destructuringContext && components.get(node) && configuration === 'never') { - context.report({ - node: node, - message: 'Must never use destructuring context assignment in SFC argument' - }); + if (!components.get(node) || components.get(node).confidence === 0) { + return; + } + + if (getConfiguration('SFC') === 'never') { + if (hasProps && isDestructuringProps) { + context.report({ + node: node, + message: 'Must never use destructuring props assignment in SFC argument' + }); + } + + if (hasContext && isDestructuringContext) { + context.report({ + node: node, + message: 'Must never use destructuring context assignment in SFC argument' + }); + } } } - function handleSFCUsage(node) { - // props.aProp || context.aProp - const isPropUsed = (node.object.name === 'props' || node.object.name === 'context') && !isAssignmentToProp(node); - if (isPropUsed && configuration === 'always') { - context.report({ - node: node, - message: `Must use destructuring ${node.object.name} assignment` - }); + function handleStatelessUsage(node, component) { + if (getConfiguration('SFC') === 'always') { + const propsName = component.params[0] ? component.params[0].name : null; + const contextName = component.params[1] ? component.params[1].name : null; + + if (!node.object.type === 'Identifier') { + return; + } + + const objName = node.object.name; + + if (objName === propsName) { + context.report({ + node: node, + message: 'Must use destructuring props assignment in SFC argument' + }); + } else if (objName === contextName) { + context.report({ + node: node, + message: 'Must use destructuring context assignment in SFC argument' + }); + } } } - function handleClassUsage(node) { + function handleClassUsage(node, component, configuration) { + // Check if we are in a render method + if (!isInRenderMethod(node, component)) { + return; + } + // this.props.Aprop || this.context.aProp || this.state.aState const isPropUsed = ( node.object.type === 'MemberExpression' && node.object.object.type === 'ThisExpression' && @@ -91,47 +150,72 @@ module.exports = { } return { - FunctionDeclaration: handleStatelessComponent, - ArrowFunctionExpression: handleStatelessComponent, - FunctionExpression: handleStatelessComponent, MemberExpression: function(node) { - const SFCComponent = components.get(context.getScope(node).block); - const classComponent = utils.getParentComponent(node); - if (SFCComponent) { - handleSFCUsage(node); + const component = utils.getParentComponent(); + + if (!component) { + return; } - if (classComponent) { - handleClassUsage(node, classComponent); + + if (utils.isES6Component(component)) { + handleClassUsage(node, component, getConfiguration('class')); + } else if (utils.isES5Component(component)) { + handleClassUsage(node, component, getConfiguration('createClass')); + } else { + handleStatelessUsage(node, component); } }, VariableDeclarator: function(node) { - const classComponent = utils.getParentComponent(node); - const SFCComponent = components.get(context.getScope(node).block); - - const destructuring = (node.init && node.id && node.id.type === 'ObjectPattern'); - // let {foo} = props; - const destructuringSFC = destructuring && (node.init.name === 'props' || node.init.name === 'context'); - // let {foo} = this.props; - const destructuringClass = destructuring && node.init.object && node.init.object.type === 'ThisExpression' && ( - node.init.property.name === 'props' || node.init.property.name === 'context' || node.init.property.name === 'state' - ); - - if (SFCComponent && destructuringSFC && configuration === 'never') { + const component = utils.getParentComponent(node); + const isDestructuring = (node.init && node.id && node.id.type === 'ObjectPattern'); + + if (!isDestructuring || !component) { + return; + } + + // Checks for class components + if (utils.isES5Component(component) || utils.isES6Component(component)) { + const isDestructuringClassProperties = node.init.object && node.init.object.type === 'ThisExpression' && ( + node.init.property.name === 'props' || node.init.property.name === 'context' || node.init.property.name === 'state' + ); + + if (!isDestructuringClassProperties || !isInRenderMethod(node, component)) { + return; + } + + if (utils.isES5Component(component) && getConfiguration('createClass') === 'never') { + context.report({ + node: node, + message: `Must never use destructuring ${node.init.property.name} assignment` + }); + } else if (utils.isES6Component(component) && getConfiguration('class') === 'never') { + context.report({ + node: node, + message: `Must never use destructuring ${node.init.property.name} assignment` + }); + } + + return; + } + + if (component.params[0] && component.params[0].name === node.init.name) { context.report({ node: node, - message: `Must never use destructuring ${node.init.name} assignment` + message: 'Must never use destructuring props assignment' }); + + return; } - if (classComponent && destructuringClass && configuration === 'never') { + if (component.params[1] && component.params[1].name === node.init.name) { context.report({ node: node, - message: `Must never use destructuring ${node.init.property.name} assignment` + message: 'Must never use destructuring context assignment' }); } } diff --git a/tests/lib/rules/destructuring-assignment.js b/tests/lib/rules/destructuring-assignment.js index ca5a85e28a..b3ac1ce0d2 100644 --- a/tests/lib/rules/destructuring-assignment.js +++ b/tests/lib/rules/destructuring-assignment.js @@ -18,106 +18,77 @@ const parserOptions = { }; const ruleTester = new RuleTester({parserOptions}); + ruleTester.run('destructuring-assignment', rule, { valid: [{ - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.props; - return
{foo}
; - } - };`, - options: ['always'], - parser: 'babel-eslint' + code: `function Component({ color }) { + return {color}; + }` }, { - code: `const MyComponent = ({ id, className }) => ( -
- );` + code: 'const Component = ({ color }) => {color};' }, { - code: `const MyComponent = (props) => { - const { id, className } = props; - return
- };`, - parser: 'babel-eslint' + code: 'const Component = ({ color }, { onChange }) => {color};' }, { - code: `const MyComponent = ({ id, className }) => ( -
- );`, - options: ['always'] + code: 'const Component = (props) => {props.color};', + options: [{SFC: 'never'}] }, { - code: `const MyComponent = (props) => { - const { id, className } = props; - return
- };` - }, { - code: `const MyComponent = (props) => { - const { id, className } = props; - return
- };`, - options: ['always'] - }, { - code: `const MyComponent = (props) => ( -
- );` - }, { - code: `const MyComponent = (props) => ( -
- );`, - options: ['always'] - }, { - code: `const MyComponent = (props, { color }) => ( -
- );` - }, { - code: `const MyComponent = (props, { color }) => ( -
- );`, - options: ['always'] - }, { - code: `const Foo = class extends React.PureComponent { - render() { - return
{this.props.foo}
; - } - };`, - options: ['never'] - }, { - code: `class Foo extends React.Component { - doStuff() {} - render() { - return
{this.props.foo}
; - } - }`, - options: ['never'] - }, { - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.props; - return
{foo}
; - } - };` - }, { - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.props; - return
{foo}
; - } - };`, - options: ['always'], - parser: 'babel-eslint' - }, { - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.props; - return
{foo}
; + code: ` + class Component extends React.Component { + render() { + return this.props.children; + } } - };` + `, + options: [{class: 'never'}] }, { - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.props; - return
{foo}
; - } - };`, - options: ['always'], + code: ` + class Component extends React.Component { + componentDidMount() { + this.props.onMount(); + } + + render() { + const { children } = this.props; + return children; + } + } + `, + options: [{class: 'always'}] + }, { + code: ` + const Component = createReactClass({ + render() { + return this.props.children; + } + }); + `, + options: [{createClass: 'never'}] + }, { + code: ` + const Component = createReactClass({ + render() { + const { children } = this.props; + + return children; + } + }); + `, + options: [{createClass: 'always'}] + }, { + code: ` + const Component = createReactClass({ + componentDidMount() { + this.props.onMount(); + }, + + render() { + const { children } = this.props; + + return children; + } + }); + `, + options: [{createClass: 'always'}], parser: 'babel-eslint' }, { code: `const MyComponent = (props) => { @@ -137,117 +108,120 @@ ruleTester.run('destructuring-assignment', rule, { }], invalid: [{ - code: `const MyComponent = (props) => { - return (
) - };`, - errors: [ - {message: 'Must use destructuring props assignment'} - ] - }, { - code: `const MyComponent = ({ id, className }) => ( -
- );`, + code: `function Component({ color }) { + return {color}; + }`, options: ['never'], - errors: [ - {message: 'Must never use destructuring props assignment in SFC argument'} - ] + errors: [{message: 'Must never use destructuring props assignment in SFC argument'}] }, { - code: `const MyComponent = (props, { color }) => ( -
- );`, + code: 'const Component = ({ color }) => {color};', options: ['never'], - errors: [ - {message: 'Must never use destructuring context assignment in SFC argument'} - ] + errors: [{message: 'Must never use destructuring props assignment in SFC argument'}] }, { - code: `const Foo = class extends React.PureComponent { - render() { - return
{this.props.foo}
; - } - };`, - errors: [ - {message: 'Must use destructuring props assignment'} - ] + code: 'const Component = (props) => {props.color};', + options: [{class: 'never'}], + errors: [{message: 'Must use destructuring props assignment in SFC argument'}] }, { - code: `const Foo = class extends React.PureComponent { - render() { - return
{this.state.foo}
; - } - };`, - errors: [ - {message: 'Must use destructuring state assignment'} - ] - }, { - code: `const Foo = class extends React.PureComponent { - render() { - return
{this.context.foo}
; - } - };`, - errors: [ - {message: 'Must use destructuring context assignment'} - ] - }, { - code: `class Foo extends React.Component { - render() { return this.foo(); } - foo() { - return this.props.children; - } - }`, - errors: [ - {message: 'Must use destructuring props assignment'} - ] - }, { - code: `var Hello = React.createClass({ - render: function() { - return {this.props.foo}; - } - });`, - errors: [ - {message: 'Must use destructuring props assignment'} - ] - }, { - code: `const Foo = class extends React.PureComponent { - render() { - const foo = this.props.foo; - return
{foo}
; - } - };`, - errors: [ - {message: 'Must use destructuring props assignment'} - ] - }, { - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.props; - return
{foo}
; - } - };`, - options: ['never'], - parser: 'babel-eslint', - errors: [ - {message: 'Must never use destructuring props assignment'} - ] + code: 'const Component = (props) => {props.color.primary};', + options: ['always'], + errors: [{message: 'Must use destructuring props assignment in SFC argument'}] }, { - code: `const MyComponent = (props) => { - const { id, className } = props; - return
- };`, + code: 'const Component = (props, { onChange }) => {props.color};', options: ['never'], - parser: 'babel-eslint', - errors: [ - {message: 'Must never use destructuring props assignment'} - ] - }, { - code: `const Foo = class extends React.PureComponent { - render() { - const { foo } = this.state; - return
{foo}
; + errors: [{message: 'Must never use destructuring context assignment in SFC argument'}] + }, { + code: 'const Component = (props) => {props.color};', + options: [{SFC: 'always'}], + errors: [{message: 'Must use destructuring props assignment in SFC argument'}] + }, { + code: 'const Component = (props, context) => {context.color};', + options: [{SFC: 'always'}], + errors: [{message: 'Must use destructuring context assignment in SFC argument'}] + }, { + code: ` + function Component(props, context) { + const { onClick } = context; + + return
+ } + `, + options: [{SFC: 'never'}], + errors: [{message: 'Must never use destructuring context assignment'}] + }, { + code: ` + function Component(props) { + const { children } = props; + + return
{children}
; + } + `, + options: [{SFC: 'never'}], + errors: [{message: 'Must never use destructuring props assignment'}] + }, { + code: ` + class Component extends React.Component { + render() { + return this.props.children; + } + } + `, + options: [{class: 'always'}], + errors: [{message: 'Must use destructuring props assignment'}] + }, { + code: ` + const Component = createReactClass({ + render() { + return this.props.children; + } + }); + `, + options: [{createClass: 'always'}], + errors: [{message: 'Must use destructuring props assignment'}] + }, { + code: ` + const Component = createReactClass({ + render() { + const { children } = this.props; + + return children; + } + }); + `, + options: [{createClass: 'never'}], + errors: [{message: 'Must never use destructuring props assignment'}] + }, { + code: ` + class Component extends React.Component { + render() { + const { children } = this.props; + + return children; + } } - };`, - options: ['never'], - parser: 'babel-eslint', - errors: [ - {message: 'Must never use destructuring state assignment'} - ] + `, + options: [{class: 'never'}], + errors: [{message: 'Must never use destructuring props assignment'}] + }, { + code: ` + class Component extends React.Component { + render() { + const { value } = this.state; + + return value; + } + } + `, + options: [{class: 'never'}], + errors: [{message: 'Must never use destructuring state assignment'}] + }, { + code: ` + class Component extends React.Component { + render() { + return this.state.value; + } + } + `, + options: [{class: 'always'}], + errors: [{message: 'Must use destructuring state assignment'}] }] });