Support "mandatoryProvidersList" in Skylark and added necessary tests
The type of attribute "providers" now is a list of lists of string. But a list
of string is still supported.
--
MOS_MIGRATED_REVID=115357326
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index f99f598..f2e984d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -36,11 +36,13 @@
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
+import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.lib.syntax.UserDefinedFunction;
import com.google.devtools.build.lib.util.FileTypeSet;
+import java.util.ArrayList;
import java.util.List;
/**
@@ -97,6 +99,11 @@
private static final String NON_EMPTY_DOC = "True if the attribute must not be empty";
private static final String PROVIDERS_ARG = "providers";
+ private static final String PROVIDERS_DOC =
+ "mandatory providers list. It should be either a list of providers, or a "
+ + "list of lists of providers. Every dependency should provide ALL providers "
+ + "from at least ONE of these lists. A single list of providers will be "
+ + "automatically converted to a list containing one list of providers.";
private static final String SINGLE_FILE_ARG = "single_file";
@@ -180,8 +187,20 @@
}
if (containsNonNoneKey(arguments, PROVIDERS_ARG)) {
- builder.mandatoryProviders(SkylarkList.castSkylarkListOrNoneToList(
- arguments.get(PROVIDERS_ARG), String.class, PROVIDERS_ARG));
+ Object obj = arguments.get(PROVIDERS_ARG);
+ SkylarkType.checkType(obj, SkylarkList.class, PROVIDERS_ARG);
+ boolean isSingleListOfStr = true;
+ for (Object o : (SkylarkList) obj) {
+ isSingleListOfStr = o instanceof String;
+ if (!isSingleListOfStr) {
+ break;
+ }
+ }
+ if (isSingleListOfStr) {
+ builder.mandatoryProviders(((SkylarkList<?>) obj).getContents(String.class, PROVIDERS_ARG));
+ } else {
+ builder.mandatoryProvidersList(getProvidersList((SkylarkList) obj));
+ }
}
if (containsNonNoneKey(arguments, CONFIGURATION_ARG)) {
@@ -190,6 +209,28 @@
return builder;
}
+ private static List<List<String>> getProvidersList(SkylarkList skylarkList) throws EvalException {
+ List<List<String>> providersList = new ArrayList<>();
+ String errorMsg = "Illegal argument: element in '%s' is of unexpected type. "
+ + "Should be list of string, but got %s. "
+ + "Notice: one single list of string as 'providers' is still supported.";
+ for (Object o : skylarkList) {
+ if (!(o instanceof SkylarkList)) {
+ throw new EvalException(null, String.format(errorMsg, PROVIDERS_ARG,
+ EvalUtils.getDataTypeName(o, true)));
+ }
+ for (Object value : (SkylarkList) o) {
+ if (!(value instanceof String)) {
+ throw new EvalException(null, String.format(errorMsg, PROVIDERS_ARG,
+ "list with an element of type "
+ + EvalUtils.getDataTypeNameFromClass(value.getClass())));
+ }
+ }
+ providersList.add(((SkylarkList<?>) o).getContents(String.class, PROVIDERS_ARG));
+ }
+ return providersList;
+ }
+
private static Descriptor createAttrDescriptor(
SkylarkDict<String, Object> kwargs, Type<?> type, FuncallExpression ast, Environment env)
throws EvalException {
@@ -336,9 +377,8 @@
@Param(
name = PROVIDERS_ARG,
type = SkylarkList.class,
- generic1 = String.class,
defaultValue = "[]",
- doc = "mandatory providers every dependency has to have"
+ doc = PROVIDERS_DOC
),
@Param(
name = ALLOW_RULES_ARG,
@@ -534,9 +574,8 @@
@Param(
name = PROVIDERS_ARG,
type = SkylarkList.class,
- generic1 = String.class,
defaultValue = "[]",
- doc = "mandatory providers every dependency has to have"
+ doc = PROVIDERS_DOC
),
@Param(
name = FLAGS_ARG,
diff --git a/src/test/java/com/google/devtools/build/lib/generatedprojecttest/util/RuleSetUtils.java b/src/test/java/com/google/devtools/build/lib/generatedprojecttest/util/RuleSetUtils.java
index 2bb34a9..32b9c83 100644
--- a/src/test/java/com/google/devtools/build/lib/generatedprojecttest/util/RuleSetUtils.java
+++ b/src/test/java/com/google/devtools/build/lib/generatedprojecttest/util/RuleSetUtils.java
@@ -48,7 +48,8 @@
RuleClassType.INVISIBLE.checkName(input);
return true;
} catch (IllegalArgumentException e) {
- return input.equals("testing_dummy_rule");
+ return input.equals("testing_dummy_rule")
+ || input.equals("testing_rule_for_mandatory_providers");
}
}
};
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 4ecfd72..20c9a4c 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -184,6 +184,27 @@
}
@Test
+ public void testAttrWithProvidersList() throws Exception {
+ Attribute attr =
+ evalAttributeDefinition("attr.label_list(allow_files = True,"
+ + " providers = [['a', 'b'], ['c']])")
+ .build("a1");
+ assertThat(attr.getMandatoryProvidersList()).containsExactly(ImmutableSet.of("a", "b"),
+ ImmutableSet.of("c"));
+ }
+
+ @Test
+ public void testAttrWithWrongProvidersList() throws Exception {
+ checkErrorContains("Illegal argument: element in 'providers' is of unexpected type."
+ + " Should be list of string, but got list with an element of type int.",
+ "attr.label_list(allow_files = True, providers = [['a', 1], ['c']])");
+
+ checkErrorContains("Illegal argument: element in 'providers' is of unexpected type."
+ + " Should be list of string, but got string.",
+ "attr.label_list(allow_files = True, providers = [['a', 'b'], 'c'])");
+ }
+
+ @Test
public void testLabelListWithAspects() throws Exception {
SkylarkAttr.Descriptor attr =
(SkylarkAttr.Descriptor) evalRuleClassCode(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index fe6003c..7f59515 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -180,6 +180,83 @@
}
}
+ @Test
+ public void testMandatoryProvidersListWithSkylark() throws Exception {
+ scratch.file("test/BUILD",
+ "load('/test/rules', 'skylark_rule', 'my_rule', 'my_other_rule')",
+ "my_rule(name = 'mylib',",
+ " srcs = ['a.py'])",
+ "skylark_rule(name = 'skyrule1',",
+ " deps = [':mylib'])",
+ "my_other_rule(name = 'my_other_lib',",
+ " srcs = ['a.py'])",
+ "skylark_rule(name = 'skyrule2',",
+ " deps = [':my_other_lib'])");
+ scratch.file("test/rules.bzl",
+ "def _impl(ctx):",
+ " return",
+ "skylark_rule = rule(",
+ " implementation = _impl,",
+ " attrs = {",
+ " 'deps': attr.label_list(providers = [['a'], ['b', 'c']],",
+ " allow_files=True)",
+ " }",
+ ")",
+ "def my_rule_impl(ctx):",
+ " return struct(a = [])",
+ "my_rule = rule(implementation = my_rule_impl, ",
+ " attrs = { 'srcs' : attr.label_list(allow_files=True)})",
+ "def my_other_rule_impl(ctx):",
+ " return struct(b = [])",
+ "my_other_rule = rule(implementation = my_other_rule_impl, ",
+ " attrs = { 'srcs' : attr.label_list(allow_files=True)})");
+ reporter.removeHandler(failFastHandler);
+ assertNotNull(getConfiguredTarget("//test:skyrule1"));
+
+ try {
+ createRuleContext("//test:skyrule2");
+ fail("Should have failed because of wrong mandatory providers");
+ } catch (Exception ex) {
+ assertContainsEvent("ERROR /workspace/test/BUILD:9:10: in deps attribute of "
+ + "skylark_rule rule //test:skyrule2: '//test:my_other_lib' does not have "
+ + "mandatory provider 'a' or 'c'");
+ }
+ }
+
+ @Test
+ public void testMandatoryProvidersListWithNative() throws Exception {
+ scratch.file("test/BUILD",
+ "load('/test/rules', 'my_rule', 'my_other_rule')",
+ "my_rule(name = 'mylib',",
+ " srcs = ['a.py'])",
+ "testing_rule_for_mandatory_providers(name = 'skyrule1',",
+ " deps = [':mylib'])",
+ "my_other_rule(name = 'my_other_lib',",
+ " srcs = ['a.py'])",
+ "testing_rule_for_mandatory_providers(name = 'skyrule2',",
+ " deps = [':my_other_lib'])");
+ scratch.file("test/rules.bzl",
+ "def my_rule_impl(ctx):",
+ " return struct(a = [])",
+ "my_rule = rule(implementation = my_rule_impl, ",
+ " attrs = { 'srcs' : attr.label_list(allow_files=True)})",
+ "def my_other_rule_impl(ctx):",
+ " return struct(b = [])",
+ "my_other_rule = rule(implementation = my_other_rule_impl, ",
+ " attrs = { 'srcs' : attr.label_list(allow_files=True)})");
+ reporter.removeHandler(failFastHandler);
+ assertNotNull(getConfiguredTarget("//test:skyrule1"));
+
+ try {
+ createRuleContext("//test:skyrule2");
+ fail("Should have failed because of wrong mandatory providers");
+ } catch (Exception ex) {
+ assertContainsEvent("ERROR /workspace/test/BUILD:9:10: in deps attribute of "
+ + "testing_rule_for_mandatory_providers rule //test:skyrule2: '//test:my_other_lib' "
+ + "does not have mandatory provider 'a' or 'c'");
+ }
+ }
+
/* Sharing setup code between the testPackageBoundaryError*() methods is not possible since the
* errors already happen when loading the file. Consequently, all tests would fail at the same
* statement. */
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java b/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java
index eaf1cee..8b708a3 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java
@@ -20,6 +20,7 @@
import static com.google.devtools.build.lib.syntax.Type.INTEGER;
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
+import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.RuleDefinition;
@@ -59,6 +60,7 @@
new ConfiguredRuleClassProvider.Builder();
addStandardRules(builder);
builder.addRuleDefinition(new TestingDummyRule());
+ builder.addRuleDefinition(new TestingRuleForMandatoryProviders());
ruleProvider = builder.build();
}
return ruleProvider;
@@ -85,4 +87,25 @@
.build();
}
}
+
+ public static final class TestingRuleForMandatoryProviders implements RuleDefinition {
+ @Override
+ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
+ return builder
+ .setUndocumented()
+ .add(attr("srcs", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE))
+ .override(builder.copy("deps").mandatoryProvidersList(ImmutableList.of(
+ ImmutableList.of("a"), ImmutableList.of("b", "c"))))
+ .build();
+ }
+
+ @Override
+ public Metadata getMetadata() {
+ return RuleDefinition.Metadata.builder()
+ .name("testing_rule_for_mandatory_providers")
+ .ancestors(BaseRuleClasses.RuleBase.class)
+ .factoryClass(UnknownRuleConfiguredTarget.class)
+ .build();
+ }
+ }
}
