Refactor, optimize and fix call args expansion

[thorn0]

Description

Checklist

• I’ve added tests to confirm my change works.
• (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
• (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[fisker]

Because this check didn't pass?

Prettier pr-13341
Playground link

--parser babel

Input:

call(function () {
  return 1;
}, $var ||
  ($var ??
  $var ??
  $var ??
  $var ??
  $var ??
  $var ??
  $var ??
  $var ??
  "test"));

call(function () {
  return 1;
}, $var ??
  ($var ??
  $var ??
  $var ??
  $var ??
  $var ??
  $var ??
  $var ??
  $var ??
  "test"));

Output:

call(function () {
  return 1;
}, $var || ($var ?? $var ?? $var ?? $var ?? $var ?? $var ?? $var ?? $var ?? "test"));

call(
  function () {
    return 1;
  },
  $var ??
    $var ??
    $var ??
    $var ??
    $var ??
    $var ??
    $var ??
    $var ??
    $var ??
    "test",
);

[thorn0]

Because this check didn't pass?

Yep. It's a hack, sure, but it does the trick for most cases.
What would you suggest? Should we add && !secondArg?.right?.right?
I decided not to touch #12892 and #4401 for now.

[fisker]

Maybe just check if it's LogicalExpression chain for now? Unless you mean to match other types with left.

[fisker]

secondArg?.right?.right will match a = b = c

[thorn0]

secondArg?.right?.right will match a = b = c

That would be perfectly okay.

Unless you mean to match other types with left.

Yes, I mean to match binary expression that are more complex than this:

setTimeout(() => {
   // ...
}, 40 * rate);

[fisker]

We have full list of those nodes with left, right, you can add more tests for them.

prettier/tests/unit/__snapshots__/visitor-keys.js.snap

Line 3 in 3cae07c

exports[`visitor keys estree 1`] = `

[thorn0]

An Interesting case fixed by this PR. I guess the body of the function was printed in MODE_FLAT.

Prettier pr-13341
Playground link

--parser babel

Input:

fs.readdirSync(suiteLoc).forEach(function (testName) {
  (skip ? it.skip : it)(
    testName,
    buildTest(binName, testName, opts),
    2_000_000,
  );
});

{
  (skip ? it.skip : it)(
    testName,
    buildTest(binName, testName, opts),
    2_000_000,
  );
}

Output:

fs.readdirSync(suiteLoc).forEach(function (testName) {
  (skip ? it.skip : it)(
    testName,
    buildTest(binName, testName, opts),
    2_000_000,
  );
});

{
  (skip ? it.skip : it)(
    testName,
    buildTest(binName, testName, opts),
    2_000_000,
  );
}

Prettier 2.7.1
Playground link

Output:

fs.readdirSync(suiteLoc).forEach(function (testName) {
  (skip
    ? it.skip
    : it)(testName, buildTest(binName, testName, opts), 2_000_000);
});

{
  (skip ? it.skip : it)(
    testName,
    buildTest(binName, testName, opts),
    2_000_000
  );
}

[fisker]

The output looks good 👍 .

Code in this file is a nightmare to understand (not because of this PR, it already is) 😢 .

[thorn0]

In this PR, I tried to make it easier to follow what's going on. At least I untangled first arg expansion from last arg expansion. Now those are separate code paths.

[fisker]

A quick search we have 130+ places of CallExpression, and 100+ places used MemberExpression.
If someone want to implement #10244, he will get big headache.

[thorn0]

It's going to be very similar to adding support for TSNonNullExpression, which Prettier already supports (almost?) everywhere it handles member chains.