From 60261c45d08b7f1a60b74b0519f05b3f799f2239 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 26 Oct 2026 10:55:12 -0400 Subject: [PATCH] fix(ack): align wrapper and lazy nullability contracts --- packages/ack/CHANGELOG.md | 8 + .../ack_schema_model_builder.dart | 18 +- .../ack/lib/src/schemas/boundary_schema.dart | 4 +- packages/ack/lib/src/schemas/lazy_schema.dart | 4 + .../ack/lib/src/schemas/wrapper_schema.dart | 7 + .../nullability_export_contract_test.dart | 182 ++++++++++++++++++ 6 files changed, 219 insertions(+), 4 deletions(-) create mode 100644 packages/ack/test/schemas/nullability_export_contract_test.dart diff --git a/packages/ack/CHANGELOG.md b/packages/ack/CHANGELOG.md index 6a5b33c2..542cdbdc 100644 --- a/packages/ack/CHANGELOG.md +++ b/packages/ack/CHANGELOG.md @@ -1,3 +1,11 @@ +## Unreleased + +### Fixed + +* Honor explicit boundary-wrapper nullability consistently during parse and + encode. Preserve non-null restrictions on wrappers and lazy references when + exporting nullable primitives, unions, and imported JSON Schema resources. + ## 2.0.0-alpha.1 ### Breaking diff --git a/packages/ack/lib/src/schema_model/ack_schema_model_builder.dart b/packages/ack/lib/src/schema_model/ack_schema_model_builder.dart index a884814a..e9b731d3 100644 --- a/packages/ack/lib/src/schema_model/ack_schema_model_builder.dart +++ b/packages/ack/lib/src/schema_model/ack_schema_model_builder.dart @@ -106,9 +106,10 @@ final class _SchemaModelBuilder { AckSchemaModel _build(AckSchema schema) { if (schema is WrapperSchema) { final base = _build(schema.inner); - final layered = base + var layered = base .withDescription(schema.description ?? base.description) - .withNullable(schema.isNullable || base.nullable); + .withNullable(schema.isNullable); + if (schema.excludesInnerNull) layered = _excludeNull(layered); // `DefaultSchema.constraints` is a passthrough to `inner.constraints`, // which `_build(schema.inner)` already applied. Re-running them here // would emit duplicate warnings (e.g. datetime range under a default). @@ -553,11 +554,12 @@ final class _SchemaModelBuilder { } AckSchemaModel _lazyRef(LazySchema schema) { - var model = AckRefSchemaModel( + AckSchemaModel model = AckRefSchemaModel( refName: schema.name, description: schema.description, nullable: schema.isNullable, ); + if (schema.excludesTargetNull) model = _excludeNull(model); final constraintCount = schema.runtimeConstraintCount; final refinementCount = schema.runtimeRefinementCount; if (constraintCount == 0 && refinementCount == 0) { @@ -602,6 +604,16 @@ final class _SchemaModelBuilder { } } +AckSchemaModel _excludeNull(AckSchemaModel model) => model.withExtensions({ + ...model.extensions, + 'allOf': [ + ...?model.extensions['allOf'] as List?, + const { + 'not': {'type': 'null'}, + }, + ], +}); + AckSchemaModel _applyConstraints( AckSchemaModel model, AckSchema schema, { diff --git a/packages/ack/lib/src/schemas/boundary_schema.dart b/packages/ack/lib/src/schemas/boundary_schema.dart index 0d545f24..8775e33e 100644 --- a/packages/ack/lib/src/schemas/boundary_schema.dart +++ b/packages/ack/lib/src/schemas/boundary_schema.dart @@ -31,9 +31,11 @@ final class BoundarySchema Object? value, SchemaContext context, ) { + final nullResult = handleNullInput(value, context); + if (nullResult != null) return nullResult; + final result = inner.parseWithContext(value, context); if (result.isFail) return SchemaResult.fail(result.getError()); - if (value == null) return SchemaResult.ok(null); final Boundary boundary; final normalizedMap = jsonMapOrNull(value); diff --git a/packages/ack/lib/src/schemas/lazy_schema.dart b/packages/ack/lib/src/schemas/lazy_schema.dart index 094f5877..594aee9a 100644 --- a/packages/ack/lib/src/schemas/lazy_schema.dart +++ b/packages/ack/lib/src/schemas/lazy_schema.dart @@ -61,6 +61,10 @@ final class LazySchema @internal AckSchema get target => _target; + /// Whether the lazy reference's own null policy narrows its target. + @internal + bool get excludesTargetNull => !acceptsNull && _target.acceptsNull; + /// Count of runtime-only constraints, including the always-present /// max-depth check that every `Ack.lazy` schema enforces. @internal diff --git a/packages/ack/lib/src/schemas/wrapper_schema.dart b/packages/ack/lib/src/schemas/wrapper_schema.dart index 469ee8c7..45b169b6 100644 --- a/packages/ack/lib/src/schemas/wrapper_schema.dart +++ b/packages/ack/lib/src/schemas/wrapper_schema.dart @@ -25,6 +25,13 @@ mixin WrapperSchema< /// The wrapped schema used for boundary-shape traversal. AnyAckSchema get inner; + /// Whether this wrapper rejects null that its inner schema would accept. + /// + /// Exporters must preserve this restriction even when null acceptance is + /// embedded in a union or an imported resource rather than a nullable flag. + @internal + bool get excludesInnerNull => !acceptsNull && inner.acceptsNull; + /// Returns a copy of this wrapper with [inner] swapped for [newInner]. /// /// Used by traversal utilities (e.g. discriminated-branch synthesis) that diff --git a/packages/ack/test/schemas/nullability_export_contract_test.dart b/packages/ack/test/schemas/nullability_export_contract_test.dart new file mode 100644 index 00000000..91010abc --- /dev/null +++ b/packages/ack/test/schemas/nullability_export_contract_test.dart @@ -0,0 +1,182 @@ +import 'package:ack/ack.dart'; +import 'package:test/test.dart'; + +AnyOfSchema _nullableUnion() => Ack.anyOf([ + Ack.anyOf([Ack.string().nullable(), Ack.integer()]), + Ack.boolean(), +]); + +AckSchema _nullableImport() => Ack.fromJsonSchema({ + 'anyOf': [ + {'type': 'string'}, + {'type': 'null'}, + ], + 'allOf': [ + {'minLength': 2}, + ], +}); + +void main() { + final cases = + <({String name, AckSchema schema, bool acceptsNull})>[ + ( + name: 'boundary widens a non-null primitive', + schema: Ack.preserveBoundary(Ack.string()).nullable(), + acceptsNull: true, + ), + ( + name: 'boundary narrows a nullable primitive', + schema: Ack.preserveBoundary( + Ack.string().nullable(), + ).nullable(value: false), + acceptsNull: false, + ), + ( + name: 'boundary narrows a nested nullable union', + schema: Ack.preserveBoundary(_nullableUnion()).nullable(value: false), + acceptsNull: false, + ), + ( + name: 'boundary narrows imported source nullability', + schema: Ack.preserveBoundary( + _nullableImport(), + ).nullable(value: false), + acceptsNull: false, + ), + ( + name: 'codec narrows a nullable primitive', + schema: Ack.string() + .nullable() + .codec(decode: (value) => value, encode: (value) => value) + .nullable(value: false), + acceptsNull: false, + ), + ( + name: 'codec narrows a nested nullable union', + schema: _nullableUnion() + .codec(decode: (value) => value, encode: (value) => value) + .nullable(value: false), + acceptsNull: false, + ), + ( + name: 'codec narrows imported source nullability', + schema: _nullableImport() + .codec(decode: (value) => value, encode: (value) => value) + .nullable(value: false), + acceptsNull: false, + ), + ( + name: 'lazy uses its non-null flag over a nullable primitive', + schema: Ack.lazy('NullableString', () => Ack.string().nullable()), + acceptsNull: false, + ), + ( + name: 'lazy narrows a nested nullable union', + schema: Ack.lazy( + 'NullableUnion', + _nullableUnion, + ).nullable().nullable(value: false), + acceptsNull: false, + ), + ( + name: 'lazy uses its non-null flag over imported source nullability', + schema: Ack.lazy('NullableImport', _nullableImport), + acceptsNull: false, + ), + ( + name: 'lazy widens a non-null target', + schema: Ack.lazy('NonNullString', Ack.string).nullable(), + acceptsNull: true, + ), + ]; + + for (final testCase in cases) { + group(testCase.name, () { + final schema = testCase.schema; + final nested = Ack.object({'value': schema}); + + test('parse and encode honor the same root and property null policy', () { + expect(schema.safeParse(null).isOk, testCase.acceptsNull); + expect(schema.safeEncode(null).isOk, testCase.acceptsNull); + expect(nested.safeParse({'value': null}).isOk, testCase.acceptsNull); + expect(nested.safeEncode({'value': null}).isOk, testCase.acceptsNull); + expect(schema.parse('ok'), 'ok'); + expect(schema.encode('ok'), 'ok'); + }); + + for (final draft7 in [false, true]) { + test( + '${draft7 ? 'Draft-7' : 'default'} export preserves null policy', + () { + final restored = Ack.fromJsonSchema( + draft7 ? schema.toJsonSchemaDraft7() : schema.toJsonSchema(), + ); + final restoredNested = Ack.fromJsonSchema( + draft7 ? nested.toJsonSchemaDraft7() : nested.toJsonSchema(), + ); + expect(restored.safeParse(null).isOk, testCase.acceptsNull); + expect( + restoredNested.safeParse({'value': null}).isOk, + testCase.acceptsNull, + ); + expect(restored.safeParse('ok').isOk, isTrue); + expect(restoredNested.safeParse({'value': 'ok'}).isOk, isTrue); + }, + ); + } + }); + } + + test('a non-null restriction keeps imported source allOf assertions', () { + final schema = Ack.preserveBoundary( + _nullableImport(), + ).nullable(value: false); + for (final exported in [ + schema.toJsonSchema(), + schema.toJsonSchemaDraft7(), + ]) { + final restored = Ack.fromJsonSchema(exported); + expect(restored.safeParse('x').isFail, isTrue); + expect(restored.safeParse(1).isFail, isTrue); + expect(restored.safeParse('ok').isOk, isTrue); + } + }); + + group('default null semantics', () { + test('nullable defaults still substitute only on parse', () { + final schema = Ack.string().nullable().withDefault('fallback'); + expect(schema.parse(null), 'fallback'); + expect(schema.encode(null), isNull); + expect( + Ack.fromJsonSchema(schema.toJsonSchema()).safeParse(null).isOk, + isTrue, + ); + }); + + test('defaults over nullable unions retain effective null acceptance', () { + final schema = _nullableUnion().withDefault('fallback'); + expect(schema.isNullable, isFalse); + expect(schema.parse(null), 'fallback'); + expect(schema.encode(null), isNull); + expect( + Ack.fromJsonSchema(schema.toJsonSchema()).safeParse(null).isOk, + isTrue, + ); + }); + + test( + 'a default outside a narrowed boundary still substitutes on parse', + () { + final schema = Ack.preserveBoundary( + _nullableUnion(), + ).nullable(value: false).withDefault('fallback'); + expect(schema.parse(null), 'fallback'); + expect(schema.safeEncode(null).isFail, isTrue); + expect( + Ack.fromJsonSchema(schema.toJsonSchema()).safeParse(null).isFail, + isTrue, + ); + }, + ); + }); +}