Bug when immutableCollections are used together with STAGED_REQUIRED_ONLY

options.md

@@ -74,6 +74,12 @@ Use `@RecordBuilder.Options(builderMode = BuilderMode.STAGED)` or `@RecordBuilde
 builders. Staged builders require that each record component is built in order and that each component is specified. The generated builder ensures
 this via individual staged builders. See [TestStagedBuilder](record-builder-test/src/test/java/io/soabase/recordbuilder/test/staged/TestStagedBuilder.java) for examples.
 
+A variant of staged builders is available that only stages required record components. Use `BuilderMode.STAGED_REQUIRED_ONLY` or `BuilderMode.STANDARD_AND_STAGED_REQUIRED_ONLY`. 
+The following are not staged and are added to the final stage:
+- optional components (when `addConcreteSettersForOptional` is enabled)  
+- Any collections matching enabled [Collection options](#collections)  
+- Any annotated compontents that match the `nullablePattern()` pattern option (e.g. `@Nullable`)
+
 ## Default Values / Initializers
 
 | option                                                             | details                                                                                                                          |
@@ -88,11 +94,12 @@ for an example.
 
 ## Null Handling
 
-| option                                                          | details                                                                                                       |
-|-----------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------|
-| `@RecordBuilder.Options(interpretNotNulls = true/false)`        | Add not-null checks for record components annotated with any null-pattern annotation. The default is `false`. |
-| `@RecordBuilder.Options(interpretNotNullsPattern = "regex")`    | The regex pattern used to determine if an annotation name means non-null.                                     |
-| `@RecordBuilder.Options(allowNullableCollections = true/false)` | Adds special null handling for record collectioncomponents. The default is `false`.                           |
+| option                                                          | details                                                                                                          |
+|-----------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
+| `@RecordBuilder.Options(interpretNotNulls = true/false)`        | Add not-null checks for record components annotated with any null-pattern annotation. The default is `false`.    |
+| `@RecordBuilder.Options(interpretNotNullsPattern = "regex")`    | The regex pattern used to determine if an annotation name means non-null.                                        |
+| `@RecordBuilder.Options(allowNullableCollections = true/false)` | Adds special null handling for record collectioncomponents. The default is `false`.                              |
+| `@RecordBuilder.Options(nullablePattern = "regex")`             | Regex pattern to use for `BuilderMode.STAGED_REQUIRED_ONLY` and `BuilderMode.STANDARD_AND_STAGED_REQUIRED_ONLY`. |
 
 ## Collections
 

record-builder-core/src/main/java/io/soabase/recordbuilder/core/RecordBuilder.java

@@ -322,6 +322,12 @@
          */
         String stagedBuilderMethodSuffix() default "Stage";
 
+        /**
+         * If {@link #builderMode()} is `STAGED_REQUIRED_ONLY` or `STANDARD_AND_STAGED_REQUIRED_ONLY, this is the regex
+         * pattern used to determine if an annotation name means "null-able"
+         */
+        String nullablePattern() default "(?i)^((null)|(nullable))$";
+
         /**
          * If true, attributes can be set/assigned only 1 time. Attempts to reassign/reset attributes will throw
          * {@code java.lang.IllegalStateException}
@@ -345,7 +351,7 @@
     }
 
     enum BuilderMode {
-        STANDARD, STAGED, STANDARD_AND_STAGED,
+        STANDARD, STAGED, STAGED_REQUIRED_ONLY, STANDARD_AND_STAGED, STANDARD_AND_STAGED_REQUIRED_ONLY,
     }
 
     /**

record-builder-processor/src/main/java/io/soabase/recordbuilder/processor/InternalRecordBuilderProcessor.java

@@ -48,8 +48,10 @@ class InternalRecordBuilderProcessor {
     private final TypeSpec.Builder builder;
     private final String uniqueVarName;
     private final Pattern notNullPattern;
+    private final Pattern nullablePattern;
     private final CollectionBuilderUtils collectionBuilderUtils;
 
+    private static final TypeName optionalType = TypeName.get(Optional.class);
     private static final TypeName overrideType = TypeName.get(Override.class);
     private static final TypeName validType = ClassName.get("javax.validation", "Valid");
     private static final TypeName validatorTypeName = ClassName.get("io.soabase.recordbuilder.validator",
@@ -72,6 +74,7 @@ class InternalRecordBuilderProcessor {
         recordComponents = buildRecordComponents(record);
         uniqueVarName = getUniqueVarName();
         notNullPattern = Pattern.compile(metaData.interpretNotNullsPattern());
+        nullablePattern = Pattern.compile(metaData.nullablePattern());
         collectionBuilderUtils = new CollectionBuilderUtils(recordComponents, this.metaData);
         constructorVisibilityModifier = metaData.publicBuilderConstructors() ? Modifier.PUBLIC : Modifier.PRIVATE;
         initializers = InitializerUtil.detectInitializers(processingEnv, record);
@@ -90,8 +93,9 @@ class InternalRecordBuilderProcessor {
         }
         if (metaData.builderMode() != BuilderMode.STANDARD) {
             addStagedBuilderClasses();
-            addStaticStagedBuilderMethod((metaData.builderMode() == BuilderMode.STANDARD_AND_STAGED)
-                    ? metaData.stagedBuilderMethodName() : metaData.builderMethodName());
+            addStaticStagedBuilderMethod(((metaData.builderMode() == BuilderMode.STANDARD_AND_STAGED)
+                    || (metaData.builderMode() == BuilderMode.STANDARD_AND_STAGED_REQUIRED_ONLY))
+                            ? metaData.stagedBuilderMethodName() : metaData.builderMethodName());
         }
         addDefaultConstructor();
         if (metaData.addStaticBuilder()) {
@@ -100,7 +104,8 @@ class InternalRecordBuilderProcessor {
         if (recordComponents.size() > 0) {
             addAllArgsConstructor();
         }
-        if (metaData.builderMode() != BuilderMode.STAGED) {
+        if ((metaData.builderMode() != BuilderMode.STAGED)
+                && (metaData.builderMode() != BuilderMode.STAGED_REQUIRED_ONLY)) {
             addStaticDefaultBuilderMethod();
         }
         addStaticCopyBuilderMethod();
@@ -195,11 +200,32 @@ private void addOnceOnlySupport() {
         builder.addField(onceOnlyField);
     }
 
+    private boolean isRequiredStage(RecordClassType recordComponent) {
+        if ((metaData.builderMode() != BuilderMode.STAGED_REQUIRED_ONLY)
+                && (metaData.builderMode() != BuilderMode.STANDARD_AND_STAGED_REQUIRED_ONLY)) {
+            return true;
+        }
+
+        if (collectionBuilderUtils.isNullableCollection(recordComponent)
+                || collectionBuilderUtils.isImmutableCollection(recordComponent)) {
+            return false;
+        }
+
+        if (isNullableAnnotated(recordComponent)) {
+            return false;
+        }
+
+        return !metaData.emptyDefaultForOptional() || !recordComponent.rawTypeName().equals(optionalType);
+    }
+
     private void addStagedBuilderClasses() {
-        IntStream.range(0, recordComponents.size()).forEach(index -> {
-            Optional<RecordClassType> nextComponent = ((index + 1) < recordComponents.size())
-                    ? Optional.of(recordComponents.get(index + 1)) : Optional.empty();
-            add1StagedBuilderClass(recordComponents.get(index), nextComponent);
+        List<RecordClassType> filteredRecordComponents = recordComponents.stream().filter(this::isRequiredStage)
+                .toList();
+
+        IntStream.range(0, filteredRecordComponents.size()).forEach(index -> {
+            Optional<RecordClassType> nextComponent = ((index + 1) < filteredRecordComponents.size())
+                    ? Optional.of(filteredRecordComponents.get(index + 1)) : Optional.empty();
+            add1StagedBuilderClass(filteredRecordComponents.get(index), nextComponent);
         });
 
         /*
@@ -218,9 +244,25 @@ private void addStagedBuilderClasses() {
         }
 
         MethodSpec buildMethod = buildMethod().addModifiers(Modifier.PUBLIC, Modifier.DEFAULT)
-                .addStatement("return builder().build()").build();
+                .addStatement("return $L().$L()", metaData.builderMethodName(), metaData.buildMethodName()).build();
         classBuilder.addMethod(buildMethod);
 
+        recordComponents.stream().filter(recordComponent -> !isRequiredStage(recordComponent))
+                .forEach(optionalComponent -> {
+                    var codeBlock = CodeBlock.builder().add("return $L().$L($L);", metaData.builderMethodName(),
+                            optionalComponent.name(), optionalComponent.name()).build();
+
+                    var parameterSpecBuilder = ParameterSpec.builder(optionalComponent.typeName(),
+                            optionalComponent.name());
+                    addConstructorAnnotations(optionalComponent, parameterSpecBuilder);
+                    var methodSpec = MethodSpec.methodBuilder(optionalComponent.name())
+                            .addAnnotation(generatedRecordBuilderAnnotation)
+                            .addJavadoc("Call builder for optional component {@code $L}", optionalComponent.name())
+                            .addModifiers(Modifier.PUBLIC, Modifier.DEFAULT).addParameter(parameterSpecBuilder.build())
+                            .addCode(codeBlock).returns(builderClassType.typeName()).build();
+                    classBuilder.addMethod(methodSpec);
+                });
+
         var builderMethod = MethodSpec.methodBuilder(metaData.builderMethodName())
                 .addJavadoc("Return a new builder with all fields set to the current values in this builder\n")
                 .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT).addAnnotation(generatedRecordBuilderAnnotation)
@@ -460,19 +502,24 @@ private void addNullCheckCodeBlock(CodeBlock.Builder builder, int index) {
         if (metaData.interpretNotNulls()) {
             var component = recordComponents.get(index);
             if (!collectionBuilderUtils.isImmutableCollection(component)) {
-                if (!component.typeName().isPrimitive() && isNullAnnotated(component)) {
+                if (!component.typeName().isPrimitive() && isNotNullAnnotated(component)) {
                     builder.addStatement("$T.requireNonNull($L, $S)", Objects.class, component.name(),
                             component.name() + " is required");
                 }
             }
         }
     }
 
-    private boolean isNullAnnotated(RecordClassType component) {
+    private boolean isNotNullAnnotated(RecordClassType component) {
         return component.getCanonicalConstructorAnnotations().stream().anyMatch(annotation -> notNullPattern
                 .matcher(annotation.getAnnotationType().asElement().getSimpleName().toString()).matches());
     }
 
+    private boolean isNullableAnnotated(RecordClassType component) {
+        return component.getCanonicalConstructorAnnotations().stream().anyMatch(annotation -> nullablePattern
+                .matcher(annotation.getAnnotationType().asElement().getSimpleName().toString()).matches());
+    }
+
     private void addAllArgsConstructor() {
         /*
          * Adds an all-args constructor similar to:
@@ -724,32 +771,24 @@ private void addStaticStagedBuilderMethod(String builderMethodName) {
          *
          * public static NameStage stagedBuilder() { return name -> age -> () -> new PersonBuilder(name, age).build(); }
          */
