Skip to content

Commit

Permalink
fix: add ts support to prefer read only props
Browse files Browse the repository at this point in the history
  • Loading branch information
@HenryBrown0
HenryBrown0 committed Jun 28, 2023
1 parent ae64aa8 commit 0d770af
Show file tree
Hide file tree
Showing 3 changed files with 261 additions and 34 deletions.
48 changes: 48 additions & 0 deletions docs/rules/prefer-read-only-props.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,8 +31,32 @@ const Hello = (props: {|name: string|}) => (
);
```

In TypeScript:

```tsx
type Props = {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}

interface Props {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
```

Examples of **correct** code for this rule:

In Flow:

```jsx
type Props = {
+name: string,
Expand All @@ -49,3 +75,25 @@ const Hello = (props: {|+name: string|}) => (
<div>Hello {props.name}</div>
);
```

In TypeScript:

```tsx
type Props = {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}

interface Props {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
```
95 changes: 62 additions & 33 deletions lib/rules/prefer-read-only-props.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
|| (
Expand All @@ -27,6 +31,14 @@ function isCovariant(node) {
);
}

function isReadonly(node) {
return (
node.typeAnnotation
&& node.typeAnnotation.parent
&& node.typeAnnotation.parent.readonly
);
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand All @@ -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 (isTypeScriptPropertyType(prop.node)) {
if (!isReadonly(prop.node)) {
reportReadOnlyProp(prop, propName, (fixer) => (
fixer.insertTextBefore(prop.node, 'readonly ')
));
}

return;
}

if (isFlowPropertyType(prop.node)) {
if (!isCovariant(prop.node)) {
reportReadOnlyProp(prop, propName, (fixer) => {
if (!prop.node.variance) {

Check warning on line 101 in lib/rules/prefer-read-only-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/prefer-read-only-props.js#L99-L101 

Added lines #L99 - L101 were not covered by tests
// Insert covariance
return fixer.insertTextBefore(prop.node, '+');

Check warning on line 103 in lib/rules/prefer-read-only-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/prefer-read-only-props.js#L103 

Added line #L103 was not covered by tests
}

// Replace contravariance with covariance
return fixer.replaceText(prop.node.variance, '+');

Check warning on line 107 in lib/rules/prefer-read-only-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/prefer-read-only-props.js#L107 

Added line #L107 was not covered by tests
});
}
}
});
});
},
};
}),
};
152 changes: 151 additions & 1 deletion tests/lib/rules/prefer-read-only-props.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Props> = ({ name }) => {
Expand All @@ -178,6 +178,61 @@ ruleTester.run('prefer-read-only-props', rule, {
`,
features: ['ts'],
},
{
code: `
import React from "react";
type Props = {
readonly firstName: string;
readonly lastName: string;
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts'],
},
{
code: `
import React from "react";
type Props = {
readonly name: string;
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts'],
},
{
code: `
import React from "react";
type Props = {
readonly name: string[];
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts'],
},
{
code: `
import React from "react";
type Props = {
readonly person: {
name: string;
}
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts'],
},
]),

invalid: parsers.all([
Expand Down Expand Up @@ -383,5 +438,100 @@ ruleTester.run('prefer-read-only-props', rule, {
},
],
},
{
code: `
type Props = {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
output: `
type Props = {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
features: ['ts'],
errors: [
{
messageId: 'readOnlyProp',
data: { name: 'name' },
},
],
},
{
code: `
interface Props {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
output: `
interface Props {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
features: ['ts'],
errors: [
{
messageId: 'readOnlyProp',
data: { name: 'name' },
},
],
},
{
code: `
type Props = {
readonly firstName: string;
lastName: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
output: `
type Props = {
readonly firstName: string;
readonly lastName: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
features: ['ts'],
errors: [
{
messageId: 'readOnlyProp',
data: { name: 'lastName' },
},
],
},
]),
});

0 comments on commit 0d770af

Please sign in to comment.