Support for inlay hints

[elamc-2]

Closes #473

Adds inlay hint support for inferred types and parameter names.

Todo:

• add tests
• hints for lambdas

consider adding a resolve method for versioning to solve textDocument/inlayHint request failure

[themkat]

Did some manual testing, and it seems to work really great 🙂 Awesome work! 👍 Looking forward to seeing the tests, which I see in your todo-list.

[elamc-2]

Did some manual testing, and it seems to work really great 🙂 Awesome work! 👍 Looking forward to seeing the tests, which I see in your todo-list.

Added tests, let me know how it looks

[themkat]

Looks good to me! 😄 Some minor comments below. Some of the detekt issues will be gone by itself once #513 is merged (once rebased/merged into your branch ofc), but the ones that will not I have commented below.

Pretty sure this is ready for merge after that is done. Will have a final look and approve at that time 🙂 Also to give @fwcd time to have a look in case they want to.

[elamc-2]

@themkat @fwcd is there a way we can skip some of the issues detekt raises?

[themkat]

@themkat @fwcd is there a way we can skip some of the issues detekt raises?

In general, one can use the @Suppress annotation for that (e.g, @Suppress("EmtpyClassBlock")) 🙂 Then you can skip rules in general places. Disabling rules for the entires code base is done in detekt.yml in the root-directory.

I see you have fixed the issues while waiting for our reply. Good job! Sorry for being a bit slow, have been busy in the last few days 🙁

[themkat]

Found one case we might consider adding. It is okay if you don't add it in this PR, but then we should create an issue for it 🙂

The case is the one-line function syntax that Kotlin support. Notice the function myfunc below:

We see that type hints appears as they should for the variable, but nothing for the function. Unsure if other IDEs give type hints in this case, but in my view it would probably be neat to have 🙂

[themkat]

Looks very good to me! 🙂 Thanks again for fixing the detekt issues.

Also see my previous comment. I'm open to merging this PR without it, but if it is reasonable we should create an issue for it.

[elamc-2]

Found one case we might consider adding. It is okay if you don't add it in this PR, but then we should create an issue for it 🙂

The case is the one-line function syntax that Kotlin support. Notice the function myfunc below:

We see that type hints appears as they should for the variable, but nothing for the function. Unsure if other IDEs give type hints in this case, but in my view it would probably be neat to have 🙂

good catch, I can add it to this PR.
Also would be nice to have additional control over settings i.e. toggling different hints since now all types are rendered by default. This is something I can work on for this PR too.

[elamc-2]

@themkat let me know what you think but I believe we can merge this now.
Just an FYI - to reflect config changes the vscode plugin needs to be updated (open an issue for this?). My setup using neovim looks like this:

lspconfig.kotlin_language_server.setup {
    cmd = { ... },
    capabilities = capabilities,
    on_attach = on_attach,
    settings = {
        kotlin = {
            hints = {
                typeHints = true,
                parameterHints = true,
                chainedHints = false,
            },
        },
    },
}

[themkat]

Your build fails after the latest commits 🙁 Also fails locally, which I tried just to make sure. Probably the toggling-properties not being set for the tests and therefore defaulting to false?

[themkat]

Just an FYI - to reflect config changes the vscode plugin needs to be updated (open an issue for this?)

As long as it doesn't crash without these properties, then we are okay for now. Tried it in VSCode, and it doesn't seem to crash at least. Feel free to create the issue in vscode-kotlin once we are closer to merging this PR 🙂 Most clients probably need to add them in some shape or form in the future anyway.

[elamc-2]

Your build fails after the latest commits 🙁 Also fails locally, which I tried just to make sure. Also seems like it stopped working in the editors (VSCode + Emacs)

My bad, tests fail it seems due to setting the defaults to false (not rendering hints and why it dosen't show up in your editor):

kotlin-language-server/server/src/main/kotlin/org/javacs/kt/Configuration.kt

Line 49 in 2f03112

data class InlayHintsConfiguration(

Is there a clean way to call config through tests? or would we prefer to set defaults as true i.e. leaving all hints on and having the option to disable them (tests will work that way too)

[themkat]

Is there a clean way to call config through tests?

The language server test fixture creates a languageServer object. This KotlinLanguageServer object has an instance of config, and I'm pretty sure you can just set the config with that one:

kotlin-language-server/server/src/main/kotlin/org/javacs/kt/KotlinLanguageServer.kt

Line 26 in 2f03112

val config = Configuration()

[themkat]

Seems like there are only detekt issues now. If you don't see any other ways to write the functions, I suggest a simple @Suppress("ReturnCount") on top of the offending functions. We may consider removing this rule in the future, or at least configuring it with a higher threshold...

[themkat]

Looks great to me, and works great 🙂 Awesome job! 🎉

Will give @fwcd a day or two before merging in case he wants to have a look.

[themkat]

Let's merge it so people can start trying it! 😄 Good job @ElamC ! 🎉