+
+        List<RecordClassType> filteredRecordComponents = recordComponents.stream().filter(this::isRequiredStage)
+                .toList();
+
         var codeBlock = CodeBlock.builder();
-        if (metaData.onceOnlyAssignment()) {
-            codeBlock.addStatement("$T $L = new $T()", builderClassType.typeName(), uniqueVarName,
-                    builderClassType.typeName());
-            codeBlock.add("return ");
-            recordComponents.forEach(recordComponent -> {
-                codeBlock.add("$L -> {\n", recordComponent.name()).indent()
-                        .addStatement("$L.$L($L)", uniqueVarName, recordComponent.name(), recordComponent.name())
-                        .add("return ");
-            });
-            codeBlock.addStatement("() -> $L", uniqueVarName);
-            IntStream.range(0, recordComponents.size()).forEach(__ -> codeBlock.unindent().addStatement("}"));
-        } else {
-            codeBlock.add("return ");
-            recordComponents.forEach(recordComponent -> codeBlock.add("$L -> ", recordComponent.name()));
-            codeBlock.add("() -> new $T(", builderClassType.typeName());
-            IntStream.range(0, recordComponents.size()).forEach(index -> {
-                if (index > 0) {
-                    codeBlock.add(", ");
-                }
-                codeBlock.add("$L", recordComponents.get(index).name());
-            });
-            codeBlock.addStatement(")");
-        }
+        codeBlock.addStatement("$T $L = new $T()", builderClassType.typeName(), uniqueVarName,
+                builderClassType.typeName());
+        codeBlock.add("return ");
+        filteredRecordComponents.forEach(recordComponent -> {
+            codeBlock.add("$L -> {\n", recordComponent.name()).indent()
+                    .addStatement("$L.$L($L)", uniqueVarName, recordComponent.name(), recordComponent.name())
+                    .add("return ");
+        });
+        codeBlock.addStatement("() -> $L", uniqueVarName);
+        IntStream.range(0, filteredRecordComponents.size()).forEach(__ -> codeBlock.unindent().addStatement("}"));
 
