diff options
5 files changed, 125 insertions, 60 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index ed8af8a6e7..ee2f8ac88f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -54,6 +54,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; /** @@ -199,7 +200,7 @@ public final class Attribute implements Comparable<Attribute> { **/ SPLIT(true); - private boolean defaultsToSelf; + private final boolean defaultsToSelf; ConfigurationTransition() { this(false); @@ -657,21 +658,29 @@ public final class Attribute implements Comparable<Attribute> { /** * See value(TYPE) above. This method is only meant for Skylark usage. * - * <p>The parameter {@code context} is relevant iff the default value is a Label string. - * In this case, {@code context} must point to the parent Label in order to be able to convert - * the default value string to a proper Label. + * <p>The parameter {@code context} is relevant iff the default value is a Label string. In this + * case, {@code context} must point to the parent Label in order to be able to convert the + * default value string to a proper Label. + * + * @param parameterName The name of the attribute to use in error messages */ - public Builder<TYPE> defaultValue(Object defaultValue, Object context) + public Builder<TYPE> defaultValue( + Object defaultValue, Object context, @Nullable String parameterName) throws ConversionException { Preconditions.checkState(!valueSet, "the default value is already set"); - value = type.convert(defaultValue, "attribute " + name, context); + value = + type.convert( + defaultValue, + ((parameterName == null) ? "" : String.format("parameter '%s' of ", parameterName)) + + String.format("attribute '%s'", name), + context); valueSet = true; return this; } /** See value(TYPE) above. This method is only meant for Skylark usage. */ public Builder<TYPE> defaultValue(Object defaultValue) throws ConversionException { - return defaultValue(defaultValue, null); + return defaultValue(defaultValue, null, null); } public boolean isValueSet() { diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java index e0b1bbcf8c..418ac4d48f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java @@ -250,6 +250,9 @@ public final class BuildType { return (Label) x; } try { + if (x instanceof String && context == null) { + return Label.parseAbsolute((String) x, false); + } return ((Label) context).getRelative(STRING.convert(x, what, context)); } catch (LabelSyntaxException e) { throw new ConversionException("invalid label '" + x + "' in " 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 67468cdf21..eb4623c012 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 @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.packages.ClassObjectConstructor; import com.google.devtools.build.lib.packages.SkylarkAspect; import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; @@ -163,7 +164,17 @@ public final class SkylarkAttr { throws EvalException { // We use an empty name now so that we can set it later. // This trick makes sense only in the context of Skylark (builtin rules should not use it). - Attribute.Builder<?> builder = Attribute.attr("", type); + return createAttribute(type, arguments, ast, env, ""); + } + + private static Attribute.Builder<?> createAttribute( + Type<?> type, + SkylarkDict<String, Object> arguments, + FuncallExpression ast, + Environment env, + String name) + throws EvalException { + Attribute.Builder<?> builder = Attribute.attr(name, type); Object defaultValue = arguments.get(DEFAULT_ARG); if (!EvalUtils.isNullOrNone(defaultValue)) { @@ -179,7 +190,7 @@ public final class SkylarkAttr { new SkylarkComputedDefaultTemplate( type, callback.getParameterNames(), callback, ast.getLocation())); } else { - builder.defaultValue(defaultValue, env.getGlobals().getTransitiveLabel()); + builder.defaultValue(defaultValue, env.getGlobals().getTransitiveLabel(), DEFAULT_ARG); } } @@ -541,7 +552,10 @@ public final class SkylarkAttr { parameters = { @Param( name = DEFAULT_ARG, - type = Label.class, + allowedTypes = { + @ParamType(type = Label.class), + @ParamType(type = String.class), + }, callbackEnabled = true, noneable = true, defaultValue = "None", @@ -549,9 +563,9 @@ public final class SkylarkAttr { positional = false, doc = DEFAULT_DOC - + " Use the <a href=\"globals.html#Label\"><code>Label</code></a> function to " - + "specify a default value ex:</p>" - + "<code>attr.label(default = Label(\"//a:b\"))</code>" + + "Use a string or the <a href=\"globals.html#Label\"><code>Label</code></a> " + + "function to specify a default value ex: " + + "<code>attr.label(default = \"//a:b\")</code>" ), @Param( name = EXECUTABLE_ARG, @@ -625,13 +639,13 @@ public final class SkylarkAttr { doc = CONFIGURATION_DOC ), @Param( - name = ASPECTS_ARG, - type = SkylarkList.class, - generic1 = SkylarkAspect.class, - defaultValue = "[]", - named = true, - positional = false, - doc = ASPECTS_ARG_DOC + name = ASPECTS_ARG, + type = SkylarkList.class, + generic1 = SkylarkAspect.class, + defaultValue = "[]", + named = true, + positional = false, + doc = ASPECTS_ARG_DOC ) }, useAst = true, @@ -655,33 +669,34 @@ public final class SkylarkAttr { throws EvalException { env.checkLoadingOrWorkspacePhase("attr.label", ast.getLocation()); try { - Attribute.Builder<?> attribute = createAttribute( - BuildType.LABEL, - EvalUtils.<String, Object>optionMap( + Attribute.Builder<?> attribute = + createAttribute( + BuildType.LABEL, + EvalUtils.<String, Object>optionMap( + env, + DEFAULT_ARG, + defaultO, + EXECUTABLE_ARG, + executable, + ALLOW_FILES_ARG, + allowFiles, + ALLOW_SINGLE_FILE_ARG, + allowSingleFile, + MANDATORY_ARG, + mandatory, + PROVIDERS_ARG, + providers, + ALLOW_RULES_ARG, + allowRules, + SINGLE_FILE_ARG, + singleFile, + CONFIGURATION_ARG, + cfg, + ASPECTS_ARG, + aspects), + ast, env, - DEFAULT_ARG, - defaultO, - EXECUTABLE_ARG, - executable, - ALLOW_FILES_ARG, - allowFiles, - ALLOW_SINGLE_FILE_ARG, - allowSingleFile, - MANDATORY_ARG, - mandatory, - PROVIDERS_ARG, - providers, - ALLOW_RULES_ARG, - allowRules, - SINGLE_FILE_ARG, - singleFile, - CONFIGURATION_ARG, - cfg, - ASPECTS_ARG, - aspects - ), - ast, - env); + "label"); return new Descriptor(attribute); } catch (EvalException e) { throw new EvalException(ast.getLocation(), e.getMessage(), e); @@ -836,9 +851,9 @@ public final class SkylarkAttr { positional = false, doc = DEFAULT_DOC - + " Use the <a href=\"globals.html#Label\"><code>Label</code></a> function to " - + "specify default values ex:</p>" - + "<code>attr.label_list(default = [ Label(\"//a:b\"), Label(\"//a:c\") ])</code>" + + "Use strings or the <a href=\"globals.html#Label\"><code>Label</code></a> " + + "function to specify default values ex: " + + "<code>attr.label_list(default = [\"//a:b\", \"//a:c\"])</code>" ), @Param( name = ALLOW_FILES_ARG, // bool or FileType filter @@ -957,11 +972,10 @@ public final class SkylarkAttr { CONFIGURATION_ARG, cfg, ASPECTS_ARG, - aspects - ); + aspects); try { Attribute.Builder<?> attribute = - createAttribute(BuildType.LABEL_LIST, kwargs, ast, env); + createAttribute(BuildType.LABEL_LIST, kwargs, ast, env, "label_list"); return new Descriptor(attribute); } catch (EvalException e) { throw new EvalException(ast.getLocation(), e.getMessage(), e); @@ -988,10 +1002,10 @@ public final class SkylarkAttr { positional = false, doc = DEFAULT_DOC - + " Use the <a href=\"globals.html#Label\"><code>Label</code></a> function to " - + "specify default values ex:</p>" + + "Use strings or the <a href=\"globals.html#Label\"><code>Label</code></a> " + + "function to specify default values ex: " + "<code>attr.label_keyed_string_dict(default = " - + "{ Label(\"//a:b\"): \"value\", Label(\"//a:c\"): \"string\" })</code>" + + "{\"//a:b\": \"value\", \"//a:c\": \"string\"})</code>" ), @Param( name = ALLOW_FILES_ARG, // bool or FileType filter @@ -1113,7 +1127,8 @@ public final class SkylarkAttr { aspects); try { Attribute.Builder<?> attribute = - createAttribute(BuildType.LABEL_KEYED_STRING_DICT, kwargs, ast, env); + createAttribute( + BuildType.LABEL_KEYED_STRING_DICT, kwargs, ast, env, "label_keyed_string_dict"); return new Descriptor(attribute); } catch (EvalException e) { throw new EvalException(ast.getLocation(), e.getMessage(), e); 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 50c477520d..aef0e11052 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 @@ -430,6 +430,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { .add(Attribute.attr("tags", Type.STRING_LIST)) .build(); } + @Test public void testAttrAllowedRuleClassesSpecificRuleClasses() throws Exception { Attribute attr = buildAttribute("a", @@ -437,6 +438,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { assertThat(attr.getAllowedRuleClassesPredicate().apply(ruleClass("java_binary"))).isTrue(); assertThat(attr.getAllowedRuleClassesPredicate().apply(ruleClass("genrule"))).isFalse(); } + @Test public void testAttrDefaultValue() throws Exception { Attribute attr = buildAttribute("a1", "attr.string(default = 'some value')"); @@ -444,6 +446,42 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { } @Test + public void testLabelAttrDefaultValueAsString() throws Exception { + Attribute sligleAttr = buildAttribute("a1", "attr.label(default = '//foo:bar')"); + assertThat(sligleAttr.getDefaultValueForTesting()) + .isEqualTo(Label.parseAbsolute("//foo:bar", false)); + + Attribute listAttr = + buildAttribute("a2", "attr.label_list(default = ['//foo:bar', '//bar:foo'])"); + assertThat(listAttr.getDefaultValueForTesting()) + .isEqualTo( + ImmutableList.of( + Label.parseAbsolute("//foo:bar", false), Label.parseAbsolute("//bar:foo", false))); + + Attribute dictAttr = + buildAttribute("a3", "attr.label_keyed_string_dict(default = {'//foo:bar': 'my value'})"); + assertThat(dictAttr.getDefaultValueForTesting()) + .isEqualTo(ImmutableMap.of(Label.parseAbsolute("//foo:bar", false), "my value")); + } + + @Test + public void testLabelAttrDefaultValueAsStringBadValue() throws Exception { + checkErrorContains( + "invalid label '/foo:bar' in parameter 'default' of attribute 'label': " + + "invalid label: /foo:bar", + "attr.label(default = '/foo:bar')"); + + checkErrorContains( + "invalid label '/bar:foo' in element 1 of parameter 'default' of attribute " + + "'label_list': invalid label: /bar:foo", + "attr.label_list(default = ['//foo:bar', '/bar:foo'])"); + + checkErrorContains( + "invalid label '/bar:foo' in dict key element: invalid label: /bar:foo", + "attr.label_keyed_string_dict(default = {'//foo:bar': 'a', '/bar:foo': 'b'})"); + } + + @Test public void testAttrDefaultValueBadType() throws Exception { checkErrorContains( "method attr.string(*, default: string, mandatory: bool, values: sequence of strings) " @@ -819,9 +857,9 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { @Test public void testLabelAttrWrongDefault() throws Exception { checkErrorContains( - "expected Label or Label-returning function or NoneType for 'default' " - + "while calling label but got string instead: //foo:bar", - "attr.label(default = '//foo:bar')"); + "expected value of type 'string' for parameter 'default' of attribute 'label', " + + "but got 123 (int)", + "attr.label(default = 123)"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java index 995683e079..a7c45f436b 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java @@ -243,7 +243,7 @@ public class TypeTest { try { BuildType.LABEL.convert("wiz", null); fail(); - } catch (NullPointerException e) { + } catch (ConversionException e) { /* ok */ } } |