Refactor Skylark rules and attributes in preparation to Skylark aspects.
1. attr.<type> functions return a wrapper object instead of
Attribute.Builder dierctly.
2. RuleClass is created once per the life-time of RuleFunction, during
export
3. Attributes are added to the RuleClass at exporting.
--
MOS_MIGRATED_REVID=108774581
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 23458c7..bca93ac 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
@@ -28,12 +28,16 @@
import com.google.devtools.build.lib.packages.PredicateWithMessage;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.rules.SkylarkAttr;
import com.google.devtools.build.lib.rules.SkylarkFileType;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions;
import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.RuleFunction;
import com.google.devtools.build.lib.skylark.util.SkylarkTestCase;
+import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileTypeSet;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -71,15 +75,22 @@
@Test
public void testCannotOverrideBuiltInAttribute() throws Exception {
- checkEvalError(
- "There is already a built-in attribute 'tags' which cannot be overridden",
- "def impl(ctx): return",
- "r = rule(impl, attrs = {'tags': attr.string_list()})");
+ ev.setFailFast(true);
+ try {
+ evalAndExport(
+ "def impl(ctx): return", "r = rule(impl, attrs = {'tags': attr.string_list()})");
+ Assert.fail("Expected error '"
+ + "There is already a built-in attribute 'tags' which cannot be overridden"
+ + "' but got no error");
+ } catch (IllegalArgumentException | EvalException e) {
+ assertThat(e).hasMessage(
+ "There is already a built-in attribute 'tags' which cannot be overridden");
+ }
}
@Test
public void testImplicitArgsAttribute() throws Exception {
- eval(
+ evalAndExport(
"def _impl(ctx):",
" pass",
"exec_rule = rule(implementation = _impl, executable = True)",
@@ -89,59 +100,57 @@
}
private RuleClass getRuleClass(String name) throws Exception {
- return ((RuleFunction) lookup(name)).getBuilder().build(name);
+ return ((RuleFunction) lookup(name)).getRuleClass();
}
private void registerDummyUserDefinedFunction() throws Exception {
eval("def impl():\n" + " return 0\n");
}
+ private Attribute.Builder<?> evalAttributeDefinition(String... lines) throws Exception {
+ return ((SkylarkAttr.Descriptor) evalRuleClassCode(lines)).getAttributeBuilder();
+ }
+
@Test
public void testAttrWithOnlyType() throws Exception {
- Object result = evalRuleClassCode("attr.string_list()");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string_list()").build("a1");
assertEquals(Type.STRING_LIST, attr.getType());
}
@Test
public void testOutputListAttr() throws Exception {
- Object result = evalRuleClassCode("attr.output_list()");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.output_list()").build("a1");
assertEquals(BuildType.OUTPUT_LIST, attr.getType());
}
@Test
public void testIntListAttr() throws Exception {
- Object result = evalRuleClassCode("attr.int_list()");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.int_list()").build("a1");
assertEquals(Type.INTEGER_LIST, attr.getType());
}
@Test
public void testOutputAttr() throws Exception {
- Object result = evalRuleClassCode("attr.output()");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.output()").build("a1");
assertEquals(BuildType.OUTPUT, attr.getType());
}
@Test
public void testStringDictAttr() throws Exception {
- Object result = evalRuleClassCode("attr.string_dict(default = {'a': 'b'})");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string_dict(default = {'a': 'b'})").build("a1");
assertEquals(Type.STRING_DICT, attr.getType());
}
@Test
public void testStringListDictAttr() throws Exception {
- Object result = evalRuleClassCode("attr.string_list_dict(default = {'a': ['b', 'c']})");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string_list_dict(default = {'a': ['b', 'c']})")
+ .build("a1");
assertEquals(Type.STRING_LIST_DICT, attr.getType());
}
@Test
public void testAttrAllowedFileTypesAnyFile() throws Exception {
- Object result = evalRuleClassCode("attr.label_list(allow_files = True)");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.label_list(allow_files = True)").build("a1");
assertEquals(FileTypeSet.ANY_FILE, attr.getAllowedFileTypesPredicate());
}
@@ -154,17 +163,17 @@
@Test
public void testAttrWithSkylarkFileType() throws Exception {
- Object result = evalRuleClassCode("attr.label_list(allow_files = FileType(['.xml']))");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.label_list(allow_files = FileType(['.xml']))")
+ .build("a1");
assertTrue(attr.getAllowedFileTypesPredicate().apply("a.xml"));
assertFalse(attr.getAllowedFileTypesPredicate().apply("a.txt"));
}
@Test
public void testAttrWithProviders() throws Exception {
- Object result =
- evalRuleClassCode("attr.label_list(allow_files = True, providers = ['a', 'b'])");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr =
+ evalAttributeDefinition("attr.label_list(allow_files = True, providers = ['a', 'b'])")
+ .build("a1");
assertEquals(ImmutableSet.of("a", "b"), attr.getMandatoryProviders());
}
@@ -191,16 +200,14 @@
}
@Test
public void testAttrAllowedRuleClassesSpecificRuleClasses() throws Exception {
- Object result =
- evalRuleClassCode("attr.label_list(allow_rules = ['java_binary'], allow_files = True)");
- Attribute attr = ((Attribute.Builder<?>) result).build("a");
+ Attribute attr = evalAttributeDefinition(
+ "attr.label_list(allow_rules = ['java_binary'], allow_files = True)").build("a");
assertTrue(attr.getAllowedRuleClassesPredicate().apply(ruleClass("java_binary")));
assertFalse(attr.getAllowedRuleClassesPredicate().apply(ruleClass("genrule")));
}
@Test
public void testAttrDefaultValue() throws Exception {
- Object result = evalRuleClassCode("attr.string(default = 'some value')");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string(default = 'some value')").build("a1");
assertEquals("some value", attr.getDefaultValueForTesting());
}
@@ -215,16 +222,14 @@
@Test
public void testAttrMandatory() throws Exception {
- Object result = evalRuleClassCode("attr.string(mandatory=True)");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string(mandatory=True)").build("a1");
assertTrue(attr.isMandatory());
assertFalse(attr.isNonEmpty());
}
@Test
public void testAttrNonEmpty() throws Exception {
- Object result = evalRuleClassCode("attr.string_list(non_empty=True)");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string_list(non_empty=True)").build("a1");
assertTrue(attr.isNonEmpty());
assertFalse(attr.isMandatory());
}
@@ -237,15 +242,14 @@
@Test
public void testAttrCfg() throws Exception {
- Object result = evalRuleClassCode("attr.label(cfg = HOST_CFG, allow_files = True)");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.label(cfg = HOST_CFG, allow_files = True)")
+ .build("a1");
assertEquals(ConfigurationTransition.HOST, attr.getConfigurationTransition());
}
@Test
public void testAttrValues() throws Exception {
- Object result = evalRuleClassCode("attr.string(values = ['ab', 'cd'])");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.string(values = ['ab', 'cd'])").build("a1");
PredicateWithMessage<Object> predicate = attr.getAllowedValues();
assertThat(predicate.apply("ab")).isTrue();
assertThat(predicate.apply("xy")).isFalse();
@@ -253,8 +257,7 @@
@Test
public void testAttrIntValues() throws Exception {
- Object result = evalRuleClassCode("attr.int(values = [1, 2])");
- Attribute attr = ((Attribute.Builder<?>) result).build("a1");
+ Attribute attr = evalAttributeDefinition("attr.int(values = [1, 2])").build("a1");
PredicateWithMessage<Object> predicate = attr.getAllowedValues();
assertThat(predicate.apply(2)).isTrue();
assertThat(predicate.apply(3)).isFalse();
@@ -262,8 +265,8 @@
@Test
public void testRuleImplementation() throws Exception {
- eval("def impl(ctx): return None", "rule1 = rule(impl)");
- RuleClass c = ((RuleFunction) lookup("rule1")).getBuilder().build("rule1");
+ evalAndExport("def impl(ctx): return None", "rule1 = rule(impl)");
+ RuleClass c = ((RuleFunction) lookup("rule1")).getRuleClass();
assertEquals("impl", c.getConfiguredTargetFunction().getName());
}
@@ -277,46 +280,55 @@
"attr.string(default=attr_value)");
}
+ private static final Label FAKE_LABEL = Label.parseAbsoluteUnchecked("//fake/label.bzl");
+
@Test
public void testRuleAddAttribute() throws Exception {
- eval("def impl(ctx): return None", "r1 = rule(impl, attrs={'a1': attr.string()})");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ evalAndExport("def impl(ctx): return None", "r1 = rule(impl, attrs={'a1': attr.string()})");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
assertTrue(c.hasAttr("a1", Type.STRING));
}
+ protected void evalAndExport(String... lines) throws Exception {
+ eval(lines);
+ SkylarkRuleClassFunctions.exportRuleFunctionsAndAspects(ev.getEnvironment(), FAKE_LABEL);
+ }
+
@Test
public void testOutputToGenfiles() throws Exception {
- eval("def impl(ctx): pass", "r1 = rule(impl, output_to_genfiles=True)");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ evalAndExport("def impl(ctx): pass", "r1 = rule(impl, output_to_genfiles=True)");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
assertFalse(c.hasBinaryOutput());
}
@Test
public void testRuleAddMultipleAttributes() throws Exception {
- eval(
+ evalAndExport(
"def impl(ctx): return None",
"r1 = rule(impl,",
" attrs = {",
" 'a1': attr.label_list(allow_files=True),",
" 'a2': attr.int()",
"})");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
assertTrue(c.hasAttr("a1", BuildType.LABEL_LIST));
assertTrue(c.hasAttr("a2", Type.INTEGER));
}
@Test
public void testRuleAttributeFlag() throws Exception {
- eval(
+ evalAndExport(
"def impl(ctx): return None",
"r1 = rule(impl, attrs = {'a1': attr.string(mandatory=True)})");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
assertTrue(c.getAttributeByName("a1").isMandatory());
}
@Test
public void testRuleOutputs() throws Exception {
- eval("def impl(ctx): return None", "r1 = rule(impl, outputs = {'a': 'a.txt'})");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ evalAndExport(
+ "def impl(ctx): return None",
+ "r1 = rule(impl, outputs = {'a': 'a.txt'})");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
ImplicitOutputsFunction function = c.getImplicitOutputsFunction();
assertEquals("a.txt", Iterables.getOnlyElement(function.getImplicitOutputs(null)));
}
@@ -351,14 +363,15 @@
registerDummyUserDefinedFunction();
checkErrorContains(
"Illegal argument: "
- + "expected <String, Builder> type for 'attrs' but got <string, string> instead",
+ + "expected <String, Descriptor> type for 'attrs' but got <string, string> instead",
"rule(impl, attrs = {'a1': 'some text'})");
}
@Test
public void testLabel() throws Exception {
Object result = evalRuleClassCode("Label('//foo/foo:foo')");
- assertEquals("//foo/foo:foo", ((Label) result).toString());
+ assertThat(result).isInstanceOf(Label.class);
+ assertEquals("//foo/foo:foo", result.toString());
}
@Test
@@ -380,19 +393,22 @@
@Test
public void testRuleLabelDefaultValue() throws Exception {
- eval(
+ evalAndExport(
"def impl(ctx): return None\n"
+ "r1 = rule(impl, attrs = {'a1': "
+ "attr.label(default = Label('//foo:foo'), allow_files=True)})");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
Attribute a = c.getAttributeByName("a1");
- assertEquals("//foo:foo", ((Label) a.getDefaultValueForTesting()).toString());
+ assertThat(a.getDefaultValueForTesting()).isInstanceOf(Label.class);
+ assertEquals("//foo:foo", a.getDefaultValueForTesting().toString());
}
@Test
public void testIntDefaultValue() throws Exception {
- eval("def impl(ctx): return None", "r1 = rule(impl, attrs = {'a1': attr.int(default = 40+2)})");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ evalAndExport(
+ "def impl(ctx): return None",
+ "r1 = rule(impl, attrs = {'a1': attr.int(default = 40+2)})");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
Attribute a = c.getAttributeByName("a1");
assertEquals(42, a.getDefaultValueForTesting());
}
@@ -406,8 +422,8 @@
@Test
public void testRuleInheritsBaseRuleAttributes() throws Exception {
- eval("def impl(ctx): return None", "r1 = rule(impl)");
- RuleClass c = ((RuleFunction) lookup("r1")).getBuilder().build("r1");
+ evalAndExport("def impl(ctx): return None", "r1 = rule(impl)");
+ RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
assertTrue(c.hasAttr("tags", Type.STRING_LIST));
assertTrue(c.hasAttr("visibility", BuildType.NODEP_LABEL_LIST));
assertTrue(c.hasAttr("deprecation", Type.STRING));