Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Emit needed parens for all kinds of Flow types #1127

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Emit needed parens for all kinds of Flow types #1127

wants to merge 4 commits into from

Commits on Jun 11, 2022

  1. parens [nfc]: Clean up slightly in preparation to handle Flow types 

    This just reorganizes a bit for organization, and fixes some comments.
    @gnprice
    gnprice committed Jun 11, 2022
    Configuration menu
    Copy the full SHA
    50c488f View commit details
    Browse the repository at this point in the history

  2. test: Use Flow parser throughout Flow tests 

    At this point there are a number of Flow features that the Esprima
parser doesn't support.  Rather than switch to the Flow parser ad hoc
for those test cases, simplify by just using it systematically.
    @gnprice
    gnprice committed Jun 11, 2022
    Configuration menu
    Copy the full SHA
    1a48ca2 View commit details
    Browse the repository at this point in the history

  3. parens: Fix OptionalIndexedAccessType inside plain IndexedAccessType 

    As the added tests show, we need these parens regardless of whether
`node.optional` is true.  That is, we need them even for a type like
`(Obj?.['foo']['bar'])['baz']`, which is
 * an IndexedAccessType, whose objectType is
 * an OptionalIndexedAccessType with `optional` false,
   whose objectType is
 * an OptionalIndexedAccessType with `optional` true.

The parens stop the OptionalIndexedAccessType nature from
propagating outward in the absence of `?.`.
    @gnprice
    gnprice committed Jun 11, 2022
    Configuration menu
    Copy the full SHA
    8a33061 View commit details
    Browse the repository at this point in the history

  4. parens: Emit parens where needed throughout Flow types; add tests 

    There were a couple of particular cases that were already covered,
but many others that were not.

I ran into one of them in practice (`A & (B | C)` was getting printed
as `A & B | C`, which means something different) and looked into it,
and decided to go about just fixing this whole class of issues.

I started by scanning through all the different kinds of FlowType
node, looking for any that could need parens around them; then I
wrote up a comprehensive list of test cases.

Once that was done, the whole thing turned out to be doable with a
pretty reasonable amount of code!  And given the tests, I'm hopeful
that this logic is pretty much complete.

Some of the added tests are skipped for now, because they involve
function types and those currently have other bugs that cause those
test cases to break.  They start passing when this branch is merged
with benjamn#1089, which fixes those other bugs.
    @gnprice
    gnprice committed Jun 11, 2022
    Configuration menu
    Copy the full SHA
    08b0ed8 View commit details
    Browse the repository at this point in the history