Skip to content

[native_assets_cli] Fix syntax required fields #2116

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 1 commit into from
Mar 20, 2025
Merged

[native_assets_cli] Fix syntax required fields #2116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 11 additions & 3 deletions pkgs/code_assets/doc/schema/shared/shared_definitions.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
"type": "integer"
}
},
"required": []
"required": [
"target_ndk_api"
]
},
"Architecture": {
"type": "string",
Expand Down Expand Up @@ -82,6 +84,7 @@
}
},
"required": [
"architecture",
"id",
"link_mode",
"os"
Expand Down Expand Up @@ -273,7 +276,10 @@
"type": "integer"
}
},
"required": []
"required": [
"target_sdk",
"target_version"
]
},
"LinkMode": {
"type": "object",
Expand Down Expand Up @@ -350,7 +356,9 @@
"type": "integer"
}
},
"required": []
"required": [
"target_version"
]
},
"OS": {
"type": "string",
Expand Down
30 changes: 5 additions & 25 deletions pkgs/code_assets/test/schema/schema_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ FieldsFunction _codeFields(AllTestData allTestData) {
required Party party,
}) {
const requiredCodeAssetFields = [
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
// ['architecture'],
['architecture'],
['os'],
['id'],
['link_mode'],
Expand All @@ -105,32 +104,22 @@ FieldsFunction _codeFields(AllTestData allTestData) {
(['config', 'code', 'macos'], expectRequiredFieldMissing),
(
['config', 'code', 'macos', 'target_version'],
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
expectOptionalFieldMissing,
expectRequiredFieldMissing,
),
if (hook == Hook.link) ...[
for (final field in requiredCodeAssetFields)
(['assets', 0, ...field], expectRequiredFieldMissing),
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
(['assets', 0, 'architecture'], expectOptionalFieldMissing),
],
],
if (inputOrOutput == InputOrOutput.output) ...[
for (final field in requiredCodeAssetFields)
(['assets', 0, ...field], expectRequiredFieldMissing),
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
(['assets', 0, 'architecture'], expectOptionalFieldMissing),
if (hook == Hook.build) ...[
for (final field in requiredCodeAssetFields)
(
['assetsForLinking', 'package_with_linker', 0, ...field],
expectRequiredFieldMissing,
),
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
(
['assetsForLinking', 'package_with_linker', 0, 'architecture'],
expectOptionalFieldMissing,
),
],
(['assets', staticIndex, 'file'], expectRequiredFieldMissing),
(
Expand Down Expand Up @@ -200,16 +189,8 @@ List<(List<Object>, void Function(ValidationResults result))> _codeFieldsIOS({
}) => <(List<Object>, void Function(ValidationResults result))>[
if (inputOrOutput == InputOrOutput.input && hook == Hook.build) ...[
(['config', 'code', 'ios'], expectRequiredFieldMissing),
(
['config', 'code', 'ios', 'target_sdk'],
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
expectOptionalFieldMissing,
),
(
['config', 'code', 'ios', 'target_version'],
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
expectOptionalFieldMissing,
),
(['config', 'code', 'ios', 'target_sdk'], expectRequiredFieldMissing),
(['config', 'code', 'ios', 'target_version'], expectRequiredFieldMissing),
],
];

Expand All @@ -223,8 +204,7 @@ _codeFieldsAndroid({
(['config', 'code', 'android'], expectRequiredFieldMissing),
(
['config', 'code', 'android', 'target_ndk_api'],
// TODO(https://github.com/dart-lang/native/issues/2039): Make required.
expectOptionalFieldMissing,
expectRequiredFieldMissing,
),
],
];
11 changes: 4 additions & 7 deletions pkgs/native_assets_cli/lib/src/code_assets/code_asset.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ final class CodeAsset {
final OS os;

/// The architecture this asset can run on.
final Architecture? architecture;
final Architecture architecture;

/// The link mode for this native code.
///
Expand Down Expand Up @@ -87,7 +87,7 @@ final class CodeAsset {
required LinkMode linkMode,
required OS os,
Uri? file,
Copy link
Member

Choose a reason for hiding this comment

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

This is a style question, and not something for the current PR, but I personally like setting nullable parameter as required to make people think about if they want to leave out the argument.

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 do like that idea in this context. It means that the semantic layer around it explicitly has to pass null. And most likely if we're adding a new optional field, we should not be forgetting to pass it from the semantic API to the syntax.

Edit: err, this is the semantic API used by users. It would make file: null for all code assets with the link mode not requiring a file. I think that might be overly verbose.

Architecture? architecture,
required Architecture architecture,
}) : this._(
id: 'package:$package/$name',
linkMode: linkMode,
Expand All @@ -114,10 +114,7 @@ final class CodeAsset {
return CodeAsset._(
id: syntaxNode.id,
os: OSSyntax.fromSyntax(syntaxNode.os),
architecture: switch (syntaxNode.architecture) {
null => null,
final a => ArchitectureSyntax.fromSyntax(a),
},
architecture: ArchitectureSyntax.fromSyntax(syntaxNode.architecture),
linkMode: LinkModeSyntax.fromSyntax(syntaxNode.linkMode),
file: syntaxNode.file,
);
Expand Down Expand Up @@ -155,7 +152,7 @@ final class CodeAsset {
EncodedAsset encode() {
final nativeCodeAsset = syntax.NativeCodeAsset.fromJson({});
nativeCodeAsset.setup(
architecture: architecture?.toSyntax(),
architecture: architecture.toSyntax(),
file: file,
id: id,
linkMode: linkMode.toSyntax(),
Expand Down
24 changes: 4 additions & 20 deletions pkgs/native_assets_cli/lib/src/code_assets/config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ class IOSCodeConfig {
IOSCodeConfig._(this._syntax);

/// Whether to target device or simulator.
IOSSdk get targetSdk => IOSSdk.fromString(_syntax.targetSdk!);
IOSSdk get targetSdk => IOSSdk.fromString(_syntax.targetSdk);

/// The lowest iOS version that the compiled code will be compatible with.
int get targetVersion => _syntax.targetVersion!;
int get targetVersion => _syntax.targetVersion;

IOSCodeConfig({required IOSSdk targetSdk, required int targetVersion})
: _syntax = syntax.IOSCodeConfig(
Expand All @@ -108,14 +108,6 @@ class IOSCodeConfig {
);
}

extension IOSConfigSyntactic on IOSCodeConfig {
IOSSdk? get targetSdkSyntactic => switch (_syntax.targetSdk) {
null => null,
final s => IOSSdk.fromString(s),
};
int? get targetVersionSyntactic => _syntax.targetVersion;
}

/// Configuration provided when [CodeConfig.targetOS] is [OS.macOS].
class AndroidCodeConfig {
final syntax.AndroidCodeConfig _syntax;
Expand All @@ -124,33 +116,25 @@ class AndroidCodeConfig {

/// The minimum Android SDK API version to that the compiled code will be
/// compatible with.
int get targetNdkApi => _syntax.targetNdkApi!;
int get targetNdkApi => _syntax.targetNdkApi;

AndroidCodeConfig({required int targetNdkApi})
: _syntax = syntax.AndroidCodeConfig(targetNdkApi: targetNdkApi);
}

extension AndroidConfigSyntactic on AndroidCodeConfig {
int? get targetNdkApiSyntactic => _syntax.targetNdkApi;
}

//// Configuration provided when [CodeConfig.targetOS] is [OS.macOS].
class MacOSCodeConfig {
final syntax.MacOSCodeConfig _syntax;

MacOSCodeConfig._(this._syntax);

/// The lowest MacOS version that the compiled code will be compatible with.
int get targetVersion => _syntax.targetVersion!;
int get targetVersion => _syntax.targetVersion;

MacOSCodeConfig({required int targetVersion})
: _syntax = syntax.MacOSCodeConfig(targetVersion: targetVersion);
}

extension MacOSConfigSyntactic on MacOSCodeConfig {
int? get targetVersionSyntactic => _syntax.targetVersion;
}

/// Extension to the [BuildOutputBuilder] providing access to emitting code
/// assets (only available if code assets are supported).
extension CodeAssetBuildOutputBuilder on EncodedAssetBuildOutputBuilder {
Expand Down
45 changes: 22 additions & 23 deletions pkgs/native_assets_cli/lib/src/code_assets/syntax.g.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,19 @@ class AndroidCodeConfig {

AndroidCodeConfig.fromJson(this.json, {this.path = const []});

AndroidCodeConfig({int? targetNdkApi}) : json = {}, path = const [] {
AndroidCodeConfig({required int targetNdkApi}) : json = {}, path = const [] {
_targetNdkApi = targetNdkApi;
json.sortOnKey();
}

int? get targetNdkApi => _reader.get<int?>('target_ndk_api');
int get targetNdkApi => _reader.get<int>('target_ndk_api');

set _targetNdkApi(int? value) {
set _targetNdkApi(int value) {
json.setOrRemove('target_ndk_api', value);
}

List<String> _validateTargetNdkApi() =>
_reader.validate<int?>('target_ndk_api');
_reader.validate<int>('target_ndk_api');

List<String> validate() => [..._validateTargetNdkApi()];

Expand Down Expand Up @@ -115,7 +115,7 @@ class NativeCodeAsset extends Asset {
NativeCodeAsset.fromJson(super.json, {super.path}) : super.fromJson();

NativeCodeAsset({
Architecture? architecture,
required Architecture architecture,
Uri? file,
required String id,
required LinkMode linkMode,
Expand All @@ -132,7 +132,7 @@ class NativeCodeAsset extends Asset {
/// Setup all fields for [NativeCodeAsset] that are not in
/// [Asset].
void setup({
Architecture? architecture,
required Architecture architecture,
Uri? file,
required String id,
required LinkMode linkMode,
Expand All @@ -146,18 +146,17 @@ class NativeCodeAsset extends Asset {
json.sortOnKey();
}

Architecture? get architecture {
final jsonValue = _reader.get<String?>('architecture');
if (jsonValue == null) return null;
Architecture get architecture {
final jsonValue = _reader.get<String>('architecture');
return Architecture.fromJson(jsonValue);
}

set _architecture(Architecture? value) {
json.setOrRemove('architecture', value?.name);
set _architecture(Architecture value) {
json['architecture'] = value.name;
}

List<String> _validateArchitecture() =>
_reader.validate<String?>('architecture');
_reader.validate<String>('architecture');

Uri? get file => _reader.optionalPath('file');

Expand Down Expand Up @@ -607,30 +606,30 @@ class IOSCodeConfig {

IOSCodeConfig.fromJson(this.json, {this.path = const []});

IOSCodeConfig({String? targetSdk, int? targetVersion})
IOSCodeConfig({required String targetSdk, required int targetVersion})
: json = {},
path = const [] {
_targetSdk = targetSdk;
_targetVersion = targetVersion;
json.sortOnKey();
}

String? get targetSdk => _reader.get<String?>('target_sdk');
String get targetSdk => _reader.get<String>('target_sdk');

set _targetSdk(String? value) {
set _targetSdk(String value) {
json.setOrRemove('target_sdk', value);
}

List<String> _validateTargetSdk() => _reader.validate<String?>('target_sdk');
List<String> _validateTargetSdk() => _reader.validate<String>('target_sdk');

int? get targetVersion => _reader.get<int?>('target_version');
int get targetVersion => _reader.get<int>('target_version');

set _targetVersion(int? value) {
set _targetVersion(int value) {
json.setOrRemove('target_version', value);
}

List<String> _validateTargetVersion() =>
_reader.validate<int?>('target_version');
_reader.validate<int>('target_version');

List<String> validate() => [
..._validateTargetSdk(),
Expand Down Expand Up @@ -838,19 +837,19 @@ class MacOSCodeConfig {

MacOSCodeConfig.fromJson(this.json, {this.path = const []});

MacOSCodeConfig({int? targetVersion}) : json = {}, path = const [] {
MacOSCodeConfig({required int targetVersion}) : json = {}, path = const [] {
_targetVersion = targetVersion;
json.sortOnKey();
}

int? get targetVersion => _reader.get<int?>('target_version');
int get targetVersion => _reader.get<int>('target_version');

set _targetVersion(int? value) {
set _targetVersion(int value) {
json.setOrRemove('target_version', value);
}

List<String> _validateTargetVersion() =>
_reader.validate<int?>('target_version');
_reader.validate<int>('target_version');

List<String> validate() => [..._validateTargetVersion()];

Expand Down
Loading