Make applyRefactoring take GHC extensions

[zliu41]

Also, parseExtensions now adds implied extensions. This requires depending on ghc-lib-parser-ex, which doesn't seem to support GHC 8.6. As a result, GHC 8.6 is no longer supported.

[jneira]

@zliu41 Thanks for this nice api addition.

As a result, GHC 8.6 is no longer supported.

That is unfortunate, as we are supporting ghc-8.6 in hls. And this is so cause there is no reliable ghc version > 8.6 for windows.
could be another alternative to avoid drop support for it?

[jneira]

In th hls-hlint-plugin we are using ghc-lib and ghc-lib-parser-ex for ghc < 8.10 but it stills support ghc-8.6:

https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-hlint-plugin/hls-hlint-plugin.cabal#L55-L62

Here we are using ghc-lib-parser-ex to parse language extensions from a string, just in case it can help:

https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs#L191-L198

[zliu41]

Yeah, I can copy the definition of impliedXFlags over - that was the reason for pulling in ghc-lib-parser-ex.

I think the proper solution is to migrate to ghc-lib-parser. That makes supporting multiple GHC versions a lot easier. However, ghc-exactprint is still on native GHC, so needs to be migrated first, and that's a huge task.

[jneira]

Yeah, I can copy the definition of impliedXFlags over - that was the reason for pulling in ghc-lib-parser-ex.

Nice, would be too much ask for that workaround? maybe under a cpp flag?

[zliu41]

Definitely not too much to ask. It's just a list of which extension implies which extension.
It may be updated when a new GHC version is out, but as you said we only need it for GHC 8.6 🙂

[zliu41]

@jneira Let me know if this looks reasonable! 🙂

[jneira]

It looks great, many thanks 👍

[jneira]

@zliu41 It is somewhat weird (cause ci is green for ghc-8.8.4) but i cant build the package with this merged and ghc-8.8.4 in windows, it throws many errors like:

src\Refact\Apply.hs:73:75: error:
    • Couldn't match expected type ‘ghc-lib-parser-8.10.2.20200916:GHC.Languagextensions.Type.Extension’
                  with actual type ‘Extension’
      NB: ‘Extension’
            is defined in ‘GHC.LanguageExtensions.Type’
                in package ‘ghc-boot-th-8.8.4’
         
           ‘ghc-lib-parser-8.10.2.20200916:GHC.LanguageExtensions.Type.Extension
            is defined in ‘GHC.LanguageExtensions.Type’
                in package ‘ghc-lib-parser-8.10.2.20200916’

    • In thehe second argument of ‘(==)’, namely ‘ext’
      In the expression: a == ext
      In a stmt of a list comprehension: a == ext
   |
73 |         impliedOff = [b | ext <- ys, (a, False, b) <- impliedXFlags, a ==xt]
   |
^^
cabal: Failed to build apply-refact-0.8.2.1 (which is required by
test:func-test from haskell-language-server-0.6.0.0).

Sorry, maybe i had to check its integration in hls before merging

[zliu41]

@jneira This error suggests that we need to disable the auto flag and enable the no-ghc-lib flag for ghc-lib-parser-ex.

I did this in cabal.project. Perhaps Windows has a different way to do that? Sorry I don't know a lot about cabal and I don't use windows

[zliu41]

Or perhaps you are using a different version of Cabal that has a different way of specifying dependency flags. My version is 3.2.0.0. You may want to check the documentation of the version you are using.

[jneira]

@zliu41 thanks, we are using ghc-lib-parser-ex for ghc < 8.10, following hlint configuration:

if ((!flag(ghc-lib) && impl(ghc >=8.10.1)) && impl(ghc <8.11.0))
  build-depends: ghc ^>= 8.10

else
  build-depends:
    , ghc
    , ghc-lib            ^>= 8.10.2.20200916
    , ghc-lib-parser-ex  ^>= 8.10

  cpp-options:   -DGHC_LIB

So not sure if that would be compatible with ghc-lib-parser-ex -auto +no-ghc-lib for all ghcs versions. I will try.
It is unfortunate that ghc-lib is being used in several projects, but in a no consistent way (ghcide uses ghc-lib for all versions or does not use it f.e.) 😟

[jneira]

