Refactor ScipOccurrences to a stateful accumulator; drop ScipRole

Author: jupblb

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/ScipOccurrences.java

@@ -2,29 +2,22 @@
 
 import org.scip_code.scip.Occurrence;
 
-import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Objects;
 
 /**
- * Helpers for deduplicating SCIP {@link Occurrence} entries by their {@code (symbol, range, roles)}
- * key. Variants that differ only in whether {@code enclosing_range} is set are collapsed,
+ * Accumulator that deduplicates SCIP {@link Occurrence} entries by their {@code (symbol, range,
+ * roles)} key. Variants that differ only in whether {@code enclosing_range} is set are collapsed,
  * preferring the one that carries the enclosing range.
  */
 final class ScipOccurrences {
 
-  private ScipOccurrences() {}
+  private final LinkedHashMap<Key, Occurrence> out = new LinkedHashMap<>();
 
-  /** Returns a new list with duplicate occurrences collapsed in insertion order. */
-  static List<Occurrence> deduplicate(List<Occurrence> occurrences) {
-    LinkedHashMap<Key, Occurrence> out = new LinkedHashMap<>();
-    for (Occurrence occ : occurrences) put(out, occ);
-    return new ArrayList<>(out.values());
-  }
-
-  /** Inserts {@code occ} into {@code out}, collapsing duplicates by {@link Key}. */
-  static void put(LinkedHashMap<Key, Occurrence> out, Occurrence occ) {
+  /** Adds {@code occ}, collapsing it into any existing entry with the same {@link Key}. */
+  void add(Occurrence occ) {
     Key key = Key.of(occ);
     Occurrence existing = out.get(key);
     if (existing == null) {
@@ -36,7 +29,17 @@ static void put(LinkedHashMap<Key, Occurrence> out, Occurrence occ) {
     }
   }
 
-  static final class Key {
+  /** Adds every occurrence in {@code occs}. */
+  void addAll(Iterable<Occurrence> occs) {
+    for (Occurrence occ : occs) add(occ);
+  }
+
+  /** Returns the deduplicated occurrences in insertion order. */
+  Collection<Occurrence> values() {
+    return out.values();
+  }
+
+  private static final class Key {
     final String symbol;
     final List<Integer> range;
     final int roles;

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/ScipRole.java

@@ -85,9 +85,9 @@ private static Document mergeDocuments(Document a, Document b) {
 
     // Deduplicate occurrences by (range, symbol, roles). Variants that differ only in
     // enclosing_range get collapsed, preferring the one that carries the enclosing range.
-    LinkedHashMap<ScipOccurrences.Key, Occurrence> occurrences = new LinkedHashMap<>();
-    for (Occurrence occ : a.getOccurrencesList()) ScipOccurrences.put(occurrences, occ);
-    for (Occurrence occ : b.getOccurrencesList()) ScipOccurrences.put(occurrences, occ);
+    ScipOccurrences occurrences = new ScipOccurrences();
+    occurrences.addAll(a.getOccurrencesList());
+    occurrences.addAll(b.getOccurrencesList());
     builder.addAllOccurrences(occurrences.values());
 
     // Deduplicate symbols by symbol string; merge relationships and documentation.

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/ScipShardWriter.java

@@ -68,7 +68,7 @@ public final class ScipVisitor extends TreePathScanner<Void, Void> {
   private final CompilationUnitTree compUnitTree;
   private final Elements elements;
   private final SemanticdbJavacOptions options;
-  private final ArrayList<Occurrence> occurrences;
+  private final ScipOccurrences occurrences;
   private final LinkedHashMap<String, SymbolInformation> symbols;
   private final String source;
   private final String relativePath;
@@ -88,7 +88,7 @@ public ScipVisitor(
     this.elements = elements;
     this.trees = trees;
     this.compUnitTree = compUnitTree;
-    this.occurrences = new ArrayList<>();
+    this.occurrences = new ScipOccurrences();
     this.symbols = new LinkedHashMap<>();
     this.source = sourceText(compUnitTree);
     this.relativePath = sourceRelativePath(compUnitTree, options);
@@ -105,7 +105,7 @@ public Index buildShard(CompilationUnitTree tree) {
     if (options.includeText) {
       document.setText(source);
     }
-    document.addAllOccurrences(ScipOccurrences.deduplicate(occurrences));
+    document.addAllOccurrences(occurrences.values());
     document.addAllSymbols(symbols.values());
 
     return Index.newBuilder().addDocuments(document).build();
@@ -119,20 +119,20 @@ public Index buildShard(CompilationUnitTree tree) {
   // ==========================
 
   private Optional<ScipRange> emitSymbolOccurrence(
-      Element sym, Tree tree, Name name, ScipRole role, CompilerRange kind) {
+      Element sym, Tree tree, Name name, int roles, CompilerRange kind) {
     if (sym == null || name == null) return Optional.empty();
     Optional<ScipRange> range = scipRangeOf(tree, kind, sym, name.toString());
-    if (role == ScipRole.DEFINITION) {
-      emitOccurrence(sym, range, role, computeEnclosingRange(tree));
+    if (roles == SymbolRole.Definition_VALUE) {
+      emitOccurrence(sym, range, roles, computeEnclosingRange(tree));
       emitSymbolInformation(sym, tree);
       return range;
     }
-    emitOccurrence(sym, range, role, Optional.empty());
+    emitOccurrence(sym, range, roles, Optional.empty());
     return range;
   }
 
   private void emitOccurrence(
-      Element sym, Optional<ScipRange> range, ScipRole role, Optional<ScipRange> enclosingRange) {
+      Element sym, Optional<ScipRange> range, int roles, Optional<ScipRange> enclosingRange) {
     if (sym == null || !range.isPresent()) return;
     String semanticdbSymbol = semanticdbSymbol(sym);
     if (semanticdbSymbol.equals(SemanticdbSymbols.NONE)) return;
@@ -141,7 +141,7 @@ private void emitOccurrence(
         Occurrence.newBuilder()
             .addAllRange(range.get().asScipRange())
             .setSymbol(ScipSymbols.fromSemanticdbSymbol(semanticdbSymbol))
-            .setSymbolRoles(scipRole(role));
+            .setSymbolRoles(roles);
     enclosingRange.ifPresent(r -> occ.addAllEnclosingRange(r.asScipRange()));
     occurrences.add(occ.build());
   }
@@ -244,16 +244,9 @@ private static boolean supportsReferenceRelationship(Element sym) {
   }
 
   // =================================================
-  // Role / kind translation for SCIP emission.
+  // Kind translation for SCIP emission.
   // =================================================
 
-  private static int scipRole(ScipRole role) {
-    if (role == ScipRole.DEFINITION || role == ScipRole.SYNTHETIC_DEFINITION) {
-      return SymbolRole.Definition_VALUE;
-    }
-    return 0;
-  }
-
   private static SymbolInformation.Kind scipKind(Element sym) {
     Set<Modifier> modifiers = sym.getModifiers();
     boolean isStatic = modifiers.contains(Modifier.STATIC);
@@ -369,7 +362,7 @@ private void resolveClassTree(ClassTree node, TreePath treePath) {
           sym,
           node,
           sym.getSimpleName(),
-          ScipRole.DEFINITION,
+          SymbolRole.Definition_VALUE,
           CompilerRange.FROM_POINT_WITH_TEXT_SEARCH);
     }
   }
@@ -381,7 +374,7 @@ private void resolveTypeParameterTree(TypeParameterTree node, TreePath treePath)
           sym,
           node,
           sym.getSimpleName(),
-          ScipRole.DEFINITION,
+          SymbolRole.Definition_VALUE,
           CompilerRange.FROM_POINT_TO_SYMBOL_NAME);
     }
   }
@@ -396,7 +389,11 @@ private void resolveMethodTree(MethodTree node, TreePath treePath) {
         else name = sym.getSimpleName();
 
         emitSymbolOccurrence(
-            sym, node, name, ScipRole.DEFINITION, CompilerRange.FROM_POINT_WITH_TEXT_SEARCH);
+            sym,
+            node,
+            name,
+            SymbolRole.Definition_VALUE,
+            CompilerRange.FROM_POINT_WITH_TEXT_SEARCH);
       }
     }
   }
@@ -409,12 +406,12 @@ private void resolveVariableTree(VariableTree node, TreePath treePath) {
               sym,
               node,
               sym.getSimpleName(),
-              ScipRole.DEFINITION,
+              SymbolRole.Definition_VALUE,
               CompilerRange.FROM_POINT_WITH_TEXT_SEARCH);
       if (sym.getKind() == ElementKind.ENUM_CONSTANT) {
         TreePath typeTreePath = nodes.get(node.getInitializer());
         Element typeSym = trees.getElement(typeTreePath);
-        if (typeSym != null) emitOccurrence(typeSym, range, ScipRole.REFERENCE, Optional.empty());
+        if (typeSym != null) emitOccurrence(typeSym, range, 0, Optional.empty());
       }
     }
   }
@@ -431,11 +428,7 @@ private void resolveIdentifierTree(IdentifierTree node, TreePath treePath) {
           Element parentSym = trees.getElement(parentPath);
           if (parentSym == null || parentSym.getKind() != null) {
             emitSymbolOccurrence(
-                sym,
-                node,
-                sym.getSimpleName(),
-                ScipRole.REFERENCE,
-                CompilerRange.FROM_START_TO_END);
+                sym, node, sym.getSimpleName(), 0, CompilerRange.FROM_START_TO_END);
           }
         }
       }
@@ -446,23 +439,15 @@ private void resolveMemberReferenceTree(MemberReferenceTree node, TreePath treeP
     Element sym = trees.getElement(treePath);
     if (sym != null) {
       emitSymbolOccurrence(
-          sym,
-          node,
-          sym.getSimpleName(),
-          ScipRole.REFERENCE,
-          CompilerRange.FROM_END_TO_SYMBOL_NAME);
+          sym, node, sym.getSimpleName(), 0, CompilerRange.FROM_END_TO_SYMBOL_NAME);
     }
   }
 
   private void resolveMemberSelectTree(MemberSelectTree node, TreePath treePath) {
     Element sym = trees.getElement(treePath);
     if (sym != null) {
       emitSymbolOccurrence(
-          sym,
-          node,
-          sym.getSimpleName(),
-          ScipRole.REFERENCE,
-          CompilerRange.FROM_END_TO_SYMBOL_NAME);
+          sym, node, sym.getSimpleName(), 0, CompilerRange.FROM_END_TO_SYMBOL_NAME);
     }
   }
 
