Skip to content

Implement abstract function - Extend to support external methods (Java standard library etc.) #357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 1, 2022

Conversation

themkat
Copy link
Collaborator

@themkat themkat commented Apr 26, 2022

General information

The current implementation only supports extending classes or implementing interfaces in the same project it seems. I often work with Java libraries where I have to implement functions from abstract classes or interfaces. This can be very tedious to do manually, so having it as a quickfix is really beneficial. The current implementation is based on KtClass, which to my understanding is only made for Kotlin classes. Java classes have their own PSI elements in the compiler. In the current implementation, we are also shown that a quickfix exists, but the edit-parts of it are empty/null.

The new implementation works with the DeclarationDescriptor objects directly, which seems like a very flexible approach.

Quirks

I originally did not see the QuickFixesTests file, so I originally had a different file with tests. That is why it seems like there is some overlap between some of the tests. I later merged them, but decided to keep the tests. I think the set of tests verifies a lot of different things; same file definitions, workspace definitions in other files, and Java standard library ones. Let me know if I instead should delete some of them.

I blame late evening coding after a beer for not noticing the QuickFixesTests file 😆

Future work

There are some useful things that should be added to this functionality. To not make this PR too big, I think it is better to have them as part of a future PR. Will Make some issues for them so people can pick them up if they want to (or I end up doing it when I have time after work).

@themkat
Copy link
Collaborator Author

themkat commented May 23, 2022

@fwcd , do you have time to look at this PR in the near future? Would be really awesome to have as part of the language server 🙂 Sorry for nagging you about this 🙁

@fwcd
Copy link
Owner

fwcd commented Jun 1, 2022

Sorry for the delay. Great work, thanks for digging into this, definitely very useful considering how interfaces/base classes often come from libraries!

An idea that I had was that it would be cool to have auto-completions for missing overridable methods, similar to how the Java/Python/Rust/etc language servers handle this, but definitely that is out-of-scope for this PR.

@fwcd fwcd merged commit a4c53b6 into fwcd:main Jun 1, 2022
@themkat themkat deleted the implement_external_abstract_member branch June 1, 2022 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants