Renaming multi component support

[jneira]

• Currently it is disabled by default in the hls .cabal file:
  

  haskell-language-server/haskell-language-server.cabal

  Lines 134 to 137 in e1075e8

  	flag rename
  	description: Enable rename plugin
  	default: False
  	manual: True

• Would be good to know what steps are needed to being able to enable it

//cc @pepeiborra @OliverMadine

---

After enabling the plugin we will track here the multi component support for renaming here

[OliverMadine]

It is just disabled due to the limited multi-cradle support.

I've had a temporary solution (just loading all components listed in the hie.yaml) working for a while now but we need #304 and #2009 to be merged first. Both of these PRs seem to be working and ready to merge.

[michaelpj]

I think if we enabled it with the caveat that it will only work reliably across one component that would be fine. Perhaps slap "(experimental)" after all the code action titles :)

[OliverMadine]

I think if we enabled it with the caveat that it will only work reliably across one component that would be fine. Perhaps slap "(experimental)" after all the code action titles :)

We don't use a code action since we get the new name directly from the client. So I don't think we have a great way of announcing this caveat to the user without them having to go through HLS logs etc.

It would be nice to have a vscode option (rather than a cabal build flag) so that we could enable the plugin for single-component use more easily, but I'm not really sure how to do this (especially without causing the plugin tests to fail).
Also, hopefully we are close to some form of multi-component support so I'd rather just wait for that.

[jneira]

i am afraid #2009 needs haskell/hie-bios#301 which in turn needs the show-build-info feature which will be avilable in Cabal-3.8 afaik

[OliverMadine]

i am afraid #2009 needs haskell/hie-bios#301 which in turn needs the show-build-info feature which will be avilable in Cabal-3.8 afaik

It was my understanding that this PR is just concerned with 'loading multiple components' and we were going to use show-build-info to 'get the components to load' later.

If not, this at least seems to be what it is doing currently so maybe we could merge what is done so far ('loading multiple components') then open a new PR to continue the work?
#304 gets the necessary API changes in place for this.

[fendor]

Sorry, that PR is indeed more a POC for what is possible with show-build-info, not of the general multiple components loading mechanism.

However, it is true that the general issue of loading multiple components can be done independently of show-build-info, just the semantics of the proposed NonEmpty needs to be discussed.

[jneira]

It would be nice to have a vscode option (rather than a cabal build flag) so that we could enable the plugin for single-component use more easily, but I'm not really sure how to do this (especially without causing the plugin tests to fail).
Also, hopefully we are close to some form of multi-component support so I'd rather just wait for that.

We could add a configuration option in vscode to enable/disable the plugin at runtime and make it disabled by default. In the config description we could note the caveats.
Not sure if we could make the plugin disabled at runtime if no config is provided at all (but i think it should be possible)

//cc @berberman

[berberman]

Not sure if we could make the plugin disabled at runtime if no config is provided at all (but i think it should be possible)

In order to disable a plugin by default (no corresponding config section provided), we will need to change the behavior of parsing
PluginConfig:

haskell-language-server/hls-plugin-api/src/Ide/Plugin/Config.hs

Lines 78 to 95 in b6d1df2

	-- TODO: Add API for plugins to expose their own LSP config options
	parseConfig :: Config -> Value -> A.Parser Config
	parseConfig defValue = A.withObject "Config" $ \v -> do
	-- Officially, we use "haskell" as the section name but for
	-- backwards compatibility we also accept "languageServerHaskell"
	c <- v .: "haskell" <|> v .:? "languageServerHaskell"
	case c of
	Nothing -> return defValue
	Just s -> flip (A.withObject "Config.settings") s $ \o -> Config
	<$> (o .:? "checkParents" <|> v .:? "checkParents") .!= checkParents defValue
	<*> (o .:? "checkProject" <|> v .:? "checkProject") .!= checkProject defValue
	<*> o .:? "hlintOn" .!= hlintOn defValue
	<*> o .:? "diagnosticsOnChange" .!= diagnosticsOnChange defValue
	<*> o .:? "liquidOn" .!= liquidOn defValue
	<*> o .:? "formatOnImportOn" .!= formatOnImportOn defValue
	<*> o .:? "formattingProvider" .!= formattingProvider defValue
	<*> o .:? "maxCompletions" .!= maxCompletions defValue
	<*> o .:? "plugin" .!= plugins defValue

