Make the eval plugin use the same default language extensions as ghci.

[fmehta]

This pull request aims to fix #1591

[jneira]

Many thanks!, a test case would be great to make visibly eval plugin is able to handle it and to avoid future changes breaks it

[jneira]

@fmehta feel free to ask for help if you need some guidance to write the test
The test suite for eval plugin is here: https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-eval-plugin/test/Eval.hs

[fmehta]

@jneira : Thanks a lot for following up on my progress and pointing me in the right direction. And yes, of course I need to include a test for this change (what was I thinking 🙄).

The reason for my delay: I was just wondering whether it even makes sense to enable this language extension at all. Maybe it makes sense to use exactly the same language extensions as ghc, since the generated comments will be part of the source code. Since there is no specification as to what the correct behaviour of the evals should be, I was wondering if you, or anyone else, could comment on what behaviour would be desirable.

I also notice that I cannot enable this language extension within the eval plugin for some reason (I guess only those that are listed in https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-eval-plugin/README.md >>> :set -XScopedTypeVariables -XStandaloneDeriving -XDataKinds -XTypeOperators -XExplicitNamespaces ) can be set. I also see that there are other language extensions that are enabled for ghci (:showi language within ghci listsNoDatatypeContexts, ExtendedDefaultRules, NoMonomorphismRestriction, NondecreasingIndentation), whereas the following are enabled by default in the eval plugin:

evalExtensions =
    [ OverlappingInstances
    , UndecidableInstances
    , FlexibleInstances
    , IncoherentInstances
    , TupleSections
    ]

I wonder whether they should not be the same?

Do you (or anyone else) have any thoughts on all of this, or should I just move on and add the tests for this commit without getting too deep here?

Thanks, FM

[pepeiborra]

I don't see anything wrong with enabilng ExtendedDefaultRules by default, and all the other ones ghci enables by default.

The set of extensions that are enabled currently for the eval plugin seem arbitrary and should be removed. /cc @tittoassini

[Ailrun]

I believe some of the extensions are used in the printing code (which is executed in Eval plugin session) of Eval plugin, so I guess it's not so trivial to remove....

[pepeiborra]

I believe some of the extensions are used in the printing code (which is executed in Eval plugin session) of Eval plugin, so I guess it's not so trivial to remove....

@berberman removed that custom printing code recently

[Ailrun]

Oh, I missed that, then probably we are good to go.

[jneira]

@fmehta hi, do you still plan to continue working on this? thanks!

[fmehta]

Hi @jneira
Thanks for your help. I have committed the tests.
Sorry for the delay. I was trying to run them through the top level contribution instructions at https://github.com/haskell/haskell-language-server/blob/master/CONTRIBUTING.md which did not work for me. I only later realised that I could just run cabal test within the eval plugin.

Could you please review and merge my changes in case they are OK. I will then create and work on a second followup issue for the rest of the default ghci extensions as mentioned in #1596 (comment).

Followup issue: #1954

Thanks,
FM

[berberman]

Could you change the purpose of this PR, resolving #1954 together?

[fmehta]

Hi @berberman

Could you change the purpose of this PR, resolving #1954 together?

#1954 needs some more work. In particular:

• I am trying to see if it is possible to get the defaults from the ghci code programatically, instead of manually.
• I am currently unsure if and how I can specify NoDatatypeContexts and NoMonomorphismRestriction, given that I can only find DatatypeContexts and MonomorphismRestriction within GHC.LanguageExtensions.Type.Extension.

(In case you have any pointers, please let me know.)

I therefore thought that I should start a new PR for the rest. But in case you think it is better to merge both together, I can do that as well. Just let me know.

Thanks
FM

[berberman]

There is no negative type of extension, so If you want to disable one, you should unset it. As far as I can tell, NoMonomorphismRestriction and ExtendedDefaultRules are configured manually in GHCi: https://github.com/ghc/ghc/blob/5abf59976c7335df760e5d8609d9488489478173/ghc/GHCi/UI.hs#L473-L483.

And evalExtensions finally comes to here:

haskell-language-server/plugins/hls-eval-plugin/src/Ide/Plugin/Eval/CodeLens.hs

Lines 304 to 306 in 0b3bb10

	df <- getSessionDynFlags
	setInteractiveDynFlags $
	(foldl xopt_set idflags evalExtensions)

[fmehta]

Dear @berberman

Thanks for your input. I have added the fix for #1954 to this pull request as well.

A third thing that I noticed and did not get my head around was that it seems that ExplicitForAll is enabled by default in the eval plugin, but not in ghci.

-- >>> plus = (+)
-- >>> :t plus
-- plus :: forall a. Num a => a -> a -> a

I cannot find out where this is added. Do you have any idea?

FM

[fmehta]

Hi @berberman
Hi @jneira

It should work now. Maybe one of you need to kickstart the CI.

Best,
FM

[jneira]

Looks great (love the comments in tests), many thanks

[fmehta]

Thanks @jneira!

[jneira]

@mergify rebase master

[mergify]

Command rebase master: success

Branch already up to date

Hey, I reacted but my real name is @Mergifyio