Merge pull request #1532 from jomasti/issue-1524

Add forbidDefaultForRequired option
jsx-eslint · Nov 15, 2017 · 27b8279 · 27b8279
2 parents 24190c6 + 072ed2c
commit 27b8279
Show file tree

Hide file tree

Showing 3 changed files with 226 additions and 14 deletions.
diff --git a/docs/rules/require-default-props.md b/docs/rules/require-default-props.md
@@ -102,6 +102,19 @@ Greeting.defaultProps = {
 };
 ```
 
+```jsx
+type Props = {
+  foo: string,
+  bar?: string
+};
+
+function MyStatelessComponent(props: Props) {
+  return <div>Hello {props.foo} {props.bar}</div>;
+}
+```
+
+The following patterns are **not** considered warnings:
+
 ```jsx
 class Greeting extends React.Component {
   render() {
@@ -121,19 +134,6 @@ class Greeting extends React.Component {
 }
 ```
 
-```jsx
-type Props = {
-  foo: string,
-  bar?: string
-};
-
-function MyStatelessComponent(props: Props) {
-  return <div>Hello {props.foo} {props.bar}</div>;
-}
-```
-
-The following patterns are **not** considered warnings:
-
 ```jsx
 function MyStatelessComponent({ foo, bar }) {
   return <div>{foo}{bar}</div>;
@@ -184,6 +184,91 @@ NotAComponent.propTypes = {
 };
 ```
 
+## Rule Options
+
+```js
+...
+"react/require-default-props": [<enabled>, { forbidDefaultForRequired: <boolean> }]
+...
+```
+
+* `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0.
+* `forbidDefaultForRequired`: optional boolean to forbid prop default for a required prop. Defaults to false.
+
+### `forbidDefaultForRequired`
+
+Forbids setting a default for props that are marked as `isRequired`.
+
+The following patterns are warnings:
+
+```jsx
+class Greeting extends React.Component {
+  render() {
+    return (
+      <h1>Hello, {this.props.foo} {this.props.bar}</h1>
+    );
+  }
+
+  static propTypes = {
+    foo: PropTypes.string,
+    bar: PropTypes.string.isRequired
+  };
+
+  static defaultProps = {
+    foo: "foo",
+    bar: "bar"
+  };
+}
+```
+
+```jsx
+function MyStatelessComponent({ foo, bar }) {
+  return <div>{foo}{bar}</div>;
+}
+
+MyStatelessComponent.propTypes = {
+  foo: PropTypes.string.isRequired,
+  bar: PropTypes.string
+};
+
+MyStatelessComponent.defaultProps = {
+  foo: 'foo',
+  bar: 'bar'
+};
+```
+
+The following patterns are **not** warnings:
+
+```jsx
+class Greeting extends React.Component {
+  render() {
+    return (
+      <h1>Hello, {this.props.foo} {this.props.bar}</h1>
+    );
+  }
+
+  static propTypes = {
+    foo: PropTypes.string,
+    bar: PropTypes.string.isRequired
+  };
+
+  static defaultProps = {
+    foo: "foo"
+  };
+}
+```
+
+```jsx
+function MyStatelessComponent({ foo, bar }) {
+  return <div>{foo}{bar}</div>;
+}
+
+MyStatelessComponent.propTypes = {
+  foo: PropTypes.string.isRequired,
+  bar: PropTypes.string.isRequired
+};
+```
+
 ## When Not To Use It
 
 If you don't care about using `defaultsProps` for your component's props that are not required, you can disable this rule.

diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js
@@ -24,12 +24,22 @@ module.exports = {
       category: 'Best Practices'
     },
 
-    schema: []
+    schema: [{
+      type: 'object',
+      properties: {
+        forbidDefaultForRequired: {
+          type: 'boolean'
+        }
+      },
+      additionalProperties: false
+    }]
   },
 
   create: Components.detect((context, components, utils) => {
     const sourceCode = context.getSourceCode();
     const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []);
+    const configuration = context.options[0] || {};
+    const forbidDefaultForRequired = configuration.forbidDefaultForRequired || false;
 
     /**
      * Find a variable by name in the current scope.
@@ -285,6 +295,13 @@ module.exports = {
 
       propTypes.forEach(prop => {
         if (prop.isRequired) {
+          if (forbidDefaultForRequired && defaultProps[prop.name]) {
+            context.report(
+              prop.node,
+              'propType "{{name}}" is required and should not have a defaultProp declaration.',
+              {name: prop.name}
+            );
+          }
           return;
         }
 

diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js
@@ -735,6 +735,20 @@ ruleTester.run('require-default-props', rule, {
         '};'
       ].join('\n'),
       parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello extends React.Component {',
+        '  static propTypes = {',
+        '    foo: PropTypes.string.isRequired',
+        '  }',
+        '  render() {',
+        '    return <div>Hello {this.props.foo}</div>;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      options: [{forbidDefaultForRequired: true}]
     }
   ],
 
@@ -1896,6 +1910,102 @@ ruleTester.run('require-default-props', rule, {
       errors: [{
         message: 'propType "first-name" is not required, but has no corresponding defaultProp declaration.'
       }]
+    },
+    {
+      code: [
+        'class Hello extends React.Component {',
+        '  render() {',
+        '    return <div>Hello {this.props.foo}</div>;',
+        '  }',
+        '}',
+        'Hello.propTypes = {',
+        '  foo: PropTypes.string.isRequired',
+        '};',
+        'Hello.defaultProps = {',
+        '  foo: \'bar\'',
+        '};'
+      ].join('\n'),
+      options: [{forbidDefaultForRequired: true}],
+      errors: [{
+        message: 'propType "foo" is required and should not have a defaultProp declaration.'
+      }]
+    },
+    {
+      code: [
+        'function Hello(props) {',
+        '  return <div>Hello {props.foo}</div>;',
+        '}',
+        'Hello.propTypes = {',
+        '  foo: PropTypes.string.isRequired',
+        '};',
+        'Hello.defaultProps = {',
+        '  foo: \'bar\'',
+        '};'
+      ].join('\n'),
+      options: [{forbidDefaultForRequired: true}],
+      errors: [{
+        message: 'propType "foo" is required and should not have a defaultProp declaration.'
+      }]
+    },
+    {
+      code: [
+        'const Hello = (props) => {',
+        '  return <div>Hello {props.foo}</div>;',
+        '};',
+        'Hello.propTypes = {',
+        '  foo: PropTypes.string.isRequired',
+        '};',
+        'Hello.defaultProps = {',
+        '  foo: \'bar\'',
+        '};'
+      ].join('\n'),
+      options: [{forbidDefaultForRequired: true}],
+      errors: [{
+        message: 'propType "foo" is required and should not have a defaultProp declaration.'
+      }]
+    },
+    {
+      code: [
+        'class Hello extends React.Component {',
+        '  static propTypes = {',
+        '    foo: PropTypes.string.isRequired',
+        '  }',
+        '  static defaultProps = {',
+        '    foo: \'bar\'',
+        '  }',
+        '  render() {',
+        '    return <div>Hello {this.props.foo}</div>;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      options: [{forbidDefaultForRequired: true}],
+      errors: [{
+        message: 'propType "foo" is required and should not have a defaultProp declaration.'
+      }]
+    },
+    {
+      code: [
+        'class Hello extends React.Component {',
+        '  static get propTypes () {',
+        '    return {',
+        '      foo: PropTypes.string.isRequired',
+        '    };',
+        '  }',
+        '  static get defaultProps() {',
+        '    return {',
+        '      foo: \'bar\'',
+        '    };',
+        '  }',
+        '  render() {',
+        '    return <div>Hello {this.props.foo}</div>;',
+        '  }',
+        '}'
+      ].join('\n'),
+      options: [{forbidDefaultForRequired: true}],
+      errors: [{
+        message: 'propType "foo" is required and should not have a defaultProp declaration.'
+      }]
     }
   ]
 });