Expose structField callable methods of skylark objects to dir() and str() calls
RELNOTES: None.
PiperOrigin-RevId: 184498836
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Info.java b/src/main/java/com/google/devtools/build/lib/packages/Info.java
index 50fcb15..33b8c95 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Info.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Info.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.packages;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
@@ -108,14 +109,17 @@
public abstract boolean hasField(String name);
/**
- * {@inheritDoc}
- *
- * <p>Overrides {@link ClassObject#getValue(String)}, but does not allow {@link EvalException} to
- * be thrown.
+ * <p>Wraps {@link ClassObject#getValue(String)}, returning null in cases where
+ * {@link EvalException} would have been thrown.
*/
- @Nullable
- @Override
- public abstract Object getValue(String name);
+ @VisibleForTesting
+ public Object getValueOrNull(String name) {
+ try {
+ return getValue(name);
+ } catch (EvalException e) {
+ return null;
+ }
+ }
/**
* Returns the result of {@link #getValue(String)}, cast as the given type, throwing {@link
@@ -172,7 +176,7 @@
return false;
}
for (String field : getFieldNames()) {
- if (!this.getValue(field).equals(other.getValue(field))) {
+ if (!Objects.equal(this.getValueOrNull(field), other.getValueOrNull(field))) {
return false;
}
}
@@ -187,7 +191,7 @@
objectsToHash.add(provider);
for (String field : fields) {
objectsToHash.add(field);
- objectsToHash.add(getValue(field));
+ objectsToHash.add(getValueOrNull(field));
}
return Objects.hashCode(objectsToHash.toArray());
}
@@ -208,7 +212,7 @@
first = false;
printer.append(fieldName);
printer.append(" = ");
- printer.repr(getValue(fieldName));
+ printer.repr(getValueOrNull(fieldName));
}
printer.append(")");
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
index e1035c8..58ea8d3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
@@ -15,7 +15,11 @@
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.FuncallExpression;
+import com.google.devtools.build.lib.syntax.FuncallExpression.MethodDescriptor;
import java.util.Map;
/** Base class for native implementations of {@link Info}. */
@@ -23,19 +27,35 @@
public class NativeInfo extends Info {
protected final ImmutableMap<String, Object> values;
+ // Initialized lazily.
+ private ImmutableSet<String> fieldNames;
+
@Override
- public Object getValue(String name) {
- return values.get(name);
+ public Object getValue(String name) throws EvalException {
+ if (values.containsKey(name)) {
+ return values.get(name);
+ } else if (hasField(name)) {
+ MethodDescriptor methodDescriptor = FuncallExpression.getStructField(this.getClass(), name);
+ return FuncallExpression.invokeStructField(methodDescriptor, name, this);
+ } else {
+ return null;
+ }
}
@Override
public boolean hasField(String name) {
- return values.containsKey(name);
+ return getFieldNames().contains(name);
}
@Override
public ImmutableCollection<String> getFieldNames() {
- return values.keySet();
+ if (fieldNames == null) {
+ fieldNames = ImmutableSet.<String>builder()
+ .addAll(values.keySet())
+ .addAll(FuncallExpression.getStructFieldNames(this.getClass()))
+ .build();
+ }
+ return fieldNames;
}
public NativeInfo(NativeProvider<?> provider) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 833b20a..241789d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -42,12 +42,14 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
import javax.annotation.Nullable;
/**
@@ -134,6 +136,48 @@
}
});
+ private static final LoadingCache<Class<?>, Map<String, MethodDescriptor>> fieldCache =
+ CacheBuilder.newBuilder()
+ .initialCapacity(10)
+ .maximumSize(100)
+ .build(
+ new CacheLoader<Class<?>, Map<String, MethodDescriptor>>() {
+
+ @Override
+ public Map<String, MethodDescriptor> load(Class<?> key) throws Exception {
+ ImmutableMap.Builder<String, MethodDescriptor> fieldMap = ImmutableMap.builder();
+ HashSet<String> fieldNamesForCollisions = new HashSet<>();
+ List<MethodDescriptor> fieldMethods =
+ methodCache
+ .get(key)
+ .values()
+ .stream()
+ .flatMap(List::stream)
+ .filter(
+ methodDescriptor -> methodDescriptor.getAnnotation().structField())
+ .collect(Collectors.toList());
+
+ for (MethodDescriptor fieldMethod : fieldMethods) {
+ SkylarkCallable callable = fieldMethod.getAnnotation();
+ String name = callable.name();
+ if (name.isEmpty()) {
+ name =
+ StringUtilities.toPythonStyleFunctionName(
+ fieldMethod.getMethod().getName());
+ }
+ // TODO(b/72113542): Validate with annotation processor instead of at runtime.
+ if (!fieldNamesForCollisions.add(name)) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Class %s has two structField methods named %s defined",
+ key.getName(), name));
+ }
+ fieldMap.put(name, fieldMethod);
+ }
+ return fieldMap.build();
+ }
+ });
+
/**
* Returns a map of methods and corresponding SkylarkCallable annotations of the methods of the
* classObj class reachable from Skylark.
@@ -260,15 +304,30 @@
return printer.toString();
}
- /**
- * Returns the list of Skylark callable Methods of objClass with the given name and argument
- * number.
- */
+ /** Returns the Skylark callable Method of objClass with structField=true and the given name. */
+ public static MethodDescriptor getStructField(Class<?> objClass, String methodName) {
+ try {
+ return fieldCache.get(objClass).get(methodName);
+ } catch (ExecutionException e) {
+ throw new IllegalStateException("Method loading failed: " + e);
+ }
+ }
+
+ /** Returns the list of names of Skylark callable Methods of objClass with structField=true. */
+ public static Set<String> getStructFieldNames(Class<?> objClass) {
+ try {
+ return fieldCache.get(objClass).keySet();
+ } catch (ExecutionException e) {
+ throw new IllegalStateException("Method loading failed: " + e);
+ }
+ }
+
+ /** Returns the list of Skylark callable Methods of objClass with the given name. */
public static List<MethodDescriptor> getMethods(Class<?> objClass, String methodName) {
try {
return methodCache.get(objClass).get(methodName);
} catch (ExecutionException e) {
- throw new IllegalStateException("method invocation failed: " + e);
+ throw new IllegalStateException("Method loading failed: " + e);
}
}
@@ -280,10 +339,25 @@
try {
return methodCache.get(objClass).keySet();
} catch (ExecutionException e) {
- throw new IllegalStateException("method invocation failed: " + e);
+ throw new IllegalStateException("Method loading failed: " + e);
}
}
+ /**
+ * Invokes the given structField=true method and returns the result.
+ *
+ * @param methodDescriptor the descriptor of the method to invoke
+ * @param fieldName the name of the struct field
+ * @param obj the object on which to invoke the method
+ * @return the method return value
+ * @throws EvalException if there was an issue evaluating the method
+ */
+ public static Object invokeStructField(
+ MethodDescriptor methodDescriptor, String fieldName, Object obj) throws EvalException {
+ Preconditions.checkArgument(methodDescriptor.getAnnotation().structField());
+ return callMethod(methodDescriptor, fieldName, obj, new Object[0], Location.BUILTIN, null);
+ }
+
static Object callMethod(MethodDescriptor methodDescriptor, String methodName, Object obj,
Object[] args, Location loc, Environment env) throws EvalException {
try {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
index fa0add7..8980635 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformInfoTest.java
@@ -95,12 +95,6 @@
.setLabel(makeLabel("//platform/plat1"))
.addConstraint(value1)
.addConstraint(value2)
- .build(),
- PlatformInfo.builder()
- .setLabel(makeLabel("//platform/plat1"))
- .addConstraint(value1)
- .addConstraint(value2)
- .setRemoteExecutionProperties("key=val") // execution properties are ignored.
.build())
.addEqualityGroup(
// Different label.
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
index ef0b746..a4cb31a 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
@@ -139,7 +139,7 @@
}
/** Asserts that a {@link SkylarkInfo} has fields a=1, b=2, c=3 (and nothing else). */
- private static void assertHasExactlyValuesA1B2C3(SkylarkInfo info) {
+ private static void assertHasExactlyValuesA1B2C3(SkylarkInfo info) throws Exception {
assertThat(info.getFieldNames()).containsExactly("a", "b", "c");
assertThat(info.getValue("a")).isEqualTo(1);
assertThat(info.getValue("b")).isEqualTo(2);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java
index dc8b505..c0e9ade 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java
@@ -19,7 +19,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.OutputJar;
@@ -724,7 +723,7 @@
}
}
- private JavaInfo fetchJavaInfo() throws LabelSyntaxException {
+ private JavaInfo fetchJavaInfo() throws Exception {
ConfiguredTarget myRuleTarget = getConfiguredTarget("//foo:my_skylark_rule");
Info info =
myRuleTarget.get(new SkylarkKey(Label.parseAbsolute("//foo:extension.bzl"), "result"));
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 6baa230..c234a92 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -76,6 +76,42 @@
};
@SkylarkModule(name = "Mock", doc = "")
+ static class NativeInfoMock extends NativeInfo {
+
+ private static final NativeProvider<NativeInfoMock> CONSTRUCTOR =
+ new NativeProvider<NativeInfoMock>(NativeInfoMock.class, "native_info_mock") {};
+
+ public NativeInfoMock() {
+ super(CONSTRUCTOR);
+ }
+
+ @SkylarkCallable(name = "callable_string", doc = "", structField = false)
+ public String callableString() {
+ return "a";
+ }
+
+ @SkylarkCallable(name = "struct_field_string", doc = "", structField = true)
+ public String structFieldString() {
+ return "a";
+ }
+
+ @SkylarkCallable(name = "struct_field_callable", doc = "", structField = true)
+ public BuiltinFunction structFieldCallable() {
+ return foobar;
+ }
+
+ @SkylarkCallable(
+ name = "struct_field_none",
+ doc = "",
+ structField = true,
+ allowReturnNones = true
+ )
+ public String structFieldNone() {
+ return null;
+ }
+ }
+
+ @SkylarkModule(name = "Mock", doc = "")
static class Mock {
@SkylarkCallable(doc = "")
public static Integer valueOf(String str) {
@@ -1193,7 +1229,7 @@
" return e",
"e = str(func())").testLookup("e", "[3, [1, 4]]");
}
-
+
@Test
public void testDictTupleAssignmentAsLValue() throws Exception {
new SkylarkTest().setUp("def func():",
@@ -1355,6 +1391,26 @@
}
@Test
+ public void testStrNativeInfo() throws Exception {
+ new SkylarkTest()
+ .update("mock", new NativeInfoMock())
+ .testEval(
+ "str(mock)",
+ "'struct(struct_field_callable = <built-in function foobar>, struct_field_none = None, "
+ + "struct_field_string = \"a\")'");
+ }
+
+ @Test
+ public void testDirNativeInfo() throws Exception {
+ new SkylarkTest()
+ .update("mock", new NativeInfoMock())
+ .testEval(
+ "dir(mock)",
+ "['callable_string', 'struct_field_callable', "
+ + "'struct_field_none', 'struct_field_string']");
+ }
+
+ @Test
public void testPrint() throws Exception {
// TODO(fwe): cannot be handled by current testing suite
setFailFast(false);
@@ -1561,10 +1617,10 @@
new SkylarkTest()
.update("val", new SkylarkClassObjectWithSkylarkCallables())
.testIfExactError(
- // TODO(bazel-team): This should probably list callable_only_field/method as well.
+ // TODO(bazel-team): This should probably list callable_only_method as well.
"'struct_with_skylark_callables' object has no attribute 'nonexistent_field'\n"
- + "Available attributes: collision_field, collision_method, values_only_field, "
- + "values_only_method",
+ + "Available attributes: callable_only_field, collision_field, collision_method, "
+ + "values_only_field, values_only_method",
"v = val.nonexistent_field");
}
