From ca772e15261484d02d50ae354ff61ce2610ab958 Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Sun, 22 Dec 2024 02:58:35 -0500 Subject: [PATCH 1/5] add import fix --- packages/pigeon/lib/swift_generator.dart | 46 ++++++-- .../ios/Classes/ProxyApiTests.gen.swift | 4 + .../macos/Classes/ProxyApiTests.gen.swift | 4 + .../pigeon/test/swift/proxy_api_test.dart | 102 ++++++++++++++++++ 4 files changed, 148 insertions(+), 8 deletions(-) diff --git a/packages/pigeon/lib/swift_generator.dart b/packages/pigeon/lib/swift_generator.dart index 4962774c9a04..0f370d13894e 100644 --- a/packages/pigeon/lib/swift_generator.dart +++ b/packages/pigeon/lib/swift_generator.dart @@ -154,14 +154,7 @@ class SwiftGenerator extends StructuredGenerator { }) { indent.writeln('import Foundation'); - final Iterable proxyApiImports = root.apis - .whereType() - .map((AstProxyApi proxyApi) => proxyApi.swiftOptions?.import) - .nonNulls - .toSet(); - for (final String import in proxyApiImports) { - indent.writeln('import $import'); - } + _writeProxyApiImports(indent, root.apis.whereType()); indent.newln(); indent.format(''' @@ -1155,6 +1148,9 @@ if (wrapped == nil) { final String swiftApiProtocolName = '${hostProxyApiPrefix}Protocol${api.name}'; indent.writeScoped('protocol $swiftApiProtocolName {', '}', () { + indent.writeln( + 'var pigeonRegistrar: ${proxyApiRegistrarName(generatorOptions)} { get }', + ); _writeProxyApiFlutterMethods( indent, api, @@ -2487,6 +2483,40 @@ private func nilOrValue(_ value: Any?) -> T? { }); }); } + + void _writeProxyApiImports(Indent indent, Iterable apis) { + final Map> apisOfImports = + >{}; + for (final AstProxyApi proxyApi in apis) { + final String? import = proxyApi.swiftOptions?.import; + if (import != null) { + if (apisOfImports.containsKey(import)) { + apisOfImports[import]!.add(proxyApi); + } else { + apisOfImports[import] = [proxyApi]; + } + } + } + + for (final String import in apisOfImports.keys) { + final List unsupportedPlatforms = [ + if (!apisOfImports[import]! + .every((AstProxyApi api) => api.swiftOptions?.supportsIos ?? true)) + '!os(iOS)', + if (!apisOfImports[import]!.every( + (AstProxyApi api) => api.swiftOptions?.supportsMacos ?? true)) + '!os(macOS)', + ]; + + if (unsupportedPlatforms.isNotEmpty) { + indent.writeln('#if ${unsupportedPlatforms.join(' || ')}'); + } + indent.writeln('import $import'); + if (unsupportedPlatforms.isNotEmpty) { + indent.writeln('#endif'); + } + } + } } typedef _VersionRequirement = ({TypeDeclaration type, Version version}); diff --git a/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift b/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift index 506565c1dd67..966bb3b3ff1b 100644 --- a/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift +++ b/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift @@ -967,6 +967,7 @@ protocol PigeonApiDelegateProxyApiTestClass { } protocol PigeonApiProtocolProxyApiTestClass { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } /// A no-op function taking no arguments and returning no value, to sanity /// test basic calling. func flutterNoop( @@ -3895,6 +3896,7 @@ protocol PigeonApiDelegateProxyApiSuperClass { } protocol PigeonApiProtocolProxyApiSuperClass { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiProxyApiSuperClass: PigeonApiProtocolProxyApiSuperClass { @@ -4000,6 +4002,7 @@ open class PigeonApiDelegateProxyApiInterface { } protocol PigeonApiProtocolProxyApiInterface { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } func anInterfaceMethod( pigeonInstance pigeonInstanceArg: ProxyApiInterface, completion: @escaping (Result) -> Void) @@ -4100,6 +4103,7 @@ protocol PigeonApiDelegateClassWithApiRequirement { } protocol PigeonApiProtocolClassWithApiRequirement { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiClassWithApiRequirement: PigeonApiProtocolClassWithApiRequirement { diff --git a/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift b/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift index 506565c1dd67..966bb3b3ff1b 100644 --- a/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift +++ b/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift @@ -967,6 +967,7 @@ protocol PigeonApiDelegateProxyApiTestClass { } protocol PigeonApiProtocolProxyApiTestClass { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } /// A no-op function taking no arguments and returning no value, to sanity /// test basic calling. func flutterNoop( @@ -3895,6 +3896,7 @@ protocol PigeonApiDelegateProxyApiSuperClass { } protocol PigeonApiProtocolProxyApiSuperClass { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiProxyApiSuperClass: PigeonApiProtocolProxyApiSuperClass { @@ -4000,6 +4002,7 @@ open class PigeonApiDelegateProxyApiInterface { } protocol PigeonApiProtocolProxyApiInterface { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } func anInterfaceMethod( pigeonInstance pigeonInstanceArg: ProxyApiInterface, completion: @escaping (Result) -> Void) @@ -4100,6 +4103,7 @@ protocol PigeonApiDelegateClassWithApiRequirement { } protocol PigeonApiProtocolClassWithApiRequirement { + var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiClassWithApiRequirement: PigeonApiProtocolClassWithApiRequirement { diff --git a/packages/pigeon/test/swift/proxy_api_test.dart b/packages/pigeon/test/swift/proxy_api_test.dart index 3c9cb12c7562..3260a5dd2bbe 100644 --- a/packages/pigeon/test/swift/proxy_api_test.dart +++ b/packages/pigeon/test/swift/proxy_api_test.dart @@ -135,6 +135,12 @@ void main() { // Delegate and class expect(code, contains('protocol PigeonApiDelegateApi')); expect(code, contains('protocol PigeonApiProtocolApi')); + expect( + code, + contains( + 'var pigeonRegistrar: MyFilePigeonProxyApiRegistrar { get }', + ), + ); expect( code, contains( @@ -187,6 +193,102 @@ void main() { ); }); + group('imports', () { + test('add check if every class does not support iOS', () { + final Root root = Root( + apis: [ + AstProxyApi( + name: 'Api', + swiftOptions: const SwiftProxyApiOptions( + import: 'MyImport', + supportsIos: false, + ), + constructors: [], + fields: [], + methods: [], + ), + ], + classes: [], + enums: [], + ); + final StringBuffer sink = StringBuffer(); + const SwiftGenerator generator = SwiftGenerator(); + generator.generate( + const SwiftOptions(), + root, + sink, + dartPackageName: DEFAULT_PACKAGE_NAME, + ); + final String code = sink.toString(); + + expect(code, contains('#if !os(iOS)\nimport MyImport\n#endif')); + }); + + test('add check if every class does not support macOS', () { + final Root root = Root( + apis: [ + AstProxyApi( + name: 'Api', + swiftOptions: const SwiftProxyApiOptions( + import: 'MyImport', + supportsMacos: false, + ), + constructors: [], + fields: [], + methods: [], + ), + ], + classes: [], + enums: [], + ); + final StringBuffer sink = StringBuffer(); + const SwiftGenerator generator = SwiftGenerator(); + generator.generate( + const SwiftOptions(), + root, + sink, + dartPackageName: DEFAULT_PACKAGE_NAME, + ); + final String code = sink.toString(); + + expect(code, contains('#if !os(macOS)\nimport MyImport\n#endif')); + }); + + test('add check if for multiple unsupported platforms', () { + final Root root = Root( + apis: [ + AstProxyApi( + name: 'Api', + swiftOptions: const SwiftProxyApiOptions( + import: 'MyImport', + supportsIos: false, + supportsMacos: false, + ), + constructors: [], + fields: [], + methods: [], + ), + ], + classes: [], + enums: [], + ); + final StringBuffer sink = StringBuffer(); + const SwiftGenerator generator = SwiftGenerator(); + generator.generate( + const SwiftOptions(), + root, + sink, + dartPackageName: DEFAULT_PACKAGE_NAME, + ); + final String code = sink.toString(); + + expect( + code, + contains('#if !os(iOS) || !os(macOS)\nimport MyImport\n#endif'), + ); + }); + }); + group('inheritance', () { test('extends', () { final AstProxyApi api2 = AstProxyApi( From 344a3abf24117212befc6013fd18a5254c0d3bab Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Sun, 22 Dec 2024 03:03:50 -0500 Subject: [PATCH 2/5] version bump --- packages/pigeon/CHANGELOG.md | 3 ++- packages/pigeon/lib/generator_tools.dart | 2 +- packages/pigeon/pubspec.yaml | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md index e8226fe98562..71b33fa74417 100644 --- a/packages/pigeon/CHANGELOG.md +++ b/packages/pigeon/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 22.7.1 +* [swift] Adds support for platform checks of imports of ProxyApis. * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. ## 22.7.0 diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart index e5a943bdcb96..c23817ad6174 100644 --- a/packages/pigeon/lib/generator_tools.dart +++ b/packages/pigeon/lib/generator_tools.dart @@ -14,7 +14,7 @@ import 'ast.dart'; /// The current version of pigeon. /// /// This must match the version in pubspec.yaml. -const String pigeonVersion = '22.7.0'; +const String pigeonVersion = '22.7.1'; /// Read all the content from [stdin] to a String. String readStdin() { diff --git a/packages/pigeon/pubspec.yaml b/packages/pigeon/pubspec.yaml index d525aba2f223..6e5378a26242 100644 --- a/packages/pigeon/pubspec.yaml +++ b/packages/pigeon/pubspec.yaml @@ -2,7 +2,7 @@ name: pigeon description: Code generator tool to make communication between Flutter and the host platform type-safe and easier. repository: https://github.com/flutter/packages/tree/main/packages/pigeon issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+pigeon%22 -version: 22.7.0 # This must match the version in lib/generator_tools.dart +version: 22.7.1 # This must match the version in lib/generator_tools.dart environment: sdk: ^3.4.0 From 3f0aabbac2038ec70dd2d46e12e181b9912999ff Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Sun, 22 Dec 2024 03:08:42 -0500 Subject: [PATCH 3/5] fix logic --- packages/pigeon/lib/swift_generator.dart | 4 +- .../pigeon/test/swift/proxy_api_test.dart | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/pigeon/lib/swift_generator.dart b/packages/pigeon/lib/swift_generator.dart index 0f370d13894e..bcc1ebc88897 100644 --- a/packages/pigeon/lib/swift_generator.dart +++ b/packages/pigeon/lib/swift_generator.dart @@ -2501,9 +2501,9 @@ private func nilOrValue(_ value: Any?) -> T? { for (final String import in apisOfImports.keys) { final List unsupportedPlatforms = [ if (!apisOfImports[import]! - .every((AstProxyApi api) => api.swiftOptions?.supportsIos ?? true)) + .any((AstProxyApi api) => api.swiftOptions?.supportsIos ?? true)) '!os(iOS)', - if (!apisOfImports[import]!.every( + if (!apisOfImports[import]!.any( (AstProxyApi api) => api.swiftOptions?.supportsMacos ?? true)) '!os(macOS)', ]; diff --git a/packages/pigeon/test/swift/proxy_api_test.dart b/packages/pigeon/test/swift/proxy_api_test.dart index 3260a5dd2bbe..ebe6514be8de 100644 --- a/packages/pigeon/test/swift/proxy_api_test.dart +++ b/packages/pigeon/test/swift/proxy_api_test.dart @@ -287,6 +287,45 @@ void main() { contains('#if !os(iOS) || !os(macOS)\nimport MyImport\n#endif'), ); }); + + test('do not add check if at least one class is supported', () { + final Root root = Root( + apis: [ + AstProxyApi( + name: 'Api', + swiftOptions: const SwiftProxyApiOptions( + import: 'MyImport', + supportsIos: false, + ), + constructors: [], + fields: [], + methods: [], + ), + AstProxyApi( + name: 'Api2', + swiftOptions: const SwiftProxyApiOptions( + import: 'MyImport', + ), + constructors: [], + fields: [], + methods: [], + ), + ], + classes: [], + enums: [], + ); + final StringBuffer sink = StringBuffer(); + const SwiftGenerator generator = SwiftGenerator(); + generator.generate( + const SwiftOptions(), + root, + sink, + dartPackageName: DEFAULT_PACKAGE_NAME, + ); + final String code = sink.toString(); + + expect(code, isNot(contains('#if !os(iOS)\nimport MyImport'))); + }); }); group('inheritance', () { From 99262ea23f2955b69b137a867c2899ec17f0aaba Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Sun, 22 Dec 2024 16:26:14 -0500 Subject: [PATCH 4/5] formatting and remove registrar breaking change --- packages/pigeon/lib/swift_generator.dart | 7 ++----- .../test_plugin/ios/Classes/ProxyApiTests.gen.swift | 4 ---- .../test_plugin/macos/Classes/ProxyApiTests.gen.swift | 4 ---- packages/pigeon/test/swift/proxy_api_test.dart | 6 ------ 4 files changed, 2 insertions(+), 19 deletions(-) diff --git a/packages/pigeon/lib/swift_generator.dart b/packages/pigeon/lib/swift_generator.dart index bcc1ebc88897..5877c0884cf6 100644 --- a/packages/pigeon/lib/swift_generator.dart +++ b/packages/pigeon/lib/swift_generator.dart @@ -1148,9 +1148,6 @@ if (wrapped == nil) { final String swiftApiProtocolName = '${hostProxyApiPrefix}Protocol${api.name}'; indent.writeScoped('protocol $swiftApiProtocolName {', '}', () { - indent.writeln( - 'var pigeonRegistrar: ${proxyApiRegistrarName(generatorOptions)} { get }', - ); _writeProxyApiFlutterMethods( indent, api, @@ -2503,8 +2500,8 @@ private func nilOrValue(_ value: Any?) -> T? { if (!apisOfImports[import]! .any((AstProxyApi api) => api.swiftOptions?.supportsIos ?? true)) '!os(iOS)', - if (!apisOfImports[import]!.any( - (AstProxyApi api) => api.swiftOptions?.supportsMacos ?? true)) + if (!apisOfImports[import]! + .any((AstProxyApi api) => api.swiftOptions?.supportsMacos ?? true)) '!os(macOS)', ]; diff --git a/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift b/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift index 966bb3b3ff1b..506565c1dd67 100644 --- a/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift +++ b/packages/pigeon/platform_tests/test_plugin/ios/Classes/ProxyApiTests.gen.swift @@ -967,7 +967,6 @@ protocol PigeonApiDelegateProxyApiTestClass { } protocol PigeonApiProtocolProxyApiTestClass { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } /// A no-op function taking no arguments and returning no value, to sanity /// test basic calling. func flutterNoop( @@ -3896,7 +3895,6 @@ protocol PigeonApiDelegateProxyApiSuperClass { } protocol PigeonApiProtocolProxyApiSuperClass { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiProxyApiSuperClass: PigeonApiProtocolProxyApiSuperClass { @@ -4002,7 +4000,6 @@ open class PigeonApiDelegateProxyApiInterface { } protocol PigeonApiProtocolProxyApiInterface { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } func anInterfaceMethod( pigeonInstance pigeonInstanceArg: ProxyApiInterface, completion: @escaping (Result) -> Void) @@ -4103,7 +4100,6 @@ protocol PigeonApiDelegateClassWithApiRequirement { } protocol PigeonApiProtocolClassWithApiRequirement { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiClassWithApiRequirement: PigeonApiProtocolClassWithApiRequirement { diff --git a/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift b/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift index 966bb3b3ff1b..506565c1dd67 100644 --- a/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift +++ b/packages/pigeon/platform_tests/test_plugin/macos/Classes/ProxyApiTests.gen.swift @@ -967,7 +967,6 @@ protocol PigeonApiDelegateProxyApiTestClass { } protocol PigeonApiProtocolProxyApiTestClass { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } /// A no-op function taking no arguments and returning no value, to sanity /// test basic calling. func flutterNoop( @@ -3896,7 +3895,6 @@ protocol PigeonApiDelegateProxyApiSuperClass { } protocol PigeonApiProtocolProxyApiSuperClass { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiProxyApiSuperClass: PigeonApiProtocolProxyApiSuperClass { @@ -4002,7 +4000,6 @@ open class PigeonApiDelegateProxyApiInterface { } protocol PigeonApiProtocolProxyApiInterface { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } func anInterfaceMethod( pigeonInstance pigeonInstanceArg: ProxyApiInterface, completion: @escaping (Result) -> Void) @@ -4103,7 +4100,6 @@ protocol PigeonApiDelegateClassWithApiRequirement { } protocol PigeonApiProtocolClassWithApiRequirement { - var pigeonRegistrar: ProxyApiTestsPigeonProxyApiRegistrar { get } } final class PigeonApiClassWithApiRequirement: PigeonApiProtocolClassWithApiRequirement { diff --git a/packages/pigeon/test/swift/proxy_api_test.dart b/packages/pigeon/test/swift/proxy_api_test.dart index ebe6514be8de..d149d5d01024 100644 --- a/packages/pigeon/test/swift/proxy_api_test.dart +++ b/packages/pigeon/test/swift/proxy_api_test.dart @@ -135,12 +135,6 @@ void main() { // Delegate and class expect(code, contains('protocol PigeonApiDelegateApi')); expect(code, contains('protocol PigeonApiProtocolApi')); - expect( - code, - contains( - 'var pigeonRegistrar: MyFilePigeonProxyApiRegistrar { get }', - ), - ); expect( code, contains( From 1976969a873b0c7ffc5c47d88b0fa4f7b8729c3f Mon Sep 17 00:00:00 2001 From: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com> Date: Sun, 22 Dec 2024 16:53:37 -0500 Subject: [PATCH 5/5] add comment --- packages/pigeon/lib/swift_generator.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/pigeon/lib/swift_generator.dart b/packages/pigeon/lib/swift_generator.dart index 5877c0884cf6..d5c09d4ec170 100644 --- a/packages/pigeon/lib/swift_generator.dart +++ b/packages/pigeon/lib/swift_generator.dart @@ -2496,6 +2496,8 @@ private func nilOrValue(_ value: Any?) -> T? { } for (final String import in apisOfImports.keys) { + // If every ProxyApi that shares an import excludes a platform for + // support, surround the import with `#if !os(...) #endif`. final List unsupportedPlatforms = [ if (!apisOfImports[import]! .any((AstProxyApi api) => api.swiftOptions?.supportsIos ?? true))