fix(unminify): fix edge cases of un-optional-chaining and `un-nulli…

…sh-coalescing`

packages/ast-utils/src/matchers/index.ts

@@ -95,12 +95,18 @@ export function isLogicalNot(j: JSCodeshift, node: ASTNode): node is UnaryExpres
     return j.UnaryExpression.check(node) && node.operator === '!'
 }
 
+/**
+ * Check if node is `value !== null` or `null !== value`
+ */
 export function isNotNullBinary(j: JSCodeshift, node: ASTNode): node is BinaryExpression {
     return j.BinaryExpression.check(node)
     && node.operator === '!=='
     && (isNull(j, node.left) || isNull(j, node.right))
 }
 
+/**
+ * Check if node is `value === null` or `null === value`
+ */
 export function isNullBinary(j: JSCodeshift, node: ASTNode): node is BinaryExpression {
     return j.BinaryExpression.check(node)
     && node.operator === '==='

packages/unminify/src/transformations/__tests__/un-nullish-coalescing.spec.ts

@@ -1,5 +1,6 @@
 import { defineInlineTest } from '@wakaru/test-utils'
 import transform from '../un-nullish-coalescing'
+import unOptionalChaining from '../un-optional-chaining'
 import unParameters from '../un-parameters'
 
 const inlineTest = defineInlineTest(transform)
@@ -275,3 +276,23 @@ function bar(bar, qux = bar ?? "qux") {}
 function foo3(foo, bar = foo ?? "bar") {}
 `,
 )
+
+inlineTest('falsy fallback value should be accepted',
+  `
+null !== (e = m.foo) && void 0 !== e ? e : void 0;
+null !== (e = l.foo.bar) && void 0 !== e && e;
+`,
+  `
+m.foo ?? void 0;
+l.foo.bar ?? false;
+`,
+)
+
+defineInlineTest([unOptionalChaining, transform])('works with optional chaining',
+  `
+null !== (o = null === (s = c.foo.bar) || void 0 === s ? void 0 : s.baz.z) && void 0 !== o && o;
+`,
+  `
+c.foo.bar?.baz.z ?? false;
+`,
+)

packages/unminify/src/transformations/__tests__/un-optional-chaining.spec.ts

@@ -781,3 +781,12 @@ foo?.bar;
 a?.b?.c;
 `,
 )
+
+inlineTest('works in nested structures',
+  `
+null !== (o = null === (s = c.foo.bar) || void 0 === s ? void 0 : s.baz.z) && void 0 !== o && o;
+`,
+  `
+null !== (o = c.foo.bar?.baz.z) && void 0 !== o && o;
+`,
+)

packages/unminify/src/transformations/un-conditionals.ts

