[Fix] Add avoid parens config for Printer

[phoenisx]

Fixes #1191, and possibly #1248

[GianlucaGuarini]

This Patch could help us to upgrade recast in our @riotjs/compiler.
@eventualbuddha have you had any chance to check this? How can we help you further?

[techLeadGuy]

This will also fix an issue in jscodeshift, namely facebook/jscodeshift#566. How can we push this PR ?

[eventualbuddha]

A quick read of this makes me concerned that enabling this flag will, in some cases, cause printing of invalid or incorrect code. Unless needsParens doesn't actually mean need in any case?

[techLeadGuy]

@eventualbuddha I think you are right, for example when you go from 1 argument in an arrow fct to no/multiple arguments, in which case you want the final code to have auto-added parens even in the case this flag would be set to true.

I guess a better solution would be to just fix the FPp.needsParens method for the cases where parens can be bypassed (if the input code did not have them in the first place). For the await case inside conditional expression, I think it's because of this line.

Unless I'm missing something, I'll be as easy as removing L451 from the printer. If you agree with this approach, I'm happy to create a PR for it.

[phoenisx]

That's a very good find.
Just curious, apart from arrow-function params are there any other syntax where adding parens is required after a code change?

What I am trying to understand here is why adding parens is a generic behaviour rather than being specific to such special cases.

[techLeadGuy]

@phoenisx Just by looking at recast's printer logic (this line), there's the case where you add type definitions for params that would also need to enforce parens.

const myFunc = name => console.log(name);
// becoming
const myFunc = (name: string) => console.log(name);

My feeling is that, since for the most part, the parens logic is handled appropriately ( keeping existing parens in place, only adding parens when the transformation requires for syntax validity and not adding extra parens when not required ), our specific issues are more exceptions than rules.

[michaelfaith]

My feeling is that, since for the most part, the parens logic is handled appropriately ( keeping existing parens in place, only adding parens when the transformation requires for syntax validity and not adding extra parens when not required ), our specific issues are more exceptions than rules.

Or the logic to detect when parens are required isn't entirely sound. I.e. getting incorrectly triggered by jsx elements: #1248 Reading through the printer code, did you see anything that stood out that might cause that behavior?

[techLeadGuy]

@michaelfaith Unfortunately not. Tried yesterday to fiddle around with recast locally, hoping to find an easy fix for all these parens related issues but there are a lot more corner cases than I originally expected and a lot of tests for them, which is great, but time consuming to properly change. Someone with a better understanding of all these cases could perhaps be a lot more efficient than myself, but as of right now, don't know when I will have the time to try to fix these again.

[topwood]

need this config too，pls fix this soon

[KernelDeimos]

August of last year... oof

edit: and reasonably so, after about 40 minutes I realized there is no good solution to this right now. The parser (babel at least; the default parser is useless to me because it's wayyy behind the spec) is lossy and does not preserve information about the parens unless you pass tokens: true