Skip to content

Commit 36570f0

Browse files
olafurpgolafurpg
authored
Merge pull request #400 from olafurpg/references
Fix code navigation for synthetic Scala symbols (including case classes)
2 parents 923efaf + 5b8d172 commit 36570f0

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)