starlark_doc_extract: match legacy Stardoc behavior for private aspec…

…t attrs and attr.license()

For private aspect propagation attrs, legacy Stardoc does not emit documentation.

For the deprecated attr.license() attributes, legacy Stardoc pretends the attribute
is a string list, so we do the same. (Note that we have to finally fix
License.repr() to return something sensible that can be parsed by
BuildType.LICENSE.convert() - i.e. by License.parseLicense() - because
starlark_doc_extract, unlike legacy Stardoc, correctly processes attribute default
values.)

PiperOrigin-RevId: 543457875
Change-Id: I5d01312e30ccdc8b1d71322f496ec8a237fa993d

src/main/java/com/google/devtools/build/lib/packages/License.java

@@ -14,6 +14,9 @@
 
 package com.google.devtools.build.lib.packages;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.base.Ascii;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -27,6 +30,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
+import java.util.stream.Stream;
 import net.starlark.java.eval.Printer;
 
 /** Support for license and distribution checking. */
@@ -35,6 +39,7 @@
 public final class License implements LicenseApi {
   private final ImmutableSet<LicenseType> licenseTypes;
   private final ImmutableSet<Label> exceptions;
+  private static final String EXCEPTION_PREFIX = "exception=";
 
   /**
    * The error that's thrown if a build file contains an invalid license string.
@@ -161,9 +166,9 @@ public static License parseLicense(List<String> licStrings) throws LicenseParsin
     Set<LicenseType> licenseTypes = EnumSet.noneOf(LicenseType.class);
     Set<Label> exceptions = Sets.newTreeSet();
     for (String str : licStrings) {
-      if (str.startsWith("exception=")) {
+      if (str.startsWith(EXCEPTION_PREFIX)) {
         try {
-          Label label = Label.parseCanonical(str.substring("exception=".length()));
+          Label label = Label.parseCanonical(str.substring(EXCEPTION_PREFIX.length()));
           exceptions.add(label);
         } catch (LabelSyntaxException e) {
           throw new LicenseParsingException(e.getMessage());
@@ -210,9 +215,9 @@ public boolean isSpecified() {
   @Override
   public String toString() {
     if (exceptions.isEmpty()) {
-      return licenseTypes.toString().toLowerCase();
+      return Ascii.toLowerCase(licenseTypes.toString());
     } else {
-      return licenseTypes.toString().toLowerCase() + " with exceptions " + exceptions;
+      return Ascii.toLowerCase(licenseTypes.toString()) + " with exceptions " + exceptions;
     }
   }
 
@@ -241,8 +246,21 @@ public boolean isImmutable() {
     return true; // licences are Starlark-hashable
   }
 
+  /**
+   * Represents the License as a canonically ordered list of strings that can be parsed by {@link
+   * License#parseLicense} to get back an equal License.
+   */
   @Override
   public void repr(Printer printer) {
-    printer.append(this.toString());
+    // The order of license types is guaranteed to be canonical by EnumSet, and the order of
+    // exceptions is guaranteed to be lexicographic order by TreeSet.
+    printer.printList(
+        Stream.concat(
+                licenseTypes.stream().map(licenseType -> Ascii.toLowerCase(licenseType.toString())),
+                exceptions.stream().map(label -> EXCEPTION_PREFIX + label.getCanonicalForm()))
+            .collect(toImmutableList()),
+        "[",
+        ", ",
+        "]");
   }
 }

src/main/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractor.java

@@ -383,7 +383,11 @@ protected void visitAspect(String qualifiedName, StarlarkDefinedAspect aspect)
               .setFile(
                   aspect.getAspectClass().getExtensionLabel().getDisplayForm(repositoryMapping)));
       aspect.getDocumentation().ifPresent(aspectInfoBuilder::setDocString);
