feat(serving): render MODIFIERs in signatures (#5819)

kythe · Aug 30, 2023 · f9d97b2 · f9d97b2
1 parent 55e39a6
commit f9d97b2
Show file tree

Hide file tree

Showing 8 changed files with 116 additions and 23 deletions.
diff --git a/kythe/cxx/doc/html_renderer.cc b/kythe/cxx/doc/html_renderer.cc
@@ -449,6 +449,7 @@ std::string RenderSignature(const HtmlRendererOptions& options,
   state.render_identifier = true;
   state.render_types = true;
   state.render_parameters = true;
+  state.render_modifier = true;
   state.linkify = linkify;
   state.options = &options;
   state.base_ticket = base_ticket;

diff --git a/kythe/go/indexer/testdata/code/rendered.go b/kythe/go/indexer/testdata/code/rendered.go
@@ -11,6 +11,7 @@ type S struct{}
 
 // - @M defines/binding M
 // - M.code/rendered/qualified_name "rendered.S.M"
+// - M.code/rendered/signature "func (s *S) M()"
 // - M.code/rendered/callsite_signature "(s) M()"
 func (s *S) M() {}
 
@@ -28,5 +29,5 @@ func (s *S) MArg(arg int) {}
 
 // - @H defines/binding H?
 // - H.code/rendered/callsite_signature "H(param)"
-// - H.code/rendered/signature "H(param func() (string, error)) error"
+// - H.code/rendered/signature "func H(param func() (string, error)) error"
 func H(param func() (string, error)) error { return nil }
diff --git a/kythe/go/util/markedsource/markedsource.go b/kythe/go/util/markedsource/markedsource.go
@@ -108,6 +108,7 @@ func RenderSignature(node *cpb.MarkedSource, format ContentType, linkify func(st
 		cpb.MarkedSource_IDENTIFIER: true,
 		cpb.MarkedSource_TYPE:       true,
 		cpb.MarkedSource_PARAMETER:  true,
+		cpb.MarkedSource_MODIFIER:   true,
 	}
 	r := &renderer{
 		buffer:  &content{},

diff --git a/kythe/java/com/google/devtools/kythe/analyzers/java/JavaEntrySets.java b/kythe/java/com/google/devtools/kythe/analyzers/java/JavaEntrySets.java
@@ -264,9 +264,23 @@ public EntrySet newPackageNodeAndEmit(String name) {
                 .setProperty(
                     "code",
                     MarkedSource.newBuilder()
-                        .setPreText(name)
-                        .setKind(MarkedSource.Kind.IDENTIFIER)
-                        .build()));
+                        .addChild(
+                            MarkedSource.newBuilder()
+                                .setPostChildText(" ")
+                                .setAddFinalListToken(true)
+                                .addChild(
+                                    MarkedSource.newBuilder()
+                                        .setPreText("package")
+                                        .setKind(MarkedSource.Kind.MODIFIER)
+                                        .build())
+                                .build())
+                        .addChild(
+                            MarkedSource.newBuilder()
+                                .setPreText(name)
+                                .setKind(MarkedSource.Kind.IDENTIFIER)
+                                .build())
+                        .build())
+                .build());
     return node;
   }
 

diff --git a/kythe/java/com/google/devtools/kythe/analyzers/java/MarkedSources.java b/kythe/java/com/google/devtools/kythe/analyzers/java/MarkedSources.java
@@ -16,6 +16,9 @@
 
 package com.google.devtools.kythe.analyzers.java;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Sets;
 import com.google.devtools.kythe.platform.java.helpers.SignatureGenerator;
 import com.google.devtools.kythe.proto.MarkedSource;
 import com.google.devtools.kythe.proto.Storage.VName;
@@ -30,6 +33,7 @@
 import java.util.Optional;
 import java.util.function.Function;
 import javax.lang.model.element.ElementKind;
+import javax.lang.model.element.Modifier;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** {@link MarkedSource} utility class. */
@@ -132,6 +136,21 @@ private static MarkedSource construct(
       @Nullable Iterable<MarkedSource> postChildren,
       @Nullable MarkedSource markedType) {
     MarkedSource.Builder markedSource = msBuilder == null ? MarkedSource.newBuilder() : msBuilder;
+    ImmutableSortedSet<Modifier> modifiers = getModifiers(sym);
+    MarkedSource.Builder mods = null;
+    if (!modifiers.isEmpty()
+        || ImmutableSet.of(
+                ElementKind.CLASS, ElementKind.ENUM, ElementKind.INTERFACE, ElementKind.PACKAGE)
+            .contains(sym.getKind())) {
+      mods = markedSource.addChildBuilder().setPostChildText(" ").setAddFinalListToken(true);
+      for (Modifier m : modifiers) {
+        mods.addChild(
+            MarkedSource.newBuilder()
+                .setKind(MarkedSource.Kind.MODIFIER)
+                .setPreText(m.toString())
+                .build());
+      }
+    }
     if (markedType != null && sym.getKind() != ElementKind.CONSTRUCTOR) {
       markedSource.addChild(markedType);
     }
@@ -163,6 +182,11 @@ private static MarkedSource construct(
       case ENUM:
       case CLASS:
       case INTERFACE:
+        mods.addChild(
+            MarkedSource.newBuilder()
+                .setKind(MarkedSource.Kind.MODIFIER)
+                .setPreText(sym.getKind().toString().toLowerCase())
+                .build());
         markedSource.addChildBuilder().setKind(MarkedSource.Kind.IDENTIFIER).setPreText(identToken);
         if (!sym.getTypeParameters().isEmpty()) {
           markedSource
@@ -183,6 +207,27 @@ private static MarkedSource construct(
     return markedSource.build();
   }
 
+  private static ImmutableSortedSet<Modifier> getModifiers(Symbol sym) {
+    ImmutableSortedSet<Modifier> modifiers = ImmutableSortedSet.copyOf(sym.getModifiers());
+    switch (sym.getKind()) {
+      case ENUM:
+        // Remove synthesized enum modifiers
+        return ImmutableSortedSet.copyOf(
+            Sets.difference(modifiers, ImmutableSet.of(Modifier.STATIC, Modifier.FINAL)));
+      case INTERFACE:
+        // Remove synthesized interface modifiers
+        return ImmutableSortedSet.copyOf(
+            Sets.difference(modifiers, ImmutableSet.of(Modifier.ABSTRACT, Modifier.STATIC)));
+      case ENUM_CONSTANT:
+        // Remove synthesized enum constantc modifiers
+        return ImmutableSortedSet.copyOf(
+            Sets.difference(
+                modifiers, ImmutableSet.of(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)));
+      default:
+        return modifiers;
+    }
+  }
+
   /**
    * Sets the provided {@link MarkedSource.Builder} to a CONTEXT node, populating it with the
    * fully-qualified parent scope for sym. Returns the identifier corresponding to sym.

diff --git a/kythe/java/com/google/devtools/kythe/doc/MarkedSourceRenderer.java b/kythe/java/com/google/devtools/kythe/doc/MarkedSourceRenderer.java
@@ -31,6 +31,7 @@
 /** Renders MarkedSource messages into user-printable {@link SafeHtml}. */
 public class MarkedSourceRenderer {
   private MarkedSourceRenderer() {}
+
   /** Don't recurse more than this many times when rendering MarkedSource. */
   private static final int MAX_RENDER_DEPTH = 10;
 
@@ -61,7 +62,10 @@ public static String renderSignatureText(MarkedSource signature) {
         .renderText(
             signature,
             Sets.immutableEnumSet(
-                MarkedSource.Kind.IDENTIFIER, MarkedSource.Kind.TYPE, MarkedSource.Kind.PARAMETER),
+                MarkedSource.Kind.IDENTIFIER,
+                MarkedSource.Kind.TYPE,
+                MarkedSource.Kind.PARAMETER,
+                MarkedSource.Kind.MODIFIER),
             0);
   }
 
@@ -311,6 +315,7 @@ private void append(String text) {
         bufferIsNonempty = true;
       }
     }
+
     /**
      * Escapes and adds {@code text} before the (non-empty) text that would be added by the next
      * call to {@link append}.
@@ -321,6 +326,7 @@ private void appendFinalListToken(String text) {
       }
       prependBuffer.append(text);
     }
+
     /**
      * Make sure there's a space between the current content of the buffer and whatever is appended
      * to it later on.
@@ -330,14 +336,19 @@ private void appendHeuristicSpace() {
         appendFinalListToken(" ");
       }
     }
+
     /** The buffer used to hold escaped HTML data. */
     private SafeHtmlBuilder htmlBuffer;
+
     /** The buffer used to hold text data. */
     private StringBuilder textBuffer;
+
     /** True if the last character in buffer is a space. */
     private boolean bufferEndsInSpace;
+
     /** True if the buffer is non-empty. */
     private boolean bufferIsNonempty;
+
     /**
      * Unescaped text that should be escaped and appended before any other text is appended to the
      * buffer.

diff --git a/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/MarkedSource.java b/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/MarkedSource.java
@@ -1,5 +1,6 @@
 //- @pkg ref Package
-//- Package code PackageId
+//- Package code PackageCode
+//- PackageCode child.1 PackageId
 //- PackageId.kind "IDENTIFIER"
 //- PackageId.pre_text "pkg"
 package pkg;
@@ -10,10 +11,10 @@
 //- @MarkedSource defines/binding Class
 //- Class childof Package
 //- Class code ClassId
-//- ClassId child.0 ClassCxt
+//- ClassId child.1 ClassCxt
 //- ClassCxt child.0 ClassCxtPackage
 //- ClassCxtPackage.pre_text "pkg"
-//- ClassId child.1 ClassTok
+//- ClassId child.2 ClassTok
 //- ClassTok.kind "IDENTIFIER"
 //- ClassTok.pre_text "MarkedSource"
 public class MarkedSource {
@@ -32,13 +33,13 @@ public class MarkedSource {
 
   //- @CONSTANT defines/binding Constant
   //- Constant code CMS
-  //- CMS child.0 ConstantType
+  //- CMS child.1 ConstantType
   //- ConstantType.kind "TYPE"
-  //- CMS child.1 ConstantCtx
+  //- CMS child.2 ConstantCtx
   //- ConstantCtx.kind "CONTEXT"
-  //- CMS child.2 ConstantIdent
+  //- CMS child.3 ConstantIdent
   //- ConstantIdent.kind "IDENTIFIER"
-  //- CMS child.3 ConstantInit
+  //- CMS child.4 ConstantInit
   //- ConstantInit.kind "INITIALIZER"
   //- ConstantInit.pre_text "\"value\""
   public static final String CONSTANT = "value";
@@ -65,9 +66,9 @@ public class MarkedSource {
   //- Ctor typed CType
   //- CType param.1 Class
   //- Ctor code CtorTypeCxtId
-  //- CtorTypeCxtId child.0 CtorCxt
-  //- CtorTypeCxtId child.1 CtorTok
-  //- CtorTypeCxtId child.2 CtorParams
+  //- CtorTypeCxtId child.1 CtorCxt
+  //- CtorTypeCxtId child.2 CtorTok
+  //- CtorTypeCxtId child.3 CtorParams
   //- CtorCxt child.1 CtorCxtClass
   //- CtorCxtClass.pre_text "MarkedSource"
   //- CtorTok.pre_text "MarkedSource"
@@ -315,13 +316,13 @@ static class InnerStatic<T extends Object> {
         //- @field defines/binding AnonField
         //- AnonField childof AnonClass
         //- AnonClass code AnonId
-        //- AnonId child.0 AnonCxt
+        //- AnonId child.1 AnonCxt
         //- AnonCxt.kind "CONTEXT"
         //- AnonCxt child.0 PkgToken
         //- PkgToken.pre_text pkg
         //- AnonCxt child.1 MksToken
         //- MksToken.pre_text "MarkedSource"
-        //- AnonId child.1 AnonToken
+        //- AnonId child.2 AnonToken
         //- AnonToken.kind "IDENTIFIER"
         //- AnonToken.pre_text "(anon 1)"
         int field;
@@ -367,7 +368,7 @@ public static Object func() {
     //- @LocalClass defines/binding LocalClass
     //- LocalClass childof Func
     //- LocalClass code LocalClassId
-    //- LocalClassId child.0 LocalClassCxt
+    //- LocalClassId child.1 LocalClassCxt
     //- LocalClassCxt child.0 LocalClassCxtPackage
     //- LocalClassCxtPackage.pre_text "pkg"
     //- LocalClassCtx child.1 LocalClassOuter
@@ -376,7 +377,7 @@ public static Object func() {
     //- LocalClassCtx child.2 LocalClassFuncOuter
     //- LocalClassFuncOuter.kind "IDENTIFIER"
     //- LocalClassFuncOuter.pre_text "func"
-    //- LocalClassId child.1 LocalClassName
+    //- LocalClassId child.2 LocalClassName
     //- LocalClassName.kind "IDENTIFIER"
     //- LocalClassName.pre_text "LocalClass"
     class LocalClass {}
@@ -432,10 +433,10 @@ static class Generic<T> {}
 
   //- String code StringId
   //- StringId.kind "BOX"
-  //- StringId child.0 StringCxt
+  //- StringId child.1 StringCxt
   //- StringCxt.post_child_text "."
   //- StringCxt.add_final_list_token true
-  //- StringId child.1 StringIdToken
+  //- StringId child.2 StringIdToken
   //- StringCxt.kind "CONTEXT"
   //- StringCxt child.0 JavaId
   //- JavaId.kind "IDENTIFIER"

diff --git a/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/RenderedCode.java b/kythe/javatests/com/google/devtools/kythe/analyzers/java/testdata/pkg/RenderedCode.java
@@ -1,18 +1,37 @@
+// - @pkg ref Package
+// - Package.code/rendered/signature "package pkg"
 package pkg;
 
 // - @RenderedCode defines/binding RenderedCode
+// - RenderedCode.code/rendered/signature "public final class RenderedCode"
 // - RenderedCode.code/rendered/qualified_name "pkg.RenderedCode"
 public final class RenderedCode {
   // - @RenderedCode defines/binding Constructor
   // - Constructor.code/rendered/qualified_name "pkg.RenderedCode.RenderedCode"
-  // - Constructor.code/rendered/signature "RenderedCode()"
+  // - Constructor.code/rendered/signature "private RenderedCode()"
   // - Constructor.code/rendered/callsite_signature "RenderedCode()"
   private RenderedCode() {}
 
   // - @Inner defines/binding Inner
   // - Inner.code/rendered/qualified_name "pkg.RenderedCode.Inner"
-  // - Inner.code/rendered/signature "Inner<T, U>"
+  // - Inner.code/rendered/signature "public static class Inner<T, U>"
   // - Inner tparam.0 T
   // - T.code/rendered/qualified_name "pkg.RenderedCode.Inner.T"
   public static class Inner<T, U> {}
+
+  // - @CONSTANT defines/binding Constant
+  // - Constant.code/rendered/signature "public static final String CONSTANT"
+  public static final String CONSTANT = "blah";
+
+  // - @E defines/binding E
+  // - E.code/rendered/signature "protected enum E"
+  protected enum E {
+    // - @VALUE defines/binding Value
+    // - Value.code/rendered/signature "E VALUE"
+    VALUE
+  }
+
+  // - @I defines/binding I
+  // - I.code/rendered/signature "interface I"
+  interface I {}
 }