Preserve comments on language pragmas

[neongreen]

Input:

{-# LANGUAGE DerivingVia #-}
{-# LANGUAGE PackageImports #-}

-- FIXME: We need this for such-and-such stupid reason. Ideally we should
-- fix it with this-and-this approach.
{-# LANGUAGE ExtendedDefaultRules #-}

module Foo where

Output:

{-# LANGUAGE DerivingVia #-}
{-# LANGUAGE ExtendedDefaultRules #-}
{-# LANGUAGE PackageImports #-}

-- FIXME: We need this for such-and-such stupid reason. Ideally we should
-- fix it with this-and-this approach.
module Foo where

This means that I can't document tricky/unpleasant exceptions for other code readers.

[mrkkrp]

This seems to be in a conflict with the idea of automated grouping of language pragmas.

[basile-henry]

The pragmas could only be sorted by groups split using comments as delimiters.

This is similar to what stylish-haskell does with imports (and maybe also language pragmas) as far as I remember. Though it uses any amount of empty lines or comments as group delimiter as opposed to just comments.

[mrkkrp]

I don't see a good way to implement it. What we currently do is we check every comment to detect those that are language pragmas. Keeping track of comments between pragmas would require more state. Seems to be quite troublesome for little benefit. I'm keeping this one with low priority.

[recursion-ninja]

There are semantic problems with this approach.

Some language extensions enable other language extensions implicitly. If you want to disable them, you must specify that after the extension that implicitly enables them is specified.

Consider this input:

{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeFamilies        #-}
-- This gap is necessary for stylish Haskell not to re-arrange                                           
-- NoMonoLocalBinds before TypeFamilies
{-# LANGUAGE NoMonoLocalBinds    #-}

module Foo (
  , bar
  ) where

This module will not have MonoLocalBinds enabled becaue the NoMonoLocalBinds language extension occurs after the TypeFamilies language pragma which implicitly enables MonoLocalBinds.

If the styler groups the language extensions and alphabetizes them like so, there will be problems:

{-# LANGUAGE NoMonoLocalBinds    #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeFamilies        #-}
-- This gap is necessary for stylish Haskell not to re-arrange                                           
-- NoMonoLocalBinds before TypeFamilies

module Foo (
  , bar
  ) where

Because NoMonoLocalBinds appears before Typefamilies, GHC considers it "disabled." GHC then continues to fold over the list of language extensions and when it encounters TypeFamilies, MonoLocalBinds is implicitly enabled.

This grouping and alphabetizing of language extensions will cause semantic differences in the input and output modules. The former does not have MonoLocalBinds enabled, while the latter does.

[mboes]

In light of this, perhaps we should reorder only inside groups separated by a newline.

[neongreen]

It seems like "don't reorder extensions at all" is safer. Or perhaps we should put all "NoX" extensions into their own group at the end?

…

On Mon, Sep 16, 2019, 11:57 Mathieu Boespflug ***@***.***> wrote: In light of this, perhaps we should reorder only inside groups separated by a newline. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#216?email_source=notifications&email_token=AALT42QGMUY7AE452WKHJLDQJ5C6FA5CNFSM4IDR45W2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6YQAKA#issuecomment-531693608>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALT42S4S2ZLVGDSDLJEWM3QJ5C6FANCNFSM4IDR45WQ> .

[neongreen]

Though frankly, it's not worth losing the "if it compiled before formatting, it will still compile after formatting" invariant for such a minor benefit (sorted pragmas).

…

On Mon, Sep 16, 2019, 13:23 Artyom Kazak ***@***.***> wrote: It seems like "don't reorder extensions at all" is safer. Or perhaps we should put all "NoX" extensions into their own group at the end? On Mon, Sep 16, 2019, 11:57 Mathieu Boespflug ***@***.***> wrote: > In light of this, perhaps we should reorder only inside groups separated > by a newline. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#216?email_source=notifications&email_token=AALT42QGMUY7AE452WKHJLDQJ5C6FA5CNFSM4IDR45W2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6YQAKA#issuecomment-531693608>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AALT42S4S2ZLVGDSDLJEWM3QJ5C6FANCNFSM4IDR45WQ> > . >

[recursion-ninja]

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.

[mrkkrp]

I agree we should not abandon sorting with single group of extensions in the output, we just should make it more clever.

[mrkkrp]

@recursion-ninja Now there is a separate issue about sorting: #404. I think it's a different problem.

[lrworth]

Just noting that the same goes for OPTIONS_GHC

-- Avoid warning produced by TH.
{-# OPTIONS_GHC -fno-warn-overlapping-patterns #-}