Check PluginDescriptor on loading

[alanz]

When HIE starts up, it is presented with a list of Plugins.

Each of these defines commands in a CommandDescriptor

For each CommandDescriptor, check that the parameter names introduced via cmdContexts and cmdAdditionalParameters are unique.

If this is not the case, exit with an error.

[cocreature]

I wonder if it wouldn't be nicer to just namespace all parameter names implicitely with the cmdName. We're going to end up with something like that anyway since there are probably similar names in a lot of plugins so enforcing that seems like a good idea.

[alanz]

I think I have phrased this badly. What I mean is that the parameters for a given command must be unique. Because the AcceptedContext values bring fixed parameter names into scope for a command this is not always obvious, and always better to check this kind of thing.

[cocreature]

Ah yeah, then we are on the same page.

[tobiasgwaaler]

So ParamDescription.pName should be unique, and not collide with the serialized names for AcceptedContext. Am I getting this right?

[alanz]

Yes, in the sense that the pNames must all be unique within a given command for the union of
https://github.com/haskell/haskell-ide-engine/blob/master/hie-plugin-api/Haskell/Ide/Engine/PluginDescriptor.hs#L133 for the contexts defined for the command and
https://github.com/haskell/haskell-ide-engine/blob/master/hie-plugin-api/Haskell/Ide/Engine/PluginDescriptor.hs#L77

[tobiasgwaaler]

Any thoughts on how the error message should be worded? My suggestion is quite verbose:

Error: Parameter name collision in plugin description
Parameter names must be unique for each command. The following collisions were found:
In plugin "ghc-mod", command "check", the parameter name "file" is defined in:
        cmdAdditionalParams
            pName = "file"
            pHelp = "the file to read for blablabla"
    and 
        cmdContexts
            pName = "file"
            pHelp = "a file name"
In plugin "ghc-mod", command "info", the parameter name "name" is defined in:
        cmdAdditionalParams
            pName = "name"
            pHelp = "name of the blabla"
    and 
        cmdAdditionalParams
            pName = "name"
            pHelp = "nickname of the blabla"

[alanz]

I think verbose is better, so long as it ends up on stdout during startup.

We may need to consider wrapping them in the transport output.

[cocreature]

I also like the verbose format. “In plugin "ghc-mod", command "check", the parameter name” sounds a bit strange to me, but I am not a native speaker. How about changing it to “In "ghc-mod:check" the parameter name”? I think the colon syntax is pretty unterstandable and we also use that in other places.

I tend to agree that we should add that to the transport at some point, but imho that’s an orthogonal issue, so feel free to not tackle it in this pr.

[tobiasgwaaler]

I like plugin:command too, so I'll go for that unless @alanz thinks its unwise

[alanz]

That is fine by me. The main thing is that we do the check.