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

Partially precise result on a Scala codebase #482

Open
kubukoz opened this issue Aug 16, 2022 · 2 comments
Open

Partially precise result on a Scala codebase #482

kubukoz opened this issue Aug 16, 2022 · 2 comments

Comments

@kubukoz
Copy link

kubukoz commented Aug 16, 2022

Hi! First of all, thanks for building great tools.

I'm trying out a private Sourcegraph instance. Ran an indexing round on a branch and now some of my definition searches look like this:

image

The definition of AccountId in question is in the same sbt module as the code I'm looking at. However, it still shows up as partially precise.

In the Definition view, all results I see are search-based:

image

Could this be a configuration issue (I'm on all the defaults when it comes to generating the index) or is something wrong with scip-java?

@olafurpg olafurpg self-assigned this Aug 17, 2022
@olafurpg
Copy link
Member

Thank you for reporting! I'm able to reproduce. A workaround is to include the argument --output=dump.lsif in the command

scip-java index --output=dump.lsif

We haven't fully reached feature parity when using the new SCIP format when it comes to dealing with situations where multiple symbols are defined in the same file (which happens for case classes). It will be necessary to use --output=dump.lsif for Scala codebases for at least a few more weeks until I can get back to closing the gap between our LSIF and SCIP support.

@ckipp01
Copy link
Contributor

ckipp01 commented Aug 24, 2022

Thank you for reporting! I'm able to reproduce. A workaround is to include the argument --output=dump.lsif in the command

scip-java index --output=dump.lsif

We haven't fully reached feature parity when using the new SCIP format when it comes to dealing with situations where multiple symbols are defined in the same file (which happens for case classes). It will be necessary to use --output=dump.lsif for Scala codebases for at least a few more weeks until I can get back to closing the gap between our LSIF and SCIP support.

I was just looking at this, and from looking at the code, I don't believe this is supported via scip-java. It doesn't look like it passes the --output to the sbt plugin at all, and index.scip is hardcoded:

private def unconditionallyGenerateScip(): Int =
Using.resource(sourcegraphSbtPluginFile()) { _ =>
val sourcegraphScip = index
.process(List("sbt", "sourcegraphEnable", "sourcegraphScip"))
val inputDump = index
.workingDirectory
.resolve("target")
.resolve("sbt-sourcegraph")
.resolve("index.scip")
if (sourcegraphScip.exitCode == 0 && Files.isRegularFile(inputDump)) {
val outputDump = index.workingDirectory.resolve("index.scip")
Files.copy(inputDump, outputDump, StandardCopyOption.REPLACE_EXISTING)
index.app.info(outputDump.toString)
}
sourcegraphScip.exitCode
}

Same with the sbt plugin actually:

https://github.com/sourcegraph/sbt-sourcegraph/blob/860f088b2cd041665996f790c6469ac4e6ee8206/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphPlugin.scala#L118-L134

It looks like a workaround would be to use index-semanticdb --output=dump.lsif but that would require semanticDB to already be present.

ckipp01 added a commit to ckipp01/mill-scip that referenced this issue Aug 24, 2022
This allows the user to pass in `--output dump.lsif` or just `dump.lsif`
or another valid output type to output other types than the default scip
which was happening before. If used the same way as before with no
argument the behavior is the same.

Refs: sourcegraph/scip-java#482
ckipp01 added a commit to ckipp01/scip-java that referenced this issue Aug 25, 2022
This ensures that when a user uses `scip-java` with the output flag
like:
```
scip-java index --output dump.lsif
```
Then this gets forwarded to Mill to ensure it creates an lsif file not a
scip one.

refs: sourcegraph#482
@olafurpg olafurpg removed their assignment May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants