Skip to content

Commit

Permalink
[New] destructuring-assignment: add option destructureInSignature
Browse files Browse the repository at this point in the history
  • Loading branch information
golopot authored and ljharb committed Mar 6, 2022
1 parent 37b4f8e commit 567bc7d
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,11 +5,15 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## Unreleased

### Added
* [`destructuring-assignment`]: add option `destructureInSignature` ([#3235][] @golopot)

### Fixed
* [`hook-use-state`]: Allow UPPERCASE setState setter prefixes ([#3244][] @duncanbeevers)
* `propTypes`: add `VFC` to react generic type param map ([#3230][] @dlech)

[#3244]: https://github.com/yannickcr/eslint-plugin-react/pull/3244
[#3235]: https://github.com/yannickcr/eslint-plugin-react/pull/3235
[#3230]: https://github.com/yannickcr/eslint-plugin-react/issues/3230

## [7.29.4] - 2022.03.13
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -123,7 +123,7 @@ Enable the rules that you would like to use.
| | | [react/boolean-prop-naming](docs/rules/boolean-prop-naming.md) | Enforces consistent naming for boolean props |
| | | [react/button-has-type](docs/rules/button-has-type.md) | Forbid "button" element without an explicit "type" attribute |
| | | [react/default-props-match-prop-types](docs/rules/default-props-match-prop-types.md) | Enforce all defaultProps are defined and not "required" in propTypes. |
| | | [react/destructuring-assignment](docs/rules/destructuring-assignment.md) | Enforce consistent usage of destructuring assignment of props, state, and context |
| | 🔧 | [react/destructuring-assignment](docs/rules/destructuring-assignment.md) | Enforce consistent usage of destructuring assignment of props, state, and context |
|| | [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-dom-props](docs/rules/forbid-dom-props.md) | Forbid certain props on DOM Nodes |
Expand Down
32 changes: 31 additions & 1 deletion docs/rules/destructuring-assignment.md
Expand Up @@ -91,7 +91,7 @@ const Foo = class extends React.PureComponent {

```js
...
"react/destructuring-assignment": [<enabled>, "always", { "ignoreClassFields": <boolean> }]
"react/destructuring-assignment": [<enabled>, "always", { "ignoreClassFields": <boolean>, "destructureInSignature": "always" | "ignore" }]
...
```

Expand All @@ -104,3 +104,33 @@ class Foo extends React.PureComponent {
bar = this.props.bar
}
```

### `destructureInSignature` (default: "ignore")

This option can be one of `always` or `ignore`. When configured with `always`, the rule will require props destructuring happens in the function signature.

Examples of **incorrect** code for `destructureInSignature: 'always'` :

```jsx
function Foo(props) {
const {a} = props;
return <>{a}</>
}
```

Examples of **correct** code for `destructureInSignature: 'always'` :

```jsx
function Foo({a}) {
return <>{a}</>
}
```

```jsx
// Ignores when props is used elsewhere
function Foo(props) {
const {a} = props;
useProps(props); // NOTE: it is a bad practice to pass the props object anywhere else!
return <Goo a={a}/>
}
```
46 changes: 45 additions & 1 deletion lib/rules/destructuring-assignment.js
Expand Up @@ -50,6 +50,7 @@ const messages = {
noDestructContextInSFCArg: 'Must never use destructuring context assignment in SFC argument',
noDestructAssignment: 'Must never use destructuring {{type}} assignment',
useDestructAssignment: 'Must use destructuring {{type}} assignment',
destructureInSignature: 'Must destructure props in the function signature.',
};

module.exports = {
Expand All @@ -60,7 +61,7 @@ module.exports = {
recommended: false,
url: docsUrl('destructuring-assignment'),
},

fixable: 'code',
messages,

schema: [{
Expand All @@ -75,6 +76,13 @@ module.exports = {
ignoreClassFields: {
type: 'boolean',
},
destructureInSignature: {
type: 'string',
enum: [
'always',
'ignore',
],
},
},
additionalProperties: false,
}],
Expand All @@ -83,6 +91,7 @@ module.exports = {
create: Components.detect((context, components, utils) => {
const configuration = context.options[0] || DEFAULT_OPTION;
const ignoreClassFields = (context.options[1] && (context.options[1].ignoreClassFields === true)) || false;
const destructureInSignature = (context.options[1] && context.options[1].destructureInSignature) || 'ignore';
const sfcParams = createSFCParams();

/**
Expand Down Expand Up @@ -230,6 +239,41 @@ module.exports = {
},
});
}

if (
SFCComponent
&& destructuringSFC
&& configuration === 'always'
&& destructureInSignature === 'always'
&& node.init.name === 'props'
) {
const scopeSetProps = context.getScope().set.get('props');
const propsRefs = scopeSetProps && scopeSetProps.references;
if (!propsRefs) {
return;
}
// Skip if props is used elsewhere
if (propsRefs.length > 1) {
return;
}
report(context, messages.destructureInSignature, 'destructureInSignature', {
node,
fix(fixer) {
const param = SFCComponent.node.params[0];
if (!param) {
return;
}
const replaceRange = [
param.range[0],
param.typeAnnotation ? param.typeAnnotation.range[0] : param.range[1],
];
return [
fixer.replaceTextRange(replaceRange, context.getSourceCode().getText(node.id)),
fixer.remove(node.parent),
];
},
});
}
},
};
}),
Expand Down
72 changes: 69 additions & 3 deletions tests/lib/rules/destructuring-assignment.js
Expand Up @@ -5,6 +5,8 @@
'use strict';

const RuleTester = require('eslint').RuleTester;
const semver = require('semver');
const eslintPkg = require('eslint/package.json');
const rule = require('../../../lib/rules/destructuring-assignment');

const parsers = require('../../helpers/parsers');
Expand Down Expand Up @@ -338,9 +340,27 @@ ruleTester.run('destructuring-assignment', rule, {
}
`,
},
{
code: `
function Foo(props) {
const {a} = props;
return <Goo {...props}>{a}</Goo>;
}
`,
options: ['always', { destructureInSignature: 'always' }],
},
{
code: `
function Foo(props) {
const {a} = props;
return <Goo f={() => props}>{a}</Goo>;
}
`,
options: ['always', { destructureInSignature: 'always' }],
},
]),

invalid: parsers.all([
invalid: parsers.all([].concat(
{
code: `
const MyComponent = (props) => {
Expand Down Expand Up @@ -632,7 +652,7 @@ ruleTester.run('destructuring-assignment', rule, {
const TestComp = (props) => {
props.onClick3102();
return (
<div
onClick={(evt) => {
Expand Down Expand Up @@ -720,5 +740,51 @@ ruleTester.run('destructuring-assignment', rule, {
},
],
},
]),
// Ignore for ESLint < 4 because ESLint < 4 does not support array fixer.
semver.satisfies(eslintPkg.version, '>= 4') ? [
{
code: `
function Foo(props) {
const {a} = props;
return <p>{a}</p>;
}
`,
options: ['always', { destructureInSignature: 'always' }],
errors: [
{
messageId: 'destructureInSignature',
line: 3,
},
],
output: `
function Foo({a}) {
return <p>{a}</p>;
}
`,
},
{
code: `
function Foo(props: FooProps) {
const {a} = props;
return <p>{a}</p>;
}
`,
options: ['always', { destructureInSignature: 'always' }],
errors: [
{
messageId: 'destructureInSignature',
line: 3,
},
],
output: `
function Foo({a}: FooProps) {
return <p>{a}</p>;
}
`,
features: ['ts', 'no-babel'],
},
] : []
)),
});

0 comments on commit 567bc7d

Please sign in to comment.