Skip to content

Commit 5b8d172

Browse files
olafurpgolafurpg
committed
Fix code navigation for synthetic Scala symbols (including case classes)
Previously, doing "find references" on case classes showed usages of all case classes because lsif-java emitted `next` edges for synthetic symbols. Now, synthetic symbols have asymmetric navigation, goto definition works from call-site but "find references" does not work at the definition site.
1 parent 923efaf commit 5b8d172

File tree

18 files changed

+183
-137
lines changed

18 files changed

+183
-137
lines changed

‎lsif-java/src/main/scala/com/sourcegraph/lsif_java/SemanticdbPrinters.scala‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import scala.jdk.CollectionConverters._
44

55
import com.sourcegraph.lsif_java.commands.CommentSyntax
66
import com.sourcegraph.lsif_java.commands.SnapshotLsifCommand
7+
import com.sourcegraph.lsif_semanticdb.LsifSemanticdb
78
import com.sourcegraph.lsif_semanticdb.LsifTextDocument
89
import com.sourcegraph.lsif_semanticdb.SignatureFormatter
910
import com.sourcegraph.lsif_semanticdb.Symtab
1011
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolOccurrence
11-
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolOccurrence.Role
1212
import com.sourcegraph.semanticdb_javac.Semanticdb.TextDocument
1313

