Create experimental flag to include source info in proto descriptors

`protoc` has an option to include source info (e.g. comments) in the
generated descriptor sets. Well done proto generators also relay the
comments into the generated source so IDEs that do source indexing
and pick them up to provide contextual tooltips/documentation/etc.

This change adds an experimental flag to include this info in the
descriptors generated by `proto_library` so we can collect some
data whether including source info by default is feasible.

See #9337 for more context.

//cc @thomasvl

Closes #10925.

PiperOrigin-RevId: 299838533

src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java

@@ -14,12 +14,12 @@
 
 package com.google.devtools.build.lib.rules.proto;
 
-import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.Iterables.isEmpty;
 import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
 import static com.google.devtools.build.lib.rules.proto.ProtoCommon.areDepsStrict;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -337,7 +337,9 @@ public static void writeDescriptorSet(
     SpawnAction.Builder actions =
         createActions(
             ruleContext,
-            ImmutableList.of(createDescriptorSetToolchain(output.getExecPathString())),
+            ImmutableList.of(
+                createDescriptorSetToolchain(
+                    ruleContext.getFragment(ProtoConfiguration.class), output.getExecPathString())),
             protoInfo,
             ruleContext.getLabel(),
             ImmutableList.of(output),
@@ -353,7 +355,13 @@ public static void writeDescriptorSet(
     ruleContext.registerAction(actions.build(ruleContext));
   }
 
-  private static ToolchainInvocation createDescriptorSetToolchain(CharSequence outReplacement) {
+  private static ToolchainInvocation createDescriptorSetToolchain(
+      ProtoConfiguration configuration, CharSequence outReplacement) {
+    ImmutableList.Builder<String> protocOpts = ImmutableList.builder();
+    if (configuration.experimentalProtoDescriptorSetsIncludeSourceInfo()) {
+      protocOpts.add("--include_source_info");
+    }
+
     return new ToolchainInvocation(
         "dontcare",
         ProtoLangToolchainProvider.create(
@@ -365,7 +373,8 @@ private static ToolchainInvocation createDescriptorSetToolchain(CharSequence out
             /* pluginExecutable= */ null,
             /* runtime= */ null,
             /* blacklistedProtos= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER)),
-        outReplacement);
+        outReplacement,
+        protocOpts.build());
   }
 
   /** Whether to use exports in the proto compile action. */
@@ -552,6 +561,8 @@ static CustomCommandLine createCommandLineFromToolchains(
             "--plugin=protoc-gen-PLUGIN_%s=%s",
             invocation.name, toolchain.pluginExecutable().getExecutable().getExecPath());
       }
+
+      cmdLine.addAll(invocation.protocOpts);
     }
 
     cmdLine.addAll(protocOpts);
@@ -749,13 +760,23 @@ public static class ToolchainInvocation {
     final String name;
     public final ProtoLangToolchainProvider toolchain;
     final CharSequence outReplacement;
+    final ImmutableList<String> protocOpts;
 
     public ToolchainInvocation(
         String name, ProtoLangToolchainProvider toolchain, CharSequence outReplacement) {
-      checkState(!name.contains(" "), "Name %s should not contain spaces", name);
+      this(name, toolchain, outReplacement, ImmutableList.of());
+    }
+
+    public ToolchainInvocation(
+        String name,
+        ProtoLangToolchainProvider toolchain,
+        CharSequence outReplacement,
+        ImmutableList<String> protocOpts) {
+      Preconditions.checkState(!name.contains(" "), "Name %s should not contain spaces", name);
       this.name = name;
       this.toolchain = toolchain;
       this.outReplacement = outReplacement;
+      this.protocOpts = Preconditions.checkNotNull(protocOpts);
     }
   }
 }

src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java

@@ -74,6 +74,15 @@ public static class Options extends FragmentOptions {
     )
     public boolean experimentalProtoExtraActions;
 
+    @Option(
+        name = "experimental_proto_descriptor_sets_include_source_info",
+        defaultValue = "false",
+        documentationCategory = OptionDocumentationCategory.OUTPUT_SELECTION,
+        effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
+        metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+        help = "Run extra actions for alternative Java api versions in a proto_library.")
+    public boolean experimentalProtoDescriptorSetsIncludeSourceInfo;
+
     @Option(
         name = "proto_compiler",
         defaultValue = "@com_google_protobuf//:protoc",
@@ -181,6 +190,8 @@ public FragmentOptions getHost() {
       host.loadProtoRulesFromBzl = loadProtoRulesFromBzl;
       host.protoCompiler = protoCompiler;
       host.protocOpts = protocOpts;
+      host.experimentalProtoDescriptorSetsIncludeSourceInfo =
+          experimentalProtoDescriptorSetsIncludeSourceInfo;
       host.experimentalProtoExtraActions = experimentalProtoExtraActions;
       host.protoCompiler = protoCompiler;
       host.protoToolchainForJava = protoToolchainForJava;
@@ -234,6 +245,10 @@ public ImmutableList<String> protocOpts() {
     return protocOpts;
   }
 
+  public boolean experimentalProtoDescriptorSetsIncludeSourceInfo() {
+    return options.experimentalProtoDescriptorSetsIncludeSourceInfo;
+  }
+
   /**
    * Returns true if we will run extra actions for actions that are not run by default. If this
    * is enabled, e.g. all extra_actions for alternative api-versions or language-flavours of a

src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java

@@ -1003,4 +1003,41 @@ public void testMigrationLabelNotRequiredWhenDisabled() throws Exception {
 
     getConfiguredTarget("//a");
   }
+
+  @Test
+  public void testNoExperimentalProtoDescriptorSetsIncludeSourceInfo() throws Exception {
+    if (!isThisBazel()) {
+      return;
+    }
+
+    scratch.file(
+        "x/BUILD",
+        TestConstants.LOAD_PROTO_LIBRARY,
+        "proto_library(",
+        "    name = 'a_proto',",
+        "    srcs = ['a.proto'],",
+        ")");
+
+    Iterable<String> commandLine = paramFileArgsForAction(getDescriptorWriteAction("//x:a_proto"));
+    assertThat(commandLine).doesNotContain("--include_source_info");
+  }
+
+  @Test
+  public void testExperimentalProtoDescriptorSetsIncludeSourceInfo() throws Exception {
+    if (!isThisBazel()) {
+      return;
+    }
+
+    useConfiguration("--experimental_proto_descriptor_sets_include_source_info");
+    scratch.file(
+        "x/BUILD",
+        TestConstants.LOAD_PROTO_LIBRARY,
+        "proto_library(",
+        "    name = 'a_proto',",
+        "    srcs = ['a.proto'],",
+        ")");
+
+    Iterable<String> commandLine = paramFileArgsForAction(getDescriptorWriteAction("//x:a_proto"));
+    assertThat(commandLine).contains("--include_source_info");
+  }
 }