Implement field declarations for declared providers. RELNOTES: Skylark providers can specify allowed fields and their documentation. PiperOrigin-RevId: 166446104
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index 5b4363e..3e850d9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -74,6 +74,7 @@ import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.BaseFunction; @@ -90,6 +91,7 @@ import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor; +import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.devtools.build.lib.syntax.SkylarkUtils; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.syntax.Type.ConversionException; @@ -288,15 +290,53 @@ defaultValue = "''", doc = "A description of the provider that can be extracted by documentation generating tools." + ), + @Param( + name = "fields", + doc = "If specified, restricts the set of allowed fields. <br>" + + "Possible values are:" + + "<ul>" + + " <li> list of fields:<br>" + + " <pre class=\"language-python\">provider(fields = ['a', 'b'])</pre><p>" + + " <li> dictionary field name -> documentation:<br>" + + " <pre class=\"language-python\">provider(\n" + + " fields = { 'a' : 'Documentation for a', 'b' : 'Documentation for b' })</pre>" + + "</ul>" + + "All fields are optional.", + allowedTypes = { + @ParamType(type = SkylarkList.class, generic1 = String.class), + @ParamType(type = SkylarkDict.class) + }, + noneable = true, + named = true, + positional = false, + defaultValue = "None" ) }, useLocation = true ) private static final BuiltinFunction provider = new BuiltinFunction("provider") { - public Provider invoke(String doc, Location location) { + public Provider invoke(String doc, Object fields, Location location) throws EvalException { + Iterable<String> fieldNames = null; + if (fields instanceof SkylarkList<?>) { + @SuppressWarnings("unchecked") + SkylarkList<String> list = (SkylarkList<String>) + SkylarkType.cast( + fields, + SkylarkList.class, String.class, location, + "Expected list of strings or dictionary of string -> string for 'fields'"); + fieldNames = list; + } else if (fields instanceof SkylarkDict) { + Map<String, String> dict = SkylarkType.castMap( + fields, + String.class, String.class, + "Expected list of strings or dictionary of string -> string for 'fields'"); + fieldNames = dict.keySet(); + } return new SkylarkProvider( "<no name>", // name is set on export. + fieldNames, location); } }; @@ -342,7 +382,7 @@ + "implicitly added and must not be specified. Attributes " + "<code>visibility</code>, <code>deprecation</code>, <code>tags</code>, " + "<code>testonly</code>, and <code>features</code> are implicitly added and " - + "cannot be overriden." + + "cannot be overridden." ), // TODO(bazel-team): need to give the types of these builtin attributes @Param(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java index d76c006..852c6c0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
@@ -14,6 +14,8 @@ package com.google.devtools.build.lib.packages; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; @@ -46,21 +48,48 @@ * * <p>Needs to be exported later. */ - public SkylarkProvider(String name, Location location) { - this(name, SIGNATURE, location); + public SkylarkProvider(String name, + @Nullable Iterable<String> fields, + Location location) { + this(name, buildSignature(fields), location); } - public SkylarkProvider( - String name, FunctionSignature.WithValues<Object, SkylarkType> signature, Location location) { + private SkylarkProvider( + String name, + FunctionSignature.WithValues<Object, SkylarkType> signature, Location location) { super(name, signature, location); this.errorMessageFormatForInstances = DEFAULT_ERROR_MESSAFE; } + private static FunctionSignature.WithValues<Object, SkylarkType> buildSignature( + @Nullable Iterable<String> fields) { + if (fields == null) { + return SIGNATURE; + } + return + FunctionSignature.WithValues.create( + FunctionSignature.namedOnly(0, ImmutableList.copyOf(fields).toArray(new String[0])) + ); + } + @Override protected Info createInstanceFromSkylark(Object[] args, Location loc) throws EvalException { - @SuppressWarnings("unchecked") - Map<String, Object> kwargs = (Map<String, Object>) args[0]; - return new SkylarkInfo(this, kwargs, loc); + if (signature.getSignature().getShape().hasKwArg()) { + @SuppressWarnings("unchecked") + Map<String, Object> kwargs = (Map<String, Object>) args[0]; + return new SkylarkInfo(this, kwargs, loc); + } else { + // todo(dslomov): implement shape sharing. + ImmutableList<String> names = signature.getSignature().getNames(); + Preconditions.checkState(names.size() == args.length); + ImmutableMap.Builder<String, Object> fields = ImmutableMap.builder(); + for (int i = 0; i < args.length; i++) { + if (args[i] != null) { + fields.put(names.get(i), args[i]); + } + } + return new SkylarkInfo(this, fields.build(), loc); + } } @Override @@ -75,8 +104,13 @@ } @Override + public String getName() { + return key != null ? key.getExportedName() : "<no name>"; + } + + @Override public String getPrintableName() { - return key != null ? key.getExportedName() : getName(); + return getName(); } @Override
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java b/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java index 489303f..0cbea6a 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java
@@ -40,7 +40,7 @@ private static final Provider P_NATIVE = new NativeProvider<Info>(Info.class, "p_native") {}; private static final SkylarkProvider P_SKYLARK = - new SkylarkProvider("p_skylark", Location.BUILTIN); + new SkylarkProvider("p_skylark", null, Location.BUILTIN); static { try {
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 6a7e201..855162d 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
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.SkylarkAspect; import com.google.devtools.build.lib.packages.SkylarkAspectClass; +import com.google.devtools.build.lib.packages.SkylarkInfo; import com.google.devtools.build.lib.packages.SkylarkProvider; import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction; @@ -1466,6 +1467,89 @@ assertThat(p1.getKey()).isEqualTo(new SkylarkProvider.SkylarkKey(FAKE_LABEL, "p")); } + @Test + public void providerWithFields() throws Exception { + evalAndExport( + "p = provider(fields = ['x', 'y'])", + "p1 = p(x = 1, y = 2)", + "x = p1.x", + "y = p1.y" + ); + SkylarkProvider p = (SkylarkProvider) lookup("p"); + SkylarkInfo p1 = (SkylarkInfo) lookup("p1"); + + + assertThat(p1.getProvider()).isEqualTo(p); + assertThat(lookup("x")).isEqualTo(1); + assertThat(lookup("y")).isEqualTo(2); + } + + @Test + public void providerWithFieldsDict() throws Exception { + evalAndExport( + "p = provider(fields = { 'x' : 'I am x', 'y' : 'I am y'})", + "p1 = p(x = 1, y = 2)", + "x = p1.x", + "y = p1.y" + ); + SkylarkProvider p = (SkylarkProvider) lookup("p"); + SkylarkInfo p1 = (SkylarkInfo) lookup("p1"); + + + assertThat(p1.getProvider()).isEqualTo(p); + assertThat(lookup("x")).isEqualTo(1); + assertThat(lookup("y")).isEqualTo(2); + } + + @Test + public void providerWithFieldsOptional() throws Exception { + evalAndExport( + "p = provider(fields = ['x', 'y'])", + "p1 = p(y = 2)", + "y = p1.y" + ); + SkylarkProvider p = (SkylarkProvider) lookup("p"); + SkylarkInfo p1 = (SkylarkInfo) lookup("p1"); + + + assertThat(p1.getProvider()).isEqualTo(p); + assertThat(lookup("y")).isEqualTo(2); + } + + @Test + public void providerWithFieldsOptionalError() throws Exception { + ev.setFailFast(false); + evalAndExport( + "p = provider(fields = ['x', 'y'])", + "p1 = p(y = 2)", + "x = p1.x" + ); + MoreAsserts.assertContainsEvent(ev.getEventCollector(), + " 'p' object has no attribute 'x'"); + } + + @Test + public void providerWithExtraFieldsError() throws Exception { + ev.setFailFast(false); + evalAndExport( + "p = provider(fields = ['x', 'y'])", + "p1 = p(x = 1, y = 2, z = 3)" + ); + MoreAsserts.assertContainsEvent(ev.getEventCollector(), + "unexpected keyword 'z' in call to p(*, x = ?, y = ?)"); + } + + @Test + public void providerWithEmptyFieldsError() throws Exception { + ev.setFailFast(false); + evalAndExport( + "p = provider(fields = [])", + "p1 = p(x = 1, y = 2, z = 3)" + ); + MoreAsserts.assertContainsEvent(ev.getEventCollector(), + "unexpected keywords 'x', 'y', 'z' in call to p()"); + } + @Test public void starTheOnlyAspectArg() throws Exception {