From 44c3b4a74df65a16dd019d05935b376e2aa49f60 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 19 Nov 2019 09:04:22 -0800 Subject: [PATCH] Don't fail when we can't find an AutoValue Builder setter method (#111) 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. --- object-construction-checker/build.gradle | 1 + ...bjectConstructionAnnotatedTypeFactory.java | 13 ++++--- .../tests/autovalue/FooParcelable.java | 37 +++++++++++++++++++ .../tests/autovalue/Parcel.java | 12 ++++++ .../tests/autovalue/Parcelable.java | 17 +++++++++ 5 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 object-construction-checker/tests/autovalue/FooParcelable.java create mode 100644 object-construction-checker/tests/autovalue/Parcel.java create mode 100644 object-construction-checker/tests/autovalue/Parcelable.java diff --git a/object-construction-checker/build.gradle b/object-construction-checker/build.gradle index 29ee18a2..cb48f41f 100644 --- a/object-construction-checker/build.gradle +++ b/object-construction-checker/build.gradle @@ -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}" diff --git a/object-construction-checker/src/main/java/org/checkerframework/checker/objectconstruction/ObjectConstructionAnnotatedTypeFactory.java b/object-construction-checker/src/main/java/org/checkerframework/checker/objectconstruction/ObjectConstructionAnnotatedTypeFactory.java index 300796dd..00873105 100644 --- a/object-construction-checker/src/main/java/org/checkerframework/checker/objectconstruction/ObjectConstructionAnnotatedTypeFactory.java +++ b/object-construction-checker/src/main/java/org/checkerframework/checker/objectconstruction/ObjectConstructionAnnotatedTypeFactory.java @@ -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; @@ -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); } @@ -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; } /** diff --git a/object-construction-checker/tests/autovalue/FooParcelable.java b/object-construction-checker/tests/autovalue/FooParcelable.java new file mode 100644 index 00000000..957e8948 --- /dev/null +++ b/object-construction-checker/tests/autovalue/FooParcelable.java @@ -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(); + } + +} diff --git a/object-construction-checker/tests/autovalue/Parcel.java b/object-construction-checker/tests/autovalue/Parcel.java new file mode 100644 index 00000000..05d686a1 --- /dev/null +++ b/object-construction-checker/tests/autovalue/Parcel.java @@ -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) {} + +} diff --git a/object-construction-checker/tests/autovalue/Parcelable.java b/object-construction-checker/tests/autovalue/Parcelable.java new file mode 100644 index 00000000..51ce9c83 --- /dev/null +++ b/object-construction-checker/tests/autovalue/Parcelable.java @@ -0,0 +1,17 @@ +package android.os; + +/** + * stub to avoid bringing in Android dependence + */ +public interface Parcelable { + public interface Creator { + + public T createFromParcel(Parcel source); + + public T[] newArray(int size); + } + + public int describeContents(); + + public void writeToParcel(Parcel dest, int flags); +} \ No newline at end of file