Merge pull request #392 from chrislaskey/required-first

Add requiredFirst option to jsx-sort-prop-types
jsx-eslint · Jan 13, 2016 · 42f9c0c · 42f9c0c
2 parents 1371c76 + ce330d8
commit 42f9c0c
Show file tree

Hide file tree

Showing 3 changed files with 141 additions and 1 deletion.
diff --git a/docs/rules/jsx-sort-prop-types.md b/docs/rules/jsx-sort-prop-types.md
@@ -80,7 +80,8 @@ class Component extends React.Component {
 ...
 "jsx-sort-prop-types": [<enabled>, {
   "callbacksLast": <boolean>,
-  "ignoreCase": <boolean>
+  "ignoreCase": <boolean>,
+  "requiredFirst": <boolean>,
 }]
 ...
 ```
@@ -105,6 +106,22 @@ var Component = React.createClass({
 });
 ```
 
+### `requiredFirst`
+
+When `true`, prop types for required props must be listed before all other props:
+
+```js
+var Component = React.createClass({
+  propTypes: {
+    barRequired: React.PropTypes.any.isRequired,
+    fooRequired: React.PropTypes.any.isRequired,
+    a: React.PropTypes.number,
+    z: React.PropTypes.string,
+  },
+...
+});
+```
+
 ## When not to use
 
 This rule is a formatting preference and not following it won't negatively affect the quality of your code. If alphabetizing props declarations isn't a part of your coding standards, then you can leave this rule off.
diff --git a/lib/rules/jsx-sort-prop-types.js b/lib/rules/jsx-sort-prop-types.js
@@ -10,6 +10,7 @@
 module.exports = function(context) {
 
   var configuration = context.options[0] || {};
+  var requiredFirst = configuration.requiredFirst || false;
   var callbacksLast = configuration.callbacksLast || false;
   var ignoreCase = configuration.ignoreCase || false;
 
@@ -38,10 +39,18 @@ module.exports = function(context) {
     return node.key.type === 'Identifier' ? node.key.name : node.key.value;
   }
 
+  function getValueName(node) {
+    return node.value.property.name;
+  }
+
   function isCallbackPropName(propName) {
     return /^on[A-Z]/.test(propName);
   }
 
+  function isRequiredProp(node) {
+    return getValueName(node) === 'isRequired';
+  }
+
   /**
    * Checks if propTypes declarations are sorted
    * @param {Array} declarations The array of AST nodes being checked.
@@ -51,6 +60,8 @@ module.exports = function(context) {
     declarations.reduce(function(prev, curr) {
       var prevPropName = getKey(prev);
       var currentPropName = getKey(curr);
+      var previousIsRequired = isRequiredProp(prev);
+      var currentIsRequired = isRequiredProp(curr);
       var previousIsCallback = isCallbackPropName(prevPropName);
       var currentIsCallback = isCallbackPropName(currentPropName);
 
@@ -59,6 +70,18 @@ module.exports = function(context) {
         currentPropName = currentPropName.toLowerCase();
       }
 
+      if (requiredFirst) {
+        if (previousIsRequired && !currentIsRequired) {
+          // Transition between required and non-required. Don't compare for alphabetical.
+          return curr;
+        }
+        if (!previousIsRequired && currentIsRequired) {
+          // Encountered a non-required prop after a required prop
+          context.report(curr, 'Required prop types must be listed before all other prop types');
+          return curr;
+        }
+      }
+
       if (callbacksLast) {
         if (!previousIsCallback && currentIsCallback) {
           // Entering the callback prop section
@@ -117,6 +140,9 @@ module.exports = function(context) {
 module.exports.schema = [{
   type: 'object',
   properties: {
+    requiredFirst: {
+      type: 'boolean'
+    },
     callbacksLast: {
       type: 'boolean'
     },

diff --git a/tests/lib/rules/jsx-sort-prop-types.js b/tests/lib/rules/jsx-sort-prop-types.js
@@ -191,6 +191,20 @@ ruleTester.run('jsx-sort-prop-types', rule, {
       '});'
     ].join('\n'),
     parserOptions: parserOptions
+  }, {
+    code: [
+      'var First = React.createClass({',
+      '  propTypes: {',
+      '    barRequired: React.PropTypes.func.isRequired,',
+      '    onBar: React.PropTypes.func,',
+      '    z: React.PropTypes.any',
+      '  },',
+      '  render: function() {',
+      '    return <div />;',
+      '  }',
+      '});'
+    ].join('\n'),
+    parserOptions: parserOptions
   }, {
     code: [
       'var First = React.createClass({',
@@ -246,6 +260,43 @@ ruleTester.run('jsx-sort-prop-types', rule, {
       callbacksLast: true
     }],
     parserOptions: parserOptions
+  }, {
+    code: [
+      'class First extends React.Component {',
+      '  render() {',
+      '    return <div />;',
+      '  }',
+      '}',
+      'First.propTypes = {',
+      '    barRequired: React.PropTypes.string.isRequired,',
+      '    a: React.PropTypes.any',
+      '};'
+    ].join('\n'),
+    options: [{
+      requiredFirst: true
+    }],
+    parserOptions: parserOptions
+  }, {
+    code: [
+      'class First extends React.Component {',
+      '  render() {',
+      '    return <div />;',
+      '  }',
+      '}',
+      'First.propTypes = {',
+      '    barRequired: React.PropTypes.string.isRequired,',
+      '    fooRequired: React.PropTypes.any.isRequired,',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onBar: React.PropTypes.func,',
+      '    onFoo: React.PropTypes.func',
+      '};'
+    ].join('\n'),
+    options: [{
+      requiredFirst: true,
+      callbacksLast: true
+    }],
+    parserOptions: parserOptions
   }],
 
   invalid: [{
@@ -483,5 +534,51 @@ ruleTester.run('jsx-sort-prop-types', rule, {
       column: 5,
       type: 'Property'
     }]
+  }, {
+    code: [
+      'var First = React.createClass({',
+      '  propTypes: {',
+      '    fooRequired: React.PropTypes.string.isRequired,',
+      '    barRequired: React.PropTypes.string.isRequired,',
+      '    a: React.PropTypes.any',
+      '  },',
+      '  render: function() {',
+      '    return <div />;',
+      '  }',
+      '});'
+    ].join('\n'),
+    options: [{
+      requiredFirst: true
+    }],
+    parserOptions: parserOptions,
+    errors: [{
+      message: ERROR_MESSAGE,
+      line: 4,
+      column: 5,
+      type: 'Property'
+    }]
+  }, {
+    code: [
+      'var First = React.createClass({',
+      '  propTypes: {',
+      '    a: React.PropTypes.any,',
+      '    barRequired: React.PropTypes.string.isRequired,',
+      '    onFoo: React.PropTypes.func',
+      '  },',
+      '  render: function() {',
+      '    return <div />;',
+      '  }',
+      '});'
+    ].join('\n'),
+    options: [{
+      requiredFirst: true
+    }],
+    parserOptions: parserOptions,
+    errors: [{
+      message: 'Required prop types must be listed before all other prop types',
+      line: 4,
+      column: 5,
+      type: 'Property'
+    }]
   }]
 });