Currently, the following "static" default values will be used if the client does not provide these options:

haskell-language-server/hls-plugin-api/src/Ide/Plugin/Config.hs

Lines 132 to 144 in b6d1df2

	instance Default PluginConfig where
	def = PluginConfig
	{ plcGlobalOn = True
	, plcCallHierarchyOn = True
	, plcCodeActionsOn = True
	, plcCodeLensOn = True
	, plcDiagnosticsOn = True
	, plcHoverOn = True
	, plcSymbolsOn = True
	, plcCompletionOn = True
	, plcRenameOn = True
	, plcConfig = mempty
	}

I think a easy way would be carrying the information that "the client does not give this value" in parsed config, rather than concealing it with a default value. So in plugin's handler, we could retrieve the config which says no option to enable this plugin is given by the client.

[malteneuss]

With the new rename plugin implementation in mind, is this plugin now suitable to be enabled? It would create a huge productivity boost, even if still has that one-component-only-caveat.

[jneira]

Yeah, i think the required changes in cabal will take more time and there is no a wip in stack so it is time to enable it, making clear its limitations in docs

[pepeiborra]

As a fallback, could we enable the rename plugin on single component projects?

[michaelpj]

Do we have a way of detecting whether we are in a single- or multi-component project? If so, we could simply make the rename handler fail informatively if it detects it is in a multi-component package.

[fendor]

Do we have a way of detecting whether we are in a single- or multi-component project?

No, we don't. In the current design of hie-bios, there is no way to enumerate all components / modules of a project.
We definitely want something like it, but no one got around to implement it, yet.

[JoeMShanahan]

We definitely want something like it, but no one got around to implement it, yet.

Is there an issue somewhere that discusses this... perhaps a potential implementation or plan of attack?

[fendor]

Hi!

Yes, there is, and it requires multiple steps. First, we need to be able to actually get multiple components from a build tool. To achieve that for cabal, we have implemented haskell/cabal#7489 and merged haskell/cabal#7498 which means we are more than halfway there! The remaining PR for proper support by cabal is haskell/cabal#7500.
I'd be very happy if someone assists to get this merged. There is nothing difficult about it, currently, just a matter of not enough spare time on my end.

Afterwards, we would like to migrate hie-bios to use the new cabal API to see whether there are any design issues. The initial implementation for an old version of cabal show-build-info can be found here: haskell/hie-bios#301
Before that, we can also try to get this merged: haskell/hie-bios#304 which basically prepares the API to be able to emit multiple components at once.

After that, we can finally come to ghcide! We need to work on the ghcide session-loading logic which is very complicated and tricky to get right: https://github.com/haskell/haskell-language-server/blob/master/ghcide/session-loader/Development/IDE/Session.hs#L405
However, we can also look into the future since GHC multiple home unit support has been merged and will be released with 9.4, we should also look into utilising that.

I can be helpful on every step you want to tackle here, just ask for a couple clarifications and I can give you for cabal/hie-bios stuff step-by-step instructions.

[santiweight]

I am curious how this is going. As an HLS user, I certainly would love to see this come to fruition!

(Having to load in each component at my work codebase is still a sore spot!)

[georgefst]

Given that this doesn't appear to be happening any time soon, does anyone know how easy it might be to relax the explicit-export-list restriction for renaming local names? It's quite silly that I can't rename x here:

module M where
f x = ()

[fendor]

@georgefst it is likely to happen rather soon for ghc versoins > 9.4 once #3462 lands and the next major cabal release.

However, for your specific case, I think it should be easy to lift the restriction.

[georgefst]

Ah yes, I'd forgotten that this is yet another thing which will be solved by #3462 (although it is actually clear now that I've read the whole thread).

[michaelpj]

Multiple home unit support now exists, so I think this should now be possible. It would still need some work:

• We need to load the whole project (see also Configuration for how much of the project to build #3873)
• We need to enumerate all the files in the project
• Then we need to run the right renaming on all of them

But that seems pretty doable!