tweag / ormolu

A formatter for Haskell source code
https://ormolu-live.tweag.io
Other
958 stars 83 forks source link

idempotency bug (including operators) #522

Closed mheinzel closed 4 years ago

mheinzel commented 4 years ago

I saw https://github.com/tweag/ormolu/issues/451, but there is a related PR that claims to solve the issue.

Minimal example:

foo = f
    . g
  =<< h . i

first turns into

foo =
  f
    . g
    =<< h . i

and then

foo =
  f
    . g
    =<< h
    . i

Version:

$ stack exec -- ormolu --version
Stack has not been tested with GHC versions above 8.6, and using 8.8.2, this ma
y fail
Stack has not been tested with Cabal versions above 2.4, but version 3.0.1.0 wa
s found, this may fail
ormolu 0.0.3.1 master 55d8b7f8c482655ea575425e55352e650f304ea0
using ghc-lib-parser 8.8.2.20200205
utdemir commented 4 years ago

Thanks!

Tricky issue. There's a heuristic trying to figure out the operator precedence, which gets confused when the same operator is used in two different ways. In this case, (.) is both used as the beginning of a line which suggests lower associativity, but then used inline which suggests the opposite. So the heuristic can not decide consistently.

One solution I can think of is to try to decide on the operator precedence just according to the first use of that specific operator instead of averaging all uses. But this might also have another set of corner cases.

I'll spend some time thinking about it.

mrkkrp commented 4 years ago

Another example:

foo n
  | x || y && z || n ** x
      || x && n =
        42
mrkkrp commented 4 years ago

Another example from Idris:

prettyBv env op sc n t v debug =
  text op <> pretty n <+> colon <+> prettySe 10 env t debug <+> text "=" <+>
    prettySe 10 env v debug <> text sc
mrkkrp commented 4 years ago

Yet another example form Idris:

prettyPs all g bnd ((n, _, Let rig t v) : bs) =
  line <> bindingOf n False <+> text "=" <+> tPretty bnd v <+> colon <+>
    align (tPretty bnd t) <> prettyPs all g ((n, False):bnd) bs