Conversation
WalkthroughMigrates the codebase from analyzer Element2-era APIs to the current Element API. Updates imports, types, accessors, extensions, and several public signatures across core sources and templates. Adjusts error handling in tests to use diagnostics. Bumps dependencies (analyzer ^8, source_gen ^4) and adds pub_semver. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (1 passed, 2 inconclusive)❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/freezed/test/typedef_parameter_test.dart (1)
18-24: Fix wrong file path passed to getErrors (will cause false negatives/positives)The generated file is typedef_parameter.freezed.dart, not typedef_parameter_test.freezed.dart. Current path will not match the resolved library and may yield spurious results.
Apply:
- final errorResult = - await main.session.getErrors( - '/freezed/test/integration/typedef_parameter_test.freezed.dart', - ) + final errorResult = + await main.session.getErrors( + '/freezed/test/integration/typedef_parameter.freezed.dart', + ) as ErrorsResult;packages/freezed/lib/src/parse_generator.dart (1)
66-70: Shadowing parameter ‘annotation’ causes a compile-time error.
A local variable redeclares the method parameter. Rename the local to avoid the collision.Apply this diff:
- final annotation = typeChecker.firstAnnotationOf( + final dartObject = typeChecker.firstAnnotationOf( element, throwOnUnresolved: false, ); - if (annotation == null) return; + if (dartObject == null) return; @@ - final datas = <AnnotationMeta>[(declaration: ast, annotation: annotation)]; + final datas = <AnnotationMeta>[(declaration: ast, annotation: dartObject)];Also applies to: 90-91
packages/freezed/lib/src/tools/type.dart (1)
42-50: Guard against null imported libraries.
importedLibrary can be null on invalid imports; avoid a potential NPE.Apply this diff:
- final owner = originLibrary.firstFragment.prefixes.firstWhereOrNull((e) { - return e.imports.any((l) { - return l.importedLibrary!.anyTransitiveExport((library) { - return library.id == _getElementForType(type)?.library?.id; - }); - }); - }); + final owner = originLibrary.firstFragment.prefixes.firstWhereOrNull((e) { + return e.imports.any((l) { + final imported = l.importedLibrary; + if (imported == null) return false; + return imported.anyTransitiveExport( + (library) => library.id == _getElementForType(type)?.library?.id, + ); + }); + });
🧹 Nitpick comments (12)
packages/freezed/lib/src/templates/concrete_template.dart (1)
525-542: Guard against null constant values when scanning annotations.
computeConstantValue()can return null; avoid!to prevent crashes on unexpected/invalid annotations. Also minor tidy.Apply this diff:
- for (final meta in metadata.annotations) { - final obj = meta.computeConstantValue()!; - if (matcher.isExactlyType(obj.type!)) { + for (final meta in metadata.annotations) { + final obj = meta.computeConstantValue(); + if (obj != null && matcher.isExactlyType(obj.type!)) { final source = meta.toSource(); final res = source.substring('@Default('.length, source.length - 1); - var needsConstModifier = + final needsConstModifier = !baseElement.type.isDartCoreString && !res.trimLeft().startsWith('const') && (res.contains('(') || res.contains('[') || res.contains('{')); - if (needsConstModifier) { - return 'const $res'; - } else { - return res; - } + return needsConstModifier ? 'const $res' : res; } }packages/freezed/lib/src/templates/prototypes.dart (1)
44-46: Make default-constructor check robust (avoid relying solely on 'new')Depending on analyzer versions, the unnamed constructor may surface as name == '' and/or as 'new' for tear-offs. Prefer handling both.
Apply:
-bool isDefaultConstructor(ConstructorElement constructor) { - return constructor.name == 'new'; -} +bool isDefaultConstructor(ConstructorElement constructor) { + // Support both unnamed constructor ('') and tear-off surface ('new'). + return constructor.name.isEmpty || constructor.name == 'new'; +}packages/freezed/lib/src/templates/properties.dart (1)
117-118: RemoveparameterElementfromProperty.copyWith. It’s unused in the body and no callers pass it.packages/freezed/lib/src/templates/parameter_template.dart (1)
38-42: Avoid unnecessary null-assert onTypeParameterElement.name
nameis non-null for type parameters; drop!to reduce noise.- return GenericsParameterTemplate(generics.map((e) => e.name!).toList()); + return GenericsParameterTemplate(generics.map((e) => e.name).toList());packages/freezed/test/decorator_test.dart (2)
36-37: Preferelement.namefor readability
LibraryElement.library.nameequalsLibraryElement.name. Use the direct property.- (element) => element.library.name == 'decorator', + (element) => element.name == 'decorator',
53-59: Make assertion order-insensitive; don’t rely on.lastAnnotation order can change; assert presence instead.
- expect(unmodifiableGetter.metadata.annotations.length, 2); - expect(unmodifiableGetter.metadata.annotations.last.toSource(), '@Foo()'); + expect(unmodifiableGetter.metadata.annotations.length, 2); + expect( + unmodifiableGetter.metadata.annotations.map((a) => a.toSource()), + contains('@Foo()'), + );packages/freezed/lib/src/ast.dart (2)
32-35: Ignore static methods when checkingtoStringStatic
toStringshouldn’t count as a customized instancetoString.- for (final method in type.methods) { - if (method.name == 'toString') { + for (final method in type.methods) { + if (!method.isStatic && method.name == 'toString') { return true; } }
88-89: Minor: unnecessary!ifClassElement.nameis non-nullIf your minimum analyzer guarantees non-null
ClassElement.name, you can drop!. Otherwise, keep as-is.- final escapedElementName = classElement.name!.replaceAll(r'$', r'\$'); + final escapedElementName = classElement.name.replaceAll(r'$', r'\$');packages/freezed/lib/src/parse_generator.dart (2)
76-80: Prefer matching elements by identity, not name.
Using name equality can misidentify in edge cases; identity is simpler and safer here.Apply this diff:
- final Object? ast = unit.unit.declarations.firstWhereOrNull( - (declaration) => - declaration.declaredFragment?.element.name == element.name!, + final Object? ast = unit.unit.declarations.firstWhereOrNull( + (declaration) => + identical(declaration.declaredFragment?.element, element),
83-89: Tighten types and drop redundant runtime check.
Casting the local to Declaration? simplifies the flow.Apply this diff:
- if (ast is! Declaration) { - throw InvalidGenerationSourceError( - 'Ast is not a Declaration', - element: element, - ); - } + // ast is already a Declaration? above when typed accordinglyAnd adjust the declaration above:
- final Object? ast = unit.unit.declarations.firstWhereOrNull( + final Declaration? ast = unit.unit.declarations.firstWhereOrNull(packages/freezed/lib/src/tools/imports.dart (1)
3-34: Migration to LibraryElement-based traversal looks good; minor naming nit.
Variable namesexport/importcan be confusing; prefer non-keyword-ish identifiers for readability.Apply this diff:
- LibraryElement? visitLibrary(LibraryElement library) { + LibraryElement? visitLibrary(LibraryElement library) { @@ - for (final export in library.exportedLibraries) { - final result = visitLibrary(export); + for (final exported in library.exportedLibraries) { + final result = visitLibrary(exported); if (result != null) return result; } @@ - for (final import in exportedLibraries) { - final result = visitLibrary(import); + for (final exportedLibrary in exportedLibraries) { + final result = visitLibrary(exportedLibrary); if (result != null) return result; }packages/freezed/lib/src/tools/type.dart (1)
64-73: Use display strings for typedef type arguments.
Joining DartType directly relies on toString; prefer getDisplayString() for consistency.Apply this diff:
- if (type.alias!.typeArguments.isNotEmpty) { - displayType += '<${type.alias!.typeArguments.join(', ')}>'; - } + if (type.alias!.typeArguments.isNotEmpty) { + final args = type.alias!.typeArguments + .map((t) => t.getDisplayString()) + .join(', '); + displayType += '<$args>'; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/freezed/lib/src/ast.dart(3 hunks)packages/freezed/lib/src/models.dart(26 hunks)packages/freezed/lib/src/parse_generator.dart(3 hunks)packages/freezed/lib/src/templates/concrete_template.dart(2 hunks)packages/freezed/lib/src/templates/parameter_template.dart(4 hunks)packages/freezed/lib/src/templates/properties.dart(3 hunks)packages/freezed/lib/src/templates/prototypes.dart(2 hunks)packages/freezed/lib/src/tools/imports.dart(1 hunks)packages/freezed/lib/src/tools/recursive_import_locator.dart(7 hunks)packages/freezed/lib/src/tools/type.dart(4 hunks)packages/freezed/pubspec.yaml(1 hunks)packages/freezed/test/bidirectional_test.dart(1 hunks)packages/freezed/test/common.dart(2 hunks)packages/freezed/test/common_types_test.dart(1 hunks)packages/freezed/test/decorator_test.dart(3 hunks)packages/freezed/test/deep_copy_test.dart(3 hunks)packages/freezed/test/generic_test.dart(1 hunks)packages/freezed/test/generics_refs_test.dart(1 hunks)packages/freezed/test/json_test.dart(1 hunks)packages/freezed/test/map_test.dart(2 hunks)packages/freezed/test/multiple_constructors_test.dart(10 hunks)packages/freezed/test/optional_maybe_test.dart(1 hunks)packages/freezed/test/single_class_constructor_test.dart(5 hunks)packages/freezed/test/typedef_parameter_test.dart(2 hunks)packages/freezed/test/when_test.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-07T20:51:22.701Z
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Applied to files:
packages/freezed/lib/src/templates/concrete_template.dartpackages/freezed/pubspec.yamlpackages/freezed/lib/src/templates/parameter_template.dartpackages/freezed/lib/src/templates/properties.dart
📚 Learning: 2025-01-07T20:51:22.701Z
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
Applied to files:
packages/freezed/lib/src/templates/concrete_template.dartpackages/freezed/pubspec.yamlpackages/freezed/lib/src/templates/prototypes.dartpackages/freezed/lib/src/templates/properties.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: freezed (packages/freezed, stable, get)
🔇 Additional comments (55)
packages/freezed/test/map_test.dart (2)
306-306: Switch to diagnostics API looks correct.
Assertion updated toerrorResult.diagnostics; matches analyzer v8.
427-427: Same here — diagnostics usage LGTM.
Keeps the intent of the test.packages/freezed/pubspec.yaml (2)
13-13: Bump analyzer to ^8.0.0 — OK. Matches SDK constraint (>=3.8.0). Please verify locally by runningdart pub upgrade --major-versions dart pub outdated --mode=null-safetyto ensure there are no transitive compatibility issues.
18-18: Approve source_gen and source_gen_test version updates
Both dependencies are declared (source_gen ^4.0.0 and source_gen_test ^1.2.0); no issues found. Run generator tests to confirm compatibility.packages/freezed/test/bidirectional_test.dart (1)
23-23: Diagnostics assertion LGTM.
Keeps the zero-issues contract.packages/freezed/test/when_test.dart (1)
305-305: Updated to diagnostics — good.
Maintains the “errors present” check under the new API.packages/freezed/test/deep_copy_test.dart (3)
34-34: Diagnostics empty check — OK.
Matches analyzer v8.
52-52: Diagnostics empty check — OK (#2).
Consistent with above.
737-740: Use of diagnosticCode for name assertion looks correct.
Reads diagnostic identifiers reliably after API migration.packages/freezed/test/json_test.dart (1)
755-755: Update lingering pre-v8 API usages
- packages/freezed_lint/lib/src/tools/element_extensions.dart: remove
import 'package:analyzer/dart/element/element2.dart'(line 1)- packages/freezed_lint/lib/src/missing_private_empty_ctor.dart: replace
fields2(line 33) andAnalysisErrorparameters (lines 61–62)- packages/freezed_lint/lib/src/missing_mixin.dart: replace
AnalysisErrorparameters (lines 50–51)- packages/freezed/test/json_test.dart: replace
definedNames2usage (line 26)⛔ Skipped due to learnings
Learnt from: lishaduck PR: rrousselGit/freezed#1147 File: packages/freezed_lint/pubspec.yaml:12-12 Timestamp: 2025-01-07T20:51:22.701Z Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.Learnt from: lishaduck PR: rrousselGit/freezed#1147 File: packages/freezed_lint/pubspec.yaml:12-12 Timestamp: 2025-01-07T20:51:22.701Z Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.packages/freezed/test/generics_refs_test.dart (1)
24-24: Diagnostics empty check — OK.
Matches the rest of the suite.packages/freezed/lib/src/templates/concrete_template.dart (1)
2-2: Import migrated to element.dart — good.
Consistent with Element API.packages/freezed/test/optional_maybe_test.dart (1)
24-24: LGTM: switched to diagnosticsThe API migration to use ErrorsResult.diagnostics looks correct here.
packages/freezed/test/common_types_test.dart (1)
28-28: LGTM: diagnostics assertionUpdated assertion to diagnostics is consistent with analyzer ^8.
packages/freezed/lib/src/templates/prototypes.dart (1)
1-1: LGTM: Element API importelement2 → element import aligns with analyzer ^8.
packages/freezed/lib/src/templates/properties.dart (2)
2-2: LGTM: Element API importelement2 → element import is correct.
55-62: Verify non-nullelement.nameand metadata access
- Confirm that every
FormalParameterElementin your use cases has a non-nullname. By language design, constructor parameters (including field‐formals) always carry a declared name. No further guarding is needed here.- Ensure
element.metadata.annotationsremains the correct API in analyzer ^8; update toelement.metadataorelement.annotationsonly if compilation errors surface.packages/freezed/test/common.dart (2)
25-27: LGTM: diagnostics-based filteringCollecting Severity.error diagnostics is the right mapping for throwsCompileError.
36-36: LGTM: CompileError now carries DiagnosticType update aligns with analyzer ^8 diagnostics.
packages/freezed/test/generic_test.dart (1)
55-55: LGTM: diagnostics assertionMatches the new ErrorsResult API.
packages/freezed/test/single_class_constructor_test.dart (5)
3-3: LGTM: Element API importelement.dart import is correct for analyzer ^8.
28-28: LGTM: analyze() return typeFuture matches the new API usage.
214-236: LGTM: PropertyAccessorElement API usagename and documentationComment checks against PropertyAccessorElement work with analyzer ^8.
382-382: LGTM: diagnostics assertionConsistent with ErrorsResult.diagnostics.
654-656: LGTM: diagnostic text assertionMapping diagnostics to strings for contains(...) is fine.
packages/freezed/test/typedef_parameter_test.dart (1)
24-24: LGTM: diagnostics assertionOnce the path is corrected, this assertion matches analyzer ^8.
packages/freezed/lib/src/templates/parameter_template.dart (1)
72-80: Confirm metadata adapter type: Element vs AST annotations
parseDecorators(e.metadata.annotations)assumes a wrapper exposing.annotations. ForFormalParameterElement, analyzer exposesList<ElementAnnotation> metadata. Please verify your adapter normalizes both cases; otherwise passe.metadatadirectly.packages/freezed/test/multiple_constructors_test.dart (5)
5-5: Analyzer Element API import migration looks correct.
26-41: ClassElement/getField migration reads well.
Switch to ClassElement and getField plus InterfaceType assertions are consistent with analyzer 8.
66-117: Accessor/field expectations updated correctly.
Using PropertyAccessorElement/FieldElement with name/documentation checks aligns with the new API.
178-185: Diagnostics API update is correct.
ErrorsResult.diagnostics is the right property with analyzer 8.
576-596: Type display assertions look consistent.
The getDisplayString() expectations for nested collections match the migration elsewhere.Also applies to: 602-621, 629-645
packages/freezed/lib/src/tools/type.dart (1)
85-91: Dynamic fallback for InvalidType looks correct.
Using element.library.typeProvider.dynamicType is the right provider with analyzer 8.packages/freezed/lib/src/tools/recursive_import_locator.dart (6)
1-4: LGTM! Analyzer API migration is correctly implemented.The migration from
LibraryElement2toLibraryElementaligns with the analyzer ^8 upgrade mentioned in the PR objectives.
14-14: Return type updated correctly.The public API change from
Iterable<Element2>toIterable<Element>is consistent with the analyzer migration.
26-31: Internal helper signature and yield statement updated properly.The migration to
Elementtype and use ofchildrenaccessor is appropriate for the new analyzer API.
35-35: Library directive accessors migrated correctly.The change from
libraryExports2/libraryImports2tolibraryExports/libraryImportsfollows the new analyzer API conventions.Also applies to: 39-39
58-59: Element name accessor updated correctly.The migration from
element.name3toelement.nameis consistent with the new analyzer API. The visibility filtering logic remains functionally equivalent.
73-73: Library resolution properties migrated successfully.The changes from
exportedLibrary2/importedLibrary2toexportedLibrary/importedLibraryand theLibraryElementfield type update are all correct for the analyzer ^8 migration.Also applies to: 94-94, 116-116
packages/freezed/lib/src/models.dart (16)
7-7: Element import and extension target updated correctly.The migration from
Element2toElementin the import and the extension definition is properly implemented.Also applies to: 30-37
39-42: Constructor element extensions migrated properly.The extension now correctly targets
ConstructorElementinstead ofConstructorElement2. The use ofnameproperty (line 41, 54) is appropriate for the new API.Also applies to: 54-54
100-102: Type element handling updated correctly.The migration properly handles:
typeElementbeing cast toClassElement(lines 100-101)- Using
parameter.name!andtypeElement.name!(lines 119, 122)These changes align with the new analyzer API structure.
Also applies to: 119-120, 122-122
272-272: Redirected constructor name access updated.The change from
redirectedConstructor?.type.name2.lexemetoredirectedConstructor?.type.name.lexemeis correct for the new API.
286-286: Parameter name access migrated correctly.The change to
element.name!is appropriate for accessing parameter names in the new API.
321-321: Library URI access updated correctly.The change to
element.baseElement.library?.uriproperly accesses the library URI in the new analyzer API.
426-427: Public API signatures for annotations updated.The migration of
parseAllmethods to acceptConstructorElementinstead ofConstructorElement2is implemented correctly for bothImplementsAnnotationandWithAnnotation.Also applies to: 453-456
439-439: Library resolution in annotations updated.Both uses of
constructor.library(instead ofconstructor.library2) are correct for resolving type strings.Also applies to: 466-466
525-525: Super mixin name comparison updated.The change from
name2tonamewith.lexemeaccessor is appropriate.
555-555: Library getter type updated correctly.The return type change from
LibraryElement2toLibraryElementaligns with the analyzer migration.
573-573: Field and type parameter access migrated properly.The changes correctly use:
element.fieldsinstead ofelement.fields2element.typeParametersinstead ofelement.typeParameters2- Direct field name access with
p.name!All align with the new analyzer API.
Also applies to: 679-680, 683-684, 686-686
748-748: Type name extraction updated.The use of
name.lexemeto extract type names from NamedType elements is correct.
836-839: Metadata extraction pattern updated for new API.The pattern matching for metadata with
Fragment(metadata:)andElement(metadata:)cases properly handles the new analyzer API structure for accessing annotations.
868-870: Library and type system access updated.The migration correctly uses:
element.libraryfor accessing the librarylibrary.typeSystemandlibrary.typeProviderfor type operationsThese changes align with the new analyzer API conventions.
Also applies to: 924-924, 970-970
1067-1067: Field element API usage updated.The migration properly handles:
FieldElementtype (line 1067)field.setteraccessor (line 1072)field.getterproperties and methods (lines 1081-1083)All changes are consistent with the analyzer ^8 API.
Also applies to: 1072-1072, 1081-1083
1135-1135: Library import checking extensions updated.The extension now correctly:
- Targets
LibraryElementinstead ofLibraryElement2- Uses the updated
findAllAvailableTopLevelElements()method- Accesses
element.nameandelement.librarywith the new API patternsThe import checking logic for JsonSerializable and Diagnosticable remains functionally equivalent.
Also applies to: 1138-1138, 1363-1377
| String get fullName { | ||
| final classElement = declaredFragment!.element.enclosingElement2; | ||
| final classElement = declaredFragment!.element.enclosingElement; | ||
|
|
||
| var generics = classElement.typeParameters2 | ||
| .map((e) => '\$${e.name3}') | ||
| var generics = classElement.typeParameters | ||
| .map((e) => '\$${e.name}') | ||
| .join(', '); | ||
| if (generics.isNotEmpty) { | ||
| generics = '<$generics>'; | ||
| } | ||
|
|
||
| final className = classElement.enclosingElement2.name3; | ||
| final className = classElement.enclosingElement.name; | ||
|
|
||
| return name == null ? '$className$generics' : '$className$generics.$name'; | ||
| } |
There was a problem hiding this comment.
Bug: fullName uses the library name instead of the class name
classElement.enclosingElement.name returns the library’s name. Use the class element’s name.
String get fullName {
final classElement = declaredFragment!.element.enclosingElement;
var generics = classElement.typeParameters
.map((e) => '\$${e.name}')
.join(', ');
if (generics.isNotEmpty) {
generics = '<$generics>';
}
- final className = classElement.enclosingElement.name;
+ final className = classElement.name!;
return name == null ? '$className$generics' : '$className$generics.$name';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String get fullName { | |
| final classElement = declaredFragment!.element.enclosingElement2; | |
| final classElement = declaredFragment!.element.enclosingElement; | |
| var generics = classElement.typeParameters2 | |
| .map((e) => '\$${e.name3}') | |
| var generics = classElement.typeParameters | |
| .map((e) => '\$${e.name}') | |
| .join(', '); | |
| if (generics.isNotEmpty) { | |
| generics = '<$generics>'; | |
| } | |
| final className = classElement.enclosingElement2.name3; | |
| final className = classElement.enclosingElement.name; | |
| return name == null ? '$className$generics' : '$className$generics.$name'; | |
| } | |
| String get fullName { | |
| final classElement = declaredFragment!.element.enclosingElement; | |
| var generics = classElement.typeParameters | |
| .map((e) => '\$${e.name}') | |
| .join(', '); | |
| if (generics.isNotEmpty) { | |
| generics = '<$generics>'; | |
| } | |
| final className = classElement.name!; | |
| return name == null ? '$className$generics' : '$className$generics.$name'; | |
| } |
🤖 Prompt for AI Agents
In packages/freezed/lib/src/ast.dart around lines 63 to 76, the code grabs the
enclosingElement (library) instead of the class element so className becomes the
library name; change the code so classElement refers to
declaredFragment!.element (the class element) and use classElement.name for the
className, keeping the existing handling of typeParameters/generics and adding a
null-safe fallback (e.g., empty string) if name is null to avoid crashes.
| factory GenericsDefinitionTemplate.fromGenericElement( | ||
| List<TypeParameterElement2> generics, | ||
| List<TypeParameterElement> generics, | ||
| ) { | ||
| return GenericsDefinitionTemplate( | ||
| generics.map((e) => e.displayString2()).toList(), | ||
| generics.map((e) => e.displayString()).toList(), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include type parameter bounds explicitly; displayString() can drop/alter them
Relying on TypeParameterElement.displayString() may omit or format bounds/nullability inconsistently. Build the string from name and bound for deterministic output.
Apply:
factory GenericsDefinitionTemplate.fromGenericElement(
List<TypeParameterElement> generics,
) {
return GenericsDefinitionTemplate(
- generics.map((e) => e.displayString()).toList(),
+ generics
+ .map((e) {
+ final name = e.name;
+ final bound = e.bound;
+ if (bound != null) {
+ final b = bound.getDisplayString(withNullability: false);
+ return '$name extends $b';
+ }
+ return name;
+ })
+ .toList(),
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| factory GenericsDefinitionTemplate.fromGenericElement( | |
| List<TypeParameterElement2> generics, | |
| List<TypeParameterElement> generics, | |
| ) { | |
| return GenericsDefinitionTemplate( | |
| generics.map((e) => e.displayString2()).toList(), | |
| generics.map((e) => e.displayString()).toList(), | |
| ); | |
| factory GenericsDefinitionTemplate.fromGenericElement( | |
| List<TypeParameterElement> generics, | |
| ) { | |
| return GenericsDefinitionTemplate( | |
| generics | |
| .map((e) { | |
| final name = e.name; | |
| final bound = e.bound; | |
| if (bound != null) { | |
| final b = bound.getDisplayString(withNullability: false); | |
| return '$name extends $b'; | |
| } | |
| return name; | |
| }) | |
| .toList(), | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/freezed/lib/src/templates/parameter_template.dart around lines
13–18, the code uses TypeParameterElement.displayString(), which can omit or
alter bounds and nullability; change it to construct each type parameter string
deterministically from the element's name and its bound: use e.name, and if
e.bound is non-null append " extends " plus the bound's display string with
nullability preserved (e.g. getDisplayString(withNullability: true)); keep the
resulting list/ordering the same and return that list to the
GenericsDefinitionTemplate constructor.
| var constructor = freezedClass.constructors.firstWhere( | ||
| (element) => element.name == 'new', | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Sanity check: constructor selector uses name == 'new'
If analyzer exposes the unnamed constructor as 'new' in constructors, this is fine. Otherwise, consider also accepting name.isEmpty for robustness (mirrors suggestion in prototypes.dart).
You can confirm with:
🏁 Script executed:
#!/bin/bash
# Inspect constructors surfaced for the target class in the analyzed library (string-based quick check)
rg -nP -C2 'class\s+_ClassWithTypedef\s' test/integration/typedef_parameter.dartLength of output: 79
🏁 Script executed:
#!/bin/bash
set -e
echo "Files matching typedef_parameter_test.dart:"
fd typedef_parameter_test.dartLength of output: 93
🏁 Script executed:
#!/bin/bash
set -e
echo "Inspect occurrences of ClassWithTypedef in the test file:"
rg -nP -C3 'ClassWithTypedef' -n packages/freezed/test/typedef_parameter_test.dartLength of output: 258
🏁 Script executed:
#!/bin/bash
set -e
echo "Directories matching 'integration':"
fd -t d '*integration*' || true
echo
echo "Search for 'integration' in Dart files:"
rg -n 'integration' --type dart || trueLength of output: 8311
#!/bin/bash
set -e
if [ -d "packages/freezed/test/integration" ]; then
echo "Listing contents of packages/freezed/test/integration:"
ls -1 packages/freezed/test/integration
else
echo "Directory does not exist: packages/freezed/test/integration"
fi#!/bin/bash
set -e
echo "Checking existence of integration file:"
if [ -f "packages/freezed/test/integration/typedef_parameter.dart" ]; then
echo "Found typedef_parameter.dart, displaying first 200 lines:"
sed -n '1,200p' packages/freezed/test/integration/typedef_parameter.dart
else
echo "File not found: packages/freezed/test/integration/typedef_parameter.dart"
exit 1
fi🏁 Script executed:
#!/bin/bash
set -e
echo "Listing packages/freezed/test directory:"
ls -1 packages/freezed/testLength of output: 674
#!/bin/bash
set -e
echo "Contents of packages/freezed/test/integration directory:"
ls -1 packages/freezed/test/integration
echo
echo "Display first 200 lines of typedef_parameter.dart if it exists:"
if [ -f packages/freezed/test/integration/typedef_parameter.dart ]; then
sed -n '1,200p' packages/freezed/test/integration/typedef_parameter.dart
else
echo "File not found: packages/freezed/test/integration/typedef_parameter.dart"
fi🏁 Script executed:
#!/bin/bash
set -e
echo "Listing packages/freezed/test/integration directory:"
ls -1 packages/freezed/test/integration
echo
echo "Checking for typedef_parameter.dart:"
if [ -f packages/freezed/test/integration/typedef_parameter.dart ]; then
echo "Displaying first 200 lines of typedef_parameter.dart:"
sed -n '1,200p' packages/freezed/test/integration/typedef_parameter.dart
else
echo "File not found: packages/freezed/test/integration/typedef_parameter.dart"
fiLength of output: 1359
Support unnamed constructor detection
In typedef_parameter_test.dart, update the constructor selector to detect the unnamed constructor (whose name is an empty string) instead of—or in addition to—matching 'new'. e.g.:
var constructor = freezedClass.constructors.firstWhere(
(element) => element.name.isEmpty, // unnamed constructor
);🤖 Prompt for AI Agents
In packages/freezed/test/typedef_parameter_test.dart around lines 42 to 44, the
test currently selects a constructor by matching element.name == 'new' which
fails for unnamed constructors whose name is an empty string; update the
selector to detect unnamed constructors by checking element.name.isEmpty (or
allow both by checking name == 'new' || name.isEmpty) so the test finds the
unnamed constructor correctly.
This reverts commit 7ee3153.
1 participant
Summary by CodeRabbit
Breaking Changes
Refactor
Chores