performance: replace optimistic getLodashIteratee with pessimistic ge…

…tLodashMethodVisitor
wix-incubator · Jan 19, 2016 · 6a2015f · 6a2015f
1 parent 9b1666d
commit 6a2015f
Show file tree

Hide file tree

Showing 11 changed files with 79 additions and 66 deletions.
diff --git a/lib/rules/matches-prop-shorthand.js b/lib/rules/matches-prop-shorthand.js
@@ -20,26 +20,25 @@ module.exports = function (context) {
         return SUPPORT_MATCHES_PROPERTY_STYLE_CB.indexOf(astUtil.getMethodName(node)) !== -1;
     }
 
-    function usesMatchesPropertySyntax(node) {
-        var iteratee = lodashUtil.getLodashIteratee(node);
+    function usesMatchesPropertySyntax(node, iteratee) {
         return iteratee && iteratee.type === 'Literal' && node.arguments[node.arguments.indexOf(iteratee) + 1];
     }
 
-    var callExpressionVisitors = {
-        always: function (node) {
-            if (methodSupportsShorthand(node) && shouldPreferMatches(lodashUtil.getLodashIteratee(node))) {
+    var callExpressionReporters = {
+        always: function (node, iteratee) {
+            if (methodSupportsShorthand(node) && shouldPreferMatches(iteratee)) {
                 context.report(node.callee.property, 'Prefer matches property syntax');
             }
         },
-        never: function (node) {
-            if (usesMatchesPropertySyntax(node)) {
+        never: function (node, iteratee) {
+            if (usesMatchesPropertySyntax(node, iteratee)) {
                 context.report(node.callee.property, 'Do not use matches property syntax');
             }
         }
     };
 
     return {
-        CallExpression: callExpressionVisitors[context.options[0] || 'always']
+        CallExpression: lodashUtil.getLodashMethodVisitor(callExpressionReporters[context.options[0] || 'always'])
     };
 };
 

diff --git a/lib/rules/matches-shorthand.js b/lib/rules/matches-shorthand.js
@@ -46,27 +46,21 @@ module.exports = function (context) {
         return SUPPORT_MATCHES_STYLE_CB.indexOf(astUtil.getMethodName(node)) !== -1;
     }
 
-    function usesMatchesShorthand(node) {
-        var iteratee = lodashUtil.getLodashIteratee(node);
-        return iteratee && iteratee.type === 'ObjectExpression';
-
-    }
-
-    var callExpressionVisitors = {
-        always: function (node) {
-            if (methodSupportsShorthand(node) && shouldPreferMatches(lodashUtil.getLodashIteratee(node))) {
-                context.report(lodashUtil.getLodashIteratee(node), 'Prefer matches syntax');
+    var callExpressionReporters = {
+        always: function (node, iteratee) {
+            if (methodSupportsShorthand(node) && shouldPreferMatches(iteratee)) {
+                context.report(iteratee, 'Prefer matches syntax');
             }
         },
-        never: function (node) {
-            if (usesMatchesShorthand(node)) {
-                context.report(lodashUtil.getLodashIteratee(node), 'Do not use matches syntax');
+        never: function (node, iteratee) {
+            if (iteratee && iteratee.type === 'ObjectExpression') {
+                context.report(iteratee, 'Do not use matches syntax');
             }
         }
     };
 
     return {
-        CallExpression: callExpressionVisitors[context.options[0] || 'always']
+        CallExpression: lodashUtil.getLodashMethodVisitor(callExpressionReporters[context.options[0] || 'always'])
     };
 };
 

diff --git a/lib/rules/no-unnecessary-bind.js b/lib/rules/no-unnecessary-bind.js
@@ -16,11 +16,10 @@ module.exports = function (context) {
     }
 
     return {
-        CallExpression: function (node) {
-            var iteratee = lodashUtil.getLodashIteratee(node);
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node, iteratee) {
             if (isBound(iteratee)) {
                 context.report(iteratee.callee.property, 'Unnecessary bind, pass `thisArg` to lodash method instead');
             }
-        }
+        })
     };
 };
diff --git a/lib/rules/prefer-compact.js b/lib/rules/prefer-compact.js
@@ -28,10 +28,10 @@ module.exports = function (context) {
     }
 
     return {
-        CallExpression: function (node) {
-            if (lodashUtil.isCallToMethod(node, 'filter') && isBooleanCastingFunction(lodashUtil.getLodashIteratee(node))) {
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node, iteratee) {
+            if (lodashUtil.isCallToMethod(node, 'filter') && isBooleanCastingFunction(iteratee)) {
                 context.report(node, 'Prefer _.compact over filtering of Boolean casting');
             }
-        }
+        })
     };
 };
diff --git a/lib/rules/prefer-filter.js b/lib/rules/prefer-filter.js
@@ -30,11 +30,11 @@ module.exports = function (context) {
     }
 
     return {
-        CallExpression: function (node) {
-            if (lodashUtil.isCallToMethod(node, 'forEach') && onlyHasSimplifiableIf(lodashUtil.getLodashIteratee(node))) {
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node, iteratee) {
+            if (lodashUtil.isCallToMethod(node, 'forEach') && onlyHasSimplifiableIf(iteratee)) {
                 context.report(node, 'Prefer _.filter or _.some over an if statement inside a _.forEach');
             }
-        }
+        })
     };
 };
 

diff --git a/lib/rules/prefer-invoke.js b/lib/rules/prefer-invoke.js
@@ -16,10 +16,10 @@ module.exports = function (context) {
     }
 
     return {
-        CallExpression: function (node) {
-            if (lodashUtil.isCallToMethod(node, 'map') && isFunctionMethodCallOfParam(lodashUtil.getLodashIteratee(node))) {
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node, iteratee) {
+            if (lodashUtil.isCallToMethod(node, 'map') && isFunctionMethodCallOfParam(iteratee)) {
                 context.report(node, 'Prefer _.invoke over map to a method call.');
             }
-        }
+        })
     };
 };
