bazel packages: make NativeProvider non-callable NativeProvider has only a single subclass that actually implements the call protocol: ConfigFeatureFlagProvider.Constructor (aka config_common.FeatureFlagInfo). This change makes it callable through the annotation mechanism. (Aside: We really need to get our story straight on the meanings of "info" and "provider". ConfigFeatureFlagProvider is an "info" yet FeatureFlagInfo is a provider. What a mess. I propose we chart a long-term course for "constructor" and "struct".) This change causes FeatureFlagInfo(...) to reject named arguments other than 'value', as it should have done all along. Also, don't require @SkylarkModule(documented=false) to redundantly specify doc="". PiperOrigin-RevId: 285193547
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java index 41a5b8a..53c6110 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
@@ -17,11 +17,7 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; -import com.google.devtools.build.lib.syntax.BaseFunction; -import com.google.devtools.build.lib.syntax.EvalException; -import com.google.devtools.build.lib.syntax.FuncallExpression; -import com.google.devtools.build.lib.syntax.FunctionSignature; -import com.google.devtools.build.lib.syntax.StarlarkThread; +import com.google.devtools.build.lib.syntax.StarlarkValue; import com.google.devtools.build.lib.util.Pair; import javax.annotation.Nullable; @@ -34,18 +30,14 @@ * <p>Typical implementation of a non-constructable from Skylark declared provider is as follows: * * <pre> - * public static final Provider PROVIDER = - * new NativeProvider("link_params") { }; + * public static final Provider PROVIDER = new NativeProvider("link_params") {}; * </pre> * - * To allow construction from Skylark and custom construction logic, override the {@link #call} - * method. - * * @deprecated use {@link BuiltinProvider} instead. */ @Immutable @Deprecated -public abstract class NativeProvider<V extends Info> extends BaseFunction implements Provider { +public abstract class NativeProvider<V extends Info> implements StarlarkValue, Provider { private final String name; private final NativeKey key; private final String errorMessageFormatForUnknownField; @@ -69,7 +61,6 @@ } protected NativeProvider(Class<V> valueClass, String name) { - super(FunctionSignature.KWARGS); this.name = name; this.key = new NativeKey(name, getClass()); this.valueClass = valueClass; @@ -109,11 +100,6 @@ } @Override - public String getName() { - return name; // for stack traces - } - - @Override public String getErrorMessageFormatForUnknownField() { return errorMessageFormatForUnknownField; } @@ -133,23 +119,6 @@ return key; } - /** - * Override this method to provide logic that is used to instantiate a declared provider from - * Skylark. - * - * <p>This is a method that is called when a constructor {@code c} is invoked as<br> - * {@code c(arg1 = val1, arg2 = val2, ...)}. - * - * @param args an array of argument values sorted as per the signature ({@see BaseFunction#call}) - */ - @Override - protected Object call(Object[] args, @Nullable FuncallExpression ast, StarlarkThread thread) - throws EvalException, InterruptedException { - Location loc = ast != null ? ast.getLocation() : Location.BUILTIN; - throw new EvalException( - loc, String.format("'%s' cannot be constructed from Starlark", getPrintableName())); - } - public static Pair<String, String> getSerializedRepresentationForNativeKey(NativeKey key) { return Pair.of(key.name, key.aClass.getName()); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD index d829ba6..8de2000 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD
@@ -21,6 +21,7 @@ "//src/main/java/com/google/devtools/build/lib:core-rules", "//src/main/java/com/google/devtools/build/lib:events", "//src/main/java/com/google/devtools/build/lib:packages-internal", + "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//src/main/java/com/google/devtools/build/lib:syntax", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java index 5c5e9a4..c6948b0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagProvider.java
@@ -19,26 +19,29 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.NativeInfo; import com.google.devtools.build.lib.packages.NativeProvider; import com.google.devtools.build.lib.packages.RequiredProviders; import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigFeatureFlagProviderApi; -import com.google.devtools.build.lib.syntax.EvalException; -import com.google.devtools.build.lib.syntax.FuncallExpression; -import com.google.devtools.build.lib.syntax.StarlarkThread; -import java.util.Map; -import javax.annotation.Nullable; +import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; +import com.google.devtools.build.lib.syntax.Printer; +import com.google.devtools.build.lib.syntax.StarlarkValue; /** Provider for exporting value and valid value predicate of feature flags to consuming targets. */ +// TODO(adonovan): rename this to *Info and its constructor to *Provider. @Immutable public class ConfigFeatureFlagProvider extends NativeInfo implements ConfigFeatureFlagProviderApi { /** Name used in Skylark for accessing ConfigFeatureFlagProvider. */ static final String SKYLARK_NAME = "FeatureFlagInfo"; - /** Skylark constructor and identifier for ConfigFeatureFlagProvider. */ + /** + * Constructor and identifier for ConfigFeatureFlagProvider. This is the value of {@code + * config_common.FeatureFlagInfo}. + */ static final NativeProvider<ConfigFeatureFlagProvider> SKYLARK_CONSTRUCTOR = new Constructor(); static final RequiredProviders REQUIRE_CONFIG_FEATURE_FLAG_PROVIDER = @@ -59,28 +62,33 @@ return new ConfigFeatureFlagProvider(value, isValidValue); } - /** A constructor callable from Skylark for OutputGroupInfo. */ - private static class Constructor extends NativeProvider<ConfigFeatureFlagProvider> { + /** + * A constructor callable from Skylark for OutputGroupInfo: {@code + * config_common.FeatureFlagInfo(value="...")} + */ + @SkylarkModule(name = "FeatureFlagInfo", documented = false) + @Immutable + private static final class Constructor extends NativeProvider<ConfigFeatureFlagProvider> + implements StarlarkValue { - private Constructor() { + Constructor() { super(ConfigFeatureFlagProvider.class, SKYLARK_NAME); } - @Override - protected ConfigFeatureFlagProvider call( - Object[] args, @Nullable FuncallExpression ast, StarlarkThread thread) - throws EvalException { - Location loc = ast != null ? ast.getLocation() : Location.BUILTIN; - - @SuppressWarnings("unchecked") - Map<String, Object> kwargs = (Map<String, Object>) args[0]; - - if (!kwargs.containsKey("value") || !(kwargs.get("value") instanceof String)) { - throw new EvalException(loc, "FeatureFlagInfo requires 'value' to be set to a string"); - } - return create((String) kwargs.get("value"), Predicates.alwaysTrue()); + @SkylarkCallable( + name = "FeatureFlagInfo", + documented = false, + parameters = {@Param(name = "value", named = true, type = String.class)}, + selfCall = true) + public ConfigFeatureFlagProvider selfcall(String value) { + return create(value, Predicates.alwaysTrue()); } -} + + @Override + public void repr(Printer printer) { + printer.append("<function FeatureFlagInfo>"); + } + } public static SkylarkProviderIdentifier id() { return SKYLARK_CONSTRUCTOR.id();
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java index 5fb9d93..1d14607 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkModule.java
@@ -61,8 +61,10 @@ /** A title for the documentation page generated for this type. */ String title() default ""; - String doc(); + /** Module documentation in HTML. May be empty only if {@code !documented()}. */ + String doc() default ""; + /** Whether the module should appear in the documentation. */ boolean documented() default true; /**