Fix NPE for uninitialized non-null collection (#98)

Closes #91

Co-authored-by: Clément MATHIEU <clement@unportant.info>
This commit is contained in:
Jordan Zimmerman
2022-03-20 10:46:39 +00:00
committed by GitHub
parent fef69af183
commit d3828eda74
7 changed files with 42 additions and 7 deletions

12
pom.xml
View File

@@ -32,6 +32,7 @@
<maven-shade-plugin-version>3.2.1</maven-shade-plugin-version>
<maven-release-plugin-version>2.5.3</maven-release-plugin-version>
<maven-jar-plugin-version>3.2.0</maven-jar-plugin-version>
<maven-surefire-plugin-version>3.0.0-M5</maven-surefire-plugin-version>
<license-file-path>src/etc/header.txt</license-file-path>
@@ -153,6 +154,12 @@
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven-surefire-plugin-version}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
@@ -352,6 +359,11 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
</plugin>
</plugins>
</build>

View File

@@ -109,6 +109,10 @@ class CollectionBuilderUtils {
return Optional.empty();
}
boolean isImmutableCollection(RecordClassType component) {
return useImmutableCollections && (isList(component) || isMap(component) || isSet(component) || component.rawTypeName().equals(collectionType));
}
boolean isList(RecordClassType component) {
return component.rawTypeName().equals(listType);
}

View File

@@ -371,8 +371,10 @@ class InternalRecordBuilderProcessor {
private void addNullCheckCodeBlock(CodeBlock.Builder builder, int index) {
if (metaData.interpretNotNulls()) {
var component = recordComponents.get(index);
if (!component.typeName().isPrimitive() && isNullAnnotated(component)) {
builder.addStatement("$T.requireNonNull($L, $S)", Objects.class, component.name(), component.name() + " is required");
if (!collectionBuilderUtils.isImmutableCollection(component)) {
if (!component.typeName().isPrimitive() && isNullAnnotated(component)) {
builder.addStatement("$T.requireNonNull($L, $S)", Objects.class, component.name(), component.name() + " is required");
}
}
}
}
@@ -521,6 +523,16 @@ class InternalRecordBuilderProcessor {
*/
var codeBuilder = CodeBlock.builder();
IntStream.range(0, recordComponents.size()).forEach(index -> {
var recordComponent = recordComponents.get(index);
if (collectionBuilderUtils.isImmutableCollection(recordComponent)) {
codeBuilder.add("$[$L = ", recordComponent.name());
collectionBuilderUtils.add(codeBuilder, recordComponents.get(index));
codeBuilder.add(";\n$]");
}
});
addNullCheckCodeBlock(codeBuilder);
codeBuilder.add("$[return ");
if (metaData.useValidationApi()) {
@@ -531,7 +543,7 @@ class InternalRecordBuilderProcessor {
if (index > 0) {
codeBuilder.add(", ");
}
collectionBuilderUtils.add(codeBuilder, recordComponents.get(index));
codeBuilder.add("$L", recordComponents.get(index).name());
});
codeBuilder.add(")");
if (metaData.useValidationApi()) {

View File

@@ -22,5 +22,5 @@ import java.util.List;
import java.util.Map;
@RecordBuilderFull
public record FullRecord(@NotNull List<Number> numbers, @NotNull Map<Number, FullRecord> fullRecords) {
public record FullRecord(@NotNull List<Number> numbers, @NotNull Map<Number, FullRecord> fullRecords, @NotNull String justAString) {
}

View File

@@ -18,8 +18,9 @@ package io.soabase.recordbuilder.test;
import io.soabase.recordbuilder.core.RecordBuilder;
import javax.validation.constraints.NotNull;
import java.util.List;
@RecordBuilder.Options(interpretNotNulls = true)
@RecordBuilder
public record RequiredRecord(@NotNull String hey, @NotNull int i) implements RequiredRecordBuilder.With {
public record RequiredRecord(@NotNull String hey, @NotNull int i, @NotNull List<String> l) implements RequiredRecordBuilder.With {
}

View File

@@ -20,11 +20,15 @@ import org.junit.jupiter.api.Test;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
class TestRecordBuilderFull {
@Test
void testNonNull() {
Assertions.assertThrows(NullPointerException.class, () -> FullRecordBuilder.builder().build());
var record = FullRecordBuilder.builder().justAString("").build();
Assertions.assertEquals(List.of(), record.numbers());
Assertions.assertEquals(Map.of(), record.fullRecords());
}
@Test
@@ -32,6 +36,7 @@ class TestRecordBuilderFull {
var record = FullRecordBuilder.builder()
.fullRecords(new HashMap<>())
.numbers(new ArrayList<>())
.justAString("")
.build();
Assertions.assertThrows(UnsupportedOperationException.class, () -> record.fullRecords().put(1, record));
Assertions.assertThrows(UnsupportedOperationException.class, () -> record.numbers().add(1));

View File

@@ -19,6 +19,7 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import javax.validation.ValidationException;
import java.util.List;
class TestValidation {
@Test
@@ -33,7 +34,7 @@ class TestValidation {
@Test
void testNotNullsWithNewProperty() {
var valid = RequiredRecordBuilder.builder().hey("hey").i(1).build();
var valid = RequiredRecordBuilder.builder().hey("hey").i(1).l(List.of()).build();
Assertions.assertThrows(NullPointerException.class, () -> valid.withHey(null));
}