[plugin][refine-import] Do not refine to Internal module

[rayshih]

So we shouldn't refine imports to use internal modules, for example the following should not happen:

import H

refined to

import H.Internal (h)

corresponding test attached :)

[pepeiborra]

It shouldn't be too hard to add configuration for the internal suffix - @berberman and @isovector are the experts

[berberman]

FYI, an example of Properties in type lenses plugin:

haskell-language-server/ghcide/src/Development/IDE/Plugin/TypeLenses.hs

Lines 89 to 112 in 7d7cfa2

	descriptor :: PluginId -> PluginDescriptor IdeState
	descriptor plId =
	(defaultPluginDescriptor plId)
	{ pluginHandlers = mkPluginHandler STextDocumentCodeLens codeLensProvider
	, pluginCommands = [PluginCommand (CommandId typeLensCommandId) "adds a signature" commandHandler]
	, pluginRules = rules
	, pluginConfigDescriptor = defaultConfigDescriptor {configCustomConfig = mkCustomConfig properties}
	}
	properties :: Properties '[ 'PropertyKey "mode" ('TEnum Mode)]
	properties = emptyProperties
	& defineEnumProperty #mode "Control how type lenses are shown"
	[ (Always, "Always displays type lenses of global bindings")
	, (Exported, "Only display type lenses of exported global bindings")
	, (Diagnostics, "Follows error messages produced by GHC about missing signatures")
	] Always
	codeLensProvider ::
	IdeState ->
	PluginId ->
	CodeLensParams ->
	LSP.LspM Config (Either ResponseError (List CodeLens))
	codeLensProvider ideState pId CodeLensParams{_textDocument = TextDocumentIdentifier uri} = do
	mode <- usePropertyLsp #mode pId properties

[jneira]

@rayshih had the example above helped to implement the config of the internal suffix?

[jneira]

@rayshih it seems it only needs make configurable the internal modules prefix, do you have still plans to continue working on this? thanks!

[Anton-Latukha]

I would operate on the fact that rayshih contributed the PR but after that never responded to a request & rerequests.

I consider it as good. As it is an improvement. & I can understand the both design approaches, to encourage only recommended & to complicate the bad, and the approach to allow customization even if Internal is a bad practice...

---

What we currently have is done in the default way & seems to roughly cover the Pareto balance. In the majority of cases, we do want this default.

Generally, people indeed do not want/should not depend on Internal modules if that is possible (& that is what HLS should advise by default).

& in 20% of cases when they want to depend on Internal uncontrollable API - we initially can allow them to do that manually.
As it is said in the good tooling/language design - the good style should be encouraged, but the bad style should be discouraged (in this case requiring manual work that is also a statement person wants it & knows what is doing).

Why to avoid Internal?
Because: http://nikita-volkov.github.io/internal-convention-is-a-mistake/

[Anton-Latukha]

I would appeal to reason that to keep people away from refining into Internals is better than to allow them. As refining into Internal is simply dangerous - as Internal explicitly by own-put definition states that permanent refining behavior into it would mean to encourage refining into modules where changes would have no notification, no documentation, no support, no consistency or expectancy of API breakages.
& to reason in http://nikita-volkov.github.io/internal-convention-is-a-mistake

[Anton-Latukha]

When a contributor does not address a request - there is no way around it.

In that case it is useful to readdress the request to the community to decide if that would be needed enough to be implemented.

I relocated the request for the plugin customization option into report #2521.

[Anton-Latukha]

The code seems simple enough that I can review it & the code is clear to me. Indeed even tests are there.

[jneira]

well I think configure the name schema to filter suggestions would generalise this nicely to "filter out modules which I don't want" in suggestions, for any reason, not only being internal
and it would allow to opt out at same time left the option null or blank

imo the last thing is required, as we are gonna change an actual behaviour and bother users who want to use internal modules

[michaelpj]

Closing as dead.