diff --git a/common/README.md b/common/README.md index 42eb6ca07e..990aa31b8b 100644 --- a/common/README.md +++ b/common/README.md @@ -3,18 +3,26 @@ Auto Common Utilities ## Overview -The Auto project has a set of common utilities to help ease use of the annotation processing environment. +The Auto project has a set of common utilities to help ease use of the annotation processing +environment. ## Utility classes of note * MoreTypes - utilities and Equivalence wrappers for TypeMirror and related subtypes * MoreElements - utilities for Element and related subtypes - * SuperficialValidation - very simple scanner to ensure an Element is valid and free from distortion from upstream compilation errors + * SuperficialValidation - very simple scanner to ensure an Element is valid and free from + distortion from upstream compilation errors * Visibility - utilities for working with Elements' visibility levels (public, protected, etc.) + * BasicAnnotationProcessor/ProcessingStep - simple types that + - implement a validating annotation processor + - defer invalid elements until later + - break processor actions into multiple steps (which may each handle different annotations) ## Usage/Setup -Auto common utilities have a standard maven setup which can be used from Gradle, Ivy, Ant, or other systems which consume binary artifacts from the central maven repositories. +Auto common utilities have a standard [Maven](http://maven.apache.org) setup which can also be +used from Gradle, Ivy, Ant, or other systems which consume binary artifacts from the central Maven +binary artifact repositories. ```xml @@ -23,3 +31,49 @@ Auto common utilities have a standard maven setup which can be used from Gradle, 1.0-SNAPSHOT ``` + +## Processor Resilience + +Auto Common Utilities is used by a variety of annotation processors in Google and new versions +may have breaking changes. Users of auto-common are urged to use +[shade](https://maven.apache.org/plugins/maven-shade-plugin/) or +[jarjar](https://code.google.com/p/jarjar/) (or something similar) in packaging their processors +so that conflicting versions of this library do not adversely interact with each other. + +For example, in a Maven build you can repackage `com.google.auto.common` into +`your.processor.shaded.auto.common` like this: + +```xml + + + + + + maven-shade-plugin + + + package + + shade + + + + + + + + + + com.google.auto.common + your.processor.shaded.auto.common + + + + + + + + + +``` + diff --git a/common/src/test/java/com/google/auto/common/BasicAnnotationProcessorTest.java b/common/src/test/java/com/google/auto/common/BasicAnnotationProcessorTest.java index 0fd41c8212..c815c07a72 100644 --- a/common/src/test/java/com/google/auto/common/BasicAnnotationProcessorTest.java +++ b/common/src/test/java/com/google/auto/common/BasicAnnotationProcessorTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.SetMultimap; +import com.google.common.collect.Sets; import com.google.testing.compile.JavaFileObjects; import org.junit.Test; @@ -38,15 +39,17 @@ import javax.lang.model.SourceVersion; import javax.lang.model.element.Element; import javax.tools.JavaFileObject; -import javax.tools.StandardLocation; @RunWith(JUnit4.class) public class BasicAnnotationProcessorTest { + + private Set elementsGeneratingCode = Sets.newHashSet(); + @Retention(RetentionPolicy.SOURCE) public @interface RequiresGeneratedCode {} /** Asserts that the code generated by {@link GeneratesCode} and its processor is present. */ - public static class RequiresGeneratedCodeProcessor extends BasicAnnotationProcessor { + public class RequiresGeneratedCodeProcessor extends BasicAnnotationProcessor { boolean processed = false; @Override @@ -61,12 +64,7 @@ protected Iterable initSteps() { public void process( SetMultimap, Element> elementsByAnnotation) { processed = true; - try { - processingEnv.getFiler() - .getResource(StandardLocation.SOURCE_OUTPUT, "test", "SomeGeneratedClass"); - } catch (IOException e) { - throw new AssertionError(e); - } + assertThat(elementsGeneratingCode).isNotEmpty(); } @Override @@ -81,7 +79,8 @@ public Set> annotations() { public @interface GeneratesCode {} /** Generates a class called {@code test.SomeGeneratedClass}. */ - public static class GeneratesCodeProcessor extends BasicAnnotationProcessor { + public class GeneratesCodeProcessor extends BasicAnnotationProcessor { + @Override public SourceVersion getSupportedSourceVersion() { return SourceVersion.latestSupported(); @@ -109,7 +108,9 @@ public Set> annotations() { }); } + // TODO(gak): Use jimfs to simulate the file system. private void generateClass(Element sourceType) throws IOException { + elementsGeneratingCode.add(sourceType); JavaFileObject source = processingEnv.getFiler().createSourceFile("test.SomeGeneratedClass", sourceType); PrintWriter writer = new PrintWriter(source.openWriter()); diff --git a/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java b/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java index 250b207070..9fafaec445 100644 --- a/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java +++ b/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Multimaps; @@ -97,11 +98,15 @@ private void doProcess(RoundEnvironment roundEnv) { ImmutableListMultimap.Builder indexedMethods = ImmutableListMultimap.builder(); - ImmutableSet.Builder implementationMethodDescriptors = - ImmutableSet.builder(); + ImmutableSetMultimap.Builder + implementationMethodDescriptorsBuilder = ImmutableSetMultimap.builder(); for (Element element : roundEnv.getElementsAnnotatedWith(AutoFactory.class)) { Optional declaration = declarationFactory.createIfValid(element); if (declaration.isPresent()) { + String factoryName = declaration.get().getFactoryName( + elements.getPackageOf(element).getQualifiedName(), + getAnnotatedType(element).getSimpleName()); + TypeElement extendingType = declaration.get().extendingType(); List supertypeMethods = ElementFilter.methodsIn(elements.getAllMembers(extendingType)); @@ -109,13 +114,14 @@ private void doProcess(RoundEnvironment roundEnv) { if (supertypeMethod.getModifiers().contains(Modifier.ABSTRACT)) { ExecutableType methodType = Elements2.getExecutableElementAsMemberOf( types, supertypeMethod, extendingType); - implementationMethodDescriptors.add(new ImplementationMethodDescriptor.Builder() - .name(supertypeMethod.getSimpleName().toString()) - .returnType(getAnnotatedType(element).getQualifiedName().toString()) - .publicMethod() - .passedParameters(Parameter.forParameterList( - supertypeMethod.getParameters(), methodType.getParameterTypes())) - .build()); + implementationMethodDescriptorsBuilder.put(factoryName, + new ImplementationMethodDescriptor.Builder() + .name(supertypeMethod.getSimpleName().toString()) + .returnType(getAnnotatedType(element).getQualifiedName().toString()) + .publicMethod() + .passedParameters(Parameter.forParameterList( + supertypeMethod.getParameters(), methodType.getParameterTypes())) + .build()); } } for (TypeElement implementingType : declaration.get().implementingTypes()) { @@ -125,13 +131,14 @@ private void doProcess(RoundEnvironment roundEnv) { if (interfaceMethod.getModifiers().contains(Modifier.ABSTRACT)) { ExecutableType methodType = Elements2.getExecutableElementAsMemberOf( types, interfaceMethod, implementingType); - implementationMethodDescriptors.add(new ImplementationMethodDescriptor.Builder() - .name(interfaceMethod.getSimpleName().toString()) - .returnType(getAnnotatedType(element).getQualifiedName().toString()) - .publicMethod() - .passedParameters(Parameter.forParameterList( - interfaceMethod.getParameters(), methodType.getParameterTypes())) - .build()); + implementationMethodDescriptorsBuilder.put(factoryName, + new ImplementationMethodDescriptor.Builder() + .name(interfaceMethod.getSimpleName().toString()) + .returnType(getAnnotatedType(element).getQualifiedName().toString()) + .publicMethod() + .passedParameters(Parameter.forParameterList( + interfaceMethod.getParameters(), methodType.getParameterTypes())) + .build()); } } } @@ -147,6 +154,9 @@ private void doProcess(RoundEnvironment roundEnv) { })); } + ImmutableSetMultimap + implementationMethodDescriptors = implementationMethodDescriptorsBuilder.build(); + for (Entry> entry : indexedMethods.build().asMap().entrySet()) { ImmutableSet.Builder extending = ImmutableSet.builder(); @@ -180,8 +190,7 @@ private void doProcess(RoundEnvironment roundEnv) { implementing.build(), publicType, ImmutableSet.copyOf(entry.getValue()), - // TODO(gak): this needs to be indexed too - implementationMethodDescriptors.build(), + implementationMethodDescriptors.get(entry.getKey()), allowSubclasses)); } catch (IOException e) { messager.printMessage(Kind.ERROR, "failed"); diff --git a/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java b/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java index 8c88c1bcbd..13e27a7c88 100644 --- a/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java +++ b/factory/src/test/java/com/google/auto/factory/processor/AutoFactoryProcessorTest.java @@ -15,7 +15,7 @@ */ package com.google.auto.factory.processor; -import static com.google.common.truth.Truth.assert_; +import static com.google.common.truth.Truth.assertAbout; import static com.google.testing.compile.JavaSourceSubjectFactory.javaSource; import static com.google.testing.compile.JavaSourcesSubjectFactory.javaSources; @@ -34,7 +34,7 @@ @RunWith(JUnit4.class) public class AutoFactoryProcessorTest { @Test public void simpleClass() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/SimpleClass.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -42,7 +42,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassNonFinal() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/SimpleClassNonFinal.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -51,7 +51,7 @@ public class AutoFactoryProcessorTest { } @Test public void publicClass() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/PublicClass.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -59,7 +59,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassCustomName() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/SimpleClassCustomName.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -67,7 +67,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassMixedDeps() { - assert_().about(javaSources()) + assertAbout(javaSources()) .that(ImmutableSet.of( JavaFileObjects.forResource("good/SimpleClassMixedDeps.java"), JavaFileObjects.forResource("support/AQualifier.java"))) @@ -78,7 +78,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassPassedDeps() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/SimpleClassPassedDeps.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -87,7 +87,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassProvidedDeps() { - assert_().about(javaSources()) + assertAbout(javaSources()) .that(ImmutableSet.of( JavaFileObjects.forResource("support/AQualifier.java"), JavaFileObjects.forResource("support/BQualifier.java"), @@ -99,7 +99,7 @@ public class AutoFactoryProcessorTest { } @Test public void constructorAnnotated() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/ConstructorAnnotated.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -108,7 +108,7 @@ public class AutoFactoryProcessorTest { } @Test public void constructorAnnotatedNonFinal() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/ConstructorAnnotatedNonFinal.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -117,7 +117,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassImplementingMarker() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/SimpleClassImplementingMarker.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -126,7 +126,7 @@ public class AutoFactoryProcessorTest { } @Test public void simpleClassImplementingSimpleInterface() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/SimpleClassImplementingSimpleInterface.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -135,7 +135,7 @@ public class AutoFactoryProcessorTest { } @Test public void mixedDepsImplementingInterfaces() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/MixedDepsImplementingInterfaces.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -145,31 +145,31 @@ public class AutoFactoryProcessorTest { @Test public void failsWithMixedFinals() { JavaFileObject file = JavaFileObjects.forResource("bad/MixedFinals.java"); - assert_().about(javaSource()) - .that(file) - .processedWith(new AutoFactoryProcessor()) - .failsToCompile() - .withErrorContaining( - "Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.") - .in(file).onLine(25).atColumn(3) - .and().withErrorContaining( - "Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.") - .in(file).onLine(26).atColumn(3); + assertAbout(javaSource()) + .that(file) + .processedWith(new AutoFactoryProcessor()) + .failsToCompile() + .withErrorContaining( + "Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.") + .in(file).onLine(25).atColumn(3) + .and().withErrorContaining( + "Cannot mix allowSubclasses=true and allowSubclasses=false in one factory.") + .in(file).onLine(26).atColumn(3); } @Test public void failsOnGenericClass() { JavaFileObject file = JavaFileObjects.forResource("bad/GenericClass.java"); - assert_().about(javaSource()) - .that(file) - .processedWith(new AutoFactoryProcessor()) - .failsToCompile() - .withErrorContaining("AutoFactory does not support generic types") - .in(file).onLine(21).atColumn(14); + assertAbout(javaSource()) + .that(file) + .processedWith(new AutoFactoryProcessor()) + .failsToCompile() + .withErrorContaining("AutoFactory does not support generic types") + .in(file).onLine(21).atColumn(14); } @Test public void providedButNoAutoFactory() { JavaFileObject file = JavaFileObjects.forResource("bad/ProvidedButNoAutoFactory.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -180,7 +180,7 @@ public class AutoFactoryProcessorTest { @Test public void providedOnMethodParameter() { JavaFileObject file = JavaFileObjects.forResource("bad/ProvidedOnMethodParameter.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -191,7 +191,7 @@ public class AutoFactoryProcessorTest { @Test public void invalidCustomName() { JavaFileObject file = JavaFileObjects.forResource("bad/InvalidCustomName.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -200,7 +200,7 @@ public class AutoFactoryProcessorTest { } @Test public void factoryExtendingAbstractClass() { - assert_().about(javaSource()) + assertAbout(javaSource()) .that(JavaFileObjects.forResource("good/FactoryExtendingAbstractClass.java")) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() @@ -211,7 +211,7 @@ public class AutoFactoryProcessorTest { @Test public void factoryExtendingAbstractClass_withConstructorParams() { JavaFileObject file = JavaFileObjects.forResource("good/FactoryExtendingAbstractClassWithConstructorParams.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -225,7 +225,7 @@ public class AutoFactoryProcessorTest { @Test public void factoryExtendingAbstractClass_multipleConstructors() { JavaFileObject file = JavaFileObjects.forResource( "good/FactoryExtendingAbstractClassWithMultipleConstructors.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError(); @@ -233,7 +233,7 @@ public class AutoFactoryProcessorTest { @Test public void factoryExtendingInterface() { JavaFileObject file = JavaFileObjects.forResource("bad/InterfaceSupertype.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -244,7 +244,7 @@ public class AutoFactoryProcessorTest { @Test public void factoryExtendingEnum() { JavaFileObject file = JavaFileObjects.forResource("bad/EnumSupertype.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -256,7 +256,7 @@ public class AutoFactoryProcessorTest { @Test public void factoryExtendingFinalClass() { JavaFileObject file = JavaFileObjects.forResource("bad/FinalSupertype.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .failsToCompile() @@ -268,11 +268,25 @@ public class AutoFactoryProcessorTest { @Test public void factoryImplementingGenericInterfaceExtension() { JavaFileObject file = JavaFileObjects.forResource("good/FactoryImplementingGenericInterfaceExtension.java"); - assert_().about(javaSource()) + assertAbout(javaSource()) .that(file) .processedWith(new AutoFactoryProcessor()) .compilesWithoutError() .and().generatesSources(JavaFileObjects.forResource( "expected/FactoryImplementingGenericInterfaceExtension.java")); } + + @Test public void multipleFactoriesImpementingInterface() { + JavaFileObject file = + JavaFileObjects.forResource("good/MultipleFactoriesImplementingInterface.java"); + assertAbout(javaSource()) + .that(file) + .processedWith(new AutoFactoryProcessor()) + .compilesWithoutError() + .and().generatesSources( + JavaFileObjects.forResource( + "expected/MultipleFactoriesImplementingInterfaceAFactory.java"), + JavaFileObjects.forResource( + "expected/MultipleFactoriesImplementingInterfaceBFactory.java")); + } } diff --git a/factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceAFactory.java b/factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceAFactory.java new file mode 100644 index 0000000000..03274a74bc --- /dev/null +++ b/factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceAFactory.java @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2015 Google, Inc. + * + * 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 tests; + +import javax.annotation.Generated; +import javax.inject.Inject; +import tests.MultipleFactoriesImplementingInterface.Base.Factory; + +@Generated("com.google.auto.factory.processor.AutoFactoryProcessor") +final class MultipleFactoriesImplementingInterfaceAFactory implements Factory { + @Inject + MultipleFactoriesImplementingInterfaceAFactory() {} + + MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceA create() { + return new MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceA(); + } + + @Override + public MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceA + abstractNonDefaultCreate() { + return create(); + } +} diff --git a/factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceBFactory.java b/factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceBFactory.java new file mode 100644 index 0000000000..a3735c5bbc --- /dev/null +++ b/factory/src/test/resources/expected/MultipleFactoriesImplementingInterfaceBFactory.java @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2015 Google, Inc. + * + * 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 tests; + +import javax.annotation.Generated; +import javax.inject.Inject; +import tests.MultipleFactoriesImplementingInterface.Base.Factory; + +@Generated("com.google.auto.factory.processor.AutoFactoryProcessor") +final class MultipleFactoriesImplementingInterfaceBFactory implements Factory { + @Inject + MultipleFactoriesImplementingInterfaceBFactory() {} + + MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceB create() { + return new MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceB(); + } + + @Override + public MultipleFactoriesImplementingInterface.MultipleFactoriesImplementingInterfaceB + abstractNonDefaultCreate() { + return create(); + } +} diff --git a/factory/src/test/resources/good/MultipleFactoriesImplementingInterface.java b/factory/src/test/resources/good/MultipleFactoriesImplementingInterface.java new file mode 100644 index 0000000000..b8952bf9ff --- /dev/null +++ b/factory/src/test/resources/good/MultipleFactoriesImplementingInterface.java @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2015 Google, Inc. + * + * 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 tests; + +import com.google.auto.factory.AutoFactory; + +class MultipleFactoriesImplementingInterface { + static interface Base { + static interface Factory { + public abstract Base abstractNonDefaultCreate(); + } + } + + @AutoFactory(implementing = Base.Factory.class) + static class MultipleFactoriesImplementingInterfaceA implements Base { } + + @AutoFactory(implementing = Base.Factory.class) + static class MultipleFactoriesImplementingInterfaceB implements Base {} +} diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java index e9e65813d1..8c1efe7210 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java @@ -36,6 +36,8 @@ import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -1476,6 +1478,44 @@ public void testBuilderWithExoticPropertyBuilders() { assertEquals(ImmutableTable.of(), empty.table()); } + @AutoValue + public abstract static class BuilderWithCopyingSetters { + public abstract ImmutableSet things(); + public abstract ImmutableList strings(); + public abstract ImmutableMap map(); + + public static Builder builder(T value) { + return new AutoValue_AutoValueTest_BuilderWithCopyingSetters.Builder() + .setStrings(ImmutableSet.of("foo", "bar")) + .setMap(Collections.singletonMap("foo", value)); + } + + @AutoValue.Builder + public interface Builder { + Builder setThings(ImmutableSet things); + Builder setThings(Iterable things); + Builder setThings(T... things); + Builder setStrings(Collection strings); + Builder setMap(Map map); + BuilderWithCopyingSetters build(); + } + } + + public void testBuilderWithCopyingSetters() { + BuilderWithCopyingSetters.Builder builder = BuilderWithCopyingSetters.builder(23); + + BuilderWithCopyingSetters a = builder.setThings(ImmutableSet.of(1, 2)).build(); + assertEquals(ImmutableSet.of(1, 2), a.things()); + assertEquals(ImmutableList.of("foo", "bar"), a.strings()); + assertEquals(ImmutableMap.of("foo", 23), a.map()); + + BuilderWithCopyingSetters b = builder.setThings(Arrays.asList(1, 2)).build(); + assertEquals(a, b); + + BuilderWithCopyingSetters c = builder.setThings(1, 2).build(); + assertEquals(a, c); + } + @Retention(RetentionPolicy.RUNTIME) @interface GwtCompatible { boolean funky() default false; diff --git a/value/src/main/java/com/google/auto/value/AutoValue.java b/value/src/main/java/com/google/auto/value/AutoValue.java index 5948c3bdce..2fb2bb8306 100644 --- a/value/src/main/java/com/google/auto/value/AutoValue.java +++ b/value/src/main/java/com/google/auto/value/AutoValue.java @@ -67,9 +67,6 @@ * } * } * - *

