From 7646889ded2a74c4377ac8decece3b879209cf35 Mon Sep 17 00:00:00 2001 From: emcmanus Date: Fri, 27 Sep 2019 12:15:01 -0700 Subject: Ensure that types are substituted correctly in property builders. The regression test tickles a bug in Eclipse so this change introduces a new place to put tests like that. I don't know how to work around the Eclipse failure, which is a NullPointerException inside Types.asMemberOf. RELNOTES=Property builders now work correctly when their actual return type is different from the corresponding property type because of type variable substitution. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=271621735 --- .../google/auto/value/AutoValueNotEclipseTest.java | 65 ++++++++++++++++++++++ .../value/processor/BuilderMethodClassifier.java | 8 ++- .../value/processor/PropertyBuilderClassifier.java | 6 +- 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java (limited to 'value/src') diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java new file mode 100644 index 00000000..17cd6352 --- /dev/null +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueNotEclipseTest.java @@ -0,0 +1,65 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.auto.value; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Like {@link AutoValueTest}, but with code that doesn't build with at least some versions of + * Eclipse, and should therefore not be included in {@link CompileWithEclipseTest}. (The latter is + * not currently present in the open-source build.) + */ +@RunWith(JUnit4.class) +public class AutoValueNotEclipseTest { + interface ImmutableListOf { + ImmutableList list(); + } + + // This provoked the following with the Eclipse compiler: + // java.lang.NullPointerException + // at org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding.readableName(ParameterizedTypeBinding.java:1021) + // at org.eclipse.jdt.internal.compiler.apt.model.DeclaredTypeImpl.toString(DeclaredTypeImpl.java:118) + // at java.lang.String.valueOf(String.java:2996) + // at java.lang.StringBuilder.append(StringBuilder.java:131) + // at org.eclipse.jdt.internal.compiler.apt.model.TypesImpl.asMemberOf(TypesImpl.java:130) + // at com.google.auto.value.processor.EclipseHack.methodReturnType(EclipseHack.java:124) + // at com.google.auto.value.processor.TypeVariables.lambda$rewriteReturnTypes$1(TypeVariables.java:106) + @AutoValue + abstract static class PropertyBuilderInheritsType implements ImmutableListOf { + static Builder builder() { + return new AutoValue_AutoValueNotEclipseTest_PropertyBuilderInheritsType.Builder(); + } + + @AutoValue.Builder + abstract static class Builder { + abstract ImmutableList.Builder listBuilder(); + abstract PropertyBuilderInheritsType build(); + } + } + + @Test + public void propertyBuilderInheritsType() { + PropertyBuilderInheritsType.Builder builder = PropertyBuilderInheritsType.builder(); + builder.listBuilder().add("foo", "bar"); + PropertyBuilderInheritsType x = builder.build(); + assertThat(x.list()).containsExactly("foo", "bar").inOrder(); + } +} diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java index 94e26ef0..3b3799b4 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java @@ -274,7 +274,13 @@ class BuilderMethodClassifier { if (getterToPropertyName.containsValue(property)) { PropertyBuilderClassifier propertyBuilderClassifier = new PropertyBuilderClassifier( - errorReporter, typeUtils, elementUtils, this, getterToPropertyName, eclipseHack); + errorReporter, + typeUtils, + elementUtils, + this, + getterToPropertyName, + getterToPropertyType, + eclipseHack); Optional propertyBuilder = propertyBuilderClassifier.makePropertyBuilder(method, property); if (propertyBuilder.isPresent()) { diff --git a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java index 41080730..3b8592b8 100644 --- a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java +++ b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java @@ -19,6 +19,7 @@ import com.google.auto.common.MoreElements; import com.google.auto.common.MoreTypes; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.util.LinkedHashMap; import java.util.List; @@ -52,6 +53,7 @@ class PropertyBuilderClassifier { private final Elements elementUtils; private final BuilderMethodClassifier builderMethodClassifier; private final ImmutableBiMap getterToPropertyName; + private final ImmutableMap getterToPropertyType; private final EclipseHack eclipseHack; PropertyBuilderClassifier( @@ -60,12 +62,14 @@ class PropertyBuilderClassifier { Elements elementUtils, BuilderMethodClassifier builderMethodClassifier, ImmutableBiMap getterToPropertyName, + ImmutableMap getterToPropertyType, EclipseHack eclipseHack) { this.errorReporter = errorReporter; this.typeUtils = typeUtils; this.elementUtils = elementUtils; this.builderMethodClassifier = builderMethodClassifier; this.getterToPropertyName = getterToPropertyName; + this.getterToPropertyType = getterToPropertyType; this.eclipseHack = eclipseHack; } @@ -207,7 +211,7 @@ class PropertyBuilderClassifier { Map barBuilderNoArgMethods = noArgMethodsOf(barBuilderTypeElement); ExecutableElement barGetter = getterToPropertyName.inverse().get(property); - TypeMirror barTypeMirror = barGetter.getReturnType(); + TypeMirror barTypeMirror = getterToPropertyType.get(barGetter); if (barTypeMirror.getKind() != TypeKind.DECLARED) { errorReporter.reportError( "Method looks like a property builder, but the type of property " -- cgit v1.2.3