Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-null constraints when in a Jspecify @NullMarked module/package #257

Merged
merged 5 commits into from
Aug 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions blackbox-test/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package example.avaje.jspecify;

import org.jspecify.annotations.Nullable;

import jakarta.validation.Valid;

@Valid
public record JSpecifyNotNull(String basic, String withMax, @Nullable String withCustom) {}
Original file line number Diff line number Diff line change
@@ -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) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package example.avaje.jspecify;

import static org.assertj.core.api.Assertions.assertThat;

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);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@org.jspecify.annotations.NullMarked
package example.avaje.jspecify;
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public record ElementAnnotationContainer(

static ElementAnnotationContainer create(Element element) {
final var hasValid = ValidPrism.isPresent(element);

Map<UType, String> typeUse1;
Map<UType, String> typeUse2;
final Map<UType, String> crossParam = new HashMap<>();
Expand Down Expand Up @@ -76,6 +75,11 @@ 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);
}
Expand All @@ -89,7 +93,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) {
Expand Down Expand Up @@ -122,6 +126,11 @@ 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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private void readField(Element element, List<FieldReader> 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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,29 @@ static String valhalla() {
return "";
}

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());
}
}
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package io.avaje.validation.generator.models.valid;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

import jakarta.validation.Valid;

@Valid
@NullMarked
public record JSpecifyNotNull(String basic, String withMax, @Nullable String withCustom) {}
Original file line number Diff line number Diff line change
@@ -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) {}
Loading