Skip to content

Commit 110a167

Browse files
committed
refactor: consolidate duplicated guards, error construction, and package list
Behavior-preserving simplifications on top of the validation/schema/tooling fixes. Each item removes a single source of duplication or redundant state: - ack.dart: extract _requireNonEmpty/_requireUniqueBy for the enumValues, enumString, and anyOf argument guards - schema.dart: extract _failFromThrown for the three thrown-error catch blocks - codec_schema.dart: drop redundant _encoderIdentity (provably identical to _encoder in every path; _decoderIdentity retained as it fixes a real bug) - lazy_schema.dart: drop write-only _LazyRecursionContext.owner (duplicated the inherited SchemaContext.schema) - ack_schema_model_builder.dart: hoist a single const _deepEquality - schema_ast_analyzer.dart: extract _rejectIfReferencesNullableSchema (2 sites) - api_check.dart: extract _writeProcessStderr - update_release_changelog.dart: drop redundant record `path` field - scripts: share publishableAckPackages across api_check and update_release_changelog (new scripts/src/workspace_packages.dart) - enum_schema/any_of_schema: point docs to the canonical AckSchema policy dart analyze clean; ack (955) + ack_generator (127) + scripts tests pass.
1 parent 4d8f693 commit 110a167

11 files changed

Lines changed: 108 additions & 94 deletions

File tree

