Skip to content

Commit 7b4a543

Browse files
authored
[pigeon] fix swift nsnull casting crash (#3545)
fixes flutter/flutter#123387 by forcing NSNull to nil also simplifies Int casting, since it was overly complex ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use `dart format`.) - [x] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes]. - [x] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style]. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat [pub versioning philosophy]: https://dart.dev/tools/pub/versioning [exempt from version changes]: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates [following repository CHANGELOG style]: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
1 parent 3df3ba5 commit 7b4a543

File tree

25 files changed

+289
-218
lines changed

25 files changed

+289
-218
lines changed

packages/pigeon/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 9.2.1
2+
3+
* [swift] Fixes NSNull casting crash.
4+
15
## 9.2.0
26

37
* [cpp] Removes experimental tags.

packages/pigeon/lib/generator_tools.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import 'ast.dart';
1111
/// The current version of pigeon.
1212
///
1313
/// This must match the version in pubspec.yaml.
14-
const String pigeonVersion = '9.2.0';
14+
const String pigeonVersion = '9.2.1';
1515

1616
/// Read all the content from [stdin] to a String.
1717
String readStdin() {

packages/pigeon/lib/swift_generator.dart

Lines changed: 107 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,7 @@ import Flutter
7575
import FlutterMacOS
7676
#else
7777
#error("Unsupported platform.")
78-
#endif
79-
''');
80-
indent.newln();
78+
#endif''');
8179
}
8280

8381
@override
@@ -176,44 +174,17 @@ import FlutterMacOS
176174
indent.addScoped('{', '}', () {
177175
enumerate(getFieldsInSerializationOrder(klass),
178176
(int index, final NamedType field) {
179-
final HostDatatype hostDatatype = _getHostDatatype(root, field);
180-
181177
final String listValue = 'list[$index]';
182-
final String fieldType = _swiftTypeForDartType(field.type);
183178

184-
if (field.type.isNullable) {
185-
if (!hostDatatype.isBuiltin &&
186-
customClassNames.contains(field.type.baseName)) {
187-
indent.writeln('var ${field.name}: $fieldType? = nil');
188-
indent.write('if let ${field.name}List = $listValue as! [Any]? ');
189-
indent.addScoped('{', '}', () {
190-
indent.writeln(
191-
'${field.name} = $fieldType.fromList(${field.name}List as [Any])');
192-
});
193-
} else if (!hostDatatype.isBuiltin &&
194-
customEnumNames.contains(field.type.baseName)) {
195-
indent.writeln('var ${field.name}: $fieldType? = nil');
196-
indent.write('if let ${field.name}RawValue = $listValue as! Int? ');
197-
indent.addScoped('{', '}', () {
198-
indent.writeln(
199-
'${field.name} = $fieldType(rawValue: ${field.name}RawValue)');
200-
});
201-
} else {
202-
indent.writeln('let ${field.name} = $listValue as! $fieldType? ');
203-
}
204-
} else {
205-
if (!hostDatatype.isBuiltin &&
206-
customClassNames.contains(field.type.baseName)) {
207-
indent.writeln(
208-
'let ${field.name} = $fieldType.fromList($listValue as! [Any])!');
209-
} else if (!hostDatatype.isBuiltin &&
210-
customEnumNames.contains(field.type.baseName)) {
211-
indent.writeln(
212-
'let ${field.name} = $fieldType(rawValue: $listValue as! Int)!');
213-
} else {
214-
indent.writeln('let ${field.name} = $listValue as! $fieldType');
215-
}
216-
}
179+
_writeDecodeCasting(
180+
root: root,
181+
indent: indent,
182+
value: listValue,
183+
variableName: field.name,
184+
type: field.type,
185+
listEncodedClassNames: customClassNames,
186+
listEncodedEnumNames: customEnumNames,
187+
);
217188
});
218189

219190
indent.newln();
@@ -345,8 +316,13 @@ import FlutterMacOS
345316
});
346317
} else {
347318
indent.addScoped('{ response in', '}', () {
348-
indent.writeln(
349-
'let result = ${_castForceUnwrap("response", func.returnType, root)}');
319+
_writeDecodeCasting(
320+
root: root,
321+
indent: indent,
322+
value: 'response',
323+
variableName: 'result',
324+
type: func.returnType,
325+
);
350326
indent.writeln('completion(result)');
351327
});
352328
}
@@ -461,9 +437,13 @@ import FlutterMacOS
461437
final String argName =
462438
_getSafeArgumentName(index, arg.namedType);
463439
final String argIndex = 'args[$index]';
464-
indent.writeln(
465-
'let $argName = ${_castForceUnwrap(argIndex, arg.type, root)}');
466-
440+
_writeDecodeCasting(
441+
root: root,
442+
indent: indent,
443+
value: argIndex,
444+
variableName: argName,
445+
type: arg.type,
446+
);
467447
if (arg.label == '_') {
468448
methodArgument.add(argName);
469449
} else {
@@ -607,6 +587,79 @@ import FlutterMacOS
607587
indent.newln();
608588
}
609589

