fix(java_indexer): blame instance/static callsites on ctor/cinit methods (kythe#4734)

This matches the C++ indexer's existing behavior and better represents the actual callgraph.

The cinit function is newly represented in this commit as a bare function node per class.

Author: schroederc

kythe/java/com/google/devtools/kythe/analyzers/java/JavaEntrySets.java

@@ -179,6 +179,16 @@ public EntrySet newLambdaAndEmit(Positions filePositions, JCTree.JCLambda lambda
             .addSignatureSalt("" + filePositions.getSpan(lambda)));
   }
 
+  /** Emits and returns a new {@link EntrySet} representing a static class initializer. */
+  public EntrySet newClassInitAndEmit(VName classNode) {
+    return emitAndReturn(
+        newNode(NodeKind.FUNCTION)
+            .setCorpusPath(CorpusPath.fromVName(classNode))
+            .addSignatureSalt(classNode)
+            .addSignatureSalt("clinit")
+            .build());
+  }
+
   /**
    * Emits and returns a new {@link EntrySet} representing a Java comment with an optional subkind.
    */

kythe/java/com/google/devtools/kythe/analyzers/java/JavaNode.java

@@ -19,6 +19,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.kythe.analyzers.base.EntrySet;
 import com.google.devtools.kythe.proto.Storage.VName;
