Skip to content

Commit ca6eace

Browse files
dcharkesdcharkes
committed
address comments
1 parent 64d9eba commit ca6eace

File tree

2 files changed

+91
-78
lines changed

2 files changed

+91
-78
lines changed

‎pkgs/native_assets_cli/lib/src/code_assets/config.dart

Lines changed: 74 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -53,35 +53,31 @@ class CodeConfig {
5353

5454
CodeConfig(HookConfig config)
5555
: linkModePreference = LinkModePreference.fromString(
56-
config.json.string(linkModePreferenceKey)),
56+
config.json.string(_linkModePreferenceKey)),
5757
// ignore: deprecated_member_use_from_same_package
5858
_targetArchitecture = (config is BuildConfig && config.dryRun)
5959
? null
60-
: Architecture.fromString(config.json.string(targetArchitectureKey,
60+
: Architecture.fromString(config.json.string(_targetArchitectureKey,
6161
validValues: Architecture.values.map((a) => a.name))),
62-
targetOS = OS.fromString(config.json.string(targetOSConfigKey)),
63-
cCompiler = switch (config.json.optionalMap(compilerKey)) {
62+
targetOS = OS.fromString(config.json.string(_targetOSConfigKey)),
63+
cCompiler = switch (config.json.optionalMap(_compilerKey)) {
6464
final Map<String, Object?> map => CCompilerConfig.fromJson(map),
6565
null => null,
6666
} {
6767
// ignore: deprecated_member_use_from_same_package
68-
_iOSConfig = (config is BuildConfig && config.dryRun)
68+
_iOSConfig = (config is BuildConfig && config.dryRun) || targetOS != OS.iOS
6969
? null
70-
: targetOS == OS.iOS
71-
? IOSConfig.fromHookConfig(config)
72-
: null;
73-
// ignore: deprecated_member_use_from_same_package
74-
_androidConfig = (config is BuildConfig && config.dryRun)
75-
? null
76-
: targetOS == OS.android
77-
? AndroidConfig.fromHookConfig(config)
78-
: null;
79-
// ignore: deprecated_member_use_from_same_package
80-
_macOSConfig = (config is BuildConfig && config.dryRun)
81-
? null
82-
: targetOS == OS.macOS
83-
? MacOSConfig.fromHookConfig(config)
84-
: null;
70+
: IOSConfig.fromHookConfig(config);
71+
_androidConfig =
72+
// ignore: deprecated_member_use_from_same_package
73+
(config is BuildConfig && config.dryRun) || targetOS != OS.android
74+
? null
75+
: AndroidConfig.fromHookConfig(config);
76+
_macOSConfig =
77+
// ignore: deprecated_member_use_from_same_package
78+
(config is BuildConfig && config.dryRun) || targetOS != OS.macOS
79+
? null
80+
: MacOSConfig.fromHookConfig(config);
8581
}
8682

8783
Architecture get targetArchitecture {
@@ -121,46 +117,71 @@ class CodeConfig {
121117
/// Configuration provided when [CodeConfig.targetOS] is [OS.iOS].
122118
class IOSConfig {
123119
/// Whether to target device or simulator.
124-
final IOSSdk targetSdk;
120+
IOSSdk get targetSdk => _targetSdk!;
121+
122+
final IOSSdk? _targetSdk;
125123

126124
/// The lowest iOS version that the compiled code will be compatible with.
127-
final int targetVersion;
125+
int get targetVersion => _targetVersion!;
126+
127+
final int? _targetVersion;
128128

129129
IOSConfig({
130-
required this.targetSdk,
131-
required this.targetVersion,
132-
});
130+
required IOSSdk targetSdk,
131+
required int targetVersion,
132+
}) : _targetSdk = targetSdk,
133+
_targetVersion = targetVersion;
133134

134135
IOSConfig.fromHookConfig(HookConfig config)
135-
: targetVersion = config.json.int(targetIOSVersionKey),
136-
targetSdk = IOSSdk.fromString(config.json.string(targetIOSSdkKey));
136+
: _targetVersion = config.json.optionalInt(_targetIOSVersionKey),
137+
_targetSdk = switch (config.json.optionalString(_targetIOSSdkKey)) {
138+
null => null,
139+
String e => IOSSdk.fromString(e)
140+
};
141+
}
142+
143+
extension IOSConfigSyntactic on IOSConfig {
144+
IOSSdk? get targetSdkSyntactic => _targetSdk;
145+
int? get targetVersionSyntactic => _targetVersion;
137146
}
138147

139148
/// Configuration provided when [CodeConfig.targetOS] is [OS.macOS].
140149
class AndroidConfig {
141150
/// The minimum Android SDK API version to that the compiled code will be
142151
/// compatible with.
143-
final int targetNdkApi;
152+
int get targetNdkApi => _targetNdkApi!;
153+
154+
final int? _targetNdkApi;
144155

145156
AndroidConfig({
146-
required this.targetNdkApi,
147-
});
157+
required int targetNdkApi,
158+
}) : _targetNdkApi = targetNdkApi;
148159

149160
AndroidConfig.fromHookConfig(HookConfig config)
150-
: targetNdkApi = config.json.int(targetAndroidNdkApiKey);
161+
: _targetNdkApi = config.json.optionalInt(_targetAndroidNdkApiKey);
162+
}
163+
164+
extension AndroidConfigSyntactic on AndroidConfig {
165+
int? get targetNdkApiSyntactic => _targetNdkApi;
151166
}
152167

153168
//// Configuration provided when [CodeConfig.targetOS] is [OS.macOS].
154169
class MacOSConfig {
155170
/// The lowest MacOS version that the compiled code will be compatible with.
156-
final int targetVersion;
171+
int get targetVersion => _targetVersion!;
172+
173+
final int? _targetVersion;
157174

158175
MacOSConfig({
159-
required this.targetVersion,
160-
});
176+
required int targetVersion,
177+
}) : _targetVersion = targetVersion;
161178

