diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java index 567789729b..d295957059 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java @@ -507,6 +507,107 @@ public void propertyBuilder() { } } + static class NestedInner { + private final int x; + + NestedInner(int x) { + this.x = x; + } + + int x() { + return x; + } + + Builder toBuilder() { + return builder().setX(x()); + } + + static Builder builder() { + return new AutoBuilder_AutoBuilderTest_NestedInner_Builder(); + } + + @AutoBuilder + interface Builder { + Builder setX(int x); + + NestedInner build(); + } + + @Override + public boolean equals(Object o) { + if (o instanceof NestedInner) { + NestedInner that = (NestedInner) o; + return this.x == that.x; + } + return false; + } + + @Override + public int hashCode() { + return Integer.hashCode(x); + } + } + + static class NestedOuter { + private final NestedInner inner; + + NestedOuter(NestedInner inner) { + this.inner = inner; + } + + NestedInner inner() { + return inner; + } + + Builder toBuilder() { + return new AutoBuilder_AutoBuilderTest_NestedOuter_Builder(this); + } + + static Builder builder() { + return new AutoBuilder_AutoBuilderTest_NestedOuter_Builder(); + } + + @AutoBuilder + interface Builder { + NestedInner.Builder innerBuilder(); + + NestedOuter build(); + } + + @Override + public boolean equals(Object o) { + if (o instanceof NestedOuter) { + NestedOuter that = (NestedOuter) o; + return Objects.equals(this.inner, that.inner); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hashCode(inner); + } + } + + @Test + public void propertyBuilderWithToBuilder() { + NestedOuter.Builder builder1 = NestedOuter.builder(); + builder1.innerBuilder().setX(23); + NestedOuter outer1 = builder1.build(); + assertThat(outer1.inner().x()).isEqualTo(23); + + NestedOuter.Builder builder2 = outer1.toBuilder(); + builder2.innerBuilder().setX(42); + NestedOuter outer2 = builder2.build(); + assertThat(outer2.inner().x()).isEqualTo(42); + + NestedOuter outer3 = outer1.toBuilder().build(); + assertThat(outer3).isEqualTo(outer1); + + NestedOuter.Builder uninitializedBuilder = NestedOuter.builder(); + assertThrows(IllegalStateException.class, uninitializedBuilder::build); + } + static String concatList(ImmutableList list) { return list.stream().map(String::valueOf).collect(joining()); } diff --git a/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java index 28a69418c2..37ba41cbaa 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java @@ -186,6 +186,9 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin ImmutableMap propertyInitializers = propertyInitializers(autoBuilderType, executable); Nullables nullables = Nullables.fromMethods(processingEnv, methods); + ImmutableMap propertyToGetter = + propertyToGetter(executable, autoBuilderType); + boolean canGenerateCopyConstructor = !propertyToGetter.isEmpty(); Optional> maybeClassifier = BuilderMethodClassifierForAutoBuilder.classify( methods, @@ -195,14 +198,13 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin builtType, autoBuilderType, propertyInitializers.keySet(), + canGenerateCopyConstructor, nullables); if (!maybeClassifier.isPresent() || errorReporter().errorCount() > 0) { // We've already output one or more error messages. return; } BuilderMethodClassifier classifier = maybeClassifier.get(); - ImmutableMap propertyToGetter = - propertyToGetter(executable, autoBuilderType); AutoBuilderTemplateVars vars = new AutoBuilderTemplateVars(); vars.props = propertySet(executable, propertyToGetter, propertyInitializers, nullables); builder.defineVars(vars, classifier); @@ -216,7 +218,7 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin forwardingClassName .map(n -> TypeSimplifier.simpleNameOf(n) + ".of") .orElseGet(executable::invoke); - vars.toBuilderConstructor = !propertyToGetter.isEmpty(); + vars.toBuilderConstructor = canGenerateCopyConstructor; vars.toBuilderMethods = ImmutableList.of(); defineSharedVarsForType(autoBuilderType, ImmutableSet.of(), nullables, vars); String text = vars.toText(); 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 d28da9bcb9..9ec56d806c 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 @@ -146,7 +146,7 @@ Set buildMethods() { } /** Classifies the given methods and sets the state of this object based on what is found. */ - boolean classifyMethods(Iterable methods, boolean autoValueHasToBuilder) { + boolean classifyMethods(Iterable methods, boolean shouldSupportCopying) { int startErrorCount = errorReporter.errorCount(); for (ExecutableElement method : methods) { classifyMethod(method); @@ -181,14 +181,15 @@ boolean classifyMethods(Iterable methods, boolean autoValueHa // method that accepts a Bar argument. boolean canMakeBarBuilder = (propertyBuilder.getBuiltToBuilder() != null || propertyBuilder.getCopyAll() != null); - boolean needToMakeBarBuilder = (autoValueHasToBuilder || hasSetter); + boolean needToMakeBarBuilder = (shouldSupportCopying || hasSetter); if (needToMakeBarBuilder && !canMakeBarBuilder) { errorReporter.reportError( propertyBuilder.getPropertyBuilderMethod(), - "[AutoValueCantMakeBuilder] Property builder method returns %1$s but there is no" - + " way to make that type from %2$s: %2$s does not have a non-static" - + " toBuilder() method that returns %1$s, and %1$s does not have a method" - + " addAll or putAll that accepts an argument of type %2$s", + "[%1$sCantMakeBuilder] Property builder method returns %2$s but there is no" + + " way to make that type from %3$s: %3$s does not have a non-static" + + " toBuilder() method that returns %2$s, and %2$s does not have a method" + + " addAll or putAll that accepts an argument of type %3$s", + autoWhat(), propertyBuilder.getBuilderTypeMirror(), propertyType); } diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifierForAutoBuilder.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifierForAutoBuilder.java index e762db6581..7c9c45da22 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifierForAutoBuilder.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifierForAutoBuilder.java @@ -75,6 +75,8 @@ private BuilderMethodClassifierForAutoBuilder( * @param builderType the builder class or interface within {@code ofClass}. * @param propertiesWithDefaults properties that have a default value, so it is not an error for * them not to have a setter. + * @param canGenerateCopyConstructor true if the target class has getters matching all properties + * so a copy constructor will be generated. * @return an {@code Optional} that contains the results of the classification if it was * successful or nothing if it was not. */ @@ -86,6 +88,7 @@ static Optional> classify( TypeMirror builtType, TypeElement builderType, ImmutableSet propertiesWithDefaults, + boolean canGenerateCopyConstructor, Nullables nullables) { ImmutableBiMap paramToPropertyName = executable.parameters().stream() @@ -103,7 +106,7 @@ static Optional> classify( rewrittenPropertyTypes, propertiesWithDefaults, nullables); - if (classifier.classifyMethods(methods, false)) { + if (classifier.classifyMethods(methods, canGenerateCopyConstructor)) { return Optional.of(classifier); } else { return Optional.empty(); diff --git a/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java index 8ebf46f4c6..d914f553d9 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java @@ -1275,6 +1275,74 @@ public void propertyBuilderNullableLocalVariable() { .contains("Inner.Builder inner$builder = Inner.builder();"); } + @Test + public void propertyBuilderWithoutToBuilder() { + JavaFileObject inner = + JavaFileObjects.forSourceLines( + "foo.bar.Inner", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Inner {", + " private final int x;", + "", + " public Inner(int x) {", + " this.x = x;", + " }", + "", + " public int x() {", + " return x;", + " }", + "", + " public static Builder builder() {", + " return new AutoBuilder_Inner_Builder();", + " }", + "", + " @AutoBuilder", + " public interface Builder {", + " Builder x(int x);", + " Inner build();", + " }", + "}"); + JavaFileObject outer = + JavaFileObjects.forSourceLines( + "foo.bar.Outer", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Outer {", + " private final Inner inner;", + "", + " public Outer(Inner inner) {", + " this.inner = inner;", + " }", + "", + " public Inner inner() {", + " return inner;", + " }", + "", + " @AutoBuilder", + " public interface Builder {", + " Inner.Builder innerBuilder();", + " Outer build();", + " }", + "}"); + Compilation compilation = + javac().withProcessors(new AutoBuilderProcessor()).compile(inner, outer); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderCantMakeBuilder] Property builder method returns foo.bar.Inner.Builder but" + + " there is no way to make that type from foo.bar.Inner: foo.bar.Inner does not" + + " have a non-static toBuilder() method that returns foo.bar.Inner.Builder, and" + + " foo.bar.Inner.Builder does not have a method addAll or putAll that accepts an" + + " argument of type foo.bar.Inner") + .inFile(outer) + .onLineContaining("Inner.Builder innerBuilder();"); + } + private static String sorted(String... imports) { return stream(imports).sorted().collect(joining("\n")); }