[MVC] TYPE_USE annotation on MVC method parameters break the annotation processor

jknack · jknack · commit de6e7fe991d0 · 2021-09-14T11:59:46.000-03:00
- Support ElementType.TYPE_USE - Support generic Nonnull and Nullable annotation types - Fixes jooby-project#2408
diff --git a/modules/jooby-apt/src/main/java/io/jooby/internal/apt/ParamDefinition.java b/modules/jooby-apt/src/main/java/io/jooby/internal/apt/ParamDefinition.java
@@ -20,9 +20,13 @@
 import io.jooby.internal.apt.asm.ParamWriter;
 
 import javax.annotation.processing.ProcessingEnvironment;
+import javax.lang.model.AnnotatedConstruct;
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.Types;
+
+import java.lang.reflect.AnnotatedType;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
 import java.net.URI;
@@ -31,6 +35,7 @@
 import java.time.Duration;
 import java.time.Period;
 import java.time.ZoneId;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -111,14 +116,32 @@ public boolean isNullable() {
   }
 
   private boolean hasAnnotation(String type) {
-    for (AnnotationMirror annotation : parameter.getAnnotationMirrors()) {
-      if (annotation.getAnnotationType().toString().endsWith(type)) {
+    Set<String> annotations = annotations(parameter);
+    for (String annotation : annotations) {
+      if (annotation.endsWith(type)) {
         return true;
       }
     }
     return false;
   }
 
+  private Set<String> annotations(VariableElement parameter) {
+    Set<String> annotations = new LinkedHashSet<>();
+    annotations.addAll(annotationsFrom(parameter));
+    annotations.addAll(annotationsFrom(parameter.asType()));
+    return annotations;
+  }
+
+  private Set<String> annotationsFrom(AnnotatedConstruct annotated) {
+    Set<String> annotations = new LinkedHashSet<>();
+    for (AnnotationMirror annotation : annotated.getAnnotationMirrors()) {
+      TypeMirror typeMirror = annotation.getAnnotationType();
+      annotations.add(typeMirror.toString());
+    }
+    return annotations;
+  }
+
+
   public Method getObjectValue() throws NoSuchMethodException {
     return getKind().valueObject(this);
   }
diff --git a/modules/jooby-apt/src/main/java/io/jooby/internal/apt/TypeDefinition.java b/modules/jooby-apt/src/main/java/io/jooby/internal/apt/TypeDefinition.java
@@ -40,6 +40,25 @@ public TypeDefinition(Types types, TypeMirror type) {
     this.type = type;
   }
 
+  /**
+   * Check for declared type and get the underlying type. This is required for annotated type.
+   * Example:
+   *
+   * <pre>{@code
+   * @Nullable @QueryParam String name
+   * }</pre>
+   *
+   * @param type
+   * @return
+   */
+  private TypeMirror unwrapType(TypeMirror type) {
+    if (type instanceof DeclaredType) {
+      return ((DeclaredType) type).asElement().asType();
+    } else {
+      return type;
+    }
+  }
+
   public String getName() {
     return getRawType().toString();
   }
@@ -49,15 +68,15 @@ public TypeMirror getType() {
   }
 
   public boolean isPrimitive() {
-    return getType().getKind().isPrimitive();
+    return unwrapType(getType()).getKind().isPrimitive();
   }
 
   public boolean isVoid() {
-    return type.getKind() == TypeKind.VOID;
+    return unwrapType(getType()).getKind() == TypeKind.VOID;
   }
 
   public TypeMirror getRawType() {
-    return typeUtils.erasure(type);
+    return typeUtils.erasure(unwrapType(getType()));
   }
 
   public boolean is(Class type, Class... arguments) {
@@ -84,10 +103,12 @@ private boolean is(String type, String... arguments) {
   }
 
   private boolean equalType(TypeMirror type, String typeName) {
-    if (!typeUtils.erasure(type).toString().equals(typeName)) {
+    TypeMirror realType = unwrapType(type);
+    TypeMirror erasure = typeUtils.erasure(realType);
+    if (!erasure.toString().equals(typeName)) {
       // check for enum subclasses:
       if (Enum.class.getName().equals(typeName)) {
-        return typeUtils.asElement(type).getKind() == ElementKind.ENUM;
+        return typeUtils.asElement(realType).getKind() == ElementKind.ENUM;
       } else {
         return false;
       }
diff --git a/modules/jooby-apt/src/main/java/io/jooby/internal/apt/asm/NameGenerator.java b/modules/jooby-apt/src/main/java/io/jooby/internal/apt/asm/NameGenerator.java
@@ -1,3 +1,8 @@
+/**
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
 package io.jooby.internal.apt.asm;
 
 import java.util.HashMap;
diff --git a/modules/jooby-apt/src/test/java/tests/i2408/C2408.java b/modules/jooby-apt/src/test/java/tests/i2408/C2408.java
@@ -0,0 +1,22 @@
+package tests.i2408;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import io.jooby.annotations.GET;
+import io.jooby.annotations.QueryParam;
+
+public class C2408 {
+  @GET("/2408/nonnull")
+  public String nonnull(@NonNull @QueryParam String name) {
+    return name;
+  }
+
+  @GET("/2408/nullable")
+  public String nullable(@Nullable @QueryParam String name, @QueryParam String blah) {
+    if (name == null) {
+      return "nothing";
+    }
+    return name;
+  }
+}
diff --git a/modules/jooby-apt/src/test/java/tests/i2408/Issue2408.java b/modules/jooby-apt/src/test/java/tests/i2408/Issue2408.java
@@ -0,0 +1,29 @@
+package tests.i2408;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.Test;
+
+import io.jooby.MockContext;
+import io.jooby.MockRouter;
+import io.jooby.apt.MvcModuleCompilerRunner;
+import io.jooby.exception.MissingValueException;
+
+public class Issue2408 {
+  @Test
+  public void shouldNotIgnoreAnnotationOnParam() throws Exception {
+    new MvcModuleCompilerRunner(new C2408())
+        .module(app -> {
+          MockRouter router = new MockRouter(app);
+          assertEquals("nothing", router.get("/2408/nullable", new MockContext()
+              .setQueryString("?blah=stuff")).value());
+
+          assertThrows(MissingValueException.class, () -> router.get("/2408/nonnull", new MockContext()).value());
+
+          assertEquals("cool", router.get("/2408/nonnull", new MockContext()
+              .setQueryString("?name=cool")).value());
+        });
+  }
+}
+
diff --git a/tests/src/test/java/io/jooby/i2408/C2408.java b/tests/src/test/java/io/jooby/i2408/C2408.java
@@ -0,0 +1,23 @@
+package io.jooby.i2408;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import io.jooby.annotations.GET;
+import io.jooby.annotations.QueryParam;
+
+public class C2408 {
+  @GET("/2408/nonnull")
+  public String nonnull(@NonNull @QueryParam String name) {
+    return name;
+  }
+
+  @GET("/2408/nullable")
+  public String nullable(@Nullable @QueryParam String name, @QueryParam String blah) {
+    if (name == null) {
+      return "nothing";
+    }
+    return name;
+  }
+
+}
diff --git a/tests/src/test/java/io/jooby/i2408/Issue2408.java b/tests/src/test/java/io/jooby/i2408/Issue2408.java
@@ -0,0 +1,29 @@
+package io.jooby.i2408;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import io.jooby.WebClient;
+import io.jooby.junit.ServerTest;
+import io.jooby.junit.ServerTestRunner;
+
+public class Issue2408 {
+  @ServerTest
+  public void shouldNotIgnoreAnnotationOnParam(ServerTestRunner runner) {
+    runner.define(app -> {
+      app.mvc(new C2408());
+      app.error((ctx, cause, code) -> {
+        ctx.send(cause.getMessage());
+      });
+    }).ready((WebClient http) -> {
+
+      http.get("/2408/nullable?blah=stuff", rsp -> {
+        assertEquals("nothing", rsp.body().string());
+      });
+
+      http.get("/2408/nonnull", rsp -> {
+        assertEquals("Missing value: 'name'", rsp.body().string());
+      });
+
+    });
+  }
+}