diff --git a/lib/rules/prefer-map.js b/lib/rules/prefer-map.js
@@ -18,10 +18,10 @@ module.exports = function (context) {
     }
 
     return {
-        CallExpression: function (node) {
-            if (lodashUtil.isCallToMethod(node, 'forEach') && onlyHasPush(lodashUtil.getLodashIteratee(node))) {
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node, iteratee) {
+            if (lodashUtil.isCallToMethod(node, 'forEach') && onlyHasPush(iteratee)) {
                 context.report(node, 'Prefer _.map over a _.forEach with a push to an array inside');
             }
-        }
+        })
     };
 };
diff --git a/lib/rules/prefer-reject.js b/lib/rules/prefer-reject.js
@@ -22,11 +22,11 @@ module.exports = function (context) {
     }
 
     return {
-        CallExpression: function (node) {
-            if (lodashUtil.isCallToMethod(node, 'filter') && isNegativeExpressionFunction(lodashUtil.getLodashIteratee(node))) {
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node, iteratee) {
+            if (lodashUtil.isCallToMethod(node, 'filter') && isNegativeExpressionFunction(iteratee)) {
                 context.report(node, 'Prefer _.reject over negative condition');
             }
-        }
+        })
     };
 };
 

diff --git a/lib/rules/preferred-alias.js b/lib/rules/preferred-alias.js
@@ -19,11 +19,11 @@ module.exports = function (context) {
     }, {});
 
     return {
-        CallExpression: function (node) {
+        CallExpression: lodashUtil.getLodashMethodVisitor(function (node) {
             var methodName = astUtil.getMethodName(node);
-            if ((lodashUtil.isLodashCall(node) || lodashUtil.isLodashWrapper(node)) && _.has(aliases, methodName)) {
+            if (_.has(aliases, methodName)) {
                 context.report(node.callee.property, "Method '{{old}}' is an alias, for consistency prefer using '{{new}}'", {old: methodName, new: aliases[methodName]});
             }
-        }
+        })
     };
 };
diff --git a/lib/rules/prop-shorthand.js b/lib/rules/prop-shorthand.js
@@ -22,27 +22,25 @@ module.exports = function (context) {
         return _.includes(aliasMap.supportsProp, astUtil.getMethodName(node));
     }
 
