Skip to content

[pigeon] removes safe casting from nullables in kotlin and swift #3284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 7 commits into from
Feb 28, 2023
Merged

[pigeon] removes safe casting from nullables in kotlin and swift #3284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1f893d7
fails with bool? only
tarrinneal Feb 21, 2023
4331ba1
change most instances of [Any?] to [Any]
tarrinneal Feb 22, 2023
2772e95
kotlin, changelog, generate, format
tarrinneal Feb 23, 2023
a6fe0b3
Merge branch 'main' into unsafe
tarrinneal Feb 23, 2023
1ac0aee
gen
tarrinneal Feb 23, 2023
8d4cba5
remove Any? from maps
tarrinneal Feb 24, 2023
307128f
?
tarrinneal Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
change most instances of [Any?] to [Any]
  • Loading branch information
@tarrinneal
tarrinneal committed Feb 22, 2023
commit 4331ba1c263df971739580e028fd5700455056be
10 changes: 5 additions & 5 deletions packages/pigeon/lib/swift_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ import FlutterMacOS
Set<String> customEnumNames,
) {
final String className = klass.name;
indent.write('static func fromList(_ list: [Any?]) -> $className? ');
indent.write('static func fromList(_ list: [Any]) -> $className? ');

indent.addScoped('{', '}', () {
enumerate(getFieldsInSerializationOrder(klass),
Expand All @@ -185,10 +185,10 @@ import FlutterMacOS
if (!hostDatatype.isBuiltin &&
customClassNames.contains(field.type.baseName)) {
indent.writeln('var ${field.name}: $fieldType? = nil');
indent.write('if let ${field.name}List = $listValue as? [Any?] ');
indent.write('if let ${field.name}List = $listValue as? [Any] ');
indent.addScoped('{', '}', () {
indent.writeln(
'${field.name} = $fieldType.fromList(${field.name}List)');
'${field.name} = $fieldType.fromList(${field.name}List as [Any])');
});
} else if (!hostDatatype.isBuiltin &&
customEnumNames.contains(field.type.baseName)) {
Expand All @@ -205,7 +205,7 @@ import FlutterMacOS
if (!hostDatatype.isBuiltin &&
customClassNames.contains(field.type.baseName)) {
indent.writeln(
'let ${field.name} = $fieldType.fromList($listValue as! [Any?])!');
'let ${field.name} = $fieldType.fromList($listValue as! [Any])!');
} else if (!hostDatatype.isBuiltin &&
customEnumNames.contains(field.type.baseName)) {
indent.writeln(
Expand Down Expand Up @@ -691,7 +691,7 @@ String _flattenTypeArguments(List<TypeDeclaration> args) {
String _swiftTypeForBuiltinGenericDartType(TypeDeclaration type) {
if (type.typeArguments.isEmpty) {
if (type.baseName == 'List') {
return '[Any?]';
return '[Any]';
} else if (type.baseName == 'Map') {
return '[AnyHashable: Any?]';
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/platform_tests/test_plugin/example/ios/RunnerTests/PrimitiveTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class MockPrimitiveHostApi: PrimitiveHostApi {
func aString(value: String) -> String { value }
func aDouble(value: Double) -> Double { value }
func aMap(value: [AnyHashable: Any?]) -> [AnyHashable: Any?] { value }
func aList(value: [Any?]) -> [Any?] { value }
func aList(value: [Any]) -> [Any] { value }
func anInt32List(value: FlutterStandardTypedData) -> FlutterStandardTypedData { value }
func aBoolList(value: [Bool?]) -> [Bool?] { value }
func aStringIntMap(value: [String?: Int32?]) -> [String?: Int32?] { value }
Expand Down
22 changes: 11 additions & 11 deletions packages/pigeon/platform_tests/test_plugin/ios/Classes/CoreTests.gen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,20 @@ struct AllTypes {
var a4ByteArray: FlutterStandardTypedData
var a8ByteArray: FlutterStandardTypedData
var aFloatArray: FlutterStandardTypedData
var aList: [Any?]
var aList: [Any]
var aMap: [AnyHashable: Any?]
var anEnum: AnEnum
var aString: String

static func fromList(_ list: [Any?]) -> AllTypes? {
static func fromList(_ list: [Any]) -> AllTypes? {
let aBool = list[0] as! Bool
let anInt = list[1] as! Int32
let aDouble = list[2] as! Double
let aByteArray = list[3] as! FlutterStandardTypedData
let a4ByteArray = list[4] as! FlutterStandardTypedData
let a8ByteArray = list[5] as! FlutterStandardTypedData
let aFloatArray = list[6] as! FlutterStandardTypedData
let aList = list[7] as! [Any?]
let aList = list[7] as! [Any]
Copy link
Contributor

@hellohuanlin hellohuanlin Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to test out both [Any?] and [Any]

Copy link
Contributor Author

@tarrinneal tarrinneal Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? I'm not sure why we would need Any? for anything in this context since Any can already be nil

Copy link
Contributor

@hellohuanlin hellohuanlin Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting - what about the maps below? Do you also want to remove the ?

e.g.

var aNullableMap: [AnyHashable: Any?]? = nil
var nullableMapWithObject: [String?: Any?]? = nil

Copy link
Contributor Author

@tarrinneal tarrinneal Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a good idea

Copy link
Contributor

@hellohuanlin hellohuanlin Feb 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we would need Any? for anything in this context since Any can already be nil

Actually, Any cannot represent nil in Swift:

Screenshot 2023-02-23 at 4 13 41 PM

Hmmm, if I provide concrete type of nil then it works. I think it's fine to keep Any then. Though I am not sure how this is related to the Bool? crash you had the other day. May be helpful if you can provide a minimal reproducible code snippet so I can try out in playground.

Copy link
Contributor Author

@tarrinneal tarrinneal Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Bool? crash was nil attempting to be cast to Bool?, it failed for the reasons mentioned here. I'm not sure how to replicate it without a lot of extra steps.

let aMap = list[8] as! [AnyHashable: Any?]
let anEnum = AnEnum(rawValue: list[9] as! Int)!
let aString = list[10] as! String
Expand Down Expand Up @@ -108,23 +108,23 @@ struct AllNullableTypes {
var aNullable4ByteArray: FlutterStandardTypedData? = nil
var aNullable8ByteArray: FlutterStandardTypedData? = nil
var aNullableFloatArray: FlutterStandardTypedData? = nil
var aNullableList: [Any?]? = nil
var aNullableList: [Any]? = nil
var aNullableMap: [AnyHashable: Any?]? = nil
var nullableNestedList: [[Bool?]?]? = nil
var nullableMapWithAnnotations: [String?: String?]? = nil
var nullableMapWithObject: [String?: Any?]? = nil
var aNullableEnum: AnEnum? = nil
var aNullableString: String? = nil

static func fromList(_ list: [Any?]) -> AllNullableTypes? {
static func fromList(_ list: [Any]) -> AllNullableTypes? {
let aNullableBool = list[0] as! Bool?
let aNullableInt = list[1] as! Int32?
let aNullableDouble = list[2] as! Double?
let aNullableByteArray = list[3] as! FlutterStandardTypedData?
let aNullable4ByteArray = list[4] as! FlutterStandardTypedData?
let aNullable8ByteArray = list[5] as! FlutterStandardTypedData?
let aNullableFloatArray = list[6] as! FlutterStandardTypedData?
let aNullableList = list[7] as! [Any?]?
let aNullableList = list[7] as! [Any]?
let aNullableMap = list[8] as! [AnyHashable: Any?]?
let nullableNestedList = list[9] as! [[Bool?]?]?
let nullableMapWithAnnotations = list[10] as! [String?: String?]?
Expand Down Expand Up @@ -176,8 +176,8 @@ struct AllNullableTypes {
struct AllNullableTypesWrapper {
var values: AllNullableTypes

static func fromList(_ list: [Any?]) -> AllNullableTypesWrapper? {
let values = AllNullableTypes.fromList(list[0] as! [Any?])!
static func fromList(_ list: [Any]) -> AllNullableTypesWrapper? {
let values = AllNullableTypes.fromList(list[0] as! [Any])!

return AllNullableTypesWrapper(
values: values
Expand All @@ -194,10 +194,10 @@ struct AllNullableTypesWrapper {
///
/// Generated class from Pigeon that represents data sent in messages.
struct TestMessage {
var testList: [Any?]? = nil
var testList: [Any]? = nil

static func fromList(_ list: [Any?]) -> TestMessage? {
let testList = list[0] as! [Any?]?
static func fromList(_ list: [Any]) -> TestMessage? {
let testList = list[0] as! [Any]?

return TestMessage(
testList: testList
Expand Down
22 changes: 11 additions & 11 deletions packages/pigeon/platform_tests/test_plugin/macos/Classes/CoreTests.gen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,20 @@ struct AllTypes {
var a4ByteArray: FlutterStandardTypedData
var a8ByteArray: FlutterStandardTypedData
var aFloatArray: FlutterStandardTypedData
var aList: [Any?]
var aList: [Any]
var aMap: [AnyHashable: Any?]
var anEnum: AnEnum
var aString: String

static func fromList(_ list: [Any?]) -> AllTypes? {
static func fromList(_ list: [Any]) -> AllTypes? {
let aBool = list[0] as! Bool
let anInt = list[1] as! Int32
let aDouble = list[2] as! Double
let aByteArray = list[3] as! FlutterStandardTypedData
let a4ByteArray = list[4] as! FlutterStandardTypedData
let a8ByteArray = list[5] as! FlutterStandardTypedData
let aFloatArray = list[6] as! FlutterStandardTypedData
let aList = list[7] as! [Any?]
let aList = list[7] as! [Any]
let aMap = list[8] as! [AnyHashable: Any?]
let anEnum = AnEnum(rawValue: list[9] as! Int)!
let aString = list[10] as! String
Expand Down Expand Up @@ -108,23 +108,23 @@ struct AllNullableTypes {
var aNullable4ByteArray: FlutterStandardTypedData? = nil
var aNullable8ByteArray: FlutterStandardTypedData? = nil
var aNullableFloatArray: FlutterStandardTypedData? = nil
var aNullableList: [Any?]? = nil
var aNullableList: [Any]? = nil
var aNullableMap: [AnyHashable: Any?]? = nil
var nullableNestedList: [[Bool?]?]? = nil
var nullableMapWithAnnotations: [String?: String?]? = nil
var nullableMapWithObject: [String?: Any?]? = nil
var aNullableEnum: AnEnum? = nil
var aNullableString: String? = nil

static func fromList(_ list: [Any?]) -> AllNullableTypes? {
static func fromList(_ list: [Any]) -> AllNullableTypes? {
let aNullableBool = list[0] as! Bool?
let aNullableInt = list[1] as! Int32?
let aNullableDouble = list[2] as! Double?
let aNullableByteArray = list[3] as! FlutterStandardTypedData?
let aNullable4ByteArray = list[4] as! FlutterStandardTypedData?
let aNullable8ByteArray = list[5] as! FlutterStandardTypedData?
let aNullableFloatArray = list[6] as! FlutterStandardTypedData?
let aNullableList = list[7] as! [Any?]?
let aNullableList = list[7] as! [Any]?
let aNullableMap = list[8] as! [AnyHashable: Any?]?
let nullableNestedList = list[9] as! [[Bool?]?]?
let nullableMapWithAnnotations = list[10] as! [String?: String?]?
Expand Down Expand Up @@ -176,8 +176,8 @@ struct AllNullableTypes {
struct AllNullableTypesWrapper {
var values: AllNullableTypes

static func fromList(_ list: [Any?]) -> AllNullableTypesWrapper? {
let values = AllNullableTypes.fromList(list[0] as! [Any?])!
static func fromList(_ list: [Any]) -> AllNullableTypesWrapper? {
let values = AllNullableTypes.fromList(list[0] as! [Any])!

return AllNullableTypesWrapper(
values: values
Expand All @@ -194,10 +194,10 @@ struct AllNullableTypesWrapper {
///
/// Generated class from Pigeon that represents data sent in messages.
struct TestMessage {
var testList: [Any?]? = nil
var testList: [Any]? = nil

static func fromList(_ list: [Any?]) -> TestMessage? {
let testList = list[0] as! [Any?]?
static func fromList(_ list: [Any]) -> TestMessage? {
let testList = list[0] as! [Any]?

return TestMessage(
testList: testList
Expand Down
8 changes: 4 additions & 4 deletions packages/pigeon/test/swift_generator_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void main() {
final String code = sink.toString();
expect(code, contains('struct Foobar'));
expect(code, contains('var field1: Int32? = nil'));
expect(code, contains('static func fromList(_ list: [Any?]) -> Foobar?'));
expect(code, contains('static func fromList(_ list: [Any]) -> Foobar?'));
expect(code, contains('func toList() -> [Any?]'));
});

Expand Down Expand Up @@ -392,7 +392,7 @@ void main() {
generator.generate(swiftOptions, root, sink);
final String code = sink.toString();
expect(code, contains('struct Foobar'));
expect(code, contains('var field1: [Any?]? = nil'));
expect(code, contains('var field1: [Any]? = nil'));
});

test('gen map', () {
Expand Down Expand Up @@ -451,8 +451,8 @@ void main() {
expect(code, contains('struct Outer'));
expect(code, contains('struct Nested'));
expect(code, contains('var nested: Nested? = nil'));
expect(code, contains('static func fromList(_ list: [Any?]) -> Outer?'));
expect(code, contains('nested = Nested.fromList(nestedList)'));
expect(code, contains('static func fromList(_ list: [Any]) -> Outer?'));
expect(code, contains('nested = Nested.fromList(nestedList as [Any])'));
expect(code, contains('func toList() -> [Any?]'));
});

Expand Down