diff --git a/.gitignore b/.gitignore index 3dc6b5ccd..ff10e03fd 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ build out/ docs/node_modules/ generated_src +generated_testSrc diff --git a/README.md b/README.md index 335734c8c..e4a9dce74 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `PreferStaticLoggers`: Prefer static loggers over instance loggers. - `LogsafeArgName`: Prevent certain named arguments as being logged as safe. Specify unsafe argument names using `LogsafeArgName:UnsafeArgNames` errorProne flag. - `ImplicitPublicBuilderConstructor`: Prevent builders from unintentionally leaking public constructors. +- `ImmutablesBuilderMissingInitialization`: Prevent building Immutables.org builders when not all fields have been populated. ### Programmatic Application diff --git a/baseline-error-prone/build.gradle b/baseline-error-prone/build.gradle index 510823853..dbaf3f88a 100644 --- a/baseline-error-prone/build.gradle +++ b/baseline-error-prone/build.gradle @@ -28,6 +28,7 @@ dependencies { annotationProcessor 'com.google.auto.service:auto-service' annotationProcessor 'org.immutables:value' + testAnnotationProcessor 'org.immutables:value' compileOnly 'com.google.auto.service:auto-service' } diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ImmutablesBuilderMissingInitialization.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ImmutablesBuilderMissingInitialization.java new file mode 100644 index 000000000..3a5ec9722 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ImmutablesBuilderMissingInitialization.java @@ -0,0 +1,316 @@ +/* + * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.common.base.CaseFormat; +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import com.google.common.collect.Streams; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.LinkType; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.ReturnTree; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.ClassSymbol; +import com.sun.tools.javac.code.Symbol.MethodSymbol; +import com.sun.tools.javac.code.Type; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.immutables.value.Generated; + +/** + * Checks that all required fields in an immutables.org generated builder have been populated. + * + * To make it decidable, the implementation is limited to the common case where: the builder is constructed in place + * using new ImmutableType.Builder() or new Type.Builder() (where Type.Builder extends ImmutableType.Builder), or using + * a method that does nothing other than construct and return the builder; and the builder is never stored to a + * variable, so the builder only lives within a single statement. Builders that do not meet these + * conditions are assumed to populate all fields, and are ignored. + * + * Mandatory fields are determined by inspecting the generated builder class to find fields that have INIT_BIT_ + * constants, which are used by Immutables to track whether required fields have been initialized. Methods that end with + * the name of a field are assumed to initialize that field. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "ImmutablesBuilderMissingInitialization", + linkType = LinkType.CUSTOM, + link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", + severity = BugPattern.SeverityLevel.ERROR, + summary = "All required fields of an Immutables builder must be initialized") +public final class ImmutablesBuilderMissingInitialization extends BugChecker implements MethodInvocationTreeMatcher { + + private static final String FIELD_INIT_BITS_PREFIX = "INIT_BIT_"; + + /** + * Prefixes on the interface getter methods that may be stripped by immutables. + * + * The default immutables style just uses get*, but some places use is* too and we can't load the style to check + * what to remove. It doesn't matter if we remove too much, because we only do a suffix match on the methods. + * + * This should only help when the unprefixed field name is an identifier, because in other cases immutables already + * uses the unprefixed name. + */ + private static final ImmutableSet GET_PREFIXES = ImmutableSet.of("GET_", "IS_"); + + private static final Matcher builderMethodMatcher = Matchers.instanceMethod() + .onClass(ImmutablesBuilderMissingInitialization::extendsImmutablesGeneratedClass) + .named("build") + .withParameters(); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + // Check that the method is a builder's build() method + if (!builderMethodMatcher.matches(tree, state)) { + return Description.NO_MATCH; + } + ClassSymbol builderClass = ASTHelpers.enclosingClass(ASTHelpers.getSymbol(tree)); + ClassSymbol immutableClass = ASTHelpers.enclosingClass(builderClass); + if (immutableClass == null) { + return Description.NO_MATCH; + } + Optional interfaceClass = Streams.concat( + immutableClass.getInterfaces().stream(), Stream.of(immutableClass.getSuperclass())) + .map(type -> type.tsym) + .map(filterByType(ClassSymbol.class)) + .flatMap(Streams::stream) + .filter(classSymbol -> + ASTHelpers.hasAnnotation(classSymbol, "org.immutables.value.Value.Immutable", state)) + .findAny(); + if (!interfaceClass.isPresent()) { + return Description.NO_MATCH; + } + + // Mandatory fields have a private static final constant in the generated builder named INIT_BIT_varname, where + // varname is the UPPER_UNDERSCORE version of the variable name. Find these fields to get the mandatory fields. + // nb. this isn't part of any immutables API, so could break. + Set requiredFields = Streams.stream(builderClass.members().getSymbols()) + .filter(Symbol::isStatic) + .filter(symbol -> symbol.getKind().isField()) + .filter(symbol -> symbol.getSimpleName().toString().startsWith(FIELD_INIT_BITS_PREFIX)) + .map(Symbol::toString) + .map(initBitsName -> removeFromStart(initBitsName, FIELD_INIT_BITS_PREFIX)) + .map(fieldName -> GET_PREFIXES.stream().reduce(fieldName, this::removeFromStart)) + .map(CaseFormat.UPPER_UNDERSCORE.converterTo(CaseFormat.LOWER_CAMEL)) + .collect(Collectors.toSet()); + + if (!checkAllFieldsCanBeInitialized(requiredFields, builderClass)) { + // Our expectation for the method names isn't right, so it's probably using a custom style + return Description.NO_MATCH; + } + + // Run the check + Set uninitializedFields = checkInitialization( + ASTHelpers.getReceiver(tree), + requiredFields, + state, + builderClass, + immutableClass, + interfaceClass.get()); + + if (uninitializedFields.isEmpty()) { + return Description.NO_MATCH; + } + return buildDescription(tree) + .setMessage(String.format( + "Some builder fields have not been initialized: %s", + Joiner.on(", ").join(uninitializedFields))) + .build(); + } + + /** + * Recursively check that all of the uninitializedFields are initialized in the expression, returning any that + * are not. + */ + private Set checkInitialization( + ExpressionTree tree, + Set uninitializedFields, + VisitorState state, + ClassSymbol builderClass, + ClassSymbol immutableClass, + ClassSymbol interfaceClass) { + if (uninitializedFields.isEmpty()) { + return ImmutableSet.of(); + } + if (tree instanceof MethodInvocationTree) { + MethodSymbol methodSymbol = ASTHelpers.getSymbol((MethodInvocationTree) tree); + if (!Objects.equals(methodSymbol.enclClass(), builderClass)) { + // This method belongs to a class other than the builder, so we are at the end of the chain + // If the only thing this method does is construct and return the builder, we continue and require + // everything to have been initialized at this point, but if it does anything more complex then give up + if (methodJustConstructsBuilder(methodSymbol, state, immutableClass, interfaceClass)) { + return uninitializedFields; + } else { + return ImmutableSet.of(); + } + } + if (methodSymbol.getSimpleName().contentEquals("from")) { + // `from` initializes all fields, so we don't need to continue + return ImmutableSet.of(); + } + + return checkInitialization( + ASTHelpers.getReceiver(tree), + removeFieldsPotentiallyInitializedBy( + uninitializedFields, methodSymbol.getSimpleName().toString()), + state, + builderClass, + immutableClass, + interfaceClass); + } else if (tree instanceof NewClassTree) { + NewClassTree newClassTree = (NewClassTree) tree; + if (newClassTree.getArguments().isEmpty()) { + // The constructor returned the builder (otherwise we would have bailed out in a previous iteration), so + // we should have seen all the field initializations + return uninitializedFields; + } + // If the constructor takes arguments, it's doing something funky + return ImmutableSet.of(); + } + + // If the chain started with something other than a method call or constructor to create the builder, give up + return ImmutableSet.of(); + } + + /** + * Check that every required field has a setter method, to ensure that there are no style rules that have not been + * accounted for here. + */ + private boolean checkAllFieldsCanBeInitialized(Set fields, Symbol builderClass) { + return Streams.stream(builderClass.members().getSymbols()) + .map(filterByType(MethodSymbol.class)) + .flatMap(Streams::stream) + .filter(symbol -> !symbol.isStaticOrInstanceInit() + && !symbol.isConstructor() + && !symbol.isAnonymous() + && symbol.getParameters().size() == 1) + .map(symbol -> symbol.getSimpleName().toString()) + .reduce(fields, this::removeFieldsPotentiallyInitializedBy, Sets::intersection) + .isEmpty(); + } + + /** + * Takes a set of uninitialized fields, and returns a set containing the fields that cannot have been initialized by + * the method methodName. + */ + private Set removeFieldsPotentiallyInitializedBy(Set uninitializedFields, String methodName) { + String methodNameLowerCase = methodName.toLowerCase(); + return uninitializedFields.stream() + .filter(fieldName -> !methodNameLowerCase.endsWith(fieldName.toLowerCase())) + .collect(Collectors.toSet()); + } + + /** + * Make sure a method only does one thing, which is to call a constructor with no arguments and return the result. + * + * We don't check which class's constructor because it must return something compatible with Builder for us to have + * got this far, and that's all we care about. + */ + private boolean methodJustConstructsBuilder( + MethodSymbol methodSymbol, VisitorState state, ClassSymbol immutableClass, ClassSymbol interfaceClass) { + MethodTree methodTree = ASTHelpers.findMethod(methodSymbol, state); + if (methodTree != null) { + // Check that the method just contains one statement, which is of the form `return new Something();` or + // `return ImmutableType.builder();` + if (methodTree.getBody().getStatements().size() != 1) { + return false; + } + return methodTree.getBody().getStatements().stream() + .findAny() + .flatMap(filterByType(ReturnTree.class)) + .map(ReturnTree::getExpression) + .map(expressionTree -> { + if (expressionTree instanceof NewClassTree) { + // To have got here, the return type must be compatible with ImmutableType, so we don't need + // to check the class being constructed + return ((NewClassTree) expressionTree) + .getArguments() + .isEmpty(); + } else if (expressionTree instanceof MethodInvocationTree) { + return Optional.ofNullable(ASTHelpers.getSymbol(expressionTree)) + .flatMap(filterByType(MethodSymbol.class)) + .filter(symbol -> symbol.getParameters().isEmpty()) + .filter(symbol -> symbol.getSimpleName().contentEquals("builder")) + .map(Symbol::enclClass) + .flatMap(filterByType(ClassSymbol.class)) + .filter(symbol -> symbol.equals(immutableClass)) + .isPresent(); + } else { + return false; + } + }) + .orElse(false); + } + // The method that was called is in a different compilation unit, so we can't access the source. + // If the method is .builder() and it's on a trusted class (the immutable implementation or the interface that + // it was generated from), assume that it just constructs the builder and nothing else. + return ((Objects.equals(methodSymbol.enclClass(), immutableClass) + || Objects.equals(methodSymbol.enclClass(), interfaceClass)) + && methodSymbol.getSimpleName().contentEquals("builder") + && methodSymbol.getParameters().isEmpty()); + } + + /** + * If input starts with toRemove, returns the rest of input with toRemove removed, otherwise just returns input. + */ + private String removeFromStart(String input, String toRemove) { + if (input.startsWith(toRemove)) { + return input.substring(toRemove.length()); + } + return input; + } + + /** + * Returns a function for use in Optional.flatMap that filters by type, and casts to that type. + */ + private Function> filterByType(Class clazz) { + return value -> { + if (clazz.isInstance(value)) { + return Optional.of(clazz.cast(value)); + } + return Optional.empty(); + }; + } + + /** + * Returns whether the provided type is a class that was generated by Immutables, or extends a class that was. + */ + private static boolean extendsImmutablesGeneratedClass(Type type, VisitorState state) { + if (type.tsym instanceof ClassSymbol) { + return Optional.ofNullable(type.tsym.getAnnotation(Generated.class)) + .map(generated -> generated.generator().equals("Immutables")) + .orElseGet(() -> extendsImmutablesGeneratedClass(((ClassSymbol) type.tsym).getSuperclass(), state)); + } + return false; + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ImmutablesBuilderMissingInitializationTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ImmutablesBuilderMissingInitializationTest.java new file mode 100644 index 000000000..b31fb7d21 --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ImmutablesBuilderMissingInitializationTest.java @@ -0,0 +1,486 @@ +/* + * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.baseline.errorprone; + +import com.google.errorprone.CompilationTestHelper; +import com.palantir.baseline.errorprone.immutablebuildersmissinginitialization.ImmutablePerson; +import java.util.List; +import java.util.Optional; +import javax.annotation.Nullable; +import org.immutables.value.Value; +import org.immutables.value.Value.Style.ImplementationVisibility; +import org.junit.jupiter.api.Test; + +public class ImmutablesBuilderMissingInitializationTest { + @Test + public void testPassesWhenAllFieldsPopulated() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " new Person.Builder().name(\"name\").age(100).partner(\"partner\").build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenOptionalFieldOmitted() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " new Person.Builder().name(\"name\").age(100).build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenPopulatedWithFrom() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " Person p1 = new Person.Builder().name(\"name\").age(100).build();", + " new Person.Builder().from(p1).build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenBuilderAssignedToVariable() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " Person.Builder builder = new Person.Builder();", + " builder.build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWithUninitializedFields_whenBuilderFromMethodThatSetsSomeFields() { + helper().addSourceLines( + "MyTest.java", + importImmutable("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " builder().build();", + " }", + " private static ImmutablePerson.Builder builder() {", + " return new ImmutablePerson.Builder().name(\"name\").age(100);", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenAllFieldsPopulated_usingInterfaceBuilderMethod() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " Person.builder().name(\"name\").age(100).partner(\"partner\").build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenAllFieldsPopulated_usingImmutableBuilderMethod() { + helper().addSourceLines( + "MyTest.java", + importImmutable("NotOvershadowedType"), + "public class MyTest {", + " public static void main(String[] args) {", + " ImmutableNotOvershadowedType.builder().name(\"name\").build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenAllFieldsPopulated_usingImmutableConstructor() { + helper().addSourceLines( + "MyTest.java", + importImmutable("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " new ImmutablePerson.Builder().name(\"name\").age(100).build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenDefaultAndDerivedFieldsOmitted() { + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithDerivedAndDefaultFields"), + "public class MyTest {", + " public static void main(String[] args) {", + " ImmutableTypeWithDerivedAndDefaultFields.builder().value(\"value\").build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenNoRequiredFields() { + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithNoMandatoryFields"), + "public class MyTest {", + " public static void main(String[] args) {", + " ImmutableTypeWithNoMandatoryFields.builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWithAllFieldsPopulated_whenGettersAndSettersPrefixed() { + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithGetStyle"), + "public class MyTest {", + " public static void main(String[] args) {", + " ImmutableTypeWithGetStyle.builder().setValue(\"value\").build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWithAllFieldsPopulated_whenGettersAndSettersPrefixedAndFieldIsIdentifier() { + // If there is a getter and setter prefix, and removing the prefix would leave a reserved word, immutables + // uses the prefixed version for the init bits. Without any special treatment, that would cause us to search for + // methods ending with isPublic (eg setIsPublic), so test that this edge case is handled correctly + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithStyleAndIdentifierFields"), + "public class MyTest {", + " public static void main(String[] args) {", + " ImmutableTypeWithStyleAndIdentifierFields.builder().setPublic(true).build();", + " }", + "}") + .doTest(); + } + + @Test + public void testPassesWhenAllFieldsPopulated_withImmutableOnAbstractClass() { + helper().addSourceLines( + "MyTest.java", + importImmutable("ClassType"), + "public class MyTest {", + " public static void main(String[] args) {", + " ImmutableClassType.builder().value(\"value\").build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenOneMandatoryFieldOmitted() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " Person.builder()", + " .age(100)", + " .partner(\"partner\")", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: " + + "name", + " .build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenAllFieldsOmitted() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: name, age", + " new Person.Builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenOneFieldOmitted_usingImmutableConstructor() { + helper().addSourceLines( + "MyTest.java", + importImmutable("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: name", + " new ImmutablePerson.Builder().age(100).build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenOneFieldOmitted_usingInterfaceBuilderMethod() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: name", + " Person.builder().age(100).build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenOneFieldOmitted_usingSimulatedBuilderMethod() { + // This simulates using Person.builder() which delegates to ImmutablePerson.builder(), when Person is in the + // same compilation unit + helper().addSourceLines( + "Helper.java", + importImmutable("NotOvershadowedType"), + "public class Helper {", + " public static ImmutableNotOvershadowedType.Builder builder() {", + " return ImmutableNotOvershadowedType.builder();", + " }", + "}") + .addSourceLines( + "MyTest.java", + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: name", + " Helper.builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenOneFieldOmitted_fromLocalMethod() { + helper().addSourceLines( + "MyTest.java", + importInterface("Person"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: name, age", + " getBuilder().build();", + " }", + " private static Person.Builder getBuilder() {", + " return new Person.Builder();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenMandatoryFieldsOmitted_withDefaultAndDerivedFieldsOmitted() { + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithDerivedAndDefaultFields"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: value", + " ImmutableTypeWithDerivedAndDefaultFields.builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWithMandatoryFieldOmitted_whenGettersAndSettersPrefixed() { + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithGetStyle"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: value", + " ImmutableTypeWithGetStyle.builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWithMandatoryFieldOmitted_whenGettersAndSettersPrefixedAndFieldIsIdentifier() { + // If there is a getter and setter prefix, and removing the prefix would leave a reserved word, immutables + // uses the prefixed version for the init bits. Without any special treatment, that would cause us to search for + // methods ending with isPublic (eg setIsPublic), so test that this edge case is handled correctly + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithStyleAndIdentifierFields"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: public", + " ImmutableTypeWithStyleAndIdentifierFields.builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testFailsWhenOneFieldOmitted_withImmutableOnAbstractClass() { + helper().addSourceLines( + "MyTest.java", + importImmutable("ClassType"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: value", + " ImmutableClassType.builder().build();", + " }", + "}") + .doTest(); + } + + @Test + public void testComputesMissingFieldNamesCorrectly() { + helper().addSourceLines( + "MyTest.java", + importImmutable("TypeWithLongNames;"), + "public class MyTest {", + " public static void main(String[] args) {", + " // BUG: Diagnostic contains: Some builder fields have not been initialized: " + + "mandatoryFieldWithLongName", + " ImmutableTypeWithLongNames.builder().build();", + " }", + "}") + .doTest(); + } + + private CompilationTestHelper helper() { + return CompilationTestHelper.newInstance(ImmutablesBuilderMissingInitialization.class, getClass()); + } + + private String importInterface(String interfaceName) { + return String.format("import %s.%s;", getClass().getCanonicalName(), interfaceName); + } + + private String importImmutable(String interfaceName) { + return String.format( + "import %s.immutablebuildersmissinginitialization.Immutable%s;", + getClass().getPackage().getName(), interfaceName); + } + + @Value.Immutable + @Value.Style( + visibility = ImplementationVisibility.PUBLIC, + overshadowImplementation = true, + packageGenerated = "*.immutablebuildersmissinginitialization") + public interface Person { + String name(); + + int age(); + + Optional partner(); + + class Builder extends ImmutablePerson.Builder {} + + static Builder builder() { + return new Builder(); + } + } + + @Value.Immutable + @DefaultImmutableStyle + public interface NotOvershadowedType { + String name(); + } + + @Value.Immutable + @DefaultImmutableStyle + public interface TypeWithNoMandatoryFields { + Optional notRequired(); + + List list(); + + @Nullable + String maybeString(); + } + + @Value.Immutable + @DefaultImmutableStyle + public interface TypeWithLongNames { + String mandatoryFieldWithLongName(); + } + + @Value.Immutable + @DefaultImmutableStyle + public interface TypeWithDerivedAndDefaultFields { + String value(); + + @Value.Derived + default String derivedString() { + return value(); + } + + @Value.Default + default String defaultString() { + return "default"; + } + + @Value.Lazy + default String lazyString() { + return "lazy"; + } + } + + @Value.Immutable + @DefaultImmutableStyle + public abstract static class ClassType { + public abstract String value(); + } + + @Value.Immutable + @Value.Style( + visibility = ImplementationVisibility.PUBLIC, + packageGenerated = "*.immutablebuildersmissinginitialization", + get = "use*", + init = "set*") + public interface TypeWithGetStyle { + String useValue(); + } + + @Value.Immutable + @Value.Style( + visibility = ImplementationVisibility.PUBLIC, + packageGenerated = "*.immutablebuildersmissinginitialization", + get = "is*", + init = "set*") + public interface TypeWithStyleAndIdentifierFields { + boolean isPublic(); + } + + @Value.Style( + visibility = ImplementationVisibility.PUBLIC, + packageGenerated = "*.immutablebuildersmissinginitialization") + private @interface DefaultImmutableStyle {} +} diff --git a/changelog/@unreleased/pr-1504.v2.yml b/changelog/@unreleased/pr-1504.v2.yml new file mode 100644 index 000000000..74673f6cb --- /dev/null +++ b/changelog/@unreleased/pr-1504.v2.yml @@ -0,0 +1,6 @@ +type: improvement +improvement: + description: Add an Errorprone rule to check that all fields in Immutables builders + have been initialized + links: + - https://github.com/palantir/gradle-baseline/pull/1504