Skip to content

Commit

Permalink
Don't fail when we can't find an AutoValue Builder setter method (#111)
Browse files Browse the repository at this point in the history
Before, we would fail with a `RuntimeException` if we couldn't match what we thought was an AutoValue property to a corresponding setter in the builder.  But, we don't currently handle AutoValue extensions (see #110), and adding full and proper handling will be pretty non-trivial.  So, instead of failing for these cases, just tolerate them.

Add a test case using the AutoValue Parcel extension to expose the problem, along with some stub Android classes so we don't need to pull in a full Android dependence.
  • Loading branch information
msridhar committed Nov 19, 2019
1 parent 07c0899 commit 44c3b4a
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 6 deletions.
1 change: 1 addition & 0 deletions object-construction-checker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ dependencies {
implementation "com.google.auto.value:auto-value-annotations:${versions.autoValue}"
testCompile "com.google.auto.value:auto-value-annotations:${versions.autoValue}"
testCompile "com.google.auto.value:auto-value:${versions.autoValue}"
testCompile "com.ryanharter.auto.value:auto-value-parcel:0.2.7"

// For Lombok Builders
testCompile "org.projectlombok:lombok:${versions.lombok}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -680,6 +681,7 @@ private AnnotationMirror createCalledMethodsForAutoValueProperties(
String[] calledMethodNames =
propertyNames.stream()
.map(prop -> autoValuePropToBuilderSetterName(prop, avBuilderSetterNames))
.filter(Objects::nonNull)
.toArray(String[]::new);
return createCalledMethods(calledMethodNames);
}
Expand Down Expand Up @@ -849,12 +851,11 @@ private static String autoValuePropToBuilderSetterName(
}
}

// nothing worked
throw new RuntimeException(
"could not find Builder setter name for property "
+ prop
+ " all names "
+ builderSetterNames);
// Could not find a corresponding setter. This is likely because an AutoValue Extension is in
// use. See https://github.com/kelloggm/object-construction-checker/issues/110
// For now we return null, but once that bug is fixed, this should be changed to an assertion
// failure.
return null;
}

/**
Expand Down
37 changes: 37 additions & 0 deletions object-construction-checker/tests/autovalue/FooParcelable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import com.google.auto.value.AutoValue;
import android.os.Parcelable;

/**
* Test for support of AutoValue Parcel extension. This test currently passes, but only because we ignore cases
* where we cannot find a matching setter for a method we think corresponds to an AutoValue property.
* See https://github.com/kelloggm/object-construction-checker/issues/110
*/
@AutoValue
abstract class FooParcelable implements Parcelable {
abstract String name();

static Builder builder() {
return new AutoValue_FooParcelable.Builder();
}

@AutoValue.Builder
abstract static class Builder {

abstract Builder setName(String value);

abstract FooParcelable build();
}

public static void buildSomethingWrong() {
Builder b = builder();
// :: error: finalizer.invocation.invalid
b.build();
}

public static void buildSomethingRight() {
Builder b = builder();
b.setName("Frank");
b.build();
}

}
12 changes: 12 additions & 0 deletions object-construction-checker/tests/autovalue/Parcel.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package android.os;

/**
* stub to avoid bringing in Android dependence
*/
public final class Parcel {

public String readString() { return ""; }

public void writeString(String val) {}

}
17 changes: 17 additions & 0 deletions object-construction-checker/tests/autovalue/Parcelable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package android.os;

/**
* stub to avoid bringing in Android dependence
*/
public interface Parcelable {
public interface Creator<T> {

public T createFromParcel(Parcel source);

public T[] newArray(int size);
}

public int describeContents();

public void writeToParcel(Parcel dest, int flags);
}

0 comments on commit 44c3b4a

Please sign in to comment.