haskell-language-server icon indicating copy to clipboard operation
haskell-language-server copied to clipboard

Stan plugin recommends StrictData pragma even when it is in scope

Open lsmor opened this issue 2 years ago • 2 comments

Your environment

Which OS do you use? Ubuntu 20.0.4 Focal Which version of GHC do you use and how did you install it? ghc-8.10.7 with ghcup How is your project built (alternative: link to the project)? with cabal

Which LSP client (editor/plugin) do you use? VsCode with haskell extension Which version of HLS do you use and how did you install it? hls-1.8.0 I did install it with ghcup manually pointing to debian bindist, instead of default fedora as explain in this comment Have you configured HLS in any way (especially: a hie.yaml file)? My hie.yaml is just

cradle:
  cabal:

Steps to reproduce

on a normal cabal project write a module

{-# LANGUAGE StrictData #-}

module MyModule where

data A = A {field :: Int, field2 :: String}

Expected behaviour

No error or warnings should happen with respect to strict fields

Actual behaviour

Stan underlies the field's accessors and recommends to add the StrictData pragma. Notice that Stan also recommends adding a bang pattern, and if you do that, then the warning disappears.

Debug information

This is the last output of hls. It seems that StrictData pragma is reconigze within the module, so I guess the problem is completely up to stan plugin. This is double inconviniently because stan can't be disable or at leat I haven't found the way either: #3157

[Trace - 19:26:53] Received notification 'window/logMessage'.
Using extensions for  NormalizedFilePath "/home/luis/Proyectos/<path/to/module>": [ MonomorphismRestriction
                                                                                  , RelaxedPolyRec
                                                                                  , ForeignFunctionInterface
                                                                                  , ImplicitPrelude
                                                                                  , DoAndIfThenElse
                                                                                  , EmptyDataDecls
                                                                                  , PatternGuards
                                                                                  , DatatypeContexts
                                                                                  , TraditionalRecordSyntax
                                                                                  , EmptyCase
                                                                                  , StrictData
                                                                                  , StarIsType
                                                                                  , CUSKs ]
[Trace - 19:26:53] Received notification 'window/logMessage'.
[Info  - 19:26:53] Typechecking reverse dependencies for NormalizedFilePath "//home/luis/Proyectos/<path/to/module>": [  ]
[Trace - 19:26:53] Received notification 'window/logMessage'.
Finished: ParentTC Took: 0.00s
[Trace - 19:26:53] Received request 'window/workDoneProgress/create - (68)'.
[Trace - 19:26:53] Sending response 'window/workDoneProgress/create - (68)'. Processing request took 0ms
[Trace - 19:26:53] Received notification '$/progress'.
[Trace - 19:26:53] Received notification '$/progress'.
[Trace - 19:26:53] Received request 'window/workDoneProgress/create - (69)'.
[Trace - 19:26:53] Sending response 'window/workDoneProgress/create - (69)'. Processing request took 0ms
[Trace - 19:26:53] Received notification '$/progress'.
[Trace - 19:26:53] Received notification '$/progress'.
[Trace - 19:26:54] Received notification '$/progress'.
2022-09-15T17:27:29.314287Z | Info | Live bytes: 181.82MB Heap size: 554.70MB

lsmor avatar Sep 15 '22 17:09 lsmor

Does this appear when running stan directly? It may just be a bug in stan.

michaelpj avatar Sep 17 '22 11:09 michaelpj

Does this appear when running stan directly? It may just be a bug in stan.

I tried stan standalone and via HLS on this project https://github.com/goolord/stan-stricdata-minimal. StrictData is correctly recognized when the code is checked directly with stan while the above error is reported through HLS.

johnhampton avatar Sep 18 '22 18:09 johnhampton

I was able to confirm that the warning disappeared after hard-coding as follows.

let cabalExtensionsMap = Map.fromList [(LSP.fromNormalizedFilePath file, Right $ ParsedExtensions ([On StrictData]) Nothing)]

So I thought about porting the part that reads Extension in the CLI, but when I call it in the CLI without calling it in the API, the StrictData warning did not occur even if I did not write the Pragma in the first place. So I am finding it difficult to compare and inspect.

ncaq avatar Sep 29 '22 14:09 ncaq

There's a fairly obvious TODO in the stan plugin where it needs extension information: https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-stan-plugin/src/Ide/Plugin/Stan.hs#L78

I think the hlint plugin has the same issue and gets the extensions somehow, so it should be possible to mirror that.

michaelpj avatar Sep 29 '22 14:09 michaelpj

The .stan.toml is not loading either, so it looks like I will have to lower the hls version until this issue is resolved.

ncaq avatar Sep 29 '22 14:09 ncaq

I was mistaken when I said that the CLI doesn't warn without a pragma. You have to generate the .hie to detect it.

ncaq avatar Sep 29 '22 15:09 ncaq

I looked at it to some extent, but I couldn't figure it out any way, so I gave up. The analysisObservations results are inevitably different. The only argument that differs is FilePath, so there may be a discrepancy there.

ncaq avatar Sep 29 '22 17:09 ncaq

Is this the same issue?

https://github.com/ndmitchell/hlint/issues/1432

mkohlhaas avatar Dec 09 '22 04:12 mkohlhaas

@michaelpj

There's a fairly obvious TODO in the stan plugin where it needs extension information: https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-stan-plugin/src/Ide/Plugin/Stan.hs#L78

But TODO was about cabal information, I think LANGUAGE pragmas should be different.

So currently the tasks here possibly are:

  • Respect LANGUAGE pragmas
  • Respect cabal enabled extensions
  • Respect Stan configuration

uhbif19 avatar Mar 17 '23 02:03 uhbif19

Just wanted to pop in and mention this still exists on HLS 2.5.0.0. We have StrictData set in our *.cabal file:

common deps-exts
  default-extensions:
    BlockArguments
    DataKinds
    DeriveAnyClass
    DerivingStrategies
    DerivingVia
    DuplicateRecordFields
    LambdaCase
    MultiWayIf
    NumericUnderscores
    OverloadedRecordDot
    OverloadedStrings
    PatternSynonyms
    QuasiQuotes
    RecordWildCards
    StrictData

but HLS continues to report diagnostics like:

1.  ✲ Name:        Data types with non-strict fields
    ✲ Description: Defining lazy fields in data types can lead to unexpected space leaks
    ✲ Severity:    Performance
    ✲ Category:    #SpaceLeak #Syntax
   Possible solutions:
     - Add '!' before the type, e.g. !Int or !(Maybe Bool)
     - Enable the 'StrictData' extension: {-# LANGUAGE StrictData #-}

EDIT: Disabling Stan in the HLS configuration is a work around that works for me, now, since Stan is not part of our CI, yet. I'm using neovim and the provided LSP client:

lua << END_LSPCONFIG
require('lspconfig').hls.setup{ settings = { haskell =
    { plugin =
      { stan = { globalOn = false }
      }
    }
} }
END_LSPCONFIG

stephen-smith avatar Dec 19 '23 15:12 stephen-smith

The problem was accidentally reintroduced when the stan plugin was re-added.

michaelpj avatar Dec 19 '23 17:12 michaelpj

Disabling Stan in the HLS configuration is a work around that works for me

Thanks for the workaround... this is a super-distracting bug! Every field of every data type is called out.

keithfancher avatar Jan 26 '24 21:01 keithfancher

I took a stab at fixing it :D

Please see above PR if you're affected!

keithfancher avatar Jan 29 '24 00:01 keithfancher

By the way, I'm not sure what the procedure is for closing out issues in this repo, but this issue should be fixed with PR https://github.com/haskell/haskell-language-server/pull/4023. (I don't think I have permission to close this out.)

keithfancher avatar Feb 26 '24 19:02 keithfancher