Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default string values not set when annotation not present #644

Merged
merged 2 commits into from
Mar 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand All @@ -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)) {
Expand All @@ -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
Expand All @@ -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())
Expand All @@ -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"));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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 {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(", ")))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading