Skip to content

[native_assets_cli] Syntax validation #2115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 20, 2025
Merged

[native_assets_cli] Syntax validation #2115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb95354
[native_assets_cli] Syntax validation
dcharkes Mar 20, 2025
30913fe
address comments
dcharkes Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 97 additions & 16 deletions pkgs/json_syntax_generator/lib/src/generator/helper_library.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,52 +22,98 @@ class JsonReader {
T get<T extends Object?>(String key) {
final value = json[key];
if (value is T) return value;
final pathString = _jsonPathToString([key]);
if (value == null) {
throw FormatException("No value was provided for '$pathString'.");
}
throwFormatException(value, T, [key]);
}

List<String> validate<T extends Object?>(String key) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to have a full Dart class in a string? This feels like it should live somewhere the analyzer can look at it...

Copy link
Collaborator Author

@dcharkes dcharkes Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to do that initially, but that means that invoking the code generator as a library needs to know where the code generator is so that it can find the file and read it as a String.

If we were to change the code generator to have a bin/ script we would know from the entry point. But we invoke it with a Dart API (and pass in a bunch of objects).

We could change the API to have to pass in URI of the package, but that also seems messy.

I'm open to suggestions.

(Orthogonal to this PR, so we should probably do that in a separate PR.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the entry point in hook/tool/generate_syntax.dart, where you could hardcode the path to the file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think passing the path to the file is not messy, but your choice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the Dart API have an extra param and less encapsulated. More ugly in the implementation but less ugly on call sites.

If this package ever gets published on pub, then the user would have to know the path of the package in the pub-cache and pass it in. That feels too messy.

(And publishing a full blown helper package package:json_syntax_runtime_lib like we have with package:jni, seems way too much overkill here.)

Let's keep the ugly inside instead of on the API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: T extends Object? is the same as just T but it's fine if you want to be more descriptive.

Not as important in generated code but a name like validateIsA better describes the type checking here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah Object? == dynamic I thought it excludes dynamic, but it doesn't.

validate

I wanted to keep it analogous to get. We could do getA, but I feel it makes it less readable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think T extends Object? is different to T regarding the avoid_dynamic_calls lint. I might be wrong on this though.

final value = json[key];
if (value is T) return [];
return [
errorString(value, T, [key]),
];
}

List<T> list<T extends Object?>(String key) =>
_castList<T>(get<List<Object?>>(key), key);

List<String> validateList<T extends Object?>(String key) {
final listErrors = validate<List<Object?>>(key);
if (listErrors.isNotEmpty) {
return listErrors;
}
return _validateListElements(get<List<Object?>>(key), key);
}

List<T>? optionalList<T extends Object?>(String key) =>
switch (get<List<Object?>?>(key)?.cast<T>()) {
null => null,
final l => _castList<T>(l, key),
};

List<String> validateOptionalList<T extends Object?>(String key) {
final listErrors = validate<List<Object?>?>(key);
if (listErrors.isNotEmpty) {
return listErrors;
}
final list = get<List<Object?>?>(key);
if (list == null) {
return [];
}
return _validateListElements(list, key);
}

/// [List.cast] but with [FormatException]s.
List<T> _castList<T extends Object?>(List<Object?> list, String key) {
var index = 0;
for (final value in list) {
for (final (index, value) in list.indexed) {
if (value is! T) {
throwFormatException(value, T, [key, index]);
}
index++;
}
return list.cast();
}

List<T>? optionalListParsed<T extends Object?>(
List<String> _validateListElements<T extends Object?>(
List<Object?> list,
String key,
T Function(Object?) elementParser,
) {
final jsonValue = optionalList(key);
if (jsonValue == null) return null;
return [for (final element in jsonValue) elementParser(element)];
final result = <String>[];
for (final (index, value) in list.indexed) {
if (value is! T) {
result.add(errorString(value, T, [key, index]));
}
}
return result;
}

Map<String, T> map$<T extends Object?>(String key) =>
_castMap<T>(get<Map<String, Object?>>(key), key);

List<String> validateMap<T extends Object?>(String key) {
final mapErrors = validate<Map<String, Object?>>(key);
if (mapErrors.isNotEmpty) {
return mapErrors;
}
return _validateMapElements<T>(get<Map<String, Object?>>(key), key);
}

Map<String, T>? optionalMap<T extends Object?>(String key) =>
switch (get<Map<String, Object?>?>(key)) {
null => null,
final m => _castMap<T>(m, key),
};

List<String> validateOptionalMap<T extends Object?>(String key) {
final mapErrors = validate<Map<String, Object?>?>(key);
if (mapErrors.isNotEmpty) {
return mapErrors;
}
final map = get<Map<String, Object?>?>(key);
if (map == null) {
return [];
}
return _validateMapElements<T>(map, key);
}

/// [Map.cast] but with [FormatException]s.
Map<String, T> _castMap<T extends Object?>(
Map<String, Object?> map_,
Expand All @@ -81,18 +127,40 @@ class JsonReader {
return map_.cast();
}

List<String> _validateMapElements<T extends Object?>(
Map<String, Object?> map_,
String parentKey,
) {
final result = <String>[];
for (final MapEntry(:key, :value) in map_.entries) {
if (value is! T) {
result.add(errorString(value, T, [parentKey, key]));
}
}
return result;
}

List<String>? optionalStringList(String key) => optionalList<String>(key);

List<String> validateOptionalStringList(String key) =>
validateOptionalList<String>(key);

List<String> stringList(String key) => list<String>(key);

List<String> validateStringList(String key) => validateList<String>(key);

Uri path$(String key) => _fileSystemPathToUri(get<String>(key));

List<String> validatePath(String key) => validate<String>(key);

Uri? optionalPath(String key) {
final value = get<String?>(key);
if (value == null) return null;
return _fileSystemPathToUri(value);
}

List<String> validateOptionalPath(String key) => validate<String?>(key);

List<Uri>? optionalPathList(String key) {
final strings = optionalStringList(key);
if (strings == null) {
Expand All @@ -101,6 +169,9 @@ class JsonReader {
return [for (final string in strings) _fileSystemPathToUri(string)];
}

List<String> validateOptionalPathList(String key) =>
validateOptionalStringList(key);

static Uri _fileSystemPathToUri(String path) {
if (path.endsWith(Platform.pathSeparator)) {
return Uri.directory(path);
Expand All @@ -115,12 +186,22 @@ class JsonReader {
Object? value,
Type expectedType,
List<Object> pathExtension,
) {
throw FormatException(errorString(value, expectedType, pathExtension));
}

String errorString(
Object? value,
Type expectedType,
List<Object> pathExtension,
) {
final pathString = _jsonPathToString(pathExtension);
throw FormatException(
"Unexpected value '$value' (${value.runtimeType}) for '$pathString'. "
'Expected a $expectedType.',
);
if (value == null) {
return "No value was provided for '$pathString'."
' Expected a $expectedType.';
}
return "Unexpected value '$value' (${value.runtimeType}) for '$pathString'."
' Expected a $expectedType.';
}
}

Expand Down
14 changes: 13 additions & 1 deletion pkgs/json_syntax_generator/lib/src/generator/normal_class_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class ClassGenerator {
final constructorSetterCalls = <String>[];
final accessors = <String>[];
final superParams = <String>[];
final validateCalls = <String>[];

final propertyNames =
{
Expand Down Expand Up @@ -62,6 +63,7 @@ class ClassGenerator {
}
if (thisClassProperty != null) {
accessors.add(PropertyGenerator(thisClassProperty).generate());
validateCalls.add('...${property.validateName}()');
}
}

Expand Down Expand Up @@ -95,6 +97,12 @@ class $className extends $superclassName {
buffer.writeln('''
${accessors.join('\n')}

@override
List<String> validate() => [
...super.validate(),
${validateCalls.join(',\n')}
];

@override
String toString() => '$className(\$json)';
}
Expand All @@ -121,6 +129,10 @@ class $className {

${accessors.join('\n')}

List<String> validate() => [
${validateCalls.join(',\n')}
];

@override
String toString() => '$className(\$json)';
}
Expand All @@ -132,7 +144,7 @@ class $className {
extension ${className}Extension on $superclassName {
bool get is$className => type == '$identifyingSubtype';

$className get as$className => $className.fromJson(json);
$className get as$className => $className.fromJson(json, path: path);
}
''');
}
Expand Down
Loading