From 6a2015feb1636097478c9e18bdb09c604bade28d Mon Sep 17 00:00:00 2001 From: Omer Ganim Date: Tue, 19 Jan 2016 11:27:51 +0200 Subject: [PATCH] performance: replace optimistic getLodashIteratee with pessimistic getLodashMethodVisitor --- lib/rules/matches-prop-shorthand.js | 15 ++++----- lib/rules/matches-shorthand.js | 22 +++++-------- lib/rules/no-unnecessary-bind.js | 5 ++- lib/rules/prefer-compact.js | 6 ++-- lib/rules/prefer-filter.js | 6 ++-- lib/rules/prefer-invoke.js | 6 ++-- lib/rules/prefer-map.js | 6 ++-- lib/rules/prefer-reject.js | 6 ++-- lib/rules/preferred-alias.js | 6 ++-- lib/rules/prop-shorthand.js | 16 ++++----- lib/util/lodashUtil.js | 51 +++++++++++++++++++++-------- 11 files changed, 79 insertions(+), 66 deletions(-) diff --git a/lib/rules/matches-prop-shorthand.js b/lib/rules/matches-prop-shorthand.js index d6d40fa..7235162 100644 --- 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 index 61bc3b0..ac8160c 100644 --- 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 index c5e8cfa..5805a33 100644 --- 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 index 785a8b7..834dfe4 100644 --- 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 index 3df75c7..7560d4a 100644 --- 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 index 99a10b0..75cae6a 100644 --- 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 index e99f75d..27d2c6c 100644 --- 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 index 01ffe19..9938df1 100644 --- 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 index de032da..92953a8 100644 --- 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 index 15fa718..a3eed0c 100644 --- 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 index 696346a..479c3be 100644 --- 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,12 +77,25 @@ 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, @@ -83,5 +103,8 @@ module.exports = { isLodashWrapperMethod: isLodashWrapperMethod, getIsTypeMethod: getIsTypeMethod, isLodashCollectionMethod: isLodashCollectionMethod, - isNativeCollectionMethodCall: isNativeCollectionMethodCall + isNativeCollectionMethodCall: isNativeCollectionMethodCall, + isImplicitChainStart: isImplicitChainStart, + isExplicitChainStart: isExplicitChainStart, + getLodashMethodVisitor: getLodashMethodVisitor };