Fix closure compiler type casts

[jridgewell]

This fixes casts when they are followed by a closing parenthesis, eg:

foo(/** @type {number} */(num) + 1);
foo(/** @type {!Array} */(arrOrString).length);

The old code would see the CallExpresion's closing ) and assume everything belonged to the typecast up till that closing parenthesis.

This would be easier to accomplish if every AST would tell us if the expression were parenthesized. If they did, we could check that the node were parenthesized and either it or an ancestor has a typecast, stopping when we find an ancestor that is itself parenthesized.

Fixes #4799.

• I’ve added tests to confirm my change works.
• (If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[thorn0]

This would be easier to accomplish if every AST would tell us if the expression were parenthesized.

Babel does this: node.extra.parenthesized.

[jridgewell]

Babel does this: node.extra.parenthesized.

Yup, but prettier seems to use allow using several different AST parsers.

[thorn0]

Yup, prettier seems to use allow using several different AST parsers.

It does, but is this change relevant for TypeScript and Flow? If not, only Babel is left.

[jridgewell]

Are the tests run with babel parser? I tried to do this directly with node.extra.parenthesized originally, but the tests always failed.

[thorn0]

JS tests are usually run with the 3 parsers, e.g. https://github.com/prettier/prettier/blob/master/tests/multiparser_js_html/jsfmt.spec.js

[jridgewell]

I'm not sure how to get the tests to pass when running the flow or typescript parsers, then.

[thorn0]

You can move your test case to a separate directory and make it babel-only by specifying only babel in jsfmt.spec.js.

E.g. you changed tests/comments/closure-compiler-type-cast.js. This test isn't run against TypeScript. See tests/comments/jsfmt.spec.js:

run_spec(__dirname, ["flow", "babel"]);

[jridgewell]

Awesome, thanks for the help! I've updated to take advantage of Babel's node.extra.parenthesized, and it's much nicer.

[rsimha]

@vjeux @j-f1 Is this PR good to merge? And if so, any chance it can be pushed as a new release?

For context, this is the last issue that's preventing the AMP Project from adopting Prettier. See ampproject/amphtml#21212 (comment) for more info.

[vjeux]

@j-f1 could you merge it assuming it's in a good shape?
@duailibe any way you could release 1.17.1 with this fix?

Thanks!

[j-f1]

Thanks for contributing!

[rsimha]

@duailibe Any chance this fix can be pushed as a new release?

[rsimha]

@vjeux @duailibe Bumping this thread to see if this fix can be released anytime soon.

[j-f1]

For now you can install prettier/prettier to get the latest version from GitHub.

[duailibe]

@jridgewell @rsimha

I'll cut a release today or tomorrow! I'm sorry for the delay

[duailibe]

1.17.1 is out

[jridgewell]

Thanks!