Refactor and optimize validation logic for improved error handling

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionChecker.java

@@ -143,6 +143,11 @@ public static ValidationResult checkSchema(
             final Type<?> type = schema.types().get(ref);
             if (type != null) {
                 checkFieldNames(type, typeName, schemaLocation, rules, result);
+            } else {
+                result.add(ValidationIssue.warning(
+                        CONVENTION_TYPE_NAME,
+                        "Type '" + typeName + "' is registered but resolves to null"
+                ).at(schemaLocation + "/" + typeName));
             }
         }
 

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionRules.java

@@ -80,8 +80,8 @@ public final class ConventionRules {
      * </ul>
      */
     public static final ConventionRules STRICT = builder()
-            .typeNamePattern(Pattern.compile("^[a-z][a-z0-9_]*$"))
-            .fieldNamePattern(Pattern.compile("^[a-z][a-z0-9_]*$"))
+            .typeNamePattern(Pattern.compile("^[a-z_][a-z0-9_]*$"))
+            .fieldNamePattern(Pattern.compile("^[a-z_][a-z0-9_]*$"))
             .schemaClassPrefix("Schema")
             .fixClassSuffix("Fix")
             .treatViolationsAsErrors(true)
@@ -249,11 +249,7 @@ public boolean isValidTypeName(@NotNull final String typeName) {
         }
 
         // Check custom validator
-        if (this.customTypeValidator != null && !this.customTypeValidator.test(typeName)) {
-            return false;
-        }
-
-        return true;
+        return this.customTypeValidator == null || this.customTypeValidator.test(typeName);
     }
 
     /**
@@ -275,11 +271,7 @@ public boolean isValidFieldName(@NotNull final String fieldName) {
         }
 
         // Check custom validator
-        if (this.customFieldValidator != null && !this.customFieldValidator.test(fieldName)) {
-            return false;
-        }
-
-        return true;
+        return this.customFieldValidator == null || this.customFieldValidator.test(fieldName);
     }
 
     /**
@@ -304,11 +296,7 @@ public boolean isValidSchemaClassName(@NotNull final String className) {
         }
 
         // Check suffix
-        if (this.schemaClassSuffix != null && !className.endsWith(this.schemaClassSuffix)) {
-            return false;
-        }
-
-        return true;
+        return this.schemaClassSuffix == null || className.endsWith(this.schemaClassSuffix);
     }
 
     /**
@@ -333,11 +321,7 @@ public boolean isValidFixClassName(@NotNull final String className) {
         }
 
         // Check suffix
-        if (this.fixClassSuffix != null && !className.endsWith(this.fixClassSuffix)) {
-            return false;
-        }
-
-        return true;
+        return this.fixClassSuffix == null || className.endsWith(this.fixClassSuffix);
     }
 
     /**

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/SchemaValidator.java

@@ -190,9 +190,14 @@ public static SchemaValidator forBootstrap(@NotNull final DataFixerBootstrap boo
 
         // Bootstrap into capturing registries
         final SchemaRegistry schemaRegistry = new SimpleSchemaRegistry();
-        final DataFixerBuilder fixerBuilder = new DataFixerBuilder(new DataVersion(Integer.MAX_VALUE));
-
         bootstrap.registerSchemas(schemaRegistry);
+
+        final int maxVersion = schemaRegistry.stream()
+                .mapToInt(s -> s.version().getVersion())
+                .max()
+                .orElse(0);
+
+        final DataFixerBuilder fixerBuilder = new DataFixerBuilder(new DataVersion(maxVersion));
         bootstrap.registerFixes(fixerBuilder);
 
         return new SchemaValidator(null, schemaRegistry, fixerBuilder);
@@ -390,11 +395,11 @@ private ValidationResult validateFixCoverageInternal() {
         final int minVersion = schemas.stream()
                 .map(s -> s.version().getVersion())
                 .min(Comparator.naturalOrder())
-                .orElse(0);
+                .orElseThrow();
         final int maxVersion = schemas.stream()
                 .map(s -> s.version().getVersion())
                 .max(Comparator.naturalOrder())
-                .orElse(0);
+                .orElseThrow();
 
         if (minVersion == maxVersion) {
             return ValidationResult.empty();

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/StructureValidator.java

@@ -117,6 +117,16 @@ public static ValidationResult validate(
 
         // Check parent chain
         if (schema.parent() != null) {
+            // Check that parent exists in the registry if one is provided
+            if (registry != null) {
+                final int parentVersion = schema.parent().version().getVersion();
+                if (registry.get(new de.splatgames.aether.datafixers.api.DataVersion(parentVersion)) == null) {
+                    result.add(ValidationIssue.error(STRUCTURE_MISSING_PARENT,
+                                    "Parent schema v" + parentVersion + " not found in registry")
+                            .at(location)
+                            .withContext("parentVersion", parentVersion));
+                }
+            }
             validateParentChain(schema, registry, result, location);
         }
 

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DataResultAssert.java

@@ -194,7 +194,7 @@ public DataResultAssert<A> hasValue(@NotNull final A expected) {
     @NotNull
     public DataResultAssert<A> hasValueSatisfying(@NotNull final Consumer<A> requirements) {
         this.isSuccess();
-        requirements.accept(this.actual.result().orElse(null));
+        requirements.accept(this.actual.result().orElseThrow());
         return this;
     }
 

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DynamicAssert.java

@@ -616,11 +616,22 @@ public DynamicAssert<T> isNotEmpty() {
     public DynamicAssert<T> containsStringValues(@NotNull final String... expected) {
         isNotNull();
         this.isList();
-        final List<String> actualValues = this.actual.asListStream()
+        final List<Dynamic<T>> elements = this.actual.asListStream()
                 .result()
-                .map(s -> s.map(d -> d.asString().orElse(null)).collect(Collectors.toList()))
+                .map(s -> s.collect(Collectors.toList()))
                 .orElse(List.of());
 
+        final List<String> actualValues = new java.util.ArrayList<>();
+        for (int i = 0; i < elements.size(); i++) {
+            final Dynamic<T> element = elements.get(i);
+            final String value = element.asString().orElse(null);
+            if (value == null && !element.isString()) {
+                failWithMessage("Expected%s element [%d] to be a string but was: %s",
+                        this.pathInfo(), i, this.describeElement(element));
+            }
+            actualValues.add(value);
+        }
+
         for (final String exp : expected) {
             if (!actualValues.contains(exp)) {
                 failWithMessage("Expected%s to contain '%s' but values were: %s",
@@ -640,11 +651,22 @@ public DynamicAssert<T> containsStringValues(@NotNull final String... expected)
     public DynamicAssert<T> containsIntValues(final int... expected) {
         isNotNull();
         this.isList();
-        final List<Integer> actualValues = this.actual.asListStream()
+        final List<Dynamic<T>> elements = this.actual.asListStream()
                 .result()
-                .map(s -> s.map(d -> d.asInt().orElse(null)).collect(Collectors.toList()))
+                .map(s -> s.collect(Collectors.toList()))
                 .orElse(List.of());
 
+        final List<Integer> actualValues = new java.util.ArrayList<>();
+        for (int i = 0; i < elements.size(); i++) {
+            final Dynamic<T> element = elements.get(i);
+            final Integer value = element.asInt().orElse(null);
+            if (value == null && !element.isNumber()) {
+                failWithMessage("Expected%s element [%d] to be an integer but was: %s",
+                        this.pathInfo(), i, this.describeElement(element));
+            }
+            actualValues.add(value);
+        }
+
         for (final int exp : expected) {
             if (!actualValues.contains(exp)) {
                 failWithMessage("Expected%s to contain %d but values were: %s",
@@ -725,6 +747,15 @@ private String availableFields() {
         return fields.isEmpty() ? "(none)" : String.join(", ", fields);
     }
 
+    private String describeElement(final Dynamic<T> element) {
+        if (element.isString()) return "string: " + element.asString().orElse("?");
+        if (element.isNumber()) return "number: " + element.asNumber().orElse(null);
+        if (element.isBoolean()) return "boolean: " + element.asBoolean().orElse(null);
+        if (element.isMap()) return "map";
+        if (element.isList()) return "list";
+        return "unknown: " + element.value();
+    }
+
     private String fieldsOf(final Dynamic<T> d) {
         return d.asMapStream()
                 .result()

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/AssertingContext.java

@@ -181,11 +181,15 @@ private static String formatMessage(final String message, final Object[] args) {
         if (args == null || args.length == 0) {
             return message;
         }
-        String result = message;
+        final StringBuilder result = new StringBuilder(message);
         for (final Object arg : args) {
-            result = result.replaceFirst("\\{}", String.valueOf(arg));
+            final int idx = result.indexOf("{}");
+            if (idx < 0) {
+                break;
+            }
+            result.replace(idx, idx + 2, String.valueOf(arg));
         }
-        return result;
+        return result.toString();
     }
 
     private enum Mode {

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/RecordingContext.java

@@ -266,11 +266,15 @@ public String formattedMessage() {
             if (this.args == null || this.args.length == 0) {
                 return this.message;
             }
-            String result = this.message;
+            final StringBuilder result = new StringBuilder(this.message);
             for (final Object arg : this.args) {
-                result = result.replaceFirst("\\{}", String.valueOf(arg));
+                final int idx = result.indexOf("{}");
+                if (idx < 0) {
+                    break;
+                }
+                result.replace(idx, idx + 2, String.valueOf(arg));
             }
-            return result;
+            return result.toString();
         }
 
         @Override

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/DataFixTester.java

@@ -223,11 +223,7 @@ public DataFixTester<T> expectOutput(@NotNull final Dynamic<T> expected) {
     public Dynamic<T> apply() {
         this.validateConfiguration();
 
-        final DataFixerContext effectiveContext = this.useRecordingContext
-                ? new RecordingContext()
-                : this.context;
-
-        return this.fix.apply(this.typeReference, this.input, effectiveContext);
+        return this.fix.apply(this.typeReference, this.input, this.resolveContext());
     }
 
     /**
@@ -241,19 +237,8 @@ public Dynamic<T> apply() {
     public DataFixVerification<T> verify() {
         this.validateConfiguration();
 
-        final RecordingContext recordingContext;
-        final DataFixerContext effectiveContext;
-
-        if (this.useRecordingContext) {
-            recordingContext = new RecordingContext();
-            effectiveContext = recordingContext;
-        } else if (this.context instanceof RecordingContext rc) {
-            recordingContext = rc;
-            effectiveContext = rc;
-        } else {
-            recordingContext = null;
-            effectiveContext = this.context;
-        }
+        final DataFixerContext effectiveContext = this.resolveContext();
+        final RecordingContext recordingContext = effectiveContext instanceof RecordingContext rc ? rc : null;
 
         final Dynamic<T> result = this.fix.apply(this.typeReference, this.input, effectiveContext);
 
@@ -272,6 +257,20 @@ public DataFixVerification<T> verify() {
         return new DataFixVerification<>(result, recordingContext, true);
     }
 
+    // ==================== Internal ====================
+
+    /**
+     * Resolves the effective context, ensuring a single instance is used
+     * across both apply() and verify().
+     */
+    @NotNull
+    private DataFixerContext resolveContext() {
+        if (this.useRecordingContext) {
+            return new RecordingContext();
+        }
+        return this.context;
+    }
+
     // ==================== Validation ====================
 
     private void validateConfiguration() {

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/MigrationTester.java

@@ -31,6 +31,9 @@
 import de.splatgames.aether.datafixers.core.fix.DataFixerBuilder;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.function.Consumer;
 
@@ -298,12 +301,10 @@ private void validateConfiguration() {
      */
     public static final class FixerSetup {
 
-        private final DataFixerBuilder builder;
+        private final List<Map.Entry<TypeReference, DataFix<?>>> pendingFixes = new ArrayList<>();
         private int maxVersion = 1;
 
         FixerSetup() {
-            // Start with a placeholder version, will be updated
-            this.builder = new DataFixerBuilder(new DataVersion(Integer.MAX_VALUE));
         }
 
         /**
@@ -317,7 +318,7 @@ public static final class FixerSetup {
         public FixerSetup addFix(@NotNull final TypeReference type, @NotNull final DataFix<?> fix) {
             Preconditions.checkNotNull(type, "type must not be null");
             Preconditions.checkNotNull(fix, "fix must not be null");
-            this.builder.addFix(type, fix);
+            this.pendingFixes.add(Map.entry(type, fix));
             this.maxVersion = Math.max(this.maxVersion, fix.toVersion().getVersion());
             return this;
         }
@@ -337,7 +338,11 @@ public FixerSetup addFix(@NotNull final String typeId, @NotNull final DataFix<?>
         }
 
         DataFixer build() {
-            return this.builder.build();
+            final DataFixerBuilder actualBuilder = new DataFixerBuilder(new DataVersion(this.maxVersion));
+            for (final Map.Entry<TypeReference, DataFix<?>> entry : this.pendingFixes) {
+                actualBuilder.addFix(entry.getKey(), entry.getValue());
+            }
+            return actualBuilder.build();
         }
     }
 }

aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/SchemaTester.java

@@ -292,6 +292,10 @@ public SchemaTester hasParent() {
      */
     @NotNull
     public SchemaTester hasNoParent() {
+        if (this.expectedParent != null) {
+            throw new IllegalStateException(
+                    "Contradictory configuration: hasNoParent() called after inheritsFrom()");
+        }
         this.expectHasParent = false;
         return this;
     }
@@ -306,6 +310,10 @@ public SchemaTester hasNoParent() {
     @NotNull
     public SchemaTester inheritsFrom(@NotNull final Schema parent) {
         Preconditions.checkNotNull(parent, "parent must not be null");
+        if (Boolean.FALSE.equals(this.expectHasParent)) {
+            throw new IllegalStateException(
+                    "Contradictory configuration: inheritsFrom() called after hasNoParent()");
+        }
         this.expectedParent = parent;
         this.expectHasParent = true;
         return this;
@@ -369,7 +377,8 @@ public SchemaTester verify() {
 
         // Validate parent existence
         if (this.expectHasParent != null) {
-            final boolean hasParent = this.schema.parent() != null;
+            final Schema actualParentRef = this.schema.parent();
+            final boolean hasParent = actualParentRef != null;
             if (this.expectHasParent && !hasParent) {
                 throw new AssertionError(String.format(
                         "Schema v%d has no parent, but one was expected",
@@ -380,7 +389,7 @@ public SchemaTester verify() {
                 throw new AssertionError(String.format(
                         "Schema v%d has a parent (v%d), but none was expected",
                         this.schema.version().getVersion(),
-                        this.schema.parent().version().getVersion()
+                        actualParentRef.version().getVersion()
                 ));
             }
         }