162179
MacOSConfig.fromHookConfig(HookConfig config)
163-
: targetVersion = config.json.int(targetMacOSVersionKey);
180+
: _targetVersion = config.json.optionalInt(_targetMacOSVersionKey);
181+
}
182+
183+
extension MacOSConfigSyntactic on MacOSConfig {
184+
int? get targetVersionSyntactic => _targetVersion;
164185
}
165186

166187
/// Extension to the [BuildOutputBuilder] providing access to emitting code
@@ -216,32 +237,23 @@ extension CodeAssetBuildConfigBuilder on HookConfigBuilder {
216237
MacOSConfig? macOSConfig,
217238
}) {
218239
if (targetArchitecture != null) {
219-
json[targetArchitectureKey] = targetArchitecture.toString();
240+
json[_targetArchitectureKey] = targetArchitecture.toString();
220241
}
221-
json[targetOSConfigKey] = targetOS.toString();
222-
json[linkModePreferenceKey] = linkModePreference.toString();
242+
json[_targetOSConfigKey] = targetOS.toString();
243+
json[_linkModePreferenceKey] = linkModePreference.toString();
223244
if (cCompilerConfig != null) {
224-
json[compilerKey] = cCompilerConfig.toJson();
245+
json[_compilerKey] = cCompilerConfig.toJson();
225246
}
226247

227-
// Note, using ?. instead of !. enables using this in tests to write wrong
228-
// invalid configs. Is this a good idea? The setup methods only create the
229-
// JSON now, but don't enable creating all forms of wrong JSON. For example
230-
// `IOSConfig` either has both fields or none.
231-
// Maybe the builders should be more construct by construction, and the
232-
// validator tests should be on the JSON. This requires duplicating JSON
233-
// serialization logic in the tests though.
234-
// This PR already duplicates deserialization logic in the validator and
235-
// `Config`s. The configs classes need to provide non-nullable accessors, so
236-
// they cannot be used inside the validators to check for invalid null
237-
// values.
248+
// Note, using ?. instead of !. makes missing data be a semantic error
249+
// rather than a syntactic error to be caught in the validation.
238250
if (targetOS == OS.android) {
239-
json[targetAndroidNdkApiKey] = androidConfig?.targetNdkApi;
251+
json[_targetAndroidNdkApiKey] = androidConfig?.targetNdkApi;
240252
} else if (targetOS == OS.iOS) {
241-
json[targetIOSSdkKey] = iOSConfig?.targetSdk.toString();
242-
json[targetIOSVersionKey] = iOSConfig?.targetVersion;
253+
json[_targetIOSSdkKey] = iOSConfig?.targetSdk.toString();
254+
json[_targetIOSVersionKey] = iOSConfig?.targetVersion;
243255
} else if (targetOS == OS.macOS) {
244-
json[targetMacOSVersionKey] = macOSConfig?.targetVersion;
256+
json[_targetMacOSVersionKey] = macOSConfig?.targetVersion;
245257
}
246258
}
247259
}
@@ -264,11 +276,11 @@ extension CodeAssetLinkOutput on LinkOutput {
264276
.toList();
265277
}
266278