590+
/// Writes decode and casting code for any type.
591+
///
592+
/// Optional parameters should only be used for class decoding.
593+
void _writeDecodeCasting({
594+
required Root root,
595+
required Indent indent,
596+
required String value,
597+
required String variableName,
598+
required TypeDeclaration type,
599+
Set<String>? listEncodedClassNames,
600+
Set<String>? listEncodedEnumNames,
601+
}) {
602+
String castForceUnwrap(String value, TypeDeclaration type, Root root) {
603+
if (isEnum(root, type)) {
604+
assert(!type.isNullable,
605+
'nullable enums require special code that this helper does not supply');
606+
return '${_swiftTypeForDartType(type)}(rawValue: $value as! Int)!';
607+
} else if (type.baseName == 'Object') {
608+
// Special-cased to avoid warnings about using 'as' with Any.
609+
return value;
610+
} else if (type.baseName == 'int') {
611+
if (type.isNullable) {
612+
// Nullable ints need to check for NSNull, and Int32 before casting can be done safely.
613+
// This nested ternary is a necessary evil to avoid less efficient conversions.
614+
return '$value is NSNull ? nil : ($value is Int64? ? $value as! Int64? : Int64($value as! Int32))';
615+
} else {
616+
return '$value is Int64 ? $value as! Int64 : Int64($value as! Int32)';
617+
}
618+
} else if (type.isNullable) {
619+
return 'nilOrValue($value)';
620+
} else {
621+
return '$value as! ${_swiftTypeForDartType(type)}';
622+
}
623+
}
624+
625+
final String fieldType = _swiftTypeForDartType(type);
626+
627+
if (type.isNullable) {
628+
if (listEncodedClassNames != null &&
629+
listEncodedClassNames.contains(type.baseName)) {
630+
indent.writeln('var $variableName: $fieldType? = nil');
631+
indent.write('if let ${variableName}List = $value as! [Any]? ');
632+
indent.addScoped('{', '}', () {
633+
indent.writeln(
634+
'$variableName = $fieldType.fromList(${variableName}List)');
635+
});
636+
} else if (listEncodedEnumNames != null &&
637+
listEncodedEnumNames.contains(type.baseName)) {
638+
indent.writeln('var $variableName: $fieldType? = nil');
639+
indent.writeln(
640+
'let ${variableName}EnumVal: Int? = ${castForceUnwrap(value, const TypeDeclaration(baseName: 'Int', isNullable: true), root)}');
641+
indent
642+
.write('if let ${variableName}RawValue = ${variableName}EnumVal ');
643+
indent.addScoped('{', '}', () {
644+
indent.writeln(
645+
'$variableName = $fieldType(rawValue: ${variableName}RawValue)!');
646+
});
647+
} else {
648+
indent.writeln(
649+
'let $variableName: $fieldType? = ${castForceUnwrap(value, type, root)}');
650+
}
651+
} else {
652+
if (listEncodedClassNames != null &&
653+
listEncodedClassNames.contains(type.baseName)) {
654+
indent.writeln(
655+
'let $variableName = $fieldType.fromList($value as! [Any])!');
656+
} else {
657+
indent.writeln(
658+
'let $variableName = ${castForceUnwrap(value, type, root)}');
659+
}
660+
}
661+
}
662+
610663
void _writeWrapResult(Indent indent) {
611664
indent.newln();
612665
indent.write('private func wrapResult(_ result: Any?) -> [Any?] ');
@@ -637,11 +690,21 @@ import FlutterMacOS
637690
});
638691
}
639692

