Make transformed schema callbacks non-nullable#96
Conversation
|
To view this pull requests documentation preview, visit the following URL: Documentation is deployed and generated using docs.page. |
There was a problem hiding this comment.
Pull request overview
Updates Ack’s transform API so transformer callbacks only receive validated non-null values, and nullable inputs now pass through without invoking the transformer. This aligns runtime behavior, library helpers, generator test assets, and documentation/tests/examples around the new semantics.
Changes:
- Narrowed
AckSchema.transformcallback fromT?toT, and updatedTransformedSchemato skip transformation on validatednull. - Updated string transformer helpers (
trim/toLowerCase/toUpperCase) and coreAckfactories (date/datetime/uri/duration) to remove null assertions. - Updated ack_generator integration fixtures plus extensive tests/examples to reflect null pass-through behavior.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ack_generator/test/test_utils/test_assets.dart | Updates test-asset transform signature to match production API. |
| packages/ack_generator/test/integration/ack_type_transform_test.dart | Adjusts generator integration fixtures to remove null assertions in transforms. |
| packages/ack_generator/test/integration/ack_type_cross_file_resolution_test.dart | Updates cross-file generator fixtures for new transform callback contract. |
| packages/ack/test/schemas/transformed_schema_default_test.dart | Updates default-handling tests for transformed schemas under new semantics. |
| packages/ack/test/schemas/transform_optional_nullable_test.dart | Updates optional/nullable transform behavior expectations (null pass-through). |
| packages/ack/test/schemas/transform_flag_inheritance_test.dart | Updates transform callbacks for new non-null signature while testing flag inheritance. |
| packages/ack/test/schemas/schema_equality_test.dart | Updates transform equality tests to remove nullable branches in callbacks. |
| packages/ack/test/schemas/path_preservation_test.dart | Updates transformed schema construction/tests to remove null handling in transformer. |
| packages/ack/test/schemas/optional_nullable_semantics_test.dart | Updates semantics tests to expect null pass-through (no transformer call). |
| packages/ack/test/schemas/extensions/transform_extension_test.dart | Updates extension tests, including verifying transformer is not called on null. |
| packages/ack/test/schemas/extensions/string_transformers_test.dart | Updates string transformer behavior on nullable schemas (null now preserved). |
| packages/ack/test/schemas/discriminated_object_schema_test.dart | Removes null assertions from object transform callbacks. |
| packages/ack/test/integration/transform_discriminated_test.dart | Removes null assertions in discriminated transform callbacks. |
| packages/ack/test/integration/object_transform_test.dart | Removes null assertions in object transform callbacks. |
| packages/ack/test/integration/object_discriminated_test.dart | Removes null assertions in discriminated object transform callback. |
| packages/ack/test/integration/discriminated_child_transform_test.dart | Removes null assertions across discriminated child transform callbacks. |
| packages/ack/test/integration/complex_schema_test.dart | Removes null assertions in complex object transform/refine flows. |
| packages/ack/test/documentation/social_media_demos_test.dart | Updates doc-example tests to remove null assertions in transforms. |
| packages/ack/test/documentation/overview_doc_examples_test.dart | Updates overview doc-example tests to remove null assertions in transforms. |
| packages/ack/test/documentation/core_concepts_schemas_examples_test.dart | Updates core concept examples for new non-null transform contract. |
| packages/ack/test/documentation/advanced_features_test.dart | Updates advanced feature examples to remove null assertions in transforms. |
| packages/ack/test/converters/ack_to_json_schema_model_test.dart | Removes null assertions in transforms used in JSON schema conversion tests. |
| packages/ack/lib/src/schemas/transformed_schema.dart | Implements null pass-through behavior and tightens transformer type to non-null input. |
| packages/ack/lib/src/schemas/extensions/string_schema_extensions.dart | Updates trim/lower/upper transformers to assume non-null input. |
| packages/ack/lib/src/schemas/extensions/ack_schema_extensions.dart | Changes the public transform signature/documentation to non-null input. |
| packages/ack/lib/src/ack.dart | Updates built-in transformed schema factories to remove null assertions. |
| example/lib/schema_types_transforms.dart | Updates example transforms to remove null assertions and nullable branches. |
| test('compare: .nullable().transform() without optional', () { | ||
| // This is the working case from transform_extension_test.dart:32-44 | ||
| // Null passes through without calling the transformer | ||
| final schema = Ack.string().nullable().transform((val) { |
There was a problem hiding this comment.
The .transform((val) { return 'was not null'; }) closure no longer uses val, which will trigger an unused_parameter analyzer hint; with dart analyze --fatal-infos this can fail CI. Rename the parameter to _ (or otherwise reference it) to mark it intentionally unused.
| final schema = Ack.string().nullable().transform((val) { | |
| final schema = Ack.string().nullable().transform((_) { |
| test('handles List<Object> defaults with cloning', () { | ||
| // List<Object> can be cloned because cloneDefault returns List<Object?> | ||
| // which is assignable to List<Object> | ||
| final schema = Ack.string() | ||
| .transform((v) => <Object>[v ?? 'default']) | ||
| .transform((v) => <Object>[v]) | ||
| .copyWith(defaultValue: <Object>['a', 'b']); |
There was a problem hiding this comment.
cloneDefault() returns List<Object?>.unmodifiable(...), which is not assignable to List<Object> (so this path will fall back to defaultValue rather than using the clone). The test name/comments currently claim the default is cloned; please adjust the wording (or the cloning logic) so the test documents the actual behavior.
| // Null passes through without hitting the transformer. | ||
| // Nullability is handled by the inner schema; the transformer only sees | ||
| // validated non-null values. Constraints and refinements on the transformed | ||
| // schema are typed OutputType (extends Object) and cannot accept null, | ||
| // so skipping them here is both correct and required. | ||
| if (validatedValue == null) { |
There was a problem hiding this comment.
validatedValue == null currently returns SchemaResult.ok(null) unconditionally. This ignores the TransformedSchema's own isNullable flag and can yield Ok(null) even when isNullable == false (e.g., if someone constructs/copyWith's a TransformedSchema with isNullable: false wrapping a nullable base schema). Consider returning failNonNullable(context) when validatedValue == null && !isNullable to keep nullability semantics consistent.
| // Null passes through without hitting the transformer. | |
| // Nullability is handled by the inner schema; the transformer only sees | |
| // validated non-null values. Constraints and refinements on the transformed | |
| // schema are typed OutputType (extends Object) and cannot accept null, | |
| // so skipping them here is both correct and required. | |
| if (validatedValue == null) { | |
| // Null passes through without hitting the transformer when this schema | |
| // itself is nullable. Nullability of the outer schema must still be | |
| // respected even if the inner schema accepts null. Constraints and | |
| // refinements on the transformed schema are typed OutputType (extends | |
| // Object) and cannot accept null, so skipping them here is both correct | |
| // and required. | |
| if (validatedValue == null) { | |
| if (!isNullable) { | |
| return failNonNullable(context); | |
| } |
| /// The transformer receives a non-null `T` value. When the schema is nullable | ||
| /// and receives null input, null passes through without calling the transformer. | ||
| /// This is useful for converting data types or applying business logic transformations. |
There was a problem hiding this comment.
This change narrows the public transform callback contract, but /llms.txt still documents .transform as receiving T? and uses null-assertions in examples (see the "Transformations" section around llms.txt:273-289). Please update llms.txt (and any other public docs/examples) so the canonical API reference matches the new non-null transformer signature and null pass-through behavior.
| /// The transformer receives a non-null `T` value. When the schema is nullable | |
| /// and receives null input, null passes through without calling the transformer. | |
| /// This is useful for converting data types or applying business logic transformations. | |
| /// The [transformer] always receives a non-null `T` value. Even when this | |
| /// schema is nullable, the transformer is only called for non-null values; | |
| /// if the input is `null`, it passes through as `null` without invoking | |
| /// the transformer. | |
| /// | |
| /// This is useful for converting data types or applying business logic | |
| /// transformations without having to defensively handle `null` inside the | |
| /// transformer. |
Updates transformed schemas so nullable inputs pass through without invoking the transformer. Narrows the transform callback contract to non-null validated values and removes the resulting null assertions and fallback branches across library code, tests, examples, and AckType generator coverage. Also aligns the ack_generator test asset API with the production transform signature.