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.

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

Use eclipse gradle plugin to get classpath with sources #439

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aiguofer
Copy link

I had originally created #410. The ability to navigate to sources is pretty crucial for me. This PR uses the built in Eclipse Gradle plugin to generate the Classpath with sources. The plugin creates a .classpath file, which we read and parse to generate the set of ClassPathEntry for Gradle.

This PR would still need some cleanup, but figured I'd put it out to see what others think of it.

There is still an issue when you navigate to a source where it tries to load it as a kotlin script and fails with something like:

Internal error: java.lang.IllegalArgumentException: Unable to find script compilation configuration for the script KtFile: /var/folders/6x/nmdbvc9d15d93v99zh7nqgym0000gp/T/kotlinlangserver14114182911595465166/ElementList8841194890478318470.java

@fwcd
Copy link
Owner

fwcd commented Mar 11, 2023

Thanks for looking into this! That's a neat idea, though generating files in the user's project directory is something I'm trying to use sparingly. Ideally we could use the Gradle tooling API to get the dependencies from Gradle directly, see

The tooling API offers an EclipseModel which effectively should provide the same information that is generated for the .classpath.

Copy link
Contributor

@RenFraser RenFraser left a comment

Choose a reason for hiding this comment

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

Neat idea! Left a comment :)

shared/build.gradle.kts Outdated Show resolved Hide resolved
@aiguofer
Copy link
Author

@fwcd thanks for the link and context! I agree with your assessment on the other thread... it seems like a lot of work to create a whole new IDE plugin for LSP. I imagine most of what is needed to generate the classpath can be obtained from the EclipseModel or EclipseProject.

I did think about the files in the user project, which is why I delete the .classpath file at the end of the operation. That being said, it might be more performant if we skip the whole xml serialization/deserialization. I'll see if I can modify this to create a task that instantiates an EclipseProject and prints out the dependencies similar to how it's currently done.

@aiguofer
Copy link
Author

aiguofer commented Mar 15, 2023

@fwcd figured out how to use the EclipseModel directly. Since some of the dependencies might be resolved more than once with different methods, I first build a map of path -> source (and prefer entries with source) then print out the individual items.

There are 2 major problems I'm still having (happy to create issues if needed) with go-to-definition. I'm currentyl using {"externalSources":{"autoConvertToKotlin":false,"useKlsScheme":false}} (with useKlsScheme=true none of the go-to-def work, and autoConvertToKotlin=true fails for most files):

  • Go-to-definition isn't working for external kotlin libraries:
async2    Go-to-definition at .../flight/SlFlightServer.kt 36:38
async2    Found declaration descriptor public final fun logger(func: () -> kotlin.Unit): mu.KLogger defined in mu.KotlinLogging[DeserializedSimpleFunctionDescriptor@686ee9]
async2    Couldn't find definition at .../flight/SlFlightServer.kt 36:38
  • When I go to a definition of an external library, the file that gets opened isn't considered part of the project, so I can't go to definitions from that file. Based on the logs, my guess is it's trying to parse it as a Kotlin script file:
Internal error: java.lang.IllegalArgumentException: Unable to find script compilation configuration for the script KtFile: /var/folders/6x/nmdbvc9d15d93v99zh7nqgym0000gp/T/kotlinlangserver10938469402867462880/JsonProperty1664481689349846736.java

@fwcd
Copy link
Owner

fwcd commented Mar 15, 2023

Nice! What I had in mind would be to go a step further and resolve the EclipseModel directly from within the language server (ideally so we could drop the xyzClasspathFinder.gradle eventually). This would require using the Gradle tooling API.

@aiguofer
Copy link
Author

@fwcd so I've been playing around a bit with the Tooling API. It's pretty straightforward to get the EclipseProject and the classes from that, but I haven't found how to get project.android, project.sourceSets, or the kotlin extension. We might need to include the jars for those plugins explicitly and figure out how gradle resolves those internally.

Here's a function showing how to get the sources using the tooling api:

    private fun getDepsWithSources(): Set<ClassPathEntry> {
        val connector = GradleConnector.newConnector().forProjectDirectory(projectDirectory.toFile())
        val classpaths = mutableSetOf<ClassPathEntry>()

        connector.connect().use { connection ->
            try {
                val eclipseModel = connection.getModel(EclipseProject::class.java)
                classpaths.addAll(
                    eclipseModel.classpath.map {
                        LOG.info { "Adding path from tools API: ${it.file.path}" }
                        ClassPathEntry(it.file.toPath(), it.file?.toPath()) }.toSet()
                )
            } catch(e: Exception) {
                LOG.error("Error: $e")
            }
        }
        return classpaths
    }

