diff options
author | Jon Brandvein <brandjon@google.com> | 2016-09-30 17:19:28 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2016-10-04 08:53:12 +0000 |
commit | 22ebfb834c9e79fec4f5ff39a3599af00130ca8f (patch) | |
tree | 4985922588f6e9d640e30469bb9f9eab94a2ec74 | |
parent | fb65f9382f34c2641b5069c30a57a08445b920a3 (diff) |
Fix NPE in skylark documentation processor
Crash was triggered by overriding a @SkylarkCallable without repeating the
annotation in the subclass's method.
--
MOS_MIGRATED_REVID=134797463
3 files changed, 48 insertions, 6 deletions
diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java index c0ab2667f3..dd70c81b82 100644 --- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java +++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; +import com.google.devtools.build.lib.syntax.FuncallExpression; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; @@ -52,7 +53,7 @@ public abstract class SkylarkMethodDoc extends SkylarkDoc { } private String getParameterString(Method method) { - SkylarkCallable annotation = method.getAnnotation(SkylarkCallable.class); + SkylarkCallable annotation = FuncallExpression.getAnnotationFromParentClass(method); int nbPositional = annotation.mandatoryPositionals(); if (annotation.parameters().length > 0 && nbPositional < 0) { nbPositional = 0; @@ -75,7 +76,7 @@ public abstract class SkylarkMethodDoc extends SkylarkDoc { } protected String getSignature(String objectName, String methodName, Method method) { - String args = method.getAnnotation(SkylarkCallable.class).structField() + String args = FuncallExpression.getAnnotationFromParentClass(method).structField() ? "" : "(" + getParameterString(method) + ")"; return String.format("%s %s.%s%s", 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 2782bc8958..c41fcec477 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 @@ -105,8 +105,7 @@ public final class FuncallExpression extends Expression { if (method.isSynthetic()) { continue; } - SkylarkCallable callable = - getAnnotationFromParentClass(method.getDeclaringClass(), method); + SkylarkCallable callable = getAnnotationFromParentClass(method); if (callable == null) { continue; } @@ -164,8 +163,15 @@ public final class FuncallExpression extends Expression { return methodMap.build(); } + /** + * 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 + * will be returned, starting with {@code classObj} and following its base classes and + * interfaces recursively. + */ @Nullable - private static SkylarkCallable getAnnotationFromParentClass(Class<?> classObj, Method method) { + public static SkylarkCallable getAnnotationFromParentClass(Class<?> classObj, Method method) { boolean keepLooking = false; try { Method superMethod = classObj.getMethod(method.getName(), method.getParameterTypes()); @@ -197,6 +203,15 @@ public final class FuncallExpression extends Expression { return null; } + /** + * Convenience version of {@code getAnnotationsFromParentClass(Class, Method)} that uses + * the declaring class of the method. + */ + @Nullable + public static SkylarkCallable getAnnotationFromParentClass(Method method) { + return getAnnotationFromParentClass(method.getDeclaringClass(), method); + } + private static class ArgumentListConversionResult { private final ImmutableList<Object> arguments; private final String error; diff --git a/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java b/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java index cb8a039a88..a536db7945 100644 --- a/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java +++ b/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java @@ -108,14 +108,16 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { assertThat(collect(SkylarkRuleContext.class)).isNotEmpty(); } + /** MockClassA */ @SkylarkModule(name = "MockClassA", doc = "") public static class MockClassA { - @SkylarkCallable(doc = "") + @SkylarkCallable(doc = "MockClassA#get") public Integer get() { return 0; } } + /** MockClassB */ @SkylarkModule(name = "MockClassB", doc = "") public static class MockClassB { @SkylarkCallable(doc = "") @@ -124,6 +126,7 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { } } + /** MockClassC */ @SkylarkModule(name = "MockClassC", doc = "") public static class MockClassC extends MockClassA { @SkylarkCallable(doc = "") @@ -132,6 +135,7 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { } } + /** MockClassD */ @SkylarkModule(name = "MockClassD", doc = "MockClassD") public static class MockClassD { @SkylarkCallable( @@ -148,6 +152,15 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { } } + /** MockClassE */ + @SkylarkModule(name = "MockClassE", doc = "MockClassE") + public static class MockClassE extends MockClassA { + @Override + public Integer get() { + return 1; + } + } + @Test public void testSkylarkJavaInterfaceExplorerOnSimpleClass() throws Exception { Map<String, SkylarkModuleDoc> objects = collect(MockClassA.class); @@ -185,6 +198,19 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { assertThat(methodDoc.getParams()).hasSize(3); } + @Test + public void testSkylarkCallableOverriding() throws Exception { + Map<String, SkylarkModuleDoc> objects = collect(MockClassE.class); + assertThat(objects).hasSize(1); + assertThat(objects).containsKey("MockClassE"); + SkylarkModuleDoc moduleDoc = objects.get("MockClassE"); + assertThat(moduleDoc.getDocumentation()).isEqualTo("MockClassE."); + assertThat(moduleDoc.getMethods()).hasSize(1); + SkylarkMethodDoc methodDoc = moduleDoc.getMethods().iterator().next(); + assertThat(methodDoc.getDocumentation()).isEqualTo("MockClassA#get."); + assertThat(methodDoc.getSignature()).isEqualTo("int MockClassE.get()"); + } + private Iterable<Method> extractMethods(Collection<SkylarkJavaMethodDoc> methods) { return Iterables.transform(methods, new Function<SkylarkJavaMethodDoc, Method>() { @Override |