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

Applying hlint lint causes TypeApplication expressions to turn into holes _ #1242

Closed
talw opened this issue Jan 21, 2021 · 11 comments
Closed
Assignees
Labels
component: hls-hlint-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@talw
Copy link

talw commented Jan 21, 2021

Your environment

Output of haskell-language-server --probe-tools or haskell-language-server-wrapper --probe-tools:

haskell-language-server version: 0.8.0.0 (GHC: 8.10.3) (PATH: /nix/store/px0cwk7xl79yq4b1pvkwaxfkwaswhis1-haskell-language-server-0.8.0.0/bin/haskell-language-server-wrapper)
Tool versions found on the $PATH
cabal:		3.2.0.0
stack:		2.5.1
ghc:		8.10.3

Which lsp-client do you use:
Neovim
Describe your project (alternative: link to the project):
A cabal project
Contents of hie.yaml:

cradle:
  cabal:
    - path: "src"
      component: "lib:something"
  direct:
    arguments:
      - -isrc
      - -ionly-for-ghci
      - -XNoImplicitPrelude

      - -XBangPatterns
      - -XBlockArguments
      - -XDataKinds
      - -XDeriveFunctor
      - -XDeriveGeneric
      - -XDerivingStrategies
      - -XExistentialQuantification
      - -XFlexibleContexts
      - -XFlexibleInstances
      - -XFunctionalDependencies
      - -XGeneralizedNewtypeDeriving
      - -XGADTs
      - -XInstanceSigs
      - -XLambdaCase
      - -XMultiParamTypeClasses
      - -XMultiWayIf
      - -XNamedFieldPuns
      - -XNumericUnderscores
      - -XOverloadedStrings
      - -XRank2Types
      - -XRecordWildCards
      - -XScopedTypeVariables
      - -XStandaloneDeriving
      - -XStrictData
      - -XTupleSections
      - -XTypeApplications
      - -XTypeFamilies
      - -XTypeOperators
      - -XTypeSynonymInstances
      - -XViewPatterns

Steps to reproduce

With the following code apply the suggested lint to remove redundant bracket

a = (readMaybe @Int content)

Expected behaviour

This appears

a = readMaybe @Int content

Actual behaviour

This appears

a = _ content

This happens in the file so even this:

a = readMaybe @Int content

b = (5)

Turns into this:

a = _ content

b = 5

When invoking hlint with apply-refact by themselves through the CLI the
correct behaviour is demonstrated, and so I opened the issue in haskell-language-server
and not in hlint or apply-refact.

The problem seems to be with TypeApplications. The extension IS enabled
in hie.yaml's cradle -> direct -> arguments -> XTypeApplications

It doesn't matter which hlint lint is applied, as far as I can tell.

@jneira jneira added component: hls-hlint-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. status: regression test needed labels Jan 21, 2021
@jneira
Copy link
Member

jneira commented Jan 21, 2021

Many thanks for the detailed report, that is a really ugly bug, will take a look

@jneira jneira self-assigned this Jan 21, 2021
@alanz
Copy link
Collaborator

alanz commented Jan 21, 2021

I think there is a TypeApplications pragma missing, and the parser is turning the area it can't parse into _. I recall hitting this with the ghc-exactprint roundtrip tests for GHC 8.10.3. .

If I put that example into a file Foo.hs I get

a = readMaybe @Int content

b = (5)

errors

 Foo.hs     2   5 error           Pattern syntax in expression context: readMaybe@Int
 Did you mean to enable TypeApplications? (lsp)
 Foo.hs     4   5 info     refact:Redundant bracket Redundant bracket
 Found:
   (5)
 Why not:
   5
 (lsp)

@alanz
Copy link
Collaborator

alanz commented Jan 21, 2021

Related: should there be a code action to enable TypeApplication?

@jneira
Copy link
Member

jneira commented Jan 21, 2021

I cant reproduce with this hie.yaml:

cradle:
  direct:
    arguments:
      - "-XTypeApplications"
      - "ApplyRefact1"

and this ApplyRefact1.hs:

module ApplyRefact1 where

a = (id @Int 1)

hls built from master and ghc-8.10.3 (maybe you are using the released 0.8.0 version?). There had been a big change in the hlint plugin since the 0.8.0 release.

@alanz
Copy link
Collaborator

alanz commented Jan 21, 2021

Sorry: for my use case I did not have a hie.yaml, just created that file and got the diagnostics. So implicit cradle.

My point is, a type application turning into a wildcard is something that happens from the parser. Precisely

  mkHsAsPatPV l v e = do
    opt_TypeApplications <- getBit TypeApplicationsBit
    let msg | opt_TypeApplications
            = "Type application syntax requires a space before '@'"
            | otherwise
            = "Did you mean to enable TypeApplications?"
    patSynErr l (pprPrefixOcc (unLoc v) <> text "@" <> ppr e) (text msg)
  mkHsLazyPatPV l e = patSynErr l (text "~" <> ppr e) empty
  mkSumOrTuplePV = mkSumOrTupleExpr

patSynErr :: SrcSpan -> SDoc -> SDoc -> PV (LHsExpr GhcPs)
patSynErr l e explanation =
  do { addError l $
        sep [text "Pattern syntax in expression context:",
             nest 4 (ppr e)] $$
        explanation
     ; return (cL l hsHoleExpr) }

So hlint/applyrefact is not seeing the TypeApplications extension being set.

@jneira
Copy link
Member

jneira commented Jan 21, 2021

So hlint/applyrefact is not seeing the TypeApplications extension being set.

I see, thanks for the info.
It is little bit weird that the final result is to replace incorrectly (unexpectedly?) with a _ and not show the error "Pattern syntax in expression context:".
could we could do it something in the plugin to get the correct (expected?) behaviour: show the error and no replacing anything?

@jneira
Copy link
Member

jneira commented Jan 21, 2021

I've added the test case presented above in #1244

@talw
Copy link
Author

talw commented Jan 22, 2021

I cant reproduce with this hie.yaml:

cradle:
  direct:
    arguments:
      - "-XTypeApplications"
      - "ApplyRefact1"

and this ApplyRefact1.hs:

module ApplyRefact1 where

a = (id @Int 1)

hls built from master and ghc-8.10.3 (maybe you are using the released 0.8.0 version?). There had been a big change in the hlint plugin since the 0.8.0 release.

Yes @jneira, I am using the just released 0.8.0 release.
Just in case, I modified the hie.yaml to be just like yours and the bug is still being reproduced.
So perhaps this was fixed in current master?

@talw
Copy link
Author

talw commented Jan 22, 2021

I think there is a TypeApplications pragma missing

Indeed @alanz adding the pragma explicitly fixed the problem. However I already have the pragma set in both hie.yaml and the cabal file so, ideally, it should work without the pragma in the source file :)

@jneira
Copy link
Member

jneira commented Jan 22, 2021

So perhaps this was fixed in current master?

I think so.

@talw
Copy link
Author

talw commented Mar 1, 2021

Confirmed problem is gone in 0.9.0.0 (excuse my late confirmation).

Thanks @jneira @alanz !

@talw talw closed this as completed Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-hlint-plugin 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

3 participants