Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Workspace symbols for JAX-RS endpoints #313

Merged
merged 1 commit into from
Jan 26, 2023
Merged

Workspace symbols for JAX-RS endpoints #313

merged 1 commit into from
Jan 26, 2023

Conversation

datho7561
Copy link
Contributor

Closes #87

Signed-off-by: David Thompson [email protected]

@datho7561
Copy link
Contributor Author

I think that Jessica's PR should be merged before mine so that I can use some of the methods she introduces or modifies in JaxRsConstants and JaxRsUtils.

@datho7561 datho7561 mentioned this pull request Nov 10, 2022
Closed
@angelozerr
Copy link
Contributor

@datho7561 could you add a little demo please in your PR, just to have an overview of your PR which looks exciting. Thanks!

@datho7561
Copy link
Contributor Author

I need to do some work on this PR; it only seems to work if one of the files in the project is open.

@datho7561
Copy link
Contributor Author

Here is a demo. I use the key combo Ctrl+T to open the workspace symbols view.

lsp4mp-workspace-symbols

@angelozerr
Copy link
Contributor

Nice! It looks like promising!

@datho7561
Copy link
Contributor Author

I need to do some work on this PR; it only seems to work if one of the files in the project is open.

It looks like this is a limitation of how lsp4mp handles projects. I'll take a look into fixing it.

@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch from fb332dd to 049750b Compare November 14, 2022 17:30
@datho7561
Copy link
Contributor Author

It looks like this is a limitation of how lsp4mp handles projects. I'll take a look into fixing it.

Alright. Now it should make sure all the projects in the workspace have been loaded before collecting symbols. Performance is okay, but not great.

@datho7561 datho7561 mentioned this pull request Nov 14, 2022
Merged
@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch from 049750b to ddc4fe7 Compare November 15, 2022 14:37
@datho7561
Copy link
Contributor Author

datho7561 commented Nov 15, 2022 •
edited
Loading

I'm running into a ClassCastException when invoking the JDT search engine when I try to run the completed Quarkus super heros demo application:

java.lang.ClassCastException: class org.eclipse.jdt.internal.core.JavaElementInfo cannot be cast to class org.eclipse.jdt.internal.core.ClassFileInfo (org.eclipse.jdt.internal.core.JavaElementInfo and org.eclipse.jdt.internal.core.ClassFileInfo are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @6feeb453)
	at org.eclipse.jdt.internal.core.ModularClassFile.getModule(ModularClassFile.java:301)
	at org.eclipse.jdt.internal.core.PackageFragmentRoot.getSourceModuleDescription(PackageFragmentRoot.java:928)
	at org.eclipse.jdt.internal.core.PackageFragmentRoot.getModuleDescription(PackageFragmentRoot.java:904)
	at org.eclipse.jdt.internal.core.JarPackageFragmentRoot.getModuleDescription(JarPackageFragmentRoot.java:307)
	at org.eclipse.jdt.internal.core.search.matching.JavaSearchNameEnvironment.addModuleClassPathInfo(JavaSearchNameEnvironment.java:290)
	at org.eclipse.jdt.internal.core.search.matching.JavaSearchNameEnvironment.mapToClassPathLocation(JavaSearchNameEnvironment.java:284)
	at org.eclipse.jdt.internal.core.search.matching.JavaSearchNameEnvironment.computeClasspathLocations(JavaSearchNameEnvironment.java:197)
	at org.eclipse.jdt.internal.core.search.matching.JavaSearchNameEnvironment.<init>(JavaSearchNameEnvironment.java:91)
	at org.eclipse.jdt.internal.core.search.matching.IndexBasedJavaSearchEnvironment.create(IndexBasedJavaSearchEnvironment.java:27)
	at org.eclipse.jdt.internal.core.search.matching.MatchLocator.initialize(MatchLocator.java:1249)
	at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1287)
	at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1400)
	at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1542)
	at org.eclipse.jdt.internal.core.search.JavaSearchParticipant.locateMatches(JavaSearchParticipant.java:135)
	at org.eclipse.jdt.internal.core.search.BasicSearchEngine.findMatches(BasicSearchEngine.java:251)
	at org.eclipse.jdt.internal.core.search.BasicSearchEngine.search(BasicSearchEngine.java:602)
	at org.eclipse.jdt.core.search.SearchEngine.search(SearchEngine.java:670)
	at org.eclipse.lsp4mp.jdt.internal.jaxrs.java.JaxRsWorkspaceSymbolParticipant.collectSymbols(JaxRsWorkspaceSymbolParticipant.java:83)
	at org.eclipse.lsp4mp.jdt.internal.core.ls.MicroProfileDelegateCommandHandlerForJava.getWorkspaceSymbolsForJava(MicroProfileDelegateCommandHandlerForJava.java:486)
	at org.eclipse.lsp4mp.jdt.internal.core.ls.MicroProfileDelegateCommandHandlerForJava.executeCommand(MicroProfileDelegateCommandHandlerForJava.java:107)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler$1.run(WorkspaceExecuteCommandHandler.java:230)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45)
	at org.eclipse.jdt.ls.core.internal.handlers.WorkspaceExecuteCommandHandler.executeCommand(WorkspaceExecuteCommandHandler.java:220)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$4(JDTLanguageServer.java:552)
	at org.eclipse.jdt.ls.core.internal.BaseJDTLanguageServer.lambda$0(BaseJDTLanguageServer.java:79)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(Unknown Source)
	at java.base/java.util.concurrent.CompletableFuture$Completion.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)

