Skip to content

Commit 72d4b22

Browse files
committed
fix(dart): preserve optional constructor flows
1 parent 5f0fb6f commit 72d4b22

4 files changed

Lines changed: 97 additions & 18 deletions

File tree

dart/packages/fory-test/lib/model/inheritance_models.dart

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,26 @@ final class CrossLibraryChild extends middle.PublicMiddleBoundary {
3939
String childMutable = '';
4040
}
4141

42+
class OptionalMutableBase {
43+
OptionalMutableBase([int decoded = 0])
44+
: inheritedMutable = decoded,
45+
observedAtConstruction = decoded;
46+
47+
@ForyField(id: 1)
48+
int inheritedMutable;
49+
50+
@ForyField(ignore: true)
51+
final int observedAtConstruction;
52+
}
53+
54+
@ForyStruct()
55+
final class OptionalMutableChild extends OptionalMutableBase {
56+
OptionalMutableChild(this.fixed, [int decoded = 0]) : super(decoded);
57+
58+
@ForyField(id: 2)
59+
final int fixed;
60+
}
61+
4262
@ForyStruct()
4363
final class XlangInheritedChild extends base.XlangInheritanceBoundary {
4464
XlangInheritedChild();

dart/packages/fory-test/test/struct_inheritance_test.dart

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,19 @@ void main() {
9999
});
100100
}
101101

102+
test('passes optional mutable state through construction', () {
103+
final fory = Fory();
104+
InheritanceModelsForyModule.register(fory, OptionalMutableChild, id: 405);
105+
106+
final result = fory.deserialize<OptionalMutableChild>(
107+
fory.serialize(OptionalMutableChild(7, 19)),
108+
);
109+
110+
expect(result.fixed, 7);
111+
expect(result.inheritedMutable, 19);
112+
expect(result.observedAtConstruction, 19);
113+
});
114+
102115
for (final compatible in <bool>[false, true]) {
103116
test('matches flat and inherited schemas compatible=$compatible', () {
104117
final privateFory = Fory(compatible: compatible);

dart/packages/fory/lib/src/codegen/fory_constructor_analysis.dart

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ final class _OrdinaryConstructorAnalyzer {
118118
}
119119

120120
for (final root in rootFrame.parameters) {
121-
if (!root.parameter.isRequired || bindingByRoot.containsKey(root)) {
121+
if (bindingByRoot.containsKey(root)) {
122122
continue;
123123
}
124124
_GeneratedFieldSpec? candidate;
@@ -135,16 +135,19 @@ final class _OrdinaryConstructorAnalyzer {
135135
ambiguousLabels.add(_fieldLabel(field));
136136
}
137137
if (candidate == null) {
138-
_throwMissingRequiredSource(
139-
declaration,
140-
targetTypeLiteral,
141-
root,
142-
fields,
143-
);
138+
if (root.parameter.isRequired) {
139+
_throwMissingRequiredSource(
140+
declaration,
141+
targetTypeLiteral,
142+
root,
143+
fields,
144+
);
145+
}
146+
continue;
144147
}
145148
if (ambiguousLabels != null) {
146149
throw InvalidGenerationSourceError(
147-
'Required constructor parameter ${root.parameter.displayName} on '
150+
'Constructor parameter ${root.parameter.displayName} on '
148151
'$targetTypeLiteral has multiple writable identity-flow sources: '
149152
'${ambiguousLabels.join(', ')}. Select a constructor '
150153
'whose parameter flow identifies exactly one serialized field.',
@@ -155,10 +158,10 @@ final class _OrdinaryConstructorAnalyzer {
155158
final existingRoot = rootByArgumentField[field];
156159
if (existingRoot != null && !identical(existingRoot, root)) {
157160
throw InvalidGenerationSourceError(
158-
'Serialized field ${_fieldLabel(field)} would supply both required '
159-
'constructor parameters ${existingRoot.parameter.displayName} and '
161+
'Serialized field ${_fieldLabel(field)} would supply constructor '
162+
'parameters ${existingRoot.parameter.displayName} and '
160163
'${root.parameter.displayName} on $targetTypeLiteral. A field may '
161-
'supply at most one required constructor argument.',
164+
'supply at most one constructor argument.',
162165
element: field.declaration,
163166
);
164167
}
@@ -251,12 +254,11 @@ final class _OrdinaryConstructorAnalyzer {
251254
if (_isMixinApplicationConstructor(constructor)) {
252255
_addMixinForwardingEdges(frame, nextFrame, declaration);
253256
} else if (_isDeclaredConstructor(constructor)) {
254-
// Field-formal elements completely prove a leaf constructor when
255-
// every required argument and final field already has an exact
256-
// storage edge. Resolve source only when an initializer or
257-
// constructor hop is still needed for that proof; eager resolution
258-
// makes the common flat-struct build pay for a whole resolved AST
259-
// without changing the construction result.
257+
// Summary elements completely prove a leaf constructor only when it
258+
// has no optional parameters and every required argument and final
259+
// field already has an exact storage edge. An explicit initializer
260+
// may connect an optional parameter to mutable storage that must
261+
// receive its decoded value during construction.
260262
if (nextFrame != null || !_summaryProvesLeaf(frame, fields)) {
261263
final node = await _astFor(constructor);
262264
if (node is ConstructorDeclaration) {
@@ -293,7 +295,7 @@ final class _OrdinaryConstructorAnalyzer {
293295
List<_GeneratedFieldSpec> fields,
294296
) {
295297
for (final parameter in frame.parameters) {
296-
if (parameter.parameter.isRequired && parameter.fieldTargets.isEmpty) {
298+
if (parameter.parameter.isOptional || parameter.fieldTargets.isEmpty) {
297299
return false;
298300
}
299301
}

dart/packages/fory/test/struct_constructor_flow_generator_test.dart

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,32 @@ final class FinalAndWritableValue {
179179
);
180180
});
181181

182+
test('passes an optional writable identity flow', () async {
183+
await _expectGenerationOutput(
184+
source: '''
185+
class OptionalMutableBase {
186+
int mutable;
187+
188+
OptionalMutableBase([int decoded = 0]) : mutable = decoded;
189+
}
190+
191+
@ForyStruct()
192+
final class OptionalMutableChild extends OptionalMutableBase {
193+
final int fixed;
194+
195+
OptionalMutableChild(this.fixed, [int decoded = 0]) : super(decoded);
196+
}
197+
''',
198+
output: allOf(
199+
contains(
200+
'final value = OptionalMutableChild('
201+
'_fixedValue, _mutableValue);',
202+
),
203+
isNot(contains('value.mutable = _mutableValue;')),
204+
),
205+
);
206+
});
207+
182208
test('omits an unbound optional named parameter', () async {
183209
await _expectGenerationOutput(
184210
source: '''
@@ -413,6 +439,24 @@ final class AmbiguousWritableValue {
413439
);
414440
});
415441

442+
test('optional root with ambiguous writable fields', () async {
443+
await _expectGenerationError(
444+
source: '''
445+
@ForyStruct()
446+
final class AmbiguousOptionalValue {
447+
final int fixed;
448+
int first;
449+
int second;
450+
451+
AmbiguousOptionalValue(this.fixed, [int decoded = 0])
452+
: first = decoded,
453+
second = decoded;
454+
}
455+
''',
456+
message: 'has multiple writable identity-flow sources',
457+
);
458+
});
459+
416460
test('required parameter without a field source', () async {
417461
await _expectGenerationError(
418462
source: '''

0 commit comments

Comments
 (0)