packages/ack/lib/src/ack.dart

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,8 @@ final class Ack {
5252

5353
/// Creates an enum schema for validating enum values.
5454
static EnumSchema<T> enumValues<T extends Enum>(List<T> values) {
55-
if (values.isEmpty) {
56-
throw ArgumentError.value(values, 'values', 'Must not be empty.');
57-
}
58-
final names = values.map((value) => value.name).toSet();
59-
if (names.length != values.length) {
60-
throw ArgumentError.value(values, 'values', 'Must be unique.');
61-
}
55+
_requireNonEmpty(values, 'values');
56+
_requireUniqueBy(values, 'values', (value) => value.name);
6257
return EnumSchema(values: List.unmodifiable(values));
6358
}
6459

@@ -79,22 +74,16 @@ final class Ack {
7974

8075
/// Creates a string schema that only accepts one of the given [values].
8176
static StringSchema enumString(List<String> values) {
82-
if (values.isEmpty) {
83-
throw ArgumentError.value(values, 'values', 'Must not be empty.');
84-
}
85-
if (values.toSet().length != values.length) {
86-
throw ArgumentError.value(values, 'values', 'Must be unique.');
87-
}
77+
_requireNonEmpty(values, 'values');
78+
_requireUniqueBy(values, 'values', (value) => value);
8879
return string().withConstraint(
8980
PatternConstraint.enumString(List.unmodifiable(values)),
9081
);
9182
}
9283

9384
/// Creates a schema that can be one of many types.
9485
static AnyOfSchema anyOf(List<AnyAckSchema> schemas) {
95-
if (schemas.isEmpty) {
96-
throw ArgumentError.value(schemas, 'schemas', 'Must not be empty.');
97-
}
86+
_requireNonEmpty(schemas, 'schemas');
9887
return AnyOfSchema(List.unmodifiable(schemas));
9988
}
10089

@@ -269,3 +258,21 @@ String _encodeIsoDate(DateTime value) {
269258
String _encodeIsoDateTime(DateTime value) {
270259
return value.toIso8601String();
271260
}
261+
262+
List<T> _requireNonEmpty<T>(List<T> values, String name) {
263+
if (values.isEmpty) {
264+
throw ArgumentError.value(values, name, 'Must not be empty.');
265+
}
266+
return values;
267+
}
268+
269+
List<T> _requireUniqueBy<T, K>(
270+
List<T> values,
271+
String name,
272+
K Function(T) keyOf,
273+
) {
274+
if (values.map(keyOf).toSet().length != values.length) {
275+
throw ArgumentError.value(values, name, 'Must be unique.');
276+
}
277+
return values;
278+
}

packages/ack/lib/src/schema_model/ack_schema_model_builder.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import '../schemas/schema.dart';
1010
import 'ack_schema_model.dart';
1111
import 'ack_schema_model_warning.dart';
1212

13+
const _deepEquality = DeepCollectionEquality();
14+
1315
extension AckSchemaModelExtension<
1416
Boundary extends Object,
1517
Runtime extends Object
@@ -47,10 +49,9 @@ final class _SchemaModelBuilder {
4749
merged[key] = entry.value;
4850
}
4951

50-
const equality = DeepCollectionEquality();
5152
for (final entry in lazyDefinitions.entries) {
5253
if (merged.containsKey(entry.key)) {
53-
if (!equality.equals(merged[entry.key], entry.value)) {
54+
if (!_deepEquality.equals(merged[entry.key], entry.value)) {
5455
throw ArgumentError(
5556
'Ack.lazy definition "${entry.key}" collides with an existing root '
5657
'JSON Schema definition. Use a unique lazy name or rename the '
@@ -353,7 +354,6 @@ AckSchemaModel _applyConstraints(
353354
AckSchema<dynamic, dynamic> schema,
354355
) {
355356
var next = model;
356-
const deepEquality = DeepCollectionEquality();
357357
final appliedKeywordValues = {
358358
for (final entry in _renderedKeywords(model).entries)
359359
entry.key: <Object?>[entry.value],
@@ -374,7 +374,7 @@ AckSchemaModel _applyConstraints(
374374
appliedKeywordValues[entry.key] = [entry.value];
375375
newKeywords[entry.key] = entry.value;
376376
} else if (!seenValues.any(
377-
(value) => deepEquality.equals(value, entry.value),
377+
(value) => _deepEquality.equals(value, entry.value),
378378
)) {
379379
seenValues.add(entry.value);
380380
conflicts[entry.key] = entry.value;

packages/ack/lib/src/schemas/any_of_schema.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ final class AnyOfSchema extends AckSchema<Object, Object>
1414

1515
/// Creates a low-level union from an immutable, non-empty list of [schemas].
1616
///
17-
/// Prefer `Ack.anyOf`, which validates and snapshots caller-owned lists.
17+
/// See [AckSchema] for the low-level-constructor policy; prefer [Ack.anyOf].
1818
/// Direct callers must not mutate [schemas] after construction.
1919
const AnyOfSchema(
2020
this.schemas, {

packages/ack/lib/src/schemas/codec_schema.dart

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,21 @@ final class CodecSchema<Boundary extends Object, Runtime extends Object>
2525
final Runtime Function(Object value) _decoder;
2626
final Object Function(Runtime value)? _encoder;
2727
final Object _decoderIdentity;
28-
final Object? _encoderIdentity;
2928

3029
CodecSchema._({
3130
required this.inputSchema,
3231
required this.outputSchema,
3332
required Runtime Function(Object value) decoder,
3433
required Object Function(Runtime value)? encoder,
3534
required Object decoderIdentity,
36-
required Object? encoderIdentity,
3735
super.isNullable,
3836
super.isOptional,
3937
super.description,
4038
super.constraints,
4139
super.refinements,
4240
}) : _decoder = decoder,
4341
_encoder = encoder,
44-
_decoderIdentity = decoderIdentity,
45-
_encoderIdentity = encoderIdentity;
42+
_decoderIdentity = decoderIdentity;
4643

4744
/// Creates a codec while preserving the input schema's runtime type.
4845
static CodecSchema<Boundary, Runtime> create<
@@ -66,7 +63,6 @@ final class CodecSchema<Boundary extends Object, Runtime extends Object>
6663
decoder: (value) => decoder(value as InputRuntime),
6764
encoder: encoder,
6865
decoderIdentity: decoder,
69-
encoderIdentity: encoder,
7066
isNullable: isNullable,
7167
isOptional: isOptional,
7268
description: description,
@@ -169,7 +165,6 @@ final class CodecSchema<Boundary extends Object, Runtime extends Object>
169165
decoder: _decoder,
170166
encoder: _encoder,
171167
decoderIdentity: _decoderIdentity,
172-
encoderIdentity: _encoderIdentity,
173168
isNullable: isNullable,
174169
isOptional: isOptional,
175170
description: description,
@@ -192,7 +187,6 @@ final class CodecSchema<Boundary extends Object, Runtime extends Object>
192187
decoder: _decoder,
193188
encoder: _encoder,
194189
decoderIdentity: _decoderIdentity,
195-
encoderIdentity: _encoderIdentity,
196190
isNullable: isNullable ?? this.isNullable,
197191
isOptional: isOptional ?? this.isOptional,
198192
description: description ?? this.description,
@@ -210,7 +204,7 @@ final class CodecSchema<Boundary extends Object, Runtime extends Object>
210204
inputSchema == other.inputSchema &&
211205
outputSchema == other.outputSchema &&
212206
_decoderIdentity == other._decoderIdentity &&
213-
_encoderIdentity == other._encoderIdentity;
207+
_encoder == other._encoder;
214208
}
215209

216210
@override
@@ -225,6 +219,6 @@ final class CodecSchema<Boundary extends Object, Runtime extends Object>
225219
inputSchema,
226220
outputSchema,
227221
_decoderIdentity,
228-
_encoderIdentity,
222+
_encoder,
229223
);
230224
}

packages/ack/lib/src/schemas/enum_schema.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ final class EnumSchema<T extends Enum> extends AckSchema<String, T>
1010
/// Creates a low-level enum schema from an immutable, non-empty set of
1111
/// uniquely named [values].
1212
///
13-
/// Prefer `Ack.enumValues`, which validates and snapshots caller-owned lists.
14-
/// Direct callers must not mutate [values] after construction.
13+
/// See [AckSchema] for the low-level-constructor policy; prefer
14+
/// [Ack.enumValues]. Direct callers must not mutate [values] after
15+
/// construction.
1516
const EnumSchema({
1617
required this.values,
1718
super.isNullable,

packages/ack/lib/src/schemas/lazy_schema.dart

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ final class LazySchema<Boundary extends Object, Runtime extends Object>
132132
SchemaContext _enterRecursion(Object? value, SchemaContext context) {
133133
return _LazyRecursionContext(
134134
name: name,
135-
owner: this as AnyAckSchema,
135+
schema: this as AnyAckSchema,
136136
recursionToken: _recursionToken,
137137
value: value,
138138
parent: context,
@@ -222,20 +222,19 @@ final class LazySchema<Boundary extends Object, Runtime extends Object>
222222
final class _LazyRecursionContext extends SchemaContext {
223223
_LazyRecursionContext({
224224
required String name,
225-
required this.owner,
225+
required AnyAckSchema schema,
226226
required this.recursionToken,
227227
required Object? value,
228228
required SchemaContext parent,
229229
}) : super(
230230
name: name,
231-
schema: owner,
231+
schema: schema,
232232
value: value,
233233
parent: parent,
234234
pathSegment: '',
235235
operation: parent.operation,
236236
);
237237

238-
final AnyAckSchema owner;
239238
final Object recursionToken;
240239
}
241240

packages/ack/lib/src/schemas/schema.dart

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,11 @@ abstract class AckSchema<Boundary extends Object, Runtime extends Object> {
150150
final violation = constraint.validate(value);
151151
if (violation != null) constraintViolations.add(violation);
152152
} catch (error, stackTrace) {
153-
return SchemaResult.fail(
154-
SchemaValidationError(
155-
message: 'Constraint "${constraint.constraintKey}" threw: $error',
156-
context: context,
157-
cause: error,
158-
stackTrace: stackTrace,
159-
),
153+
return _failFromThrown(
154+
'Constraint "${constraint.constraintKey}" threw: $error',
155+
context,
156+
error,
157+
stackTrace,
160158
);
161159
}
162160
}
@@ -177,13 +175,11 @@ abstract class AckSchema<Boundary extends Object, Runtime extends Object> {
177175
try {
178176
isValid = refinement.validate(value);
179177
} catch (error, stackTrace) {
180-
return SchemaResult.fail(
181-
SchemaValidationError(
182-
message: 'Refinement threw: $error',
183-
context: context,
184-
cause: error,
185-
stackTrace: stackTrace,
186-
),
178+
return _failFromThrown(
179+
'Refinement threw: $error',
180+
context,
181+
error,
182+
stackTrace,
187183
);
188184
}
189185
if (!isValid) {
@@ -196,6 +192,22 @@ abstract class AckSchema<Boundary extends Object, Runtime extends Object> {
196192
return SchemaResult.ok(value);
197193
}
198194

195+
SchemaResult<Runtime> _failFromThrown(
196+
String message,
197+
SchemaContext context,
198+
Object error,
199+
StackTrace stackTrace,
200+
) {
201+
return SchemaResult.fail(
202+
SchemaValidationError(
203+
message: message,
204+
context: context,
205+
cause: error,
206+
stackTrace: stackTrace,
207+
),
208+
);
209+
}
210+
199211
/// Helper for schemas whose boundary == runtime: validates the runtime
200212
/// value and, if it passes, returns it as the boundary value unchanged.
201213
/// Only safe to call when `Boundary` and `Runtime` are the same type.
@@ -361,13 +373,11 @@ abstract class AckSchema<Boundary extends Object, Runtime extends Object> {
361373
try {
362374
return parseWithContext(value, context);
363375
} catch (error, stackTrace) {
364-
return SchemaResult.fail(
365-
SchemaValidationError(
366-
message: 'Validation threw: $error',
367-
context: context,
368-
cause: error,
369-
stackTrace: stackTrace,
370-
),
376+
return _failFromThrown(
377+
'Validation threw: $error',
378+
context,
379+
error,
380+
stackTrace,
371381
);
372382
}
373383
}

packages/ack_generator/lib/src/analyzer/schema_ast_analyzer.dart

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2098,14 +2098,7 @@ class SchemaAstAnalyzer {
20982098
}
20992099

21002100
if (chain.schemaReference != null) {
2101-
final resolved = _resolveSchemaReference(
2102-
chain.schemaReference!,
2103-
element,
2104-
);
2105-
_rejectNullableListElement(
2106-
resolved?.modelInfo.isNullableSchema ?? false,
2107-
element,
2108-
);
2101+
_rejectIfReferencesNullableSchema(chain.schemaReference!, element);
21092102
final mapping = _resolveSchemaVariableType(
21102103
chain.schemaReference!,
21112104
element,
@@ -2168,11 +2161,7 @@ class SchemaAstAnalyzer {
21682161
}
21692162

21702163
if (ref.schemaRef != null) {
2171-
final resolved = _resolveSchemaReference(ref.schemaRef!, element);
2172-
_rejectNullableListElement(
2173-
resolved?.modelInfo.isNullableSchema ?? false,
2174-
element,
2175-
);
2164+
_rejectIfReferencesNullableSchema(ref.schemaRef!, element);
21762165
final mapping = _resolveSchemaVariableType(
21772166
ref.schemaRef!,
21782167
element,
@@ -2207,6 +2196,17 @@ class SchemaAstAnalyzer {
22072196
);
22082197
}
22092198

2199+
void _rejectIfReferencesNullableSchema(
2200+
_SchemaReference reference,
2201+
Element2 element,
2202+
) {
2203+
final resolved = _resolveSchemaReference(reference, element);
2204+
_rejectNullableListElement(
2205+
resolved?.modelInfo.isNullableSchema ?? false,
2206+
element,
2207+
);
2208+
}
2209+
22102210
_SchemaTypeMapping _wrapListElementMapping(
22112211
_SchemaTypeMapping elementMapping,
22122212
TypeProvider typeProvider,

scripts/api_check.dart

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,9 @@
33
import 'dart:convert';
44
import 'dart:io';
55

6-
const ackPackages = [
7-
'ack',
8-
'ack_annotations',
9-
'ack_generator',
10-
'ack_firebase_ai',
11-
'ack_json_schema_builder',
12-
];
6+
import 'src/workspace_packages.dart';
7+
8+
final ackPackages = publishableAckPackages;
139
const dartApiToolVersion = '0.23.0';
1410

1511
Future<void> main(List<String> args) async {
@@ -164,9 +160,7 @@ Future<bool> checkPackage(
164160
print('✅ $packageName: API check completed');
165161
} else {
166162
stderr.writeln('❌ $packageName: API changes detected or check failed');
167-
if ((result.stderr as String).isNotEmpty) {
168-
stderr.writeln(result.stderr);
169-
}
163+
_writeProcessStderr(result);
170164
}
171165

172166
final reportExists = report.existsSync();
@@ -185,15 +179,19 @@ Future<bool> runCommand(String command, List<String> args) async {
185179
if (result.exitCode == 0) return true;
186180

187181
stderr.writeln('Error running $command ${args.join(' ')}');
188-
if ((result.stderr as String).isNotEmpty) {
189-
stderr.writeln(result.stderr);
190-
}
182+
_writeProcessStderr(result);
191183
} on ProcessException catch (error) {
192184
stderr.writeln('Error running $command ${args.join(' ')}: $error');
193185
}
194186
return false;
195187
}
196188

189+
void _writeProcessStderr(ProcessResult result) {
190+
if ((result.stderr as String).isNotEmpty) {
191+
stderr.writeln(result.stderr);
192+
}
193+
}
194+
197195
void printUsage() {
198196
print('');
199197
print('Usage: dart scripts/api_check.dart [PACKAGE] [VERSION]');

0 commit comments

Comments
 (0)