From 35a311d8a870207a26878909dcd335b8667ef4f3 Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Fri, 30 Aug 2024 18:43:17 -0400 Subject: [PATCH 1/4] Add non-null constraints using jspecify --- blackbox-test/src/main/java/module-info.java | 2 + .../avaje/jspecify/JSpecifyNotNull.java | 10 +++++ .../avaje/jspecify/JSpecifyNullUnmarked.java | 10 +++++ .../example/avaje/jspecify/JSpecifyTest.java | 38 +++++++++++++++++++ .../example/avaje/jspecify/package-info.java | 2 + .../generator/ElementAnnotationContainer.java | 15 +++++++- .../io/avaje/validation/generator/Util.java | 28 ++++++++++++++ .../validation/generator/package-info.java | 7 +++- .../models/valid/JSpecifyNotNull.java | 12 ++++++ .../models/valid/JSpecifyNullUnmarked.java | 9 +++++ 10 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java create mode 100644 blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNullUnmarked.java create mode 100644 blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java create mode 100644 blackbox-test/src/test/java/example/avaje/jspecify/package-info.java create mode 100644 validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java create mode 100644 validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNullUnmarked.java diff --git a/blackbox-test/src/main/java/module-info.java b/blackbox-test/src/main/java/module-info.java index d523685f..4f068da0 100644 --- a/blackbox-test/src/main/java/module-info.java +++ b/blackbox-test/src/main/java/module-info.java @@ -6,6 +6,8 @@ requires io.avaje.validation.contraints; requires jakarta.validation; requires jakarta.inject; + requires org.jspecify; + provides io.avaje.validation.spi.ValidationExtension with example.avaje.valid.GeneratedValidatorComponent; provides io.avaje.inject.spi.InjectExtension with example.avaje.GeneratedModule; } diff --git a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java new file mode 100644 index 00000000..8a92a1a6 --- /dev/null +++ b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java @@ -0,0 +1,10 @@ +package example.avaje.jspecify; + +import org.jspecify.annotations.Nullable; + +import io.avaje.validation.constraints.NotBlank; +import jakarta.validation.Valid; + +@Valid +public record JSpecifyNotNull( + @NotBlank String basic, @NotBlank String withMax, @Nullable @NotBlank String withCustom) {} diff --git a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNullUnmarked.java b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNullUnmarked.java new file mode 100644 index 00000000..b8a125c9 --- /dev/null +++ b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNullUnmarked.java @@ -0,0 +1,10 @@ +package example.avaje.jspecify; + +import org.jspecify.annotations.NullUnmarked; + +import io.avaje.validation.constraints.Null; +import jakarta.validation.Valid; + +@Valid +@NullUnmarked +public record JSpecifyNullUnmarked(@Null String basic) {} diff --git a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java new file mode 100644 index 00000000..55b5a53f --- /dev/null +++ b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java @@ -0,0 +1,38 @@ +package example.avaje.jspecify; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +import java.util.Locale; + +import org.junit.jupiter.api.Test; + +import io.avaje.validation.ConstraintViolationException; +import io.avaje.validation.Validator; + +public class JSpecifyTest { + + final Validator validator = + Validator.builder() + .add(JSpecifyNotNull.class, JSpecifyNotNullValidationAdapter::new) + .add(JSpecifyNullUnmarked.class, JSpecifyNullUnmarkedValidationAdapter::new) + .addLocales(Locale.GERMAN) + .build(); + + @Test + void valid() { + var value = new JSpecifyNotNull("ok", "ok", "ok"); + validator.validate(value); + validator.validate(new JSpecifyNullUnmarked(null)); + } + + @Test + void inValidNull() { + var value = new JSpecifyNotNull(null, null, null); + try { + validator.validate(value); + } catch (ConstraintViolationException e) { + assertThat(e.violations()).hasSize(2); + } + } +} diff --git a/blackbox-test/src/test/java/example/avaje/jspecify/package-info.java b/blackbox-test/src/test/java/example/avaje/jspecify/package-info.java new file mode 100644 index 00000000..cdfb9cd3 --- /dev/null +++ b/blackbox-test/src/test/java/example/avaje/jspecify/package-info.java @@ -0,0 +1,2 @@ +@org.jspecify.annotations.NullMarked +package example.avaje.jspecify; diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java b/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java index 94a4b7c9..ff656944 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java @@ -26,7 +26,6 @@ public record ElementAnnotationContainer( static ElementAnnotationContainer create(Element element) { final var hasValid = ValidPrism.isPresent(element); - Map typeUse1; Map typeUse2; final Map crossParam = new HashMap<>(); @@ -76,6 +75,12 @@ static ElementAnnotationContainer create(Element element) { a -> UType.parse(a.getAnnotationType()), a -> AnnotationUtil.annotationAttributeMap(a, element))); + if (Util.isNonNullable(element)) { + var nonNull = UType.parse(APContext.typeElement(NonNullPrism.PRISM_TYPE).asType()); + + annotations.put(nonNull, "Map.of(\"message\",\"{avaje.NotNull.message}\")"); + } + return new ElementAnnotationContainer( uType, hasValid, annotations, typeUse1, typeUse2, crossParam); } @@ -89,7 +94,7 @@ static boolean hasMetaConstraintAnnotation(Element element) { return ConstraintPrism.isPresent(element); } - // it seems we cannot directly retrieve mirrors from var elements, for varElements needs special + // it seems we cannot directly retrieve mirrors from var elements, so var Elements needs special // handling static ElementAnnotationContainer create(VariableElement varElement) { @@ -122,6 +127,12 @@ static ElementAnnotationContainer create(VariableElement varElement) { final boolean hasValid = uType.annotations().stream().anyMatch(ValidPrism::isInstance); + if (Util.isNonNullable(varElement)) { + var nonNull = UType.parse(APContext.typeElement(NonNullPrism.PRISM_TYPE).asType()); + + annotations.put(nonNull, "Map.of(\"message\",\"{avaje.NotNull.message}\")"); + } + return new ElementAnnotationContainer( uType, hasValid, annotations, typeUse1, typeUse2, Map.of()); } diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/Util.java b/validator-generator/src/main/java/io/avaje/validation/generator/Util.java index 22208488..aa16d688 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/Util.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/Util.java @@ -168,4 +168,32 @@ static String valhalla() { return ""; } + public static boolean isNonNullable(Element e) { + UType uType; + if (e instanceof final ExecutableElement executableElement) { + uType = UType.parse(executableElement.getReturnType()); + + } else { + uType = UType.parse(e.asType()); + } + + for (var mirror : uType.annotations()) { + if (mirror.getAnnotationType().toString().endsWith("Nullable")) { + return false; + } else if (NonNullPrism.isInstance(mirror)) { + return true; + } + } + + return checkNullMarked(e); + } + + private static boolean checkNullMarked(Element e) { + if (e == null || NullUnmarkedPrism.isPresent(e)) { + return false; + } else if (NullMarkedPrism.isPresent(e)) { + return true; + } + return checkNullMarked(e.getEnclosingElement()); + } } diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/package-info.java b/validator-generator/src/main/java/io/avaje/validation/generator/package-info.java index b919b67b..51a8cfde 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/package-info.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/package-info.java @@ -1,10 +1,13 @@ -@GeneratePrism(io.avaje.validation.ImportValidPojo.class) @GeneratePrism(io.avaje.validation.adapter.ConstraintAdapter.class) +@GeneratePrism(io.avaje.validation.ImportValidPojo.class) @GeneratePrism(io.avaje.validation.spi.MetaData.class) @GeneratePrism(io.avaje.validation.spi.MetaData.Factory.class) @GeneratePrism(io.avaje.validation.spi.MetaData.AnnotationFactory.class) -@GeneratePrism(io.avaje.validation.ValidMethod.class) @GeneratePrism(io.avaje.validation.MixIn.class) +@GeneratePrism(org.jspecify.annotations.NullMarked.class) +@GeneratePrism(org.jspecify.annotations.NullUnmarked.class) +@GeneratePrism(org.jspecify.annotations.NonNull.class) +@GeneratePrism(io.avaje.validation.ValidMethod.class) package io.avaje.validation.generator; import io.avaje.prism.GeneratePrism; diff --git a/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java b/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java new file mode 100644 index 00000000..bf8d37ce --- /dev/null +++ b/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java @@ -0,0 +1,12 @@ +package io.avaje.validation.generator.models.valid; + +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import io.avaje.validation.constraints.NotBlank; +import jakarta.validation.Valid; + +@Valid +@NullMarked +public record JSpecifyNotNull( + @NotBlank String basic, @NotBlank String withMax, @Nullable @NotBlank String withCustom) {} diff --git a/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNullUnmarked.java b/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNullUnmarked.java new file mode 100644 index 00000000..5892ad61 --- /dev/null +++ b/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNullUnmarked.java @@ -0,0 +1,9 @@ +package io.avaje.validation.generator.models.valid; + +import org.jspecify.annotations.NullUnmarked; + +import jakarta.validation.Valid; + +@Valid +@NullUnmarked +public record JSpecifyNullUnmarked(String basic, String withMax, String withCustom) {} From 0f98c15e2ed8107beaa7518d2c0165629edb69ca Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Sat, 31 Aug 2024 14:21:26 -0400 Subject: [PATCH 2/4] no longer need a cosntraint annotation --- .../src/test/java/example/avaje/jspecify/JSpecifyNotNull.java | 4 +--- .../src/test/java/example/avaje/jspecify/JSpecifyTest.java | 1 - .../main/java/io/avaje/validation/generator/TypeReader.java | 4 +++- .../validation/generator/models/valid/JSpecifyNotNull.java | 4 +--- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java index 8a92a1a6..d0daddc2 100644 --- a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java +++ b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyNotNull.java @@ -2,9 +2,7 @@ import org.jspecify.annotations.Nullable; -import io.avaje.validation.constraints.NotBlank; import jakarta.validation.Valid; @Valid -public record JSpecifyNotNull( - @NotBlank String basic, @NotBlank String withMax, @Nullable @NotBlank String withCustom) {} +public record JSpecifyNotNull(String basic, String withMax, @Nullable String withCustom) {} diff --git a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java index 55b5a53f..ba05ee08 100644 --- a/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java +++ b/blackbox-test/src/test/java/example/avaje/jspecify/JSpecifyTest.java @@ -1,7 +1,6 @@ package example.avaje.jspecify; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; import java.util.Locale; diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java b/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java index c05009e8..da31fd58 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java @@ -129,7 +129,9 @@ int genericTypeParamsCount() { private boolean includeField(Element element) { return !element.getModifiers().contains(Modifier.TRANSIENT) - && (!element.getAnnotationMirrors().isEmpty() || element.asType().toString().contains("@")); + && (!element.getAnnotationMirrors().isEmpty() + || element.asType().toString().contains("@") + || Util.isNonNullable(element)); } private void readMethod(Element element, TypeElement type, List localFields) { diff --git a/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java b/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java index bf8d37ce..98a88af6 100644 --- a/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java +++ b/validator-generator/src/test/java/io/avaje/validation/generator/models/valid/JSpecifyNotNull.java @@ -3,10 +3,8 @@ import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; -import io.avaje.validation.constraints.NotBlank; import jakarta.validation.Valid; @Valid @NullMarked -public record JSpecifyNotNull( - @NotBlank String basic, @NotBlank String withMax, @Nullable @NotBlank String withCustom) {} +public record JSpecifyNotNull(String basic, String withMax, @Nullable String withCustom) {} From eb8d198105d0aa9164a206f4f2f47f0704621918 Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Sat, 31 Aug 2024 14:25:42 -0400 Subject: [PATCH 3/4] no automatic nonnull for methods --- .../main/java/io/avaje/validation/generator/TypeReader.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java b/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java index da31fd58..d3a2d795 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/TypeReader.java @@ -107,7 +107,7 @@ private void readField(Element element, List localFields) { element = mixInField; } - if (includeField(element)) { + if (includeField(element) || Util.isNonNullable(element)) { seenFields.add(element.toString()); var reader = new FieldReader(element, genericTypeParams); if (reader.hasConstraints() || ValidPrism.isPresent(element)) { @@ -129,9 +129,7 @@ int genericTypeParamsCount() { private boolean includeField(Element element) { return !element.getModifiers().contains(Modifier.TRANSIENT) - && (!element.getAnnotationMirrors().isEmpty() - || element.asType().toString().contains("@") - || Util.isNonNullable(element)); + && (!element.getAnnotationMirrors().isEmpty() || element.asType().toString().contains("@")); } private void readMethod(Element element, TypeElement type, List localFields) { From aea687691c9589fde6c53510bffab0dc29f31ee7 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Sun, 1 Sep 2024 08:18:53 +1200 Subject: [PATCH 4/4] Format, remove unneeded public modifier --- .../validation/generator/ElementAnnotationContainer.java | 2 -- .../src/main/java/io/avaje/validation/generator/Util.java | 5 +---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java b/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java index ff656944..50570360 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/ElementAnnotationContainer.java @@ -77,7 +77,6 @@ static ElementAnnotationContainer create(Element element) { if (Util.isNonNullable(element)) { var nonNull = UType.parse(APContext.typeElement(NonNullPrism.PRISM_TYPE).asType()); - annotations.put(nonNull, "Map.of(\"message\",\"{avaje.NotNull.message}\")"); } @@ -129,7 +128,6 @@ static ElementAnnotationContainer create(VariableElement varElement) { if (Util.isNonNullable(varElement)) { var nonNull = UType.parse(APContext.typeElement(NonNullPrism.PRISM_TYPE).asType()); - annotations.put(nonNull, "Map.of(\"message\",\"{avaje.NotNull.message}\")"); } diff --git a/validator-generator/src/main/java/io/avaje/validation/generator/Util.java b/validator-generator/src/main/java/io/avaje/validation/generator/Util.java index aa16d688..218f555c 100644 --- a/validator-generator/src/main/java/io/avaje/validation/generator/Util.java +++ b/validator-generator/src/main/java/io/avaje/validation/generator/Util.java @@ -168,15 +168,13 @@ static String valhalla() { return ""; } - public static boolean isNonNullable(Element e) { + static boolean isNonNullable(Element e) { UType uType; if (e instanceof final ExecutableElement executableElement) { uType = UType.parse(executableElement.getReturnType()); - } else { uType = UType.parse(e.asType()); } - for (var mirror : uType.annotations()) { if (mirror.getAnnotationType().toString().endsWith("Nullable")) { return false; @@ -184,7 +182,6 @@ public static boolean isNonNullable(Element e) { return true; } } - return checkNullMarked(e); }