Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hls-hlint-plugin: Build fails with ghc-lib >= 9.0 and ghc <= 8.10.7 #2728

Closed
maralorn opened this issue Feb 20, 2022 · 19 comments
Closed

hls-hlint-plugin: Build fails with ghc-lib >= 9.0 and ghc <= 8.10.7 #2728

maralorn opened this issue Feb 20, 2022 · 19 comments
Labels
component: hls-hlint-plugin status: blocked Not actionable, because blocked by upstream/GHC etc. type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@maralorn
Copy link
Contributor

I am trying to build hls-hlint-plugin in the above mentioned combination.

I get the following error:

src/Ide/Plugin/Hlint.hs:132:44: error:
    • The constructor ‘GHC.RealSrcSpan’ should have 2 arguments, but has been given 1
    • In the pattern: GHC.RealSrcSpan x
      In the pattern: (GHC.RealSrcSpan x, y)
      In the pattern: (, Nothing) -> (GHC.RealSrcSpan x, y)
    |
132 | pattern RealSrcSpan x y <- ((,Nothing) -> (GHC.RealSrcSpan x, y))
    |                                            ^^^^^^^^^^^^^^^^^

src/Ide/Plugin/Hlint.hs:230:23: error:
    • The constructor ‘RealSrcSpan’ should have no arguments, but has been given 2
    • In the pattern: RealSrcSpan span _
      In an equation for ‘srcSpanToRange’:
          srcSpanToRange (RealSrcSpan span _)
            = Range
                {_start = Position
                            {_line = fromIntegral $ srcSpanStartLine span - 1,
                             _character = fromIntegral $ srcSpanStartCol span - 1},
                 _end = Position
                          {_line = fromIntegral $ srcSpanEndLine span - 1,
                           _character = fromIntegral $ srcSpanEndCol span - 1}}
      In an equation for ‘rules’:
          rules plugin
            = do define $ \ GetHlintDiagnostics file -> ...                 defineNoFile $ \ GetHlintSettings -> ...
                 action
                   $ do files <- getFilesOfInterestUntracked
                        ....
            where
                diagnostics ::
                  NormalizedFilePath -> Either ParseError [Idea] -> [FileDiagnostic]
                diagnostics file (Right ideas)
                  = [(file, ShowDiag, ideaToDiagnostic i) |
                       i <- ideas, ideaSeverity i /= Ignore]
                diagnostics file (Left parseErr)
                  = [(file, ShowDiag, parseErrorToDiagnostic parseErr)]
                ideaToDiagnostic :: Idea -> Diagnostic
                ideaToDiagnostic idea
                  = Diagnostic
                      {_range = srcSpanToRange $ ideaSpan idea, _severity = Just DsInfo,
                       _code = Just (InR $ T.pack $ codePre ++ ideaHint idea),
                       _source = Just "hlint", _message = idea2Message idea,
                       _relatedInformation = Nothing, _tags = Nothing}
                  where
                      codePre = ...
                ....
    |