-        var returnType = stagedBuilderType(recordComponents.isEmpty() ? builderClassType : recordComponents.get(0));
+        var returnType = stagedBuilderType(
+                filteredRecordComponents.isEmpty() ? builderClassType : filteredRecordComponents.get(0));
 
         var methodSpec = MethodSpec.methodBuilder(builderMethodName)
                 .addJavadoc("Return the first stage of a staged builder\n")

record-builder-test/src/main/java/io/soabase/recordbuilder/test/staged/CombinedSimpleStagedRequiredOnly.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2019 The original author or authors
+ *
+ * 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 io.soabase.recordbuilder.test.staged;
+
+import io.soabase.recordbuilder.core.RecordBuilder;
+
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Null;
+import java.util.Set;
+
+@RecordBuilder
+@RecordBuilder.Options(useImmutableCollections = true, builderMode = RecordBuilder.BuilderMode.STANDARD_AND_STAGED_REQUIRED_ONLY, allowNullableCollections = true, interpretNotNulls = true)
+public record CombinedSimpleStagedRequiredOnly(Set<Integer> numbers, @Null String foo, @NotNull Set<Integer> requiredNumbers) {
+}

record-builder-test/src/main/java/io/soabase/recordbuilder/test/staged/OptionalListStaged.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2019 The original author or authors
+ *
+ * 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 io.soabase.recordbuilder.test.staged;
+
+import io.soabase.recordbuilder.core.RecordBuilder;
+
+import javax.validation.constraints.Null;
+import java.time.Instant;
+import java.util.List;
+import java.util.Optional;
+
+@RecordBuilder
+@RecordBuilder.Options(builderMode = RecordBuilder.BuilderMode.STAGED_REQUIRED_ONLY, interpretNotNulls = true, useImmutableCollections = true)
+public record OptionalListStaged(int a, Optional<String> b, double c, List<Instant> d, @Null String e, String f) {
+}

