fwcd / kotlin-language-server

Kotlin code completion, diagnostics and more for any editor/IDE using the Language Server Protocol
MIT License
1.68k stars 212 forks source link

Report unused imports as diagnostics #353

Open themkat opened 2 years ago

themkat commented 2 years ago

IntellIJ IDEA reports unused imports as a diagnostic for the user. While formatters can remove unused imports automatically, it still might be nice to have a diagnostic/warning on it in your editor.

Could not find any additional compiler flags to automatically include it in the current diagnostics. There might be a need for a lightweight analyzer that adds to the current diagnostics (it seems like IntelliJs codebase does that, but we can probably take some shortcuts in our implementation)

themkat commented 2 years ago

One other option than doing something like this would be to have support for something like Detekt (which supports unused imports as well as other diagnostics and best practices). IntelliJ has a plugin for this purpose: https://github.com/detekt/detekt-intellij-plugin

My idea would be that we could utilize Detekt (with the option to turn it off, off course) for these kind of diagnostics. That would save us time for implementing these kinds of diagnostics in the language server itself. I'm a little apprehensive about introducing something like Detekt into our codebase directly for this. It would make more sense as a plugin, but unsure on how we could make a plugin for the language server itself (some type of non standard requests / protocol extensions enabling it?). Or I'm I just weird here? Would it make sense to add it as a configurable extension? Posting my thoughts here in case someone has any thoughts or cool ideas here 😄

RenFraser commented 1 year ago

Using detekt would be a great idea @themkat. We could always have it configured to exclude style (and similar types) warnings. This would just leave us with analysis of the code base. Things like performance (not using the spread operator) and potential-bugs rules (like unreachable code or UnnecessaryNotNullCheck) would be a huge win!

I feel like this would help beef up the language server and provide users with a bit more confidence in its analysis. So it might be a good way to drive adoption and therefore better feedback, more contributors/maintainers and so on! :)

themkat commented 1 year ago

Using detekt would be a great idea @themkat. We could always have it configured to exclude style (and similar types) warnings. This would just leave us with analysis of the code base. Things like performance (not using the spread operator) and potential-bugs rules (like unreachable code or UnnecessaryNotNullCheck) would be a huge win!

I feel like this would help beef up the language server and provide users with a bit more confidence in its analysis. So it might be a good way to drive adoption and therefore better feedback, more contributors/maintainers and so on! :)

Great to see that someone else liked the idea 😄 I was a little bit unsure if it would be too heavy to introduce a tool like Detekt into the codebase. Maybe it was just my persistent issue of second-guessing myself 🙁 Will try to get my small Detekt Diagnostic demo PR out in the coming weeks 🙂

RenFraser commented 1 year ago

Will try to get my small Detekt Diagnostic demo PR out

Sounds great. Could you mention me it in it so that I get a notification? I'd love to see how that sort of thing is implemented to get my head around the code base.

fwcd commented 10 months ago

Like the idea! Some thoughts: