improve implementation of unwrap rule to not be recursive

wix-incubator · Sep 4, 2016 · 95be25a · 95be25a
1 parent 7c56cca
commit 95be25a
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 35 deletions.
diff --git a/src/rules/unwrap.js b/src/rules/unwrap.js
@@ -8,18 +8,36 @@
 // ------------------------------------------------------------------------------
 
 module.exports = function (context) {
-    const {isExplicitMethodChaining, isChainBreaker, isCallToMethod, isChainable, isEndOfChain} = require('../util/lodashUtil')
+    const {isImplicitChainStart, isExplicitChainStart, isChainable, isCallToMethod, isChainBreaker} = require('../util/lodashUtil')
     const settings = require('../util/settingsUtil').getSettings(context)
-    function isExplicitChainWithoutBreaker(node) {
-        return isExplicitMethodChaining(node, settings.version) && !isChainBreaker(node, settings.version)
+    const {getCaller} = require('../util/astUtil')
+    const negate = require('lodash/negate')
+
+    function isCommit(node) {
+        return isCallToMethod(node, settings.version, 'commit')
     }
-    function isEvaluatedWhenLazy(node) {
-        return isCallToMethod(node, settings.version, 'commit') || !isChainable(node, settings.version)
+
+    function getEndOfChain(node, isExplicit) {
+        const stillInChain = isExplicit ? negate(isChainBreaker) : isChainable
+        let curr = node.parent.parent
+        while (curr === getCaller(curr.parent.parent) && stillInChain(curr, settings.version)) {
+            curr = curr.parent.parent
+        }
+        return curr
     }
+
     return {
         CallExpression(node) {
-            if (isEndOfChain(node, settings.pragma, settings.version) && (!isEvaluatedWhenLazy(node) || isExplicitChainWithoutBreaker(node))) {
-                context.report(node, 'Missing unwrapping at end of chain')
+            if (isImplicitChainStart(node, settings.pragma)) {
+                const end = getEndOfChain(node, false)
+                if (!isCommit(end) && isChainable(end, settings.version)) {
+                    context.report(end, 'Missing unwrapping at end of chain')
+                }
+            } else if (isExplicitChainStart(node, settings.pragma)) {
+                const end = getEndOfChain(node, true)
+                if (!isCommit(end) && !isChainBreaker(end, settings.version)) {
+                    context.report(end, 'Missing unwrapping at end of chain')
+                }
             }
         }
     }

diff --git a/src/util/lodashUtil.js b/src/util/lodashUtil.js
@@ -63,20 +63,6 @@ function isChainBreaker(node, version) {
     return methodDataUtil.isAliasOfMethod(version, 'value', astUtil.getMethodName(node))
 }
 
-/**
- * Returns whether or not the node is part of an explicit chain
- * @param {Object} node
- * @param {number} version
- * @returns {boolean}
- */
-function isExplicitMethodChaining(node, version) {
-    const methodName = astUtil.getMethodName(node)
-    if (methodName === 'chain') {
-        return true
-    }
-    return astUtil.isMethodCall(node) && !isChainBreaker(node, version) && isExplicitMethodChaining(node.callee.object, version)
-}
-
 /**
  * Returns whether the node is in a lodash chain
  * @param {Object} node
@@ -102,17 +88,6 @@ function isLodashWrapper(node, pragma, version) {
     return chainable ? isLodashChainStart(currentNode, pragma) : isExplicitChainStart(currentNode, pragma)
 }
 
-/**
- * Returns whether or not the node is the last call of a Lodash chain
- * @param {Object} node
- * @param {string} pragma
- * @param {number} version
- * @returns {boolean}
- */
-function isEndOfChain(node, pragma, version) {
-    return isLodashWrapper(astUtil.getCaller(node), pragma, version) && !astUtil.isObjectOfMethodCall(node)
-}
-
 /**
  * Returns whether the node is a call to the specified method or one of its aliases in the version
  * @param {Object} node
@@ -240,9 +215,7 @@ module.exports = {
     isLodashChainStart,
     isChainable,
     isLodashWrapper,
-    isEndOfChain,
     isChainBreaker,
-    isExplicitMethodChaining,
     isCallToMethod,
     isLodashCallToMethod,
     isLodashWrapperMethod,

diff --git a/tests/lib/rules/unwrap.js b/tests/lib/rules/unwrap.js
@@ -18,7 +18,10 @@ ruleTester.run('unwrap', rule, {
         'var x = _(a).map(f).reduce(g)',
         'var x = _(a).map(f).filter(g).value()',
         'var x = _.chain(a).map(f).value()',
-        'var stillWrapped = _(a).forEach(f).commit();'
+        'var stillWrapped = _(a).remove(f).commit();',
+        'var stillWrapper = _.chain(a).remove(f).commit();',
+        'var unwrappedEarly = _(a).reduce(f, x).map(g)',
+        'var unwrappedEarly = _.chain(a).map(f).value().map(g)'
     ],
     invalid: [
         'var x = _(a).map(f);',