With those flags (ghc-lib-parser-ex -auto +no-ghc-lib) is hlint-3.2.1 that cant be built with ghc-8.8 (afaiu cause it is expecting a ghc-lib-parser-ex version with -no-ghc-lib):

Building library for hlint-3.2.1..
[ 1 of 58] Compiling EmbedData        ( src\EmbedData.hs, dist\build\EmbedData.o)
[ 2 of 58] Compiling Extension        ( src\Extension.hs, dist\build\Extension.o)

src\Extension.hs:62:25: error:
    • Couldn't match type ‘ghc-boot-th-8.8.4:GHC.LanguageExtensions.Type.Extension’
                     with ‘Extension’
      NB: ‘Extension’
            is defined in ‘GHC.LanguageExtensions.Type’
                in package ‘ghc-lib-parser-8.10.2.20200916’
          ‘ghc-boot-th-8.8.4:GHC.LanguageExtensions.Type.Extension’
            is defined in ‘GHC.LanguageExtensions.Type’
                in package ‘ghc-boot-th-8.8.4’
      Expected type: [(Extension, ([Extension], [Extension]))]
        Actual type: [(ghc-boot-th-8.8.4:GHC.LanguageExtensions.Type.Extension,
                       ([ghc-boot-th-8.8.4:GHC.LanguageExtensions.Type.Extension],
                        [ghc-boot-th-8.8.4:GHC.LanguageExtensions.Type.Extension]))]
    • In the expression:n: GhclibParserEx.extensionImplications
      In an equation for ‘extensionImplications’:
          extensionImplications = GhclibParserEx.extensionImplications
   |
62 | extensionImplications = GhclibParserEx.extensionImplications
   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cabal: Failed to build hlint-3.2.1 (which is required by test:func-test from
haskell-language-server-0.6.0.0).

Maybe @shayne-fletcher could give us some advise in how to combine these pieces 🤔

[jneira]

I suppose a possible solution could be apply-refact would follow hlint with a similar configuration in the cabal file, using the real ghc for the las version and ghc-lib for older ones.

[zliu41]

I don't think apply-refact can use ghc-lib, because ghc-exactprint uses native GHC, rather than ghc-lib, and apply-refact depends on it.

I wonder if there's an easy way to convert GHC's Extension into ghc-lib's Extension, or vice versa.

If not, I can change applyRefactorings to take [String] instead of ([Extension], [Extension]). I should probably do this either way, since otherwise callers of applyRefactorings can only pass GHC Extensions and not ghc-lib Extensions.

[jneira]

I wonder if there's an easy way to convert GHC's Extension into ghc-lib's Extension, or vice versa.

I am doing just that 😄
In the plugin i have to deal with real ghc modules (as ghcide uses it) and the ghc-lib-* modules (used by hlint) at the same time:

setExtensions flags = do
          hsc <- hscEnv <$> use_ GhcSession nfp
          let dflags = hsc_dflags hsc
          let hscExts = EnumSet.toList (extensionFlags dflags)
          let hscExts' = mapMaybe (GhclibParserEx.readExtension . show) hscExts
          let hlintExts = nub $ enabledExtensions flags ++ hscExts'
          logm $ "hlint:getIdeas:setExtensions:" ++ show hlintExts
          return $ flags { enabledExtensions = hlintExts }

Here hscExts are from the real ghc and i am using GhclibParserEx.readExtension . show (so using String as intermmediate data)

[jneira]

I can change applyRefactorings to take [String] instead of ([Extension], [Extension]). I should probably do this either way, since otherwise callers of applyRefactorings can only pass GHC Extensions and not ghc-lib Extensions.

Agree with that, we have to switch to dynamic typing here 😝

[shayne-fletcher]

Maybe @shayne-fletcher could give us some advise in how to combine these pieces 🤔

I'm not sure I can but I can explain those flags in case it helps.

ghc-lib-parser-ex needs to link one of ghc or ghc-lib-parser. Its default is to behave like hlint i.e. if the compiler is 8.10, link ghc else link an 8.10 version of ghc-lib-parser. That's the meaning of auto. So,

• If auto and compiling with ghc-8.10, link ghc-8.10;
• Otherwise:
  • If auto and not compiling with ghc-8.10 use an 8.10 ghc-lib-parser;
  • Else:
    • If no-ghc-lib, use whatever ghc you find in scope;
    • Otherwise use whatever ghc-lib-parser you find in scope.