Merge pull request #644 from F43nd1r/issue_643

Fix default string values not set when annotation not present
ACRA · Mar 21, 2018 · 80c9221 · 80c9221
2 parents 22ea442 + affaa20
commit 80c9221
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 73 deletions.
diff --git a/annotationprocessor/src/main/java/org/acra/processor/creator/BuildMethodCreator.java b/annotationprocessor/src/main/java/org/acra/processor/creator/BuildMethodCreator.java
@@ -17,7 +17,6 @@
 package org.acra.processor.creator;
 
 import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
 
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.CodeBlock;
@@ -35,7 +34,6 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.ExecutableElement;
 
 /**
@@ -45,7 +43,7 @@
 
 public class BuildMethodCreator {
     private final MethodSpec.Builder methodBuilder;
-    private final Map<String, AnnotationValue> anyNonDefault;
+    private final Map<String, CodeBlock> anyNonDefault;
     private final ClassName config;
     private final List<CodeBlock> statements;
 
@@ -75,7 +73,7 @@ public void addInstantiatable(@NonNull String name) {
         methodBuilder.addStatement("$T.check($L)", ClassValidator.class, name);
     }
 
-    public void addAnyNonDefault(@NonNull String name, @Nullable AnnotationValue defaultValue) {
+    public void addAnyNonDefault(@NonNull String name, @NonNull CodeBlock defaultValue) {
         anyNonDefault.put(name, defaultValue);
     }
 
@@ -110,7 +108,8 @@ public void addDelegateCall(@NonNull String methodName) {
     @NonNull
     MethodSpec build() {
         if (anyNonDefault.size() > 0) {
-            methodBuilder.beginControlFlow("if ($L)", anyNonDefault.entrySet().stream().map(field -> field.getKey() + " == " + field.getValue()).collect(Collectors.joining(" && ")))
+            methodBuilder.beginControlFlow("if ($L)", anyNonDefault.entrySet().stream().map(field -> CodeBlock.builder().add(field.getKey()).add(" == ").add(field.getValue()).build())
+                    .reduce((c1, c2) -> CodeBlock.builder().add(c1).add(" && ").add(c2).build()).orElseGet(() -> CodeBlock.of("true")))
                     .addStatement("throw new $T(\"One of $L must not be default\")", ACRAConfigurationException.class,
                             anyNonDefault.keySet().stream().collect(Collectors.joining(", ")))
                     .endControlFlow();

diff --git a/annotationprocessor/src/main/java/org/acra/processor/creator/ClassCreator.java b/annotationprocessor/src/main/java/org/acra/processor/creator/ClassCreator.java
@@ -107,11 +107,14 @@ private void createBuilderClass(@NonNull List<Element> elements) throws IOExcept
         }
         final CodeBlock.Builder always = CodeBlock.builder();
         final CodeBlock.Builder whenAnnotationPresent = CodeBlock.builder();
+        final CodeBlock.Builder whenAnnotationMissing = CodeBlock.builder();
         ClassName builder = ClassName.get(PACKAGE, builderName);
-        elements.stream().filter(BuilderElement.class::isInstance).map(BuilderElement.class::cast).forEach(m -> m.addToBuilder(classBuilder, builder, always, whenAnnotationPresent));
+        elements.stream().filter(BuilderElement.class::isInstance).map(BuilderElement.class::cast).forEach(m -> m.addToBuilder(classBuilder, builder, always, whenAnnotationPresent, whenAnnotationMissing));
         constructor.addCode(always.build())
                 .beginControlFlow("if ($L)", Strings.FIELD_ENABLED)
                 .addCode(whenAnnotationPresent.build())
+                .nextControlFlow("else")
+                .addCode(whenAnnotationMissing.build())
                 .endControlFlow();
         classBuilder.addMethod(constructor.build());
         final BuildMethodCreator build = new BuildMethodCreator(Types.getOnlyMethod(processingEnv, ConfigurationBuilder.class.getName()), ClassName.get(PACKAGE, configName));

diff --git a/annotationprocessor/src/main/java/org/acra/processor/element/AnnotationField.java b/annotationprocessor/src/main/java/org/acra/processor/element/AnnotationField.java
@@ -20,14 +20,16 @@
 import android.support.annotation.Nullable;
 import com.squareup.javapoet.*;
 import org.acra.processor.creator.BuildMethodCreator;
-import org.acra.processor.util.InitializerVisitor;
+import org.acra.processor.util.ToCodeBlockVisitor;
+import org.acra.processor.util.IsValidResourceVisitor;
 import org.acra.processor.util.Strings;
 import org.acra.processor.util.Types;
 import org.apache.commons.text.WordUtils;
 
 import javax.lang.model.element.AnnotationValue;
 import javax.lang.model.element.Modifier;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
@@ -39,31 +41,37 @@
 abstract class AnnotationField extends AbstractElement implements TransformedField.Transformable {
     private final Collection<ClassName> markers;
     private final String javadoc;
+    private final AnnotationValue defaultValue;
 
-    AnnotationField(@NonNull String name, @NonNull TypeName type, @NonNull Collection<AnnotationSpec> annotations, @Nullable String javadoc, @NonNull Collection<ClassName> markers) {
+    AnnotationField(@NonNull String name, @NonNull TypeName type, @NonNull Collection<AnnotationSpec> annotations, @Nullable String javadoc, @NonNull Collection<ClassName> markers, AnnotationValue defaultValue) {
         super(name, type, annotations);
         this.javadoc = javadoc;
         this.markers = markers;
+        this.defaultValue = defaultValue;
     }
 
     boolean hasMarker(@NonNull ClassName marker) {
         return markers.contains(marker);
     }
 
     @Override
-    public final void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways, @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
-        addWithoutGetter(builder, builderName, constructorAlways, constructorWhenAnnotationPresent);
+    public final void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways, @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
+        addWithoutGetter(builder, builderName, constructorAlways, constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
         addGetter(builder);
     }
 
     @Override
-    public final void addWithoutGetter(@NonNull TypeSpec.Builder builder, ClassName builderName, CodeBlock.Builder constructorAlways, CodeBlock.Builder constructorWhenAnnotationPresent) {
-        TransformedField.Transformable.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent);
+    public final void addWithoutGetter(@NonNull TypeSpec.Builder builder, ClassName builderName, CodeBlock.Builder constructorAlways, CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
+        TransformedField.Transformable.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
         addSetter(builder, builderName);
-        addInitializer(constructorWhenAnnotationPresent);
+        addInitializer(constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
     }
 
-    protected abstract void addInitializer(CodeBlock.Builder constructorWhenAnnotationPresent);
+    protected abstract void addInitializer(CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing);
+
+    AnnotationValue getDefaultValue() {
+        return defaultValue;
+    }
 
     @Override
     public void configureSetter(@NonNull MethodSpec.Builder builder) {
@@ -74,30 +82,22 @@ public void configureSetter(@NonNull MethodSpec.Builder builder) {
     }
 
     static class Normal extends AnnotationField {
-        private final AnnotationValue defaultValue;
 
         Normal(String name, TypeName type, Collection<AnnotationSpec> annotations, Collection<ClassName> markers, AnnotationValue defaultValue, String javadoc) {
-            super(name, type, annotations, javadoc, markers);
-            this.defaultValue = defaultValue;
+            super(name, type, annotations, javadoc, markers, defaultValue);
         }
 
         @Override
-        public void addInitializer(@NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
+        public void addInitializer(@NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
             constructorWhenAnnotationPresent.addStatement("$1L = $2L.$1L()", getName(), Strings.VAR_ANNOTATION);
-        }
-
-        @Override
-        public void configureField(@NonNull FieldSpec.Builder builder) {
-            if (defaultValue != null) {
-                final List<Object> parameters = new ArrayList<>();
-                final String statement = defaultValue.accept(new InitializerVisitor(getType()), parameters);
-                builder.initializer(statement, parameters.toArray(new Object[parameters.size()]));
+            if (getDefaultValue() != null) {
+                constructorWhenAnnotationMissing.addStatement("$L = $L", getName(), getDefaultValue().accept(new ToCodeBlockVisitor(getType()), null));
             }
         }
 
         @Override
         public void addToBuildMethod(@NonNull BuildMethodCreator method) {
-            if (defaultValue == null) {
+            if (getDefaultValue() == null) {
                 method.addNotUnset(getName(), getType());
             }
             if (hasMarker(Types.NON_EMPTY)) {
@@ -107,33 +107,34 @@ public void addToBuildMethod(@NonNull BuildMethodCreator method) {
                 method.addInstantiatable(getName());
             }
             if (hasMarker(Types.ANY_NON_DEFAULT)) {
-                method.addAnyNonDefault(getName(), defaultValue);
+                method.addAnyNonDefault(getName(), getDefaultValue().accept(new ToCodeBlockVisitor(getType()), null));
             }
         }
 
     }
 
     static class StringResource extends AnnotationField {
-        @NonNull
         private final String originalName;
-        private final boolean allowNull;
+        private final boolean hasDefault;
 
         StringResource(String name, Collection<AnnotationSpec> annotations, Collection<ClassName> markers,
-                       boolean allowNull, String javadoc) {
-            super((name.startsWith(Strings.PREFIX_RES)) ? WordUtils.uncapitalize(name.substring(Strings.PREFIX_RES.length())) : name, Types.STRING, annotations, javadoc, markers);
+                       AnnotationValue defaultValue, String javadoc) {
+            super((name.startsWith(Strings.PREFIX_RES)) ? WordUtils.uncapitalize(name.substring(Strings.PREFIX_RES.length())) : name, Types.STRING, annotations, javadoc, markers, defaultValue);
             this.originalName = name;
-            this.allowNull = allowNull;
+            this.hasDefault = defaultValue != null && getDefaultValue().accept(new IsValidResourceVisitor(), null);
             getAnnotations().remove(Types.STRING_RES);
-            if (allowNull) {
-                getAnnotations().add(Types.NULLABLE);
-            }
+            getAnnotations().add(hasDefault ? Types.NON_NULL : Types.NULLABLE);
         }
 
         @Override
-        public void addInitializer(@NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
+        public void addInitializer(@NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
             constructorWhenAnnotationPresent.beginControlFlow("if ($L.$L() != 0)", Strings.VAR_ANNOTATION, originalName)
                     .addStatement("$L = $L.getString($L.$L())", getName(), Strings.FIELD_CONTEXT, Strings.VAR_ANNOTATION, originalName)
                     .endControlFlow();
+            if (hasDefault) {
+                constructorWhenAnnotationMissing.addStatement("$L = $L.getString($L)", getName(), Strings.FIELD_CONTEXT, getDefaultValue());
+            }
+
         }
 
         @Override
@@ -146,10 +147,10 @@ public void addSetter(@NonNull TypeSpec.Builder builder, @NonNull ClassName buil
             builder.addMethod(setter.build());
         }
 
-        private MethodSpec.Builder baseResSetter(ClassName builderName){
+        private MethodSpec.Builder baseResSetter(ClassName builderName) {
             final String parameterName = Strings.PREFIX_RES + WordUtils.capitalize(getName());
             final List<AnnotationSpec> annotations = new ArrayList<>(getAnnotations());
-            annotations.removeIf(Types.NULLABLE::equals);
+            annotations.removeIf(Arrays.asList(Types.NULLABLE, Types.NON_NULL)::contains);
             annotations.add(Types.STRING_RES);
             return MethodSpec.methodBuilder(Strings.PREFIX_SETTER + WordUtils.capitalize(parameterName))
                     .addParameter(ParameterSpec.builder(TypeName.INT, parameterName).addAnnotations(annotations).build())
@@ -168,11 +169,11 @@ public void addToBuilderInterface(@NonNull TypeSpec.Builder builder, @NonNull Cl
 
         @Override
         public void addToBuildMethod(@NonNull BuildMethodCreator method) {
-            if (!allowNull) {
+            if (getDefaultValue() == null) {
                 method.addNotUnset(getName(), getType());
             }
             if (hasMarker(Types.ANY_NON_DEFAULT)) {
-                method.addAnyNonDefault(getName(), null);
+                method.addAnyNonDefault(getName(), CodeBlock.of("null"));
             }
         }
     }

diff --git a/annotationprocessor/src/main/java/org/acra/processor/element/BuilderElement.java b/annotationprocessor/src/main/java/org/acra/processor/element/BuilderElement.java
@@ -33,7 +33,7 @@
 public interface BuilderElement extends Element {
 
     default void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways,
-                              @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
+                              @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
         final FieldSpec.Builder field = FieldSpec.builder(getType(), getName(), Modifier.PRIVATE).addAnnotations(getAnnotations());
         configureField(field);
         builder.addField(field.build());
@@ -101,8 +101,8 @@ public Context() {
 
         @Override
         public void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways,
-                                 @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
-            Final.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent);
+                                 @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
+            Final.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
             constructorAlways.addStatement("$L = $L", getName(), Strings.PARAM_0);
         }
     }
@@ -117,8 +117,8 @@ class Delegate extends AbstractElement implements Final {
 
         @Override
         public void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways,
-                                 @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
-            Final.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent);
+                                 @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
+            Final.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
             if (hasContextParameter) {
                 constructorAlways.addStatement("$L = new $T($L)", getName(), getType(), Strings.PARAM_0);
             } else {
@@ -134,8 +134,8 @@ public Enabled() {
 
         @Override
         public void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways,
-                                 @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
-            Interface.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent);
+                                 @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
+            Interface.super.addToBuilder(builder, builderName, constructorAlways, constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
             addSetter(builder, builderName);
             addGetter(builder);
             constructorAlways.addStatement("$L = $L != null", getName(), Strings.VAR_ANNOTATION);

diff --git a/annotationprocessor/src/main/java/org/acra/processor/element/DelegateMethod.java b/annotationprocessor/src/main/java/org/acra/processor/element/DelegateMethod.java
@@ -47,7 +47,7 @@ class DelegateMethod extends AbstractElement implements BuilderElement.Interface
     }
 
     @Override
-    public void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways, @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
+    public void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways, @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
         final MethodSpec.Builder method = baseMethod(builderName);
         if (getType().equals(TypeName.VOID)) {
             method.addStatement("$L.$L($L)", Strings.FIELD_DELEGATE, getName(), parameters.stream().map(p -> p.name).collect(Collectors.joining(", ")))

diff --git a/annotationprocessor/src/main/java/org/acra/processor/element/ElementFactory.java b/annotationprocessor/src/main/java/org/acra/processor/element/ElementFactory.java
@@ -69,7 +69,7 @@ public Element fromAnnotationMethod(@NonNull ExecutableElement method) {
     public Element fromStringResourceAnnotationMethod(@NonNull ExecutableElement method) {
         final Pair<List<AnnotationSpec>, Set<ClassName>> annotations = getAnnotations(method);
         return new AnnotationField.StringResource(method.getSimpleName().toString(), annotations.getLeft(), annotations.getRight(),
-                method.getDefaultValue() != null, elements.getDocComment(method));
+                method.getDefaultValue(), elements.getDocComment(method));
     }
 
     @NonNull

diff --git a/annotationprocessor/src/main/java/org/acra/processor/element/TransformedField.java b/annotationprocessor/src/main/java/org/acra/processor/element/TransformedField.java
@@ -48,8 +48,8 @@ class TransformedField implements ConfigElement, BuilderElement.Interface, Valid
 
     @Override
     public void addToBuilder(@NonNull TypeSpec.Builder builder, @NonNull ClassName builderName, @NonNull CodeBlock.Builder constructorAlways,
-                             @NonNull CodeBlock.Builder constructorWhenAnnotationPresent) {
-        transform.addWithoutGetter(builder, builderName, constructorAlways, constructorWhenAnnotationPresent);
+                             @NonNull CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing) {
+        transform.addWithoutGetter(builder, builderName, constructorAlways, constructorWhenAnnotationPresent, constructorWhenAnnotationMissing);
         addGetter(builder);
     }
 
@@ -84,6 +84,6 @@ public Collection<AnnotationSpec> getAnnotations() {
     }
 
     public interface Transformable extends ConfigElement, Interface, ValidatedElement {
-        void addWithoutGetter(TypeSpec.Builder builder, ClassName builderName, CodeBlock.Builder constructorAlways, CodeBlock.Builder constructorWhenAnnotationPresent);
+        void addWithoutGetter(TypeSpec.Builder builder, ClassName builderName, CodeBlock.Builder constructorAlways, CodeBlock.Builder constructorWhenAnnotationPresent, CodeBlock.Builder constructorWhenAnnotationMissing);
     }
 }
diff --git a/annotationprocessor/src/main/java/org/acra/processor/util/IsValidResourceVisitor.java b/annotationprocessor/src/main/java/org/acra/processor/util/IsValidResourceVisitor.java
@@ -0,0 +1,15 @@
+package org.acra.processor.util;
+
+import javax.lang.model.util.SimpleAnnotationValueVisitor8;
+
+public class IsValidResourceVisitor extends SimpleAnnotationValueVisitor8<Boolean, Void> {
+    @Override
+    protected Boolean defaultAction(Object o, Void aVoid) {
+        return false;
+    }
+
+    @Override
+    public Boolean visitInt(int i, Void aVoid) {
+        return i != 0;
+    }
+}