-
Notifications
You must be signed in to change notification settings - Fork 29
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
Kotlin find references shows refs for wrong symbol #678
Comments
Another instance of references for the wrong symbol showing up: https://sourcegraph.com/github.com/sourcegraph/jetbrains@e6c887b12291c31bcd23210409db87affa8ecc6d/-/blob/src/main/kotlin/com/sourcegraph/cody/UpdatableChat.kt?L6:11-6:24#tab=references |
Here, the problem is that we're emitting However, that's incorrect, we should only be emitting an This is covered in the SCIP documentation. https://github.com/sourcegraph/scip/blob/main/scip.proto#L439-L472 // When resolving "Find references", this field documents what other symbols
// should be included together with this symbol. For example, consider the
// following TypeScript code that defines two symbols `Animal#sound()` and
// `Dog#sound()`:
// ```ts
// interface Animal {
// ^^^^^^ definition Animal#
// sound(): string
// ^^^^^ definition Animal#sound()
// }
// class Dog implements Animal {
// ^^^ definition Dog#, relationships = [{symbol: "Animal#", is_implementation: true}]
// public sound(): string { return "woof" }
// ^^^^^ definition Dog#sound(), references_symbols = Animal#sound(), relationships = [{symbol: "Animal#sound()", is_implementation:true, is_reference: true}]
// }
// const animal: Animal = new Dog()
// ^^^^^^ reference Animal#
// console.log(animal.sound())
// ^^^^^ reference Animal#sound()
// ```
// Doing "Find references" on the symbol `Animal#sound()` should return
// references to the `Dog#sound()` method as well. Vice-versa, doing "Find
// references" on the `Dog#sound()` method should include references to the
// `Animal#sound()` method as well.
bool is_reference = 2;
// Similar to `is_reference` but for "Find implementations".
// It's common for `is_implementation` and `is_reference` to both be true but
// it's not always the case.
// In the TypeScript example above, observe that `Dog#` has an
// `is_implementation` relationship with `"Animal#"` but not `is_reference`.
// This is because "Find references" on the "Animal#" symbol should not return
// "Dog#". We only want "Dog#" to return as a result for "Find
// implementations" on the "Animal#" symbol.
bool is_implementation = 3; Maybe all that is needed is to add a call to |
@varungandhi-src the root bug seems to be that semanticdb-kotlinc is setting Alternatively, we can update scip-java to add an extra case for |
Fixes #678 Previously, doing "Find references" on a symbol with kind `INTERFACE` or `TYPE` returned results for all references to symbols that *implement* that symbol. This bug was particularly noticeable for Kotlin sources because scip-kotlin emits the `TYPE` kind for classes. Now, we no longer emit reference relationships so the implementations only appear in "Find implementation" and not "Find references".
Used the opportunity while fixing Kotlin 1.9 support to also fix this bug #682 |
I'm trying to confirm 100% that this bug has been fixed but now I'm hitting on another bug in our code search UI. Reported here https://sourcegraph.slack.com/archives/C05MHAP318B/p1706882647263079 I'm not able to manually upload the SCIP index anymore because I don't have site-admin access. Either way, we should be able to confirm it's fixed as soon as the |
Re-opening since this was reported again in Slack. The commit for the index is quite new, so either auto-indexing is not picking up the version of the scip-java with this fix, or the bug is not fully fixed. |
Steps to reproduce:
JLayeredPane
andComponent
The text was updated successfully, but these errors were encountered: