Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[destructuring-assignment] : new rule #1462

Merged
merged 5 commits into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#

* [react/boolean-prop-naming](docs/rules/boolean-prop-naming.md): Enforces consistent naming for boolean props
* [react/default-props-match-prop-types](docs/rules/default-props-match-prop-types.md): Prevent extraneous defaultProps on components
* [react/destructuring-assignment](docs/rules/destructuring-assignment.md): Rule enforces consistent usage of destructuring assignment in component
* [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition
* [react/forbid-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components
* [react/forbid-elements](docs/rules/forbid-elements.md): Forbid certain elements
Expand Down
87 changes: 87 additions & 0 deletions docs/rules/destructuring-assignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Enforce consistent usage of destructuring assignment of props, state, and context (react/destructuring-assignment)

Rule can be set to either of `always` or `never`;
```js
"react/destructuring-assignment": [<enabled>, 'always']
```

## Rule Details

By default rule is set to `always` enforce destructuring assignment. The following patterns are considered warnings:

```js
const MyComponent = (props) => {
return (<div id={props.id} />)
};
```

```js
const Foo = class extends React.PureComponent {
render() {
return <div>{this.context.foo}</div>;
}
};
```

Below pattern is correct:

```js
const MyComponent = ({id}) => {
return (<div id={id} />)
};
```

```js
const MyComponent = (props, context) => {
const { id } = props;
return (<div id={id} />)
};
```

```js
const Foo = class extends React.PureComponent {
render() {
const { title } = this.context;
return <div>{title}</div>;
}
};
```

If rule is set to `never`, the following patterns are considered warning:

```js
const MyComponent = ({id}) => {
return (<div id={id} />)
};
```

```js
const MyComponent = (props) => {
const { id } = props;
return (<div id={id} />)
};
```

```js
const Foo = class extends React.PureComponent {
render() {
const { title } = this.state;
return <div>{title}</div>;
}
};
```

and below pattern is correct:

```js
const MyComponent = (props) => {
return (<div id={props.id} />)
};
```

```js
const Foo = class extends React.PureComponent {
render() {
return <div>{this.state.title}</div>;
}
};
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const has = require('has');
const allRules = {
'boolean-prop-naming': require('./lib/rules/boolean-prop-naming'),
'default-props-match-prop-types': require('./lib/rules/default-props-match-prop-types'),
'destructuring-assignment': require('./lib/rules/destructuring-assignment'),
'display-name': require('./lib/rules/display-name'),
'forbid-component-props': require('./lib/rules/forbid-component-props'),
'forbid-elements': require('./lib/rules/forbid-elements'),
Expand Down
137 changes: 137 additions & 0 deletions lib/rules/destructuring-assignment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/**
* @fileoverview Enforce consistent usage of destructuring assignment of props, state, and context.
**/
'use strict';

const Components = require('../util/Components');

const DEFAULT_OPTION = 'always';

module.exports = {
meta: {
docs: {
description: 'Enforce consistent usage of destructuring assignment of props, state, and context',
category: 'Stylistic Issues',
recommended: false
},
schema: [{
type: 'string',
enum: [
'always',
'never'
]
}]
},

create: Components.detect((context, components, utils) => {
const configuration = context.options[0] || 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
);
}
/**
* @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';

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'
});
}
}

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 handleClassUsage(node) {
// this.props.Aprop || this.context.aProp || this.state.aState
const isPropUsed = (node.object.type === 'MemberExpression' && node.object.object.type === 'ThisExpression' &&
(node.object.property.name === 'props' || node.object.property.name === 'context' || node.object.property.name === 'state') &&
node.object.type === 'MemberExpression'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node.object.type === 'MemberExpression' is asserted twice

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, thanks - want to make a PR to clean it up? :-D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure: #1502

);

if (isPropUsed && configuration === 'always') {
context.report({
node: node,
message: `Must use destructuring ${node.object.property.name} assignment`
});
}
}

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);
}
if (classComponent) {
handleClassUsage(node, classComponent);
}
},

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') {
context.report({
node: node,
message: `Must never use destructuring ${node.init.name} assignment`
});
}

if (classComponent && destructuringClass && configuration === 'never') {
context.report({
node: node,
message: `Must never use destructuring ${node.init.property.name} assignment`
});
}
}
};
})
};