693+
void _writeNilOrValue(Indent indent) {
694+
indent.format('''
695+
696+
private func nilOrValue<T>(_ value: Any?) -> T? {
697+
if value is NSNull { return nil }
698+
return (value as Any) as! T?
699+
}''');
700+
}
701+
640702
@override
641703
void writeGeneralUtilities(
642704
SwiftOptions generatorOptions, Root root, Indent indent) {
643705
_writeWrapResult(indent);
644706
_writeWrapError(indent);
707+
_writeNilOrValue(indent);
645708
}
646709
}
647710

@@ -667,23 +730,6 @@ String _camelCase(String text) {
667730
return pascal[0].toLowerCase() + pascal.substring(1);
668731
}
669732

670-
String _castForceUnwrap(String value, TypeDeclaration type, Root root) {
671-
final String forceUnwrap = type.isNullable ? '' : '!';
672-
final String castUnwrap = type.isNullable ? '?' : '';
673-
if (isEnum(root, type)) {
674-
final String nullableConditionPrefix =
675-
type.isNullable ? '$value == nil ? nil : ' : '';
676-
return '$nullableConditionPrefix${_swiftTypeForDartType(type)}(rawValue: $value as! Int)$forceUnwrap';
677-
} else if (type.baseName == 'Object') {
678-
// Special-cased to avoid warnings about using 'as' with Any.
679-
return value;
680-
} else if (type.baseName == 'int') {
681-
return '($value is Int) ? Int64($value as! Int) : $value as! Int64$castUnwrap';
682-
} else {
683-
return '$value as! ${_swiftTypeForDartType(type)}$castUnwrap';
684-
}
685-
}
686-
687733
/// Converts a [List] of [TypeDeclaration]s to a comma separated [String] to be
688734
/// used in Swift code.
689735
String _flattenTypeArguments(List<TypeDeclaration> args) {

packages/pigeon/mock_handler_tester/test/message.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/mock_handler_tester/test/test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, unnecessary_import
88
// ignore_for_file: avoid_relative_lib_imports

packages/pigeon/platform_tests/alternate_language_test_plugin/android/src/main/java/com/example/alternate_language_test_plugin/CoreTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77

88
package com.example.alternate_language_test_plugin;

packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/CoreTests.gen.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77

88
#import <Foundation/Foundation.h>

packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/CoreTests.gen.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77

88
#import "CoreTests.gen.h"

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/core_tests.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/flutter_unittests.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/multiple_arity.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/non_null_fields.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/null_fields.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/nullable_returns.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/flutter_null_safe_unit_tests/lib/primitive.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/shared_test_plugin_code/lib/integration_tests.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,6 +1255,15 @@ void runPigeonIntegrationTests(TargetGenerator targetGenerator) {
12551255
expect(echoObject, sentObject);
12561256
});
12571257

1258+
testWidgets('nullable big ints serialize and deserialize correctly',
1259+
(WidgetTester _) async {
1260+
final HostIntegrationCoreApi api = HostIntegrationCoreApi();
1261+
1262+
const int sentObject = _biggerThanBigInt;
1263+
final int? echoObject = await api.callFlutterEchoNullableInt(sentObject);
1264+
expect(echoObject, sentObject);
1265+
});
1266+
12581267
testWidgets('null ints serialize and deserialize correctly',
12591268
(WidgetTester _) async {
12601269
final HostIntegrationCoreApi api = HostIntegrationCoreApi();

packages/pigeon/platform_tests/shared_test_plugin_code/lib/src/generated/core_tests.gen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import
88

packages/pigeon/platform_tests/test_plugin/android/src/main/kotlin/com/example/test_plugin/CoreTests.gen.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44
//
5-
// Autogenerated from Pigeon (v9.2.0), do not edit directly.
5+
// Autogenerated from Pigeon (v9.2.1), do not edit directly.
66
// See also: https://pub.dev/packages/pigeon
77

88
package com.example.test_plugin

0 commit comments

Comments
 (0)