+import java.util.List;
+import java.util.Optional;
 
 /** Kythe graph node representing a Java language construct. */
 class JavaNode {
@@ -27,6 +29,9 @@ class JavaNode {
   private final VName vName;
   private JavaNode type;
 
+  private List<VName> classConstructors = ImmutableList.of();
+  private Optional<VName> classInit = Optional.empty();
+
   // I think order matters for the wildcards because the abs node will be connected to them with
   // param edges, which are numbered. If order doesn't matter, we should change this to something
   // like bazel's NestedSet.
@@ -65,4 +70,22 @@ JavaNode setType(JavaNode type) {
   JavaNode getType() {
     return type;
   }
+
+  JavaNode setClassConstructors(List<VName> constructors) {
+    this.classConstructors = constructors;
+    return this;
+  }
+
+  List<VName> getClassConstructors() {
+    return classConstructors;
+  }
+
+  JavaNode setClassInit(VName classInit) {
+    this.classInit = Optional.ofNullable(classInit);
+    return this;
+  }
+
+  Optional<VName> getClassInit() {
+    return classInit;
+  }
 }

kythe/java/com/google/devtools/kythe/analyzers/java/KytheTreeScanner.java

@@ -64,6 +64,7 @@
 import com.sun.tools.javac.tree.JCTree.JCAssert;
 import com.sun.tools.javac.tree.JCTree.JCAssign;
 import com.sun.tools.javac.tree.JCTree.JCAssignOp;
+import com.sun.tools.javac.tree.JCTree.JCBlock;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
 import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
 import com.sun.tools.javac.tree.JCTree.JCExpression;
@@ -336,7 +337,7 @@ public JavaNode visitClassDef(JCClassDecl classDef, TreeContext owner) {
     // However we can't restrict ourselves to just classes contained in methods here,
     // because that would miss the case of local/anonymous classes in static/member
     // initializers. But there's no harm in emitting the same fact twice!
-    getScope(ctx).ifPresent(scope -> entrySets.emitEdge(classNode, EdgeKind.CHILDOF, scope));
+    getScope(ctx).forEach(scope -> entrySets.emitEdge(classNode, EdgeKind.CHILDOF, scope));
 
     NestingKind nestingKind = classDef.sym.getNestingKind();
     if (nestingKind != NestingKind.LOCAL
@@ -428,16 +429,58 @@ public JavaNode visitClassDef(JCClassDecl classDef, TreeContext owner) {
     // directly in the class body (in static initializers or member initializers).
     JavaNode node = ctx.setNode(new JavaNode(classNode));
 
+    List<VName> constructors = new ArrayList<>();
     for (JCTree member : classDef.getMembers()) {
-      JavaNode n = scan(member, ctx);
-      if (n != null) {
-        entrySets.emitEdge(n.getVName(), EdgeKind.CHILDOF, classNode);
+      if (member instanceof JCMethodDecl) {
+        JCMethodDecl method = (JCMethodDecl) member;
+        if (!method.sym.isConstructor()) {
+          continue;
+        }
+
+        JavaNode n = scanChild(member, ctx, classNode);
+        if (n != null) {
+          constructors.add(n.getVName());
+        }
+      }
+    }
+    node.setClassConstructors(constructors);
+
+    // TODO(schroeder): flesh out cinit w/ MarkedSource
+    node.setClassInit(entrySets.newClassInitAndEmit(classNode).getVName());
+    entrySets.emitEdge(node.getClassInit().get(), EdgeKind.CHILDOF, classNode);
+
+    for (JCTree member : classDef.getMembers()) {
+      if (member instanceof JCMethodDecl) {
+        JCMethodDecl method = (JCMethodDecl) member;
+        if (method.sym.isConstructor()) {
+          // Already handled above.
+          continue;
+        }
       }
+      scanChild(member, ctx, classNode);
     }
 
     return node;
   }
 
+  private JavaNode scanChild(JCTree child, TreeContext owner, VName parent) {
+    JavaNode n = scan(child, owner);
+    if (n != null) {
+      entrySets.emitEdge(n.getVName(), EdgeKind.CHILDOF, parent);
+    }
+    return n;
+  }
+
+  @Override
+  public JavaNode visitBlock(JCBlock block, TreeContext owner) {
+    TreeContext ctx = owner;
+    if (block.isStatic() && owner.getNode().getClassInit().isPresent()) {
+      ctx = owner.down(block);
+      ctx.setNode(new JavaNode(owner.getNode().getClassInit().get()));
+    }
+    return scan(block.getStatements(), ctx);
+  }
+
   @Override
   public JavaNode visitMethodDef(JCMethodDecl methodDef, TreeContext owner) {
     TreeContext ctx = owner.down(methodDef);
@@ -667,7 +710,7 @@ public JavaNode visitVarDef(JCVariableDecl varDef, TreeContext owner) {
       entrySets.emitEdge(varNode, EdgeKind.NAMED, jvmNode);
     }
 
-    getScope(ctx).ifPresent(scope -> entrySets.emitEdge(varNode, EdgeKind.CHILDOF, scope));
+    getScope(ctx).forEach(scope -> entrySets.emitEdge(varNode, EdgeKind.CHILDOF, scope));
     visitAnnotations(varNode, varDef.getModifiers().getAnnotations(), ctx);
 
     if (varDef.getModifiers().getFlags().contains(Modifier.STATIC)) {
@@ -848,7 +891,7 @@ public JavaNode visitNewClass(JCNewClass newClass, TreeContext owner) {
     emitAnchor(anchor, EdgeKind.REF, ctorNode, getScope(ctx));
 
     EntrySet callAnchor = entrySets.newAnchorAndEmit(filePositions, callSpan, ctx.getSnippet());
-    emitAnchor(callAnchor, EdgeKind.REF_CALL, ctorNode, getScope(ctx));
+    emitAnchor(callAnchor, EdgeKind.REF_CALL, ctorNode, getCallScope(ctx));
 
     scanList(newClass.getTypeArguments(), ctx);
     scanList(newClass.getArguments(), ctx);
@@ -1002,7 +1045,7 @@ void emitDocReference(Symbol sym, int startChar, int endChar) {
             filePositions.charToByteOffset(startChar), filePositions.charToByteOffset(endChar));
     EntrySet anchor = entrySets.newAnchorAndEmit(filePositions, loc);
     if (anchor != null) {
-      emitAnchor(anchor, EdgeKind.REF_DOC, node, Optional.empty());
+      emitAnchor(anchor, EdgeKind.REF_DOC, node, ImmutableList.of());
     }
   }
 
@@ -1180,16 +1223,27 @@ private JavaNode emitNameUsage(TreeContext ctx, Symbol sym, Name name, EdgeKind
           edgeKind,
           node.getVName(),
           ctx.getSnippet(),
-          getScope(ctx));
+          edgeKind == EdgeKind.REF_CALL ? getCallScope(ctx) : getScope(ctx));
       statistics.incrementCounter("name-usages-emitted");
     }
     return node;
   }
 
-  private static Optional<VName> getScope(TreeContext ctx) {
-    return Optional.ofNullable(ctx.getClassOrMethodParent())
+  private static List<VName> getCallScope(TreeContext ctx) {
+    TreeContext parent = ctx.getScope();
+    if (parent.getTree() instanceof JCClassDecl) {
+      // Special-case callsites in non-static initializer blocks to scope to all constructors.
+      return parent.getNode().getClassConstructors();
+    }
+    return getScope(ctx);
+  }
+
+  private static List<VName> getScope(TreeContext ctx) {
+    return Optional.ofNullable(ctx.getScope())
         .map(TreeContext::getNode)
-        .map(JavaNode::getVName);
+        .map(JavaNode::getVName)
+        .map(ImmutableList::of)
+        .orElse(ImmutableList.of());
   }
 
   // Returns the reference node for the given symbol.
@@ -1264,12 +1318,12 @@ private EntrySet emitAnchor(TreeContext anchorContext, EdgeKind kind, VName node
             filePositions, anchorContext.getTreeSpan(), anchorContext.getSnippet()),
         kind,
         node,
-        getScope(anchorContext));
+        kind == EdgeKind.REF_CALL ? getCallScope(anchorContext) : getScope(anchorContext));
   }
 
   // Creates/emits an anchor (for an identifier) and an associated edge
   private EntrySet emitAnchor(
-      Name name, int startOffset, EdgeKind kind, VName node, Span snippet, Optional<VName> scope) {
+      Name name, int startOffset, EdgeKind kind, VName node, Span snippet, List<VName> scope) {
     EntrySet anchor = entrySets.newAnchorAndEmit(filePositions, name, startOffset, snippet);
     if (anchor == null) {
       // TODO(schroederc): Special-case these anchors (most come from visitSelect)
@@ -1304,22 +1358,21 @@ private EntrySet emitDefinesBindingAnchorEdge(
     return anchor;
   }
 
-  private void emitDefinesBindingEdge(
-      Span span, EntrySet anchor, VName node, Optional<VName> scope) {
+  private void emitDefinesBindingEdge(Span span, EntrySet anchor, VName node, List<VName> scope) {
     emitMetadata(span, node);
     emitAnchor(anchor, EdgeKind.DEFINES_BINDING, node, scope);
   }
 
   // Creates/emits an anchor and an associated edge
-  private EntrySet emitAnchor(EntrySet anchor, EdgeKind kind, VName node, Optional<VName> scope) {
+  private EntrySet emitAnchor(EntrySet anchor, EdgeKind kind, VName node, List<VName> scope) {
     Preconditions.checkArgument(
         kind.isAnchorEdge(), "EdgeKind was not intended for ANCHORs: %s", kind);
     if (anchor == null) {
       return null;
     }
     entrySets.emitEdge(anchor.getVName(), kind, node);
     if (kind == EdgeKind.REF_CALL || config.getEmitAnchorScopes()) {
-      scope.ifPresent(s -> entrySets.emitEdge(anchor.getVName(), EdgeKind.CHILDOF, s));
+      scope.forEach(s -> entrySets.emitEdge(anchor.getVName(), EdgeKind.CHILDOF, s));
     }
     return anchor;
   }

kythe/java/com/google/devtools/kythe/analyzers/java/TreeContext.java

@@ -20,6 +20,7 @@
 import com.google.devtools.kythe.analyzers.java.SourceText.Positions;
 import com.google.devtools.kythe.util.Span;
 import com.sun.tools.javac.tree.JCTree;
+import com.sun.tools.javac.tree.JCTree.JCBlock;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
 import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
 import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
@@ -92,10 +93,12 @@ public TreeContext getMethodParent() {
     return parent;
   }
 
-  public TreeContext getClassOrMethodParent() {
+  public TreeContext getScope() {
     TreeContext parent = up();
     while (parent != null
-        && !(parent.getTree() instanceof JCMethodDecl || parent.getTree() instanceof JCClassDecl)) {
+        && !(parent.getTree() instanceof JCMethodDecl
+            || parent.getTree() instanceof JCClassDecl
+            || parent.getTree() instanceof JCBlock)) {
       parent = parent.up();
     }
     return parent;

kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/Callgraph.java

@@ -3,32 +3,53 @@
 //- @Callgraph defines/binding Class
 public class Callgraph {
 
+  // Implicit static class initializer
+  //- ClassInit.node/kind function
+  //- ClassInit childof Class
+  //- !{ ClassInit.subkind "constructor" }
+
   //- StaticGCall.loc/start @^"g()"
   //- StaticGCall.loc/end @$"g()"
   //- StaticGCall ref/call G
-  //- StaticGCall childof Class
+  //- StaticGCall childof ECtor
+  //- StaticGCall childof SCtor
+  //- !{ StaticGCall childof Class }
+  //- !{ StaticGCall childof ClassInit }
   final int ZERO = g();
 
   static {
     //- StaticBlockGCall.loc/start @^"g()"
     //- StaticBlockGCall.loc/end @$"g()"
     //- StaticBlockGCall ref/call G
-    //- StaticBlockGCall childof Class
+    //- StaticBlockGCall childof ClassInit
+    //- !{ StaticBlockGCall childof ECtor }
+    //- !{ StaticBlockGCall childof SCtor }
     int zero = g();
+
+    {
+      //- NestedStaticBlockGCall.loc/start @^"g()"
+      //- NestedStaticBlockGCall.loc/end @$"g()"
+      //- NestedStaticBlockGCall ref/call G
+      //- NestedStaticBlockGCall childof ClassInit
+      //- NestedStaticBlockGCall childof ClassInit
+      g();
+    }
   }
 
   {
     //- BlockGCall.loc/start @^"g()"
     //- BlockGCall.loc/end @$"g()"
     //- BlockGCall ref/call G
-    //- BlockGCall childof Class
+    //- BlockGCall childof ECtor
+    //- BlockGCall childof SCtor
     int zero = g();
   }
 
-  //- StaticCtorCall.loc/start @^"new Callgraph()"
-  //- StaticCtorCall.loc/end @$"new Callgraph()"
-  //- StaticCtorCall ref/call ECtor
-  //- StaticCtorCall childof Class
+  //- CtorCall.loc/start @^"new Callgraph()"
+  //- CtorCall.loc/end @$"new Callgraph()"
+  //- CtorCall ref/call ECtor
+  //- CtorCall childof ECtor
+  //- CtorCall childof SCtor
   final Callgraph INSTANCE = new Callgraph();
 
   //- @Callgraph defines/binding ECtor
@@ -72,4 +93,30 @@ static int g() {
     //- CallAnchor childof  G
     return 0;
   }
+
+  //- @Nested defines/binding NestedClass
+  static class Nested {
+    //- NestedInit.node/kind function
+    //- NestedInit childof NestedClass
+    //- !{ NestedInit.subkind "constructor" }
+
+    //- ImplicitConstructor.node/kind function
+    //- ImplicitConstructor.subkind "constructor"
+    //- ImplicitConstructor childof NestedClass
+
+    //- NestedClassCall.loc/start @^"g()"
+    //- NestedClassCall.loc/end   @$"g()"
+    //- NestedClassCall  childof  ImplicitConstructor
+    final int INT = g();
+
+    static {
+      //- NestedClassStaticCall.loc/start @^"g()"
+      //- NestedClassStaticCall.loc/end   @$"g()"
+      //- NestedClassStaticCall  childof  NestedInit
+      g();
+    }
+
+    //- !{ NestedClassCall childof NestedInit
+    //-    NestedClassStaticCall childof ImplicitConstructor }
+  }
 }

kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/Classes.java

@@ -77,10 +77,13 @@ private void localFunc() {
     class LocalClass {};
   }
 
-
+  //- ClassInit.node/kind function
+  //- ClassInit childof N
+  // !{ ClassInit.subkind "constructor" }
+  
   static {
     //- @LocalClassInStaticInitializer defines/binding LCISI
-    //- LCISI childof N
+    //- LCISI childof ClassInit
     class LocalClassInStaticInitializer {};
   }
 }