@@ -477,18 +462,13 @@ private void resolveNewClassTree(NewClassTree node, TreePath treePath) {
           Element identifierSym = trees.getElement(identifierTreePath);
           if (identifierSym != null) {
             emitSymbolOccurrence(
-                sym,
-                node,
-                identifierSym.getSimpleName(),
-                ScipRole.REFERENCE,
-                CompilerRange.FROM_TEXT_SEARCH);
+                sym, node, identifierSym.getSimpleName(), 0, CompilerRange.FROM_TEXT_SEARCH);
           } else if (node.getIdentifier().getKind() == Tree.Kind.ANNOTATED_TYPE) {
             AnnotatedTypeTree annotatedTypeTree = (AnnotatedTypeTree) node.getIdentifier();
             if (annotatedTypeTree.getUnderlyingType() != null
                 && annotatedTypeTree.getUnderlyingType().getKind() == Tree.Kind.IDENTIFIER) {
               IdentifierTree ident = (IdentifierTree) annotatedTypeTree.getUnderlyingType();
-              emitSymbolOccurrence(
-                  sym, ident, ident.getName(), ScipRole.REFERENCE, CompilerRange.FROM_TEXT_SEARCH);
+              emitSymbolOccurrence(sym, ident, ident.getName(), 0, CompilerRange.FROM_TEXT_SEARCH);
             }
           }
         }