Support for inlay hints

[elamc-2]

Closes #473

Adds inlay hint support for inferred types and parameter names.

Todo:

• [x] add tests
• [x] hints for lambdas

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

[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

[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 🙂

[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]

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): https://github.com/fwcd/kotlin-language-server/blob/2f031129911d62e34aaeca9d9973f5968bc34f61/server/src/main/kotlin/org/javacs/kt/Configuration.kt#L49

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: https://github.com/fwcd/kotlin-language-server/blob/2f031129911d62e34aaeca9d9973f5968bc34f61/server/src/main/kotlin/org/javacs/kt/KotlinLanguageServer.kt#L26

[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]

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