diff options
6 files changed, 106 insertions, 25 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java index 9a5317f5a0..7451dd68b8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.packages.SkylarkClassObject; import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor.Key; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -27,6 +28,14 @@ import javax.annotation.Nullable; * <p>This is an ephemeral object created only for the analysis of a single configured target. After * that configured target is analyzed, this is thrown away. */ +// TODO(bazel-team): This annotation is needed for compatibility with Skylark code that expects +// type() of this class to be "MergedConfiguredTarget" instead of "Target". The dependent code +// should be refactored, possibly by adding a field to this class that it could read instead. +@SkylarkModule( + name = "MergedConfiguredTarget", + doc = "", + documented = false +) public final class MergedConfiguredTarget extends AbstractConfiguredTarget { private final ConfiguredTarget base; private final TransitiveInfoProviderMap providers; diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java index 95d4f4fcf5..348a9ccef4 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkInterfaceUtils.java @@ -23,40 +23,59 @@ import javax.annotation.Nullable; public class SkylarkInterfaceUtils { /** + * Returns the {@link SkylarkModule} annotation for the given class, if it exists, and + * null otherwise. The first annotation found will be returned, starting with {@code classObj} + * and following its base classes and interfaces recursively. + */ + @Nullable + public static SkylarkModule getSkylarkModule(Class<?> classObj) { + if (classObj.isAnnotationPresent(SkylarkModule.class)) { + return classObj.getAnnotation(SkylarkModule.class); + } + Class<?> superclass = classObj.getSuperclass(); + if (superclass != null) { + SkylarkModule annotation = getSkylarkModule(superclass); + if (annotation != null) { + return annotation; + } + } + for (Class<?> interfaceObj : classObj.getInterfaces()) { + SkylarkModule annotation = getSkylarkModule(interfaceObj); + if (annotation != null) { + return annotation; + } + } + return null; + } + + /** * Returns the {@link SkylarkCallable} annotation for the given method, if it exists, and - * null otherwise. The method must be declared in {@code classObj} or one of its base classes - * or interfaces. The first annotation of an overridden version of the method that is found + * null otherwise. The first annotation of an overridden version of the method that is found * will be returned, starting with {@code classObj} and following its base classes and - * interfaces recursively. + * interfaces recursively, skipping any annotation inside a class not marked + * {@link SkylarkModule}. */ @Nullable public static SkylarkCallable getSkylarkCallable(Class<?> classObj, Method method) { - boolean keepLooking = false; try { Method superMethod = classObj.getMethod(method.getName(), method.getParameterTypes()); if (classObj.isAnnotationPresent(SkylarkModule.class) && superMethod.isAnnotationPresent(SkylarkCallable.class)) { return superMethod.getAnnotation(SkylarkCallable.class); - } else { - keepLooking = true; } } catch (NoSuchMethodException e) { - // The class might not have the specified method, so an exceptions is OK. - keepLooking = true; + // The class might not have the specified method, so an exception is OK. } - if (keepLooking) { - if (classObj.getSuperclass() != null) { - SkylarkCallable annotation = - getSkylarkCallable(classObj.getSuperclass(), method); - if (annotation != null) { - return annotation; - } + if (classObj.getSuperclass() != null) { + SkylarkCallable annotation = getSkylarkCallable(classObj.getSuperclass(), method); + if (annotation != null) { + return annotation; } - for (Class<?> interfaceObj : classObj.getInterfaces()) { - SkylarkCallable annotation = getSkylarkCallable(interfaceObj, method); - if (annotation != null) { - return annotation; - } + } + for (Class<?> interfaceObj : classObj.getInterfaces()) { + SkylarkCallable annotation = getSkylarkCallable(interfaceObj, method); + if (annotation != null) { + return annotation; } } return null; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java index 5b1834d868..9609544e19 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java @@ -21,6 +21,7 @@ import com.google.common.collect.Ordering; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; import com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils; @@ -255,9 +256,9 @@ public final class EvalUtils { * when the given class identifies a Skylark name space. */ public static String getDataTypeNameFromClass(Class<?> c, boolean highlightNameSpaces) { - if (c.isAnnotationPresent(SkylarkModule.class)) { - SkylarkModule module = c.getAnnotation(SkylarkModule.class); - return c.getAnnotation(SkylarkModule.class).name() + SkylarkModule module = SkylarkInterfaceUtils.getSkylarkModule(c); + if (module != null) { + return module.name() + ((module.namespace() && highlightNameSpaces) ? " (a language module)" : ""); } else if (c.equals(Object.class)) { return "unknown"; diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java index 7ce40aa90c..65b175c015 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; @@ -52,6 +53,15 @@ public class EvalUtilsTest extends EvaluationTestCase { assertThat(EvalUtils.toIterable("abc", null)).hasSize(3); } + /** MockClassA */ + @SkylarkModule(name = "MockClassA", doc = "MockClassA") + public static class MockClassA { + } + + /** MockClassB */ + public static class MockClassB extends MockClassA { + } + @Test public void testDataTypeNames() throws Exception { assertEquals("string", EvalUtils.getDataTypeName("foo")); @@ -60,6 +70,8 @@ public class EvalUtilsTest extends EvaluationTestCase { assertEquals("list", EvalUtils.getDataTypeName(makeList(null))); assertEquals("dict", EvalUtils.getDataTypeName(makeDict(null))); assertEquals("NoneType", EvalUtils.getDataTypeName(Runtime.NONE)); + assertEquals("MockClassA", EvalUtils.getDataTypeName(new MockClassA())); + assertEquals("MockClassA", EvalUtils.getDataTypeName(new MockClassB())); } @Test 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 97a3935d2a..2fee2c36fd 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 @@ -778,7 +778,7 @@ public class SkylarkEvaluationTest extends EvaluationTest { new SkylarkTest() .update("mock", new MockSubClass()) .testIfExactError( - "Type MockSubClass has no function is_empty_class_not_annotated(string)", + "Type Mock has no function is_empty_class_not_annotated(string)", "b = mock.is_empty_class_not_annotated('a')"); } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java index 4172024e2a..23fc0b39d3 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkInterfaceUtilsTest.java @@ -81,6 +81,41 @@ public class SkylarkInterfaceUtilsTest { public void foo() {} } + /** MockClassZ */ + public static class MockClassZ { + } + + @Test + public void testGetSkylarkModuleBasic() throws Exception { + // Normal case. + SkylarkModule ann = SkylarkInterfaceUtils.getSkylarkModule(MockClassA.class); + assertThat(ann).isNotNull(); + assertThat(ann.doc()).isEqualTo("MockClassA"); + } + + @Test + public void testGetSkylarkModuleSubclass() throws Exception { + // Subclass's annotation is used. + SkylarkModule ann = SkylarkInterfaceUtils.getSkylarkModule(MockClassC.class); + assertThat(ann).isNotNull(); + assertThat(ann.doc()).isEqualTo("MockClassC"); + } + + @Test + public void testGetSkylarkModuleSubclassNoSubannotation() throws Exception { + // Falls back on superclass's annotation. + SkylarkModule ann = SkylarkInterfaceUtils.getSkylarkModule(MockClassD.class); + assertThat(ann).isNotNull(); + assertThat(ann.doc()).isEqualTo("MockClassC"); + } + + @Test + public void testGetSkylarkModuleNotFound() throws Exception { + // Doesn't exist. + SkylarkModule ann = SkylarkInterfaceUtils.getSkylarkModule(MockClassZ.class); + assertThat(ann).isNull(); + } + @Test public void testGetSkylarkCallableBasic() throws Exception { // Normal case. Ensure two-arg form is consistent with one-arg form. @@ -127,10 +162,15 @@ public class SkylarkInterfaceUtilsTest { SkylarkCallable ann = SkylarkInterfaceUtils.getSkylarkCallable(method); assertThat(ann).isNull(); - // ... including when it's only present in a subclass that was bypassed. + // ... including when it's only present in a subclass that was bypassed... method = MockClassC.class.getMethod("baz"); ann = SkylarkInterfaceUtils.getSkylarkCallable(MockClassA.class, method); assertThat(ann).isNull(); + + // ... or when the method itself is only in the subclass that was bypassed. + method = MockClassC.class.getMethod("qux"); + ann = SkylarkInterfaceUtils.getSkylarkCallable(MockClassA.class, method); + assertThat(ann).isNull(); } @Test |