230 |       srcSpanToRange (RealSrcSpan span _) = Range {

I might be holding it wrong, but my impression is, that this line should not compare ghc, but ghc-lib versions.

If someone more knowledgeable (e.g. @fendor) could tell me if that’s right, I‘d be very grateful.

@maralorn maralorn added status: needs triage type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Feb 20, 2022
@maralorn
Copy link
Contributor Author

My initial assumption about what the problem is, was wrong.

pattern RealSrcSpan x y <- ((,Nothing) -> (GHC.RealSrcSpan x, y))

I could fix the problem for this specific config by replacing the above line with

pattern RealSrcSpan x y <- ((,Nothing) -> (GHC.RealSrcSpan x Nothing, y))

The other solution did not work because then I got a mismatch in the type of BufSpan (the ghc or ghc lib BufSpan vs. the IDE Compat BufSpan).

@maralorn
Copy link
Contributor Author

That fix isn‘t a fix after all. We get the following runtime error:

Source:   compiler
Severity: DsError
Message: 
  src/Ide/Plugin/Hlint.hs:(250,7)-(258,48): Non-exhaustive patterns in function srcSpanToRange

@michaelpj
Copy link
Collaborator

cc @pepeiborra

@michaelpj
Copy link
Collaborator

michaelpj commented May 24, 2022

I've actually also observed that runtime error in HLS from nixpkgs recently.

@pepeiborra
Copy link
Collaborator

Currently you need to build hlint with the ghc-lib flag until ndmitchell/hlint#1376 is fixed and released

@mjrussell
Copy link
Contributor

Is there an example for how to build hlint with the ghc-lib with nix? Do you override the Haskell package HLS inputs somehow?

@pepeiborra
Copy link
Collaborator

Looks like there is an incomplete pattern match in the hls-hlint-plugin itself? Perhaps this wasn't noticed before because the code path was CPP'd out.

@mjrussell
Copy link
Contributor

Presumably from here?

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

It doesn't look non-exhaustive to me and it doesn't look like its changed in a recent GHC version to add a new constructor. Am I missing something obvious?

@maralorn
Copy link
Contributor Author

maralorn commented May 24, 2022

The runtime error in nixpkgs could totally be my fault, because I have added this line in nixpkgs to make the setup compile:

https://github.com/NixOS/nixpkgs/blob/8d8b6e8f442c658f53ae4b10a5060adbd1859c56/pkgs/development/haskell-modules/configuration-ghc-8.10.x.nix#L97

@mjrussell
Copy link
Contributor

mjrussell commented May 24, 2022

Ohhhh, yes indeed I think that could be the culprit

Stepping back for a second, and sorry if this is obvious as I'm a bit new to GHC Lib, but shouldn't we be trying to compile GHC-Lib 8.10.7 with GHC 8.10.7? Can we force the package bounds in the nix derivation?

[edit below]

Im wondering if hls-hlint-plugin = addBuildDepend self.ghc-lib_8_10_7 ... would work without a patch.

@anka-213
Copy link
Contributor

anka-213 commented May 26, 2022

I believe your initial assessment was indeed correct. The pragma line should be changed from

#if MIN_VERSION_ghc(9,0,0)

to

#if MIN_GHC_API_VERSION(9,0,0)

It was changed from the latter to the former in #2128, probably by mistake, when updating the pattern synonym to take two arguments instead of one.

@July541
Copy link
Collaborator

July541 commented May 26, 2022

I believe your initial assessment was indeed correct. The pragma line should be changed from

#if MIN_VERSION_ghc(9,0,0)

to

#if MIN_GHC_API_VERSION(9,0,0)

It was from the latter to the former in #2128, probably by mistake, when updating the pattern synonym to take two arguments instead of one.

I think it has been fixed in a recent commit

@anka-213
Copy link
Contributor

The change to

pattern RealSrcSpan x y <- ((,Nothing) -> (GHC.RealSrcSpan x Nothing, y))

would assert that the second field must be nothing, which is what caused the non-exhaustive pattern match error.

The correct version would be

pattern RealSrcSpan x y <- ((,Nothing) -> (GHC.RealSrcSpan x y, _))

but that is just equivalent to

pattern RealSrcSpan x y <- GHC.RealSrcSpan x y

which is what the other branch of the #if says.

(All of these versions only works with the ghc-9 api, since that's where GHC.RealSrcSpan was changed to have two fields instead of one)

@anka-213
Copy link
Contributor

I think it has been fixed in a recent commit

Yes, #2854 seems to have fixed it.

@maralorn Can you try with the latest version from master and see if it works now?

@maralorn
Copy link
Contributor Author

Ah, well I hadn‘t dabbled with pattern synonyms before and got confused somewhere along the way.

The easiest way for me to try this is out is waiting for the next release. Depending on when that happens I‘d rather wait for that.

@mjrussell
Copy link
Contributor

@maralorn do you know when that release will be? I'd love to get this working again and am happy to try to build it if I can get some instructions on how to do so. It wasn't super obvious how to do so in the nixpkgs repo

@shinzui
Copy link

shinzui commented Jun 16, 2022

Is there a workaround for getting hlint to work with hls on 8.10.7 in nix?

@maralorn
Copy link
Contributor Author

@shinzui Since this is now a downstream issue, I have continued the discussion here: NixOS/nixpkgs#168064

@kokobd kokobd added status: blocked Not actionable, because blocked by upstream/GHC etc. component: hls-hlint-plugin and removed status: needs triage labels Sep 7, 2022
@michaelpj
Copy link
Collaborator

GHC 8.10.7 is no longer supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-hlint-plugin status: blocked Not actionable, because blocked by upstream/GHC etc. type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

8 participants