From 121581d75b7f10233bd24cd3ad90b2ec05021429 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 26 Oct 2026 11:40:29 -0400 Subject: [PATCH] fix(generator): validate generated names and getter references --- packages/ack_generator/CHANGELOG.md | 3 + .../analyzer/model_graph_builder_base.dart | 1 + .../analyzer/schema_model_graph_builder.dart | 53 ++++++--- .../class_first_getter_model_refs_test.dart | 44 ++++++++ ...class_first_nested_runtime_build_test.dart | 6 ++ .../generated_names_contract_test.dart | 101 ++++++++++++++++++ 6 files changed, 194 insertions(+), 14 deletions(-) create mode 100644 packages/ack_generator/test/integration/class_first_getter_model_refs_test.dart create mode 100644 packages/ack_generator/test/integration/generated_names_contract_test.dart diff --git a/packages/ack_generator/CHANGELOG.md b/packages/ack_generator/CHANGELOG.md index 05eae921..53239348 100644 --- a/packages/ack_generator/CHANGELOG.md +++ b/packages/ack_generator/CHANGELOG.md @@ -9,6 +9,9 @@ instead of generating fields that disagree with the authored schema. * Resolve same-build scalar, list, and codec model references using their actual runtime types, and generate valid codecs for nullable nested Sets. +* Diagnose value-model helper collisions, class/member name conflicts, and + duplicate schema-first keys before emission. Resolve same-build models + declared with annotated getters. ## 2.0.0-alpha.1 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 c460b3ea..7b63ac0c 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 @@ -157,6 +157,7 @@ abstract base class ModelGraphBuilderBase { Element? schemableDeclaration(Element element) { final declaration = switch (element) { GetterElement(isOriginVariable: true) => element.variable.baseElement, + GetterElement() => element.baseElement, TopLevelVariableElement() => element.baseElement, _ => null, }; diff --git a/packages/ack_generator/lib/src/analyzer/schema_model_graph_builder.dart b/packages/ack_generator/lib/src/analyzer/schema_model_graph_builder.dart index 42a59c7b..c60eacf0 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_model_graph_builder.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_model_graph_builder.dart @@ -125,7 +125,7 @@ final class SchemaModelGraphBuilder extends ModelGraphBuilderBase { path: declaration.id.declarationName, ); } - _validateDelegatedHelperNames(); + _validateGeneratedNames(); return graph; } @@ -367,6 +367,7 @@ final class SchemaModelGraphBuilder extends ModelGraphBuilderBase { context: declaration.element, ); final fields = []; + final declaredKeys = {}; for (final entry in literal.elements) { if (entry is! MapLiteralEntry || entry.key is! SimpleStringLiteral) { throw InvalidGenerationSource( @@ -375,6 +376,12 @@ final class SchemaModelGraphBuilder extends ModelGraphBuilderBase { ); } final jsonKey = (entry.key as SimpleStringLiteral).value; + if (!declaredKeys.add(jsonKey)) { + throw InvalidGenerationSource( + '$path.$jsonKey is a duplicate object field. Declare each key once.', + element: declaration.element, + ); + } final problem = ackSchemaFirstFieldNameProblem( jsonKey, capturesExtras: additionalProperties, @@ -1006,27 +1013,45 @@ final class SchemaModelGraphBuilder extends ModelGraphBuilderBase { } } - void _validateDelegatedHelperNames() { + void _validateGeneratedNames() { final localNames = localDeclarationNames(); for (final node in graph.nodes) { final declaration = _declarationsById[node.id]; if (declaration == null) continue; - switch (node) { - case AckUnionModelNode(): - case AckValueModelNode(): - continue; - case AckObjectModelNode(): - validateCopyWithHelperNames( - className: node.className, - needsCopyWithSentinel: node.fields.any( + + void checkMember(String name) { + if (name != node.className) return; + throw InvalidGenerationSource( + '${node.id.declarationName}.$name conflicts with the generated ' + 'class name "${node.className}".', + element: declaration.element, + ); + } + + if (node is AckUnionModelNode) { + checkMember(node.discriminatorKey); + continue; + } + if (node is AckObjectModelNode) { + for (final field in node.fields) { + checkMember(field.dartName); + } + if (node.discriminatorKey case final discriminator?) { + checkMember(discriminator); + } + } + validateCopyWithHelperNames( + className: node.className, + needsCopyWithSentinel: + node is AckObjectModelNode && + node.fields.any( (field) => field.jsonKey != node.discriminatorKey && (field.nullable || !field.isRequired), ), - element: declaration.element, - localNames: localNames, - ); - } + element: declaration.element, + localNames: localNames, + ); } } diff --git a/packages/ack_generator/test/integration/class_first_getter_model_refs_test.dart b/packages/ack_generator/test/integration/class_first_getter_model_refs_test.dart new file mode 100644 index 00000000..dcdee38c --- /dev/null +++ b/packages/ack_generator/test/integration/class_first_getter_model_refs_test.dart @@ -0,0 +1,44 @@ +import 'package:ack_generator/src/builder.dart'; +import 'package:build/build.dart'; +import 'package:build_test/build_test.dart'; +import 'package:test/test.dart'; + +void main() { + test( + 'class-first fields resolve same-build getter-generated models', + () async { + final readerWriter = TestReaderWriter(rootPackage: 'test_pkg'); + await readerWriter.testing.loadIsolateSources(); + await testBuilder( + ackModelBuilder(BuilderOptions.empty), + { + 'test_pkg|lib/models.dart': r''' +import 'package:ack/ack.dart'; +part 'models.g.dart'; + +@Schemable() +StringSchema get labelSchema => Ack.string(); + +@Schemable() +final class Holder with _$HolderAck { + const Holder({required this.label, required this.history}); + final Label label; + final List