tweag / ormolu

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

Pre-position qualified imports should be ok. #1010

Closed tonyday567 closed 1 year ago

tonyday567 commented 1 year ago

Describe the bug Recent versions of ormolu change pre-qualified imports to post-qualified imports.

Leaving it alone would be a better approach.

To Reproduce

cabal install ormolu
ormolu --mode check Lib.hs

With a pre-qualified import and with ImportQualifiedPost or GHC2021 on:

- import qualified Prelude as P
+ import Prelude qualified as P

With a post-qualified import but without ImportQualifiedPost and, say Haskell2010:

src/NumHask/Algebra/Field.hs:27:16-24
  The GHC parser (in Haddock mode) failed:
  Found `qualified' in postpositive position. 

Expected behavior

ImportQualifiedPost is an option to qualify imports in post-position, not a requirement. Ormolu should leave pre-qualified imports alone, even in the presence of ImportQualifiedPost.

Environment cabal-3.10.1.0 ghc-9.6.1

ormolu 0.5.3.0 using ghc-lib-parser 9.4.4.20221225

Additional context Even if you considered post-qualified to be best practice, the behaviour causes problems with supporting older GHCs. A recent ormolu cannot be used to format code that is intended to support ghc-8.8.4 and earlier, as post-qualified doesn't compile and ImportQualifiedPost doesn't exist.

By not allowing pre-qualified imports in the presence of ImportQualifiedPost, a choice is artifically created; between supporting older GHCs, dropping usage of GHC2021 (which is the cabal default now), dropping usage of ormolu, or resorting to CPP everywhere.

ocharles commented 1 year ago

I think ormolu is doing the correct thing here. If the flag is on, then I don't want a random assortment of pre/post qualification, I want consistency.

I don't understand your point about supporting older GHCs. Ormolu only does this if the option is turned on, which is a choice by you that rules out older GHCs. Can you elaborate on this point?

tonyday567 commented 1 year ago

Firstly, ImportQualifiedPost allows you to qualify imports in posterior position, but it allows you to continue to pre-qualify as well. import qualified Prelude as P, for example, is still valid and still compiles if ImportQualifiedPost is on.

https://ghc.gitlab.haskell.org/ghc/doc/users_guide/exts/import_qualified_post.html

GHC2021 include ImportQualifiedPost and is the default language in recent cabal versions. Being the default means that, in many cases, the user is not explicitly choosing to have ImportQualifiedPost turned on as an option, but has it on by default. I expect that the flag is part of GHC2021 because it permits post-qualification rather than enforces it, so that the inclusion is benign for existing code.

I would like to choose to:

I can see this dilemna in a recent CI run, where I am contemplating the inclusion of an ormolu check:

https://github.com/tonyday567/numhask/actions/runs/4662166061

ocharles commented 1 year ago

It sounds like what you want is to just use GHC2021 + NoImportQualifiedPost (conditionally in your Cabal file). Would that work?

tonyday567 commented 1 year ago

Yes, worked perfectly. Thanks for your consideration!

Closing

ocharles commented 1 year ago

Ace, glad I could help (and thanks for numhask, we love it!)