Skip to content

Commit

Permalink
Merge pull request #2110 from drx/master
Browse files Browse the repository at this point in the history
[new] Add prefer-read-only-props rule
  • Loading branch information
ljharb committed Jan 21, 2019
2 parents e635964 + a1deb95 commit 57f0127
Show file tree
Hide file tree
Showing 4 changed files with 372 additions and 0 deletions.
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';
}

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];

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.'
}]
}
]
});

0 comments on commit 57f0127

Please sign in to comment.