Support using Freezed on normal Dart classes#1157
Conversation
WalkthroughThe pull request updates several aspects of the Freezed package. In the build configuration, the file inclusion/exclusion lists were modified for test files. In the AST file, the documentation getter now returns a nullable string with an added conditional check. The Freezed generator has been refactored to remove the intermediary properties class, and models were extended with new extension methods and property adaptations. Templates now accept additional parameters (such as globalData) and include significant refactorings (including new sealed classes and enum definitions) along with adjusted method signatures. Several tests were updated with minor comment removals and added override annotations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CopyWith
Client->>CopyWith: call copyWithGetter(needsCast: true/false)
CopyWith->>CopyWith: Evaluate needsCast parameter
alt needsCast true
CopyWith-->>Client: Return copyWith with cast applied
else
CopyWith-->>Client: Return copyWith without cast
end
sequenceDiagram
participant FromJson
participant ClassData
FromJson->>ClassData: Check for presence of constructors
alt No constructors found
ClassData-->>FromJson: Indicate absence
FromJson-->>Client: Return empty JSON conversion string
else
ClassData-->>FromJson: Provide constructors details
FromJson-->>Client: Process and return JSON conversion logic
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/freezed/lib/src/templates/properties.dart (1)
125-156:⚠️ Potential issueFix potential null reference in copyWith method.
The
isSyntheticparameter is missing in the method signature but is used directly in the constructor call. This could lead to unexpected behavior.Apply this diff to fix the issue:
Property copyWith({ String? type, String? name, bool? isNullable, bool? isDartList, bool? isDartMap, bool? isDartSet, bool? isFinal, + bool? isSynthetic, List<String>? decorators, String? defaultValueSource, bool? hasJsonKey, String? doc, bool? isPossiblyDartCollection, ParameterElement? parameterElement, }) { return Property( type: type ?? this.type, name: name ?? this.name, - isSynthetic: isSynthetic, + isSynthetic: isSynthetic ?? this.isSynthetic, isNullable: isNullable ?? this.isNullable, // ... rest of the parameters ); }
🧹 Nitpick comments (2)
packages/freezed/lib/src/templates/concrete_template.dart (2)
14-18: Initial sealed class hierarchy.
ClassesFoo,Bar, andBazillustrate a sealed hierarchy. If these are just placeholders, consider relocating them to test or sample code to avoid confusion in production.
107-108: Potential out-of-bounds error.
.where(...).firstcan throw ifp.nameis not found. Consider using.firstWhereOrNullor a null safety check to avoid potential runtime exceptions.- final correspondingProperty = constructor.properties - .where((element) => element.name == p.name) - .first; + final correspondingProperty = constructor.properties + .firstWhereOrNull((element) => element.name == p.name); + if (correspondingProperty == null) { + // Handle missing property scenario + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/freezed/build.yaml(1 hunks)packages/freezed/lib/src/ast.dart(2 hunks)packages/freezed/lib/src/freezed_generator.dart(3 hunks)packages/freezed/lib/src/models.dart(15 hunks)packages/freezed/lib/src/templates/abstract_template.dart(2 hunks)packages/freezed/lib/src/templates/concrete_template.dart(6 hunks)packages/freezed/lib/src/templates/copy_with.dart(1 hunks)packages/freezed/lib/src/templates/from_json_template.dart(1 hunks)packages/freezed/lib/src/templates/parameter_template.dart(3 hunks)packages/freezed/lib/src/templates/properties.dart(5 hunks)packages/freezed/test/concrete_test.dart(0 hunks)packages/freezed/test/decorator_test.dart(0 hunks)packages/freezed/test/deep_copy_test.dart(0 hunks)packages/freezed/test/integration/deep_copy.dart(1 hunks)packages/freezed/test/integration/extend.dart(1 hunks)packages/freezed/test/integration/multiple_constructors.dart(3 hunks)packages/freezed/test/integration/single_class_constructor.dart(4 hunks)packages/freezed/test/manual_test.dart(1 hunks)
💤 Files with no reviewable changes (3)
- packages/freezed/test/decorator_test.dart
- packages/freezed/test/deep_copy_test.dart
- packages/freezed/test/concrete_test.dart
✅ Files skipped from review due to trivial changes (2)
- packages/freezed/test/integration/deep_copy.dart
- packages/freezed/test/integration/multiple_constructors.dart
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: freezed (packages/freezed_lint, master, get)
- GitHub Check: freezed (packages/freezed_annotation, master, get)
- GitHub Check: freezed (packages/freezed, master, get)
🔇 Additional comments (44)
packages/freezed/lib/src/templates/from_json_template.dart (1)
15-17: Well-structured change that improves flexibility!The addition of the early return for classes without constructors, along with the clear documentation, is an excellent approach to supporting manual Dart classes. This change:
- Prevents unwanted JSON serialization code generation for manual classes
- Respects the original class structure
- Avoids potential compilation issues from adding annotations in parts
- Maintains existing behavior for classes that need JSON serialization
packages/freezed/test/integration/extend.dart (2)
1-3: LGTM!The import and part statement are correctly set up for using Freezed annotations.
5-8: LGTM!The base class is well-designed with:
- Immutable implementation using
constconstructor- Final field for value
- Clear and concise named constructor
packages/freezed/lib/src/templates/abstract_template.dart (4)
4-4: New import looks consistent.
The import ofconcrete_template.dartis justified, as this file now references themethods(...)function from that module.
13-19: AddedglobalDatain constructor and field.
Injecting aLibrary globalDataprovides broader contextual data to template generation. Ensure all call sites supply this new parameter consistently to avoid runtime errors.
23-32: Conditional property generation strategy.
UsingneedsAbstractGettersto always generate abstract getters iftoJson,==,toString, orcopyWithis enabled, otherwise only for synthetic fields, aligns with Freezed’s design goals. This logic appears correct and maintains backward compatibility.
44-52: PassingglobalDataintomethods(...).
IncludingglobalDataexpands the context for method generation, and callingcopyWithGetter(needsCast: true)in mixin scenarios is a neat solution for ensuring correct type casts. All changes here look good.packages/freezed/lib/src/templates/concrete_template.dart (11)
39-39: Union key property check.
constructor.properties.every((e) => e.name != data.options.annotation.unionKey)ensures no naming collisions on theunionKey. This is a good safeguard.
53-67: Expanded constructor output and copyWith usage.
Displaying_properties, generatingcopyWithGetter(needsCast: false), and passingsource: Source.syntheticClassintomethods(...)align with the new approach for synthetic classes. Also exposingcopyWith?.interfaceandcopyWith?.concreteImpl(...)is consistent.
233-234: Filtering synthetic properties.
Narrowingconstructor.propertiesto only those markedisSyntheticis consistent with the new design, ensuring that the “real” fields remain unaffected.
312-312: Closing brace formatting.
This trailing brace marks the end of_concreteFromJsonConstructor. Everything is syntactically consistent.
314-317: New enumSource.
DefiningSource.mixinandSource.syntheticClassclarifies where particular template code is generated. This is a sensible enum extension.
319-344: Aggregated method generator.
Themethods(...)function centralizes the generation oftoJson,debugFillProperties,operator==,hashCode, andtoString. It's a clean approach, reducing duplication across templates.
346-386:toJsontemplate logic.
Supporting different behaviors forSource.mixinvs.Source.syntheticClasscovers manual vs. generated classes. OmittingtoJsonfor manual classes is consistent with the constraints of part files.
388-410:debugFillPropertiesfor diagnostics.
Conditionally generating diagnostic properties ifglobalData.hasDiagnosticsis a helpful approach, ensuring minimal overhead when not needed.
412-435:toStringMethodwith optional diagnostic parameter.
Providing a parameter for controllingDiagnosticLevelis a thoughtful addition. This helps unify debugging experiences for classes implementingDiagnosticableTreeMixin.
437-474:operator==with deep collection equality.
This method gracefully handles dart:core collections, employingDeepCollectionEqualityto compare nested elements.
476-521:hashCodeMethodhandles large property sets.
Falling back toObject.hashAllfor >= 20 fields is a robust strategy, avoiding overly largehashcalls. The approach is well structured and maintains consistency with the rest of Freezed.packages/freezed/lib/src/models.dart (12)
148-162: Formal parameter type extraction.
ThetypeAnnotation()extension onFormalParametercovers different AST parameter variants. This improves reliability when analyzing parameter types.
191-193: Tracking redirected factory constructors.
DefiningfreezedCtorsas those withfactoryKeywordand aredirectedConstructorensures we only handle relevant union constructors.
195-199: Single non-factory constructor rule.
Strengthening the error message enforces that only one non-factory constructor is allowed for a@freezed-annotated class. This clarifies usage and prevents accidental misuse.
290-292: Synthesizing properties from formal parameters.
Call toProperty.fromFormalParameterensures consistent property definitions for each constructor’s parameters, factoring in synthetic vs. real fields.
338-338: Parameter forwarding.
Forwardingconstructor.parameters.parameterstoParametersTemplateis essential for uniform generation of parameter-based code.
470-480:ConstructorInvocationrefactor.
Renaming and repurposing the old super invocation logic intoConstructorInvocationclarifies how super calls or named constructors are invoked.
481-485: IntroducingCopyWithTarget.
Encapsulating the name and parameters forcopyWithlogic is a neat abstraction that simplifies passing around relevant details.
493-493: Note on line 493.
No~was shown for this line in the snippet; skipping comment.
593-614:_computeCloneablePropertieslogic for single vs. multiple constructors.
Generating cloneable properties either from shared constructor parameters or from a dedicatedcopyWithTargetcovers both union and single-constructor scenarios. Good coverage.
618-690:_computeReadablePropertiesmerges fields and constructor parameters.
By collecting definitions from both actual fields and constructor parameters, this ensures a complete view of properties for the user-facing API. Thoughtful approach.
931-942: Escaping class names inescapedName.
Replacing$in the class name and applying generics transforms avoids collisions or syntax errors.
944-950: SeparatePropertyListfor readability vs. cloneability.
Decoupling which fields appear in the abstract interface vs. copyWith calls clarifies usage, keeping the code base tidy.packages/freezed/lib/src/ast.dart (1)
5-17: LGTM! Improved null-safety handling for documentation.The change to return
nullinstead of an empty string when no documentation exists is a good practice. It makes the API more explicit about the absence of documentation.packages/freezed/test/manual_test.dart (2)
7-18: Add tests for JSON serialization.The
JsonManualclass implementsfromJsonandtoJsonmethods, but there are no tests verifying this functionality.Would you like me to generate test cases for JSON serialization?
75-135: LGTM! Comprehensive test coverage for core functionality.The test cases thoroughly verify:
- copyWith functionality
- equality comparisons
- toString implementation
- constructor behaviors
packages/freezed/lib/src/freezed_generator.dart (1)
50-68: LGTM! Simplified property handling.The removal of the
CommonPropertiesclass and direct usage ofdata.propertiesreduces complexity while maintaining functionality. This is a good example of the KISS principle.Also applies to: 105-119
packages/freezed/lib/src/templates/copy_with.dart (2)
64-78: LGTM! The newcopyWithGettermethod improves type safety.The method now handles type casting more explicitly through the
needsCastparameter, ensuring proper type safety when generating thecopyWithgetter.
83-124: LGTM! Enhanced pattern matching forcopyWithTarget.The changes improve the handling of
copyWithTargetby:
- Adding a condition to check for
copyWithTarget- Using pattern matching to determine the return type
- Maintaining the existing functionality for cases without a target
packages/freezed/test/integration/single_class_constructor.dart (4)
380-382: LGTM! Explicit override annotation added.The
@overrideannotation clarifies thatcontaineris overriding a member from a superclass or mixin.
401-404: LGTM! Explicit override annotations added.The
@overrideannotations clarify that bothbandcare overriding members from a superclass or mixin.
412-414: LGTM! Explicit override annotation added.The
@overrideannotation clarifies thatfirstis overriding a member from a superclass or mixin.
421-428: LGTM! Explicit override annotation added and improved readability.The
@overrideannotation clarifies thatoddis overriding a member from a superclass or mixin. The implementation is clear and readable.packages/freezed/lib/src/templates/parameter_template.dart (3)
66-67: LGTM! Enhanced parameter type flexibility.Changed parameter type from
FormalParameterListtoIterable<FormalParameter>to accept a wider range of inputs.
84-85: LGTM! Improved documentation handling.Added null coalescing operator to ensure
docfield is never null by defaulting to an empty string.
95-104: LGTM! Streamlined parameter processing.Direct use of the
parametersiterable simplifies the code by removing the need to access a nestedparametersproperty.packages/freezed/build.yaml (1)
9-12: LGTM! Improved test file handling.The changes enhance test coverage by:
- Excluding only specific test files that shouldn't be processed
- Including a broader range of test files through updated patterns
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/freezed/test/integration/multiple_constructors.dart (1)
8-15: Consider documenting the purpose of thetimefield.While the implementation is correct, adding documentation comments would help explain:
- Why the time field exists
- The significance of the default value
DateTime(0, 0, 0)- When to use the optional time parameter in
Response.datasealed class Response<T> with _$Response<T> { Response._({DateTime? time}) : time = time ?? DateTime(0, 0, 0); - // Constructors may enable passing parameters to ._(); + /// Private constructor that initializes the response timestamp. + /// Uses [DateTime(0, 0, 0)] as default if no time is provided. factory Response.data(T value, {DateTime? time}) = ResponseData; - // If those parameters are named optionals, they are not required to be passed. + /// Creates a success response containing [value] and an optional [time]. factory Response.error(Object error) = ResponseError; + /// The timestamp when this response was created. + /// Defaults to [DateTime(0, 0, 0)] if not specified. @override final DateTime time; }packages/freezed/test/multiple_constructors_test.dart (1)
29-37: Enhance test coverage for the Response class.While the current tests are good, consider adding these test cases:
- Verify that Response.error also accepts custom time
- Test copyWith functionality
- Test pattern matching with sealed class
test('Response', () { expect(Response.data('a').time, DateTime(0, 0, 0)); expect( Response.data('a', time: DateTime(1, 0, 0)).time, DateTime(1, 0, 0), ); expect(Response<String>.error('err').time, DateTime(0, 0, 0)); + expect( + Response<String>.error('err', time: DateTime(1, 0, 0)).time, + DateTime(1, 0, 0), + ); + + // Test copyWith + final response = Response.data('a', time: DateTime(1, 0, 0)); + expect( + response.copyWith(time: DateTime(2, 0, 0)).time, + DateTime(2, 0, 0), + ); + + // Test pattern matching + final result = switch (response) { + ResponseData<String>(value: final v) => v, + ResponseError<String>() => 'error' + }; + expect(result, 'a'); });packages/freezed/CHANGELOG.md (4)
3-4: Clarify the "mixed mode" description.
The new text “Freezed 3.0 is about supporting a "mixed mode". From now on, Freezed supports both the usual syntax:” nicely introduces the feature. Consider expanding slightly or linking to documentation that explains what “mixed mode” means and its benefits.
23-27: Neutral tone for listing benefits.
The bullet point "- Simple classes don't need Freezed's "weird" syntax and can stay simple" might be perceived as informal. Consider rephrasing to a more neutral tone, for example:
- Simple classes can use a more straightforward syntax without the extra boilerplate
This maintains the intended message while keeping the tone professional.
46-46: Remove trailing punctuation in heading.
The heading “### Breaking changes:” includes a trailing colon, which conflicts with markdown style guidelines (as noted by MD026).Apply the following diff:
-### Breaking changes: +### Breaking changes🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
46-46: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
48-50: Refine phrasing for grammatical consistency.
The breaking change description reads:
Freezed classes should now either beabstract,sealed, or manually implements_$MyClass.
For grammatical correctness, consider changing “manually implements” to “manually implement”.Apply the following diff:
-Freezed classes should now either be `abstract`, `sealed`, or manually implements `_$MyClass`. +Freezed classes should now either be `abstract`, `sealed`, or manually implement `_$MyClass`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/freezed/CHANGELOG.md(1 hunks)packages/freezed/test/integration/multiple_constructors.dart(4 hunks)packages/freezed/test/multiple_constructors_test.dart(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/freezed/CHANGELOG.md
46-46: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: freezed (packages/freezed_lint, master, get)
- GitHub Check: freezed (packages/freezed_annotation, master, get)
- GitHub Check: freezed (packages/freezed, master, get)
🔇 Additional comments (6)
packages/freezed/test/integration/multiple_constructors.dart (1)
5-7: LGTM! Well-structured sealed class implementation.The sealed class with a private constructor that handles default DateTime initialization is a good design choice. The generic type parameter
Tprovides flexibility for different response types.packages/freezed/CHANGELOG.md (5)
6-11: Check the example for usual syntax.
The code block showing the traditional (sealed class factory) example is well formatted and clear. Optionally, you might add a brief inline comment (or a sentence immediately after the code block) to emphasize that this remains the conventional way to define Freezed classes.
13-14: Smooth transition to the alternative syntax.
The “But also:” line effectively signals that an alternative, more natural Dart class syntax is supported. No changes are required, though you might consider clarifying in a follow-up sentence what benefits this alternative brings.
15-21: Review the alternative Dart class example.
The example using a regular Dart class (with a non-factory constructor and a final field) is clear and succinct. Ensure that any accompanying documentation further explains how this new usage integrates with existing Freezed features (like immutability and copyWith).
28-31: Highlighting inheritance and default handling is clear.
The explanation that complex unions now support inheritance and non-constant default values (using a non-factoryMyClass._()constructor) is informative. You might consider adding a brief note on when to favor this pattern over the conventional approach.
32-44: Complex union example is well demonstrated.
The provided code sample for theResponse<T>class clearly illustrates the new approach with a custom constructor and default value handling. This example should help users understand how to leverage inheritance and default-initialization within their Freezed classes.
|
This is an extremely cool feature. We will definitely be using this new (highly familiar) syntax. Is
|
You have to manually add the |
2 participants
We can now do:
Summary by CodeRabbit
New Features
Response<T>with factory constructors for data and error handling.FromJsonclass with additional checks for constructor presence.Refactor
Abstractclass to improve property handling and mixin generation.Tests
Responseclass.