Skip to content

Improve handling of moniker import/exports and the JDK#227

Merged
olafurpg merged 2 commits into
scip-code:mainfrom
olafurpg:jdk-first
May 19, 2021
Merged

Improve handling of moniker import/exports and the JDK#227
olafurpg merged 2 commits into
scip-code:mainfrom
olafurpg:jdk-first

Conversation

@olafurpg

@olafurpg olafurpg commented May 19, 2021

Copy link
Copy Markdown
Contributor

See individual commits for details. Related issue https://github.com/sourcegraph/sourcegraph/issues/21058

olafurpg added 2 commits May 19, 2021 16:13
Previously, when a Maven package re-defined a class from the JDK then
lsif-java would emit an export against the JDK instead of the Maven
package. This caused problems with navigation to the JDK on Sourcegraph
because there were multiple repositories that exported monikers against
the `jdk/8` packageInformation node. Now, only the JDK repo should
export monikers for the JDK repo.
Previously, there was a small risk that lsif-java would export symbols
against the JDK when indexing non-JDK packages like
com.google.gwt:gwt-user:2.9.0, which contain source files for
`java.lang.String` but don't define `java/lang/String.class`.

This commit, adds a special case for the JDK so that monikers for JDK
symbols are always imported when indexing non-JDK repos.
@Strum355

Copy link
Copy Markdown
Contributor

Just so I understand correctly, will this mean that definitions of jdk classes in non jdk repos will have an import moniker attached to them instead of an export moniker? How does that behaviour work in sourcegraph? Im assuming go-to-def on the "shadowed" definition will bring you to the jdk definition instead, which would seem reasonable, and go-to-def on references to the shadowed symbol will also bring you to the jdk definition instead? The only way to find the shadowed definitions then are via find references. Does this sound right?

@olafurpg

Copy link
Copy Markdown
Contributor Author

I haven't validated the answers below but will answer with my intuition about the expected behavior.

Im assuming go-to-def on the "shadowed" definition will bring you to the jdk definition instead, which would seem reasonable, and go-to-def on references to the shadowed symbol will also bring you to the jdk definition instead?

Correct, that's my intuition as well.

The only way to find the shadowed definitions then are via find references. Does this sound right?

Correct, the shadow definition will only appear as a reference.

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