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

[new] Add prefer-read-only-props rule #2110

Merged
merged 6 commits into from Jan 21, 2019
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
47 changes: 47 additions & 0 deletions docs/rules/prefer-read-only-props.md
@@ -0,0 +1,47 @@
# Enforce that props are read-only (react/prefer-read-only-props)

Using Flow, one can define types for props. This rule enforces that prop types are read-only (covariant).

## Rule Details

The following patterns are considered warnings:

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

function Hello(props: {-name: string}) {
return <div>Hello {props.name}</div>;
}

const Hello = (props: {|name: string|}) => (
<div>Hello {props.name}</div>
);
```

The following patterns are **not** considered warnings:

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

function Hello(props: {+name: string}) {
return <div>Hello {props.name}</div>;
}

const Hello = (props: {|+name: string|}) => (
<div>Hello {props.name}</div>
);
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -70,6 +70,7 @@ const allRules = {
'no-unused-state': require('./lib/rules/no-unused-state'),
'no-will-update-set-state': require('./lib/rules/no-will-update-set-state'),
'prefer-es6-class': require('./lib/rules/prefer-es6-class'),
'prefer-read-only-props': require('./lib/rules/prefer-read-only-props'),
'prefer-stateless-function': require('./lib/rules/prefer-stateless-function'),
'prop-types': require('./lib/rules/prop-types'),
'react-in-jsx-scope': require('./lib/rules/react-in-jsx-scope'),
Expand Down
74 changes: 74 additions & 0 deletions lib/rules/prefer-read-only-props.js
@@ -0,0 +1,74 @@
/**
* @fileoverview Require component props to be typed as read-only.
* @author Luke Zapart
*/
'use strict';

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

function isFlowPropertyType(node) {
return node.type === 'ObjectTypeProperty';
ljharb marked this conversation as resolved.
Show resolved Hide resolved
}

function isCovariant(node) {
return node.variance && node.variance.kind === 'plus';
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Require read-only props.',
category: 'Stylistic Issues',
recommended: false,
url: docsUrl('prefer-read-only-props')
},
fixable: 'code',
schema: []
},

create: Components.detect((context, components) => ({
'Program:exit': function () {
const list = components.list();

Object.keys(list).forEach(key => {
const component = list[key];

if (!component.declaredPropTypes) {
return;
}

Object.keys(component.declaredPropTypes).forEach(propName => {
const prop = component.declaredPropTypes[propName];
Copy link
Contributor

Choose a reason for hiding this comment

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

So cool to see the propTypes detection in use 👍


if (!isFlowPropertyType(prop.node)) {
return;
}

if (!isCovariant(prop.node)) {
context.report({
node: prop.node,
message: 'Prop \'{{propName}}\' should be read-only.',
data: {
propName
},
fix: fixer => {
if (!prop.node.variance) {
// Insert covariance
return fixer.insertTextBefore(prop.node, '+');
}

// Replace contravariance with covariance
return fixer.replaceText(prop.node.variance, '+');
}
});
}
});
});
}
}))
};
250 changes: 250 additions & 0 deletions tests/lib/rules/prefer-read-only-props.js
@@ -0,0 +1,250 @@
/**
* @fileoverview Require component props to be typed as read-only.
* @author Luke Zapart
*/
'use strict';

// -----------------------------------------------------------------------------
// Requirements
// -----------------------------------------------------------------------------

const rule = require('../../../lib/rules/prefer-read-only-props');
const RuleTester = require('eslint').RuleTester;

const parserOptions = {
ecmaVersion: 2018,
sourceType: 'module',
ecmaFeatures: {
jsx: true
}
};

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

const ruleTester = new RuleTester({parserOptions});
ruleTester.run('prefer-read-only-props', rule, {

valid: [
{
// Class component with type parameter
code: `
type Props = {
+name: string,
}

class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
parser: 'babel-eslint'
},
{
// Class component with typed props property
code: `
class Hello extends React.Component {
props: {
+name: string,
}

render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
parser: 'babel-eslint'
},
{
// Functional component with typed props argument
code: `
function Hello(props: {+name: string}) {
return <div>Hello {props.name}</div>;
}
`,
parser: 'babel-eslint'
},
{
// Functional component with type intersection
code: `
type PropsA = {+firstName: string};
type PropsB = {+lastName: string};
type Props = PropsA & PropsB;

function Hello({firstName, lastName}: Props) {
return <div>Hello {firstName} {lastName}</div>;
}
`,
parser: 'babel-eslint'
},
{
// Arrow function
code: `
const Hello = (props: {+name: string}) => (
<div>Hello {props.name}</div>
);
`,
parser: 'babel-eslint'
},
{
// Destructured props
code: `
const Hello = ({name}: {+name: string}) => (
<div>Hello {props.name}</div>
);
`,
parser: 'babel-eslint'
},
{
// No error, because this is not a component
code: `
const notAComponent = (props: {n: number}) => {
return props.n + 1;
};
`,
parser: 'babel-eslint'
},
{
// No error, because there is no Props flow type
code: `
class Hello extends React.Component {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`
},
{
// No error, because PropTypes do not support variance
code: `
class Hello extends React.Component {
render () {
return <div>Hello {this.props.name}</div>;
}
}
Hello.propTypes = {
name: PropTypes.string,
};
`
}
],

invalid: [
{
// Props.name is not read-only
code: `
type Props = {
name: string,
}

class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
},
{
// Props.name is contravariant
code: `
type Props = {
-name: string,
}

class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
},
{
code: `
class Hello extends React.Component {
props: {
name: string,
}

render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
},
{
code: `
function Hello(props: {name: string}) {
return <div>Hello {props.name}</div>;
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
},
{
code: `
function Hello(props: {|name: string|}) {
return <div>Hello {props.name}</div>;
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
},
{
code: `
function Hello({name}: {name: string}) {
return <div>Hello {props.name}</div>;
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
},
{
code: `
type PropsA = {firstName: string};
type PropsB = {lastName: string};
type Props = PropsA & PropsB;

function Hello({firstName, lastName}: Props) {
return <div>Hello {firstName} {lastName}</div>;
}
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'firstName\' should be read-only.'
}, {
message: 'Prop \'lastName\' should be read-only.'
}]
},
{
code: `
const Hello = (props: {-name: string}) => (
<div>Hello {props.name}</div>
);
`,
parser: 'babel-eslint',
errors: [{
message: 'Prop \'name\' should be read-only.'
}]
}
]
});