From 567bc7dac32418acbb8c02fed3ef8fe17ff29077 Mon Sep 17 00:00:00 2001 From: Chiawen Chen Date: Sun, 6 Mar 2022 18:02:55 +0800 Subject: [PATCH] [New] `destructuring-assignment`: add option `destructureInSignature` --- CHANGELOG.md | 4 ++ README.md | 2 +- docs/rules/destructuring-assignment.md | 32 ++++++++- lib/rules/destructuring-assignment.js | 46 ++++++++++++- tests/lib/rules/destructuring-assignment.js | 72 ++++++++++++++++++++- 5 files changed, 150 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 658879ba01..59623060b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/README.md b/README.md index b1efbcd0de..49c727fc81 100644 --- a/README.md +++ b/README.md @@ -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 | diff --git a/docs/rules/destructuring-assignment.md b/docs/rules/destructuring-assignment.md index 622d348665..e930b31cbb 100644 --- a/docs/rules/destructuring-assignment.md +++ b/docs/rules/destructuring-assignment.md @@ -91,7 +91,7 @@ const Foo = class extends React.PureComponent { ```js ... -"react/destructuring-assignment": [, "always", { "ignoreClassFields": }] +"react/destructuring-assignment": [, "always", { "ignoreClassFields": , "destructureInSignature": "always" | "ignore" }] ... ``` @@ -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 +} +``` diff --git a/lib/rules/destructuring-assignment.js b/lib/rules/destructuring-assignment.js index 8850df0d3d..2536bd671d 100644 --- a/lib/rules/destructuring-assignment.js +++ b/lib/rules/destructuring-assignment.js @@ -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 = { @@ -60,7 +61,7 @@ module.exports = { recommended: false, url: docsUrl('destructuring-assignment'), }, - + fixable: 'code', messages, schema: [{ @@ -75,6 +76,13 @@ module.exports = { ignoreClassFields: { type: 'boolean', }, + destructureInSignature: { + type: 'string', + enum: [ + 'always', + 'ignore', + ], + }, }, additionalProperties: false, }], @@ -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(); /** @@ -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), + ]; + }, + }); + } }, }; }), diff --git a/tests/lib/rules/destructuring-assignment.js b/tests/lib/rules/destructuring-assignment.js index 6f4286958e..ac104de085 100644 --- a/tests/lib/rules/destructuring-assignment.js +++ b/tests/lib/rules/destructuring-assignment.js @@ -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'); @@ -338,9 +340,27 @@ ruleTester.run('destructuring-assignment', rule, { } `, }, + { + code: ` + function Foo(props) { + const {a} = props; + return {a}; + } + `, + options: ['always', { destructureInSignature: 'always' }], + }, + { + code: ` + function Foo(props) { + const {a} = props; + return props}>{a}; + } + `, + options: ['always', { destructureInSignature: 'always' }], + }, ]), - invalid: parsers.all([ + invalid: parsers.all([].concat( { code: ` const MyComponent = (props) => { @@ -632,7 +652,7 @@ ruleTester.run('destructuring-assignment', rule, { const TestComp = (props) => { props.onClick3102(); - + return (
{ @@ -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

{a}

; + } + `, + options: ['always', { destructureInSignature: 'always' }], + errors: [ + { + messageId: 'destructureInSignature', + line: 3, + }, + ], + output: ` + function Foo({a}) { + + return

{a}

; + } + `, + }, + { + code: ` + function Foo(props: FooProps) { + const {a} = props; + return

{a}

; + } + `, + options: ['always', { destructureInSignature: 'always' }], + errors: [ + { + messageId: 'destructureInSignature', + line: 3, + }, + ], + output: ` + function Foo({a}: FooProps) { + + return

{a}

; + } + `, + features: ['ts', 'no-babel'], + }, + ] : [] + )), });