This API is provisional and subject to change.

- * - * * @author Éamonn McManus */ @Retention(RetentionPolicy.SOURCE) diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java index e8eaa3111f..0fa142022d 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java @@ -419,6 +419,7 @@ private void defineVarsForType(TypeElement type, AutoValueTemplateVars vars) { ImmutableSet toBuilderMethods; if (builder.isPresent()) { toBuilderMethods = builder.get().toBuilderMethods(typeUtils, methodsToImplement); + types.addAll(builder.get().referencedTypes()); } else { toBuilderMethods = ImmutableSet.of(); } diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java b/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java index 1f9b280e55..28be3130c8 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java @@ -17,14 +17,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import org.apache.velocity.runtime.parser.node.SimpleNode; -import java.util.Collections; -import java.util.Set; - import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.util.Types; @@ -134,9 +132,11 @@ class AutoValueTemplateVars extends TemplateVars { String buildMethodName = ""; /** - * A map from property names (like foo) to the corresponding setter method names (foo or setFoo). + * A multimap from property names (like foo) to the corresponding setters. The same property may + * be set by more than one setter. For example, an ImmutableList might be set by + * {@code setFoo(ImmutableList)} and {@code setFoo(String[])}. */ - ImmutableMap builderSetterNames = ImmutableMap.of(); + ImmutableMultimap builderSetters = ImmutableMultimap.of(); /** * A map from property names to information about the associated property builder. A property 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 d52d2264d7..0e674489c2 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 @@ -20,9 +20,14 @@ import com.google.common.base.Equivalence; import com.google.common.base.Optional; import com.google.common.collect.ImmutableBiMap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import java.beans.Introspector; @@ -31,9 +36,12 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; +import javax.lang.model.util.ElementFilter; import javax.lang.model.util.Types; /** @@ -53,10 +61,10 @@ class BuilderMethodClassifier { private final Set buildMethods = Sets.newLinkedHashSet(); private final Set propertiesWithBuilderGetters = Sets.newLinkedHashSet(); - private final Map propertyNameToPrefixedSetter = - Maps.newLinkedHashMap(); - private final Map propertyNameToUnprefixedSetter = - Maps.newLinkedHashMap(); + private final Multimap propertyNameToPrefixedSetters = + LinkedListMultimap.create(); + private final Multimap propertyNameToUnprefixedSetters = + LinkedListMultimap.create(); private final Map propertyNameToPropertyBuilder = Maps.newLinkedHashMap(); private boolean settersPrefixed; @@ -81,7 +89,7 @@ private BuilderMethodClassifier( } /** - * Classify the given methods from a builder type and its ancestors. + * Classifies the given methods from a builder type and its ancestors. * * @param methods the methods in {@code builderType} and its ancestors. * @param errorReporter where to report errors. @@ -109,15 +117,15 @@ static Optional classify( } /** - * Returns a map from the name of a property to the method that sets it. If the property is + * Returns a multimap from the name of a property to the methods that set it. If the property is * defined by an abstract method in the {@code @AutoValue} class called {@code foo()} or * {@code getFoo()} then the name of the property is {@code foo} and there will be an entry in * the map where the key is {@code "foo"} and the value is a method in the builder called * {@code foo} or {@code setFoo}. */ - Map propertyNameToSetter() { - return ImmutableMap.copyOf( - settersPrefixed ? propertyNameToPrefixedSetter : propertyNameToUnprefixedSetter); + ImmutableMultimap propertyNameToSetters() { + return ImmutableMultimap.copyOf( + settersPrefixed ? propertyNameToPrefixedSetters : propertyNameToUnprefixedSetters); } Map propertyNameToPropertyBuilder() { @@ -154,17 +162,16 @@ private boolean classifyMethods(Iterable methods) { if (!ok) { return false; } - Map propertyNameToSetter; - if (propertyNameToPrefixedSetter.isEmpty()) { - propertyNameToSetter = propertyNameToUnprefixedSetter; + Multimap propertyNameToSetter; + if (propertyNameToPrefixedSetters.isEmpty()) { + propertyNameToSetter = propertyNameToUnprefixedSetters; this.settersPrefixed = false; - } else if (propertyNameToUnprefixedSetter.isEmpty()) { - propertyNameToSetter = propertyNameToPrefixedSetter; + } else if (propertyNameToUnprefixedSetters.isEmpty()) { + propertyNameToSetter = propertyNameToPrefixedSetters; this.settersPrefixed = true; } else { - errorReporter.reportError( - "If any setter methods use the setFoo convention then all must", - propertyNameToUnprefixedSetter.values().iterator().next()); + errorReporter.reportError("If any setter methods use the setFoo convention then all must", + propertyNameToUnprefixedSetters.values().iterator().next()); return false; } for (Map.Entry getterEntry : getterToPropertyName.entrySet()) { @@ -191,7 +198,7 @@ private boolean classifyMethods(Iterable methods) { } /** - * Classify a method and update the state of this object based on what is found. + * Classifies a method and update the state of this object based on what is found. * * @return true if the method was successfully classified, false if an error has been reported. */ @@ -208,7 +215,7 @@ private boolean classifyMethod(ExecutableElement method) { } /** - * Classify a method given that it has no arguments. Currently a method with no + * Classifies a method given that it has no arguments. Currently a method with no * arguments can only be a {@code build()} method, meaning that its return type must be the * {@code @AutoValue} class. * @@ -300,7 +307,7 @@ private boolean classifyPropertyBuilder(ExecutableElement method, String propert } /** - * Classify a method given that it has one argument. Currently, a method with one argument can + * Classifies a method given that it has one argument. Currently, a method with one argument can * only be a setter, meaning that it must look like {@code foo(T)} or {@code setFoo(T)}, where * the {@code AutoValue} class has a property called {@code foo} of type {@code T}. * @@ -311,37 +318,139 @@ private boolean classifyMethodOneArg(ExecutableElement method) { Map propertyNameToGetter = getterToPropertyName.inverse(); String propertyName = null; ExecutableElement valueGetter = propertyNameToGetter.get(methodName); - Map propertyNameToSetter = null; + Multimap propertyNameToSetters = null; if (valueGetter != null) { propertyName = methodName; - propertyNameToSetter = propertyNameToUnprefixedSetter; + propertyNameToSetters = propertyNameToUnprefixedSetters; } else if (valueGetter == null && methodName.startsWith("set") && methodName.length() > 3) { propertyName = Introspector.decapitalize(methodName.substring(3)); - propertyNameToSetter = propertyNameToPrefixedSetter; + propertyNameToSetters = propertyNameToPrefixedSetters; valueGetter = propertyNameToGetter.get(propertyName); } - if (valueGetter == null || propertyNameToSetter == null) { + if (valueGetter == null || propertyNameToSetters == null) { // The second disjunct isn't needed but convinces control-flow checkers that - // propertyNameToSetter can't be null when we call put on it below. + // propertyNameToSetters can't be null when we call put on it below. errorReporter.reportError( "Method does not correspond to a property of " + autoValueClass, method); return false; } - TypeMirror parameterType = method.getParameters().get(0).asType(); - TypeMirror expectedType = valueGetter.getReturnType(); - if (!TYPE_EQUIVALENCE.equivalent(parameterType, expectedType)) { - errorReporter.reportError( - "Parameter type of setter method should be " + expectedType + " to match getter " - + autoValueClass + "." + valueGetter.getSimpleName(), method); + if (!checkSetterParameter(valueGetter, method)) { return false; } else if (!TYPE_EQUIVALENCE.equivalent(method.getReturnType(), builderType.asType())) { errorReporter.reportError( "Setter methods must return " + builderType + typeParamsString(), method); return false; } else { - propertyNameToSetter.put(propertyName, method); + propertyNameToSetters.put(propertyName, method); + return true; + } + } + + /** + * Checks that the given setter method has a parameter type that is compatible with the return + * type of the given getter. Compatible means either that it is the same, or that it is a type + * that can be copied using a method like {@code ImmutableList.copyOf}. + * + * @return true if the types correspond, false if an error has been reported. + */ + private boolean checkSetterParameter(ExecutableElement valueGetter, ExecutableElement setter) { + TypeMirror targetType = valueGetter.getReturnType(); + TypeMirror parameterType = setter.getParameters().get(0).asType(); + if (TYPE_EQUIVALENCE.equivalent(parameterType, targetType)) { return true; } + ImmutableList copyOfMethods = copyOfMethods(targetType); + if (!copyOfMethods.isEmpty()) { + return canMakeCopyUsing(copyOfMethods, valueGetter, setter); + } + String error = String.format( + "Parameter type of setter method should be %s to match getter %s.%s", + targetType, autoValueClass, valueGetter.getSimpleName()); + errorReporter.reportError(error, setter); + return false; + } + + /** + * Checks that the given setter method has a parameter type that can be copied to the return type + * of the given getter using one of the given {@code copyOf} methods. + * + * @return true if the copy can be made, false if an error has been reported. + */ + private boolean canMakeCopyUsing( + ImmutableList copyOfMethods, + ExecutableElement valueGetter, + ExecutableElement setter) { + TypeMirror targetType = valueGetter.getReturnType(); + TypeMirror parameterType = setter.getParameters().get(0).asType(); + for (ExecutableElement copyOfMethod : copyOfMethods) { + if (canMakeCopyUsing(copyOfMethod, targetType, parameterType)) { + return true; + } + } + DeclaredType targetDeclaredType = MoreTypes.asDeclared(targetType); + String targetTypeSimpleName = targetDeclaredType.asElement().getSimpleName().toString(); + String error = String.format( + "Parameter type of setter method should be %s to match getter %s.%s, or it should be a " + + "type that can be passed to %s.copyOf", + targetType, autoValueClass, valueGetter.getSimpleName(), targetTypeSimpleName); + errorReporter.reportError(error, setter); + return false; + } + + /** + * Returns true if {@code copyOfMethod} can be used to copy the {@code parameterType} + * to the {@code targetType}. + */ + private boolean canMakeCopyUsing( + ExecutableElement copyOfMethod, TypeMirror targetType, TypeMirror parameterType) { + // We have a parameter type, for example Set, and we want to know if it can be + // passed to the given copyOf method, which might for example be one of these methods from + // ImmutableSet: + // public static ImmutableSet copyOf(Collection elements) + // public static ImmutableSet copyOf(E[] elements) + // Additionally, if it can indeed be passed to the method, we want to know whether the result + // (here ImmutableSet) is compatible with the property to be set, bearing in mind + // that the T in question is the one from the @AutoValue class and not the Builder class. + // The logic to do that properly would be quite complex, and we don't get much help from the + // annotation processing API, so for now we simply check that the erased types correspond. + // This means that an incorrect type will lead to a compilation error in the generated code, + // which is less than ideal. + // TODO(b/20691134): make this work properly + TypeMirror erasedParameterType = typeUtils.erasure(parameterType); + TypeMirror erasedCopyOfParameterType = + typeUtils.erasure(Iterables.getOnlyElement(copyOfMethod.getParameters()).asType()); + // erasedParameterType is Set in the example and erasedCopyOfParameterType is Collection + if (!typeUtils.isAssignable(erasedParameterType, erasedCopyOfParameterType)) { + return false; + } + TypeMirror erasedCopyOfReturnType = typeUtils.erasure(copyOfMethod.getReturnType()); + TypeMirror erasedTargetType = typeUtils.erasure(targetType); + // erasedCopyOfReturnType and erasedTargetType are both ImmutableSet in the example. + // In fact for Guava immutable collections the check could be for equality. + return typeUtils.isAssignable(erasedCopyOfReturnType, erasedTargetType); + } + + /** + * Returns {@code copyOf} methods from the given type. These are static methods called + * {@code copyOf} with a single parameter. All of Guava's concrete immutable collection types have + * at least one such method, but we will also accept other classes with an appropriate method, + * such as {@link java.util.EnumSet}. + */ + private ImmutableList copyOfMethods(TypeMirror targetType) { + if (!targetType.getKind().equals(TypeKind.DECLARED)) { + return ImmutableList.of(); + } + TypeElement immutableTargetType = MoreElements.asType(typeUtils.asElement(targetType)); + ImmutableList.Builder copyOfMethods = ImmutableList.builder(); + for (ExecutableElement method : + ElementFilter.methodsIn(immutableTargetType.getEnclosedElements())) { + if (method.getSimpleName().contentEquals("copyOf") + && method.getParameters().size() == 1 + && method.getModifiers().contains(Modifier.STATIC)) { + copyOfMethods.add(method); + } + } + return copyOfMethods.build(); } private String prefixWithSet(String propertyName) { diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java index be6aacfc85..16d3bec69b 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java @@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; @@ -43,6 +44,7 @@ import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; import javax.lang.model.element.TypeParameterElement; +import javax.lang.model.element.VariableElement; import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; @@ -163,6 +165,18 @@ ImmutableSet toBuilderMethods( return builderMethods; } + Set referencedTypes() { + Set types = new TypeMirrorSet(); + for (ExecutableElement method : + ElementFilter.methodsIn(builderTypeElement.getEnclosedElements())) { + types.add(method.getReturnType()); + for (VariableElement parameter : method.getParameters()) { + types.add(parameter.asType()); + } + } + return types; + } + void defineVars( AutoValueTemplateVars vars, TypeSimplifier typeSimplifier, @@ -200,12 +214,15 @@ void defineVars( vars.buildMethodName = buildMethod.getSimpleName().toString(); vars.propertiesWithBuilderGetters = classifier.propertiesWithBuilderGetters(); - ImmutableMap.Builder setterNameBuilder = ImmutableMap.builder(); + ImmutableMultimap.Builder setterBuilder = ImmutableMultimap.builder(); for (Map.Entry entry : - classifier.propertyNameToSetter().entrySet()) { - setterNameBuilder.put(entry.getKey(), entry.getValue().getSimpleName().toString()); + classifier.propertyNameToSetters().entries()) { + String property = entry.getKey(); + ExecutableElement setter = entry.getValue(); + TypeMirror propertyType = getterToPropertyName.inverse().get(property).getReturnType(); + setterBuilder.put(property, new PropertySetter(setter, propertyType, typeSimplifier)); } - vars.builderSetterNames = setterNameBuilder.build(); + vars.builderSetters = setterBuilder.build(); vars.builderPropertyBuilders = makeBuilderPropertyBuilderMap(classifier, typeSimplifier, getterToPropertyName); @@ -237,6 +254,49 @@ private ImmutableMap makeBuilderPropertyBuilderMap( } } + /** + * Information about a property setter, referenced from the autovalue.vm template. A property + * called foo (defined by a method {@code T foo()} or {@code T getFoo()}) can have a setter + * method {@code foo(T)} or {@code setFoo(T)} that returns the builder type. Additionally, it + * can have a setter with a type that can be copied to {@code T} through a {@code copyOf} method; + * for example a property {@code foo} of type {@code ImmutableSet} can be set with a + * method {@code setFoo(Collection foos)}. + */ + public class PropertySetter { + private final String name; + private final String parameterTypeString; + private final String copyFormat; + + public PropertySetter( + ExecutableElement setter, TypeMirror propertyType, TypeSimplifier typeSimplifier) { + this.name = setter.getSimpleName().toString(); + TypeMirror parameterType = Iterables.getOnlyElement(setter.getParameters()).asType(); + String simplifiedParameterType = typeSimplifier.simplify(parameterType); + if (setter.isVarArgs()) { + simplifiedParameterType = simplifiedParameterType.replaceAll("\\[\\]$", "..."); + } + this.parameterTypeString = simplifiedParameterType; + Types typeUtils = processingEnv.getTypeUtils(); + TypeMirror erasedPropertyType = typeUtils.erasure(propertyType); + boolean sameType = typeUtils.isSameType(typeUtils.erasure(parameterType), erasedPropertyType); + this.copyFormat = sameType + ? "%s" + : typeSimplifier.simplifyRaw(erasedPropertyType) + ".copyOf(%s)"; + } + + public String getName() { + return name; + } + + public String getParameterType() { + return parameterTypeString; + } + + public String copy(AutoValueProcessor.Property property) { + return String.format(copyFormat, property); + } + } + /** * Information about a property builder, referenced from the autovalue.vm template. A property * called foo (defined by a method foo() or getFoo()) can have a property builder called diff --git a/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java b/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java index 2b6bb0131f..0afa393f71 100644 --- a/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java +++ b/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java @@ -16,6 +16,7 @@ package com.google.auto.value.processor; import com.google.common.base.Optional; +import com.google.common.collect.Multimap; import org.apache.velocity.runtime.parser.node.SimpleNode; @@ -90,7 +91,7 @@ void maybeWriteGwtSerializer(AutoValueTemplateVars autoVars) { vars.formalTypes = autoVars.formalTypes; vars.actualTypes = autoVars.actualTypes; vars.useBuilder = !autoVars.builderTypeName.isEmpty(); - vars.builderSetterNames = autoVars.builderSetterNames; + vars.builderSetters = autoVars.builderSetters; vars.generated = autoVars.generated; String className = (vars.pkg.isEmpty() ? "" : vars.pkg + ".") + vars.subclass + "_CustomFieldSerializer"; @@ -201,10 +202,10 @@ static class GwtTemplateVars extends TemplateVars { Boolean useBuilder; /** - * A map from property names (like foo) to the corresponding setter method names + * A multimap from property names (like foo) to the corresponding setter methods * (foo or setFoo). */ - Map builderSetterNames; + Multimap builderSetters; /** The simple name of the generated GWT serializer class. */ String serializerClass; diff --git a/value/src/main/java/com/google/auto/value/processor/autovalue.vm b/value/src/main/java/com/google/auto/value/processor/autovalue.vm index ba34854e49..e3f0f7a345 100644 --- a/value/src/main/java/com/google/auto/value/processor/autovalue.vm +++ b/value/src/main/java/com/google/auto/value/processor/autovalue.vm @@ -232,14 +232,11 @@ final class $subclass$formalTypes extends $origClass$actualTypes { #if ($builderPropertyBuilders[$p.name]) - ${p}.${builderPropertyBuilders[$p.name].copyAll}(source.${p.getter}()); + this.${p}.${builderPropertyBuilders[$p.name].copyAll}(source.${p.getter}()); #else - $builderSetterNames[$p.name](source.${p.getter}()); - ## We use the setter methods rather than assigning the fields directly so we don't have - ## to duplicate the logic for cloning arrays. Not cloning arrays would conceivably be - ## a security problem. + this.$p = source.${p.getter}(); #end @@ -251,16 +248,11 @@ final class $subclass$formalTypes extends $origClass$actualTypes { ## Setter and/or property builder - #if ($builderSetterNames[$p.name]) + #foreach ($setter in $builderSetters[$p.name]) @Override - public ${builderTypeName}${builderActualTypes} $builderSetterNames[$p.name]($p.type $p) { - #if ($builderPropertyBuilders[$p.name]) - - this.$p = ${builderPropertyBuilders[$p.name].initializer}; - this.$p.${builderPropertyBuilders[$p.name].copyAll}($p); - - #elseif ($p.kind == "ARRAY") + public ${builderTypeName}${builderActualTypes} ${setter.name}($setter.parameterType $p) { + #if ($p.kind == "ARRAY") #if ($p.nullable) this.$p = ($p == null) ? null : ${p}.clone(); @@ -272,7 +264,7 @@ final class $subclass$formalTypes extends $origClass$actualTypes { #end #else - this.$p = $p; + this.$p = ${setter.copy($p)}; #end diff --git a/value/src/main/java/com/google/auto/value/processor/gwtserializer.vm b/value/src/main/java/com/google/auto/value/processor/gwtserializer.vm index e9f0f8fa24..029f61abf7 100644 --- a/value/src/main/java/com/google/auto/value/processor/gwtserializer.vm +++ b/value/src/main/java/com/google/auto/value/processor/gwtserializer.vm @@ -37,7 +37,7 @@ public final class $serializerClass { #if ($useBuilder) return (${subclass}${actualTypes}) new ${subclass}.Builder${actualTypes}() #foreach ($p in $props) - .$builderSetterNames[$p.name]($p) + .${builderSetters[$p.name].iterator().next().name}($p) #end .build(); #else diff --git a/value/src/test/java/com/google/auto/value/processor/CompilationTest.java b/value/src/test/java/com/google/auto/value/processor/CompilationTest.java index 409558731f..d0af9b7868 100644 --- a/value/src/test/java/com/google/auto/value/processor/CompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/CompilationTest.java @@ -645,11 +645,11 @@ public void testCorrectBuilder() throws Exception { " }", "", " Builder(Baz source) {", - " anInt(source.anInt());", - " aByteArray(source.aByteArray());", - " aNullableIntArray(source.aNullableIntArray());", - " aList(source.aList());", - " anImmutableList.addAll(source.anImmutableList());", + " this.anInt = source.anInt();", + " this.aByteArray = source.aByteArray();", + " this.aNullableIntArray = source.aNullableIntArray();", + " this.aList = source.aList();", + " this.anImmutableList.addAll(source.anImmutableList());", " }", "", " @Override", @@ -908,6 +908,72 @@ public void testAutoValueBuilderWrongTypeSetter() { .in(javaFileObject).onLine(12); } + public void testAutoValueBuilderWrongTypeSetterWithCopyOf() { + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoValue;", + "import com.google.common.collect.ImmutableList;", + "", + "@AutoValue", + "public abstract class Baz {", + " abstract String blim();", + " abstract ImmutableList blam();", + "", + " @AutoValue.Builder", + " public interface Builder {", + " Builder blim(String x);", + " Builder blam(String x);", + " Baz build();", + " }", + "}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AutoValueProcessor(), new AutoValueBuilderProcessor()) + .failsToCompile() + .withErrorContaining( + "Parameter type of setter method should be " + + "com.google.common.collect.ImmutableList to match getter " + + "foo.bar.Baz.blam, or it should be a type that can be passed to " + + "ImmutableList.copyOf") + .in(javaFileObject).onLine(14); + } + + public void testAutoValueBuilderWrongTypeSetterWithCopyOfGenericallyWrong() { + // This puts the finger on our insufficient error-detection logic for the case where the + // parameter would be compatible with copyOf were it not for generics. Currently, this leads to + // a compile error in the generated code. We don't want to suppose anything about the error + // message the compiler might come up with. It might be something like this for example: + // incompatible types: inference variable E has incompatible bounds + // equality constraints: java.lang.String + // lower bounds: java.lang.Integer + JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoValue;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Collection;", + "", + "@AutoValue", + "public abstract class Baz {", + " abstract String blim();", + " abstract ImmutableList blam();", + "", + " @AutoValue.Builder", + " public interface Builder {", + " Builder blim(String x);", + " Builder blam(Collection x);", + " Baz build();", + " }", + "}"); + assertAbout(javaSource()) + .that(javaFileObject) + .processedWith(new AutoValueProcessor(), new AutoValueBuilderProcessor()) + .failsToCompile(); + } + public void testAutoValueBuilderWrongTypeSetterWithGetPrefix() { JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( "foo.bar.Baz",