brittany cannot parse parentheses

[Atry]

module Foo where

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

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

[matt-noonan]

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]

For this case specifically:

(x `f` y) z = z

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

[eborden]

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

[lspitzner]

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]

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

[eborden]

Agreed on normalizing. :+1: