Sorting language pragmas should be more intelligent

[mrkkrp]

This was mentioned in #216, but it is a separate issue.

The problem is that a list of language extensions is not always a list of commutative elements, however it often is a list of commutative elements and users will likely expect the alphabetized behavior in the latter case.

If one were to take care to note which language extensions were implied by which other ones, we can determine which elements in the list of language extensions could not be commuted "past" one another when alphabetizing. If this bookkeepping was performed, we could indempotently output a mostly alphabetized list of language extensions when one or more elements cannot commute "past" each other or a completely alphabetized list when all elements can be commuted, preserving the module's semantics in either case.

While this approach would require much more work to implement and maintain, it is the most correct option for an opinionated code styler.

IOW, we should take into account that some pragmas that disable extensions enabled as side-effect of enabling other extensions should be put after those extensions because sometimes the order of extension pragmas matters.

[neongreen]

An almost sorted list is worse than an unsorted list, IMO. It's going to be very nonobvious to anyone reading formatted code that it's not a bug/accident.

[mrkkrp]

What matters here I guess is that the order will be deterministic, thus it'll prevent diffs from shuffling.

[teofr]

I'm not sure there's a unique most general almost sorted list, so I don't think it's obvious to choose a sorting algorithm.

For instance, if you have pragmas B, X and Z, and B needs to come after Z, which of the following would you choose:

X, Z, B
Z, B, X

They're both equally far from being sorted.

I'm not saying it shouldn't be done, just that the sorting won't be obvious.

[mboes]

Almost sorted won't work. At the end of the day, Ormolu is a programmable formatter. You sketch out the format you want, and it does the rest. So the semantics of the source file, resulting from sorting the language pragmas, could well be the semantics the programmer intends, even if it's sometimes different from the input.

The much simpler option is to allow pragmas to be grouped using intervening blank lines, and then to sort each group independently but not reorder the groups.

[mrkkrp]

I'd like to have a single section because it's more normalized that way. It's not a problem to invent an ordering. Returning to @teofr 's example, we could say that if X < Z, then we choose:

X, Z, B

Or we could pick all extensions that disable things that not enabled by default and put them all after everything else.

[mboes]

But you're ignoring @neongreen's concern. A very valid one.

[mrkkrp]

I think it's not a big deal because in practice in 99% of cases extensions will be simply alphabetically sorted. It's not common to enable an extension X and then add some more extensions to disable something that was enabled by X as side-effect.

[mrkkrp]

It's going to be very nonobvious to anyone reading formatted code that it's not a bug/accident.

Even if a user decides that it is a bug, he/she will report it here to discover that it is not.

[utdemir]

Just to note, here is the relevant part of the GHC source code. Unfortunately it isn't exported from ghc the library.

[aspiwack]

@utdemir if you make a patch to export it in the API next week, and ask for it to be in 8.10 will be available in the next release.

[neongreen]

The linked table has only two turnOff entries:

(LangExt.RebindableSyntax, turnOff, LangExt.ImplicitPrelude)
(LangExt.StandaloneKindSignatures, turnOff, LangExt.CUSKs)

I think we can ignore LANGUAGE CUSKs, it seems to be very, very rare (only one example on GitHub). And funnily, you can't actually have RebindableSyntax and ImplicitPrelude together, no matter the order:

{-# LANGUAGE RebindableSyntax #-}
{-# LANGUAGE ImplicitPrelude #-}

main = if True then print "hi" else print "bye"

$ ghc x.hs
[1 of 1] Compiling Main             ( x.hs, x.o )

x.hs:4:8: error: Not in scope: ‘ifThenElse’
  |
4 | main = if True then print "hi" else print "bye"
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This leaves only the case where the user says {-# LANGUAGE NoX #-} to disable some X enabled by a different extension earlier. This can be handled by putting all NoX pragmas into a separate section (my preferred option), or simply sorting them at the end of the list.

Yes, we can no longer say "if it compiled before reformatting, it will compile after reformatting", but on the other hand we can prevent people from having to debug "why doesn't it compile even though I said NoX?" errors. Plus, I find myself somewhat persuaded by @mboes's comment:

[...] the semantics of the source file, resulting from sorting the language pragmas, could well be the semantics the programmer intends, even if it's sometimes different from the input.

[neongreen]

Update: I changed my mind, one block of pragmas seems better than two blocks. Then we can have "one block = one type of pragmas" (LANGUAGE, OPTIONS_GHC, etc).

[cdsmith]

@neongreen

And funnily, you can't actually have RebindableSyntax and ImplicitPrelude together, no matter the order

This is not true. It is true that Prelude in base doesn't define ifThenElse, but Prelude can implicitly imported from other packages besides base. Or, of course, ifThenElse could be imported explicitly (which is, after all, what you would have had to do with NoImplicitPrelude also).

I regularly build student code with -XRebindableSyntax -XImplicitPrelude, though that's on the command line rather than in pragmas. But I don't think you can dismiss this use case.

[neongreen]

This is not true. It is true that Prelude in base doesn't define ifThenElse

Oops, right. Then we can't dismiss this case.

Ok, what about this?

-- all extensions
-- all NoExtensions
-- CUSKs
-- ImplicitPrelude

And in the meantime someone has to make a GHC patch so that eventually we'd be able to ask GHC for extension interactions instead of hard-coding the logic.

[cdsmith]

It's worth asking: if someone asks for ImplicitPrelude followed by RebindableSyntax, of course this is a mistake... but should reformatting their source code really fix their mistake anyway -- possibly changing the behavior of their code? I'd hope that running ormolu should never break your code, no matter how poorly the original was written!