brittany cannot parse parentheses #219

New Issue

2019-02-08T15:15:01+01:00

Atry commented

2019-02-08 15:15:01 +01:00

(Migrated from github.com)

module Foo where

(f >=> g) k = f k >>= g

$ brittany Foo.hs
ERROR: brittany pretty printer returned syntactically invalid result.

``` haskell module Foo where (f >=> g) k = f k >>= g ``` ``` $ brittany Foo.hs ERROR: brittany pretty printer returned syntactically invalid result. ```

matt-noonan commented

2019-02-11 15:53:01 +01:00

(Migrated from github.com)

There seems to be a couple of presumably related bugs in this area:

$ cat test.hs
(f x y) z = z
(x `f` y) z = z
(f >=> g) k = f k >>= g

$ brittany --output-on-errors test.hs
f x y )z =
  z
x f y z = z
f >=> g k = f k >>= g

There seems to be a couple of presumably related bugs in this area: ``` $ cat test.hs (f x y) z = z (x `f` y) z = z (f >=> g) k = f k >>= g $ brittany --output-on-errors test.hs f x y )z = z x f y z = z f >=> g k = f k >>= g ```

tfausak commented

2019-06-18 03:46:22 +02:00

(Migrated from github.com)

For this case specifically:

(x `f` y) z = z

There have been many issues related to infix functions with backticks. Most recently #234.

For this case specifically: > ``(x `f` y) z = z`` There have been many issues related to infix functions with backticks. Most recently #234.

eborden commented

2019-06-25 07:10:22 +02:00

(Migrated from github.com)

I did a bit of digging here. It looks like the parenthesis in these LHS binds are not represented in the AST.

I dumped the full AST for (f >=> g) k = f k >>= g, the bit to focus on is the annotations of Match. ghc-exactprint is representing AnnOpenP and AnnCloseP in its KeywordId annotations. This type of parenthesized bind doesn't end up in the normal AST via ParPat. To fix this brittany will have to change its pattern layouter to take ghc-exactprint annotations into account.

Just (Ann (DP (0,0)) [] [] [((G AnnOpenP),DP (0,0)),((G AnnCloseP),DP (0,0)),((G AnnEqual),DP (0,1))] Nothing Nothing)

The full dumped AST is here:

$ echo "(f >=> g) k = f k >>= g" | cabal v2-exec brittany -- --dump-ast-full

A Just (Ann (DP (0,0)) [] [] [((G AnnEofPos),DP (1,0))] Nothing Nothing)
  HsModule
    Nothing
    Nothing
    []
    [ A Just (Ann (DP (0,0)) [] [] [] Nothing Nothing)
        ValD
          NoExt
          FunBind
            NoExt
            A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
              Unqual {OccName: >=>}
            MG
              NoExt
              A Nothing
                [ A Just (Ann (DP (0,0)) [] [] [((G AnnOpenP),DP (0,0)),((G AnnCloseP),DP (0,0)),((G AnnEqual),DP (0,1))] Nothing Nothing)
                    Match
                      NoExt
                      FunRhs
                        A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                          Unqual {OccName: >=>}
                        Infix
                        NoSrcStrict
                      [ A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                          VarPat NoExt (A (Nothing) (Unqual {OccName: f}))
                      , A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                          VarPat NoExt (A (Nothing) (Unqual {OccName: g}))
                      , A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                          VarPat NoExt (A (Nothing) (Unqual {OccName: k}))
                      ]
                      GRHSs
                        NoExt
                        [ A Just (Ann (DP (0,-1)) [] [] [] Nothing Nothing)
                            GRHS
                              NoExt
                              []
                              A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing)
                                OpApp
                                  NoExt
                                  A Just (Ann (DP (0,0)) [] [] [] Nothing Nothing)
                                    HsApp
                                      NoExt
                                      A Just (Ann (DP (0,0)) [] [] [] Nothing Nothing)
                                        HsVar
                                          NoExt
                                          A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                                            Unqual {OccName: f}
                                      A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing)
                                        HsVar
                                          NoExt
                                          A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                                            Unqual {OccName: k}
                                  A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing)
                                    HsVar
                                      NoExt
                                      A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                                        Unqual {OccName: >>=}
                                  A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing)
                                    HsVar
                                      NoExt
                                      A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
                                        Unqual {OccName: g}
                        ]
                        A (Nothing) (EmptyLocalBinds NoExt)
                ]
              FromSource
            WpHole
            []
    ]
    Nothing
    Nothing

