From e0127526344a3d6ca8340ec484a7adb72cf61ce3 Mon Sep 17 00:00:00 2001 From: Francisco Magdaleno Date: Fri, 3 Apr 2020 11:51:01 -0700 Subject: [PATCH] [flutter_tools] Don't generate native registrant classes if no pluginClass is defined (#53785) --- .../lib/src/platform_plugins.dart | 83 +++++++++++------ packages/flutter_tools/lib/src/plugins.dart | 29 +++++- .../flutter_tools/schema/pubspec_yaml.json | 9 +- .../general.shard/plugin_parsing_test.dart | 27 ++++++ .../test/general.shard/plugins_test.dart | 88 ++++++++++++++++++- 5 files changed, 203 insertions(+), 33 deletions(-) diff --git a/packages/flutter_tools/lib/src/platform_plugins.dart b/packages/flutter_tools/lib/src/platform_plugins.dart index cdf31adb34..b98b5ee261 100644 --- a/packages/flutter_tools/lib/src/platform_plugins.dart +++ b/packages/flutter_tools/lib/src/platform_plugins.dart @@ -9,6 +9,12 @@ import 'base/common.dart'; import 'base/file_system.dart'; import 'globals.dart' as globals; +/// Constant for 'pluginClass' key in plugin maps. +const String kPluginClass = 'pluginClass'; + +/// Constant for 'pluginClass' key in plugin maps. +const String kDartPluginClass = 'dartPluginClass'; + /// Marker interface for all platform specific plugin config impls. abstract class PluginPlatform { const PluginPlatform(); @@ -16,6 +22,12 @@ abstract class PluginPlatform { Map toMap(); } +abstract class NativeOrDartPlugin { + /// Determines whether the plugin has a native implementation or if it's a + /// Dart-only plugin. + bool isNative(); +} + /// Contains parameters to template an Android plugin. /// /// The required fields include: [name] of the plugin, [package] of the plugin and @@ -191,19 +203,21 @@ class IOSPlugin extends PluginPlatform { /// Contains the parameters to template a macOS plugin. /// -/// The required fields include: [name] of the plugin, and [pluginClass] that will -/// be the entry point to the plugin's native code. -class MacOSPlugin extends PluginPlatform { +/// The [name] of the plugin is required. Either [dartPluginClass] or [pluginClass] are required. +/// [pluginClass] will be the entry point to the plugin's native code. +class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin { const MacOSPlugin({ @required this.name, - @required this.pluginClass, + this.pluginClass, + this.dartPluginClass, }); factory MacOSPlugin.fromYaml(String name, YamlMap yaml) { assert(validate(yaml)); return MacOSPlugin( name: name, - pluginClass: yaml['pluginClass'] as String, + pluginClass: yaml[kPluginClass] as String, + dartPluginClass: yaml[kDartPluginClass] as String, ); } @@ -211,38 +225,45 @@ class MacOSPlugin extends PluginPlatform { if (yaml == null) { return false; } - return yaml['pluginClass'] is String; + return yaml[kPluginClass] is String || yaml[kDartPluginClass] is String; } static const String kConfigKey = 'macos'; final String name; final String pluginClass; + final String dartPluginClass; + + @override + bool isNative() => pluginClass != null; @override Map toMap() { return { 'name': name, - 'class': pluginClass, + if (pluginClass != null) 'class': pluginClass, + if (dartPluginClass != null) 'dartPluginClass': dartPluginClass, }; } } /// Contains the parameters to template a Windows plugin. /// -/// The required fields include: [name] of the plugin, and [pluginClass] that will -/// be the entry point to the plugin's native code. -class WindowsPlugin extends PluginPlatform { +/// The [name] of the plugin is required. Either [dartPluginClass] or [pluginClass] are required. +/// [pluginClass] will be the entry point to the plugin's native code. +class WindowsPlugin extends PluginPlatform implements NativeOrDartPlugin{ const WindowsPlugin({ @required this.name, - @required this.pluginClass, - }); + this.pluginClass, + this.dartPluginClass, + }) : assert(pluginClass != null || dartPluginClass != null); factory WindowsPlugin.fromYaml(String name, YamlMap yaml) { assert(validate(yaml)); return WindowsPlugin( name: name, - pluginClass: yaml['pluginClass'] as String, + pluginClass: yaml[kPluginClass] as String, + dartPluginClass: yaml[kDartPluginClass] as String, ); } @@ -250,39 +271,46 @@ class WindowsPlugin extends PluginPlatform { if (yaml == null) { return false; } - return yaml['pluginClass'] is String; + return yaml[kDartPluginClass] is String || yaml[kPluginClass] is String; } static const String kConfigKey = 'windows'; final String name; final String pluginClass; + final String dartPluginClass; + + @override + bool isNative() => pluginClass != null; @override Map toMap() { return { 'name': name, - 'class': pluginClass, - 'filename': _filenameForCppClass(pluginClass), + if (pluginClass != null) 'class': pluginClass, + if (pluginClass != null) 'filename': _filenameForCppClass(pluginClass), + if (dartPluginClass != null) 'dartPluginClass': dartPluginClass, }; } } /// Contains the parameters to template a Linux plugin. /// -/// The required fields include: [name] of the plugin, and [pluginClass] that will -/// be the entry point to the plugin's native code. -class LinuxPlugin extends PluginPlatform { +/// The [name] of the plugin is required. Either [dartPluginClass] or [pluginClass] are required. +/// [pluginClass] will be the entry point to the plugin's native code. +class LinuxPlugin extends PluginPlatform implements NativeOrDartPlugin { const LinuxPlugin({ @required this.name, - @required this.pluginClass, - }); + this.pluginClass, + this.dartPluginClass, + }) : assert(pluginClass != null || dartPluginClass != null); factory LinuxPlugin.fromYaml(String name, YamlMap yaml) { assert(validate(yaml)); return LinuxPlugin( name: name, - pluginClass: yaml['pluginClass'] as String, + pluginClass: yaml[kPluginClass] as String, + dartPluginClass: yaml[kDartPluginClass] as String, ); } @@ -290,20 +318,25 @@ class LinuxPlugin extends PluginPlatform { if (yaml == null) { return false; } - return yaml['pluginClass'] is String; + return yaml[kPluginClass] is String || yaml[kDartPluginClass] is String; } static const String kConfigKey = 'linux'; final String name; final String pluginClass; + final String dartPluginClass; + + @override + bool isNative() => pluginClass != null; @override Map toMap() { return { 'name': name, - 'class': pluginClass, - 'filename': _filenameForCppClass(pluginClass), + if (pluginClass != null) 'class': pluginClass, + if (pluginClass != null) 'filename': _filenameForCppClass(pluginClass), + if (dartPluginClass != null) 'dartPluginClass': dartPluginClass, }; } } diff --git a/packages/flutter_tools/lib/src/plugins.dart b/packages/flutter_tools/lib/src/plugins.dart index c5a7aae592..366bcbc916 100644 --- a/packages/flutter_tools/lib/src/plugins.dart +++ b/packages/flutter_tools/lib/src/plugins.dart @@ -871,7 +871,8 @@ Future _writeIOSPluginRegistrant(FlutterProject project, List plug } Future _writeLinuxPluginFiles(FlutterProject project, List plugins) async { - final List> linuxPlugins = _extractPlatformMaps(plugins, LinuxPlugin.kConfigKey); + final ListnativePlugins = _filterNativePlugins(plugins, LinuxPlugin.kConfigKey); + final List> linuxPlugins = _extractPlatformMaps(nativePlugins, LinuxPlugin.kConfigKey); // The generated makefile is checked in, so can't use absolute paths. It is // included by the main makefile, so relative paths must be relative to that // file's directory. @@ -901,7 +902,8 @@ Future _writeLinuxPluginMakefile(Directory destination, Map _writeMacOSPluginRegistrant(FlutterProject project, List plugins) async { - final List> macosPlugins = _extractPlatformMaps(plugins, MacOSPlugin.kConfigKey); + final ListnativePlugins = _filterNativePlugins(plugins, MacOSPlugin.kConfigKey); + final List> macosPlugins = _extractPlatformMaps(nativePlugins, MacOSPlugin.kConfigKey); final Map context = { 'os': 'macos', 'framework': 'FlutterMacOS', @@ -915,8 +917,25 @@ Future _writeMacOSPluginRegistrant(FlutterProject project, List pl ); } +/// Filters out Dart-only plugins, which shouldn't be added to the native generated registrants. +List _filterNativePlugins(List plugins, String platformKey) { + return plugins.where((Plugin element) { + final PluginPlatform plugin = element.platforms[platformKey]; + if (plugin == null) { + return false; + } + if (plugin is NativeOrDartPlugin) { + return (plugin as NativeOrDartPlugin).isNative(); + } + // Not all platforms have the ability to create Dart-only plugins. Therefore, any plugin that doesn't + // implement NativeOrDartPlugin is always native. + return true; + }).toList(); +} + Future _writeWindowsPluginFiles(FlutterProject project, List plugins) async { - final List> windowsPlugins = _extractPlatformMaps(plugins, WindowsPlugin.kConfigKey); + final ListnativePlugins = _filterNativePlugins(plugins, WindowsPlugin.kConfigKey); + final List> windowsPlugins = _extractPlatformMaps(nativePlugins, WindowsPlugin.kConfigKey); final Map context = { 'plugins': windowsPlugins, }; @@ -1092,7 +1111,9 @@ Future injectPlugins(FlutterProject project, {bool checkProjects = false}) } if (featureFlags.isWindowsEnabled && project.windows.existsSync()) { await _writeWindowsPluginFiles(project, plugins); - await VisualStudioSolutionUtils(project: project.windows, fileSystem: globals.fs).updatePlugins(plugins); + + final ListnativePlugins = _filterNativePlugins(plugins, WindowsPlugin.kConfigKey); + await VisualStudioSolutionUtils(project: project.windows, fileSystem: globals.fs).updatePlugins(nativePlugins); } for (final XcodeBasedProject subproject in [project.ios, project.macos]) { if (!project.isModule && (!checkProjects || subproject.existsSync())) { diff --git a/packages/flutter_tools/schema/pubspec_yaml.json b/packages/flutter_tools/schema/pubspec_yaml.json index acb0d052dd..d32784dfbf 100644 --- a/packages/flutter_tools/schema/pubspec_yaml.json +++ b/packages/flutter_tools/schema/pubspec_yaml.json @@ -80,21 +80,24 @@ "type": "object", "additionalProperties": false, "properties": { - "pluginClass": {"type": "string"} + "pluginClass": {"type": "string"}, + "dartPluginClass": {"type": "string"} } }, "macos": { "type": "object", "additionalProperties": false, "properties": { - "pluginClass": {"type": "string"} + "pluginClass": {"type": "string"}, + "dartPluginClass": {"type": "string"} } }, "windows": { "type": "object", "additionalProperties": false, "properties": { - "pluginClass": {"type": "string"} + "pluginClass": {"type": "string"}, + "dartPluginClass": {"type": "string"} } } } diff --git a/packages/flutter_tools/test/general.shard/plugin_parsing_test.dart b/packages/flutter_tools/test/general.shard/plugin_parsing_test.dart index 3753fc4580..680b343740 100644 --- a/packages/flutter_tools/test/general.shard/plugin_parsing_test.dart +++ b/packages/flutter_tools/test/general.shard/plugin_parsing_test.dart @@ -122,6 +122,33 @@ void main() { expect(windowsPlugin.pluginClass, 'WinSamplePlugin'); }); + test('Allow for Dart-only plugins without a pluginClass', () { + /// This is currently supported only on macOS, linux, Windows. + const String pluginYamlRaw = 'implements: same_plugin\n' // this should be ignored by the tool + 'platforms:\n' + ' linux:\n' + ' dartPluginClass: LSamplePlugin\n' + ' macos:\n' + ' dartPluginClass: MSamplePlugin\n' + ' windows:\n' + ' dartPluginClass: WinSamplePlugin\n'; + + final dynamic pluginYaml = loadYaml(pluginYamlRaw); + final Plugin plugin = + Plugin.fromYaml(_kTestPluginName, _kTestPluginPath, pluginYaml as YamlMap, const []); + + final LinuxPlugin linuxPlugin = plugin.platforms[LinuxPlugin.kConfigKey] as LinuxPlugin; + final MacOSPlugin macOSPlugin = plugin.platforms[MacOSPlugin.kConfigKey] as MacOSPlugin; + final WindowsPlugin windowsPlugin = plugin.platforms[WindowsPlugin.kConfigKey] as WindowsPlugin; + + expect(linuxPlugin.pluginClass, isNull); + expect(macOSPlugin.pluginClass, isNull); + expect(windowsPlugin.pluginClass, isNull); + expect(linuxPlugin.dartPluginClass, 'LSamplePlugin'); + expect(macOSPlugin.dartPluginClass, 'MSamplePlugin'); + expect(windowsPlugin.dartPluginClass, 'WinSamplePlugin'); + }); + test('Legacy Format and Multi-Platform Format together is not allowed and error message contains plugin name', () { const String pluginYamlRaw = 'androidPackage: com.flutter.dev\n' 'platforms:\n' diff --git a/packages/flutter_tools/test/general.shard/plugins_test.dart b/packages/flutter_tools/test/general.shard/plugins_test.dart index b4da1f5e32..bbc52a18fb 100644 --- a/packages/flutter_tools/test/general.shard/plugins_test.dart +++ b/packages/flutter_tools/test/general.shard/plugins_test.dart @@ -6,6 +6,7 @@ import 'dart:convert'; import 'package:file/file.dart'; import 'package:file/memory.dart'; +import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/time.dart'; import 'package:flutter_tools/src/dart/package_map.dart'; import 'package:flutter_tools/src/features.dart'; @@ -56,6 +57,8 @@ void main() { when(flutterProject.macos).thenReturn(macosProject); when(macosProject.podfile).thenReturn(flutterProject.directory.childDirectory('macos').childFile('Podfile')); when(macosProject.podManifestLock).thenReturn(flutterProject.directory.childDirectory('macos').childFile('Podfile.lock')); + final Directory macosManagedDirectory = flutterProject.directory.childDirectory('macos').childDirectory('Flutter'); + when(macosProject.managedDirectory).thenReturn(macosManagedDirectory); when(macosProject.pluginConfigKey).thenReturn('macos'); when(macosProject.existsSync()).thenReturn(false); androidProject = MockAndroidProject(); @@ -129,7 +132,6 @@ void main() { '''); } - void createNewJavaPlugin1() { final Directory pluginUsingJavaAndNewEmbeddingDir = fs.systemTempDirectory.createTempSync('flutter_plugin_using_java_and_new_embedding_dir.'); @@ -879,6 +881,33 @@ web_plugin_with_nested:${webPluginWithNestedFile.childDirectory('lib').uri.toStr FeatureFlags: () => featureFlags, }); + testUsingContext('Injecting creates generated macos registrant, but does not include Dart-only plugins', () async { + when(macosProject.existsSync()).thenReturn(true); + when(featureFlags.isMacOSEnabled).thenReturn(true); + when(flutterProject.isModule).thenReturn(true); + // Create a plugin without a pluginClass. + dummyPackageDirectory.parent.childFile('pubspec.yaml') + ..createSync(recursive: true) + ..writeAsStringSync(''' +flutter: + plugin: + platforms: + macos: + dartPluginClass: SomePlugin + '''); + + await injectPlugins(flutterProject, checkProjects: true); + + final File registrantFile = macosProject.managedDirectory.childFile('GeneratedPluginRegistrant.swift'); + + expect(registrantFile, exists); + expect(registrantFile, isNot(contains('SomePlugin'))); + }, overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: () => featureFlags, + }); + testUsingContext('Injecting creates generated Linux registrant', () async { when(linuxProject.existsSync()).thenReturn(true); when(featureFlags.isLinuxEnabled).thenReturn(true); @@ -899,6 +928,33 @@ web_plugin_with_nested:${webPluginWithNestedFile.childDirectory('lib').uri.toStr FeatureFlags: () => featureFlags, }); + testUsingContext('Injecting creates generated Linux registrant, but does not include Dart-only plugins', () async { + when(linuxProject.existsSync()).thenReturn(true); + when(featureFlags.isLinuxEnabled).thenReturn(true); + when(flutterProject.isModule).thenReturn(false); + // Create a plugin without a pluginClass. + dummyPackageDirectory.parent.childFile('pubspec.yaml') + ..createSync(recursive: true) + ..writeAsStringSync(''' +flutter: + plugin: + platforms: + linux: + dartPluginClass: SomePlugin + '''); + + await injectPlugins(flutterProject, checkProjects: true); + + final File registrantImpl = linuxProject.managedDirectory.childFile('generated_plugin_registrant.cc'); + + expect(registrantImpl, exists); + expect(registrantImpl, isNot(contains('SomePlugin'))); + }, overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: () => featureFlags, + }); + testUsingContext('Injecting creates generated Linux plugin makefile', () async { when(linuxProject.existsSync()).thenReturn(true); when(featureFlags.isLinuxEnabled).thenReturn(true); @@ -946,6 +1002,36 @@ web_plugin_with_nested:${webPluginWithNestedFile.childDirectory('lib').uri.toStr FeatureFlags: () => featureFlags, }); + testUsingContext('Injecting creates generated Windows registrant, but does not include Dart-only plugins', () async { + when(windowsProject.existsSync()).thenReturn(true); + when(featureFlags.isWindowsEnabled).thenReturn(true); + when(flutterProject.isModule).thenReturn(false); + // Create a plugin without a pluginClass. + dummyPackageDirectory.parent.childFile('pubspec.yaml') + ..createSync(recursive: true) + ..writeAsStringSync(''' +flutter: + plugin: + platforms: + windows: + dartPluginClass: SomePlugin + '''); + + createDummyWindowsSolutionFile(); + createDummyPluginWindowsProjectFile(); + + await injectPlugins(flutterProject, checkProjects: true); + + final File registrantImpl = windowsProject.managedDirectory.childFile('generated_plugin_registrant.cc'); + + expect(registrantImpl, exists); + expect(registrantImpl, isNot(contains('SomePlugin'))); + }, overrides: { + FileSystem: () => fs, + ProcessManager: () => FakeProcessManager.any(), + FeatureFlags: () => featureFlags, + }); + testUsingContext('Injecting creates generated Windows plugin properties', () async { when(windowsProject.existsSync()).thenReturn(true); when(featureFlags.isWindowsEnabled).thenReturn(true);