diff --git a/packages/flutter_tools/lib/src/flutter_plugins.dart b/packages/flutter_tools/lib/src/flutter_plugins.dart index 9fc22d563c6b..a09486229680 100644 --- a/packages/flutter_tools/lib/src/flutter_plugins.dart +++ b/packages/flutter_tools/lib/src/flutter_plugins.dart @@ -1191,13 +1191,13 @@ bool hasPlugins(FlutterProject project) { /// Resolves the platform implementation for Dart-only plugins. /// -/// * If there are multiple direct pub dependencies on packages that implement the -/// frontend plugin for the current platform, fail. +/// * If there is only one dependency on a package that implements the +/// frontend plugin for the current platform, use that. /// * If there is a single direct dependency on a package that implements the -/// frontend plugin for the target platform, this package is the selected implementation. -/// * If there is no direct dependency on a package that implements the frontend -/// plugin for the target platform, and the frontend plugin has a default implementation -/// for the target platform the default implementation is selected. +/// frontend plugin for the current platform, use that. +/// * If there is no direct dependency on a package that implements the +/// frontend plugin, but there is a default for the current platform, +/// use that. /// * Else fail. /// /// For more details, https://flutter.dev/go/federated-plugins. @@ -1214,11 +1214,15 @@ List resolvePlatformImplementation( MacOSPlugin.kConfigKey, WindowsPlugin.kConfigKey, ]; - final Map directDependencyResolutions - = {}; + final Map> possibleResolutions + = >{}; final Map defaultImplementations = {}; - bool didFindError = false; + // Generates a key for the maps above. + String getResolutionKey({required String platform, required String packageName}) { + return '$packageName:$platform'; + } + bool hasPubspecError = false; for (final Plugin plugin in plugins) { for (final String platform in platforms) { if (plugin.platforms[platform] == null && @@ -1257,11 +1261,12 @@ List resolvePlatformImplementation( '\n' ); } - didFindError = true; + hasPubspecError = true; continue; } + final String defaultImplementationKey = getResolutionKey(platform: platform, packageName: plugin.name); if (defaultImplementation != null) { - defaultImplementations['$platform/${plugin.name}'] = defaultImplementation; + defaultImplementations[defaultImplementationKey] = defaultImplementation; continue; } else { // An app-facing package (i.e., one with no 'implements') with an @@ -1281,52 +1286,87 @@ List resolvePlatformImplementation( minFlutterVersion.compareTo(semver.Version(2, 11, 0)) >= 0; if (!isDesktop || hasMinVersionForImplementsRequirement) { implementsPackage = plugin.name; - defaultImplementations['$platform/${plugin.name}'] = plugin.name; + defaultImplementations[defaultImplementationKey] = plugin.name; + } else { + // If it doesn't meet any of the conditions, it isn't eligible for + // auto-registration. + continue; } } } + // If there's no Dart implementation, there's nothing to register. if (plugin.pluginDartClassPlatforms[platform] == null || plugin.pluginDartClassPlatforms[platform] == 'none') { continue; } - final String resolutionKey = '$platform/$implementsPackage'; - if (directDependencyResolutions.containsKey(resolutionKey)) { - final PluginInterfaceResolution? currResolution = directDependencyResolutions[resolutionKey]; - if (currResolution != null && currResolution.plugin.isDirectDependency) { - if (plugin.isDirectDependency) { - if (throwOnPluginPubspecError) { - globals.printError( - 'Plugin `${plugin.name}` implements an interface for `$platform`, which was already ' - 'implemented by plugin `${currResolution.plugin.name}`.\n' - 'To fix this issue, remove either dependency from pubspec.yaml.' - '\n\n' - ); - } - didFindError = true; - } - // Use the plugin implementation added by the user as a direct dependency. - continue; - } + + // If it hasn't been skipped, it's a candidate for auto-registration, so + // add it as a possible resolution. + final String resolutionKey = getResolutionKey(platform: platform, packageName: implementsPackage); + if (!possibleResolutions.containsKey(resolutionKey)) { + possibleResolutions[resolutionKey] = []; } - directDependencyResolutions[resolutionKey] = PluginInterfaceResolution( + possibleResolutions[resolutionKey]!.add(PluginInterfaceResolution( plugin: plugin, platform: platform, - ); + )); } } - if (didFindError && throwOnPluginPubspecError) { + if (hasPubspecError && throwOnPluginPubspecError) { throwToolExit('Please resolve the errors'); } + + // Now resolve all the possible resolutions to a single option for each + // plugin, or throw if that's not possible. + bool hasResolutionError = false; final List finalResolution = []; - for (final MapEntry resolution in directDependencyResolutions.entries) { - if (resolution.value.plugin.isDirectDependency) { - finalResolution.add(resolution.value); - } else if (defaultImplementations.containsKey(resolution.key)) { - // Pick the default implementation. - if (defaultImplementations[resolution.key] == resolution.value.plugin.name) { - finalResolution.add(resolution.value); + for (final MapEntry> entry in possibleResolutions.entries) { + final List candidates = entry.value; + // If there's only one candidate, use it. + if (candidates.length == 1) { + finalResolution.add(candidates.first); + continue; + } + // Next, try direct dependencies of the resolving application. + final Iterable directDependencies = candidates.where((PluginInterfaceResolution r) { + return r.plugin.isDirectDependency; + }); + if (directDependencies.isNotEmpty) { + if (directDependencies.length > 1) { + globals.printError( + 'Plugin ${entry.key} has conflicting direct dependency implementations:\n' + '${directDependencies.map((PluginInterfaceResolution r) => ' ${r.plugin.name}\n').join()}' + 'To fix this issue, remove all but one of these dependencies from pubspec.yaml.\n' + ); + hasResolutionError = true; + } else { + finalResolution.add(directDependencies.first); + } + continue; + } + // Next, defer to the default implementation if there is one. + final String? defaultPackageName = defaultImplementations[entry.key]; + if (defaultPackageName != null) { + final int defaultIndex = candidates + .indexWhere((PluginInterfaceResolution r) => r.plugin.name == defaultPackageName); + if (defaultIndex != -1) { + finalResolution.add(candidates[defaultIndex]); + continue; } } + // Otherwise, require an explicit choice. + if (candidates.length > 1) { + globals.printError( + 'Plugin ${entry.key} has multiple possible implementations:\n' + '${candidates.map((PluginInterfaceResolution r) => ' ${r.plugin.name}\n').join()}' + 'To fix this issue, add one of these dependencies to pubspec.yaml.\n' + ); + hasResolutionError = true; + continue; + } + } + if (hasResolutionError) { + throwToolExit('Please resolve the errors'); } return finalResolution; } diff --git a/packages/flutter_tools/lib/src/plugins.dart b/packages/flutter_tools/lib/src/plugins.dart index b799a560856f..f43309a4e7c8 100644 --- a/packages/flutter_tools/lib/src/plugins.dart +++ b/packages/flutter_tools/lib/src/plugins.dart @@ -418,4 +418,9 @@ class PluginInterfaceResolution { 'dartClass': plugin.pluginDartClassPlatforms[platform] ?? '', }; } + + @override + String toString() { + return ''; + } } diff --git a/packages/flutter_tools/test/general.shard/dart_plugin_test.dart b/packages/flutter_tools/test/general.shard/dart_plugin_test.dart index a42d265b45c2..6c5633ff19de 100644 --- a/packages/flutter_tools/test/general.shard/dart_plugin_test.dart +++ b/packages/flutter_tools/test/general.shard/dart_plugin_test.dart @@ -38,7 +38,7 @@ void main() { }); group('resolvePlatformImplementation', () { - testWithoutContext('selects implementation from direct dependency', () async { + testWithoutContext('selects uncontested implementation from direct dependency', () async { final Set directDependencies = { 'url_launcher_linux', 'url_launcher_macos', @@ -76,14 +76,38 @@ void main() { fileSystem: fs, appDependencies: directDependencies, ), + ]); + + expect(resolutions.length, equals(2)); + expect(resolutions[0].toMap(), equals( + { + 'pluginName': 'url_launcher_linux', + 'dartClass': 'UrlLauncherPluginLinux', + 'platform': 'linux', + }) + ); + expect(resolutions[1].toMap(), equals( + { + 'pluginName': 'url_launcher_macos', + 'dartClass': 'UrlLauncherPluginMacOS', + 'platform': 'macos', + }) + ); + }); + + testWithoutContext('selects uncontested implementation from transitive dependency', () async { + final Set directDependencies = { + 'url_launcher_macos', + }; + final List resolutions = resolvePlatformImplementation([ Plugin.fromYaml( - 'undirect_dependency_plugin', + 'url_launcher_macos', '', YamlMap.wrap({ 'implements': 'url_launcher', 'platforms': { - 'windows': { - 'dartPluginClass': 'UrlLauncherPluginWindows', + 'macos': { + 'dartPluginClass': 'UrlLauncherPluginMacOS', }, }, }), @@ -92,17 +116,14 @@ void main() { fileSystem: fs, appDependencies: directDependencies, ), - ]); - - resolvePlatformImplementation([ Plugin.fromYaml( - 'url_launcher_macos', + 'transitive_dependency_plugin', '', YamlMap.wrap({ 'implements': 'url_launcher', 'platforms': { - 'macos': { - 'dartPluginClass': 'UrlLauncherPluginMacOS', + 'windows': { + 'dartPluginClass': 'UrlLauncherPluginWindows', }, }, }), @@ -116,16 +137,16 @@ void main() { expect(resolutions.length, equals(2)); expect(resolutions[0].toMap(), equals( { - 'pluginName': 'url_launcher_linux', - 'dartClass': 'UrlLauncherPluginLinux', - 'platform': 'linux', + 'pluginName': 'url_launcher_macos', + 'dartClass': 'UrlLauncherPluginMacOS', + 'platform': 'macos', }) ); expect(resolutions[1].toMap(), equals( { - 'pluginName': 'url_launcher_macos', - 'dartClass': 'UrlLauncherPluginMacOS', - 'platform': 'macos', + 'pluginName': 'transitive_dependency_plugin', + 'dartClass': 'UrlLauncherPluginWindows', + 'platform': 'windows', }) ); }); @@ -301,14 +322,17 @@ void main() { fileSystem: fs, appDependencies: directDependencies, ), + // Include three possible implementations, one before and one after + // to ensure that the selection is working as intended, not just by + // coincidence of order. Plugin.fromYaml( - 'url_launcher_linux', + 'another_url_launcher_linux', '', YamlMap.wrap({ 'implements': 'url_launcher', 'platforms': { 'linux': { - 'dartPluginClass': 'UrlLauncherPluginLinux', + 'dartPluginClass': 'UnofficialUrlLauncherPluginLinux', }, }, }), @@ -317,28 +341,14 @@ void main() { fileSystem: fs, appDependencies: directDependencies, ), - ]); - expect(resolutions.length, equals(1)); - expect(resolutions[0].toMap(), equals( - { - 'pluginName': 'url_launcher_linux', - 'dartClass': 'UrlLauncherPluginLinux', - 'platform': 'linux', - }) - ); - }); - - testWithoutContext('selects default implementation if interface is direct dependency', () async { - final Set directDependencies = {'url_launcher'}; - - final List resolutions = resolvePlatformImplementation([ Plugin.fromYaml( - 'url_launcher', + 'url_launcher_linux', '', YamlMap.wrap({ + 'implements': 'url_launcher', 'platforms': { 'linux': { - 'default_package': 'url_launcher_linux', + 'dartPluginClass': 'UrlLauncherPluginLinux', }, }, }), @@ -348,13 +358,13 @@ void main() { appDependencies: directDependencies, ), Plugin.fromYaml( - 'url_launcher_linux', + 'yet_another_url_launcher_linux', '', YamlMap.wrap({ 'implements': 'url_launcher', 'platforms': { 'linux': { - 'dartPluginClass': 'UrlLauncherPluginLinux', + 'dartPluginClass': 'UnofficialUrlLauncherPluginLinux2', }, }, }), @@ -374,11 +384,8 @@ void main() { ); }); - testWithoutContext('selects user selected implementation despites default implementation', () async { - final Set directDependencies = { - 'user_selected_url_launcher_implementation', - 'url_launcher', - }; + testWithoutContext('selects default implementation if interface is direct dependency', () async { + final Set directDependencies = {'url_launcher'}; final List resolutions = resolvePlatformImplementation([ Plugin.fromYaml( @@ -412,27 +419,11 @@ void main() { fileSystem: fs, appDependencies: directDependencies, ), - Plugin.fromYaml( - 'user_selected_url_launcher_implementation', - '', - YamlMap.wrap({ - 'implements': 'url_launcher', - 'platforms': { - 'linux': { - 'dartPluginClass': 'UrlLauncherPluginLinux', - }, - }, - }), - null, - [], - fileSystem: fs, - appDependencies: directDependencies, - ), ]); expect(resolutions.length, equals(1)); expect(resolutions[0].toMap(), equals( { - 'pluginName': 'user_selected_url_launcher_implementation', + 'pluginName': 'url_launcher_linux', 'dartClass': 'UrlLauncherPluginLinux', 'platform': 'linux', }) @@ -545,22 +536,27 @@ void main() { ), ]); - expect( - testLogger.errorText, - 'Plugin `url_launcher_linux_2` implements an interface for `linux`, which was already implemented by plugin `url_launcher_linux_1`.\n' - 'To fix this issue, remove either dependency from pubspec.yaml.' - '\n\n' - ); }, throwsToolExit( message: 'Please resolve the errors', )); + + expect( + testLogger.errorText, + 'Plugin url_launcher:linux has conflicting direct dependency implementations:\n' + ' url_launcher_linux_1\n' + ' url_launcher_linux_2\n' + 'To fix this issue, remove all but one of these dependencies from pubspec.yaml.' + '\n\n' + ); }); testUsingContext('provides all errors when user selected multiple implementations', () async { final Set directDependencies = { 'url_launcher_linux_1', 'url_launcher_linux_2', + 'url_launcher_windows_1', + 'url_launcher_windows_2', }; expect(() { resolvePlatformImplementation([ @@ -596,18 +592,109 @@ void main() { fileSystem: fs, appDependencies: directDependencies, ), + Plugin.fromYaml( + 'url_launcher_windows_1', + '', + YamlMap.wrap({ + 'implements': 'url_launcher', + 'platforms': { + 'windows': { + 'dartPluginClass': 'UrlLauncherPluginWindows1', + }, + }, + }), + null, + [], + fileSystem: fs, + appDependencies: directDependencies, + ), + Plugin.fromYaml( + 'url_launcher_windows_2', + '', + YamlMap.wrap({ + 'implements': 'url_launcher', + 'platforms': { + 'windows': { + 'dartPluginClass': 'UrlLauncherPluginWindows2', + }, + }, + }), + null, + [], + fileSystem: fs, + appDependencies: directDependencies, + ), ]); + }, + throwsToolExit( + message: 'Please resolve the errors', + )); - expect( - testLogger.errorText, - 'Plugin `url_launcher_linux_2` implements an interface for `linux`, which was already implemented by plugin `url_launcher_linux_1`.\n' - 'To fix this issue, remove either dependency from pubspec.yaml.' - '\n\n' - ); + expect( + testLogger.errorText, + 'Plugin url_launcher:linux has conflicting direct dependency implementations:\n' + ' url_launcher_linux_1\n' + ' url_launcher_linux_2\n' + 'To fix this issue, remove all but one of these dependencies from pubspec.yaml.' + '\n\n' + 'Plugin url_launcher:windows has conflicting direct dependency implementations:\n' + ' url_launcher_windows_1\n' + ' url_launcher_windows_2\n' + 'To fix this issue, remove all but one of these dependencies from pubspec.yaml.' + '\n\n' + ); + }); + + testUsingContext('provides error when user needs to select among multiple implementations', () async { + final Set directDependencies = {}; + expect(() { + resolvePlatformImplementation([ + Plugin.fromYaml( + 'url_launcher_linux_1', + '', + YamlMap.wrap({ + 'implements': 'url_launcher', + 'platforms': { + 'linux': { + 'dartPluginClass': 'UrlLauncherPluginLinux1', + }, + }, + }), + null, + [], + fileSystem: fs, + appDependencies: directDependencies, + ), + Plugin.fromYaml( + 'url_launcher_linux_2', + '', + YamlMap.wrap({ + 'implements': 'url_launcher', + 'platforms': { + 'linux': { + 'dartPluginClass': 'UrlLauncherPluginLinux2', + }, + }, + }), + null, + [], + fileSystem: fs, + appDependencies: directDependencies, + ), + ]); }, throwsToolExit( message: 'Please resolve the errors', )); + + expect( + testLogger.errorText, + 'Plugin url_launcher:linux has multiple possible implementations:\n' + ' url_launcher_linux_1\n' + ' url_launcher_linux_2\n' + 'To fix this issue, add one of these dependencies to pubspec.yaml.' + '\n\n' + ); }); }); @@ -994,8 +1081,11 @@ void createFakeDartPlugins( ) { final Directory fakePubCache = fs.systemTempDirectory.childDirectory('cache'); final File packagesFile = flutterProject.directory - .childFile('.packages') - ..createSync(recursive: true); + .childFile('.packages'); + if (packagesFile.existsSync()) { + packagesFile.deleteSync(); + } + packagesFile.createSync(recursive: true); for (final MapEntry entry in plugins.entries) { final String name = fs.path.basename(entry.key);