Generate missing-property checks in a way that is easier for null-ana…

…lysis to understand. The new code is more verbose than the old, since it has two null checks for every property (when there is more than one required property). The second check will only be executed when at least one property is missing and we are about to throw an exception. The extra checks also don't happen when compiling without identifiers, as is typically the case in code-size-sensitive environments like Android. RELNOTES=Generated builder code is now slightly friendly to null-analysis. PiperOrigin-RevId: 374652915
google · May 19, 2021 · f00c32a · f00c32a
1 parent cd55f63
commit f00c32a
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 31 deletions.
diff --git a/value/src/main/java/com/google/auto/value/processor/builder.vm b/value/src/main/java/com/google/auto/value/processor/builder.vm
@@ -239,31 +239,41 @@ class ${builderName}${builderFormalTypes} ##
 #end
 
 #if (!$builderRequiredProperties.empty)
+    if (#foreach ($p in $builderRequiredProperties)##
+        this.$p == null##
+          #if ($foreach.hasNext)
+
+        || #end
+        #end) {
+
   #if ($identifiers)  ## build a friendly message showing all missing properties
+    #if ($builderRequiredProperties.size() == 1)
 
-    `java.lang.String` missing = "";
+    `java.lang.String` missing = " $builderRequiredProperties.iterator().next()";
 
-    #foreach ($p in $builderRequiredProperties)
+    #else
+
+    `java.lang.StringBuilder` missing = new `java.lang.StringBuilder`();
+
+      #foreach ($p in $builderRequiredProperties)
 
     if (this.$p == null) {
-      missing += " $p.name";
+      missing.append(" $p.name");
     }
 
+      #end
     #end
 
-    if (!missing.isEmpty()) {
-      throw new IllegalStateException("Missing required properties:" + missing);
-    }
+    throw new IllegalStateException("Missing required properties:" + missing);
 
   #else  ## just throw an exception if anything is missing
 
-    if (#foreach ($p in $builderRequiredProperties)##
-        this.$p == null##
-        #if ($foreach.hasNext) || #end
-        #end) {
-      throw new IllegalStateException();
-    }
+    throw new IllegalStateException();
+
   #end
+
+  }
+
 #end
 
     #if ($builtType != "void") return #end ${build}(

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
@@ -58,14 +58,15 @@ public final class AutoBuilderCompilationTest {
           "",
           "  @Override",
           "  public Baz build() {",
-          "    String missing = \"\";",
-          "    if (this.anInt == null) {",
-          "      missing += \" anInt\";",
-          "    }",
-          "    if (this.aString == null) {",
-          "      missing += \" aString\";",
-          "    }",
-          "    if (!missing.isEmpty()) {",
+          "    if (this.anInt == null",
+          "        || this.aString == null) {",
+          "      StringBuilder missing = new StringBuilder();",
+          "      if (this.anInt == null) {",
+          "        missing.append(\" anInt\");",
+          "      }",
+          "      if (this.aString == null) {",
+          "        missing.append(\" aString\");",
+          "      }",
           "      throw new IllegalStateException(\"Missing required properties:\" + missing);",
           "    }",
           "    return new Baz(",

diff --git a/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java
@@ -1311,17 +1311,19 @@ public void correctBuilder() {
                 + "NestedAutoValue.builder();",
             "        this.aNestedAutoValue = aNestedAutoValue$builder.build();",
             "      }",
-            "      String missing = \"\";",
-            "      if (this.anInt == null) {",
-            "        missing += \" anInt\";",
-            "      }",
-            "      if (this.aByteArray == null) {",
-            "        missing += \" aByteArray\";",
-            "      }",
-            "      if (this.aList == null) {",
-            "        missing += \" aList\";",
-            "      }",
-            "      if (!missing.isEmpty()) {",
+            "      if (this.anInt == null",
+            "          || this.aByteArray == null",
+            "          || this.aList == null) {",
+            "        StringBuilder missing = new StringBuilder();",
+            "        if (this.anInt == null) {",
+            "          missing.append(\" anInt\");",
+            "        }",
+            "        if (this.aByteArray == null) {",
+            "          missing.append(\" aByteArray\");",
+            "        }",
+            "        if (this.aList == null) {",
+            "          missing.append(\" aList\");",
+            "        }",
             "        throw new IllegalStateException(\"Missing required properties:\" + missing);",
             "      }",
             "      return new AutoValue_Baz<T>(",