-      aspectInfoBuilder.addAllAspectAttribute(aspect.getAttributeAspects());
+      for (String aspectAttribute : aspect.getAttributeAspects()) {
+        if (isPublicName(aspectAttribute)) {
+          aspectInfoBuilder.addAspectAttribute(aspectAttribute);
+        }
+      }
       aspectInfoBuilder.addAttribute(IMPLICIT_NAME_ATTRIBUTE_INFO); // name comes first
       for (Attribute attribute : aspect.getAttributes()) {
         if (isPublicName(attribute.getPublicName())) {
@@ -472,6 +476,12 @@ private static AttributeType getAttributeType(Attribute attribute, String where)
         return AttributeType.OUTPUT;
       } else if (type.equals(BuildType.OUTPUT_LIST)) {
         return AttributeType.OUTPUT_LIST;
+      } else if (type.equals(BuildType.LICENSE)) {
+        // TODO(https://github.com/bazelbuild/bazel/issues/6420): deprecated, disabled in Bazel by
+        // default, broken and with almost no remaining users, so we don't have an AttributeType for
+        // it. Until this type is removed, following the example of legacy Stardoc, pretend it's a
+        // list of strings.
+        return AttributeType.STRING_LIST;
       }
 
       throw new ExtractionException(

src/test/java/com/google/devtools/build/lib/packages/LicenseTest.java

@@ -15,8 +15,17 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.packages.License.LicenseType;
 import java.util.Arrays;
+import net.starlark.java.eval.Module;
+import net.starlark.java.eval.Mutability;
+import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
+import net.starlark.java.syntax.FileOptions;
+import net.starlark.java.syntax.ParserInput;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -35,4 +44,41 @@ public void testLeastRestrictive() {
     assertThat(License.leastRestrictive(Arrays.<LicenseType>asList()))
         .isEqualTo(LicenseType.BY_EXCEPTION_ONLY);
   }
+
+  /** Evaluates a string as a Starlark expression returning a sequence of strings. */
+  private static Sequence<String> evalAsSequence(String string) throws Exception {
+    ParserInput input = ParserInput.fromLines(string);
+    Mutability mutability = Mutability.create("test");
+    Object parsedValue =
+        Starlark.execFile(
+            input,
+            FileOptions.DEFAULT,
+            Module.create(),
+            new StarlarkThread(mutability, StarlarkSemantics.DEFAULT));
+    mutability.freeze();
+    return Sequence.cast(parsedValue, String.class, "evalAsSequence() input");
+  }
+
+  @Test
+  public void repr() throws Exception {
+    assertThat(Starlark.repr(License.NO_LICENSE)).isEqualTo("[\"none\"]");
+    assertThat(License.parseLicense(evalAsSequence(Starlark.repr(License.NO_LICENSE))))
+        .isEqualTo(License.NO_LICENSE);
+
+    License withoutExceptions = License.parseLicense(ImmutableList.of("notice", "restricted"));
+    // License types sorted by LicenseType enum order.
+    assertThat(Starlark.repr(withoutExceptions)).isEqualTo("[\"restricted\", \"notice\"]");
+    assertThat(License.parseLicense(evalAsSequence(Starlark.repr(withoutExceptions))))
+        .isEqualTo(withoutExceptions);
+
+    License withExceptions =
+        License.parseLicense(
+            ImmutableList.of("notice", "restricted", "exception=//foo:bar", "exception=//baz:qux"));
+    // Exceptions sorted alphabetically.
+    assertThat(Starlark.repr(withExceptions))
+        .isEqualTo(
+            "[\"restricted\", \"notice\", \"exception=//baz:qux\", \"exception=//foo:bar\"]");
+    assertThat(License.parseLicense(evalAsSequence(Starlark.repr(withExceptions))))
+        .isEqualTo(withExceptions);
+  }
 }

src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/ModuleInfoExtractorTest.java

@@ -18,6 +18,7 @@
 import static com.google.common.truth.Truth8.assertThat;
 import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -54,7 +55,12 @@ public final class ModuleInfoExtractorTest {
   private String fakeLabelString = null; // set by exec()
 
   private Module exec(String... lines) throws Exception {
+    return execWithOptions(ImmutableList.of(), lines);
+  }
+
+  private Module execWithOptions(ImmutableList<String> options, String... lines) throws Exception {
     BazelEvaluationTestCase ev = new BazelEvaluationTestCase();
+    ev.setSemantics(options.toArray(new String[0]));
     Module module = ev.getModule();
     Label fakeLabel = BazelModuleContext.of(module).label();
     fakeLabelString = fakeLabel.getCanonicalForm();
@@ -434,7 +440,10 @@ public void ruleAdvertisedProviders() throws Exception {
   @Test
   public void ruleAttributes() throws Exception {
     Module module =
-        exec(
+        execWithOptions(
+            // TODO(https://github.com/bazelbuild/bazel/issues/6420): attr.license() is deprecated,
+            // and will eventually be removed from Bazel.
+            ImmutableList.of("--noincompatible_no_attr_license"),
             "MyInfo1 = provider()",
             "MyInfo2 = provider()",
             "MyInfo3 = provider()",
@@ -448,6 +457,7 @@ public void ruleAttributes() throws Exception {
             "        'c': attr.label(providers = [MyInfo1, MyInfo2]),",
             "        'd': attr.label(providers = [[MyInfo1, MyInfo2], [MyInfo3]]),",
             "        '_e': attr.string(doc = 'Hidden attribute'),",
+            "        'deprecated_license': attr.license(),",
             "    }",
             ")");
     ModuleInfo moduleInfo = getExtractor().extractFrom(module);
@@ -495,6 +505,11 @@ public void ruleAttributes() throws Exception {
                         .addProviderName("MyInfo3")
                         .addOriginKey(
                             OriginKey.newBuilder().setName("MyInfo3").setFile(fakeLabelString)))
+                .build(),
+            AttributeInfo.newBuilder()
+                .setName("deprecated_license")
+                .setType(AttributeType.STRING_LIST)
+                .setDefaultValue("[\"none\"]")
                 .build());
   }
 
@@ -710,7 +725,7 @@ public void aspectAttributes() throws Exception {
             "    pass",
             "my_aspect = aspect(",
             "    implementation = _my_impl,",
-            "    attr_aspects = ['deps', 'srcs'],",
+            "    attr_aspects = ['deps', 'srcs', '_private'],",
             "    attrs = {",
             "        'a': attr.string(doc = 'My doc', default = 'foo'),",
             "        'b': attr.string(mandatory = True),",

src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java

@@ -3112,7 +3112,7 @@ public void testLicenseType() throws Exception {
 
     getConfiguredTarget("//test:test");
 
-    assertContainsEvent("[none]");
+    assertContainsEvent("[\"none\"]");
   }
 
   @Test