From 5e1ceafa08fc5387d495fb32353a8c54aca33f58 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 11 Apr 2023 03:42:41 -0700 Subject: [PATCH] Prevent repository_rule from showing up twice in documentation - Removed @GlobalMethods from StarlarkRepositoryModule as the interface RepositoryModuleApi is already annotated with @GlobalMethods - Moved the two methods, `moduleExtension` and `tagClass`, and their documentation, to the interface instead - Added no-op fake implementation for the two methods in FakeRepositoryModule - TagClass's documentation is moved to an interface RepositoryModuleApi.TagClassApi (so that we can still refer to it); ModuleExtension's documentation is removed, as that type is never meaningfully used in Starlark (only exported) Work towards https://github.com/bazelbuild/bazel/issues/16383 PiperOrigin-RevId: 523356118 Change-Id: Ic937be3c41bcf7d748945d7bc2d2afb124d9c4bd # Conflicts: # src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD # src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java # src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TagClass.java # src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java # src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java # Conflicts: # src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java # src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java --- .../devtools/build/lib/bazel/bzlmod/BUILD | 2 +- .../lib/bazel/bzlmod/ModuleExtension.java | 52 +---------- .../build/lib/bazel/bzlmod/TagClass.java | 6 +- .../build/lib/bazel/repository/starlark/BUILD | 1 + .../starlark/StarlarkRepositoryModule.java | 73 +-------------- .../repository/RepositoryModuleApi.java | 93 +++++++++++++++++++ .../repository/FakeRepositoryModule.java | 17 ++++ .../bazel/bzlmod/StarlarkBazelModuleTest.java | 3 +- 8 files changed, 120 insertions(+), 127 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 0830f09d87d558..c3751456fc1f9a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -59,7 +59,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", - "//src/main/java/net/starlark/java/annot", + "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", "//third_party:auto_value", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java index 20dc263ccb1d66..408710dbefc4a0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtension.java @@ -14,16 +14,9 @@ package com.google.devtools.build.lib.bazel.bzlmod; -import com.google.api.client.util.Preconditions; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.docgen.annot.DocCategory; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.events.EventHandler; -import com.google.devtools.build.lib.packages.StarlarkExportable; -import javax.annotation.Nullable; -import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.eval.StarlarkCallable; import net.starlark.java.eval.StarlarkValue; import net.starlark.java.syntax.Location; @@ -32,11 +25,8 @@ * A module extension object, which can be used to perform arbitrary logic in order to create repos. */ @AutoValue -@StarlarkBuiltin( - name = "module_extension", - category = DocCategory.BUILTIN, - doc = "A module extension declared using the module_extension function.") public abstract class ModuleExtension implements StarlarkValue { + public abstract StarlarkCallable getImplementation(); public abstract ImmutableMap getTagClasses(); @@ -58,8 +48,6 @@ public abstract static class Builder { public abstract Builder setLocation(Location value); - public abstract Builder setName(String value); - public abstract Builder setImplementation(StarlarkCallable value); public abstract Builder setTagClasses(ImmutableMap value); @@ -68,42 +56,4 @@ public abstract static class Builder { public abstract ModuleExtension build(); } - - /** - * A {@link ModuleExtension} exposed to Starlark. We can't use {@link ModuleExtension} directly - * because the name isn't known until the object is exported, so this class holds a builder until - * it's exported, at which point it sets the name and builds the underlying {@link - * ModuleExtension}. - */ - @StarlarkBuiltin(name = "module_extension", doc = "A module extension.") - public static class InStarlark implements StarlarkExportable { - private final Builder builder; - @Nullable - private ModuleExtension built; - - public InStarlark() { - builder = builder(); - built = null; - } - - public Builder getBuilder() { - return builder; - } - - @Override - public boolean isExported() { - return built != null; - } - - @Override - public void export(EventHandler handler, Label extensionLabel, String exportedName) { - built = builder.setName(exportedName).build(); - } - - /** Throws {@link IllegalStateException} if this is not exported yet. */ - public ModuleExtension get() { - Preconditions.checkState(isExported(), "the module extension was never exported"); - return built; - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TagClass.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TagClass.java index f2bdc7c1e93e8d..fbc3b8d83313d0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TagClass.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TagClass.java @@ -18,16 +18,14 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.packages.Attribute; -import net.starlark.java.annot.StarlarkBuiltin; -import net.starlark.java.eval.StarlarkValue; +import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi.TagClassApi; import net.starlark.java.syntax.Location; /** * Represents a tag class, which is a "class" of {@link Tag}s that share the same attribute schema. */ -@StarlarkBuiltin(name = "tag_class", doc = "Defines a schema of attributes for a tag.") @AutoValue -public abstract class TagClass implements StarlarkValue { +public abstract class TagClass implements TagClassApi { /** The list of attributes of this tag class. */ public abstract ImmutableList getAttributes(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 9fda919f03e600..8bdb024b79cced 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD @@ -52,6 +52,7 @@ java_library( "//src/main/java/net/starlark/java/annot", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", + "//third_party:error_prone_annotations", "//third_party:guava", "//third_party:java-diff-utils", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index 96da8a38220eb2..a350c5956420e4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java @@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.docgen.annot.DocCategory; -import com.google.devtools.build.docgen.annot.DocumentMethods; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor; import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtension; @@ -49,9 +48,7 @@ import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi; import java.util.Map; -import net.starlark.java.annot.Param; import net.starlark.java.annot.StarlarkBuiltin; -import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Module; @@ -66,7 +63,6 @@ * The Starlark module containing the definition of {@code repository_rule} function to define a * Starlark remote repository. */ -@DocumentMethods public class StarlarkRepositoryModule implements RepositoryModuleApi { @Override @@ -266,48 +262,7 @@ public void failWithIncompatibleUseCcConfigureFromRulesCc(StarlarkThread thread) } } - @StarlarkMethod( - name = "module_extension", - doc = - "Creates a new module extension. Store it in a global value, so that it can be exported" - + " and used in a MODULE.bazel file.", - parameters = { - @Param( - name = "implementation", - named = true, - doc = - "The function that implements this module extension. Must take a single parameter," - + " module_ctx. The function is" - + " called once at the beginning of a build to determine the set of available" - + " repos."), - @Param( - name = "tag_classes", - defaultValue = "{}", - doc = - "A dictionary to declare all the tag classes used by the extension. It maps from" - + " the name of the tag class to a tag_class object.", - named = true, - positional = false), - @Param( - name = "doc", - defaultValue = "''", - doc = - "A description of the module extension that can be extracted by documentation" - + " generating tools.", - named = true, - positional = false), - @Param( - name = "environ", - defaultValue = "[]", - doc = - "Provides a list of environment variable that this module extension depends on. If " - + "an environment variable in that list changes, the extension will be " - + "re-evaluated.", - named = true, - positional = false) - }, - useStarlarkThread = true) + @Override public ModuleExtension moduleExtension( StarlarkCallable implementation, Dict tagClasses, // Dict @@ -320,34 +275,12 @@ public ModuleExtension moduleExtension( .setTagClasses( ImmutableMap.copyOf(Dict.cast(tagClasses, String.class, TagClass.class, "tag_classes"))) .setDoc(doc) + .setEnvVariables(ImmutableList.copyOf(Sequence.cast(environ, String.class, "environ"))) .setLocation(thread.getCallerLocation()) .build(); } - @StarlarkMethod( - name = "tag_class", - doc = - "Creates a new tag_class object, which defines an attribute schema for a class of tags," - + " which are data objects usable by a module extension.", - parameters = { - @Param( - name = "attrs", - defaultValue = "{}", - named = true, - doc = - "A dictionary to declare all the attributes of this tag class. It maps from an" - + " attribute name to an attribute object (see attr" - + " module)."), - @Param( - name = "doc", - defaultValue = "''", - doc = - "A description of the tag class that can be extracted by documentation" - + " generating tools.", - named = true, - positional = false) - }, - useStarlarkThread = true) + @Override public TagClass tagClass( Dict attrs, // Dict String doc, diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java index cb9268c92de3cd..81d267d0f62cb0 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository/RepositoryModuleApi.java @@ -14,10 +14,12 @@ package com.google.devtools.build.lib.starlarkbuildapi.repository; +import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.docgen.annot.DocumentMethods; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; +import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; @@ -25,6 +27,7 @@ import net.starlark.java.eval.Sequence; import net.starlark.java.eval.StarlarkCallable; import net.starlark.java.eval.StarlarkThread; +import net.starlark.java.eval.StarlarkValue; /** * The Starlark module containing the definition of {@code repository_rule} function to define a @@ -118,6 +121,96 @@ StarlarkCallable repositoryRule( StarlarkThread thread) throws EvalException; + @StarlarkMethod( + name = "module_extension", + doc = + "Creates a new module extension. Store it in a global value, so that it can be exported" + + " and used in a MODULE.bazel file.", + parameters = { + @Param( + name = "implementation", + named = true, + doc = + "The function that implements this module extension. Must take a single parameter," + + " module_ctx. The function is" + + " called once at the beginning of a build to determine the set of available" + + " repos."), + @Param( + name = "tag_classes", + defaultValue = "{}", + doc = + "A dictionary to declare all the tag classes used by the extension. It maps from" + + " the name of the tag class to a tag_class object.", + named = true, + positional = false), + @Param( + name = "doc", + defaultValue = "''", + doc = + "A description of the module extension that can be extracted by documentation" + + " generating tools.", + named = true, + positional = false), + @Param( + name = "environ", + allowedTypes = { + @ParamType(type = Sequence.class, generic1 = String.class), + }, + defaultValue = "[]", + doc = + "Provides a list of environment variable that this module extension depends on. If " + + "an environment variable in that list changes, the extension will be " + + "re-evaluated.", + named = true, + positional = false), + }, + useStarlarkThread = true) + Object moduleExtension( + StarlarkCallable implementation, + Dict tagClasses, // Dict + String doc, + Sequence environ, // + StarlarkThread thread) + throws EvalException; + + @StarlarkMethod( + name = "tag_class", + doc = + "Creates a new tag_class object, which defines an attribute schema for a class of tags," + + " which are data objects usable by a module extension.", + parameters = { + @Param( + name = "attrs", + defaultValue = "{}", + named = true, + doc = + "A dictionary to declare all the attributes of this tag class. It maps from an" + + " attribute name to an attribute object (see attr" + + " module)."), + @Param( + name = "doc", + defaultValue = "''", + doc = + "A description of the tag class that can be extracted by documentation" + + " generating tools.", + named = true, + positional = false) + }, + useStarlarkThread = true) + TagClassApi tagClass( + Dict attrs, // Dict + String doc, + StarlarkThread thread) + throws EvalException; + + /** Represents a tag class, which is a "class" of tags that share the same attribute schema. */ + @StarlarkBuiltin( + name = "tag_class", + category = DocCategory.BUILTIN, + doc = "Defines a schema of attributes for a tag.") + interface TagClassApi extends StarlarkValue {} + @StarlarkMethod( name = "__do_not_use_fail_with_incompatible_use_cc_configure_from_rules_cc", doc = diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java index 2cbf475276304a..69f597b44f3e83 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/repository/FakeRepositoryModule.java @@ -115,6 +115,23 @@ public String getName() { } } + @Override + public Object moduleExtension( + StarlarkCallable implementation, + Dict tagClasses, + String doc, + Sequence environ, + StarlarkThread thread) + throws EvalException { + return new Object(); + } + + @Override + public TagClassApi tagClass(Dict attrs, String doc, StarlarkThread thread) + throws EvalException { + return new TagClassApi() {}; + } + @Override public void failWithIncompatibleUseCcConfigureFromRulesCc(StarlarkThread thread) throws EvalException { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java index 0528a15687e476..189b2cb91f8952 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java @@ -61,7 +61,8 @@ private static ModuleExtension.Builder getBaseExtensionBuilder() { return ModuleExtension.builder() .setDoc("") .setLocation(Location.BUILTIN) - .setImplementation(() -> "maven"); + .setImplementation(() -> "maven") + .setEnvVariables(ImmutableList.of()); } @Test