-    function usesPropShorthand(node) {
-        var iteratee = lodashUtil.getLodashIteratee(node);
+    function usesPropShorthand(node, iteratee) {
         return iteratee && iteratee.type === 'Literal' && !node.arguments[node.arguments.indexOf(iteratee) + 1];
     }
 
-    var callExpressionVisitors = {
-        always: function (node) {
-            var iteratee = lodashUtil.getLodashIteratee(node);
+    var callExpressionReporters = {
+        always: function (node, iteratee) {
             if (methodSupportsShorthand(node) && canUseShorthand(iteratee)) {
                 context.report(iteratee, 'Prefer property shorthand syntax');
             }
         },
-        never: function (node) {
-            if (usesPropShorthand(node)) {
-                context.report(lodashUtil.getLodashIteratee(node), 'Do not use property shorthand syntax');
+        never: function (node, iteratee) {
+            if (usesPropShorthand(node, iteratee)) {
+                context.report(iteratee, 'Do not use property shorthand syntax');
             }
         }
     };
 
     return {
-        CallExpression: callExpressionVisitors[context.options[0] || 'always']
+        CallExpression: lodashUtil.getLodashMethodVisitor(callExpressionReporters[context.options[0] || 'always'])
     };
 };
 

diff --git a/lib/util/lodashUtil.js b/lib/util/lodashUtil.js
@@ -11,8 +11,16 @@ function isChainable(node) {
     return _.includes(aliasMap.CHAINABLE_ALIASES, astUtil.getMethodName(node));
 }
 
+function isImplicitChainStart(node) {
+    return node.callee.name === '_';
+}
+
+function isExplicitChainStart(node) {
+    return isLodashCall(node) && astUtil.getMethodName(node) === 'chain';
+}
+
 function isLodashChainStart(node) {
-    return node && node.type === 'CallExpression' && (node.callee.name === '_' || (_.get(node, 'callee.object.name') === '_' && astUtil.getMethodName(node) === 'chain'));
+    return node && node.type === 'CallExpression' && (isImplicitChainStart(node) || isExplicitChainStart(node));
 }
 
 
@@ -29,20 +37,19 @@ function isExplicitMethodChaining(node) {
 }
 
 function isLodashWrapper(node) {
-    if (isLodashChainStart(node)) {
-        return true;
+    var currentNode = node;
+    while (astUtil.isMethodCall(currentNode)) {
+        if (isLodashChainStart(currentNode)) {
+            return true;
+        }
+        if (!isChainable(currentNode)) {
+            return false;
+        }
+        currentNode = astUtil.getCaller(currentNode);
     }
-    return astUtil.isMethodCall(node) && isChainable(node) && isLodashWrapper(node.callee.object);
+    return isLodashChainStart(currentNode);
 }
 
-function getLodashIteratee(node) {
-    if (isLodashCall(node)) {
-        return node.arguments && node.arguments[1];
-    }
-    return isLodashWrapper(astUtil.getCaller(node)) && node.arguments && node.arguments[0];
-}
-
-
 function isEndOfChain(node) {
     return isLodashWrapper(astUtil.getCaller(node)) && !astUtil.isObjectOfMethodCall(node);
 }
@@ -70,18 +77,34 @@ function isLodashCollectionMethod(node) {
     return _.includes(aliasMap.collectionMethods, astUtil.getMethodName(node));
 }
 
+function getLodashMethodVisitor(reporter) {
+    return function (node) {
+        if (isLodashChainStart(node)) {
+            node = node.parent.parent;
+            while (astUtil.isMethodCall(node) && !isChainBreaker(node)) {
+                reporter(node, node.arguments[0]);
+                node = node.parent.parent;
+            }
+        } else if (isLodashCall(node)) {
+            reporter(node, node.arguments[1]);
+        }
+    };
+}
+
 module.exports = {
     isLodashCall: isLodashCall,
     isLodashChainStart: isLodashChainStart,
     isChainable: isChainable,
     isLodashWrapper: isLodashWrapper,
-    getLodashIteratee: getLodashIteratee,
     isEndOfChain: isEndOfChain,
     isChainBreaker: isChainBreaker,
     isExplicitMethodChaining: isExplicitMethodChaining,
     isCallToMethod: isCallToMethod,
     isLodashWrapperMethod: isLodashWrapperMethod,
     getIsTypeMethod: getIsTypeMethod,
     isLodashCollectionMethod: isLodashCollectionMethod,
-    isNativeCollectionMethodCall: isNativeCollectionMethodCall
+    isNativeCollectionMethodCall: isNativeCollectionMethodCall,
+    isImplicitChainStart: isImplicitChainStart,
+    isExplicitChainStart: isExplicitChainStart,
+    getLodashMethodVisitor: getLodashMethodVisitor
 };