Allow a method to be in different places if it's matches multiple pat…

…terns
jsx-eslint · May 14, 2015 · e0c3cd4 · e0c3cd4
1 parent 5aadd63
commit e0c3cd4
Show file tree

Hide file tree

Showing 2 changed files with 152 additions and 46 deletions.
diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js
@@ -91,15 +91,16 @@ module.exports = function(context) {
   var regExpRegExp = /\/(.*)\/([g|y|i|m]*)/;
 
   /**
-   * Get index of the matching pattern in methods order configuration
+   * Get indexes of the matching patterns in methods order configuration
    * @param {String} method - Method name.
-   * @returns {Number} The matching pattern index. Return Infinity if there is no match.
+   * @returns {Array} The matching patterns indexes. Return [Infinity] if there is no match.
    */
-  function getRefPropIndex(method) {
+  function getRefPropIndexes(method) {
     var isRegExp;
     var matching;
     var i;
     var j;
+    var indexes = [];
     for (i = 0, j = methodsOrder.length; i < j; i++) {
       isRegExp = methodsOrder[i].match(regExpRegExp);
       if (isRegExp) {
@@ -108,26 +109,33 @@ module.exports = function(context) {
         matching = methodsOrder[i] === method;
       }
       if (matching) {
-        return i;
+        indexes.push(i);
       }
     }
 
     // No matching pattern, return 'everything-else' index
-    for (i = 0, j = methodsOrder.length; i < j; i++) {
-      if (methodsOrder[i] === 'everything-else') {
-        return i;
+    if (indexes.length === 0) {
+      for (i = 0, j = methodsOrder.length; i < j; i++) {
+        if (methodsOrder[i] === 'everything-else') {
+          indexes.push(i);
+        }
       }
     }
 
-    return Infinity;
+    // No matching pattern and no 'everything-else' group
+    if (indexes.length === 0) {
+      indexes.push(Infinity);
+    }
+
+    return indexes;
   }
 
   /**
    * Get properties name
    * @param {Object} prop - Property.
    * @returns {String} Property name.
    */
-  function getPropName(prop) {
+  function getPropertyName(prop) {
     return prop.key.name;
   }
 
@@ -211,6 +219,118 @@ module.exports = function(context) {
     }
   }
 
+  /**
+   * Get properties for a given AST node
+   * @param {ASTNode} node The AST node being checked.
+   * @returns {Array} Properties array.
+   */
+  function getComponentProperties(node) {
+    if (node.type === 'ClassDeclaration') {
+      return node.body.body;
+    }
+    return node.properties;
+  }
+
+  /**
+   * Compare two properties and find out if they are in the right order
+   * @param {Array} propertiesNames Array containing all the properties names.
+   * @param {String} propA First property name.
+   * @param {String} propB Second property name.
+   * @returns {Object} Object containing a correct true/false flag and the correct indexes for the two properties.
+   */
+  function comparePropsOrder(propertiesNames, propA, propB) {
+    var i;
+    var j;
+    var k;
+    var l;
+    var refIndexA;
+    var refIndexB;
+
+    // Get references indexes (the correct position) for given properties
+    var refIndexesA = getRefPropIndexes(propA);
+    var refIndexesB = getRefPropIndexes(propB);
+
+    // Get current indexes for given properties
+    var classIndexA = propertiesNames.indexOf(propA);
+    var classIndexB = propertiesNames.indexOf(propB);
+
+    // Loop around the references indexes for the 1st property
+    for (i = 0, j = refIndexesA.length; i < j; i++) {
+      refIndexA = refIndexesA[i];
+
+      // Loop around the properties for the 2nd property (for comparison)
+      for (k = 0, l = refIndexesB.length; k < l; k++) {
+        refIndexB = refIndexesB[k];
+
+        if (
+          // Comparing the same properties
+          refIndexA === refIndexB ||
+          // 1st property is placed before the 2nd one in reference and in current component
+          refIndexA < refIndexB && classIndexA < classIndexB ||
+          // 1st property is placed after the 2nd one in reference and in current component
+          refIndexA > refIndexB && classIndexA > classIndexB
+        ) {
+          return {
+            correct: true,
+            indexA: classIndexA,
+            indexB: classIndexB
+          };
+        }
+
+      }
+    }
+
+    // We did not find any correct match between reference and current component
+    return {
+      correct: false,
+      indexA: refIndexA,
+      indexB: refIndexB
+    };
+  }
+
+  /**
+   * Check properties order from a properties list and store the eventual errors
+   * @param {Array} properties Array containing all the properties.
+   */
+  function checkPropsOrder(properties) {
+    var propertiesNames = properties.map(getPropertyName);
+    var i;
+    var j;
+    var k;
+    var l;
+    var propA;
+    var propB;
+    var order;
+
+    // Loop around the properties
+    for (i = 0, j = propertiesNames.length; i < j; i++) {
+      propA = propertiesNames[i];
+
+      // Loop around the properties a second time (for comparison)
+      for (k = 0, l = propertiesNames.length; k < l; k++) {
+        propB = propertiesNames[k];
+
+        // Compare the properties order
+        order = comparePropsOrder(propertiesNames, propA, propB);
+
+        // Continue to next comparison is order is correct
+        if (order.correct === true) {
+          continue;
+        }
+
+        // Store an error if the order is incorrect
+        storeError({
+          node: properties[i],
+          index: order.indexA
+        }, {
+          node: properties[k],
+          index: order.indexB
+        });
+      }
+    }
+
+  }
+
   return {
     'Program:exit': function() {
       var list = componentList.getList();
@@ -219,43 +339,8 @@ module.exports = function(context) {
         if (!list.hasOwnProperty(component) || !mustBeValidated(list[component])) {
           continue;
         }
-
-        var properties;
-        if (list[component].node.type === 'ClassDeclaration') {
-          properties = list[component].node.body.body;
-        } else {
-          properties = list[component].node.properties;
-        }
-
-        var propertiesName = properties.map(getPropName);
-
-        for (var i = 0, j = propertiesName.length; i < j; i++) {
-          var propA = propertiesName[i];
-
-          for (var k = 0, l = propertiesName.length; k < l; k++) {
-            var propB = propertiesName[k];
-
-            var refIndexA = getRefPropIndex(propA);
-            var refIndexB = getRefPropIndex(propB);
-            var classIndexA = propertiesName.indexOf(propA);
-            var classIndexB = propertiesName.indexOf(propB);
-
-            if (
-              refIndexA === refIndexB ||
-              refIndexA < refIndexB && classIndexA < classIndexB ||
-              refIndexA > refIndexB && classIndexA > classIndexB
-            ) {
-              continue;
-            }
-            storeError({
-              node: properties[i],
-              index: refIndexA
-            }, {
-              node: properties[k],
-              index: refIndexB
-            });
-          }
-        }
+        var properties = getComponentProperties(list[component].node);
+        checkPropsOrder(properties);
       }
 
       reportErrors();

diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js
@@ -140,6 +140,27 @@ eslintTester.addRuleTest('lib/rules/sort-comp', {
       classes: true,
       jsx: true
     }
+  }, {
+    // Must allow a method to be in different places if it's matches multiple patterns
+    code: [
+      'class Hello extends React.Component {',
+      '  render() {',
+      '    return <div>Hello</div>;',
+      '  }',
+      '  onClick() {}',
+      '}'
+    ].join('\n'),
+    args: [1, {
+      order: [
+        '/on.*/',
+        'render',
+        '/.*Click/'
+      ]
+    }],
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    }
   }],
 
   invalid: [{