un-optional-chaining bug

[dzqdzq]

Describe the bug

Input code

var t,
    n,
    o,
    a =
      null !==
        (t =
          null == r ||
          null === (n = r.app_info) ||
          void 0 === n ||
          null === (o = n.base_info) ||
          void 0 === o
            ? void 0
            : o.app_name) && void 0 !== t
        ? t
        : "game";

Reproduction

No response

Steps to reproduce

Expected behavior

const a = r?.app_info?.base_info?.app_name ?? "game";

Actual behavior

const a = null == r || r.app_info?.base_info?.app_name ?? "game";

[pionxzh]

This was caused by some race condition between un-optional-chaining and un-nullish-coalescing. Not sure will I have time to check and fix it tbh. I will give you an ETA of 2-3 weeks.

[dzqdzq]

This was caused by some race condition between un-optional-chaining and un-nullish-coalescing. Not sure will I have time to check and fix it tbh. I will give you an ETA of 2-3 weeks.
Thanks for your reply! I have tried to fix this problem with claude sonnet 3.7 and asked many times, but it didn't solve it! I think only you can fix this problem. Do you have Alipay App? I will buy you a cup of coffee!

[0xdevalias]

This is obviously far from an ideal / minimal solution, but I wanted an excuse to try GitHub Copilot Agent Mode; and after a bunch of iterating, this was a solution it came up with that seemed to resolve this test case and seemingly not break other things:

• fix missing optional chaining on root object in nullish coalescing expression #143

It did so by re-implementing new utils rather than improving the existing ones; and since I don't even begin to actually understand whats going on in this code at the moment, I wouldn't really know how to translate that into a minimal fix.

But maybe it'll help someone who does understand things better figure it out?

[0xdevalias]

While I don't think this is a complete/ideal solution, I gave ChatGPT's Codex Agent a run at solving this bug yesterday, and it seems to have come up with a smaller patch proposal.

The first time I ran the agent on my fork, which was severely outdated, so while it seemed to identify what might be a core fix, it couldn't actually run the tests to finish things properly:

• Fix nullish optional chaining with loose equality 0xdevalias/wakaru#1

I then properly synced/updated my fork, and then let the agent have another run at fixing it, which came up with a similar fix proposal to some of the AST helpers, but also needed to add some extra changes within the un-optional-chaining logic:

• Handle loose null checks in optional chaining 0xdevalias/wakaru#3

That latter PR also includes some deeper explanations of the proposed patch from both Claude and Gemini:

• Claude: Handle loose null checks in optional chaining 0xdevalias/wakaru#3 (comment)
• Gemini: Handle loose null checks in optional chaining 0xdevalias/wakaru#3 (comment)

I still don't think that PR as currently proposed is exactly correct, the un-optional-chaining changes feel a bit too specific/weird based on the flow of the rest of that function; but I figured it might at the very least give people a better starting point for thinking about how best to solve this properly.

The root cause of the bug seems to be that currently the isNotNullBinary / isNullBinary / isUndefinedBinary AST helpers are only checking for strict equality (===, !==), whereas the case in the example code that doesn't unminify properly is using a loose equality check (==, !=); specifically the null == r || part:

- Expected
+ Received

  var a =
-   r?.app_info?.base_info?.app_name ?? "game";
+   null == r || r.app_info?.base_info?.app_name ?? "game";