[breaking] Remove TSParenthesizedType unless createParenthesizedExpressions is enabled

[nicolo-ribaudo]

I don't understand why we generate TSParenthesizedType nodes by default. Example: AST Explorer. It isn't needed (it isn't part of the AST, but more a CST thing) and we don't use it from Flow.

Also, we should add it to Flow when that option is enabled.

Ref: #8025

[smelukov]

I want to try to deal with it

[smelukov]

@nicolo-ribaudo
Seems like it breaks some cases:

Input: let x: [string, number?, (string | number)?]
Output: let x: [string, number?, string | number?]

It makes a different sense

Input: type Element<T> = T extends (infer U)[] ? U : T;
Output: type Element<T> = T extends infer U[] ? U : T;

Parsing error

[smelukov]

Seems like we should create TSParenthesizedType nodes even if createParenthesizedExpressions is disabled because it breaks some cases.

[nicolo-ribaudo]

The second one is like a @babel/generator bug: it should automatically add parentheses when needed, based on the AST structure.

Parentheses are handled by packages/babel-generator/src/node/parentheses.js: as you can see, there are many functions that, given a node and its parent, return true if the node should be parenthesized.

For example, to fix your first case we would need a function like this:

export function TSInferType(node: Object, parent: Object): boolean {
  return (
    t.isTSArrayType(parent) ||
    t.isTSOptionalType(parent)
  );
}

This is already a bug in Babel 7, because a plugin could create a TSArrayType node whose child is a TSInferType node, and it wouldn't be printed correctly. Would you like to fix it in a separate PR, so that we can release it in Babel 7?

On the other hand, I cannot reproduce the first case.
Here I wrote a small plugin which replaces TSParenthesizedType nodes with their children, as if they weren't generated by the parser in the first place. As you can see, it reproduces the second case but not the first one.

Maybe the AST generated by the parser is wrong? You need to manually check if it is correct, and if it isn't it's probably caused by some changes in the parser logic while disabling the TSParenthesizedType node.

I'd be happy to look at a "work in progress" PR if you need help.

[smelukov]

I'd be happy to look at a "work in progress" PR if you need help.

@nicolo-ribaudo #11200 look at the failed tests

[smelukov]

I think that I will fix babel 7 in a separated PR

[nicolo-ribaudo]

Thanks! We will do like this:

• Merge the Babel 7 PR to master
• I will merge master into next-8-dev
  -You can then rebase your PR on the new next-8-dev

[smelukov]

This is already a bug in Babel 7

It there an issue with a broken example for babel 7?

[smelukov]

Now I can't reproduce the second case...
I suggest fix it for babel 8 for now

[nicolo-ribaudo]

I will write a reproduction for Babel 7 in ~1 hour

[smelukov]

@nicolo-ribaudo ☺️

[nicolo-ribaudo]

Sorry for this super long hour! 😛

You can add this test to babel-generator/test/index.js (it has a section for TypeScript)

    it("wraps around infer inside an array type", () => {
      const type = t.tsArrayType(
        t.tsInferType(t.tsTypeParameter(null, null, "T")),
      );

      const output = generate(type).code;
      expect(output).toBe("(infer T)[]");
    });

[smelukov]

#11227