1414
object SemanticdbPrinters {
@@ -17,9 +17,7 @@ object SemanticdbPrinters {
1717
comments: CommentSyntax = CommentSyntax.default
1818
): String = {
1919
val occurrencesByLine = LsifTextDocument
20-
.sortedSymbolOccurrences(
21-
LsifTextDocument.manifestOccurrencesForSyntheticSymbols(doc)
22-
)
20+
.sortedSymbolOccurrences(doc)
2321
.asScala
2422
.groupBy(_.getRange.getStartLine)
2523
val out = new StringBuilder()
@@ -90,7 +88,7 @@ object SemanticdbPrinters {
9088
)
9189
.append(
9290
symtab.symbols.asScala.get(occ.getSymbol) match {
93-
case Some(info) if occ.getRole == Role.DEFINITION =>
91+
case Some(info) if LsifSemanticdb.isDefinitionRole(occ.getRole) =>
9492
val signature: String =
9593
if (info.hasSignature) {
9694
new SignatureFormatter(info, symtab)

‎lsif-java/src/main/scala/com/sourcegraph/lsif_java/commands/SnapshotLsifCommand.scala‎

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import java.nio.charset.StandardCharsets
55
import java.nio.file.Files
66
import java.nio.file.Path
77
import java.nio.file.Paths
8+
import java.util.concurrent.atomic.AtomicInteger
89
import java.util.stream.Collectors
910
import java.util.stream.Stream
1011

@@ -118,17 +119,23 @@ object SnapshotLsifCommand {
118119
.predecessors(o)
119120
.asScala
120121
.exists(_.getLabel == "definitionResult")
122+
val isSyntheticDefinition = isDefinition && !lsif.next.contains(o.getId)
121123
val role =
122-
if (isDefinition)
124+
if (isSyntheticDefinition)
125+
Role.SYNTHETIC_DEFINITION
126+
else if (isDefinition)
123127
Role.DEFINITION
124128
else
125129
Role.REFERENCE
126130

127131
val symbol = lsif
128132
.symbolFromRange(o)
133+
.orElse(lsif.monikerViaDefinition(o))
129134
.getOrElse {
130-
val id = lsif.next.getOrElse(o.getId, o.getId)
131-
s"missingMoniker$id"
135+
val id = lsif
136+
.next
137+
.getOrElse(o.getId, lsif.nextMissingMonikerId(doc.getUri))
138+
s"localMissingMoniker$id"
132139
}
133140
val occ = SymbolOccurrence
134141
.newBuilder()
@@ -147,15 +154,7 @@ object SnapshotLsifCommand {
147154
doc.addOccurrences(occ)
148155

149156
if (isDefinition) {
150-
val hover =
151-
(
152-
for {
153-
resultSetId <- lsif.next.get(o.getId).toList
154-
hoverId <- lsif.hoverEdges.get(resultSetId).toList
155-
hover <- lsif.hoverVertexes.get(hoverId).toList
156-
line <- signatureLines(hover.getContents.getValue)
157-
} yield line
158-
).mkString("\n")
157+
val hover = lsif.hoverViaDefinition(o)
159158
val symInfo = SymbolInformation
160159
.newBuilder()
161160
// we cheese it a bit here, as this is less work than trying to reconstruct
@@ -184,6 +183,11 @@ object SnapshotLsifCommand {
184183
) {
185184
val documents = mutable.Map.empty[Int, TextDocument.Builder]
186185
val next = mutable.Map.empty[Int, Int]
186+
private val missingMonikerCounter = mutable.Map.empty[String, AtomicInteger]
187+
def nextMissingMonikerId(uri: String) =
188+
missingMonikerCounter
189+
.getOrElseUpdate(uri, new AtomicInteger())
190+
.incrementAndGet()
187191
val monikerIdentifier = mutable.Map.empty[Int, String]
188192
val moniker = mutable.Map.empty[Int, Int]
189193
val monikerInverse = mutable.Map.empty[Int, Int]
@@ -378,6 +382,35 @@ object SnapshotLsifCommand {
378382
S.build()
379383
}
380384

385+
def definitioResultSet(node: LsifObject): Option[LsifObject] = {
386+
for {
387+
definitionResult <- G.predecessors(node).asScala
388+
if definitionResult.getLabel == "definitionResult"
389+
resultSet <- G.predecessors(definitionResult).asScala
390+
if resultSet.getLabel == "resultSet"
391+
} yield resultSet
392+
}.headOption
393+
394+
def monikerViaDefinition(node: LsifObject): Option[String] = {
395+
for {
396+
resultSet <- definitioResultSet(node).toList
397+
moniker <- G.successors(resultSet).asScala
398+
if moniker.getLabel == "moniker"
399+
} yield moniker.getIdentifier
400+
}.headOption
401+
402+
def hoverViaDefinition(node: LsifObject): String = {
403+
for {
404+
resultSet <- definitioResultSet(node).toList
405+
hover <-
406+
G.successors(resultSet)
407+
.asScala
408+
.find(_.getLabel == "hoverResult")
409+
.toList
410+
line <- signatureLines(hover.getResult.getContents.getValue)
411+
} yield line
412+
}.mkString("\n")
413+
381414
val byId = objects.iterator.map(o => o.getId -> o).toMap
382415
val byType = objects.groupBy(_.getType)
383416
val byLabel = objects.groupBy(_.getLabel)

‎lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdb.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ private String typedSymbol(String symbol, Package pkg) {
7474
return "semanticdb maven " + pkg.repoName() + " " + pkg.version() + " " + symbol;
7575
}
7676

77+
public static boolean isDefinitionRole(Role role) {
78+
return role == Role.DEFINITION || role == Role.SYNTHETIC_DEFINITION;
79+
}
80+
7781
private void processTypedDocument(Path path, PackageTable packages) {
7882
for (LsifTextDocument doc : parseTextDocument(path).collect(Collectors.toList())) {
7983
if (doc.semanticdb.getOccurrencesCount() == 0) {
@@ -89,7 +93,7 @@ private void processTypedDocument(Path path, PackageTable packages) {
8993
LsifTyped.Document.newBuilder().setRelativePath(relativePath);
9094
for (SymbolOccurrence occ : doc.sortedSymbolOccurrences()) {
9195
int role = 0;
92-
if (occ.getRole() == Role.DEFINITION) {
96+
if (isDefinitionRole(occ.getRole())) {
9397
role |= LsifTyped.SymbolRole.Definition_VALUE;
9498
}
9599
boolean isSingleLineRange = occ.getRange().getStartLine() == occ.getRange().getEndLine();
@@ -182,7 +186,7 @@ private Set<String> exportSymbols(List<Path> files) {
182186
.flatMap(
183187
d ->
184188
d.semanticdb.getOccurrencesList().stream()
185-
.filter(occ -> occ.getRole() == Role.DEFINITION)
189+
.filter(occ -> isDefinitionRole(occ.getRole()))
186190
.map(SymbolOccurrence::getSymbol)
187191
.filter(SemanticdbSymbols::isGlobal))
188192
.collect(Collectors.toSet());
@@ -212,7 +216,7 @@ private Integer processDocumentUnsafe(
212216
doc.semanticdb.getOccurrencesList().stream()
213217
.filter(
214218
occ ->
215-
occ.getRole() == Role.DEFINITION && SemanticdbSymbols.isLocal(occ.getSymbol()))
219+
isDefinitionRole(occ.getRole()) && SemanticdbSymbols.isLocal(occ.getSymbol()))
216220
.map(SymbolOccurrence::getSymbol)
217221
.collect(Collectors.toSet());
218222
doc.id = documentId;
@@ -228,13 +232,15 @@ private Integer processDocumentUnsafe(
228232
rangeIds.add(rangeId);
229233

230234
// Range
231-
writer.emitNext(rangeId, ids.resultSet);
235+
if (occ.getRole() != Role.SYNTHETIC_DEFINITION) {
236+
writer.emitNext(rangeId, ids.resultSet);
237+
}
232238

233239
// Reference
234240
writer.emitItem(ids.referenceResult, rangeId, doc.id);
235241

236242
// Definition
237-
if (occ.getRole() == SymbolOccurrence.Role.DEFINITION) {
243+
if (isDefinitionRole(occ.getRole())) {
238244
if (ids.isDefinitionDefined()) {
239245
writer.emitItem(ids.definitionResult, rangeId, doc.id);
240246
} else {
@@ -277,7 +283,9 @@ private Integer processDocumentUnsafe(
277283
}
278284

279285
// Overrides
280-
if (symbolInformation.getOverriddenSymbolsCount() > 0) {
286+
if (symbolInformation.getOverriddenSymbolsCount() > 0
287+
&& SemanticdbSymbols.isMethod(symbolInformation.getSymbol())
288+
&& occ.getRole() == Role.DEFINITION) {
281289
List<Integer> overriddenReferenceResultIds =
282290
new ArrayList<>(symbolInformation.getOverriddenSymbolsCount());
283291
for (int i = 0; i < symbolInformation.getOverriddenSymbolsCount(); i++) {

‎lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifTextDocument.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ public static Semanticdb.TextDocument manifestOccurrencesForSyntheticSymbols(
104104
if (alternativeDefinition != null) {
105105
builder.addOccurrences(
106106
Semanticdb.SymbolOccurrence.newBuilder(alternativeDefinition)
107+
.setRole(Semanticdb.SymbolOccurrence.Role.SYNTHETIC_DEFINITION)
107108
.setSymbol(info.getSymbol()));
108109
break;
109110
}

‎semanticdb-java/src/main/protobuf/semanticdb.proto‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,10 @@ message SymbolOccurrence {
169169
UNKNOWN_ROLE = 0;
170170
REFERENCE = 1;
171171
DEFINITION = 2;
172+
// NOTE: this role does not exist in the upstream SemanticDB spec.
173+
// WE added SYNTHETIC_DEFINITION fix the following lsif-java issue:
174+
// https://github.com/sourcegraph/lsif-java/issues/399
175+
SYNTHETIC_DEFINITION = 3;
172176
}
173177
Range range = 1;
174178
string symbol = 2;

‎tests/snapshots/src/main/generated/BaseByteRenderer.scala‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import upickle.core.{ArrVisitor, ObjVisitor}
2323
*/
2424
class BaseByteRenderer[T <: upickle.core.ByteOps.Output]
2525
// ^^^^^^^^^^^^^^^^ definition ujson/BaseByteRenderer# class BaseByteRenderer[T <: Output]
26-
// ^^^^^^^^^^^^^^^^ definition ujson/BaseByteRenderer. object BaseByteRenderer
26+
// ^^^^^^^^^^^^^^^^ synthetic_definition ujson/BaseByteRenderer. object BaseByteRenderer
2727
// ^ definition ujson/BaseByteRenderer#[T] T <: Output
2828
// ^^^^^^^ reference upickle/
2929
// ^^^^ reference upickle/core/
@@ -66,13 +66,13 @@ class BaseByteRenderer[T <: upickle.core.ByteOps.Output]
6666

6767
private[this] var depth: Int = 0
6868
// ^^^^^ definition ujson/BaseByteRenderer#depth(). private[this] var depth: Int
69-
// ^^^^^ definition ujson/BaseByteRenderer#`depth_=`(). private[this] var depth_=(x$1: Int): Unit
69+
// ^^^^^ synthetic_definition ujson/BaseByteRenderer#`depth_=`(). private[this] var depth_=(x$1: Int): Unit
7070
// ^^^ reference scala/Int#
7171

7272

7373
private[this] var commaBuffered = false
7474
// ^^^^^^^^^^^^^ definition ujson/BaseByteRenderer#commaBuffered(). private[this] var commaBuffered: Boolean
75-
// ^^^^^^^^^^^^^ definition ujson/BaseByteRenderer#`commaBuffered_=`(). private[this] var commaBuffered_=(x$1: Boolean): Unit
75+
// ^^^^^^^^^^^^^ synthetic_definition ujson/BaseByteRenderer#`commaBuffered_=`(). private[this] var commaBuffered_=(x$1: Boolean): Unit
7676

7777
def flushBuffer() = {
7878
// ^^^^^^^^^^^ definition ujson/BaseByteRenderer#flushBuffer(). def flushBuffer(): Unit

‎tests/snapshots/src/main/generated/BaseCharRenderer.scala‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import upickle.core.{ArrVisitor, ObjVisitor}
2323
*/
2424
class BaseCharRenderer[T <: upickle.core.CharOps.Output]
2525
// ^^^^^^^^^^^^^^^^ definition ujson/BaseCharRenderer# class BaseCharRenderer[T <: Output]
26-
// ^^^^^^^^^^^^^^^^ definition ujson/BaseCharRenderer. object BaseCharRenderer
26+
// ^^^^^^^^^^^^^^^^ synthetic_definition ujson/BaseCharRenderer. object BaseCharRenderer
2727
// ^ definition ujson/BaseCharRenderer#[T] T <: Output
2828
// ^^^^^^^ reference upickle/
2929
// ^^^^ reference upickle/core/
@@ -66,13 +66,13 @@ class BaseCharRenderer[T <: upickle.core.CharOps.Output]
6666

6767
private[this] var depth: Int = 0
6868
// ^^^^^ definition ujson/BaseCharRenderer#depth(). private[this] var depth: Int
69-
// ^^^^^ definition ujson/BaseCharRenderer#`depth_=`(). private[this] var depth_=(x$1: Int): Unit
69+
// ^^^^^ synthetic_definition ujson/BaseCharRenderer#`depth_=`(). private[this] var depth_=(x$1: Int): Unit
7070
// ^^^ reference scala/Int#
7171

7272

7373
private[this] var commaBuffered = false
7474
// ^^^^^^^^^^^^^ definition ujson/BaseCharRenderer#commaBuffered(). private[this] var commaBuffered: Boolean
75-
// ^^^^^^^^^^^^^ definition ujson/BaseCharRenderer#`commaBuffered_=`(). private[this] var commaBuffered_=(x$1: Boolean): Unit
75+
// ^^^^^^^^^^^^^ synthetic_definition ujson/BaseCharRenderer#`commaBuffered_=`(). private[this] var commaBuffered_=(x$1: Boolean): Unit
7676

7777
def flushBuffer() = {
7878
// ^^^^^^^^^^^ definition ujson/BaseCharRenderer#flushBuffer(). def flushBuffer(): Unit

‎tests/snapshots/src/main/generated/minimized/Issue396.scala‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package minimized
33

44
case class Issue396(a: Int)
55
// ^^^^^^^^ definition minimized/Issue396# case class Issue396(a: Int)
6-
// ^^^^^^^^ definition minimized/Issue396#copy(). def copy(a: Int): Issue396
7-
// ^^^^^^^^ definition minimized/Issue396#productElement(). def productElement(x$1: Int): Any
8-
// ^^^^^^^^ definition minimized/Issue396. object Issue396
9-
// ^^^^^^^^ definition minimized/Issue396.apply(). def apply(a: Int): Issue396
10-
// ^^^^^^^^ definition minimized/Issue396#productElementName(). def productElementName(x$1: Int): String
6+
// ^^^^^^^^ synthetic_definition minimized/Issue396#copy(). def copy(a: Int): Issue396
7+
// ^^^^^^^^ synthetic_definition minimized/Issue396#productElement(). def productElement(x$1: Int): Any
8+
// ^^^^^^^^ synthetic_definition minimized/Issue396. object Issue396
9+
// ^^^^^^^^ synthetic_definition minimized/Issue396.apply(). def apply(a: Int): Issue396
10+
// ^^^^^^^^ synthetic_definition minimized/Issue396#productElementName(). def productElementName(x$1: Int): String
1111
// definition minimized/Issue396#`<init>`(). def this(a: Int)
1212
// ^ definition minimized/Issue396#a. val a: Int
1313
// ^^^ reference scala/Int#

‎tests/snapshots/src/main/generated/minimized/Issue397.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class Issue397 {
66
// definition minimized/Issue397#`<init>`(). def this()
77
var blah = Set("abc")
88
// ^^^^ definition minimized/Issue397#blah(). var blah: Set[String]
9-
// ^^^^ definition minimized/Issue397#`blah_=`(). var blah_=(x$1: Set[String]): Unit
9+
// ^^^^ synthetic_definition minimized/Issue397#`blah_=`(). var blah_=(x$1: Set[String]): Unit
1010
// ^^^ reference scala/Predef.Set.
1111
// reference scala/collection/IterableFactory#apply().
1212
blah = Set.empty[String]

‎tests/snapshots/src/main/generated/minimized/MinimizedScalaSignatures.scala‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ package minimized
66

77
case class MinimizedCaseClass(value: String) {
88
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass# case class MinimizedCaseClass(value: String)
9-
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass.apply(). def apply(value: String): MinimizedCaseClass
10-
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass#productElement(). def productElement(x$1: Int): Any
11-
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass. object MinimizedCaseClass
12-
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass#productElementName(). def productElementName(x$1: Int): String
13-
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass#copy(). def copy(value: String): MinimizedCaseClass
9+
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass.apply(). def apply(value: String): MinimizedCaseClass
10+
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass#productElement(). def productElement(x$1: Int): Any
11+
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass. object MinimizedCaseClass
12+
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass#productElementName(). def productElementName(x$1: Int): String
13+
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass#copy(). def copy(value: String): MinimizedCaseClass
1414
// definition minimized/MinimizedCaseClass#`<init>`(). def this(value: String)
1515
// ^^^^^ definition minimized/MinimizedCaseClass#value. val value: String
1616
// ^^^^^^ reference scala/Predef.String#

0 commit comments

Comments
 (0)