fix corner cases in optional_chains

[alexlamsl]

No description provided.

[alexlamsl]

@kzc TIL:

$ echo 'console.log([ function(){return this} ][0]())' | node
[ [Function (anonymous)] ]

$ echo 'console.log(([ function(){return this} ][0])())' | node
[ [Function (anonymous)] ]

$ echo 'console.log((0, [ function(){return this} ][0])())' | node
<ref *1> Object [global]

So far so good − parentheses alone won't make a difference.

Now, for the main course:

$ echo 'console.log(null?.f())' | node
undefined

$ echo 'console.log((null?.f)())' | node
[stdin]:1
console.log((null?.f)())
                     ^

TypeError: (intermediate value) is not a function
    at [stdin]:1:22

What is this I don't even.

[kzc]

I think it makes sense. They are different expressions and have different ASTs:

$ echo 'null?.f()' | acorn --module --ecma2021 | egrep -v '"(start|end)"'
{
  "type": "Program",
  "body": [
    {
      "type": "ExpressionStatement",
      "expression": {
        "type": "ChainExpression",
        "expression": {
          "type": "CallExpression",
          "callee": {
            "type": "MemberExpression",
            "object": {
              "type": "Literal",
              "value": null,
              "raw": "null"
            },
            "property": {
              "type": "Identifier",
              "name": "f"
            },
            "computed": false,
            "optional": true
          },
          "arguments": [],
          "optional": false
        }
      }
    }
  ],
  "sourceType": "module"
}

$ echo '(null?.f)()' | acorn --module --ecma2021 | egrep -v '"(start|end)"'
{
  "type": "Program",
  "body": [
    {
      "type": "ExpressionStatement",
      "expression": {
        "type": "CallExpression",
        "callee": {
          "type": "ChainExpression",
          "expression": {
            "type": "MemberExpression",
            "object": {
              "type": "Literal",
              "value": null,
              "raw": "null"
            },
            "property": {
              "type": "Identifier",
              "name": "f"
            },
            "computed": false,
            "optional": true
          }
        },
        "arguments": [],
        "optional": false
      }
    }
  ],
  "sourceType": "module"
}

esbuild shows how they are different:

$ echo 'console.log(null?.f())' | esbuild --target=es6
console.log(void 0);

$ echo 'console.log((null?.f)())' | esbuild --target=es6
console.log((void 0)());

It looks like a bug in uglify-js lib/parse.js:

$ echo 'x?.f(); (x?.g)();' | esbuild --target=es2021
x?.f();
(x?.g)();

$ echo 'x?.f(); (x?.g)();' | uglify-js -b
x?.f();

x?.g();

[kzc]

This confirms it's a parse problem, not an output issue:

$ echo 'x?.f()' | uglify-js -o ast | shasum
a6ce9249ff8d61c47a5c51b1bf28c372859ca016  -

$ echo '(x?.f)()' | uglify-js -o ast | shasum
a6ce9249ff8d61c47a5c51b1bf28c372859ca016  -

[alexlamsl]

I think it makes sense.

Only if you assume moronic specifications − removing support for this "feature" should be the real fix here.

In fact, that's where I will start...

[kzc]

To be honest, I'd expect them to be evaluated differently due to the parentheses - the first form is a conditional call or undefined, and the second is an unconditional call of an expression that may be undefined. The former form is useful, and the latter form is not. That part is self-consistent.

What is not consistent is the call object x in the second form below:

$ echo 'x?.f(123); (x?.g)(123);' | esbuild --target=es6
x == null ? void 0 : x.f(123);
(x == null ? void 0 : x.g).call(x, 123);

In my opinion the parenthesized call expression (x?.g) in (x?.g)(123) should not have been a special case. In a more consistent hypothetical ES specification it ought to have been evaluated to (x == null ? void 0 : x.g).call(0, 123) or simply (x == null ? void 0 : x.g)(123). But because programmers would avoid the generally useless form (x?.g)(123), the questionable call object is not a big concern.

[alexlamsl]

Thanks for the detailed analysis − will tackle this during the next spare cycle.

[kzc]

After thinking about it, I think the motivation for keeping the call object o in (o?.g)(123) can be seen in the examples below:

$ echo '"use strict";var o={a:1,m(x){console.log(this.a,x)}}; o.m(2);' | node
1 2
$ echo '"use strict";var o={a:1,m(x){console.log(this.a,x)}}; (o.m)(2);' | node
1 2
$ echo '"use strict";var o={a:1,m(x){console.log(this.a,x)}}; o?.m(2);' | node
1 2
$ echo '"use strict";var o={a:1,m(x){console.log(this.a,x)}}; (o?.m)(2);' | node
1 2
$ echo '"use strict";var o={a:1,m(x){console.log(this.a,x)}}; (0,o?.m)(2);' | node
[stdin]:1
"use strict";var o={a:1,m(x){console.log(this.a,x)}}; (0,o?.m)(2);
                                              ^
TypeError: Cannot read property 'a' of undefined

(o?.m)(2) is considered to be a member expression call like the other examples above, rather than a call of an arbitrary expression.