Add @Nullable annotations on getters

Generated getters’ documentation states that they can return null if
no value is present; this patch adds a @Nullable annotation to
accompany the comment, allowing null-pointer analysis on generated
bindings.

This would help avoid various NPEs we’ve seen in practice in Genius
and NetVirt.

Change-Id: I8c8cfa9ae24d34f3a7140623065dfb377f5f2a13
Signed-off-by: Stephen Kitt <skitt@redhat.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
index 047716a10f96d6fef14aff548a322357f7c4a2ed..6fb0fdcbd9bc6afd7f3b7a41fc63de7a4aa133ed 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
@@ -1751,6 +1751,10 @@ abstract class AbstractTypeGenerator {
         if (node.getStatus() == Status.DEPRECATED) {
             getMethod.addAnnotation("java.lang", "Deprecated");
         }
+        if (!returnType.getPackageName().isEmpty()) {
+            // The return type has a package, so it's not a primitive type
+            getMethod.addAnnotation("javax.annotation", "Nullable");
+        }
         addComment(getMethod, node);
 
         return getMethod;

diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java
index df86d87863878f783b4af78178ff160567d9edc6..3ede285aba6dca0a1f3864b1143cd035e0144389 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java
+++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTest.java
@@ -13,8 +13,10 @@ import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Range;
 import java.io.File;
+import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
@@ -23,12 +25,15 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.WildcardType;
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
 import org.junit.Test;
 import org.opendaylight.yangtools.yang.binding.ChildOf;
 import org.opendaylight.yangtools.yang.binding.annotations.RoutingContext;
@@ -46,7 +51,8 @@ public class CompilationTest extends BaseCompilationTest {
      */
     private static Collection<Method> abstractMethods(final Class<?> clazz) {
         // Filter out
-        return Collections2.filter(Arrays.asList(clazz.getDeclaredMethods()), input -> Modifier.isAbstract(input.getModifiers()));
+        return Collections2.filter(Arrays.asList(clazz.getDeclaredMethods()),
+            input -> Modifier.isAbstract(input.getModifiers()));
     }
 
     @Test
@@ -90,9 +96,12 @@ public class CompilationTest extends BaseCompilationTest {
         CompilationTestUtils.testCompilation(sourcesOutputDir, compiledOutputDir);
 
         final ClassLoader loader = new URLClassLoader(new URL[] { compiledOutputDir.toURI().toURL() });
-        final Class<?> keyArgsClass = Class.forName(CompilationTestUtils.BASE_PKG + ".urn.opendaylight.test.rev131008.KeyArgs", true, loader);
-        final Class<?> linksClass = Class.forName(CompilationTestUtils.BASE_PKG + ".urn.opendaylight.test.rev131008.Links", true, loader);
-        final Class<?> linksKeyClass = Class.forName(CompilationTestUtils.BASE_PKG + ".urn.opendaylight.test.rev131008.LinksKey", true, loader);
+        final Class<?> keyArgsClass = Class.forName(CompilationTestUtils.BASE_PKG
+            + ".urn.opendaylight.test.rev131008.KeyArgs", true, loader);
+        final Class<?> linksClass = Class.forName(CompilationTestUtils.BASE_PKG
+            + ".urn.opendaylight.test.rev131008.Links", true, loader);
+        final Class<?> linksKeyClass = Class.forName(CompilationTestUtils.BASE_PKG
+            + ".urn.opendaylight.test.rev131008.LinksKey", true, loader);
 
         // Test generated 'grouping key-args'
         assertTrue(keyArgsClass.isInterface());
@@ -405,7 +414,8 @@ public class CompilationTest extends BaseCompilationTest {
         final List<Range<Integer>> lengthConstraints = new ArrayList<>();
         lengthConstraints.add(Range.closed(1, 10));
         byte[] arg = new byte[] {};
-        String expectedMsg = String.format("Invalid length: %s, expected: %s.", Arrays.toString(arg), lengthConstraints);
+        String expectedMsg = String.format("Invalid length: %s, expected: %s.", Arrays.toString(arg),
+            lengthConstraints);
         CompilationTestUtils.assertContainsRestrictionCheck(builderObj, m, expectedMsg, arg);
 
         m = CompilationTestUtils.assertContainsMethod(builderClass, builderClass, "setIdDecimal64", BigDecimal.class);
@@ -419,7 +429,8 @@ public class CompilationTest extends BaseCompilationTest {
     }
 
     @Test
-    public void testGenerationContextReferenceExtension() throws Exception {
+    public void testGenerationContextReferenceExtension() throws IOException, URISyntaxException,
+            ClassNotFoundException {
         final File sourcesOutputDir = CompilationTestUtils.generatorOutput("context-reference");
         final File compiledOutputDir = CompilationTestUtils.compilerOutput("context-reference");
         generateTestSources("/compilation/context-reference", sourcesOutputDir);
@@ -439,25 +450,26 @@ public class CompilationTest extends BaseCompilationTest {
         CompilationTestUtils.testCompilation(sourcesOutputDir, compiledOutputDir);
 
         final ClassLoader loader = new URLClassLoader(new URL[] { compiledOutputDir.toURI().toURL() });
-        final Class<?> nodesClass = Class.forName(CompilationTestUtils.BASE_PKG + ".urn.opendaylight.foo.rev131008.Nodes", true, loader);
+        final Class<?> nodesClass = Class.forName(CompilationTestUtils.BASE_PKG
+            + ".urn.opendaylight.foo.rev131008.Nodes", true, loader);
         final Class<?> identityClass = Class
                 .forName(CompilationTestUtils.BASE_PKG + ".urn.opendaylight.bar.rev131008.IdentityClass", true, loader);
 
         // test identity
-        final Class<?> baseIdentity = Class.forName("org.opendaylight.yangtools.yang.binding.BaseIdentity", true, loader);
+        final Class<?> baseIdentity = Class.forName("org.opendaylight.yangtools.yang.binding.BaseIdentity", true,
+            loader);
         assertEquals(ImmutableList.of(baseIdentity), Arrays.asList(identityClass.getInterfaces()));
 
         // Test annotation
+        final Method getId;
         try {
-            final Method getId = nodesClass.getMethod("getId");
-            final Annotation[] annotations = getId.getAnnotations();
-            assertEquals(1, annotations.length);
-            final Annotation routingContext = annotations[0];
-            assertEquals(RoutingContext.class, routingContext.annotationType());
+            getId = nodesClass.getMethod("getId");
         } catch (final NoSuchMethodException e) {
-            throw new AssertionError("Method getId() not found");
+            throw new AssertionError("Method getId() not found", e);
         }
 
+        assertEquals(ImmutableSet.of(RoutingContext.class, Nullable.class), Arrays.stream(getId.getAnnotations())
+            .map(Annotation::annotationType).collect(Collectors.toSet()));
         CompilationTestUtils.cleanUp(sourcesOutputDir, compiledOutputDir);
     }
 
@@ -597,11 +609,13 @@ public class CompilationTest extends BaseCompilationTest {
         final ClassLoader loader = new URLClassLoader(new URL[] { compiledOutputDir.toURI().toURL() });
         final Class<?> outputActionClass = Class.forName(CompilationTestUtils.BASE_PKG
                 + ".urn.test.foo.rev140717.action.action.output.action._case.OutputAction", true, loader);
-        final Class<?> actionClass = Class.forName(CompilationTestUtils.BASE_PKG + ".urn.test.foo.rev140717.Action", true, loader);
+        final Class<?> actionClass = Class.forName(CompilationTestUtils.BASE_PKG + ".urn.test.foo.rev140717.Action",
+            true, loader);
 
         // Test generated 'container output-action'
         assertTrue(outputActionClass.isInterface());
-        CompilationTestUtils.assertImplementsParameterizedIfc(outputActionClass, ChildOf.class.toString(), actionClass.getCanonicalName());
+        CompilationTestUtils.assertImplementsParameterizedIfc(outputActionClass, ChildOf.class.toString(),
+            actionClass.getCanonicalName());
 
         CompilationTestUtils.cleanUp(sourcesOutputDir, compiledOutputDir);
     }
@@ -634,47 +648,36 @@ public class CompilationTest extends BaseCompilationTest {
     }
 
     private static void testReturnTypeIdentityref(final Class<?> clazz, final String methodName,
-            final String returnTypeStr) throws Exception {
-        Method method;
-        java.lang.reflect.Type returnType;
-        try {
-            method = clazz.getMethod(methodName);
-            assertEquals(java.lang.Class.class, method.getReturnType());
-            returnType = method.getGenericReturnType();
-            assertTrue(returnType instanceof ParameterizedType);
-            final ParameterizedType pt = (ParameterizedType) returnType;
-            final java.lang.reflect.Type[] parameters = pt.getActualTypeArguments();
-            assertEquals(1, parameters.length);
-            final java.lang.reflect.Type parameter = parameters[0];
-            assertTrue(parameter instanceof WildcardType);
-            final WildcardType wildcardType = (WildcardType) parameter;
-            assertEquals("? extends " + returnTypeStr, wildcardType.toString());
-        } catch (final NoSuchMethodException e) {
-            throw new AssertionError("Method '" + methodName + "' not found");
-        }
+            final String returnTypeStr) throws NoSuchMethodException {
+        Method method = clazz.getMethod(methodName);
+        assertEquals(java.lang.Class.class, method.getReturnType());
+        java.lang.reflect.Type returnType = method.getGenericReturnType();
+        assertTrue(returnType instanceof ParameterizedType);
+        final ParameterizedType pt = (ParameterizedType) returnType;
+        final java.lang.reflect.Type[] parameters = pt.getActualTypeArguments();
+        assertEquals(1, parameters.length);
+        final java.lang.reflect.Type parameter = parameters[0];
+        assertTrue(parameter instanceof WildcardType);
+        final WildcardType wildcardType = (WildcardType) parameter;
+        assertEquals("? extends " + returnTypeStr, wildcardType.toString());
     }
 
     private static void testReturnTypeInstanceIdentitifer(final ClassLoader loader, final Class<?> clazz,
-            final String methodName) throws Exception {
+            final String methodName) throws ClassNotFoundException, NoSuchMethodException, SecurityException {
         Method method;
         Class<?> rawReturnType;
         java.lang.reflect.Type returnType;
-        try {
-            method = clazz.getMethod(methodName);
-            rawReturnType = Class.forName("org.opendaylight.yangtools.yang.binding.InstanceIdentifier", true, loader);
-            assertEquals(rawReturnType, method.getReturnType());
-            returnType = method.getGenericReturnType();
-            assertTrue(returnType instanceof ParameterizedType);
-            final ParameterizedType pt = (ParameterizedType) returnType;
-            final java.lang.reflect.Type[] parameters = pt.getActualTypeArguments();
-            assertEquals(1, parameters.length);
-            final java.lang.reflect.Type parameter = parameters[0];
-            assertTrue(parameter instanceof WildcardType);
-            final WildcardType wildcardType = (WildcardType) parameter;
-            assertEquals("?", wildcardType.toString());
-        } catch (final NoSuchMethodException e) {
-            throw new AssertionError("Method '" + methodName + "' not found");
-        }
+        method = clazz.getMethod(methodName);
+        rawReturnType = Class.forName("org.opendaylight.yangtools.yang.binding.InstanceIdentifier", true, loader);
+        assertEquals(rawReturnType, method.getReturnType());
+        returnType = method.getGenericReturnType();
+        assertTrue(returnType instanceof ParameterizedType);
+        final ParameterizedType pt = (ParameterizedType) returnType;
+        final java.lang.reflect.Type[] parameters = pt.getActualTypeArguments();
+        assertEquals(1, parameters.length);
+        final java.lang.reflect.Type parameter = parameters[0];
+        assertTrue(parameter instanceof WildcardType);
+        final WildcardType wildcardType = (WildcardType) parameter;
+        assertEquals("?", wildcardType.toString());
     }
-
 }

diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java
index d9c8e88f5b83399f4ee2cd4efbc5414296045cb1..ee866de4b25c03638c348fe3f958233ac4dc9db7 100644 (file)
--- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java
+++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/test/CompilationTestUtils.java
@@ -387,9 +387,9 @@ public class CompilationTestUtils {
         File[] dirContent = dir.listFiles();
         if (dirContent == null) {
             throw new AssertionError("File " + dir + " doesn't exists or it's not a directory");
-        } else {
-            assertEquals("Unexpected count of generated files", count, dirContent.length);
         }
+
+        assertEquals("Unexpected count of generated files", count, dirContent.length);
     }
 
     /**