fourmolu icon indicating copy to clipboard operation
fourmolu copied to clipboard

Incorrect tuple formatting using column-limit

Open Raveline opened this issue 1 year ago • 7 comments

Is your bug specific to Fourmolu?

  • [X] Yes, I've verified that Ormolu does not have this bug

Did you try it on the web app?

  • [X] Yes, I've verified that https://fourmolu.github.io can reproduce this bug

Did you search for existing issues?

  • [X] Yes, there are no open or closed issues related to my issue

Describe the bug

Consider the following input:

fun :: (Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer) -> (Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer)
fun (argument0, argument1, argument2, argument3, argument4, argument5, argument6, argument7, argument8, argument9) = (argument0, argument1, argument2, argument3, argument4, argument5, argument6, argument7, argument8, argument9)

A first call to fourmolu with the option column-limit set at 90 will produce:

fun
  :: (Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer)
  -> (Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer)
fun ( argument0
      , argument1
      , argument2
      , argument3
      , argument4
      , argument5
      , argument6
      , argument7
      , argument8
      , argument9
      ) =
  ( argument0
  , argument1
  , argument2
  , argument3
  , argument4
  , argument5
  , argument6
  , argument7
  , argument8
  , argument9
  )

A second call on this result will produce:

fun
  :: (Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer)
  -> (Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer, Integer)
fun
  ( argument0
    , argument1
    , argument2
    , argument3
    , argument4
    , argument5
    , argument6
    , argument7
    , argument8
    , argument9
    ) =
    ( argument0
    , argument1
    , argument2
    , argument3
    , argument4
    , argument5
    , argument6
    , argument7
    , argument8
    , argument9
    )
  • The signatures do not respect the max width as this gives me 95 character lengths lines;
  • This is not idempotent (though it is my understanding that idempotency is not guaranteed using column-limit);
  • Tuples as arguments are formatted differently (and incorrectly) from tuples as results.

I would have expected:

fun
  :: ( Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     )
  -> ( Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     , Integer
     )
fun
  ( argument0
  , argument1
  , argument2
  , argument3
  , argument4
  , argument5
  , argument6
  , argument7
  , argument8
  , argument9
  ) =
  ( argument0
  , argument1
  , argument2
  , argument3
  , argument4
  , argument5
  , argument6
  , argument7
  , argument8
  , argument9
  )

Raveline avatar Feb 26 '24 13:02 Raveline

Thanks for the report! As you mentioned, idempotency isn't guaranteed with this flag. We'd accept a PR, but a fix probably won't be prioritized

cc @CrystalSplitter

brandonchinn178 avatar Feb 27 '24 04:02 brandonchinn178

Yeah, this is (unfortunately) a known big limitation with the column wrap implementation. The way it works is by marking a given spansLayout [srcSpan] as MultiLine if it's beyond the column limit. But, as it turns out, MultiLine isn't actually respected always--specifically, it seems to never force MultiLine on function signatures.

I actually don't know why, but I could try to figure out? It may require an Ormolu change though. It's been a while since I looked at this project, but last time I checked there was still a heavy focus on working with the existing Ormolu implementation.

Idempotency for this is probably beyond the scope that I can fix. It's not an easy issue, and I think it would require a much deeper structural change to how layouts work to guarantee Idempotency with line wraps. But perhaps someone else can figure something out :)

CrystalSplitter avatar Feb 27 '24 07:02 CrystalSplitter

(I would probably rename this issue as "column-limit doesn't wrap function signatures". The tuple issue I think is an unrelated idempotency issue?)

CrystalSplitter avatar Feb 27 '24 07:02 CrystalSplitter

@CrystalSplitter Yes, we want to reduce merge conflicts with Ormolu as much as possible, but if it's worth it, we can weigh the trade-offs.

And yes, let's focus this issue on the idempotency issue. The formatting in the below snippet happens even without setting column-limit. I can resolve that separately

f
    ( a
        , b
        , c
        ) =
        ( a
        , b
        , c
        )

brandonchinn178 avatar Feb 28 '24 03:02 brandonchinn178

Coming back to this much later, I was able to confirm that the full function type annotation span is being correctly identified as a MultiLine Layout. It does also try to run Ormolu.Printer.Internal.newline, and does seem to insert the raw newline, but only at the end of the tuple (breaking on -> with an indent). Perhaps it's not recursively descending into each argument?

CrystalSplitter avatar Mar 18 '24 04:03 CrystalSplitter