From 2298a95593f1f1d6a2428ac00afae021a6426079 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 20 Aug 2024 11:07:56 +0100 Subject: [PATCH] Fix: multiple compilation units in a file lead to information loss (#738) Fixes GRAPH-753 We discovered that javac issues multiple events per file if there are multiple top level definitions that match what javac considers to be a compilation unit - interfaces or classes for example. These multiple invocations produce slightly different results, and all writing to the same file, meaning the information gets lost. To fix this, we defensively read the file if it already exists and carefully deduplicate symbols/occurrences/synthetics before writing back. --- build.sbt | 1 + .../SemanticdbTaskListener.java | 115 ++++++++++++++++-- .../src/main/java/minimized/Interfaces.java | 12 ++ .../java/minimized/AnnotationParameters.java | 13 ++ .../AnnotationsOnParameterizedTypes.java | 56 +++++++-- .../src/main/java/minimized/Interfaces.java | 35 ++++++ .../MinimizedSnapshotScipGenerator.scala | 1 + 7 files changed, 212 insertions(+), 21 deletions(-) diff --git a/build.sbt b/build.sbt index 7549aee0e..eaa153c82 100644 --- a/build.sbt +++ b/build.sbt @@ -545,6 +545,7 @@ lazy val docs = project .enablePlugins(DocusaurusPlugin) lazy val javaOnlySettings = List[Def.Setting[_]]( + javafmtOnCompile := false, autoScalaLibrary := false, incOptions ~= { old => old.withEnabled(false).withApiDebug(true) diff --git a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java index f2594c76f..7c5238f65 100644 --- a/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java +++ b/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java @@ -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 @@ -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 + if (e.getKind() == TaskEvent.Kind.ENTER) { + inferBazelSourceroot(e.getSourceFile()); + Result 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) { @@ -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(); @@ -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); } @@ -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 symbols = new HashSet<>(textDocument.getSymbolsList()); + Set occurrences = + new HashSet<>(textDocument.getOccurrencesList()); + Set 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) @@ -210,10 +302,13 @@ private Result 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()); diff --git a/tests/minimized/src/main/java/minimized/Interfaces.java b/tests/minimized/src/main/java/minimized/Interfaces.java index c06faea6e..370db5d14 100644 --- a/tests/minimized/src/main/java/minimized/Interfaces.java +++ b/tests/minimized/src/main/java/minimized/Interfaces.java @@ -9,3 +9,15 @@ default String defaultInterfaceMethod() { return "default"; } } + +interface BookService { + void checkPages(); +} + +interface MyService { + BookService bookService(); + + default void example() { + bookService().checkPages(); + } +} diff --git a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java index 7704629ca..d2c068fae 100644 --- a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java +++ b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java @@ -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 { @@ -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 { @@ -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 { diff --git a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationsOnParameterizedTypes.java b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationsOnParameterizedTypes.java index 40c1222e0..e50392d38 100644 --- a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationsOnParameterizedTypes.java +++ b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationsOnParameterizedTypes.java @@ -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#``(). } Function adapter(Class contract, Class 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 Function adapter(Class contract, Class 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 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 wrappedClass +// enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypes#adapter(). } @@ -82,13 +116,13 @@ public Function adapter(Class contract, Class 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 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 wrappedClass // enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#adapter(). @@ -97,19 +131,19 @@ public Function adapter(Class contract, Class 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 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; } @@ -128,7 +162,7 @@ private Function getConstructor(Class 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 contract // enclosing_symbol semanticdb maven . . minimized/AnnotationsOnParameterizedTypesImpl#getConstructor(). @@ -137,17 +171,17 @@ private Function getConstructor(Class contract) { Constructor constructor = (Constructor) 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 constructor +// signature_documentation java @SuppressWarnings\nConstructor 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(). @@ -158,7 +192,7 @@ private Function getConstructor(Class 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; } diff --git a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/Interfaces.java b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/Interfaces.java index ccab9b4ea..4ad676630 100644 --- a/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/Interfaces.java +++ b/tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/Interfaces.java @@ -28,3 +28,38 @@ default String defaultInterfaceMethod() { return "default"; } } + +interface BookService { +// ^^^^^^^^^^^ definition semanticdb maven . . minimized/BookService# +// display_name BookService +// signature_documentation java interface BookService +// kind Interface + void checkPages(); +// ^^^^^^^^^^ definition semanticdb maven . . minimized/BookService#checkPages(). +// display_name checkPages +// signature_documentation java public abstract void checkPages() +// kind AbstractMethod +} + +interface MyService { +// ^^^^^^^^^ definition semanticdb maven . . minimized/MyService# +// display_name MyService +// signature_documentation java interface MyService +// kind Interface + BookService bookService(); +//^^^^^^^^^^^ reference semanticdb maven . . minimized/BookService# +// ^^^^^^^^^^^ definition semanticdb maven . . minimized/MyService#bookService(). +// display_name bookService +// signature_documentation java public abstract BookService bookService() +// kind AbstractMethod + + default void example() { +// ^^^^^^^ definition semanticdb maven . . minimized/MyService#example(). +// display_name example +// signature_documentation java public default void example() +// kind Method + bookService().checkPages(); +// ^^^^^^^^^^^ reference semanticdb maven . . minimized/MyService#bookService(). +// ^^^^^^^^^^ reference semanticdb maven . . minimized/BookService#checkPages(). + } +} diff --git a/tests/snapshots/src/main/scala/tests/MinimizedSnapshotScipGenerator.scala b/tests/snapshots/src/main/scala/tests/MinimizedSnapshotScipGenerator.scala index 9c8150b8b..b608943da 100644 --- a/tests/snapshots/src/main/scala/tests/MinimizedSnapshotScipGenerator.scala +++ b/tests/snapshots/src/main/scala/tests/MinimizedSnapshotScipGenerator.scala @@ -76,6 +76,7 @@ class MinimizedSnapshotScipGenerator extends SnapshotGenerator { document.getRelativePath ).mkString("/") ) + val absolutePath = AbsolutePath(Paths.get(uri)) val text = FileIO.slurp(absolutePath, StandardCharsets.UTF_8) ScipPrinters.printTextDocument(document, text)