From 9c5ac98c506de10d48044740ee4f6bb48886f67e Mon Sep 17 00:00:00 2001 From: HenryBrown0 <26250092+HenryBrown0@users.noreply.github.com> Date: Wed, 28 Jun 2023 20:23:51 +0100 Subject: [PATCH] [Fix] `prefer-read-only-props`: add TS support --- CHANGELOG.md | 2 + docs/rules/prefer-read-only-props.md | 48 +++++++ lib/rules/prefer-read-only-props.js | 95 ++++++++----- tests/lib/rules/prefer-read-only-props.js | 154 +++++++++++++++++++++- 4 files changed, 264 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31c090ad94..296d1b1b69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,12 +21,14 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange * [`no-unused-state`]: avoid crashing on a class field function with destructured state ([#3568][] @ljharb) * [`no-unused-prop-types`]: allow using spread with object expression in jsx ([#3570][] @akulsr0) * Revert "[`destructuring-assignment`]: Handle destructuring of useContext in SFC" ([#3583][] [#2797][] @102) +* [`prefer-read-only-props`]: add TS support ([#3593][] @HenryBrown0) ### Changed * [Docs] [`jsx-newline`], [`no-unsafe`], [`static-property-placement`]: Fix code syntax highlighting ([#3563][] @nbsp1221) * [readme] resore configuration URL ([#3582][] @gokaygurcan) * [Docs] [`jsx-no-bind`]: reword performance rationale ([#3581][] @gpoole) +[#3593]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3593 [#3583]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3583 [#3582]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3582 [#3581]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3581 diff --git a/docs/rules/prefer-read-only-props.md b/docs/rules/prefer-read-only-props.md index 3e3d25f52a..9559235820 100644 --- a/docs/rules/prefer-read-only-props.md +++ b/docs/rules/prefer-read-only-props.md @@ -10,6 +10,8 @@ Using Flow, one can define types for props. This rule enforces that prop types a Examples of **incorrect** code for this rule: +In Flow: + ```jsx type Props = { name: string, @@ -29,8 +31,32 @@ const Hello = (props: {|name: string|}) => ( ); ``` +In TypeScript: + +```tsx +type Props = { + name: string; +} +class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } +} + +interface Props { + name: string; +} +class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } +} +``` + Examples of **correct** code for this rule: +In Flow: + ```jsx type Props = { +name: string, @@ -49,3 +75,25 @@ const Hello = (props: {|+name: string|}) => (
Hello {props.name}
); ``` + +In TypeScript: + +```tsx +type Props = { + readonly name: string; +} +class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } +} + +interface Props { + readonly name: string; +} +class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } +} +``` diff --git a/lib/rules/prefer-read-only-props.js b/lib/rules/prefer-read-only-props.js index a0353b38c0..31d7114643 100644 --- a/lib/rules/prefer-read-only-props.js +++ b/lib/rules/prefer-read-only-props.js @@ -16,6 +16,10 @@ function isFlowPropertyType(node) { return node.type === 'ObjectTypeProperty'; } +function isTypescriptPropertyType(node) { + return node.type === 'TSPropertySignature'; +} + function isCovariant(node) { return (node.variance && node.variance.kind === 'plus') || ( @@ -27,6 +31,14 @@ function isCovariant(node) { ); } +function isReadonly(node) { + return ( + node.typeAnnotation + && node.typeAnnotation.parent + && node.typeAnnotation.parent.readonly + ); +} + // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -50,38 +62,55 @@ module.exports = { schema: [], }, - create: Components.detect((context, components) => ({ - 'Program:exit'() { - flatMap( - values(components.list()), - (component) => component.declaredPropTypes || [] - ).forEach((declaredPropTypes) => { - Object.keys(declaredPropTypes).forEach((propName) => { - const prop = declaredPropTypes[propName]; - - if (!prop.node || !isFlowPropertyType(prop.node)) { - return; - } - - if (!isCovariant(prop.node)) { - report(context, messages.readOnlyProp, 'readOnlyProp', { - node: prop.node, - data: { - name: propName, - }, - fix: (fixer) => { - if (!prop.node.variance) { - // Insert covariance - return fixer.insertTextBefore(prop.node, '+'); - } - - // Replace contravariance with covariance - return fixer.replaceText(prop.node.variance, '+'); - }, - }); - } - }); + create: Components.detect((context, components) => { + function reportReadOnlyProp(prop, propName, fixer) { + report(context, messages.readOnlyProp, 'readOnlyProp', { + node: prop.node, + data: { + name: propName, + }, + fix: fixer, }); - }, - })), + } + + return { + 'Program:exit'() { + flatMap( + values(components.list()), + (component) => component.declaredPropTypes || [] + ).forEach((declaredPropTypes) => { + Object.keys(declaredPropTypes).forEach((propName) => { + const prop = declaredPropTypes[propName]; + if (!prop.node) { + return; + } + + if (isFlowPropertyType(prop.node)) { + if (!isCovariant(prop.node)) { + reportReadOnlyProp(prop, propName, (fixer) => { + if (!prop.node.variance) { + // Insert covariance + return fixer.insertTextBefore(prop.node, '+'); + } + + // Replace contravariance with covariance + return fixer.replaceText(prop.node.variance, '+'); + }); + } + + return; + } + + if (isTypescriptPropertyType(prop.node)) { + if (!isReadonly(prop.node)) { + reportReadOnlyProp(prop, propName, (fixer) => ( + fixer.insertTextBefore(prop.node, 'readonly ') + )); + } + } + }); + }); + }, + }; + }), }; diff --git a/tests/lib/rules/prefer-read-only-props.js b/tests/lib/rules/prefer-read-only-props.js index e2867902ed..7cef537361 100644 --- a/tests/lib/rules/prefer-read-only-props.js +++ b/tests/lib/rules/prefer-read-only-props.js @@ -167,7 +167,7 @@ ruleTester.run('prefer-read-only-props', rule, { import React from "react"; interface Props { - name: string; + readonly name: string; } const MyComponent: React.FC = ({ name }) => { @@ -176,7 +176,62 @@ ruleTester.run('prefer-read-only-props', rule, { export default MyComponent; `, - features: ['ts'], + features: ['ts', 'no-babel-old'], + }, + { + code: ` + import React from "react"; + type Props = { + readonly firstName: string; + readonly lastName: string; + } + const MyComponent: React.FC = ({ name }) => { + return
{name}
; + }; + export default MyComponent; + `, + features: ['ts', 'no-babel-old'], + }, + { + code: ` + import React from "react"; + type Props = { + readonly name: string; + } + const MyComponent: React.FC = ({ name }) => { + return
{name}
; + }; + export default MyComponent; + `, + features: ['ts', 'no-babel-old'], + }, + { + code: ` + import React from "react"; + type Props = { + readonly name: string[]; + } + const MyComponent: React.FC = ({ name }) => { + return
{name}
; + }; + export default MyComponent; + `, + features: ['ts', 'no-babel-old'], + }, + { + code: ` + import React from "react"; + type Props = { + readonly person: { + name: string; + } + } + const MyComponent: React.FC = ({ name }) => { + return
{name}
; + }; + export default MyComponent; + `, + features: ['ts', 'no-babel-old'], }, ]), @@ -383,5 +438,100 @@ ruleTester.run('prefer-read-only-props', rule, { }, ], }, + { + code: ` + type Props = { + name: string; + } + + class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } + } + `, + output: ` + type Props = { + readonly name: string; + } + + class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } + } + `, + features: ['ts', 'no-babel-old'], + errors: [ + { + messageId: 'readOnlyProp', + data: { name: 'name' }, + }, + ], + }, + { + code: ` + interface Props { + name: string; + } + + class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } + } + `, + output: ` + interface Props { + readonly name: string; + } + + class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } + } + `, + features: ['ts', 'no-babel-old'], + errors: [ + { + messageId: 'readOnlyProp', + data: { name: 'name' }, + }, + ], + }, + { + code: ` + type Props = { + readonly firstName: string; + lastName: string; + } + + class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } + } + `, + output: ` + type Props = { + readonly firstName: string; + readonly lastName: string; + } + + class Hello extends React.Component { + render () { + return
Hello {this.props.name}
; + } + } + `, + features: ['ts', 'no-babel-old'], + errors: [ + { + messageId: 'readOnlyProp', + data: { name: 'lastName' }, + }, + ], + }, ]), });