diff options
Diffstat (limited to 'value')
3 files changed, 349 insertions, 11 deletions
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 696c9c5d..db6fc783 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 @@ -110,8 +110,8 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { AnnotationMirror autoBuilderAnnotation = getAnnotationMirror(autoBuilderType, AUTO_BUILDER_NAME).get(); TypeElement ofClass = getOfClass(autoBuilderType, autoBuilderAnnotation); + checkModifiersIfNested(ofClass, autoBuilderType, "AutoBuilder ofClass"); String callMethod = findCallMethodValue(autoBuilderAnnotation); - checkModifiersIfNested(ofClass); // TODO: error message is wrong ImmutableSet<ExecutableElement> methods = abstractMethodsIn( getLocalAndInheritedMethods(autoBuilderType, typeUtils(), elementUtils())); @@ -160,7 +160,8 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { ImmutableSet<ExecutableElement> methods) { List<ExecutableElement> executables = findRelevantExecutables(ofClass, callMethod, autoBuilderType); - String description = callMethod.isEmpty() ? "constructor" : "static method named " + callMethod; + String description = + callMethod.isEmpty() ? "constructor" : "static method named \"" + callMethod + "\""; switch (executables.size()) { case 0: throw errorReporter() diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java index a12792fa..3aa95b46 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java @@ -643,27 +643,33 @@ abstract class AutoValueishProcessor extends AbstractProcessor { } /** - * Checks that, if the given {@code @AutoValue} or {@code @AutoOneOf} class is nested, it is - * static and not private. This check is not necessary for correctness, since the generated code - * would not compile if the check fails, but it produces better error messages for the user. + * Checks that, if the given {@code @AutoValue}, {@code @AutoOneOf}, or {@code @AutoBuilder} class + * is nested, it is static and not private. This check is not necessary for correctness, since the + * generated code would not compile if the check fails, but it produces better error messages for + * the user. */ final void checkModifiersIfNested(TypeElement type) { + checkModifiersIfNested(type, type, simpleAnnotationName); + } + + final void checkModifiersIfNested(TypeElement type, TypeElement reportedType, String what) { ElementKind enclosingKind = type.getEnclosingElement().getKind(); if (enclosingKind.isClass() || enclosingKind.isInterface()) { if (type.getModifiers().contains(Modifier.PRIVATE)) { errorReporter.abortWithError( - type, "[AutoValuePrivate] @%s class must not be private", simpleAnnotationName); + reportedType, "[%sPrivate] @%s class must not be private", simpleAnnotationName, what); } else if (Visibility.effectiveVisibilityOfElement(type).equals(Visibility.PRIVATE)) { // The previous case, where the class itself is private, is much commoner so it deserves // its own error message, even though it would be caught by the test here too. errorReporter.abortWithError( - type, - "[AutoValueInPrivate] @%s class must not be nested in a private class", - simpleAnnotationName); + reportedType, + "[%sInPrivate] @%s class must not be nested in a private class", + simpleAnnotationName, + what); } if (!type.getModifiers().contains(Modifier.STATIC)) { errorReporter.abortWithError( - type, "[AutoValueInner] Nested @%s class must be static", simpleAnnotationName); + reportedType, "[%sInner] Nested @%s class must be static", simpleAnnotationName, what); } } // In principle type.getEnclosingElement() could be an ExecutableElement (for a class 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 296651c8..831fbb25 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 @@ -154,5 +154,336 @@ public final class AutoBuilderCompilationTest { .hasSourceEquivalentTo(EXPECTED_SIMPLE_OUTPUT); } - // TODO(b/183005059): error tests + @Test + public void autoBuilderOnEnum() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "@AutoBuilder", + "public enum Baz {", + " ZIG, ZAG, DUSTIN,", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderWrongType] @AutoBuilder only applies to classes and interfaces") + .inFile(javaFileObject) + .onLineContaining("enum Baz"); + } + + @Test + public void autoBuilderPrivate() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " @AutoBuilder", + " private interface Builder {", + " Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining("[AutoBuilderPrivate] @AutoBuilder class must not be private") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void autoBuilderNestedInPrivate() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " private static class Private {", + " @AutoBuilder", + " public interface Builder {", + " Baz build();", + " }", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderInPrivate] @AutoBuilder class must not be nested in a private class") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void autoBuilderInner() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " @AutoBuilder", + " abstract class Builder {", + " abstract Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining("[AutoBuilderInner] Nested @AutoBuilder class must be static") + .inFile(javaFileObject) + .onLineContaining("class Builder"); + } + + @Test + public void innerConstructor() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " class Inner {}", + "", + " @AutoBuilder(ofClass = Inner.class)", + " interface Builder {", + " abstract Inner build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining("[AutoBuilderInner] Nested @AutoBuilder ofClass class must be static") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void noVisibleConstructor() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " @AutoBuilder(ofClass = System.class)", + " interface Builder {", + " abstract System build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining("[AutoBuilderNoVisible] No visible constructor for java.lang.System") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void noVisibleMethod() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " private static Baz of() {", + " return new Baz();", + " }", + "", + " @AutoBuilder(callMethod = \"of\")", + " interface Builder {", + " abstract Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderNoVisible] No visible static method named \"of\" for foo.bar.Baz") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void methodNotStatic() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " Baz of() {", + " return this;", + " }", + "", + " @AutoBuilder(callMethod = \"of\")", + " interface Builder {", + " abstract Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderNoVisible] No visible static method named \"of\" for foo.bar.Baz") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void noMatchingConstructor() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " public Baz(int notMe) {}", + "", + " public Baz(String notMeEither) {}", + "", + " @AutoBuilder", + " interface Builder {", + " Builder setBuh(String x);", + " abstract Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderNoMatch] Property names do not correspond to the parameter names of any" + + " constructor:\n" + + " Baz(int notMe)\n" + + " Baz(java.lang.String notMeEither)") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void twoMatchingConstructors() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public class Baz {", + " public Baz() {}", + "", + " public Baz(int buh) {}", + "", + " public Baz(String buh) {}", + "", + " @AutoBuilder", + " interface Builder {", + " Builder setBuh(String x);", + " abstract Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderAmbiguous] Property names correspond to more than one constructor:\n" + + " Baz(int buh)\n" + + " Baz(java.lang.String buh)") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } + + @Test + public void constructInterface() { + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoBuilder;", + "", + "public interface Baz {", + " @AutoBuilder", + " interface Builder {", + " abstract Baz build();", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoBuilderProcessor()) + .withOptions("-Acom.google.auto.value.AutoBuilderIsUnstable") + .compile(javaFileObject); + assertThat(compilation).failed(); + assertThat(compilation) + .hadErrorContaining( + "[AutoBuilderEnclosing] @AutoBuilder must specify ofClass=Something.class or it must" + + " be nested inside the class to be built; actually nested inside interface" + + " foo.bar.Baz") + .inFile(javaFileObject) + .onLineContaining("interface Builder"); + } } |