Chasing annotation passing and fixes for serialization #117 and #116

immutables · Jun 10, 2015 · e6f3470 · e6f3470
1 parent 3293edc
commit e6f3470
Show file tree

Hide file tree

Showing 14 changed files with 145 additions and 29 deletions.
diff --git a/generator/src/org/immutables/generator/AnnotationMirrors.java b/generator/src/org/immutables/generator/AnnotationMirrors.java
@@ -169,8 +169,8 @@ void visitValue(AnnotationValue value) {
     }
 
     @Override
-    protected Void defaultAction(Object o, Void p) {
-      builder.append(o);
+    public Void visitBoolean(boolean b, Void p) {
+      builder.append(b);
       return null;
     }
 
@@ -218,6 +218,9 @@ public Void visitChar(char c, Void p) {
 
     @Override
     public Void visitString(String s, Void p) {
+      // Known issue in javac is that unresolved class literal is being
+      // passes as "<error>" string literal here
+      // Current decision - do nothing, avoid annotation with unresolved classes
       builder.append(StringLiterals.toLiteral(s));
       return null;
     }

diff --git a/serial/src/org/immutables/serial/Serial.java b/serial/src/org/immutables/serial/Serial.java
@@ -22,6 +22,7 @@
 
 /**
  * Use nested annotations to control serialization of value objects.
+ * This serialization module currently is in experimental state.
  * <p>
  * This umbrella annotaion does nothing.
  * @see Version

diff --git a/value-annotations/src/org/immutables/value/Value.java b/value-annotations/src/org/immutables/value/Value.java
@@ -15,12 +15,14 @@
  */
 package org.immutables.value;
 
-import java.util.Collections;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Annotation;
 import java.lang.annotation.Documented;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.Collections;
 import java.util.NavigableMap;
 import java.util.NavigableSet;
 import java.util.SortedMap;
@@ -529,6 +531,23 @@
      */
     boolean allParameters() default false;
 
+    /**
+     * List type of annotations to copy over from abstract value type to immutable implementation
+     * class. Very often this functionality is not needed when annoatations are declared as
+     * {@link Inherited}, but there are cases where you need to pass specific non-inherited
+     * annotations to the implementation class. In general, copying all type-level annotations is
+     * not very safe for annotation processing and some other annotation consumers. By default, no
+     * type-level annotations are copied unless you specify non-empty annotation type list as value
+     * for {@code passAnnotations} attibute.
+     * <p>
+     * This style attribute has nothing to do with attribute-level annotations, which are copied by
+     * default excluding {@code java.lang.*} and {@code org.immutables.**} annotations.
+     * <p>
+     * This style parameter is experimental and may change in future.
+     * @return types of annotations to pass to an immutable implementation class.
+     */
+    Class<? extends Annotation>[] passAnnotations() default {};
+
     /**
      * Specify the mode in which visibility of generated value type is derived from abstract value
      * type. It is a good idea to not specify such attributea inline with immutable values, but

diff --git a/value-fixture/pom.xml b/value-fixture/pom.xml
@@ -56,6 +56,7 @@
       <artifactId>gson</artifactId>
       <version>2.3.1</version>
     </dependency>
+    <!-- 2.6.0-rc2 -->
     <dependency>
       <groupId>com.fasterxml.jackson.datatype</groupId>
       <artifactId>jackson-datatype-guava</artifactId>

diff --git a/value-fixture/src/org/immutables/fixture/annotation/AbstractValForPass.java b/value-fixture/src/org/immutables/fixture/annotation/AbstractValForPass.java
@@ -0,0 +1,23 @@
+package org.immutables.fixture.annotation;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import org.immutables.value.Value;
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface A1 {}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface A2 {}
+
+@Retention(RetentionPolicy.RUNTIME)
+@interface B1 {}
+
+// Should copy @A1 and @A2, but not other annotations
+// to the implementation class
+@A1
+@A2
+@B1
+@Value.Immutable
+@Value.Style(passAnnotations = {A1.class, A2.class})
+interface AbstractValForPass {}
diff --git a/value-fixture/test/org/immutables/fixture/annotation/PassAnnotationTest.java b/value-fixture/test/org/immutables/fixture/annotation/PassAnnotationTest.java
@@ -0,0 +1,13 @@
+package org.immutables.fixture.annotation;
+
+import org.junit.Test;
+import static org.immutables.check.Checkers.*;
+
+public class PassAnnotationTest {
+  @Test
+  public void passAnnotations() {
+    check(ImmutableValForPass.class.getAnnotation(A1.class)).notNull();
+    check(ImmutableValForPass.class.getAnnotation(A2.class)).notNull();
+    check(ImmutableValForPass.class.getAnnotation(B1.class)).isNull();
+  }
+}
diff --git a/value-processor/src/org/immutables/value/processor/Immutables.generator b/value-processor/src/org/immutables/value/processor/Immutables.generator
@@ -101,6 +101,9 @@ import [starImport];
 [if classpath.available 'javax.annotation.concurrent.Immutable']
 @javax.annotation.concurrent.Immutable
 [/if]
+[for a in type.passedAnnotations]
+[a]
+[/for]
 [type.typeImmutable.access][if not topLevel]static [/if]final [output.linesShortable]class [type.typeImmutable.simple]
     [extendsImplements type][/output.linesShortable]
 [if type.annotationType]
@@ -1341,7 +1344,7 @@ public static [type.typeValue.relative] fromAllAttributes(
   [if type.useCopyMethods]
     [for o in type.constructorOmited if o.generateDefault or o.generateAbstract]
   if ([o.name] != null) {
-    [instanceIdentifier] = [instanceIdentifier].[o.names.with]([o.name]());
+    [instanceIdentifier] = [instanceIdentifier].[o.names.with]([o.name]);
   }
     [/for]
   [/if]

diff --git a/value-processor/src/org/immutables/value/processor/meta/Annotations.java b/value-processor/src/org/immutables/value/processor/meta/Annotations.java
@@ -15,6 +15,8 @@
  */
 package org.immutables.value.processor.meta;
 
+import com.google.common.base.Optional;
+import java.util.Set;
 import javax.lang.model.element.TypeElement;
 import com.google.common.collect.Lists;
 import java.lang.annotation.ElementType;
@@ -33,38 +35,48 @@ private Annotations() {}
 
   static final String NULLABLE_SIMPLE_NAME = "Nullable";
 
-  /**
-   * The gymnastics that exists only to workaround annotation processors that do not fully print
-   * annotation values (like Eclipse compiler)
-   */
-  static List<CharSequence> getAnnotationLines(Element element, ElementType elementType) {
+  static List<CharSequence> getAnnotationLines(
+      Element element,
+      Optional<Set<String>> includeAnnotations,
+      ElementType elementType) {
     List<CharSequence> lines = Lists.newArrayList();
 
     for (AnnotationMirror annotation : element.getAnnotationMirrors()) {
       TypeElement annotationElement = (TypeElement) annotation.getAnnotationType().asElement();
 
-      String string = annotationElement.getQualifiedName().toString();
-      if (string.startsWith(PREFIX_ORG_IMMUTABLES)
-          || string.startsWith(PREFIX_JAVA_LANG)
-          || string.equals(JsonPropertyMirror.QUALIFIED_NAME)) {
-        // skip immutables and core java annotations (like Override etc)
-        // Also skip JsonProperty annotation as we will add it separately
-        continue;
+      if (annotationMatchesName(annotationElement, includeAnnotations)
+          && annotationMatchesTarget(annotationElement, elementType)) {
+        lines.add(AnnotationMirrors.toCharSequence(annotation));
       }
-      if (annotationElement.getSimpleName().contentEquals(NULLABLE_SIMPLE_NAME)) {
-        // we expect to propagate nullability separately
-        continue;
-      }
-      if (!annotationMatchesTarget(
-          annotationElement,
-          elementType)) {
-        continue;
-      }
-      lines.add(AnnotationMirrors.toCharSequence(annotation));
     }
     return lines;
   }
 
+  private static boolean annotationMatchesName(
+      TypeElement annotationElement,
+      Optional<Set<String>> includeAnnotations) {
+    String qualifiedName = annotationElement.getQualifiedName().toString();
+
+    if (includeAnnotations.isPresent()) {
+      return includeAnnotations.get().contains(qualifiedName);
+    }
+
+    if (qualifiedName.startsWith(PREFIX_ORG_IMMUTABLES)
+        || qualifiedName.startsWith(PREFIX_JAVA_LANG)
+        || qualifiedName.equals(JsonPropertyMirror.QUALIFIED_NAME)) {
+      // skip immutables and core java annotations (like Override etc)
+      // Also skip JsonProperty annotation as we will add it separately
+      return false;
+    }
+
+    if (annotationElement.getSimpleName().contentEquals(NULLABLE_SIMPLE_NAME)) {
+      // we expect to propagate nullability separately
+      return false;
+    }
+
+    return true;
+  }
+
   static boolean annotationMatchesTarget(Element annotationElement, ElementType elementType) {
     @Nullable
     Target target = annotationElement.getAnnotation(Target.class);

diff --git a/value-processor/src/org/immutables/value/processor/meta/Proto.java b/value-processor/src/org/immutables/value/processor/meta/Proto.java
@@ -906,6 +906,7 @@ public StyleInfo apply(StyleMirror input) {
           input.strictBuilder(),
           input.allParameters(),
           input.jdkOnly(),
+          ImmutableSet.copyOf(input.passAnnotationsName()),
           input.visibility());
     }
   }

diff --git a/value-processor/src/org/immutables/value/processor/meta/StyleInfo.java b/value-processor/src/org/immutables/value/processor/meta/StyleInfo.java
@@ -15,6 +15,7 @@
  */
 package org.immutables.value.processor.meta;
 
+import com.google.common.collect.ImmutableSet;
 import java.lang.annotation.Annotation;
 import org.immutables.value.Value;
 import org.immutables.value.Value.Immutable;
@@ -121,9 +122,17 @@ public Class<? extends Annotation> annotationType() {
   @Override
   public abstract boolean jdkOnly();
 
+  @Value.Parameter
+  public abstract ImmutableSet<String> passAnnotationsNames();
+
   @Value.Parameter
   @Override
   public abstract ImplementationVisibility visibility();
+
+  @Override
+  public Class<? extends Annotation>[] passAnnotations() {
+    throw new UnsupportedOperationException("Use StyleInfo.passAnnotationsNames() instead");
+  }
 
   @Value.Lazy
   public Styles getStyles() {

diff --git a/value-processor/src/org/immutables/value/processor/meta/ValueAttribute.java b/value-processor/src/org/immutables/value/processor/meta/ValueAttribute.java
@@ -15,6 +15,8 @@
  */
 package org.immutables.value.processor.meta;
 
+import com.google.common.collect.ImmutableSet;
+import java.util.Collections;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
@@ -198,11 +200,11 @@ public String getType() {
   }
 
   public List<CharSequence> getAnnotations() {
-    return Annotations.getAnnotationLines(element, ElementType.METHOD);
+    return Annotations.getAnnotationLines(element, Optional.<Set<String>>absent(), ElementType.METHOD);
   }
 
   public List<CharSequence> getParameterAnnotations() {
-    return Annotations.getAnnotationLines(element, ElementType.LOCAL_VARIABLE);
+    return Annotations.getAnnotationLines(element, Optional.<Set<String>>absent(), ElementType.PARAMETER);
   }
 
   public boolean isGsonIgnore() {

diff --git a/value-processor/src/org/immutables/value/processor/meta/ValueMirrors.java b/value-processor/src/org/immutables/value/processor/meta/ValueMirrors.java
@@ -15,6 +15,7 @@
  */
 package org.immutables.value.processor.meta;
 
+import java.lang.annotation.Annotation;
 import org.immutables.mirror.Mirror;
 
 public final class ValueMirrors {
@@ -122,6 +123,8 @@ private ValueMirrors() {}
 
     boolean jdkOnly() default false;
 
+    Class<? extends Annotation>[] passAnnotations() default {};
+
     ImplementationVisibility visibility() default ImplementationVisibility.SAME;
 
     public enum ImplementationVisibility {

diff --git a/value-processor/src/org/immutables/value/processor/meta/ValueType.java b/value-processor/src/org/immutables/value/processor/meta/ValueType.java
@@ -15,6 +15,7 @@
  */
 package org.immutables.value.processor.meta;
 
+import java.lang.annotation.ElementType;
 import com.google.common.base.CaseFormat;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
@@ -233,6 +234,12 @@ public CaseStructure getCases() {
     return caseStructure;
   }
 
+  public List<CharSequence> passedAnnotations() {
+    return Annotations.getAnnotationLines(element,
+        Optional.<Set<String>>of(constitution.protoclass().styles().style().passAnnotationsNames()),
+        ElementType.TYPE);
+  }
+
   public Iterable<ValueType> allValues() {
     List<ValueType> values = Lists.newArrayList();
     if (kind().isValue()) {

diff --git a/value/src/org/immutables/value/Value.java b/value/src/org/immutables/value/Value.java
@@ -15,12 +15,14 @@
  */
 package org.immutables.value;
 
-import java.util.Collections;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Annotation;
 import java.lang.annotation.Documented;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.Collections;
 import java.util.NavigableMap;
 import java.util.NavigableSet;
 import java.util.SortedMap;
@@ -529,6 +531,23 @@
      */
     boolean allParameters() default false;
 
+    /**
+     * List type of annotations to copy over from abstract value type to immutable implementation
+     * class. Very often this functionality is not needed when annoatations are declared as
+     * {@link Inherited}, but there are cases where you need to pass specific non-inherited
+     * annotations to the implementation class. In general, copying all type-level annotations is
+     * not very safe for annotation processing and some other annotation consumers. By default, no
+     * type-level annotations are copied unless you specify non-empty annotation type list as value
+     * for {@code passAnnotations} attibute.
+     * <p>
+     * This style attribute has nothing to do with attribute-level annotations, which are copied by
+     * default excluding {@code java.lang.*} and {@code org.immutables.**} annotations.
+     * <p>
+     * This style parameter is experimental and may change in future.
+     * @return types of annotations to pass to immutable value type
+     */
+    Class<? extends Annotation>[] passAnnotations() default {};
+
     /**
      * Specify the mode in which visibility of generated value type is derived from abstract value
      * type. It is a good idea to not specify such attributea inline with immutable values, but