Skip to content

Add signature pretty-printer to lsif-semanticdb conversion process #131

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.

Sign up for GitHub

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

Merged
Strum355 merged 18 commits into from
Apr 1, 2021
Merged

Add signature pretty-printer to lsif-semanticdb conversion process #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
9b8cbe0
add SemanticDB symbol parser to lsif-semanticdb
Strum355 Mar 29, 2021
54402af
change LsifWriter to take varargs of MarkedString for multiple hovers
Strum355 Mar 29, 2021
23322e7
add signature pretty-printer skeleton and usage sites
Strum355 Mar 29, 2021
2dbe8af
add basic class signature formatting
Strum355 Mar 29, 2021
d7c07a7
add basic method signature formatting
Strum355 Mar 29, 2021
b4f1182
add basic field/param/variable signature formatting
Strum355 Mar 29, 2021
7ebdfe5
add basic generic type variable signature formatting
Strum355 Mar 29, 2021
2135c05
add modifiers and visibility keywords to signature formatting
Strum355 Mar 29, 2021
f170c72
update snapshots with signature formatter
Strum355 Mar 29, 2021
b284432
fix pretty printing of Array signatures
Strum355 Mar 29, 2021
4fc0ce9
fix lsif snapshot diff with extra cheese
Strum355 Mar 29, 2021
2a08071
improved enum and enum constant pretty printing
Strum355 Mar 30, 2021
580aabd
fix 'this' pointing to non-existant field called 'this'
Strum355 Mar 30, 2021
d82dd45
fix annotation signature pretty printing
Strum355 Mar 31, 2021
8bb41e6
fix adding void|scala/Unit# type to signatures
Strum355 Mar 31, 2021
87068e0
fix constructor pretty printing; <init> -> Name
Strum355 Mar 31, 2021
a285b29
address review comments
Strum355 Mar 31, 2021
2c6a47d
add implements vs extends differentiating in pretty-printing
Strum355 Apr 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions lsif-java/src/main/scala/com/sourcegraph/lsif_java/SemanticdbPrinters.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package com.sourcegraph.lsif_java

import scala.jdk.CollectionConverters._

import com.sourcegraph.lsif_semanticdb.SignatureFormatter
import com.sourcegraph.lsif_semanticdb.Symtab
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolOccurrence
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolOccurrence.Role
import com.sourcegraph.semanticdb_javac.Semanticdb.TextDocument