record-builder-test/src/test/java/io/soabase/recordbuilder/test/staged/TestStagedBuilder.java

@@ -20,6 +20,9 @@
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.time.Instant;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -73,4 +76,31 @@ void testNoFields() {
         NoFieldsStaged obj = NoFieldsStagedBuilder.builder().build();
         assertEquals(new NoFieldsStaged(), obj);
     }
+
+    @Test
+    void testOptionalList() {
+        OptionalListStaged obj = OptionalListStagedBuilder.builder().a(1).c(1.1).f("ffff").b(Optional.of("bbbb"))
+                .d(List.of(Instant.EPOCH)).e("eeee").build();
+        assertEquals(new OptionalListStaged(1, Optional.of("bbbb"), 1.1, List.of(Instant.EPOCH), "eeee", "ffff"), obj);
+
+        obj = OptionalListStagedBuilder.builder().a(1).c(1.1).f("ffff").build();
+        assertEquals(new OptionalListStaged(1, Optional.empty(), 1.1, List.of(), null, "ffff"), obj);
+    }
+
+    @Test
+    void testCombinedSimpleStagedRequiredOnly() {
+        CombinedSimpleStagedRequiredOnly obj = CombinedSimpleStagedRequiredOnlyBuilder.stagedBuilder()
+                .numbers(Set.of(5, 4)).build();
+        assertEquals(new CombinedSimpleStagedRequiredOnly(Set.of(5, 4), null, Set.of()), obj);
+
+        obj = CombinedSimpleStagedRequiredOnlyBuilder.stagedBuilder().build();
+        assertEquals(new CombinedSimpleStagedRequiredOnly(null, null, Set.of()), obj);
+
+        obj = CombinedSimpleStagedRequiredOnlyBuilder.stagedBuilder()
+                .foo("ok")
+                .numbers(Set.of(5, 2))
+                .requiredNumbers(Set.of())
+                .build();
+        assertEquals(new CombinedSimpleStagedRequiredOnly(Set.of(5, 2), "ok", Set.of()), obj);
+    }
 }