diff --git a/packages/ast-utils/src/matchers/index.ts b/packages/ast-utils/src/matchers/index.ts index 5bb6ddfd..1b0b9341 100644 --- a/packages/ast-utils/src/matchers/index.ts +++ b/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 === '===' diff --git a/packages/unminify/src/transformations/__tests__/un-nullish-coalescing.spec.ts b/packages/unminify/src/transformations/__tests__/un-nullish-coalescing.spec.ts index 83f4e2db..aa0cd5f0 100644 --- a/packages/unminify/src/transformations/__tests__/un-nullish-coalescing.spec.ts +++ b/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; +`, +) diff --git a/packages/unminify/src/transformations/__tests__/un-optional-chaining.spec.ts b/packages/unminify/src/transformations/__tests__/un-optional-chaining.spec.ts index 68521691..d78b0a25 100644 --- a/packages/unminify/src/transformations/__tests__/un-optional-chaining.spec.ts +++ b/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; +`, +) diff --git a/packages/unminify/src/transformations/un-conditionals.ts b/packages/unminify/src/transformations/un-conditionals.ts index 2d78fee6..6cb1ce23 100644 --- a/packages/unminify/src/transformations/un-conditionals.ts +++ b/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) diff --git a/packages/unminify/src/transformations/un-nullish-coalescing.ts b/packages/unminify/src/transformations/un-nullish-coalescing.ts index a897ad94..12e41edc 100644 --- a/packages/unminify/src/transformations/un-nullish-coalescing.ts +++ b/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,10 +65,11 @@ export const transformAST: ASTTransformation = (context) => { } function convertOptionalChaining(j: JSCodeshift, path: ASTPath): 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) @@ -64,7 +77,7 @@ function convertOptionalChaining(j: JSCodeshift, path: ASTPath { 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( 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) diff --git a/packages/unminify/src/transformations/un-optional-chaining.ts b/packages/unminify/src/transformations/un-optional-chaining.ts index 75dbc560..a8dff82a 100644 --- a/packages/unminify/src/transformations/un-optional-chaining.ts +++ b/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,9 +62,11 @@ export const transformAST: ASTTransformation = (context) => { } function convertOptionalChaining(j: JSCodeshift, path: ASTPath): 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) @@ -60,7 +74,7 @@ function convertOptionalChaining(j: JSCodeshift, path: ASTPath { const { left: tempVariable, right: originalVariable } = curr @@ -183,6 +199,7 @@ function applyOptionalChaining( 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( 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( 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( 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( 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( 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( 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 } diff --git a/packages/unminify/src/utils/decisionTree.ts b/packages/unminify/src/utils/decisionTree.ts index ee561943..8c8cba5a 100644 --- a/packages/unminify/src/utils/decisionTree.ts +++ b/packages/unminify/src/utils/decisionTree.ts @@ -8,12 +8,12 @@ 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), } } @@ -21,16 +21,20 @@ export function makeDecisionTree(j: JSCodeshift, node: ExpressionKind): Decision 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) +}