tweag / ormolu

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

Extraneous empty case (`of {}`) inserted by the formatter #1024

Closed Kleidukos closed 1 year ago

Kleidukos commented 1 year ago

Hi. :)

I stumbled across this interesting diff today:

{-# LANGUAGE CPP #-}

hashFromTUF :: Sec.Hash -> HashValue
hashFromTUF (Sec.Hash hashstr) =
-   case Base16.decode (BS.pack hashstr) of
+   case Base16.decode (BS.pack hashstr) of {}
#if MIN_VERSION_base16_bytestring(1,0,0)
    Right hash -> HashValue hash
    Left _ -> error "hashFromTUF: cannot decode base16"
#else
    (hash, trailing) | not (BS.null hash) && BS.null trailing
      -> HashValue hash
    _ -> error "hashFromTUF: cannot decode base16 hash"
#endif

I can consistently reproduce it in the playground. Right now the only workaround that comes to mind is to blacklist the module. Perhaps is there a better trick?

amesgen commented 1 year ago

Thanks for the report, this is expected behavior: Ormolu can't support CPP in general (see "Limitations"); rather, it splits up the input before and after any occurrence of CPP, formats the non-CPP snippets individually and concats everything in the end.

In your case, this means that Ormolu will format just

{-# LANGUAGE CPP #-}

hashFromTUF :: Sec.Hash -> HashValue
hashFromTUF (Sec.Hash hashstr) =
  case Base16.decode (BS.pack hashstr) of

and then concat the formatted result with the CPP snippet

#if MIN_VERSION_base16_bytestring(1,0,0)
    Right hash -> HashValue hash
    Left _ -> error "hashFromTUF: cannot decode base16"
#else
    (hash, trailing) | not (BS.null hash) && BS.null trailing
      -> HashValue hash
    _ -> error "hashFromTUF: cannot decode base16 hash"
#endif

Now, the first snippet looks like an empty case, so Ormolu formats it as such.

The recommended approach is to insert Ormolu's magic comments at appropriate places; in this case, e.g.

{-# LANGUAGE CPP #-}

hashFromTUF :: Sec.Hash -> HashValue
{- ORMOLU_DISABLE -}
hashFromTUF (Sec.Hash hashstr) =
  case Base16.decode (BS.pack hashstr) of
#if MIN_VERSION_base16_bytestring(1,0,0)
    Right hash -> HashValue hash
    Left _ -> error "hashFromTUF: cannot decode base16"
#else
    (hash, trailing) | not (BS.null hash) && BS.null trailing
      -> HashValue hash
    _ -> error "hashFromTUF: cannot decode base16 hash"
#endif
{- ORMOLU_ENABLE -}

which is a fixed point.

Hope this helps!

Kleidukos commented 1 year ago

Thank you! :)