Skip to content

Commit

Permalink
Promote synthetic definitions into normal definitions if they share n…
Browse files Browse the repository at this point in the history 
…ames

Previously, we generated synthetic definitions for all synthetic
definitions. However, this meant that "find references" never showed
results for synthetic symbols share the same name as the non-synthetic
definition. This commit changes that so synthetic definitions that share
the same name as the non-synthetic symbol are included with "find
references" results.

While testing this change I noticed that we don't handle synthetic
constructor parameters so I added support for those along the way.
  • Loading branch information
@olafurpg
olafurpg committed Feb 25, 2022
1 parent eb12a0d commit 0c51ae3
Show file tree
Hide file tree
Showing 18 changed files with 192 additions and 108 deletions.
34 changes: 19 additions & 15 deletions lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifTextDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,14 @@ public static Semanticdb.TextDocument manifestOccurrencesForSyntheticSymbols(
if (definition != null) {
continue;
}
for (String alternativeSymbol : alternativeSymbols(info)) {
for (Semanticdb.SymbolOccurrence alternativeSymbol : alternativeSymbols(info)) {
Semanticdb.SymbolOccurrence alternativeDefinition =
definitionOccurrences.get(alternativeSymbol);
definitionOccurrences.get(alternativeSymbol.getSymbol());
if (alternativeDefinition != null) {
builder.addOccurrences(
Semanticdb.SymbolOccurrence.newBuilder(alternativeDefinition)
.setRole(Semanticdb.SymbolOccurrence.Role.SYNTHETIC_DEFINITION)
Semanticdb.SymbolOccurrence.newBuilder()
.setRange(alternativeDefinition.getRange())
.setRole(alternativeSymbol.getRole())
.setSymbol(info.getSymbol()));
break;
}
Expand All @@ -118,29 +119,31 @@ public static Semanticdb.TextDocument manifestOccurrencesForSyntheticSymbols(
public static final Set<String> syntheticCompanionObjectNames =
new HashSet<>(Arrays.asList("apply", "copy"));

public static List<String> alternativeSymbols(Semanticdb.SymbolInformation info) {
ArrayList<String> alternatives = new ArrayList<>();
public static List<Semanticdb.SymbolOccurrence> alternativeSymbols(
Semanticdb.SymbolInformation info) {
SymbolOccurrences alternatives = new SymbolOccurrences();
SymbolDescriptor sym = SymbolDescriptor.parseFromSymbol(info.getSymbol());
switch (sym.descriptor.kind) {
case Method:
if (sym.descriptor.name.endsWith("_=")) {
String newName = sym.descriptor.name.substring(0, sym.descriptor.name.length() - 2);
alternatives.add(SemanticdbSymbols.global(sym.owner, sym.descriptor.withName(newName)));
alternatives.addDefinition(
SemanticdbSymbols.global(sym.owner, sym.descriptor.withName(newName)));
} else if (syntheticCaseClassMethodNames.contains(sym.descriptor.name)) {
alternatives.add(sym.owner);
alternatives.addSyntheticDefinition(sym.owner);
} else if (syntheticCompanionObjectNames.contains(sym.descriptor.name)) {
alternatives.add(sym.owner);
alternatives.addSyntheticDefinition(sym.owner);
SymbolDescriptor owner = SymbolDescriptor.parseFromSymbol(sym.owner);
alternatives.add(
alternatives.addSyntheticDefinition(
SemanticdbSymbols.global(
owner.owner, owner.descriptor.withKind(SemanticdbSymbols.Descriptor.Kind.Type)));
}
break;
case Parameter:
SymbolDescriptor owner = SymbolDescriptor.parseFromSymbol(sym.owner);
if (owner.descriptor.name.equals("copy")) {
if (owner.descriptor.name.equals("copy") || owner.descriptor.name.equals("<init>")) {
// case classes copy method parameter.
alternatives.add(
alternatives.addDefinition(
SemanticdbSymbols.global(
owner.owner, sym.descriptor.withKind(SemanticdbSymbols.Descriptor.Kind.Term)));
} else if (owner.descriptor.name.equals("apply")) {
Expand All @@ -150,17 +153,18 @@ public static List<String> alternativeSymbols(Semanticdb.SymbolInformation info)
SemanticdbSymbols.global(
grandparent.owner,
grandparent.descriptor.withKind(SemanticdbSymbols.Descriptor.Kind.Type));
alternatives.add(
alternatives.addDefinition(
SemanticdbSymbols.global(
companion, sym.descriptor.withKind(SemanticdbSymbols.Descriptor.Kind.Term)));
}
case Term:
alternatives.add(
alternatives.addDefinition(
SemanticdbSymbols.global(
sym.owner, sym.descriptor.withKind(SemanticdbSymbols.Descriptor.Kind.Type)));
break;
default:
}
return alternatives;

return alternatives.occurrences;
}
}
26 changes: 26 additions & 0 deletions lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/SymbolOccurrences.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.sourcegraph.lsif_semanticdb;

import com.sourcegraph.semanticdb_javac.Semanticdb;

import java.util.ArrayList;
import java.util.List;

public class SymbolOccurrences {
public List<Semanticdb.SymbolOccurrence> occurrences = new ArrayList<>();

public void addSyntheticDefinition(String sym) {
occurrences.add(
Semanticdb.SymbolOccurrence.newBuilder()
.setSymbol(sym)
.setRole(Semanticdb.SymbolOccurrence.Role.SYNTHETIC_DEFINITION)
.build());
}

public void addDefinition(String sym) {
occurrences.add(
Semanticdb.SymbolOccurrence.newBuilder()
.setSymbol(sym)
.setRole(Semanticdb.SymbolOccurrence.Role.DEFINITION)
.build());
}
}
9 changes: 6 additions & 3 deletions tests/snapshots/src/main/generated/BaseByteRenderer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,23 @@ import upickle.core.{ArrVisitor, ObjVisitor}
*/
class BaseByteRenderer[T <: upickle.core.ByteOps.Output]
// ^^^^^^^^^^^^^^^^ definition ujson/BaseByteRenderer# class BaseByteRenderer[T <: Output]
// ^^^^^^^^^^^^^^^^ synthetic_definition ujson/BaseByteRenderer. object BaseByteRenderer
// ^^^^^^^^^^^^^^^^ definition ujson/BaseByteRenderer. object BaseByteRenderer
// ^ definition ujson/BaseByteRenderer#[T] T <: Output
// ^^^^^^^ reference upickle/
// ^^^^ reference upickle/core/
// ^^^^^^^ reference upickle/core/ByteOps.
// ^^^^^^ reference upickle/core/ByteOps.Output#
(out: T,
// ^^^ definition ujson/BaseByteRenderer#out. private[this] val out: T
// ^^^ definition ujson/BaseByteRenderer#`<init>`().(out) out: T
// ^ reference ujson/BaseByteRenderer#[T]
indent: Int = -1,
// ^^^^^^ definition ujson/BaseByteRenderer#indent. private[this] val indent: Int
// ^^^^^^ definition ujson/BaseByteRenderer#`<init>`().(indent) default indent: Int
// ^^^ reference scala/Int#
escapeUnicode: Boolean = false) extends JsVisitor[T, T]{
// ^^^^^^^^^^^^^ definition ujson/BaseByteRenderer#escapeUnicode. private[this] val escapeUnicode: Boolean
// ^^^^^^^^^^^^^ definition ujson/BaseByteRenderer#`<init>`().(escapeUnicode) default escapeUnicode: Boolean
// ^^^^^^^ reference scala/Boolean#
// ^^^^^^^^^ reference ujson/JsVisitor#
// ^ reference ujson/BaseByteRenderer#[T]
Expand Down Expand Up @@ -66,13 +69,13 @@ class BaseByteRenderer[T <: upickle.core.ByteOps.Output]

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


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

def flushBuffer() = {
// ^^^^^^^^^^^ definition ujson/BaseByteRenderer#flushBuffer(). def flushBuffer(): Unit
Expand Down
9 changes: 6 additions & 3 deletions tests/snapshots/src/main/generated/BaseCharRenderer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,23 @@ import upickle.core.{ArrVisitor, ObjVisitor}
*/
class BaseCharRenderer[T <: upickle.core.CharOps.Output]
// ^^^^^^^^^^^^^^^^ definition ujson/BaseCharRenderer# class BaseCharRenderer[T <: Output]
// ^^^^^^^^^^^^^^^^ synthetic_definition ujson/BaseCharRenderer. object BaseCharRenderer
// ^^^^^^^^^^^^^^^^ definition ujson/BaseCharRenderer. object BaseCharRenderer
// ^ definition ujson/BaseCharRenderer#[T] T <: Output
// ^^^^^^^ reference upickle/
// ^^^^ reference upickle/core/
// ^^^^^^^ reference upickle/core/CharOps.
// ^^^^^^ reference upickle/core/CharOps.Output#
(out: T,
// ^^^ definition ujson/BaseCharRenderer#out. private[this] val out: T
// ^^^ definition ujson/BaseCharRenderer#`<init>`().(out) out: T
// ^ reference ujson/BaseCharRenderer#[T]
indent: Int = -1,
// ^^^^^^ definition ujson/BaseCharRenderer#indent. private[this] val indent: Int
// ^^^^^^ definition ujson/BaseCharRenderer#`<init>`().(indent) default indent: Int
// ^^^ reference scala/Int#
escapeUnicode: Boolean = false) extends JsVisitor[T, T]{
// ^^^^^^^^^^^^^ definition ujson/BaseCharRenderer#escapeUnicode. private[this] val escapeUnicode: Boolean
// ^^^^^^^^^^^^^ definition ujson/BaseCharRenderer#`<init>`().(escapeUnicode) default escapeUnicode: Boolean
// ^^^^^^^ reference scala/Boolean#
// ^^^^^^^^^ reference ujson/JsVisitor#
// ^ reference ujson/BaseCharRenderer#[T]
Expand Down Expand Up @@ -66,13 +69,13 @@ class BaseCharRenderer[T <: upickle.core.CharOps.Output]

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


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

def flushBuffer() = {
// ^^^^^^^^^^^ definition ujson/BaseCharRenderer#flushBuffer(). def flushBuffer(): Unit
Expand Down
7 changes: 4 additions & 3 deletions tests/snapshots/src/main/generated/minimized/Issue396.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ case class Issue396(a: Int)
// ^^^^^^^^ definition minimized/Issue396# case class Issue396(a: Int)
// ^^^^^^^^ synthetic_definition minimized/Issue396#copy(). def copy(a: Int): Issue396
// ^^^^^^^^ synthetic_definition minimized/Issue396#productElement(). def productElement(x$1: Int): Any
// ^^^^^^^^ synthetic_definition minimized/Issue396. object Issue396
// ^^^^^^^^ definition minimized/Issue396. object Issue396
// ^^^^^^^^ synthetic_definition minimized/Issue396.apply(). def apply(a: Int): Issue396
// ^^^^^^^^ synthetic_definition minimized/Issue396#productElementName(). def productElementName(x$1: Int): String
// definition minimized/Issue396#`<init>`(). def this(a: Int)
// ^ definition minimized/Issue396#a. val a: Int
// ^ synthetic_definition minimized/Issue396.apply().(a) a: Int
// ^ synthetic_definition minimized/Issue396#copy().(a) default a: Int
// ^ definition minimized/Issue396.apply().(a) a: Int
// ^ definition minimized/Issue396#`<init>`().(a) a: Int
// ^ definition minimized/Issue396#copy().(a) default a: Int
// ^^^ reference scala/Int#
object Issue396App {
// ^^^^^^^^^^^ definition minimized/Issue396App. object Issue396App
Expand Down
2 changes: 1 addition & 1 deletion tests/snapshots/src/main/generated/minimized/Issue397.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class Issue397 {
// definition minimized/Issue397#`<init>`(). def this()
var blah = Set("abc")
// ^^^^ definition minimized/Issue397#blah(). var blah: Set[String]
// ^^^^ synthetic_definition minimized/Issue397#`blah_=`(). var blah_=(x$1: Set[String]): Unit
// ^^^^ definition minimized/Issue397#`blah_=`(). var blah_=(x$1: Set[String]): Unit
// ^^^ reference scala/Predef.Set.
// reference scala/collection/IterableFactory#apply().
blah = Set.empty[String]
Expand Down
7 changes: 4 additions & 3 deletions tests/snapshots/src/main/generated/minimized/MinimizedScalaSignatures.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ case class MinimizedCaseClass(value: String) {
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass# case class MinimizedCaseClass(value: String)
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass.apply(). def apply(value: String): MinimizedCaseClass
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass#productElement(). def productElement(x$1: Int): Any
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass. object MinimizedCaseClass
// ^^^^^^^^^^^^^^^^^^ definition minimized/MinimizedCaseClass. object MinimizedCaseClass
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass#productElementName(). def productElementName(x$1: Int): String
// ^^^^^^^^^^^^^^^^^^ synthetic_definition minimized/MinimizedCaseClass#copy(). def copy(value: String): MinimizedCaseClass
// definition minimized/MinimizedCaseClass#`<init>`(). def this(value: String)
// ^^^^^ definition minimized/MinimizedCaseClass#value. val value: String
// ^^^^^ synthetic_definition minimized/MinimizedCaseClass#copy().(value) default value: String
// ^^^^^ synthetic_definition minimized/MinimizedCaseClass.apply().(value) value: String
// ^^^^^ definition minimized/MinimizedCaseClass#copy().(value) default value: String
// ^^^^^ definition minimized/MinimizedCaseClass#`<init>`().(value) value: String
// ^^^^^ definition minimized/MinimizedCaseClass.apply().(value) value: String
// ^^^^^^ reference scala/Predef.String#
def this() = this("value")
// ^^^^ definition minimized/MinimizedCaseClass#`<init>`(+1). def this()
Expand Down
6 changes: 5 additions & 1 deletion tests/snapshots/src/main/generated/ujson/AstTransformer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,12 @@ trait AstTransformer[I] extends Transformer[I] with JsVisitor[I, I]{
// ^ definition ujson/AstTransformer#AstObjVisitor#[T] T
// definition ujson/AstTransformer#AstObjVisitor#`<init>`(). def this(build: (T) => I)(factory: Factory[(String, I), T])
// ^^^^^ definition ujson/AstTransformer#AstObjVisitor#build. private[this] val build: (T) => I
// ^^^^^ definition ujson/AstTransformer#AstObjVisitor#`<init>`().(build) build: (T) => I
// ^ reference ujson/AstTransformer#AstObjVisitor#[T]
// ^ reference ujson/AstTransformer#[I]
(implicit factory: Factory[(String, I), T])extends ObjVisitor[I, I] {
// ^^^^^^^ definition ujson/AstTransformer#AstObjVisitor#factory. private[this] implicit val factory: Factory[(String, I), T]
// ^^^^^^^ definition ujson/AstTransformer#AstObjVisitor#`<init>`().(factory) implicit factory: Factory[(String, I), T]
// ^^^^^^^ reference upickle/core/compat/package.Factory#
// ^^^^^^ reference scala/Predef.String#
// ^ reference ujson/AstTransformer#[I]
Expand All @@ -119,7 +121,7 @@ trait AstTransformer[I] extends Transformer[I] with JsVisitor[I, I]{

private[this] var key: String = null
// ^^^ definition ujson/AstTransformer#AstObjVisitor#key(). private[this] var key: String
// ^^^ synthetic_definition ujson/AstTransformer#AstObjVisitor#`key_=`(). private[this] var key_=(x$1: String): Unit
// ^^^ definition ujson/AstTransformer#AstObjVisitor#`key_=`(). private[this] var key_=(x$1: String): Unit
// ^^^^^^ reference scala/Predef.String#
private[this] val vs = factory.newBuilder
// ^^ definition ujson/AstTransformer#AstObjVisitor#vs. private[this] val vs: Builder[(String, I), T]
Expand Down Expand Up @@ -170,11 +172,13 @@ trait AstTransformer[I] extends Transformer[I] with JsVisitor[I, I]{
// ^ definition ujson/AstTransformer#AstArrVisitor#[T] T
// definition ujson/AstTransformer#AstArrVisitor#`<init>`(). def this(build: (T[I]) => I)(factory: Factory[I, T[I]])
// ^^^^^ definition ujson/AstTransformer#AstArrVisitor#build. private[this] val build: (T[I]) => I
// ^^^^^ definition ujson/AstTransformer#AstArrVisitor#`<init>`().(build) build: (T[I]) => I
// ^ reference ujson/AstTransformer#AstArrVisitor#[T]
// ^ reference ujson/AstTransformer#[I]
// ^ reference ujson/AstTransformer#[I]
(implicit factory: Factory[I, T[I]]) extends ArrVisitor[I, I]{
// ^^^^^^^ definition ujson/AstTransformer#AstArrVisitor#factory. private[this] implicit val factory: Factory[I, T[I]]
// ^^^^^^^ definition ujson/AstTransformer#AstArrVisitor#`<init>`().(factory) implicit factory: Factory[I, T[I]]
// ^^^^^^^ reference upickle/core/compat/package.Factory#
// ^ reference ujson/AstTransformer#[I]
// ^ reference ujson/AstTransformer#AstArrVisitor#[T]
Expand Down
1 change: 1 addition & 0 deletions tests/snapshots/src/main/generated/ujson/ByteArrayParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ final class ByteArrayParser[J](src: Array[Byte]) extends ByteParser[J]{
// ^ definition ujson/ByteArrayParser#[J] J
// definition ujson/ByteArrayParser#`<init>`(). def this(src: Array[Byte])
// ^^^ definition ujson/ByteArrayParser#src. private[this] val src: Array[Byte]
// ^^^ definition ujson/ByteArrayParser#`<init>`().(src) src: Array[Byte]
// ^^^^^ reference scala/Array#
// ^^^^ reference scala/Byte#
// ^^^^^^^^^^ reference ujson/ByteParser#
Expand Down
1 change: 1 addition & 0 deletions tests/snapshots/src/main/generated/ujson/ByteBufferParser.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ final class ByteBufferParser[J](src: ByteBuffer) extends ByteParser[J]{
// ^ definition ujson/ByteBufferParser#[J] J
// definition ujson/ByteBufferParser#`<init>`(). def this(src: ByteBuffer)
// ^^^ definition ujson/ByteBufferParser#src. private[this] val src: ByteBuffer
// ^^^ definition ujson/ByteBufferParser#`<init>`().(src) src: ByteBuffer
// ^^^^^^^^^^ reference java/nio/ByteBuffer#
// ^^^^^^^^^^ reference ujson/ByteParser#
// ^ reference ujson/ByteBufferParser#[J]
Expand Down
Loading

0 comments on commit 0c51ae3

Please sign in to comment.