Skip to content

Commit b4f925e

Browse files
authored
refactor: Differentiate pubspec and resolution errors for plugins (#142035)
Part of #137040 and #80374 - Differentiate pubspec and resolution errors - Rename platform to platformKey - Add TODO for rework logic of flag [throwOnPluginPubspecError] - Swap for loop: handle by platform and then by plugin
1 parent 9d53808 commit b4f925e

File tree

2 files changed

+85
-27
lines changed

2 files changed

+85
-27
lines changed

packages/flutter_tools/lib/src/flutter_plugins.dart

+25-24
Original file line numberDiff line numberDiff line change
@@ -1237,39 +1237,42 @@ bool hasPlugins(FlutterProject project) {
12371237
///
12381238
/// For more details, https://flutter.dev/go/federated-plugins.
12391239
// TODO(stuartmorgan): Expand implementation to apply to all implementations,
1240-
// not just Dart-only, per the federated plugin spec.
1240+
// not just Dart-only, per the federated plugin spec.
1241+
// TODO(gustl22): The flag [throwOnPluginPubspecError] is currently only used
1242+
// for testing dart plugins as the logic is not working correctly.
12411243
List<PluginInterfaceResolution> resolvePlatformImplementation(
12421244
List<Plugin> plugins, {
12431245
bool throwOnPluginPubspecError = true,
12441246
}) {
1245-
final List<String> platforms = <String>[
1247+
const Iterable<String> platformKeys = <String>[
12461248
AndroidPlugin.kConfigKey,
12471249
IOSPlugin.kConfigKey,
12481250
LinuxPlugin.kConfigKey,
12491251
MacOSPlugin.kConfigKey,
12501252
WindowsPlugin.kConfigKey,
12511253
];
1252-
final Map<String, List<PluginInterfaceResolution>> possibleResolutions
1253-
= <String, List<PluginInterfaceResolution>>{};
1254+
final Map<String, List<PluginInterfaceResolution>> possibleResolutions =
1255+
<String, List<PluginInterfaceResolution>>{};
12541256
final Map<String, String> defaultImplementations = <String, String>{};
12551257
// Generates a key for the maps above.
12561258
String getResolutionKey({required String platform, required String packageName}) {
12571259
return '$packageName:$platform';
12581260
}
12591261

12601262
bool hasPubspecError = false;
1261-
for (final Plugin plugin in plugins) {
1262-
for (final String platform in platforms) {
1263-
if (plugin.platforms[platform] == null &&
1264-
plugin.defaultPackagePlatforms[platform] == null) {
1263+
for (final String platformKey in platformKeys) {
1264+
for (final Plugin plugin in plugins) {
1265+
if (plugin.platforms[platformKey] == null &&
1266+
plugin.defaultPackagePlatforms[platformKey] == null) {
12651267
// The plugin doesn't implement this platform.
12661268
continue;
12671269
}
12681270
String? implementsPackage = plugin.implementsPackage;
12691271
if (implementsPackage == null || implementsPackage.isEmpty) {
1270-
final String? defaultImplementation = plugin.defaultPackagePlatforms[platform];
1272+
final String? defaultImplementation =
1273+
plugin.defaultPackagePlatforms[platformKey];
12711274
final bool hasInlineDartImplementation =
1272-
plugin.pluginDartClassPlatforms[platform] != null;
1275+
plugin.pluginDartClassPlatforms[platformKey] != null;
12731276
if (defaultImplementation == null && !hasInlineDartImplementation) {
12741277
if (throwOnPluginPubspecError) {
12751278
globals.printError(
@@ -1279,27 +1282,27 @@ List<PluginInterfaceResolution> resolvePlatformImplementation(
12791282
'flutter:\n'
12801283
' plugin:\n'
12811284
' platforms:\n'
1282-
' $platform:\n'
1285+
' $platformKey:\n'
12831286
' $kDartPluginClass: <plugin-class>\n'
12841287
'\n'
12851288
'To set a default implementation, use:\n'
12861289
'flutter:\n'
12871290
' plugin:\n'
12881291
' platforms:\n'
1289-
' $platform:\n'
1292+
' $platformKey:\n'
12901293
' $kDefaultPackage: <plugin-implementation>\n'
12911294
'\n'
12921295
'To implement an interface, use:\n'
12931296
'flutter:\n'
12941297
' plugin:\n'
12951298
' implements: <plugin-interface>'
1296-
'\n'
1299+
'\n',
12971300
);
12981301
}
12991302
hasPubspecError = true;
13001303
continue;
13011304
}
1302-
final String defaultImplementationKey = getResolutionKey(platform: platform, packageName: plugin.name);
1305+
final String defaultImplementationKey = getResolutionKey(platform: platformKey, packageName: plugin.name);
13031306
if (defaultImplementation != null) {
13041307
defaultImplementations[defaultImplementationKey] = defaultImplementation;
13051308
continue;
@@ -1313,7 +1316,7 @@ List<PluginInterfaceResolution> resolvePlatformImplementation(
13131316
// - the plugin requires at least Flutter 2.11 (when this opt-in logic
13141317
// was added), so that existing plugins continue to work.
13151318
// See https://github.com/flutter/flutter/issues/87862 for details.
1316-
final bool isDesktop = platform == 'linux' || platform == 'macos' || platform == 'windows';
1319+
final bool isDesktop = platformKey == 'linux' || platformKey == 'macos' || platformKey == 'windows';
13171320
final semver.VersionConstraint? flutterConstraint = plugin.flutterConstraint;
13181321
final semver.Version? minFlutterVersion = flutterConstraint != null &&
13191322
flutterConstraint is semver.VersionRange ? flutterConstraint.min : null;
@@ -1330,25 +1333,23 @@ List<PluginInterfaceResolution> resolvePlatformImplementation(
13301333
}
13311334
}
13321335
// If there's no Dart implementation, there's nothing to register.
1333-
if (plugin.pluginDartClassPlatforms[platform] == null ||
1334-
plugin.pluginDartClassPlatforms[platform] == 'none') {
1336+
if (plugin.pluginDartClassPlatforms[platformKey] == null ||
1337+
plugin.pluginDartClassPlatforms[platformKey] == 'none') {
13351338
continue;
13361339
}
13371340

13381341
// If it hasn't been skipped, it's a candidate for auto-registration, so
13391342
// add it as a possible resolution.
1340-
final String resolutionKey = getResolutionKey(platform: platform, packageName: implementsPackage);
1341-
if (!possibleResolutions.containsKey(resolutionKey)) {
1342-
possibleResolutions[resolutionKey] = <PluginInterfaceResolution>[];
1343-
}
1343+
final String resolutionKey = getResolutionKey(platform: platformKey, packageName: implementsPackage);
1344+
possibleResolutions.putIfAbsent(resolutionKey, () => <PluginInterfaceResolution>[]);
13441345
possibleResolutions[resolutionKey]!.add(PluginInterfaceResolution(
13451346
plugin: plugin,
1346-
platform: platform,
1347+
platform: platformKey,
13471348
));
13481349
}
13491350
}
13501351
if (hasPubspecError && throwOnPluginPubspecError) {
1351-
throwToolExit('Please resolve the errors');
1352+
throwToolExit('Please resolve the pubspec errors');
13521353
}
13531354

13541355
// Now resolve all the possible resolutions to a single option for each
@@ -1401,7 +1402,7 @@ List<PluginInterfaceResolution> resolvePlatformImplementation(
14011402
}
14021403
}
14031404
if (hasResolutionError) {
1404-
throwToolExit('Please resolve the errors');
1405+
throwToolExit('Please resolve the plugin implementation selection errors');
14051406
}
14061407
return finalResolution;
14071408
}

packages/flutter_tools/test/general.shard/dart_plugin_test.dart

+60-3
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,63 @@ void main() {
9595
);
9696
});
9797

98+
testWithoutContext(
99+
'selects uncontested implementation from direct dependency with additional native implementation',
100+
() async {
101+
final Set<String> directDependencies = <String>{
102+
'url_launcher_linux',
103+
'url_launcher_macos',
104+
};
105+
final List<PluginInterfaceResolution> resolutions =
106+
resolvePlatformImplementation(
107+
<Plugin>[
108+
// Following plugin is native only and is not resolved as a dart plugin:
109+
Plugin.fromYaml(
110+
'url_launcher_linux',
111+
'',
112+
YamlMap.wrap(<String, dynamic>{
113+
'platforms': <String, dynamic>{
114+
'linux': <String, dynamic>{
115+
'package': 'com.example.url_launcher',
116+
'pluginClass': 'UrlLauncherPluginLinux',
117+
},
118+
},
119+
}),
120+
null,
121+
<String>[],
122+
fileSystem: fs,
123+
appDependencies: directDependencies,
124+
),
125+
Plugin.fromYaml(
126+
'url_launcher_macos',
127+
'',
128+
YamlMap.wrap(<String, dynamic>{
129+
'implements': 'url_launcher',
130+
'platforms': <String, dynamic>{
131+
'macos': <String, dynamic>{
132+
'dartPluginClass': 'UrlLauncherPluginMacOS',
133+
},
134+
},
135+
}),
136+
null,
137+
<String>[],
138+
fileSystem: fs,
139+
appDependencies: directDependencies,
140+
),
141+
],
142+
throwOnPluginPubspecError: false,
143+
);
144+
145+
expect(resolutions.length, equals(1));
146+
expect(
147+
resolutions[0].toMap(),
148+
equals(<String, String>{
149+
'pluginName': 'url_launcher_macos',
150+
'dartClass': 'UrlLauncherPluginMacOS',
151+
'platform': 'macos',
152+
}));
153+
});
154+
98155
testWithoutContext('selects uncontested implementation from transitive dependency', () async {
99156
final Set<String> directDependencies = <String>{
100157
'url_launcher_macos',
@@ -538,7 +595,7 @@ void main() {
538595

539596
},
540597
throwsToolExit(
541-
message: 'Please resolve the errors',
598+
message: 'Please resolve the plugin implementation selection errors',
542599
));
543600

544601
expect(
@@ -627,7 +684,7 @@ void main() {
627684
]);
628685
},
629686
throwsToolExit(
630-
message: 'Please resolve the errors',
687+
message: 'Please resolve the plugin implementation selection errors',
631688
));
632689

633690
expect(
@@ -684,7 +741,7 @@ void main() {
684741
]);
685742
},
686743
throwsToolExit(
687-
message: 'Please resolve the errors',
744+
message: 'Please resolve the plugin implementation selection errors',
688745
));
689746

690747
expect(

0 commit comments

Comments
 (0)