object SemanticdbPrinters {
Expand All @@ -12,13 +15,14 @@ object SemanticdbPrinters {
.asScala
.groupBy(_.getRange.getStartLine)
val out = new StringBuilder()
val symtab = new Symtab(doc)
doc
.getText
.linesWithSeparators
.zipWithIndex
.foreach { case (line, i) =>
out.append(line.replace("\t", "→"))
val occurences = occurrencesByLine
val occurrences = occurrencesByLine
.getOrElse(i, Nil)
.sortBy(o =>
(
Expand All @@ -27,8 +31,8 @@ object SemanticdbPrinters {
o.getRange.getEndCharacter
)
)
occurences.foreach { occ =>
formatOccurrence(out, occ, line)
occurrences.foreach { occ =>
formatOccurrence(out, occ, line, symtab)
}
}
out.toString()
Expand All @@ -37,7 +41,8 @@ object SemanticdbPrinters {
private def formatOccurrence(
out: StringBuilder,
occ: SymbolOccurrence,
line: String
line: String,
symtab: Symtab
): Unit = {
val r = occ.getRange
val isMultiline = r.getStartLine != r.getEndLine
Expand Down Expand Up @@ -76,6 +81,19 @@ object SemanticdbPrinters {
else
""
)
.append(
symtab.symbols.asScala.get(occ.getSymbol) match {
case Some(info) if occ.getRole == Role.DEFINITION =>
val sig = new SignatureFormatter(info, symtab).formatSymbol()
if (sig.isEmpty)
" " + info.getDisplayName
else
" " + sig
case _ =>
""
}
)
.append("\n")
}

}
22 changes: 22 additions & 0 deletions lsif-java/src/main/scala/com/sourcegraph/lsif_java/commands/SnapshotLsifCommand.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.sourcegraph.lsif_protocol.LsifObject
import com.sourcegraph.lsif_protocol.LsifPosition
import com.sourcegraph.semanticdb_javac.Semanticdb
import com.sourcegraph.semanticdb_javac.Semanticdb.Language
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolInformation
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolOccurrence
import com.sourcegraph.semanticdb_javac.Semanticdb.SymbolOccurrence.Role
import com.sourcegraph.semanticdb_javac.Semanticdb.TextDocument
Expand Down Expand Up @@ -119,6 +120,27 @@ object SnapshotLsifCommand {
.setSymbol(symbol)
.build()
doc.addOccurrences(occ)

if (isDefinition) {
val hover =
(
for {
resultSetId <- lsif.next.get(o.getId).toList
hoverId <- lsif.hoverEdges.get(resultSetId).toList
hover <- lsif.hoverVertexes.get(hoverId).toList
contents <- hover.getContentsList.asScala
if contents.getLanguage !=
Language.UNKNOWN_LANGUAGE.toString.toLowerCase
} yield contents.getValue
).mkString("\n")
val symInfo = SymbolInformation
.newBuilder()
// we cheese it a bit here, as this is less work than trying to reconstruct
// a Signature from the pretty-printed Signature, with accompanying logic
// to fallback to display_name in SemanticdbPrinters.scala
.setDisplayName(hover).setSymbol(symbol).build()
Copy link
Contributor

@olafurpg olafurpg Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use Documentation instead?

Copy link
Contributor Author

@Strum355 Strum355 Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we (currently) dont emit Documentation in the snapshots, only the Signature with DisplayName as fallback.
This should probably still not use Documentation even if we did, if we decide to go ahead with the original "musing" of formatting documentation in a fashion similar to Lorem ipsum ... facilisis, in convallis ex imperdiet., truncating for brevity

Copy link
Contributor

@olafurpg olafurpg Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the android.view.View class is indeed missing from the classpath that we're using in the snapshots

❯ javap -cp $(cs fetch -r https://maven.google.com/ 'com.airbnb.android:epoxy:4.3.1' -p) android.view.View

Error: class not found: android.view.View

The class is defined in android.jar from the SDK

❯ javap -cp $ANDROID_HOME/platforms/android-30/android.jar android.view.View | head
Compiled from "View.java"
public class android.view.View implements android.graphics.drawable.Drawable$Callback,android.view.KeyEvent$Callback,android.view.accessibility.AccessibilityEventSource {
  public static final int ACCESSIBILITY_LIVE_REGION_ASSERTIVE;
  public static final int ACCESSIBILITY_LIVE_REGION_NONE;
  public static final int ACCESSIBILITY_LIVE_REGION_POLITE;
  public static final android.util.Property<android.view.View, java.lang.Float> ALPHA;
  public static final int AUTOFILL_FLAG_INCLUDE_NOT_IMPORTANT_VIEWS;
  public static final java.lang.String AUTOFILL_HINT_CREDIT_CARD_EXPIRATION_DATE;
  public static final java.lang.String AUTOFILL_HINT_CREDIT_CARD_EXPIRATION_DAY;

Copy link
Contributor Author

@Strum355 Strum355 Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its weird because many other android types do resolve. 🤷

doc.addSymbols(symInfo)
}
}
lsif.documents.values.map(_.build()).toList
}
Expand Down
16 changes: 15 additions & 1 deletion lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdb.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ private Stream<Integer> processPath(Path path, Set<String> isExportedSymbol) {
}

private Integer processDocument(LsifTextDocument doc, Set<String> isExportedSymbol) {
Symtab symtab = new Symtab(doc.semanticdb);

int documentId = writer.emitDocument(doc);
Set<String> localDefinitions =
doc.semanticdb.getOccurrencesList().stream()
Expand Down Expand Up @@ -112,9 +114,21 @@ private Integer processDocument(LsifTextDocument doc, Set<String> isExportedSymb
}

// Hover
ArrayList<MarkedString> markedStrings = new ArrayList<>();
String documentation = symbolInformation.getDocumentation().getMessage();
if (!documentation.isEmpty()) {
int hoverId = writer.emitHoverResult(doc.semanticdb.getLanguage(), documentation);
markedStrings.add(new MarkedString(Semanticdb.Language.UNKNOWN_LANGUAGE, documentation));
Copy link
Contributor

@olafurpg olafurpg Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use unknown language here?

Copy link
Contributor Author

@Strum355 Strum355 Mar 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im a bit unsure about how to approach this one.

  • On one side, by setting it to unknown language, we can extract the proper pretty printed signature for the SnapshotLsifCommand (explained here)
  • On the other hand, we may lose syntax highlighting of the javadoc comment. This one's a bit of a double edged sword, as depending on the highlighter, it may end up highlighting words that are not meant to be (like this when used in a sentence as opposed to a code snippet in the javadoc comment)

Copy link
Contributor

@olafurpg olafurpg Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding a new Documentation.Format called SIGNATURE? We can then make the Documentation documentation = 20; field repeated (which is a binary backwards compatible change). Do you think that would make this cleaner?

Copy link
Contributor Author

@Strum355 Strum355 Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good for a follow-up PR after discussing in our 1:1 👍

}

if (symbolInformation.hasSignature()) {
markedStrings.add(
new MarkedString(
doc.semanticdb.getLanguage(),
new SignatureFormatter(symbolInformation, symtab).formatSymbol()));
}

if (!markedStrings.isEmpty()) {
int hoverId = writer.emitHoverResult(markedStrings.toArray(new MarkedString[0]));
writer.emitHoverEdge(ids.resultSet, hoverId);
}
}
Expand Down
19 changes: 14 additions & 5 deletions lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;

/** High-level utility methods to write LSIF vertex/edge objects into the LSIF output stream. */
public class LsifWriter implements AutoCloseable {
Expand Down Expand Up @@ -90,15 +93,21 @@ public int emitDefinitionResult(int resultSet) {
return definitionResult;
}

public int emitHoverResult(Semanticdb.Language language, String value) {
public int emitHoverResult(MarkedString[] markedStrings) {
return emitObject(
lsifVertex("hoverResult")
.setResult(
LsifHover.newBuilder()
.addContents(
Content.newBuilder()
.setLanguage(language.toString().toLowerCase(Locale.ROOT))
.setValue(value))));
.addAllContents(
Arrays.stream(markedStrings)
.map(
(ms) ->
Content.newBuilder()
.setLanguage(
ms.language.toString().toLowerCase(Locale.ROOT))
.setValue(ms.value)
.build())
.collect(Collectors.toList()))));
}

public void emitHoverEdge(int outV, int inV) {
Expand Down
13 changes: 13 additions & 0 deletions lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/MarkedString.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.sourcegraph.lsif_semanticdb;

import com.sourcegraph.semanticdb_javac.Semanticdb;

public class MarkedString {
public final String value;
public final Semanticdb.Language language;

public MarkedString(Semanticdb.Language language, String value) {
this.value = value;
this.language = language;
}
}
Loading