@@ -107,7 +107,7 @@ export const transformAST: ASTTransformation = (context) => {
         })
         .forEach((path) => {
             const conditionExpression = path.node.expression as ConditionalExpression
-            const decisionTree = makeDecisionTree(j, conditionExpression)
+            const decisionTree = makeDecisionTree(j, conditionExpression, false)
             if (!shouldTransform(j, decisionTree)) return
 
             const replacements = renderDecisionTree(j, decisionTree)
@@ -140,7 +140,7 @@ export const transformAST: ASTTransformation = (context) => {
         })
         .forEach((path) => {
             const conditionExpression = path.node.argument as ConditionalExpression
-            const decisionTree = makeDecisionTree(j, conditionExpression)
+            const decisionTree = makeDecisionTree(j, conditionExpression, true)
             if (!shouldTransform(j, decisionTree)) return
 
             const replacements = renderDecisionTreeWithReturn(j, decisionTree)
@@ -158,7 +158,7 @@ export const transformAST: ASTTransformation = (context) => {
         })
         .forEach((path) => {
             const logicalExpression = path.node.expression as LogicalExpression
-            const decisionTree = makeDecisionTree(j, logicalExpression)
+            const decisionTree = makeDecisionTree(j, logicalExpression, false)
             if (!shouldTransform(j, decisionTree)) return
 
             const replacements = renderDecisionTree(j, decisionTree)

packages/unminify/src/transformations/un-nullish-coalescing.ts

@@ -10,6 +10,18 @@ import type { ASTTransformation } from '@wakaru/shared/rule'
 import type { ExpressionKind } from 'ast-types/lib/gen/kinds'
 import type { ASTPath, ConditionalExpression, JSCodeshift, LogicalExpression } from 'jscodeshift'
 
+/**
+ * Indicates whether should the transformation be applied.
+ *
+ * We use a dirty global variable to prevent the rule from
+ * transforming result that doesn't actually have optional chaining.
+ *
+ * This is to prevent the infinite loop and incorrect transformation
+ * since translate decision tree back to the original expression
+ * may not be perfect.
+ */
+let transformed = false
+
 /**
  * Restore nullish coalescing syntax.
  *
@@ -53,18 +65,19 @@ export const transformAST: ASTTransformation = (context) => {
 }
 
 function convertOptionalChaining(j: JSCodeshift, path: ASTPath<ConditionalExpression | LogicalExpression>): ExpressionKind | null {
-    // console.log('\n\n>>>', `${picocolors.green(j(expression).toSource())}`)
+    transformed = false
 
     const expression = path.node
-    const _decisionTree = makeDecisionTreeWithConditionSplitting(j, makeDecisionTree(j, expression))
+    // console.log('\n\n>>>', `${picocolors.green(j(expression).toSource())}`)
+    const _decisionTree = makeDecisionTreeWithConditionSplitting(j, makeDecisionTree(j, expression, true))
     const shouldNegate = isNotNullBinary(j, _decisionTree.condition)
     const decisionTree = shouldNegate
         ? negateDecisionTree(j, _decisionTree)
         : _decisionTree
     // renderDebugDecisionTree(j, decisionTree)
 
     const result = constructNullishCoalescing(j, path, decisionTree, 0, shouldNegate)
-    if (result) {
+    if (transformed && result) {
         mergeComments(result, expression.comments)
         // console.log('<<<', `${picocolors.cyan(j(result).toSource())}`)
     }
@@ -89,12 +102,14 @@ function constructNullishCoalescing(
         if (!isFalsyBranch(j, trueBranch)) return null
 
         if (isNullBinary(j, condition)) {
-            const { left, right: _ } = condition
+            const { left, right } = condition
+            const nonNullExpr = j.NullLiteral.check(left) ? right : left
+
             const cond = constructNullishCoalescing(j, path, falseBranch, 1, isNegated)
             if (!cond) return null
-            if (j.AssignmentExpression.check(left) && j.Identifier.check(left.left)) {
-                const nestedAssignment = j(left).find(j.AssignmentExpression, { left: { type: 'Identifier' } }).nodes()
-                const allAssignment = [left, ...nestedAssignment]
+            if (j.AssignmentExpression.check(nonNullExpr) && j.Identifier.check(nonNullExpr.left)) {
+                const nestedAssignment = j(nonNullExpr).find(j.AssignmentExpression, { left: { type: 'Identifier' } }).nodes()
+                const allAssignment = [nonNullExpr, ...nestedAssignment]
                 const result = allAssignment.reduce((acc, curr) => {
                     const { left: tempVariable, right: originalVariable } = curr
                     return variableReplacing(j, acc, tempVariable as ExpressionKind, originalVariable)
@@ -110,7 +125,7 @@ function constructNullishCoalescing(
                 return result
             }
             else {
-                return variableReplacing(j, cond, left)
+                return variableReplacing(j, cond, nonNullExpr)
             }
         }
     }
@@ -125,6 +140,7 @@ function constructNullishCoalescing(
                         isNegated ? negateCondition(j, falseBranch.condition) : falseBranch.condition,
                         isNegated ? negateCondition(j, trueBranch.condition) : trueBranch.condition,
                     )
+                    transformed = true
                     return nullishCoalescing
                 }
                 return null
@@ -144,6 +160,9 @@ function variableReplacing<T extends ExpressionKind>(
     targetExpression?: ExpressionKind,
 ): T {
     // console.log('variableReplacing', node.type, j(node).toSource(), '|', tempVariable ? j(tempVariable).toSource() : null, '|', targetExpression ? j(targetExpression).toSource() : null)
+    if (j.BooleanLiteral.check(node)) {
+        return node
+    }
 
     if (j.LogicalExpression.check(node)) {
         node.left = variableReplacing(j, node.left, tempVariable, targetExpression)

packages/unminify/src/transformations/un-optional-chaining.ts

@@ -10,6 +10,18 @@ import type { ASTTransformation } from '@wakaru/shared/rule'
 import type { ExpressionKind } from 'ast-types/lib/gen/kinds'
 import type { ASTPath, ConditionalExpression, Identifier, JSCodeshift, LogicalExpression, MemberExpression, SequenceExpression, SpreadElement } from 'jscodeshift'
 
+/**
+ * Indicates whether should the transformation be applied.
+ *
+ * We use a dirty global variable to prevent the rule from
+ * transforming result that doesn't actually have optional chaining.
+ *
+ * This is to prevent the infinite loop and incorrect transformation
+ * since translate decision tree back to the original expression
+ * may not be perfect.
+ */
+let transformed = false
+
 /**
  * Restore optional chaining syntax.
  *
@@ -50,17 +62,19 @@ export const transformAST: ASTTransformation = (context) => {
 }
 
 function convertOptionalChaining(j: JSCodeshift, path: ASTPath<ConditionalExpression | LogicalExpression>): ExpressionKind | null {
+    transformed = false
+
     const expression = path.node
     // console.log('\n\n>>>', `${picocolors.green(j(expression).toSource())}`)
-    const _decisionTree = makeDecisionTreeWithConditionSplitting(j, makeDecisionTree(j, expression))
+    const _decisionTree = makeDecisionTreeWithConditionSplitting(j, makeDecisionTree(j, expression, false))
     const isNotNull = isNotNullBinary(j, _decisionTree.condition)
     const decisionTree = isNotNull
         ? negateDecisionTree(j, _decisionTree)
         : _decisionTree
     // renderDebugDecisionTree(j, decisionTree)
 
     const _result = constructOptionalChaining(j, path, decisionTree, 0)
-    if (!_result) return null
+    if (!transformed || !_result || path.node === _result) return null
 
     const result = isNotNull ? negateCondition(j, _result) : _result
     // console.log('<<<', `${picocolors.cyan(j(result).toSource())}`)
@@ -76,7 +90,6 @@ function constructOptionalChaining(
     flag: 0 | 1,
 ): ExpressionKind | null {
     const { condition, trueBranch, falseBranch } = tree
-
     const deepestFalseBranch = getDeepestFalseBranch(tree)
     /**
      * if the deepest node is `delete` operator, we should accept true as the
@@ -116,12 +129,15 @@ function constructOptionalChaining(
         }
 
         if (isNullBinary(j, condition)) {
-            const { left, right: _ } = condition
+            const { left, right } = condition
+            const nonNullExpr = j.NullLiteral.check(left) ? right : left
+
             const cond = constructOptionalChaining(j, path, falseBranch, 1)
             if (!cond) return null
-            if (j.AssignmentExpression.check(left) && j.Identifier.check(left.left)) {
-                const nestedAssignment = j(left).find(j.AssignmentExpression, { left: { type: 'Identifier' } }).nodes()
-                const allAssignment = [left, ...nestedAssignment]
+
+            if (j.AssignmentExpression.check(nonNullExpr) && j.Identifier.check(nonNullExpr.left)) {
+                const nestedAssignment = j(nonNullExpr).find(j.AssignmentExpression, { left: { type: 'Identifier' } }).nodes()
+                const allAssignment = [nonNullExpr, ...nestedAssignment]
                 const result = allAssignment.reduce((acc, curr) => {
                     const { left: tempVariable, right: originalVariable } = curr
 
@@ -183,6 +199,7 @@ function applyOptionalChaining<T extends ExpressionKind>(
             const object = targetExpression
                 ? smartParenthesized(j, targetExpression)
                 : node.object
+            transformed = true
             return j.optionalMemberExpression(object, node.property, node.computed) as T
         }
 
@@ -207,6 +224,7 @@ function applyOptionalChaining<T extends ExpressionKind>(
                     optionalCallExpression.arguments = optionalCallExpression.arguments.map((arg) => {
                         return j.SpreadElement.check(arg) ? arg : applyOptionalChaining(j, arg, tempVariable, targetExpression)
                     })
+                    transformed = true
                     return optionalCallExpression as T
                 }
 
@@ -223,6 +241,7 @@ function applyOptionalChaining<T extends ExpressionKind>(
                     optionalCallExpression.arguments = optionalCallExpression.arguments.map((arg) => {
                         return j.SpreadElement.check(arg) ? arg : applyOptionalChaining(j, arg, tempVariable, targetExpression)
                     })
+                    transformed = true
                     return optionalCallExpression as T
                 }
 
@@ -235,6 +254,7 @@ function applyOptionalChaining<T extends ExpressionKind>(
                     const memberExpression = isOptional
                         ? j.optionalMemberExpression(calleeObj.object, calleeObj.property, calleeObj.computed)
                         : j.memberExpression(calleeObj.object, calleeObj.property, calleeObj.computed)
+                    if (isOptional) transformed = true
                     memberExpression.object = applyOptionalChaining(j, memberExpression.object, tempVariable, targetExpression)
                     memberExpression.property = applyOptionalChaining(j, memberExpression.property, tempVariable, targetExpression)
                     return memberExpression as T
@@ -249,6 +269,7 @@ function applyOptionalChaining<T extends ExpressionKind>(
                         optionalCallExpression.arguments = optionalCallExpression.arguments.map((arg) => {
                             return j.SpreadElement.check(arg) ? arg : applyOptionalChaining(j, arg, tempVariable, targetExpression)
                         }).splice(1)
+                        transformed = true
                         return optionalCallExpression as T
                     }
                     else if (node.callee.property.name === 'apply') {
@@ -263,6 +284,7 @@ function applyOptionalChaining<T extends ExpressionKind>(
                         optionalCallExpression.arguments = optionalCallExpression.arguments.map((arg) => {
                             return j.SpreadElement.check(arg) ? arg : applyOptionalChaining(j, arg, tempVariable, targetExpression)
                         })
+                        transformed = true
                         return optionalCallExpression as T
                     }
                 }
@@ -285,10 +307,12 @@ function applyOptionalChaining<T extends ExpressionKind>(
             optionalCallExpression.arguments = optionalCallExpression.arguments.map((arg) => {
                 return j.SpreadElement.check(arg) ? arg : applyOptionalChaining(j, arg, tempVariable, targetExpression)
             })
+            transformed = true
             return optionalCallExpression as T
         }
 
         if (areNodesEqual(j, node.callee, tempVariable)) {
+            transformed = true
             const target = targetExpression || node.callee
             return j.optionalCallExpression(target, node.arguments) as T
         }

packages/unminify/src/utils/decisionTree.ts

@@ -8,29 +8,33 @@ export interface DecisionTree {
     falseBranch: DecisionTree | null
 }
 
-export function makeDecisionTree(j: JSCodeshift, node: ExpressionKind): DecisionTree {
+export function makeDecisionTree(j: JSCodeshift, node: ExpressionKind, requireReturnValue: boolean): DecisionTree {
     if (j.ConditionalExpression.check(node)) {
         return {
             condition: node.test,
-            trueBranch: makeDecisionTree(j, node.consequent),
-            falseBranch: makeDecisionTree(j, node.alternate),
+            trueBranch: makeDecisionTree(j, node.consequent, requireReturnValue),
+            falseBranch: makeDecisionTree(j, node.alternate, requireReturnValue),
         }
     }
 
     if (j.LogicalExpression.check(node)) {
         if (node.operator === '&&') {
             return {
                 condition: node.left,
-                trueBranch: makeDecisionTree(j, node.right),
-                falseBranch: null,
+                trueBranch: makeDecisionTree(j, node.right, requireReturnValue),
+                falseBranch: requireReturnValue && isNodeReturningBoolean(j, node.left)
+                    ? { condition: j.booleanLiteral(false), trueBranch: null, falseBranch: null }
+                    : null,
             }
         }
 
         if (node.operator === '||') {
             return {
                 condition: node.left,
-                trueBranch: null,
-                falseBranch: makeDecisionTree(j, node.right),
+                trueBranch: requireReturnValue && isNodeReturningBoolean(j, node.left)
+                    ? { condition: j.booleanLiteral(true), trueBranch: null, falseBranch: null }
+                    : null,
+                falseBranch: makeDecisionTree(j, node.right, requireReturnValue),
             }
         }
 
@@ -109,3 +113,18 @@ export function renderDebugDecisionTree(j: JSCodeshift, tree: DecisionTree) {
     // eslint-disable-next-line no-console
     console.log(output)
 }
+
+function isNodeReturningBoolean(j: JSCodeshift, node: ExpressionKind): boolean {
+    if (j.BooleanLiteral.check(node)) return true
+
+    if (j.LogicalExpression.check(node) && (node.operator === '&&' || node.operator === '||')) {
+        return isNodeReturningBoolean(j, node.left) && isNodeReturningBoolean(j, node.right)
+    }
+
+    return isComparison(j, node)
+}
+
+const comparisonOperators = ['==', '===', '!=', '!==', '>', '>=', '<', '<=']
+function isComparison(j: JSCodeshift, node: ExpressionKind): boolean {
+    return j.BinaryExpression.check(node) && comparisonOperators.includes(node.operator)
+}