Not sure if this is an upstream issue or something related to the parameters I passed to the search engine.

Update: I think this might be upstream, I'll file this in jdt.core

@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch 3 times, most recently from de30654 to c7d7056 Compare November 15, 2022 20:32
@datho7561 datho7561 marked this pull request as ready for review November 15, 2022 20:32
@datho7561 datho7561 mentioned this pull request Nov 16, 2022
Merged
@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch from c7d7056 to 3ada9c8 Compare December 14, 2022 15:04
@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch from 3ada9c8 to f4ebe98 Compare January 4, 2023 17:17
@datho7561 datho7561 requested a review from angelozerr January 18, 2023 13:44
@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch from f4ebe98 to bd5d732 Compare January 18, 2023 16:52
@datho7561 datho7561 requested a review from angelozerr January 18, 2023 16:52
@angelozerr
Copy link
Contributor

@datho7561 please fix conflicts.

@datho7561 datho7561 force-pushed the 87-endpoint-workspace-symbols branch from bd5d732 to adedf4d Compare January 23, 2023 15:21
@datho7561 datho7561 mentioned this pull request Jan 23, 2023
Closed
@datho7561
Copy link
Contributor Author

I can't reproduce the test failure

@datho7561
Copy link
Contributor Author

test this please

@datho7561
Copy link
Contributor Author

darn, looks like that's not set up on this repo

@datho7561 datho7561 requested a review from angelozerr January 24, 2023 16:22
angelozerr
angelozerr reviewed Jan 26, 2023
View reviewed changes
...rc/main/java/org/eclipse/lsp4mp/jdt/internal/jaxrs/java/JaxRsWorkspaceSymbolParticipant.java
return;
}

String endpointUri = getEndpointUriFromAnnotatable(annotatable, applicationPrefix);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to play with your PR with https://github.com/FroMage/quarkus-renarde-todo/blob/main/src/main/java/rest/Todos.java and it doesn't work.

It seems it is because methods doesn't use @path although the method is annotated with @post. If there are no @path, we should use the method name if I remember.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok let's see that in an another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at how Renard figures out which methods are REST endpoints, and it looks like any public method in a class that implements io.quarkiverse.renarde.Controller counts as a REST endpoint. Generally, the logic for calculating the method name looks different enough that I think it's worthwhile to provide a mechanism for quarkus-ls to disable the default WorkspaceSymbol support when it detects Renarde, and instead use the Renard support contributed through quarkus-ls.

@angelozerr angelozerr merged commit c51d1ad into eclipse-lsp4mp:master Jan 26, 2023
@angelozerr
Copy link
Contributor

Thank so much @datho7561 for your great contribution!

@datho7561 datho7561 deleted the 87-endpoint-workspace-symbols branch January 26, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angelozerr angelozerr angelozerr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Navigation code with MicroProfile (WorkspaceSymbol)
2 participants
@datho7561 @angelozerr