267-
const String compilerKey = 'c_compiler';
268-
const String linkModePreferenceKey = 'link_mode_preference';
269-
const String targetAndroidNdkApiKey = 'target_android_ndk_api';
270-
const String targetArchitectureKey = 'target_architecture';
271-
const String targetIOSSdkKey = 'target_ios_sdk';
272-
const String targetIOSVersionKey = 'target_ios_version';
273-
const String targetMacOSVersionKey = 'target_macos_version';
274-
const String targetOSConfigKey = 'target_os';
279+
const String _compilerKey = 'c_compiler';
280+
const String _linkModePreferenceKey = 'link_mode_preference';
281+
const String _targetAndroidNdkApiKey = 'target_android_ndk_api';
282+
const String _targetArchitectureKey = 'target_architecture';
283+
const String _targetIOSSdkKey = 'target_ios_sdk';
284+
const String _targetIOSVersionKey = 'target_ios_version';
285+
const String _targetMacOSVersionKey = 'target_macos_version';
286+
const String _targetOSConfigKey = 'target_os';

‎pkgs/native_assets_cli/lib/src/code_assets/validation.dart

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,60 +5,61 @@
55
import 'dart:io';
66

77
import '../../code_assets_builder.dart';
8-
import '../json_utils.dart';
98
import 'config.dart';
109
import 'link_mode.dart';
1110

12-
// TODO: The validation should work on the JSON, not on the parsed config.
1311
Future<ValidationErrors> validateCodeAssetBuildConfig(
1412
BuildConfig config) async =>
1513
_validateCodeConfig(
1614
'BuildConfig',
1715
// ignore: deprecated_member_use_from_same_package
1816
config.dryRun,
19-
config.json,
17+
config.codeConfig,
2018
);
2119

2220
Future<ValidationErrors> validateCodeAssetLinkConfig(LinkConfig config) async =>
23-
_validateCodeConfig('LinkConfig', false, config.json);
21+
_validateCodeConfig(
22+
'LinkConfig',
23+
false,
24+
config.codeConfig,
25+
);
2426

2527
ValidationErrors _validateCodeConfig(
26-
String configName, bool dryRun, Map<String, Object?> json) {
28+
String configName,
29+
bool dryRun,
30+
CodeConfig codeConfig,
31+
) {
2732
// The dry run will be removed soon.
2833
if (dryRun) return const [];
2934

3035
final errors = <String>[];
31-
final targetOSJson = json[targetOSConfigKey] as String?;
32-
final targetOS = targetOSJson == null ? null : OS.fromString(targetOSJson);
36+
final targetOS = codeConfig.targetOS;
3337
switch (targetOS) {
34-
case null:
35-
errors.add('targetOS is missing');
3638
case OS.macOS:
37-
if (json[targetMacOSVersionKey] == null) {
39+
if (codeConfig.macOSConfig.targetVersionSyntactic == null) {
3840
errors.add('$configName.targetOS is OS.macOS but '
3941
'$configName.codeConfig.macOSConfig.targetVersion was missing');
4042
}
4143
break;
4244
case OS.iOS:
43-
if (json[targetIOSSdkKey] == null) {
45+
if (codeConfig.iOSConfig.targetSdkSyntactic == null) {
4446
errors.add('$configName.targetOS is OS.iOS but '
4547
'$configName.codeConfig.targetIOSSdk was missing');
4648
}
47-
if (json[targetIOSVersionKey] == null) {
49+
if (codeConfig.iOSConfig.targetVersionSyntactic == null) {
4850
errors.add('$configName.targetOS is OS.iOS but '
4951
'$configName.codeConfig.iOSConfig.targetVersion was missing');
5052
}
5153
break;
5254
case OS.android:
53-
if (json[targetAndroidNdkApiKey] == null) {
55+
if (codeConfig.androidConfig.targetNdkApiSyntactic == null) {
5456
errors.add('$configName.targetOS is OS.android but '
5557
'$configName.codeConfig.androidConfig.targetNdkApi was missing');
5658
}
5759
break;
5860
}
59-
final compilerConfigJson = json.optionalMap(compilerKey);
60-
if (compilerConfigJson != null) {
61-
final compilerConfig = CCompilerConfig.fromJson(compilerConfigJson);
61+
final compilerConfig = codeConfig.cCompiler;
62+
if (compilerConfig != null) {
6263
final compiler = compilerConfig.compiler.toFilePath();
6364
if (!File(compiler).existsSync()) {
6465
errors.add('$configName.codeConfig.compiler ($compiler) does not exist.');

0 commit comments

Comments
 (0)