diff --git a/packages/ack_generator/CHANGELOG.md b/packages/ack_generator/CHANGELOG.md index e309cd59..2a24c4e5 100644 --- a/packages/ack_generator/CHANGELOG.md +++ b/packages/ack_generator/CHANGELOG.md @@ -12,6 +12,9 @@ * Diagnose value-model helper collisions, class/member name conflicts, and duplicate schema-first keys before emission. Resolve same-build models declared with annotated getters. +* Preserve application-generic argument nullability when selecting custom + schemas, and substitute nested generic bounds before validating type-owned + schema methods. * Allow built-in and contextual Dart keywords such as `required` as generated field, discriminator, and capture-member names without changing their JSON diff --git a/packages/ack_generator/lib/src/analyzer/custom_schema_resolver.dart b/packages/ack_generator/lib/src/analyzer/custom_schema_resolver.dart index 0b3c308d..def485cd 100644 --- a/packages/ack_generator/lib/src/analyzer/custom_schema_resolver.dart +++ b/packages/ack_generator/lib/src/analyzer/custom_schema_resolver.dart @@ -6,6 +6,7 @@ import 'package:source_gen/source_gen.dart'; import '../models/schema_model_graph.dart'; import '../utils/string_literal.dart'; +import '../utils/type_parameter_bounds.dart'; import 'generated_companion_visibility.dart'; import 'model_graph_builder_base.dart'; @@ -593,17 +594,14 @@ final class CustomSchemaResolver { element: field, ); } + final bounds = instantiateTypeParameterBounds( + method.typeParameters, + type.typeArguments, + ); for (var i = 0; i < method.typeParameters.length; i++) { final typeParam = method.typeParameters[i]; - final rawBound = typeParam.bound; - if (rawBound == null) continue; - final bound = - rawBound is TypeParameterType && - method.typeParameters.contains(rawBound.element) - ? type.typeArguments[method.typeParameters.indexOf( - rawBound.element, - )] - : rawBound; + final bound = bounds[i]; + if (bound == null) continue; final argument = type.typeArguments[i]; if (!_library.element.typeSystem.isSubtypeOf(argument, bound)) { throw InvalidGenerationSource( diff --git a/packages/ack_generator/lib/src/analyzer/model_graph_builder_base.dart b/packages/ack_generator/lib/src/analyzer/model_graph_builder_base.dart index 7b63ac0c..5f359646 100644 --- a/packages/ack_generator/lib/src/analyzer/model_graph_builder_base.dart +++ b/packages/ack_generator/lib/src/analyzer/model_graph_builder_base.dart @@ -241,8 +241,9 @@ abstract base class ModelGraphBuilderBase { /// Whether a schema producing [left] can back a field of type [right]. /// - /// The types must match apart from nullability: collection schemas such as - /// `Ack.map` type their values as nullable. + /// Outer nullability and converted core collection elements may differ: + /// `Ack.map` types its values as nullable. Application type arguments must + /// remain exact because generated code does not convert inside those values. bool sameTypeIgnoringNullability(DartType left, DartType right) { final typeSystem = library.element.typeSystem; final a = typeSystem.promoteToNonNull(left); @@ -252,6 +253,9 @@ abstract base class ModelGraphBuilderBase { a.typeArguments.length != b.typeArguments.length) { return false; } + if (!a.isDartCoreList && !a.isDartCoreSet && !a.isDartCoreMap) { + return typeSystem.isSubtypeOf(a, b) && typeSystem.isSubtypeOf(b, a); + } for (var i = 0; i < a.typeArguments.length; i++) { if (!sameTypeIgnoringNullability( a.typeArguments[i], diff --git a/packages/ack_generator/lib/src/utils/type_parameter_bounds.dart b/packages/ack_generator/lib/src/utils/type_parameter_bounds.dart new file mode 100644 index 00000000..ab3e1440 --- /dev/null +++ b/packages/ack_generator/lib/src/utils/type_parameter_bounds.dart @@ -0,0 +1,22 @@ +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +// The public TypeSystem API cannot substitute nested or F-bounded types. +// Keep this analyzer-13-specific dependency here while the package is pinned +// to >=13.0.0 <14.0.0, rather than maintaining a partial type substituter. +// ignore: implementation_imports +import 'package:analyzer/src/dart/element/type_algebra.dart' show Substitution; + +/// Substitutes all method type arguments into their parameter bounds. +List instantiateTypeParameterBounds( + List parameters, + List arguments, +) { + final substitution = Substitution.fromPairs2(parameters, arguments); + return [ + for (final parameter in parameters) + if (parameter.bound case final bound?) + substitution.substituteType(bound) + else + null, + ]; +} diff --git a/packages/ack_generator/test/integration/generic_schema_contract_test.dart b/packages/ack_generator/test/integration/generic_schema_contract_test.dart new file mode 100644 index 00000000..31cfc695 --- /dev/null +++ b/packages/ack_generator/test/integration/generic_schema_contract_test.dart @@ -0,0 +1,160 @@ +import 'package:ack_generator/src/builder.dart'; +import 'package:build/build.dart'; +import 'package:build_test/build_test.dart'; +import 'package:test/test.dart'; + +const _head = r''' +import 'package:ack/ack.dart'; +part 'models.g.dart'; +'''; + +Future _build( + String source, { + Matcher? output, + List errors = const [], +}) async { + final readerWriter = TestReaderWriter(rootPackage: 'test_pkg'); + await readerWriter.testing.loadIsolateSources(); + final actualErrors = []; + await testBuilder( + ackModelBuilder(BuilderOptions.empty), + {'test_pkg|lib/models.dart': '$_head\n$source'}, + generateFor: const {'test_pkg|lib/models.dart'}, + readerWriter: readerWriter, + outputs: output == null + ? const {} + : {'test_pkg|lib/models.g.dart': decodedMatches(output)}, + onLog: (log) { + if (log.level.name == 'SEVERE') actualErrors.add(log.message); + }, + ); + for (final error in errors) { + expect(actualErrors, contains(contains(error))); + } +} + +const _box = r''' +final class Box { + const Box(this.value); + final T value; +} +AckSchema> boxSchema() => Ack.string().codec>( + decode: Box.new, encode: (box) => box.value, +); +AckSchema> nullableBoxSchema() => + Ack.string().codec>( + decode: Box.new, encode: (box) => box.value!, + ); +'''; + +const _bounded = r''' +final class Bounded { + const Bounded(this.value); + final T value; + static AckSchema> schema>() => + Ack.string().codec>( + decode: (value) => Bounded(value as U), + encode: (bounded) => bounded.value.toString(), + ); +} +'''; + +void main() { + test( + 'does not register nullable application arguments for nonnullable fields', + () async { + await _build( + ''' +final class Box { + const Box(this.value); + final T value; +} +AckSchema> nullableBoxSchema() => + Ack.string().codec>( + decode: Box.new, encode: (box) => box.value!, + ); +@Schemable(schemas: [nullableBoxSchema]) +final class Holder with _\$HolderAck { + const Holder({required this.box}); + final Box box; +} +''', + errors: ['Holder.box', 'unsupported Box'], + ); + }, + ); + + test( + 'distinguishes registrations by application argument nullability', + () async { + await _build( + ''' +$_box +@Schemable(schemas: [boxSchema, nullableBoxSchema]) +final class Holder with _\$HolderAck { + const Holder({required this.box, required this.nullableBox}); + final Box box; + final Box nullableBox; +} +''', + output: allOf([ + contains("'box': boxSchema()"), + contains("'nullableBox': nullableBoxSchema()"), + ]), + ); + }, + ); + + test( + 'rejects a type-owned schema with different argument nullability', + () async { + await _build( + r''' +final class Box { + const Box(this.value); + final T value; + static AckSchema> schema() => Ack.string().codec>( + decode: (value) => Box(value as U?), + encode: (box) => box.value.toString(), + ); +} +@Schemable() +final class Holder with _$HolderAck { + const Holder({required this.box}); + final Box box; +} +''', + errors: [ + 'Holder.box', + 'produces Box', + 'field type is Box', + ], + ); + }, + ); + + test('substitutes nested F-bounds for valid type-owned schemas', () async { + await _build(''' +$_bounded +@Schemable() +final class Holder with _\$HolderAck { + const Holder({required this.box}); + final Bounded box; +} +''', output: contains('Bounded.schema()')); + }); + + test('reports the substituted bound for invalid type arguments', () async { + await _build( + ''' +$_bounded +@Schemable() +final class Holder with _\$HolderAck { + const Holder({required this.box}); + final Bounded box; +} +''', + errors: ['Holder.box', 'Comparable', 'received Object?'], + ); + }); +} diff --git a/packages/ack_generator/test/integration/type_owned_schema_build_test.dart b/packages/ack_generator/test/integration/type_owned_schema_build_test.dart index 6707ef07..ea9c19e1 100644 --- a/packages/ack_generator/test/integration/type_owned_schema_build_test.dart +++ b/packages/ack_generator/test/integration/type_owned_schema_build_test.dart @@ -86,6 +86,22 @@ final class Box { ) => value.codec>(decode: Box.new, encode: (box) => box.value); } +final class ComparableBox { + const ComparableBox(this.value); + final T value; + + static AckSchema> schema>() => + Ack.string().codec>( + decode: (value) => ComparableBox(value as U), + encode: (box) => box.value.toString(), + ); +} + +final class NullableBox { + const NullableBox(this.value); + final T value; +} + final class Trigger { const Trigger(this.name); @@ -151,6 +167,23 @@ final colorSchema = Ack.string().codec( encode: (color) => color.hex, ); +AckSchema> stringBoxSchema() => + Ack.string().codec>( + decode: NullableBox.new, encode: (box) => box.value, + ); +AckSchema> nullableStringBoxSchema() => + Ack.string().codec>( + decode: NullableBox.new, encode: (box) => box.value!, + ); + +@Schemable(schemas: [stringBoxSchema, nullableStringBoxSchema]) +final class GenericFields with _$GenericFieldsAck { + const GenericFields({required this.bounded, required this.strict, required this.loose}); + final ComparableBox bounded; + final NullableBox strict; + final NullableBox loose; +} + @Schemable() final class Habit with _$HabitAck { const Habit({required this.name}); @@ -240,6 +273,17 @@ import 'package:ack_type_owned/types.dart'; import 'package:test/test.dart'; void main() { + test('F-bounds and distinct generic nullability compile and round-trip', () { + final json = {'bounded': 'bound', 'strict': 'present', 'loose': 'nullable'}; + final fields = GenericFieldsSchema.parse(json); + expect(fields.bounded.value, 'bound'); + expect(fields.strict, isA>()); + expect(fields.loose, isA>()); + expect(fields.strict.value, 'present'); + expect(fields.loose.value, 'nullable'); + expect(fields.toJson(), json); + }); + test('class-first fields decode through their types', () { final json = { 'header': 'top',