Skip to content
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

Fix: multiple compilation units in a file lead to information loss #738

Merged
merged 9 commits into from
Aug 20, 2024
Merged

Fix: multiple compilation units in a file lead to information loss #738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c055917
investigation
antonsviridov-src Aug 15, 2024
c02e4d4
WIP: fix by appending, not overwriting
antonsviridov-src Aug 19, 2024
bf6995e
Clean up semanticdb file upon entering a particular source file
antonsviridov-src Aug 20, 2024
7a6b655
Make document appender more robust
antonsviridov-src Aug 20, 2024
82df232
Rerun snapshots
antonsviridov-src Aug 20, 2024
956383a
cleanup
antonsviridov-src Aug 20, 2024
94e8cb8
Protect against NPE in path absolutisation
antonsviridov-src Aug 20, 2024
69699b6
Call inferBazelSourceroot defensively
antonsviridov-src Aug 20, 2024
80b5f0c
Separate writing and appending to Semanticdb
antonsviridov-src Aug 20, 2024
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
1 change: 1 addition & 0 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ lazy val docs = project
.enablePlugins(DocusaurusPlugin)

lazy val javaOnlySettings = List[Def.Setting[_]](
javafmtOnCompile := false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was doing my head in – IntelliJ was formatting one way, running SBT session (~snapshots/run) another way, and IJ constantly producing a warning to load filesystem changes.

Formatting on compile is evil.

Copy link
Member

Choose a reason for hiding this comment

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

Formatting on compile is evil.

Agreed.

autoScalaLibrary := false,
incOptions ~= { old =>
old.withEnabled(false).withApiDebug(true)
Expand Down
115 changes: 105 additions & 10 deletions semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
import javax.lang.model.util.Types;

import javax.tools.JavaFileObject;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.*;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Callback hook that generates SemanticDB when the compiler has completed typechecking a Java
Expand Down Expand Up @@ -44,7 +45,22 @@ public SemanticdbTaskListener(
}

@Override
public void started(TaskEvent e) {}
public void started(TaskEvent e) {
// Upon first encounter with a file (before any other tasks are run)
// we remove the semanticdb file for this source file to ensure
// stale data doesn't cause problems
Comment on lines +49 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that we read previous state, we shouldn't expect the folder to be clean – without doing so I was getting very confusing results when running snapshots.

if (e.getKind() == TaskEvent.Kind.ENTER) {
inferBazelSourceroot(e.getSourceFile());
Result<Path, String> semanticdbPath = semanticdbOutputPath(options, e);
if (semanticdbPath.isOk()) {
try {
Files.deleteIfExists(semanticdbPath.getOrThrow());
} catch (IOException ex) {
this.reportException(ex, e);
}
}
}
}

@Override
public void finished(TaskEvent e) {
Expand Down Expand Up @@ -78,8 +94,10 @@ public void finished(TaskEvent e) {
}
}

// Uses reporter.error with the full stack trace of the exception instead of reporter.exception
// because reporter.exception doesn't seem to print any meaningful information about the
// Uses reporter.error with the full stack trace of the exception instead of
// reporter.exception
// because reporter.exception doesn't seem to print any meaningful information
// about the
// exception, it just prints the location with an empty message.
private void reportException(Throwable exception, TaskEvent e) {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
Expand All @@ -96,7 +114,9 @@ private void onFinishedAnalyze(TaskEvent e) {
Semanticdb.TextDocument textDocument =
new SemanticdbVisitor(globals, e.getCompilationUnit(), options, types, trees, elements)
.buildTextDocument(e.getCompilationUnit());
writeSemanticdb(e, path.getOrThrow(), textDocument);
Path output = path.getOrThrow();
if (Files.exists(output)) appendSemanticdb(e, output, textDocument);
else writeSemanticdb(e, output, textDocument);
} else {
reporter.error(path.getErrorOrThrow(), e);
}
Expand All @@ -114,6 +134,78 @@ private void writeSemanticdb(TaskEvent event, Path output, Semanticdb.TextDocume
}
}

private void appendSemanticdb(
TaskEvent event, Path output, Semanticdb.TextDocument textDocument) {
/*
* If there already is a semanticdb file at the given path,
* we do the following:
* - Read a documents collection
* - Try to find the document with the matching relative path (matching the incoming textDocument)
* - Then, depending on whether a matching document already exists in the collection:
* - if YES, mutate it in place to only add entries from the incoming document
* - if NO, simply add the incoming text document to the collection
* - Write the collection back to disk
* */
Semanticdb.TextDocument document = null;
int documentIndex = -1;
Semanticdb.TextDocuments documents = null;

try (InputStream is = Files.newInputStream(output.toFile().toPath())) {
documents = Semanticdb.TextDocuments.parseFrom(is);

for (int i = 0; i < documents.getDocumentsCount(); i++) {
Semanticdb.TextDocument candidate = documents.getDocuments(i);
if (document == null && candidate.getUri().equals(textDocument.getUri())) {
document = candidate;
documentIndex = i;
}
}

} catch (IOException e) {
this.reportException(e, event);
return;
}

if (document != null) {
// If there is a previous semanticdb document at this path, we need
// to deduplicate symbols and occurrences and mutate the document in place
Set<Semanticdb.SymbolInformation> symbols = new HashSet<>(textDocument.getSymbolsList());
Set<Semanticdb.SymbolOccurrence> occurrences =
new HashSet<>(textDocument.getOccurrencesList());
Set<Semanticdb.Synthetic> synthetics = new HashSet<>(textDocument.getSyntheticsList());

symbols.addAll(document.getSymbolsList());
occurrences.addAll(document.getOccurrencesList());
synthetics.addAll(document.getSyntheticsList());

documents
.toBuilder()
.addDocuments(
documentIndex,
document
.toBuilder()
.clearOccurrences()
.addAllOccurrences(occurrences)
.clearSymbols()
.addAllSymbols(symbols)
.clearSynthetics()
.addAllSynthetics(synthetics));

} else {
// If no prior document was found, we can just add the incoming one to the collection
documents = documents.toBuilder().addDocuments(textDocument).build();
}

byte[] bytes = documents.toByteArray();

try {
Files.createDirectories(output.getParent());
Files.write(output, bytes);
} catch (IOException e) {
this.reportException(e, event);
}
}

public static Path absolutePathFromUri(SemanticdbJavacOptions options, JavaFileObject file) {
URI uri = file.toUri();
if ((options.uriScheme == UriScheme.SBT || options.uriScheme == UriScheme.ZINC)
Expand Down Expand Up @@ -210,10 +302,13 @@ private Result<Path, String> semanticdbOutputPath(SemanticdbJavacOptions options

switch (options.noRelativePath) {
case INDEX_ANYWAY:
// Come up with a unique relative path for this file even if it's not under the sourceroot.
// By indexing auto-generated files, we collect SymbolInformation for auto-generated symbol,
// Come up with a unique relative path for this file even if it's not under the
// sourceroot.
// By indexing auto-generated files, we collect SymbolInformation for
// auto-generated symbol,
// which results in more useful hover tooltips in the editor.
// In the future, we may want to additionally embed the full text contents of these files
// In the future, we may want to additionally embed the full text contents of
// these files
// so that it's possible to browse generated files with precise code navigation.
String uniqueFilename =
String.format("%d.%s.semanticdb", ++noRelativePathCounter, absolutePath.getFileName());
Expand Down
12 changes: 12 additions & 0 deletions tests/minimized/src/main/java/minimized/Interfaces.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,15 @@ default String defaultInterfaceMethod() {
return "default";
}
}

interface BookService {
void checkPages();
}

interface MyService {
BookService bookService();

default void example() {
bookService().checkPages();
}
}
13 changes: 13 additions & 0 deletions ...hots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
// kind Interface
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
→double value();
// ^^^^^ definition semanticdb maven . . minimized/Bar#value().
// display_name value
// signature_documentation java public abstract double value()
// kind AbstractMethod
}

@interface BarB {
Expand All @@ -16,6 +20,10 @@
// kind Interface
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
→boolean value();
// ^^^^^ definition semanticdb maven . . minimized/BarB#value().
// display_name value
// signature_documentation java public abstract boolean value()
// kind AbstractMethod
}

@interface Nullable {
Expand All @@ -25,6 +33,11 @@
// kind Interface
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
→String value() default "";
//^^^^^ reference semanticdb maven jdk 11 java/lang/String#
// ^^^^^ definition semanticdb maven . . minimized/Nullable#value().
// display_name value
// signature_documentation java public abstract String value()
// kind AbstractMethod
}

interface Foo {
Expand Down
56 changes: 45 additions & 11 deletions ...in/generated/tests/minimized/src/main/java/minimized/AnnotationsOnParameterizedTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,44 @@ public interface AnnotationsOnParameterizedTypes {
// kind Interface

public static AnnotationsOnParameterizedTypes getInstance() {
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#
// ^^^^^^^^^^^ definition semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#getInstance().
// display_name getInstance
// signature_documentation java public static AnnotationsOnParameterizedTypes getInstance()
// kind StaticMethod
return new AnnotationsOnParameterizedTypesImpl();
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#`<init>`().
}

<C, W> Function<W, C> adapter(Class<C> contract, Class<W> wrappedClass);
// ^ definition semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().[C]
// display_name C
// signature_documentation java C
// kind TypeParameter
// ^ definition semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().[W]
// display_name W
// signature_documentation java W
// kind TypeParameter
// ^^^^^^^^ reference semanticdb maven jdk 11 java/util/function/Function#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().[W]
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().[C]
// ^^^^^^^ definition semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().
// display_name adapter
// signature_documentation java public abstract <C, W> Function<W, C> adapter(Class<C> contract, Class<W> wrappedClass)
// kind AbstractMethod
// relationship is_reference is_implementation semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().
// ^^^^^ reference semanticdb maven jdk 11 java/lang/Class#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().[C]
// ^^^^^^^^ definition local 0
// display_name contract
// signature_documentation java Class<C> contract
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().
// ^^^^^ reference semanticdb maven jdk 11 java/lang/Class#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().[W]
// ^^^^^^^^^^^^ definition local 1
// display_name wrappedClass
// signature_documentation java Class<W> wrappedClass
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().
}


Expand Down Expand Up @@ -82,13 +116,13 @@ public <C, W> Function<W, C> adapter(Class<C> contract, Class<W> wrappedClass) {
// relationship is_reference is_implementation semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter().
// ^^^^^ reference semanticdb maven jdk 11 java/lang/Class#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().[C]
// ^^^^^^^^ definition local 0
// ^^^^^^^^ definition local 2
// display_name contract
// signature_documentation java Class<C> contract
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().
// ^^^^^ reference semanticdb maven jdk 11 java/lang/Class#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().[W]
// ^^^^^^^^^^^^ definition local 1
// ^^^^^^^^^^^^ definition local 3
// display_name wrappedClass
// signature_documentation java Class<W> wrappedClass
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().
Expand All @@ -97,19 +131,19 @@ public <C, W> Function<W, C> adapter(Class<C> contract, Class<W> wrappedClass) {
// ^^^^^^^^ reference semanticdb maven jdk 11 java/util/function/Function#
// ^^^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/reflect/InvocationHandler#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().[C]
// ^^^^^^^^^^^ definition local 2
// ^^^^^^^^^^^ definition local 4
// display_name constructor
// signature_documentation java Function<InvocationHandler, C> constructor
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter().
// kind Variable
// ^^^^^^^^^^^^^^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().
// ^^^^^^^^ reference local 0
// ^^^^^^^^ reference local 2

System.out.println(constructor);
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/System#
// ^^^ reference semanticdb maven jdk 11 java/lang/System#out.
// ^^^^^^^ reference semanticdb maven jdk 11 java/io/PrintStream#println(+9).
// ^^^^^^^^^^^ reference local 2
// ^^^^^^^^^^^ reference local 4

return null;
}
Expand All @@ -128,7 +162,7 @@ private <T> Function<InvocationHandler, T> getConstructor(Class<T> contract) {
// kind Method
// ^^^^^ reference semanticdb maven jdk 11 java/lang/Class#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().[T]
// ^^^^^^^^ definition local 3
// ^^^^^^^^ definition local 5
// display_name contract
// signature_documentation java Class<T> contract
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().
Expand All @@ -137,17 +171,17 @@ private <T> Function<InvocationHandler, T> getConstructor(Class<T> contract) {
Constructor<T> constructor = (Constructor<T>) proxyConstructors.computeIfAbsent(contract, c -> {
// ^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/reflect/Constructor#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().[T]
// ^^^^^^^^^^^ definition local 4
// ^^^^^^^^^^^ definition local 6
// display_name constructor
// signature_documentation java @SuppressWarnings("unchecked")\nConstructor<T> constructor
// signature_documentation java @SuppressWarnings\nConstructor<T> constructor
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().
// kind Variable
// ^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/reflect/Constructor#
// ^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().[T]
// ^^^^^^^^^^^^^^^^^ reference semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#proxyConstructors.
// ^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/util/concurrent/ConcurrentMap#computeIfAbsent().
// ^^^^^^^^ reference local 3
// ^ definition local 5
// ^^^^^^^^ reference local 5
// ^ definition local 7
// display_name c
// signature_documentation java Class<?> c
// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor().
Expand All @@ -158,7 +192,7 @@ private <T> Function<InvocationHandler, T> getConstructor(Class<T> contract) {
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/System#
// ^^^ reference semanticdb maven jdk 11 java/lang/System#out.
// ^^^^^^^ reference semanticdb maven jdk 11 java/io/PrintStream#println(+9).
// ^^^^^^^^^^^ reference local 4
// ^^^^^^^^^^^ reference local 6

return null;
}
Expand Down
Loading
Loading