From 753ca39d2aa9fb1fbdf33d8241ab67e7947f204f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Feb 2026 17:06:14 +0000 Subject: [PATCH 01/10] fix(generator): support enumString, enumValues, and literal as fields in Ack.object() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These schema types were handled correctly as top-level @AckType schemas but threw "Unsupported schema method" when used as field values inside Ack.object({...}). The bug was in _mapSchemaTypeToDartType (field-level type resolution) and _mapSchemaMethodToType (list element type strings), which were missing cases for these three schema types. Fixes: - _mapSchemaTypeToDartType: add enumString/literal → String, enumValues → resolved enum DartType - _mapSchemaMethodToType: add enumString/literal → 'String' - _extractListElementTypeString: handle enumValues with type extraction from invocation - Add _resolveEnumValuesType helper to resolve enum DartType from library elements - Add _extractEnumValuesTypeName helper for string-based type resolution in list contexts - Update test mock Ack class with enumString(), enumValues(), literal() static methods - Add 6 regression tests covering all field-level enum/literal scenarios https://claude.ai/code/session_01VF6gNrszmCJwYe1AcWPzez --- .../lib/src/analyzer/schema_ast_analyzer.dart | 99 ++++++- .../test/bugs/schema_variable_bugs_test.dart | 266 ++++++++++++++++++ .../test/test_utils/test_assets.dart | 15 +- 3 files changed, 378 insertions(+), 2 deletions(-) diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index e659f08..ae3019d 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -583,6 +583,18 @@ class SchemaAstAnalyzer { ), null, ); + case 'enumString': + case 'literal': + // enumString and literal are StringSchema variants — representation is String + return (typeProvider.stringType, null); + case 'enumValues': + // Try to resolve the enum DartType from the type argument: Ack.enumValues(...) + final resolvedType = _resolveEnumValuesType(invocation, library); + if (resolvedType != null) { + return (resolvedType, null); + } + // Fallback: treat as String (safe default for enum-like schemas) + return (typeProvider.stringType, null); default: throw InvalidGenerationSourceError( 'Unsupported schema method: Ack.$schemaMethod()', @@ -591,6 +603,60 @@ class SchemaAstAnalyzer { } } + /// Attempts to resolve the enum DartType from an `Ack.enumValues(...)` invocation. + /// + /// Tries two strategies: + /// 1. Extract type name from explicit type argument: `Ack.enumValues(...)` + /// 2. Infer from argument pattern: `Ack.enumValues(UserRole.values)` + /// + /// Then looks up the type in the library's declared enums and classes. + /// Returns `null` if the type cannot be resolved. + DartType? _resolveEnumValuesType( + MethodInvocation invocation, + LibraryElement2 library, + ) { + String? enumTypeName; + + // Strategy 1: Extract from type arguments — Ack.enumValues(...) + final typeArgs = invocation.typeArguments?.arguments; + if (typeArgs != null && typeArgs.isNotEmpty) { + final typeAnnotation = typeArgs.first; + if (typeAnnotation is NamedType) { + enumTypeName = typeAnnotation.name2.lexeme; + } + } + + // Strategy 2: Infer from argument — Ack.enumValues(UserRole.values) + if (enumTypeName == null) { + final args = invocation.argumentList.arguments; + if (args.isNotEmpty) { + final firstArg = args.first; + if (firstArg is PrefixedIdentifier && + firstArg.identifier.name == 'values') { + enumTypeName = firstArg.prefix.name; + } + } + } + + if (enumTypeName == null) return null; + + // Look up enum in the library's declared enums + for (final enumElement in library.enums) { + if (enumElement.name3 == enumTypeName) { + return enumElement.thisType; + } + } + + // Also check classes (for class-based enums or imported types) + for (final classElement in library.classes) { + if (classElement.name3 == enumTypeName) { + return classElement.thisType; + } + } + + return null; + } + _ListElementRef _resolveListElementRef(Expression firstArg) { if (firstArg is MethodInvocation) { final baseInvocation = _findBaseAckInvocation(firstArg); @@ -1112,7 +1178,12 @@ class SchemaAstAnalyzer { return 'List<$nestedType>'; } - // Map primitive schema types + // Handle enumValues — needs type extraction from invocation + if (methodName == 'enumValues') { + return _extractEnumValuesTypeName(ref.ackBase!) ?? 'dynamic'; + } + + // Map primitive schema types (handles string, enumString, literal, etc.) return _mapSchemaMethodToType(methodName); } @@ -1264,6 +1335,8 @@ class SchemaAstAnalyzer { String _mapSchemaMethodToType(String methodName) { switch (methodName) { case 'string': + case 'enumString': + case 'literal': return 'String'; case 'integer': return 'int'; @@ -1282,6 +1355,30 @@ class SchemaAstAnalyzer { } } + /// Extracts the enum type name from an `Ack.enumValues(...)` invocation as a string. + /// + /// Used for list element type resolution where we need a string representation + /// rather than a DartType. + String? _extractEnumValuesTypeName(MethodInvocation invocation) { + // Strategy 1: Type argument — Ack.enumValues(...) + final typeArgs = invocation.typeArguments?.arguments; + if (typeArgs != null && typeArgs.isNotEmpty) { + return typeArgs.first.toString(); + } + + // Strategy 2: Argument pattern — Ack.enumValues(UserRole.values) + final args = invocation.argumentList.arguments; + if (args.isNotEmpty) { + final firstArg = args.first; + if (firstArg is PrefixedIdentifier && + firstArg.identifier.name == 'values') { + return firstArg.prefix.name; + } + } + + return null; + } + /// Validates that a field name is a valid Dart identifier /// /// Throws [InvalidGenerationSourceError] if the field name: diff --git a/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart b/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart index b46e6f2..306b532 100644 --- a/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart +++ b/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart @@ -802,4 +802,270 @@ final keywordSchema = Ack.object({ } }); }); + + group('Enum, literal, and enumValues as fields inside Ack.object()', () { + test('handles Ack.enumString() as field in Ack.object()', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final reviewSchema = Ack.object({ + 'file': Ack.string(), + 'severity': Ack.enumString(['error', 'warning', 'info']), + 'message': Ack.string(), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'reviewSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + expect(modelInfo!.fields.length, 3); + + final severityField = modelInfo.fields.firstWhere( + (f) => f.name == 'severity', + ); + + expect( + severityField.type.isDartCoreString, + isTrue, + reason: + 'Expected String for enumString field, got ' + '${severityField.type.getDisplayString(withNullability: false)}', + ); + }); + }); + + test('handles Ack.enumString() with optional/nullable modifiers', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final formSchema = Ack.object({ + 'priority': Ack.enumString(['low', 'medium', 'high']).optional().nullable(), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'formSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final priorityField = modelInfo!.fields.firstWhere( + (f) => f.name == 'priority', + ); + + expect(priorityField.type.isDartCoreString, isTrue); + expect(priorityField.isRequired, isFalse); + expect(priorityField.isNullable, isTrue); + }); + }); + + test('handles Ack.literal() as field in Ack.object()', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final eventSchema = Ack.object({ + 'type': Ack.literal('click'), + 'target': Ack.string(), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'eventSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final typeField = modelInfo!.fields.firstWhere( + (f) => f.name == 'type', + ); + + expect( + typeField.type.isDartCoreString, + isTrue, + reason: + 'Expected String for literal field, got ' + '${typeField.type.getDisplayString(withNullability: false)}', + ); + }); + }); + + test('handles Ack.enumValues() as field in Ack.object()', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum UserRole { admin, editor, viewer } + +@AckType() +final userSchema = Ack.object({ + 'name': Ack.string(), + 'role': Ack.enumValues(UserRole.values), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'userSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + expect(modelInfo!.fields.length, 2); + + final roleField = modelInfo.fields.firstWhere( + (f) => f.name == 'role', + ); + + // enumValues should resolve to the UserRole enum type + expect( + roleField.type.element3, + isNotNull, + reason: 'Expected resolved type element for enum field', + ); + expect( + roleField.type.getDisplayString(withNullability: false), + equals('UserRole'), + reason: + 'Expected UserRole for enumValues field, got ' + '${roleField.type.getDisplayString(withNullability: false)}', + ); + }); + }); + + test( + 'handles Ack.enumValues() with optional modifier in Ack.object()', + () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum Priority { low, medium, high, critical } + +@AckType() +final taskSchema = Ack.object({ + 'title': Ack.string(), + 'priority': Ack.enumValues(Priority.values).optional(), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'taskSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final priorityField = modelInfo!.fields.firstWhere( + (f) => f.name == 'priority', + ); + + expect(priorityField.isRequired, isFalse); + expect( + priorityField.type.getDisplayString(withNullability: false), + equals('Priority'), + ); + }); + }, + ); + + test('handles Ack.list(Ack.enumString()) in Ack.object()', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final configSchema = Ack.object({ + 'tags': Ack.list(Ack.enumString(['a', 'b', 'c'])), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'configSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final tagsField = modelInfo!.fields.firstWhere( + (f) => f.name == 'tags', + ); + + expect(tagsField.type.isDartCoreList, isTrue); + + final listType = tagsField.type as InterfaceType; + final elementType = listType.typeArguments.first; + expect( + elementType.isDartCoreString, + isTrue, + reason: + 'Expected String element type, got ' + '${elementType.getDisplayString(withNullability: false)}', + ); + }); + }); + }); } diff --git a/packages/ack_generator/test/test_utils/test_assets.dart b/packages/ack_generator/test/test_utils/test_assets.dart index de0c71d..5b09d66 100644 --- a/packages/ack_generator/test/test_utils/test_assets.dart +++ b/packages/ack_generator/test/test_utils/test_assets.dart @@ -134,6 +134,10 @@ class Ack { discriminatorKey: discriminatorKey, schemas: schemas, ); + + static StringSchema literal(String value) => const StringSchema(); + static StringSchema enumString(List values) => const StringSchema(); + static EnumSchema enumValues(List values) => EnumSchema(); } abstract class AckSchema { @@ -259,11 +263,20 @@ class ObjectSchema extends AckSchema> { extension ObjectSchemaExtensions on ObjectSchema { ObjectSchema passthrough() => copyWith(additionalProperties: true); } +class EnumSchema extends AckSchema { + EnumSchema(); + EnumSchema nullable() => this; + EnumSchema optional() => this; + EnumSchema describe(String description) => this; + + @override + Map toJsonSchema() => {'type': 'string', 'enum': []}; +} class DiscriminatedSchema extends AckSchema> { final String discriminatorKey; final Map schemas; const DiscriminatedSchema({required this.discriminatorKey, required this.schemas}); - + @override Map toJsonSchema() => { 'oneOf': schemas.values.map((schema) => schema.toJsonSchema()).toList(), From c6852a856fdd833dd427955279d837f443c09865 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Feb 2026 18:28:54 +0000 Subject: [PATCH 02/10] test(generator): add golden tests and edge cases for enumString/enumValues/literal fields - Add 9 golden tests verifying generated extension type getters for enumString, literal, and enumValues fields inside Ack.object() schemas - Add 3 edge case tests for Ack.list(Ack.enumValues()), Ack.list(Ack.literal()), and Ack.enumValues().nullable() without optional() - Refactor duplicate enum type name extraction into shared _extractEnumTypeNameFromInvocation() helper https://claude.ai/code/session_01VF6gNrszmCJwYe1AcWPzez --- .../lib/src/analyzer/schema_ast_analyzer.dart | 63 ++-- .../test/bugs/schema_variable_bugs_test.dart | 142 +++++++++ .../ack_type_enum_literal_fields_test.dart | 298 ++++++++++++++++++ 3 files changed, 467 insertions(+), 36 deletions(-) create mode 100644 packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index ae3019d..291b1f8 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -603,41 +603,47 @@ class SchemaAstAnalyzer { } } - /// Attempts to resolve the enum DartType from an `Ack.enumValues(...)` invocation. + /// Extracts the enum type name string from an `Ack.enumValues(...)` invocation. /// /// Tries two strategies: - /// 1. Extract type name from explicit type argument: `Ack.enumValues(...)` + /// 1. Extract from explicit type argument: `Ack.enumValues(...)` /// 2. Infer from argument pattern: `Ack.enumValues(UserRole.values)` /// - /// Then looks up the type in the library's declared enums and classes. - /// Returns `null` if the type cannot be resolved. - DartType? _resolveEnumValuesType( - MethodInvocation invocation, - LibraryElement2 library, - ) { - String? enumTypeName; - + /// Returns `null` if the type name cannot be determined. + String? _extractEnumTypeNameFromInvocation(MethodInvocation invocation) { // Strategy 1: Extract from type arguments — Ack.enumValues(...) final typeArgs = invocation.typeArguments?.arguments; if (typeArgs != null && typeArgs.isNotEmpty) { final typeAnnotation = typeArgs.first; if (typeAnnotation is NamedType) { - enumTypeName = typeAnnotation.name2.lexeme; + return typeAnnotation.name2.lexeme; } + return typeArgs.first.toString(); } // Strategy 2: Infer from argument — Ack.enumValues(UserRole.values) - if (enumTypeName == null) { - final args = invocation.argumentList.arguments; - if (args.isNotEmpty) { - final firstArg = args.first; - if (firstArg is PrefixedIdentifier && - firstArg.identifier.name == 'values') { - enumTypeName = firstArg.prefix.name; - } + final args = invocation.argumentList.arguments; + if (args.isNotEmpty) { + final firstArg = args.first; + if (firstArg is PrefixedIdentifier && + firstArg.identifier.name == 'values') { + return firstArg.prefix.name; } } + return null; + } + + /// Attempts to resolve the enum DartType from an `Ack.enumValues(...)` invocation. + /// + /// Uses [_extractEnumTypeNameFromInvocation] to get the type name, then + /// looks it up in the library's declared enums and classes. + /// Returns `null` if the type cannot be resolved. + DartType? _resolveEnumValuesType( + MethodInvocation invocation, + LibraryElement2 library, + ) { + final enumTypeName = _extractEnumTypeNameFromInvocation(invocation); if (enumTypeName == null) return null; // Look up enum in the library's declared enums @@ -1357,26 +1363,11 @@ class SchemaAstAnalyzer { /// Extracts the enum type name from an `Ack.enumValues(...)` invocation as a string. /// + /// Delegates to [_extractEnumTypeNameFromInvocation] for the shared extraction logic. /// Used for list element type resolution where we need a string representation /// rather than a DartType. String? _extractEnumValuesTypeName(MethodInvocation invocation) { - // Strategy 1: Type argument — Ack.enumValues(...) - final typeArgs = invocation.typeArguments?.arguments; - if (typeArgs != null && typeArgs.isNotEmpty) { - return typeArgs.first.toString(); - } - - // Strategy 2: Argument pattern — Ack.enumValues(UserRole.values) - final args = invocation.argumentList.arguments; - if (args.isNotEmpty) { - final firstArg = args.first; - if (firstArg is PrefixedIdentifier && - firstArg.identifier.name == 'values') { - return firstArg.prefix.name; - } - } - - return null; + return _extractEnumTypeNameFromInvocation(invocation); } /// Validates that a field name is a valid Dart identifier diff --git a/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart b/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart index 306b532..aefca8b 100644 --- a/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart +++ b/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart @@ -1067,5 +1067,147 @@ final configSchema = Ack.object({ ); }); }); + + test('handles Ack.list(Ack.enumValues()) in Ack.object()', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum UserRole { admin, editor, viewer } + +@AckType() +final teamSchema = Ack.object({ + 'name': Ack.string(), + 'roles': Ack.list(Ack.enumValues(UserRole.values)), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'teamSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final rolesField = modelInfo!.fields.firstWhere( + (f) => f.name == 'roles', + ); + + expect(rolesField.type.isDartCoreList, isTrue); + + final listType = rolesField.type as InterfaceType; + final elementType = listType.typeArguments.first; + expect( + elementType.getDisplayString(withNullability: false), + equals('UserRole'), + reason: + 'Expected UserRole element type for list of enumValues, got ' + '${elementType.getDisplayString(withNullability: false)}', + ); + }); + }); + + test('handles Ack.list(Ack.literal()) in Ack.object()', () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final actionSchema = Ack.object({ + 'types': Ack.list(Ack.literal('click')), + 'name': Ack.string(), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'actionSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final typesField = modelInfo!.fields.firstWhere( + (f) => f.name == 'types', + ); + + expect(typesField.type.isDartCoreList, isTrue); + + final listType = typesField.type as InterfaceType; + final elementType = listType.typeArguments.first; + expect( + elementType.isDartCoreString, + isTrue, + reason: + 'Expected String element type for list of literals, got ' + '${elementType.getDisplayString(withNullability: false)}', + ); + }); + }); + + test('handles Ack.enumValues().nullable() without optional()', + () async { + final assets = { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum UserRole { admin, editor, viewer } + +@AckType() +final profileSchema = Ack.object({ + 'name': Ack.string(), + 'role': Ack.enumValues(UserRole.values).nullable(), +}); +''', + }; + + await resolveSources(assets, (resolver) async { + final library = await resolver.libraryFor( + AssetId('test_pkg', 'lib/schema.dart'), + ); + final schemaVar = library.topLevelVariables + .whereType() + .firstWhere((e) => e.name3 == 'profileSchema'); + + final analyzer = SchemaAstAnalyzer(); + final modelInfo = analyzer.analyzeSchemaVariable(schemaVar); + + expect(modelInfo, isNotNull); + + final roleField = modelInfo!.fields.firstWhere( + (f) => f.name == 'role', + ); + + expect( + roleField.type.getDisplayString(withNullability: false), + equals('UserRole'), + ); + expect(roleField.isNullable, isTrue); + expect( + roleField.isRequired, + isTrue, + reason: 'nullable() alone should not make the field optional', + ); + }); + }); }); } diff --git a/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart b/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart new file mode 100644 index 0000000..b030b16 --- /dev/null +++ b/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart @@ -0,0 +1,298 @@ +import 'package:ack_generator/builder.dart'; +import 'package:build/build.dart'; +import 'package:build_test/build_test.dart'; +import 'package:test/test.dart'; + +import '../test_utils/test_assets.dart'; + +void main() { + group('@AckType with enumString, literal, and enumValues fields', () { + test('enumString field generates String getter', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final reviewSchema = Ack.object({ + 'file': Ack.string(), + 'severity': Ack.enumString(['error', 'warning', 'info']), + 'message': Ack.string(), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type ReviewType(Map _data)', + ), + contains('String get file'), + contains("_data['file'] as String"), + contains('String get severity'), + contains("_data['severity'] as String"), + contains('String get message'), + contains("_data['message'] as String"), + ]), + ), + }, + ); + }); + + test('literal field generates String getter', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final eventSchema = Ack.object({ + 'type': Ack.literal('click'), + 'target': Ack.string(), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type EventType(Map _data)', + ), + contains('String get type'), + contains("_data['type'] as String"), + contains('String get target'), + contains("_data['target'] as String"), + ]), + ), + }, + ); + }); + + test('enumValues field generates enum type getter', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum UserRole { admin, editor, viewer } + +@AckType() +final userSchema = Ack.object({ + 'name': Ack.string(), + 'role': Ack.enumValues(UserRole.values), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type UserType(Map _data)', + ), + contains('String get name'), + contains('UserRole get role'), + contains("_data['role'] as UserRole"), + ]), + ), + }, + ); + }); + + test( + 'enumString with optional().nullable() generates String? getter', + () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final formSchema = Ack.object({ + 'title': Ack.string(), + 'priority': Ack.enumString(['low', 'medium', 'high']).optional().nullable(), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type FormType(Map _data)', + ), + contains('String get title'), + contains('String? get priority'), + contains("_data['priority'] as String?"), + ]), + ), + }, + ); + }, + ); + + test('enumValues with optional() generates T? getter', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum Priority { low, medium, high, critical } + +@AckType() +final taskSchema = Ack.object({ + 'title': Ack.string(), + 'priority': Ack.enumValues(Priority.values).optional(), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type TaskType(Map _data)', + ), + contains('String get title'), + contains('Priority? get priority'), + contains("_data['priority'] as Priority?"), + ]), + ), + }, + ); + }); + + test('Ack.list(Ack.enumString()) generates List getter', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final configSchema = Ack.object({ + 'name': Ack.string(), + 'tags': Ack.list(Ack.enumString(['a', 'b', 'c'])), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type ConfigType(Map _data)', + ), + contains('String get name'), + contains('List get tags'), + contains("(_data['tags'] as List).cast()"), + ]), + ), + }, + ); + }); + + test('Ack.list(Ack.enumValues()) generates List getter', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum UserRole { admin, editor, viewer } + +@AckType() +final teamSchema = Ack.object({ + 'name': Ack.string(), + 'roles': Ack.list(Ack.enumValues(UserRole.values)), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type TeamType(Map _data)', + ), + contains('String get name'), + contains('List get roles'), + contains("(_data['roles'] as List).cast()"), + ]), + ), + }, + ); + }); + + test('mixed schema with literal, enumString, enumValues, and string fields', + () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum Color { red, green, blue } + +@AckType() +final widgetSchema = Ack.object({ + 'type': Ack.literal('button'), + 'label': Ack.string(), + 'color': Ack.enumValues(Color.values), + 'style': Ack.enumString(['solid', 'outline', 'ghost']), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type WidgetType(Map _data)', + ), + contains('String get type'), + contains("_data['type'] as String"), + contains('String get label'), + contains("_data['label'] as String"), + contains('Color get color'), + contains("_data['color'] as Color"), + contains('String get style'), + contains("_data['style'] as String"), + ]), + ), + }, + ); + }); + }); +} From 2a08cbb3ea6f5786fe9cf328751e7a0ad1d13989 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Feb 2026 19:16:48 +0000 Subject: [PATCH 03/10] refactor: clean up AI slop from enum/literal field support - Remove pointless _extractEnumValuesTypeName wrapper; call shared helper directly - Strip redundant inline comments that narrate the obvious - Trim over-documented docstrings to match surrounding code style - Remove verbose reason strings from test assertions where the framework output is already clear https://claude.ai/code/session_01VF6gNrszmCJwYe1AcWPzez --- .../lib/src/analyzer/schema_ast_analyzer.dart | 41 +++++------------ .../test/bugs/schema_variable_bugs_test.dart | 45 +++---------------- 2 files changed, 16 insertions(+), 70 deletions(-) diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index 291b1f8..e790da6 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -585,15 +585,13 @@ class SchemaAstAnalyzer { ); case 'enumString': case 'literal': - // enumString and literal are StringSchema variants — representation is String return (typeProvider.stringType, null); case 'enumValues': - // Try to resolve the enum DartType from the type argument: Ack.enumValues(...) final resolvedType = _resolveEnumValuesType(invocation, library); if (resolvedType != null) { return (resolvedType, null); } - // Fallback: treat as String (safe default for enum-like schemas) + // Fallback to String if the enum type can't be resolved return (typeProvider.stringType, null); default: throw InvalidGenerationSourceError( @@ -603,15 +601,12 @@ class SchemaAstAnalyzer { } } - /// Extracts the enum type name string from an `Ack.enumValues(...)` invocation. + /// Extracts the enum type name from an `Ack.enumValues(...)` invocation. /// - /// Tries two strategies: - /// 1. Extract from explicit type argument: `Ack.enumValues(...)` - /// 2. Infer from argument pattern: `Ack.enumValues(UserRole.values)` - /// - /// Returns `null` if the type name cannot be determined. + /// Tries the explicit type argument first, then infers from the argument + /// pattern (e.g. `UserRole.values`). String? _extractEnumTypeNameFromInvocation(MethodInvocation invocation) { - // Strategy 1: Extract from type arguments — Ack.enumValues(...) + // From type argument: Ack.enumValues(...) final typeArgs = invocation.typeArguments?.arguments; if (typeArgs != null && typeArgs.isNotEmpty) { final typeAnnotation = typeArgs.first; @@ -621,7 +616,7 @@ class SchemaAstAnalyzer { return typeArgs.first.toString(); } - // Strategy 2: Infer from argument — Ack.enumValues(UserRole.values) + // From argument pattern: Ack.enumValues(UserRole.values) final args = invocation.argumentList.arguments; if (args.isNotEmpty) { final firstArg = args.first; @@ -634,11 +629,8 @@ class SchemaAstAnalyzer { return null; } - /// Attempts to resolve the enum DartType from an `Ack.enumValues(...)` invocation. - /// - /// Uses [_extractEnumTypeNameFromInvocation] to get the type name, then - /// looks it up in the library's declared enums and classes. - /// Returns `null` if the type cannot be resolved. + /// Resolves the enum DartType from an `Ack.enumValues(...)` invocation + /// by extracting the type name and looking it up in the library. DartType? _resolveEnumValuesType( MethodInvocation invocation, LibraryElement2 library, @@ -646,14 +638,13 @@ class SchemaAstAnalyzer { final enumTypeName = _extractEnumTypeNameFromInvocation(invocation); if (enumTypeName == null) return null; - // Look up enum in the library's declared enums for (final enumElement in library.enums) { if (enumElement.name3 == enumTypeName) { return enumElement.thisType; } } - // Also check classes (for class-based enums or imported types) + // Also check classes (for class-based enums) for (final classElement in library.classes) { if (classElement.name3 == enumTypeName) { return classElement.thisType; @@ -1184,12 +1175,11 @@ class SchemaAstAnalyzer { return 'List<$nestedType>'; } - // Handle enumValues — needs type extraction from invocation if (methodName == 'enumValues') { - return _extractEnumValuesTypeName(ref.ackBase!) ?? 'dynamic'; + return _extractEnumTypeNameFromInvocation(ref.ackBase!) ?? 'dynamic'; } - // Map primitive schema types (handles string, enumString, literal, etc.) + // Map primitive schema types return _mapSchemaMethodToType(methodName); } @@ -1361,15 +1351,6 @@ class SchemaAstAnalyzer { } } - /// Extracts the enum type name from an `Ack.enumValues(...)` invocation as a string. - /// - /// Delegates to [_extractEnumTypeNameFromInvocation] for the shared extraction logic. - /// Used for list element type resolution where we need a string representation - /// rather than a DartType. - String? _extractEnumValuesTypeName(MethodInvocation invocation) { - return _extractEnumTypeNameFromInvocation(invocation); - } - /// Validates that a field name is a valid Dart identifier /// /// Throws [InvalidGenerationSourceError] if the field name: diff --git a/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart b/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart index aefca8b..6495610 100644 --- a/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart +++ b/packages/ack_generator/test/bugs/schema_variable_bugs_test.dart @@ -838,13 +838,7 @@ final reviewSchema = Ack.object({ (f) => f.name == 'severity', ); - expect( - severityField.type.isDartCoreString, - isTrue, - reason: - 'Expected String for enumString field, got ' - '${severityField.type.getDisplayString(withNullability: false)}', - ); + expect(severityField.type.isDartCoreString, isTrue); }); }); @@ -917,13 +911,7 @@ final eventSchema = Ack.object({ (f) => f.name == 'type', ); - expect( - typeField.type.isDartCoreString, - isTrue, - reason: - 'Expected String for literal field, got ' - '${typeField.type.getDisplayString(withNullability: false)}', - ); + expect(typeField.type.isDartCoreString, isTrue); }); }); @@ -962,18 +950,10 @@ final userSchema = Ack.object({ (f) => f.name == 'role', ); - // enumValues should resolve to the UserRole enum type - expect( - roleField.type.element3, - isNotNull, - reason: 'Expected resolved type element for enum field', - ); + expect(roleField.type.element3, isNotNull); expect( roleField.type.getDisplayString(withNullability: false), equals('UserRole'), - reason: - 'Expected UserRole for enumValues field, got ' - '${roleField.type.getDisplayString(withNullability: false)}', ); }); }); @@ -1058,13 +1038,7 @@ final configSchema = Ack.object({ final listType = tagsField.type as InterfaceType; final elementType = listType.typeArguments.first; - expect( - elementType.isDartCoreString, - isTrue, - reason: - 'Expected String element type, got ' - '${elementType.getDisplayString(withNullability: false)}', - ); + expect(elementType.isDartCoreString, isTrue); }); }); @@ -1109,9 +1083,6 @@ final teamSchema = Ack.object({ expect( elementType.getDisplayString(withNullability: false), equals('UserRole'), - reason: - 'Expected UserRole element type for list of enumValues, got ' - '${elementType.getDisplayString(withNullability: false)}', ); }); }); @@ -1152,13 +1123,7 @@ final actionSchema = Ack.object({ final listType = typesField.type as InterfaceType; final elementType = listType.typeArguments.first; - expect( - elementType.isDartCoreString, - isTrue, - reason: - 'Expected String element type for list of literals, got ' - '${elementType.getDisplayString(withNullability: false)}', - ); + expect(elementType.isDartCoreString, isTrue); }); }); From 39e979cb2e2fa6285d92b1f0b7e22977ec58d9ae Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 16 Feb 2026 15:58:42 -0500 Subject: [PATCH 04/10] chore: commit pending changes --- code_review_2026-02-16_1541.md | 177 +++++++++++++++ .../lib/src/analyzer/schema_ast_analyzer.dart | 211 +++++++++++++----- .../lib/src/builders/type_builder.dart | 7 +- .../lib/src/models/field_info.dart | 9 + .../ack_type_enum_literal_fields_test.dart | 158 +++++++++---- .../test/src/test_utilities.dart | 8 + 6 files changed, 470 insertions(+), 100 deletions(-) create mode 100644 code_review_2026-02-16_1541.md diff --git a/code_review_2026-02-16_1541.md b/code_review_2026-02-16_1541.md new file mode 100644 index 0000000..1bfd1b9 --- /dev/null +++ b/code_review_2026-02-16_1541.md @@ -0,0 +1,177 @@ +# Code Review - 2026-02-16 (Consolidated) + +Branch: `claude/install-dart-melos-UQthl` (24 commits vs `main`) + +## Summary + +| Category | Total | Should Fix | Notes | +|----------|-------|------------|-------| +| Code Quality | 7 | 6 | 1 | +| Code Hygiene | 3 | 1 | 2 | +| **Combined** | **10** | **7** | **3** | + +No blockers. All findings validated by expert review and cross-checked against actual file contents. + +--- + +## Code Quality Findings + +### F1 — Tag pattern too broad for publish pipeline + +**Location**: `.github/workflows/release.yml:6` +**Severity**: SHOULD_FIX + +The release workflow triggers on `- 'v*'`, which matches any tag starting with `v` (e.g., `vfoo`, `v`, `v1`). This is risky for a pipeline that publishes to pub.dev. + +**Note**: GitHub Actions uses glob matching, not regex. The original suggestion (`v[0-9]+.[0-9]+.[0-9]+*`) is regex-like and won't work as a tag filter. + +**Action**: Use a tighter glob like `v[0-9]*.[0-9]*.[0-9]*` and/or add a job-level validation step that checks the tag is valid semver before publishing. + +--- + +### F2 — Floating reusable workflow references + +**Locations**: +- `.github/workflows/release.yml:10` — `btwld/dart-actions/.github/workflows/publish.yml@main` +- `.github/workflows/ci.yml:11` — `btwld/dart-actions/.github/workflows/ci.yml@main` + +**Severity**: SHOULD_FIX + +Both workflows pin to `@main`, which is mutable. Upstream changes can silently alter CI/CD behavior without any change in this repo. + +**Action**: Pin both references to an immutable commit SHA or locked release tag. Update intentionally when the upstream action changes. + +--- + +### F3 — Quality gate gap for ack_annotations + +**Location**: Root `pubspec.yaml:53` (melos scripts) +**Severity**: SHOULD_FIX + +The `melos analyze` scope includes `ack`, `ack_generator`, `ack_firebase_ai`, `ack_json_schema_builder`, and `ack_example` but excludes `ack_annotations`. However, the release workflow (`.github/workflows/release.yml:16-17`) now publishes `ack_annotations`. + +**Action**: Add `ack_annotations` to the analyze and test script scopes so it gets the same quality gates as other published packages. + +--- + +### F4 — Version mismatch between docs and packages + +**Locations**: +- `packages/ack_generator/README.md:22` — shows `ack: ^1.0.0`, `ack_generator: ^1.0.0` +- `packages/ack_annotations/README.md:13` — shows `ack_annotations: ^1.0.0`, `ack_generator: ^1.0.0` + +**Severity**: SHOULD_FIX + +All package pubspec.yaml files show version `1.0.0-beta.6`. Users following the README examples will get dependency resolution failures because `^1.0.0` excludes prereleases. + +**Action**: Align README version constraints with actually published versions, or use a version-agnostic reference pattern. + +--- + +### F5 — API docs stale on discriminated schema signature + +**Location**: `docs/api-reference/index.mdx:21` (also line 288) +**Severity**: SHOULD_FIX + +Docs show `Map` for `Ack.object()` and discriminated unions, but code at `packages/ack/lib/src/ack.dart:36` now requires `Map>`. + +**Action**: Update the API reference to reflect the current type signature. + +--- + +### F6 — Breaking API change not communicated + +**Location**: `packages/ack_annotations/lib/src/ack_field.dart:14` +**Severity**: SHOULD_FIX + +`@AckField` changed from `required` parameter to `requiredMode` (using `AckFieldRequiredMode` enum). This is a breaking change for consumers, but `packages/ack_annotations/CHANGELOG.md` only lists it under generic "Improvements" with no breaking-change callout or migration guidance. + +**Action**: Add a breaking-change note to the changelog with migration instructions (e.g., `required: true` → `requiredMode: AckFieldRequiredMode.always`). + +--- + +### F7 — Missing test coverage for generated toJson() + +**Location**: `packages/ack_generator/lib/src/builders/type_builder.dart:94` +**Severity**: SHOULD_FIX + +The generator now emits `toJson()` for all extension types (lines 94, 377-387). No `toJson` assertions exist in `packages/ack_generator/test/`. Extension-type behavior IS tested in integration tests (typed getters, parse/safeParse, list mapping), but `toJson()` specifically is not covered. + +**Action**: Add targeted tests for `toJson()` generation output and runtime behavior across object, primitive, and collection extension types. + +--- + +## Code Hygiene Findings + +### S1 — Redundant inline comments in schema.dart + +**Location**: `packages/ack/lib/src/schemas/schema.dart:271` +**Severity**: NOTE + +Comments at lines 271, 275, 293, 304 restate adjacent code (e.g., "Use centralized null handling" before `handleNullInput(...)`, "After null check, inputValue is guaranteed non-null" before `inputValue!`). + +**Action**: Remove comments that narrate obvious operations. Keep only comments that explain *why*. + +--- + +### S2 — Verbose narration in transformed_schema.dart + +**Location**: `packages/ack/lib/src/schemas/transformed_schema.dart:51` +**Severity**: SHOULD_FIX + +Dense step-by-step comment narration around straightforward control flow. Measured density: ~61% for the cited block (lines 51-82), ~39% for the full file. The rationale is valid (explaining why `TransformedSchema` doesn't use centralized null handling), but it's expressed with excessive line-by-line narration. + +**Action**: Compress to a concise 2-3 line rationale comment at method entry. Remove procedural commentary. + +--- + +### S3 — Repeated test assertion pattern + +**Location**: `packages/ack/test/schemas/schema_equality_test.dart:12` +**Severity**: NOTE + +15 adjacent `expect(a, equals(b))` + `expect(a.hashCode, equals(b.hashCode))` pairs across the equality tests. This is standard test duplication, not AI slop. + +**Action**: Optional — extract a shared `expectEqualAndHash()` helper if the pattern continues to grow. + +--- + +## Dropped / Demoted from Original Review + +| Original | Disposition | Reason | +|----------|-------------|--------| +| F8 (comment-only override files) | **Dropped** | Melos-managed placeholders; deletion causes churn for no real benefit | +| S3 severity SHOULD_FIX | **Demoted to NOTE** | Test duplication is common and helper extraction is optional | +| S2 density "~77%" | **Corrected to ~61%** | Actual measurement: 19 comment / 31 nonblank lines in cited block | + +--- + +## Prioritized Action Items + +| Priority | ID | Location | Issue | Action | +|----------|----|----------|-------|--------| +| 1 | F6 | `ack_field.dart:14` | Breaking API change undocumented | Add breaking-change note + migration guide | +| 2 | F2 | `release.yml:10`, `ci.yml:11` | Floating `@main` workflow refs | Pin to commit SHA | +| 3 | F3 | Root `pubspec.yaml:53` | ack_annotations missing from quality gates | Add to analyze/test scopes | +| 4 | F4 | `ack_generator/README.md:22`, `ack_annotations/README.md:13` | Docs show `^1.0.0`, packages are beta.6 | Align version references | +| 5 | F5 | `docs/api-reference/index.mdx:21` | Stale discriminated schema type in docs | Update to current signature | +| 6 | F1 | `release.yml:6` | Tag pattern `v*` too broad | Tighten glob + add semver validation step | +| 7 | F7 | `type_builder.dart:94` | `toJson()` generation untested | Add targeted toJson tests | +| 8 | S2 | `transformed_schema.dart:51` | Verbose narration (~61% comment density) | Compress to rationale-only | + +--- + +## AI Slop Assessment + +- **Lines analyzed**: 11,192 +- **AI-generated code likelihood**: **low** +- **Conclusion**: No significant AI slop patterns detected. Minor comment hygiene issues only. + +--- + +## Methodology + +1. **Phase 1**: Gathered full branch diff (2,999 lines across 24 commits) +2. **Phase 2**: Ran Architect agent (code quality, 5 domains) and Slop pipeline (3-stage detection) +3. **Phase 3**: Expert validation — Reviewer agent verified all findings against actual file contents +4. **Phase 4**: Cross-checked against domain expert feedback; corrected inaccuracies, dropped over-engineered findings diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index e790da6..b95a658 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -533,6 +533,18 @@ class SchemaAstAnalyzer { baseInvocation, element, ); + final schemaMethod = baseInvocation.methodName.name; + + String? displayTypeOverride; + String? collectionElementDisplayTypeOverride; + + if (schemaMethod == 'enumValues') { + displayTypeOverride = _extractEnumTypeNameFromInvocation(baseInvocation); + } else if (schemaMethod == 'list') { + collectionElementDisplayTypeOverride = _extractListEnumElementTypeName( + baseInvocation, + ); + } return FieldInfo( name: fieldName, @@ -542,6 +554,9 @@ class SchemaAstAnalyzer { isNullable: isNullable, constraints: [], listElementSchemaRef: listElementSchemaRef, + displayTypeOverride: displayTypeOverride, + collectionElementDisplayTypeOverride: + collectionElementDisplayTypeOverride, ); } @@ -587,12 +602,20 @@ class SchemaAstAnalyzer { case 'literal': return (typeProvider.stringType, null); case 'enumValues': - final resolvedType = _resolveEnumValuesType(invocation, library); + final resolvedType = _resolveEnumValuesType( + invocation, + library: library, + ); if (resolvedType != null) { return (resolvedType, null); } - // Fallback to String if the enum type can't be resolved - return (typeProvider.stringType, null); + // Fallback to `dynamic` if the enum type can't be resolved. + // This avoids incorrectly assuming `String` when EnumSchema.parse() + // returns the enum value type T. + _log.warning( + 'Could not resolve enum type for Ack.enumValues(); falling back to dynamic.', + ); + return (typeProvider.dynamicType, null); default: throw InvalidGenerationSourceError( 'Unsupported schema method: Ack.$schemaMethod()', @@ -603,50 +626,144 @@ class SchemaAstAnalyzer { /// Extracts the enum type name from an `Ack.enumValues(...)` invocation. /// - /// Tries the explicit type argument first, then infers from the argument - /// pattern (e.g. `UserRole.values`). + /// Prefers resolved static types so imported/re-exported enums and prefixed + /// references (e.g., `alias.UserRole`) are preserved in generated code. + /// Falls back to source-based extraction if static typing is unavailable. String? _extractEnumTypeNameFromInvocation(MethodInvocation invocation) { - // From type argument: Ack.enumValues(...) + final sourceTypeName = _extractEnumTypeNameFromSource(invocation); + if (sourceTypeName != null) { + return sourceTypeName; + } + + final resolvedType = _resolveEnumValuesType(invocation); + if (resolvedType != null) { + return resolvedType.getDisplayString(withNullability: false); + } + + return null; + } + + String? _extractEnumTypeNameFromSource(MethodInvocation invocation) { + // From type argument: Ack.enumValues(...) or Ack.enumValues(...) final typeArgs = invocation.typeArguments?.arguments; if (typeArgs != null && typeArgs.isNotEmpty) { - final typeAnnotation = typeArgs.first; - if (typeAnnotation is NamedType) { - return typeAnnotation.name2.lexeme; - } - return typeArgs.first.toString(); + return typeArgs.first.toSource(); } - // From argument pattern: Ack.enumValues(UserRole.values) + // From argument pattern: Ack.enumValues(UserRole.values) / Ack.enumValues(alias.UserRole.values) final args = invocation.argumentList.arguments; if (args.isNotEmpty) { final firstArg = args.first; if (firstArg is PrefixedIdentifier && firstArg.identifier.name == 'values') { - return firstArg.prefix.name; + return firstArg.prefix.toSource(); + } + if (firstArg is PropertyAccess && + firstArg.propertyName.name == 'values') { + return firstArg.target?.toSource(); } } return null; } - /// Resolves the enum DartType from an `Ack.enumValues(...)` invocation - /// by extracting the type name and looking it up in the library. + String? _extractListEnumElementTypeName(MethodInvocation listInvocation) { + final args = listInvocation.argumentList.arguments; + if (args.isEmpty) return null; + + final ref = _resolveListElementRef(args.first); + final elementSchema = ref.ackBase; + if (elementSchema == null || + elementSchema.methodName.name != 'enumValues') { + return null; + } + + return _extractEnumTypeNameFromInvocation(elementSchema); + } + + /// Resolves enum type `T` from an `Ack.enumValues(...)` invocation. + /// + /// Resolution strategy (in order): + /// 1. Explicit type argument's resolved type (`Ack.enumValues(...)`) + /// 2. Invocation static type argument (`EnumSchema`) + /// 3. First argument static type (`List` from `T.values`) + /// 4. Source name lookup in the library/import scope DartType? _resolveEnumValuesType( - MethodInvocation invocation, - LibraryElement2 library, - ) { - final enumTypeName = _extractEnumTypeNameFromInvocation(invocation); - if (enumTypeName == null) return null; + MethodInvocation invocation, { + LibraryElement2? library, + }) { + final typeArgs = invocation.typeArguments?.arguments; + if (typeArgs != null && typeArgs.isNotEmpty) { + final explicitType = typeArgs.first.type; + if (explicitType is InterfaceType) { + return explicitType; + } + } + + final invocationType = invocation.staticType; + if (invocationType is InterfaceType && + invocationType.typeArguments.isNotEmpty) { + final schemaTypeArg = invocationType.typeArguments.first; + if (schemaTypeArg is InterfaceType) { + return schemaTypeArg; + } + } + + final args = invocation.argumentList.arguments; + if (args.isNotEmpty) { + final argType = args.first.staticType; + if (argType is InterfaceType) { + if (argType.isDartCoreList && argType.typeArguments.isNotEmpty) { + final elementType = argType.typeArguments.first; + if (elementType is InterfaceType) { + return elementType; + } + } else { + return argType; + } + } + } + + if (library != null) { + final enumTypeName = _extractEnumTypeNameFromSource(invocation); + if (enumTypeName != null) { + final resolvedByName = _resolveTypeByName(enumTypeName, library); + if (resolvedByName != null) { + return resolvedByName; + } + } + } + + return null; + } + + DartType? _resolveTypeByName(String typeName, LibraryElement2 library) { + final normalizedTypeName = typeName.trim(); + if (normalizedTypeName.isEmpty) return null; + + final scopeResult = library.firstFragment.scope.lookup(normalizedTypeName); + final scopeType = _resolveTypeFromElement(scopeResult.getter); + if (scopeType != null) { + return scopeType; + } + + // Try import namespaces directly as a fallback for prefixed names. + for (final import in library.firstFragment.libraryImports) { + final importedElement = import.namespace.get2(normalizedTypeName); + final importedType = _resolveTypeFromElement(importedElement); + if (importedType != null) { + return importedType; + } + } + // Last-resort local lookup. for (final enumElement in library.enums) { - if (enumElement.name3 == enumTypeName) { + if (enumElement.name3 == normalizedTypeName) { return enumElement.thisType; } } - - // Also check classes (for class-based enums) for (final classElement in library.classes) { - if (classElement.name3 == enumTypeName) { + if (classElement.name3 == normalizedTypeName) { return classElement.thisType; } } @@ -654,6 +771,25 @@ class SchemaAstAnalyzer { return null; } + DartType? _resolveTypeFromElement(Element2? element) { + if (element is EnumElement2) { + return element.thisType; + } + + if (element is ClassElement2) { + return element.thisType; + } + + if (element is TypeAliasElement2) { + final aliasedType = element.aliasedType; + if (aliasedType is InterfaceType) { + return aliasedType; + } + } + + return null; + } + _ListElementRef _resolveListElementRef(Expression firstArg) { if (firstArg is MethodInvocation) { final baseInvocation = _findBaseAckInvocation(firstArg); @@ -1267,32 +1403,7 @@ class SchemaAstAnalyzer { customTypeName: customTypeName, ); - // Strategy 1: Try to extract from type arguments: Ack.enumValues([...]) - final typeArgs = invocation.typeArguments?.arguments; - String? enumTypeName; - - if (typeArgs != null && typeArgs.isNotEmpty) { - // Get the first type argument (the enum type) - final typeArg = typeArgs.first; - enumTypeName = typeArg.toString(); - } else { - // Strategy 2: Try to infer from argument list: Ack.enumValues(UserRole.values) - final args = invocation.argumentList.arguments; - if (args.isNotEmpty) { - final firstArg = args.first; - - // Check if it's EnumType.values (PrefixedIdentifier) - if (firstArg is PrefixedIdentifier) { - final prefix = firstArg.prefix.name; - final identifier = firstArg.identifier.name; - - if (identifier == 'values') { - // UserRole.values → use 'UserRole' - enumTypeName = prefix; - } - } - } - } + final enumTypeName = _extractEnumTypeNameFromInvocation(invocation); // If we couldn't extract the enum type, throw an error if (enumTypeName == null) { diff --git a/packages/ack_generator/lib/src/builders/type_builder.dart b/packages/ack_generator/lib/src/builders/type_builder.dart index fc670e8..78a01af 100644 --- a/packages/ack_generator/lib/src/builders/type_builder.dart +++ b/packages/ack_generator/lib/src/builders/type_builder.dart @@ -727,7 +727,8 @@ ${cases.join(',\n')}, // Enums if (field.isEnum) { - return field.type.getDisplayString(withNullability: false); + return field.displayTypeOverride ?? + field.type.getDisplayString(withNullability: false); } // Lists @@ -803,6 +804,10 @@ ${cases.join(',\n')}, return kMapType; } + if (field.collectionElementDisplayTypeOverride != null) { + return field.collectionElementDisplayTypeOverride!; + } + if (field.type is! ParameterizedType) return 'dynamic'; final paramType = field.type as ParameterizedType; diff --git a/packages/ack_generator/lib/src/models/field_info.dart b/packages/ack_generator/lib/src/models/field_info.dart index eece63f..1b38687 100644 --- a/packages/ack_generator/lib/src/models/field_info.dart +++ b/packages/ack_generator/lib/src/models/field_info.dart @@ -27,6 +27,13 @@ class FieldInfo { /// properly typed getters like `AddressType get address`. final String? nestedSchemaRef; + /// Optional display type override used when source qualification matters + /// (e.g., `alias.UserRole` from a prefixed import). + final String? displayTypeOverride; + + /// Optional collection element display type override for list/set fields. + final String? collectionElementDisplayTypeOverride; + const FieldInfo({ required this.name, required this.jsonKey, @@ -37,6 +44,8 @@ class FieldInfo { this.description, this.listElementSchemaRef, this.nestedSchemaRef, + this.displayTypeOverride, + this.collectionElementDisplayTypeOverride, }); /// Whether this field references another schema model diff --git a/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart b/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart index b030b16..0a1117e 100644 --- a/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart +++ b/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart @@ -29,9 +29,7 @@ final reviewSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type ReviewType(Map _data)', - ), + contains('extension type ReviewType(Map _data)'), contains('String get file'), contains("_data['file'] as String"), contains('String get severity'), @@ -65,9 +63,7 @@ final eventSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type EventType(Map _data)', - ), + contains('extension type EventType(Map _data)'), contains('String get type'), contains("_data['type'] as String"), contains('String get target'), @@ -101,9 +97,7 @@ final userSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type UserType(Map _data)', - ), + contains('extension type UserType(Map _data)'), contains('String get name'), contains('UserRole get role'), contains("_data['role'] as UserRole"), @@ -136,9 +130,7 @@ final formSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type FormType(Map _data)', - ), + contains('extension type FormType(Map _data)'), contains('String get title'), contains('String? get priority'), contains("_data['priority'] as String?"), @@ -172,9 +164,7 @@ final taskSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type TaskType(Map _data)', - ), + contains('extension type TaskType(Map _data)'), contains('String get title'), contains('Priority? get priority'), contains("_data['priority'] as Priority?"), @@ -205,9 +195,7 @@ final configSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type ConfigType(Map _data)', - ), + contains('extension type ConfigType(Map _data)'), contains('String get name'), contains('List get tags'), contains("(_data['tags'] as List).cast()"), @@ -240,9 +228,7 @@ final teamSchema = Ack.object({ outputs: { 'test_pkg|lib/schema.g.dart': decodedMatches( allOf([ - contains( - 'extension type TeamType(Map _data)', - ), + contains('extension type TeamType(Map _data)'), contains('String get name'), contains('List get roles'), contains("(_data['roles'] as List).cast()"), @@ -252,15 +238,88 @@ final teamSchema = Ack.object({ ); }); - test('mixed schema with literal, enumString, enumValues, and string fields', - () async { - final builder = ackGenerator(BuilderOptions.empty); + test( + 'enumValues with imported prefixed enum keeps prefixed field type', + () async { + final builder = ackGenerator(BuilderOptions.empty); - await testBuilder( - builder, - { - ...allAssets, - 'test_pkg|lib/schema.dart': ''' + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/enums.dart': ''' +enum UserRole { admin, editor, viewer } +''', + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; +import 'enums.dart' as models; + +@AckType() +final userSchema = Ack.object({ + 'role': Ack.enumValues(models.UserRole.values), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains('extension type UserType(Map _data)'), + contains('models.UserRole get role'), + contains("_data['role'] as models.UserRole"), + ]), + ), + }, + ); + }, + ); + + test( + 'top-level list enumValues preserves imported prefixed enum type', + () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/enums.dart': ''' +enum UserRole { admin, editor, viewer } +''', + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; +import 'enums.dart' as models; + +@AckType() +final roleListSchema = Ack.list(Ack.enumValues(models.UserRole.values)); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type RoleListType(List _value)', + ), + contains('implements List'), + contains('RoleListType(validated as List)'), + ]), + ), + }, + ); + }, + ); + + test( + 'mixed schema with literal, enumString, enumValues, and string fields', + () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' import 'package:ack/ack.dart'; import 'package:ack_annotations/ack_annotations.dart'; @@ -274,25 +333,26 @@ final widgetSchema = Ack.object({ 'style': Ack.enumString(['solid', 'outline', 'ghost']), }); ''', - }, - outputs: { - 'test_pkg|lib/schema.g.dart': decodedMatches( - allOf([ - contains( - 'extension type WidgetType(Map _data)', - ), - contains('String get type'), - contains("_data['type'] as String"), - contains('String get label'), - contains("_data['label'] as String"), - contains('Color get color'), - contains("_data['color'] as Color"), - contains('String get style'), - contains("_data['style'] as String"), - ]), - ), - }, - ); - }); + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains( + 'extension type WidgetType(Map _data)', + ), + contains('String get type'), + contains("_data['type'] as String"), + contains('String get label'), + contains("_data['label'] as String"), + contains('Color get color'), + contains("_data['color'] as Color"), + contains('String get style'), + contains("_data['style'] as String"), + ]), + ), + }, + ); + }, + ); }); } diff --git a/packages/ack_generator/test/src/test_utilities.dart b/packages/ack_generator/test/src/test_utilities.dart index b47aa6f..cac1d0a 100644 --- a/packages/ack_generator/test/src/test_utilities.dart +++ b/packages/ack_generator/test/src/test_utilities.dart @@ -50,6 +50,12 @@ class MockFieldInfo implements FieldInfo { @override final String? nestedSchemaRef; + @override + final String? displayTypeOverride; + + @override + final String? collectionElementDisplayTypeOverride; + final String typeName; final String? listItemTypeName; final String? mapKeyTypeName; @@ -74,6 +80,8 @@ class MockFieldInfo implements FieldInfo { this.mapValueTypeName, this.listElementSchemaRef, this.nestedSchemaRef, + this.displayTypeOverride, + this.collectionElementDisplayTypeOverride, }) : jsonKey = name; @override From d3c293d723935dc62028ddfab3362c4478e92b9b Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 16 Feb 2026 16:13:48 -0500 Subject: [PATCH 05/10] chore: apply pending workspace updates --- .github/workflows/ci.yml | 2 +- .github/workflows/release.yml | 20 +- code_review_2026-02-16_1541.md | 172 ++++-------------- docs/api-reference/index.mdx | 4 +- example/test/extension_type_to_json_test.dart | 28 +++ .../lib/src/schemas/transformed_schema.dart | 18 +- packages/ack_annotations/CHANGELOG.md | 1 + packages/ack_annotations/README.md | 4 +- packages/ack_generator/README.md | 6 +- .../integration/ack_type_to_json_test.dart | 48 +++++ pubspec.yaml | 1 + 11 files changed, 145 insertions(+), 159 deletions(-) create mode 100644 example/test/extension_type_to_json_test.dart create mode 100644 packages/ack_generator/test/integration/ack_type_to_json_test.dart diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc3e0f4..db52269 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,7 +8,7 @@ on: jobs: test: - uses: btwld/dart-actions/.github/workflows/ci.yml@main + uses: btwld/dart-actions/.github/workflows/ci.yml@9075ce1232ec77b8747953f2ff4a349190e5a805 secrets: token: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 09a0c18..3681a30 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,8 +6,26 @@ on: - 'v*' jobs: + validate-tag: + runs-on: ubuntu-latest + outputs: + semver_ok: ${{ steps.validate-tag.outputs.semver_ok }} + steps: + - name: Validate release tag as SemVer 2.0.0 with optional pre-release/build metadata + id: validate-tag + shell: bash + run: | + TAG="${{ github.ref_name }}" + if [[ ! "$TAG" =~ ^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-(0|[1-9][0-9]*|[0-9A-Za-z-]+\.)*[0-9A-Za-z-]+)?(\+(0|[1-9][0-9]*|[0-9A-Za-z-]+\.)*[0-9A-Za-z-]+)?$ ]]; then + echo "::error::Release tag '$TAG' is not SemVer 2.0.0 compatible. Use tags like v1.0.0, v1.0.0-alpha.1, or v1.0.0-beta.6+exp.sha." + exit 1 + fi + echo "semver_ok=true" >> "$GITHUB_OUTPUT" + publish: - uses: btwld/dart-actions/.github/workflows/publish.yml@main + needs: validate-tag + if: needs.validate-tag.outputs.semver_ok == 'true' + uses: btwld/dart-actions/.github/workflows/publish.yml@9075ce1232ec77b8747953f2ff4a349190e5a805 permissions: id-token: write with: diff --git a/code_review_2026-02-16_1541.md b/code_review_2026-02-16_1541.md index 1bfd1b9..8184f66 100644 --- a/code_review_2026-02-16_1541.md +++ b/code_review_2026-02-16_1541.md @@ -1,163 +1,64 @@ -# Code Review - 2026-02-16 (Consolidated) +# Code Review - 2026-02-16 (Final) Branch: `claude/install-dart-melos-UQthl` (24 commits vs `main`) ## Summary -| Category | Total | Should Fix | Notes | -|----------|-------|------------|-------| -| Code Quality | 7 | 6 | 1 | -| Code Hygiene | 3 | 1 | 2 | -| **Combined** | **10** | **7** | **3** | +| Category | Open | Already Fixed | Total Reviewed | +|----------|------|---------------|----------------| +| Code Quality | 1 | 6 | 7 | +| Code Hygiene | 1 | 0 | 3 | +| **Totals** | **2 actionable** | **6 resolved** | **10** | -No blockers. All findings validated by expert review and cross-checked against actual file contents. +No blockers. Branch is in good shape — 6 of 7 code quality findings were already addressed in later commits. --- -## Code Quality Findings +## Open Findings (Action Needed) -### F1 — Tag pattern too broad for publish pipeline - -**Location**: `.github/workflows/release.yml:6` -**Severity**: SHOULD_FIX - -The release workflow triggers on `- 'v*'`, which matches any tag starting with `v` (e.g., `vfoo`, `v`, `v1`). This is risky for a pipeline that publishes to pub.dev. - -**Note**: GitHub Actions uses glob matching, not regex. The original suggestion (`v[0-9]+.[0-9]+.[0-9]+*`) is regex-like and won't work as a tag filter. - -**Action**: Use a tighter glob like `v[0-9]*.[0-9]*.[0-9]*` and/or add a job-level validation step that checks the tag is valid semver before publishing. - ---- - -### F2 — Floating reusable workflow references - -**Locations**: -- `.github/workflows/release.yml:10` — `btwld/dart-actions/.github/workflows/publish.yml@main` -- `.github/workflows/ci.yml:11` — `btwld/dart-actions/.github/workflows/ci.yml@main` - -**Severity**: SHOULD_FIX - -Both workflows pin to `@main`, which is mutable. Upstream changes can silently alter CI/CD behavior without any change in this repo. - -**Action**: Pin both references to an immutable commit SHA or locked release tag. Update intentionally when the upstream action changes. - ---- - -### F3 — Quality gate gap for ack_annotations - -**Location**: Root `pubspec.yaml:53` (melos scripts) -**Severity**: SHOULD_FIX - -The `melos analyze` scope includes `ack`, `ack_generator`, `ack_firebase_ai`, `ack_json_schema_builder`, and `ack_example` but excludes `ack_annotations`. However, the release workflow (`.github/workflows/release.yml:16-17`) now publishes `ack_annotations`. - -**Action**: Add `ack_annotations` to the analyze and test script scopes so it gets the same quality gates as other published packages. - ---- - -### F4 — Version mismatch between docs and packages - -**Locations**: -- `packages/ack_generator/README.md:22` — shows `ack: ^1.0.0`, `ack_generator: ^1.0.0` -- `packages/ack_annotations/README.md:13` — shows `ack_annotations: ^1.0.0`, `ack_generator: ^1.0.0` - -**Severity**: SHOULD_FIX - -All package pubspec.yaml files show version `1.0.0-beta.6`. Users following the README examples will get dependency resolution failures because `^1.0.0` excludes prereleases. - -**Action**: Align README version constraints with actually published versions, or use a version-agnostic reference pattern. - ---- - -### F5 — API docs stale on discriminated schema signature - -**Location**: `docs/api-reference/index.mdx:21` (also line 288) -**Severity**: SHOULD_FIX - -Docs show `Map` for `Ack.object()` and discriminated unions, but code at `packages/ack/lib/src/ack.dart:36` now requires `Map>`. - -**Action**: Update the API reference to reflect the current type signature. - ---- - -### F6 — Breaking API change not communicated - -**Location**: `packages/ack_annotations/lib/src/ack_field.dart:14` -**Severity**: SHOULD_FIX - -`@AckField` changed from `required` parameter to `requiredMode` (using `AckFieldRequiredMode` enum). This is a breaking change for consumers, but `packages/ack_annotations/CHANGELOG.md` only lists it under generic "Improvements" with no breaking-change callout or migration guidance. - -**Action**: Add a breaking-change note to the changelog with migration instructions (e.g., `required: true` → `requiredMode: AckFieldRequiredMode.always`). - ---- - -### F7 — Missing test coverage for generated toJson() +### F7 — Missing test coverage for generated toJson() — SHOULD_FIX **Location**: `packages/ack_generator/lib/src/builders/type_builder.dart:94` -**Severity**: SHOULD_FIX -The generator now emits `toJson()` for all extension types (lines 94, 377-387). No `toJson` assertions exist in `packages/ack_generator/test/`. Extension-type behavior IS tested in integration tests (typed getters, parse/safeParse, list mapping), but `toJson()` specifically is not covered. +The generator emits `toJson()` for all extension types (object, primitive, collection). No `toJson()` assertions exist in `packages/ack_generator/test/`. Extension-type behavior IS tested in integration tests (typed getters, parse/safeParse, list mapping), but `toJson()` specifically is not covered. -**Action**: Add targeted tests for `toJson()` generation output and runtime behavior across object, primitive, and collection extension types. - ---- - -## Code Hygiene Findings - -### S1 — Redundant inline comments in schema.dart - -**Location**: `packages/ack/lib/src/schemas/schema.dart:271` -**Severity**: NOTE - -Comments at lines 271, 275, 293, 304 restate adjacent code (e.g., "Use centralized null handling" before `handleNullInput(...)`, "After null check, inputValue is guaranteed non-null" before `inputValue!`). +**Evidence**: `grep -r 'toJson' packages/ack_generator/test/` returns only `toJsonSchema` helper methods in `test_utils/test_assets.dart`. Zero `toJson()` assertions. -**Action**: Remove comments that narrate obvious operations. Keep only comments that explain *why*. +**Action**: Add targeted tests for `toJson()` generation output and runtime behavior across object, primitive, and collection extension types. --- -### S2 — Verbose narration in transformed_schema.dart +### S2 — Verbose narration in transformed_schema.dart — NOTE -**Location**: `packages/ack/lib/src/schemas/transformed_schema.dart:51` -**Severity**: SHOULD_FIX +**Location**: `packages/ack/lib/src/schemas/transformed_schema.dart:51-76` -Dense step-by-step comment narration around straightforward control flow. Measured density: ~61% for the cited block (lines 51-82), ~39% for the full file. The rationale is valid (explaining why `TransformedSchema` doesn't use centralized null handling), but it's expressed with excessive line-by-line narration. +The top-level rationale comment (lines 51-56) is valuable — it explains *why* TransformedSchema doesn't use centralized null handling. However, the inline comments at lines 63-72 are step-by-step narration of straightforward control flow. -**Action**: Compress to a concise 2-3 line rationale comment at method entry. Remove procedural commentary. +**Action**: Optional cleanup — compress inline comments to shorter form. Keep the rationale block at lines 51-56 as-is. --- -### S3 — Repeated test assertion pattern - -**Location**: `packages/ack/test/schemas/schema_equality_test.dart:12` -**Severity**: NOTE - -15 adjacent `expect(a, equals(b))` + `expect(a.hashCode, equals(b.hashCode))` pairs across the equality tests. This is standard test duplication, not AI slop. - -**Action**: Optional — extract a shared `expectEqualAndHash()` helper if the pattern continues to grow. - ---- +## Verified Fixed (No Action Needed) -## Dropped / Demoted from Original Review +These findings were identified during review but have already been addressed in the current branch state: -| Original | Disposition | Reason | -|----------|-------------|--------| -| F8 (comment-only override files) | **Dropped** | Melos-managed placeholders; deletion causes churn for no real benefit | -| S3 severity SHOULD_FIX | **Demoted to NOTE** | Test duplication is common and helper extraction is optional | -| S2 density "~77%" | **Corrected to ~61%** | Actual measurement: 19 comment / 31 nonblank lines in cited block | +| ID | Issue | How It's Fixed | +|----|-------|----------------| +| F1 | Tag `v*` too broad for publish | `release.yml:9-23` — `validate-tag` job with full semver 2.0.0 regex validation | +| F2 | Floating `@main` workflow refs | Both `release.yml:28` and `ci.yml:11` pinned to SHA `9075ce1` | +| F3 | ack_annotations missing from quality gates | `pubspec.yaml:59` — now included in analyze scope | +| F4 | README version mismatch (`^1.0.0` vs beta) | Both READMEs updated to `^1.0.0-beta.6` | +| F5 | Stale discriminated schema type in docs | `index.mdx:21` shows correct `Map>` | +| F6 | Breaking API change undocumented | `ack_annotations/CHANGELOG.md:7` — `**Breaking**:` callout with migration guide | --- -## Prioritized Action Items +## Low-Priority Notes (Optional) -| Priority | ID | Location | Issue | Action | -|----------|----|----------|-------|--------| -| 1 | F6 | `ack_field.dart:14` | Breaking API change undocumented | Add breaking-change note + migration guide | -| 2 | F2 | `release.yml:10`, `ci.yml:11` | Floating `@main` workflow refs | Pin to commit SHA | -| 3 | F3 | Root `pubspec.yaml:53` | ack_annotations missing from quality gates | Add to analyze/test scopes | -| 4 | F4 | `ack_generator/README.md:22`, `ack_annotations/README.md:13` | Docs show `^1.0.0`, packages are beta.6 | Align version references | -| 5 | F5 | `docs/api-reference/index.mdx:21` | Stale discriminated schema type in docs | Update to current signature | -| 6 | F1 | `release.yml:6` | Tag pattern `v*` too broad | Tighten glob + add semver validation step | -| 7 | F7 | `type_builder.dart:94` | `toJson()` generation untested | Add targeted toJson tests | -| 8 | S2 | `transformed_schema.dart:51` | Verbose narration (~61% comment density) | Compress to rationale-only | +| ID | Location | Issue | +|----|----------|-------| +| S1 | `schema.dart:271,275,293,304` | Inline comments restate obvious code ("Use centralized null handling", "Type compatibility check") | +| S3 | `schema_equality_test.dart:12` | 15 repeated equality/hashCode assertion pairs — standard test pattern, extract helper if it grows | --- @@ -165,13 +66,14 @@ Dense step-by-step comment narration around straightforward control flow. Measur - **Lines analyzed**: 11,192 - **AI-generated code likelihood**: **low** -- **Conclusion**: No significant AI slop patterns detected. Minor comment hygiene issues only. +- **Conclusion**: No significant AI slop patterns. Minor comment hygiene only. --- ## Methodology -1. **Phase 1**: Gathered full branch diff (2,999 lines across 24 commits) -2. **Phase 2**: Ran Architect agent (code quality, 5 domains) and Slop pipeline (3-stage detection) -3. **Phase 3**: Expert validation — Reviewer agent verified all findings against actual file contents -4. **Phase 4**: Cross-checked against domain expert feedback; corrected inaccuracies, dropped over-engineered findings +1. Gathered full branch diff (2,999 lines across 24 commits) +2. Ran Architect agent (code quality) and 3-stage Slop pipeline via Codex +3. Expert validation via Reviewer and Architect agents +4. Cross-checked against domain expert feedback; corrected inaccuracies +5. Final verification: read actual current files to confirm which findings are still open vs. already resolved diff --git a/docs/api-reference/index.mdx b/docs/api-reference/index.mdx index c0cb046..9006cd3 100644 --- a/docs/api-reference/index.mdx +++ b/docs/api-reference/index.mdx @@ -18,7 +18,7 @@ Entry point for creating schemas. See [Schema Types](../core-concepts/schemas.md - `Ack.enumString(List values)`: Creates a string schema that only accepts specific values. - `Ack.anyOf(List schemas)`: Creates an `AnyOfSchema` for union types. - `Ack.any()`: Creates an `AnySchema` that accepts any value. -- `Ack.discriminated({required String discriminatorKey, required Map schemas})`: Creates a discriminated union schema. +- `Ack.discriminated({required String discriminatorKey, required Map> schemas})`: Creates a discriminated union schema. ## `AckSchema` (Base Class) @@ -285,7 +285,7 @@ Schema for union types (value must match one of several schemas). Schema for polymorphic validation based on a discriminator field. -- Created using `Ack.discriminated({required String discriminatorKey, required Map schemas})` +- Created using `Ack.discriminated({required String discriminatorKey, required Map> schemas})` ### `AnySchema` diff --git a/example/test/extension_type_to_json_test.dart b/example/test/extension_type_to_json_test.dart new file mode 100644 index 0000000..c831da7 --- /dev/null +++ b/example/test/extension_type_to_json_test.dart @@ -0,0 +1,28 @@ +import 'package:ack_example/schema_types_primitives.dart'; +import 'package:ack_example/schema_types_simple.dart'; +import 'package:test/test.dart'; + +void main() { + group('Extension type toJson', () { + test('object extension type returns map data', () { + final user = UserType.parse({'name': 'Alice', 'age': 30, 'active': true}); + + expect(user.toJson(), {'name': 'Alice', 'age': 30, 'active': true}); + expect(user.toJson(), isA>()); + }); + + test('primitive extension type returns wrapped value', () { + final password = PasswordType.parse('mySecurePassword123'); + + expect(password.toJson(), 'mySecurePassword123'); + expect(password.toJson(), isA()); + }); + + test('collection extension type returns wrapped list', () { + final tags = TagsType.parse(['dart', 'ack']); + + expect(tags.toJson(), ['dart', 'ack']); + expect(tags.toJson(), isA>()); + }); + }); +} diff --git a/packages/ack/lib/src/schemas/transformed_schema.dart b/packages/ack/lib/src/schemas/transformed_schema.dart index 09ada78..8eca694 100644 --- a/packages/ack/lib/src/schemas/transformed_schema.dart +++ b/packages/ack/lib/src/schemas/transformed_schema.dart @@ -60,36 +60,24 @@ class TransformedSchema Object? inputValue, SchemaContext context, ) { - // Handle TransformedSchema's own defaultValue for null input. - // Clone the default to prevent mutation of shared state. - // This must happen BEFORE delegating to wrapped schema, because the wrapped - // schema might not accept null (e.g., non-nullable StringSchema). - // - // NOTE: cloneDefault() returns List or Map for - // collections, which cannot be safely cast to parameterized OutputType like - // List. We use runtime type checking: if the clone is type-compatible, - // use it; otherwise fall back to the original (accepts mutation risk for - // parameterized collection defaults, but avoids runtime TypeError). + // Handle defaults before delegation, since wrapped schemas may reject null. + // If cloning loses generic type information, fall back to the original value. if (inputValue == null && defaultValue != null) { final cloned = cloneDefault(defaultValue!); final safeDefault = (cloned is OutputType) ? cloned : defaultValue!; return applyConstraintsAndRefinements(safeDefault, context); } - // For non-null input OR null input without default: - // Delegate to underlying schema (handles type conversion, null validation, constraints) - // The inner schema determines if null is valid based on its own isNullable setting. + // Delegate validation/parsing to the wrapped schema for all other cases. final originalResult = schema.parseAndValidate(inputValue, context); if (originalResult.isFail) { return SchemaResult.fail(originalResult.getError()); } - // Transform the validated value (may be null if underlying schema is nullable) final validatedValue = originalResult.getOrNull(); try { final transformedValue = transformer(validatedValue); - // Apply TransformedSchema's own constraints and refinements return applyConstraintsAndRefinements(transformedValue, context); } catch (e, st) { return SchemaResult.fail( diff --git a/packages/ack_annotations/CHANGELOG.md b/packages/ack_annotations/CHANGELOG.md index 2711b52..910bdba 100644 --- a/packages/ack_annotations/CHANGELOG.md +++ b/packages/ack_annotations/CHANGELOG.md @@ -4,6 +4,7 @@ * **AckType**: Refined annotation parameters and improved type handling (#50). * **AckField**: Improved field annotation correctness (#50). +* **Breaking**: `AckField.required` was replaced by `requiredMode` (`AckFieldRequiredMode.auto|required|optional`). Migrate `@AckField(required: true)` to `@AckField(requiredMode: AckFieldRequiredMode.required)` and `required: false` to `requiredMode: AckFieldRequiredMode.optional`. ## 1.0.0-beta.5 (2026-01-14) diff --git a/packages/ack_annotations/README.md b/packages/ack_annotations/README.md index 2a3fc7b..b3cdadc 100644 --- a/packages/ack_annotations/README.md +++ b/packages/ack_annotations/README.md @@ -10,10 +10,10 @@ Add to your `pubspec.yaml` (check [pub.dev](https://pub.dev/packages/ack_annotat ```yaml dependencies: - ack_annotations: ^1.0.0 + ack_annotations: ^1.0.0-beta.6 dev_dependencies: - ack_generator: ^1.0.0 + ack_generator: ^1.0.0-beta.6 build_runner: ^2.4.0 ``` diff --git a/packages/ack_generator/README.md b/packages/ack_generator/README.md index 7ec0c5e..537cf15 100644 --- a/packages/ack_generator/README.md +++ b/packages/ack_generator/README.md @@ -19,11 +19,11 @@ Add the following dependencies to your `pubspec.yaml` (check [pub.dev](https://p ```yaml dependencies: - ack: ^1.0.0 - ack_annotations: ^1.0.0 + ack: ^1.0.0-beta.6 + ack_annotations: ^1.0.0-beta.6 dev_dependencies: - ack_generator: ^1.0.0 + ack_generator: ^1.0.0-beta.6 build_runner: ^2.4.0 ``` diff --git a/packages/ack_generator/test/integration/ack_type_to_json_test.dart b/packages/ack_generator/test/integration/ack_type_to_json_test.dart new file mode 100644 index 0000000..02b9236 --- /dev/null +++ b/packages/ack_generator/test/integration/ack_type_to_json_test.dart @@ -0,0 +1,48 @@ +import 'package:ack_generator/builder.dart'; +import 'package:build/build.dart'; +import 'package:build_test/build_test.dart'; +import 'package:test/test.dart'; + +import '../test_utils/test_assets.dart'; + +void main() { + group('@AckType toJson generation', () { + test('emits toJson for object, primitive, and collection types', () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +@AckType() +final userSchema = Ack.object({ + 'name': Ack.string(), +}); + +@AckType() +final passwordSchema = Ack.string(); + +@AckType() +final tagsSchema = Ack.list(Ack.string()); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains('extension type UserType(Map _data)'), + contains('Map toJson() => _data;'), + contains('extension type PasswordType(String _value)'), + contains('String toJson() => _value;'), + contains('extension type TagsType(List _value)'), + contains('List toJson() => _value;'), + ]), + ), + }, + ); + }); + }); +} diff --git a/pubspec.yaml b/pubspec.yaml index cf40b17..46d3972 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -56,6 +56,7 @@ melos: - ack_firebase_ai - ack_json_schema_builder - ack_example + - ack_annotations fix: run: melos exec -- "dart fix --apply" From a85b6907c1e97027617eafd4e518bc243ae0ccb9 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 16 Feb 2026 16:19:06 -0500 Subject: [PATCH 06/10] chore: remove review artifact and verify generated code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated examples verified — build_runner produces identical output, all toJson tests pass, and dart analyze is clean. --- code_review_2026-02-16_1541.md | 79 ---------------------------------- 1 file changed, 79 deletions(-) delete mode 100644 code_review_2026-02-16_1541.md diff --git a/code_review_2026-02-16_1541.md b/code_review_2026-02-16_1541.md deleted file mode 100644 index 8184f66..0000000 --- a/code_review_2026-02-16_1541.md +++ /dev/null @@ -1,79 +0,0 @@ -# Code Review - 2026-02-16 (Final) - -Branch: `claude/install-dart-melos-UQthl` (24 commits vs `main`) - -## Summary - -| Category | Open | Already Fixed | Total Reviewed | -|----------|------|---------------|----------------| -| Code Quality | 1 | 6 | 7 | -| Code Hygiene | 1 | 0 | 3 | -| **Totals** | **2 actionable** | **6 resolved** | **10** | - -No blockers. Branch is in good shape — 6 of 7 code quality findings were already addressed in later commits. - ---- - -## Open Findings (Action Needed) - -### F7 — Missing test coverage for generated toJson() — SHOULD_FIX - -**Location**: `packages/ack_generator/lib/src/builders/type_builder.dart:94` - -The generator emits `toJson()` for all extension types (object, primitive, collection). No `toJson()` assertions exist in `packages/ack_generator/test/`. Extension-type behavior IS tested in integration tests (typed getters, parse/safeParse, list mapping), but `toJson()` specifically is not covered. - -**Evidence**: `grep -r 'toJson' packages/ack_generator/test/` returns only `toJsonSchema` helper methods in `test_utils/test_assets.dart`. Zero `toJson()` assertions. - -**Action**: Add targeted tests for `toJson()` generation output and runtime behavior across object, primitive, and collection extension types. - ---- - -### S2 — Verbose narration in transformed_schema.dart — NOTE - -**Location**: `packages/ack/lib/src/schemas/transformed_schema.dart:51-76` - -The top-level rationale comment (lines 51-56) is valuable — it explains *why* TransformedSchema doesn't use centralized null handling. However, the inline comments at lines 63-72 are step-by-step narration of straightforward control flow. - -**Action**: Optional cleanup — compress inline comments to shorter form. Keep the rationale block at lines 51-56 as-is. - ---- - -## Verified Fixed (No Action Needed) - -These findings were identified during review but have already been addressed in the current branch state: - -| ID | Issue | How It's Fixed | -|----|-------|----------------| -| F1 | Tag `v*` too broad for publish | `release.yml:9-23` — `validate-tag` job with full semver 2.0.0 regex validation | -| F2 | Floating `@main` workflow refs | Both `release.yml:28` and `ci.yml:11` pinned to SHA `9075ce1` | -| F3 | ack_annotations missing from quality gates | `pubspec.yaml:59` — now included in analyze scope | -| F4 | README version mismatch (`^1.0.0` vs beta) | Both READMEs updated to `^1.0.0-beta.6` | -| F5 | Stale discriminated schema type in docs | `index.mdx:21` shows correct `Map>` | -| F6 | Breaking API change undocumented | `ack_annotations/CHANGELOG.md:7` — `**Breaking**:` callout with migration guide | - ---- - -## Low-Priority Notes (Optional) - -| ID | Location | Issue | -|----|----------|-------| -| S1 | `schema.dart:271,275,293,304` | Inline comments restate obvious code ("Use centralized null handling", "Type compatibility check") | -| S3 | `schema_equality_test.dart:12` | 15 repeated equality/hashCode assertion pairs — standard test pattern, extract helper if it grows | - ---- - -## AI Slop Assessment - -- **Lines analyzed**: 11,192 -- **AI-generated code likelihood**: **low** -- **Conclusion**: No significant AI slop patterns. Minor comment hygiene only. - ---- - -## Methodology - -1. Gathered full branch diff (2,999 lines across 24 commits) -2. Ran Architect agent (code quality) and 3-stage Slop pipeline via Codex -3. Expert validation via Reviewer and Architect agents -4. Cross-checked against domain expert feedback; corrected inaccuracies -5. Final verification: read actual current files to confirm which findings are still open vs. already resolved From 80379c40d211a9e95d8faf862d8c1bc64c2c8f5f Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 16 Feb 2026 22:01:36 -0500 Subject: [PATCH 07/10] fix(generator): prefer resolved types over source extraction for enumValues Source-based enum type extraction was prioritized over resolved static types in _extractEnumTypeNameFromInvocation, which could produce incorrect generated types when a non-enum class exposes a .values getter (e.g., Ack.enumValues(holder.values) would generate 'holder' instead of the actual enum type). --- .../lib/src/analyzer/schema_ast_analyzer.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index b95a658..dc45e38 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -630,16 +630,16 @@ class SchemaAstAnalyzer { /// references (e.g., `alias.UserRole`) are preserved in generated code. /// Falls back to source-based extraction if static typing is unavailable. String? _extractEnumTypeNameFromInvocation(MethodInvocation invocation) { - final sourceTypeName = _extractEnumTypeNameFromSource(invocation); - if (sourceTypeName != null) { - return sourceTypeName; - } - final resolvedType = _resolveEnumValuesType(invocation); if (resolvedType != null) { return resolvedType.getDisplayString(withNullability: false); } + final sourceTypeName = _extractEnumTypeNameFromSource(invocation); + if (sourceTypeName != null) { + return sourceTypeName; + } + return null; } From 58b136637cdefe878c1a40b1c466240854e7e41b Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 16 Feb 2026 22:15:04 -0500 Subject: [PATCH 08/10] refactor(generator): use Dart 3 switch expressions in analyzers Convert traditional switch/case statements to concise switch expressions in field_analyzer and schema_ast_analyzer for improved readability. --- .../lib/src/analyzer/field_analyzer.dart | 29 +++++++------------ .../lib/src/analyzer/schema_ast_analyzer.dart | 29 ++++++------------- 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/packages/ack_generator/lib/src/analyzer/field_analyzer.dart b/packages/ack_generator/lib/src/analyzer/field_analyzer.dart index 31a0f04..5d00a47 100644 --- a/packages/ack_generator/lib/src/analyzer/field_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/field_analyzer.dart @@ -69,14 +69,11 @@ class FieldAnalyzer { } // Tri-state mode is authoritative. - switch (_getRequiredMode(annotation)) { - case AckFieldRequiredMode.required: - return true; - case AckFieldRequiredMode.optional: - return false; - case AckFieldRequiredMode.auto: - return _inferRequiredFromField(field); - } + return switch (_getRequiredMode(annotation)) { + AckFieldRequiredMode.required => true, + AckFieldRequiredMode.optional => false, + AckFieldRequiredMode.auto => _inferRequiredFromField(field), + }; } bool _inferRequiredFromField(FieldElement2 field) { @@ -89,16 +86,12 @@ class FieldAnalyzer { .getField('requiredMode') ?.getField('index') ?.toIntValue(); - switch (modeIndex) { - case 0: - return AckFieldRequiredMode.auto; - case 1: - return AckFieldRequiredMode.required; - case 2: - return AckFieldRequiredMode.optional; - default: - return AckFieldRequiredMode.auto; - } + return switch (modeIndex) { + 0 => AckFieldRequiredMode.auto, + 1 => AckFieldRequiredMode.required, + 2 => AckFieldRequiredMode.optional, + _ => AckFieldRequiredMode.auto, + }; } List _extractConstraints( diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index dc45e38..33ab9b5 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -1440,26 +1440,15 @@ class SchemaAstAnalyzer { /// Used for generating string representations of types in list element contexts. /// For nested lists, this function is called recursively via [_parseListSchema]. String _mapSchemaMethodToType(String methodName) { - switch (methodName) { - case 'string': - case 'enumString': - case 'literal': - return 'String'; - case 'integer': - return 'int'; - case 'double': - return 'double'; - case 'boolean': - return 'bool'; - case 'object': - return _kMapType; - case 'list': - // Note: Nested lists are handled by _parseListSchema recursively - // This case exists for consistency but should not be reached in normal flow - return 'List'; - default: - return 'dynamic'; - } + return switch (methodName) { + 'string' || 'enumString' || 'literal' => 'String', + 'integer' => 'int', + 'double' => 'double', + 'boolean' => 'bool', + 'object' => _kMapType, + 'list' => 'List', + _ => 'dynamic', + }; } /// Validates that a field name is a valid Dart identifier From bf17755658a201b65ae515635ffdb52d2a0ef89d Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Mon, 16 Feb 2026 22:19:58 -0500 Subject: [PATCH 09/10] fix(generator): preserve qualified enum type names in enumValues --- docs/api-reference/index.mdx | 4 ++-- .../lib/src/analyzer/schema_ast_analyzer.dart | 24 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/docs/api-reference/index.mdx b/docs/api-reference/index.mdx index 9006cd3..20dde53 100644 --- a/docs/api-reference/index.mdx +++ b/docs/api-reference/index.mdx @@ -18,7 +18,7 @@ Entry point for creating schemas. See [Schema Types](../core-concepts/schemas.md - `Ack.enumString(List values)`: Creates a string schema that only accepts specific values. - `Ack.anyOf(List schemas)`: Creates an `AnyOfSchema` for union types. - `Ack.any()`: Creates an `AnySchema` that accepts any value. -- `Ack.discriminated({required String discriminatorKey, required Map> schemas})`: Creates a discriminated union schema. +- `Ack.discriminated({required String discriminatorKey, required Map>> schemas})`: Creates a discriminated union schema. ## `AckSchema` (Base Class) @@ -285,7 +285,7 @@ Schema for union types (value must match one of several schemas). Schema for polymorphic validation based on a discriminator field. -- Created using `Ack.discriminated({required String discriminatorKey, required Map> schemas})` +- Created using `Ack.discriminated({required String discriminatorKey, required Map>> schemas})` ### `AnySchema` diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index 33ab9b5..7ee9dc8 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -626,21 +626,25 @@ class SchemaAstAnalyzer { /// Extracts the enum type name from an `Ack.enumValues(...)` invocation. /// - /// Prefers resolved static types so imported/re-exported enums and prefixed - /// references (e.g., `alias.UserRole`) are preserved in generated code. - /// Falls back to source-based extraction if static typing is unavailable. + /// Prefers source text only when it contains a qualifier + /// (e.g., `alias.UserRole`) so import prefixes are preserved in generated + /// part files. + /// + /// For non-qualified names, prefers resolved static types to avoid + /// incorrectly treating arbitrary `.values` receivers as enum type names + /// (for example, `holder.values` should resolve to the list element type). String? _extractEnumTypeNameFromInvocation(MethodInvocation invocation) { + final sourceTypeName = _extractEnumTypeNameFromSource(invocation); + if (sourceTypeName != null && sourceTypeName.contains('.')) { + return sourceTypeName; + } + final resolvedType = _resolveEnumValuesType(invocation); if (resolvedType != null) { return resolvedType.getDisplayString(withNullability: false); } - final sourceTypeName = _extractEnumTypeNameFromSource(invocation); - if (sourceTypeName != null) { - return sourceTypeName; - } - - return null; + return sourceTypeName; } String? _extractEnumTypeNameFromSource(MethodInvocation invocation) { @@ -747,7 +751,7 @@ class SchemaAstAnalyzer { return scopeType; } - // Try import namespaces directly as a fallback for prefixed names. + // Try import namespaces directly as a fallback for simple imported names. for (final import in library.firstFragment.libraryImports) { final importedElement = import.namespace.get2(normalizedTypeName); final importedType = _resolveTypeFromElement(importedElement); From 530bb13103ff52eb6e9c7982309f91cd705c3e32 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Tue, 17 Feb 2026 21:13:21 -0500 Subject: [PATCH 10/10] fix(generator): resolve enumValues inference and copyWith enum prefixes --- .../lib/src/analyzer/schema_ast_analyzer.dart | 177 ++++++++++++++++-- .../lib/src/builders/type_builder.dart | 28 ++- .../ack_type_enum_literal_fields_test.dart | 81 ++++++++ 3 files changed, 269 insertions(+), 17 deletions(-) diff --git a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart index 7ee9dc8..695bb28 100644 --- a/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart +++ b/packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart @@ -660,17 +660,38 @@ class SchemaAstAnalyzer { final firstArg = args.first; if (firstArg is PrefixedIdentifier && firstArg.identifier.name == 'values') { - return firstArg.prefix.toSource(); + final targetSource = firstArg.prefix.toSource(); + if (_looksLikeTypeReference(targetSource)) { + return targetSource; + } } if (firstArg is PropertyAccess && firstArg.propertyName.name == 'values') { - return firstArg.target?.toSource(); + final targetSource = firstArg.target?.toSource(); + if (targetSource != null && _looksLikeTypeReference(targetSource)) { + return targetSource; + } } } return null; } + bool _looksLikeTypeReference(String source) { + final trimmed = source.trim(); + if (trimmed.isEmpty) return false; + + final identifier = trimmed.split('.').last; + if (identifier.isEmpty) return false; + + final firstCodeUnit = identifier.codeUnitAt(0); + const uppercaseA = 65; + const uppercaseZ = 90; + const underscore = 95; + return (firstCodeUnit >= uppercaseA && firstCodeUnit <= uppercaseZ) || + firstCodeUnit == underscore; + } + String? _extractListEnumElementTypeName(MethodInvocation listInvocation) { final args = listInvocation.argumentList.arguments; if (args.isEmpty) return null; @@ -715,16 +736,12 @@ class SchemaAstAnalyzer { final args = invocation.argumentList.arguments; if (args.isNotEmpty) { - final argType = args.first.staticType; - if (argType is InterfaceType) { - if (argType.isDartCoreList && argType.typeArguments.isNotEmpty) { - final elementType = argType.typeArguments.first; - if (elementType is InterfaceType) { - return elementType; - } - } else { - return argType; - } + final resolvedFromArgument = _resolveEnumValuesTypeFromArgument( + args.first, + library: library, + ); + if (resolvedFromArgument != null) { + return resolvedFromArgument; } } @@ -741,6 +758,142 @@ class SchemaAstAnalyzer { return null; } + DartType? _resolveEnumValuesTypeFromArgument( + Expression argument, { + LibraryElement2? library, + }) { + final enumFromStaticType = _extractEnumTypeFromCandidate( + argument.staticType, + ); + if (enumFromStaticType != null) { + return enumFromStaticType; + } + + if (library == null) { + return null; + } + + final resolvedExpressionType = _resolveExpressionType(argument, library); + return _extractEnumTypeFromCandidate(resolvedExpressionType); + } + + DartType? _extractEnumTypeFromCandidate(DartType? candidate) { + if (candidate is! InterfaceType) { + return null; + } + + if (candidate.element3 is EnumElement2) { + return candidate; + } + + if (candidate.isDartCoreList && candidate.typeArguments.isNotEmpty) { + final elementType = candidate.typeArguments.first; + if (elementType is InterfaceType && + elementType.element3 is EnumElement2) { + return elementType; + } + } + + return null; + } + + DartType? _resolveExpressionType( + Expression expression, + LibraryElement2 library, + ) { + final staticType = expression.staticType; + if (staticType != null && staticType is! DynamicType) { + return staticType; + } + + if (expression is SimpleIdentifier) { + final variableType = _schemaVarsByName(library)[expression.name]?.type; + if (variableType != null) { + return variableType; + } + + final getterType = _schemaGettersByName( + library, + )[expression.name]?.returnType; + if (getterType != null) { + return getterType; + } + + return _resolveTypeByName(expression.name, library); + } + + if (expression is PrefixedIdentifier) { + final targetType = _resolveExpressionType(expression.prefix, library); + if (targetType is InterfaceType) { + final memberType = _resolveClassMemberType( + targetType: targetType, + memberName: expression.identifier.name, + library: library, + ); + if (memberType != null) { + return memberType; + } + } + + return _resolveTypeByName(expression.toSource(), library); + } + + if (expression is PropertyAccess) { + final target = expression.target; + if (target != null) { + final targetType = _resolveExpressionType(target, library); + if (targetType is InterfaceType) { + final memberType = _resolveClassMemberType( + targetType: targetType, + memberName: expression.propertyName.name, + library: library, + ); + if (memberType != null) { + return memberType; + } + } + } + } + + return null; + } + + DartType? _resolveClassMemberType({ + required InterfaceType targetType, + required String memberName, + required LibraryElement2 library, + }) { + final className = targetType.element3?.name3; + if (className == null) return null; + + final classElement = _classesByName(library)[className]; + if (classElement == null) return null; + + final allFields = [ + ...classElement.fields2, + ...classElement.allSupertypes.expand((type) => type.element3.fields2), + ]; + + final field = allFields.cast().firstWhere( + (current) => current?.name3 == memberName, + orElse: () => null, + ); + if (field != null) { + return field.type; + } + + final allGetters = [ + ...classElement.getters2, + ...classElement.allSupertypes.expand((type) => type.element3.getters2), + ]; + + final getter = allGetters.cast().firstWhere( + (current) => current?.name3 == memberName, + orElse: () => null, + ); + return getter?.returnType; + } + DartType? _resolveTypeByName(String typeName, LibraryElement2 library) { final normalizedTypeName = typeName.trim(); if (normalizedTypeName.isEmpty) return null; diff --git a/packages/ack_generator/lib/src/builders/type_builder.dart b/packages/ack_generator/lib/src/builders/type_builder.dart index 78a01af..c20390e 100644 --- a/packages/ack_generator/lib/src/builders/type_builder.dart +++ b/packages/ack_generator/lib/src/builders/type_builder.dart @@ -968,11 +968,7 @@ ${cases.join(',\n')}, return Parameter( (p) => p ..name = field.name - ..type = _referenceFromDartType( - field.type, - forceNullable: true, - stripNullability: true, - ) + ..type = _buildCopyWithParameterType(field) ..named = true, ); }).toList(); @@ -1006,6 +1002,28 @@ ${assignments.join(',\n')}, ); } + Reference _buildCopyWithParameterType(FieldInfo field) { + if (field.isEnum && field.displayTypeOverride != null) { + return _typeReference(field.displayTypeOverride!, isNullable: true); + } + + if ((field.isList || field.isSet) && + field.collectionElementDisplayTypeOverride != null) { + final collectionType = field.isSet ? 'Set' : 'List'; + return _typeReference( + collectionType, + types: [_typeReference(field.collectionElementDisplayTypeOverride!)], + isNullable: true, + ); + } + + return _referenceFromDartType( + field.type, + forceNullable: true, + stripNullability: true, + ); + } + /// Builds the `args` getter that returns additional properties /// /// Returns a Map containing only properties that are not explicitly diff --git a/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart b/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart index 0a1117e..0b2b727 100644 --- a/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart +++ b/packages/ack_generator/test/integration/ack_type_enum_literal_fields_test.dart @@ -267,6 +267,7 @@ final userSchema = Ack.object({ contains('extension type UserType(Map _data)'), contains('models.UserRole get role'), contains("_data['role'] as models.UserRole"), + contains('models.UserRole? role'), ]), ), }, @@ -310,6 +311,41 @@ final roleListSchema = Ack.list(Ack.enumValues(models.UserRole.values)); }, ); + test( + 'list enumValues with imported prefixed enum keeps prefixed copyWith type', + () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/enums.dart': ''' +enum UserRole { admin, editor, viewer } +''', + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; +import 'enums.dart' as models; + +@AckType() +final teamSchema = Ack.object({ + 'roles': Ack.list(Ack.enumValues(models.UserRole.values)), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains('List get roles'), + contains('List? roles'), + ]), + ), + }, + ); + }, + ); + test( 'mixed schema with literal, enumString, enumValues, and string fields', () async { @@ -354,5 +390,50 @@ final widgetSchema = Ack.object({ ); }, ); + + test( + 'enumValues infers enum type from variable and property inputs', + () async { + final builder = ackGenerator(BuilderOptions.empty); + + await testBuilder( + builder, + { + ...allAssets, + 'test_pkg|lib/schema.dart': ''' +import 'package:ack/ack.dart'; +import 'package:ack_annotations/ack_annotations.dart'; + +enum UserRole { admin, editor, viewer } + +final roleValues = UserRole.values; + +class RoleHolder { + const RoleHolder(this.values); + final List values; +} + +const holder = RoleHolder(UserRole.values); + +@AckType() +final userSchema = Ack.object({ + 'roleFromVar': Ack.enumValues(roleValues), + 'roleFromProp': Ack.enumValues(holder.values), +}); +''', + }, + outputs: { + 'test_pkg|lib/schema.g.dart': decodedMatches( + allOf([ + contains('UserRole get roleFromVar'), + contains("_data['roleFromVar'] as UserRole"), + contains('UserRole get roleFromProp'), + contains("_data['roleFromProp'] as UserRole"), + ]), + ), + }, + ); + }, + ); }); }