I did a bit of digging here. It looks like the parenthesis in these LHS binds are not represented in the AST. I dumped the full AST for `(f >=> g) k = f k >>= g`, the bit to focus on is the annotations of `Match`. `ghc-exactprint` is representing `AnnOpenP` and `AnnCloseP` in its `KeywordId` annotations. This type of parenthesized bind doesn't end up in the normal AST via `ParPat`. To fix this `brittany` will have to change its pattern layouter to take `ghc-exactprint` annotations into account. ```hs Just (Ann (DP (0,0)) [] [] [((G AnnOpenP),DP (0,0)),((G AnnCloseP),DP (0,0)),((G AnnEqual),DP (0,1))] Nothing Nothing) ``` The full dumped AST is here: ``` $ echo "(f >=> g) k = f k >>= g" | cabal v2-exec brittany -- --dump-ast-full ``` ```hs A Just (Ann (DP (0,0)) [] [] [((G AnnEofPos),DP (1,0))] Nothing Nothing) HsModule Nothing Nothing [] [ A Just (Ann (DP (0,0)) [] [] [] Nothing Nothing) ValD NoExt FunBind NoExt A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) Unqual {OccName: >=>} MG NoExt A Nothing [ A Just (Ann (DP (0,0)) [] [] [((G AnnOpenP),DP (0,0)),((G AnnCloseP),DP (0,0)),((G AnnEqual),DP (0,1))] Nothing Nothing) Match NoExt FunRhs A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) Unqual {OccName: >=>} Infix NoSrcStrict [ A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) VarPat NoExt (A (Nothing) (Unqual {OccName: f})) , A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) VarPat NoExt (A (Nothing) (Unqual {OccName: g})) , A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) VarPat NoExt (A (Nothing) (Unqual {OccName: k})) ] GRHSs NoExt [ A Just (Ann (DP (0,-1)) [] [] [] Nothing Nothing) GRHS NoExt [] A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing) OpApp NoExt A Just (Ann (DP (0,0)) [] [] [] Nothing Nothing) HsApp NoExt A Just (Ann (DP (0,0)) [] [] [] Nothing Nothing) HsVar NoExt A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) Unqual {OccName: f} A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing) HsVar NoExt A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) Unqual {OccName: k} A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing) HsVar NoExt A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) Unqual {OccName: >>=} A Just (Ann (DP (0,1)) [] [] [] Nothing Nothing) HsVar NoExt A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing) Unqual {OccName: g} ] A (Nothing) (EmptyLocalBinds NoExt) ] FromSource WpHole [] ] Nothing Nothing ```

lspitzner commented

2019-06-28 21:35:26 +02:00

(Migrated from github.com)

This code from type signature layouting seems mildly related. hasAnnKeyword ltype AnnOpenP might come handy here too.

Although this is all not very well tested (Nesting? Can you nest parentheses?):

988d5b4353/src/Language/Haskell/Brittany/Internal/Layouters/Type.hs (L619-L667)

This code from type signature layouting seems mildly related. `hasAnnKeyword ltype AnnOpenP` might come handy here too. Although this is all not very well tested (Nesting? _Can_ you nest parentheses?): https://github.com/lspitzner/brittany/blob/988d5b435390f2391583b09e79304814c35dfd2b/src/Language/Haskell/Brittany/Internal/Layouters/Type.hs#L619-L667

lspitzner commented

2019-06-30 21:52:38 +02:00

(Migrated from github.com)

The following two lines have the same syntax-tree and the exact same annotations:

(((f >=> g) k) a) b = f k >>= g a b

(( f >=> g) k  a) b = f k >>= g a b

So I am mildly certain that exactprint does not give us enough information to retain redundant parentheses. This is a strong indicator that we should just "normalize" LHS parentheses (dropping any redundant ones): Ignore any annotations, and do the minimum to make the output syntactically valid.

This should also be the easiest approach to implement :p

The following two lines have the same syntax-tree and the exact same annotations: ~~~~.hs (((f >=> g) k) a) b = f k >>= g a b ~~~~ ~~~~.hs (( f >=> g) k a) b = f k >>= g a b ~~~~ So I am mildly certain that exactprint does not give us enough information to retain redundant parentheses. This is a strong indicator that we should just "normalize" LHS parentheses (dropping any redundant ones): Ignore any annotations, and do the minimum to make the output syntactically valid. This should also be the easiest approach to implement :p

👍 1

eborden commented

2019-07-01 04:58:32 +02:00

(Migrated from github.com)

Agreed on normalizing. 👍

Agreed on normalizing. :+1:

Sign in to join this conversation.

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#219