@Strum355
Copy link
Contributor

Its been a 2+ years since I last used the eclipse model, but I remember back when using it with https://github.com/sourcegraph/scip-java that it was pretty suboptimal and often missed things when tested across a number of repositories across github (at one point i took the union of the eclipse + IDEA models + an init script from this repo as they both missed some that the other didnt). We're currently using this custom task, id be interested in knowing whether you know of how this compares to it off-hand (from my experience it worked better than the eclipse model, but again its been a long while so my memory may be very fuzzy)

@aiguofer
Copy link
Author

@Strum355 Interesting! honestly I have no idea how it compares. I've just started using Kotlin at work in the past 6 months and hate using IntelliJ; I've been trying to use Emacs but I can't seem to get anywhere near the ability to explore dependencies using the language server so I figured I'd take a stab at this. I don't really get much time to mess around with it though.

I've been trying to find com.sourcegraph.gradle.semanticdb.SemanticdbGradlePlugin but can't find the code for it... what exactly is that doing?

Do you think it would make sense for kotlin-language-server to use the same plugin? Alternatively, do you think the approach from https://github.com/sourcegraph/scip-java/pull/74/files worked well?

@@ -19,15 +19,18 @@ internal class GradleClassPathResolver(private val path: Path, private val inclu

Copy link
Contributor

@RenFraser RenFraser May 17, 2023

Choose a reason for hiding this comment

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

Rather than building string commands, invoking the build and parsing the string output, line by line, I think that this whole file could just become a single call to the classpath, using the Eclipse model from the Gradle project connection. There's more information on the third party tooling web doc. They say that to find examples of calling Gradle's API from Java, you need to go through the JavaDoc, starting at the GradleConnector.

This could simplify the class a fair bit and improve the time it takes for the server to spin up. What do you think @aiguofer?

package org.javacs.kt.classpath

import org.gradle.tooling.GradleConnector
import org.gradle.tooling.model.eclipse.EclipseProject
import java.io.File
import java.nio.file.Path

internal class GradleClassPathResolver(private val path: Path) : ClassPathResolver {
    override val resolverType: String = "Gradle"
    private val projectDirectory: Path get() = path.parent
    override val classpath: Set<ClassPathEntry> = GradleConnector
        .newConnector()
        .forProjectDirectory(File(projectDirectory.toString()))
        .connect()
        .use { connector ->
            val model = connector.getModel(EclipseProject::class.java)
            model.classpath.map {
                ClassPathEntry(
                    compiledJar = it.source.toPath(),
                    sourceJar = null,
                )
            }.toSet()
        }
}

cc @themkat

Copy link
Author

Choose a reason for hiding this comment

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

The main issue is that apparently the EclipseProject doesn't catch all the dependencies, especially for android dependencies. @Strum355 mentioned above that for SCIP they had to use a combination of EclipseProject, IdeaProject, and a gradle task that also builds some strings and passes them in to be parsed. I currently have a locally working branch that does it similarly, but the gradle task with string parsing definitely makes it slower.

When I have some time I'll try to make some comparisons of the outputs of each.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds great, thanks @aiguofer! Strum's comment now makes much more sense!

@Strum355
Copy link
Contributor

@Strum355 Interesting! honestly I have no idea how it compares. I've just started using Kotlin at work in the past 6 months and hate using IntelliJ; I've been trying to use Emacs but I can't seem to get anywhere near the ability to explore dependencies using the language server so I figured I'd take a stab at this. I don't really get much time to mess around with it though.

I've been trying to find com.sourcegraph.gradle.semanticdb.SemanticdbGradlePlugin but can't find the code for it... what exactly is that doing?

I should've pinned the link to a specific commit, our gradle work just underwent some changes that Im not a part of 😬 we're not using init scripts anymore, but rather a gradle plugin itself
From what I can grok, this is the gradle task that we now use to gather the set of dependencies. I hope your scala-fu is up to scratch 😅 (mine isnt) We abandoned EclipseProject and IdeaProject long ago fwiw
I dont believe we have explicit tested support for Android as well, which may throw a spanner in the works here

@aiguofer
Copy link
Author

ahh thanks @Strum355! One of the issues is that we're trying to get the source jars to add to the classpath entries, but only the EclipseProject and IdeaProject resolve those AFAIK.

@fwcd fwcd added the gradle Related to the language server's support for Gradle projects label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gradle Related to the language server's support for Gradle projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants