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
remove Any? from maps
  • Loading branch information
@tarrinneal
tarrinneal committed Feb 24, 2023
commit 8d4cba50647dc3a77fd3793e404fb212fafb37b4
2 changes: 1 addition & 1 deletion packages/pigeon/lib/swift_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ String _swiftTypeForBuiltinGenericDartType(TypeDeclaration type) {
if (type.baseName == 'List') {
return '[Any]';
} else if (type.baseName == 'Map') {
return '[AnyHashable: Any?]';
return '[AnyHashable: Any]';
} else {
return 'Any';
}
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 @@ -11,7 +11,7 @@ class MockPrimitiveHostApi: PrimitiveHostApi {
func aBool(value: Bool) -> Bool { value }
func aString(value: String) -> String { value }
func aDouble(value: Double) -> Double { value }
func aMap(value: [AnyHashable: Any?]) -> [AnyHashable: Any?] { value }
func aMap(value: [AnyHashable: Any]) -> [AnyHashable: Any] { value }
func aList(value: [Any]) -> [Any] { value }
func anInt32List(value: FlutterStandardTypedData) -> FlutterStandardTypedData { value }
func aBoolList(value: [Bool?]) -> [Bool?] { value }
Expand Down
8 changes: 4 additions & 4 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 @@ -51,7 +51,7 @@ struct AllTypes {
var a8ByteArray: FlutterStandardTypedData
var aFloatArray: FlutterStandardTypedData
var aList: [Any]
var aMap: [AnyHashable: Any?]
var aMap: [AnyHashable: Any]
var anEnum: AnEnum
var aString: String

Expand All @@ -64,7 +64,7 @@ struct AllTypes {
let a8ByteArray = list[5] as! FlutterStandardTypedData
let aFloatArray = list[6] as! FlutterStandardTypedData
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 aMap = list[8] as! [AnyHashable: Any]
let anEnum = AnEnum(rawValue: list[9] as! Int)!
let aString = list[10] as! String

Expand Down Expand Up @@ -109,7 +109,7 @@ struct AllNullableTypes {
var aNullable8ByteArray: FlutterStandardTypedData? = nil
var aNullableFloatArray: FlutterStandardTypedData? = nil
var aNullableList: [Any]? = nil
var aNullableMap: [AnyHashable: Any?]? = nil
var aNullableMap: [AnyHashable: Any]? = nil
var nullableNestedList: [[Bool?]?]? = nil
var nullableMapWithAnnotations: [String?: String?]? = nil
var nullableMapWithObject: [String?: Any?]? = nil
Expand All @@ -125,7 +125,7 @@ struct AllNullableTypes {
let aNullable8ByteArray = list[5] as! FlutterStandardTypedData?
let aNullableFloatArray = list[6] as! FlutterStandardTypedData?
let aNullableList = list[7] as! [Any]?
let aNullableMap = list[8] as! [AnyHashable: Any?]?
let aNullableMap = list[8] as! [AnyHashable: Any]?
let nullableNestedList = list[9] as! [[Bool?]?]?
let nullableMapWithAnnotations = list[10] as! [String?: String?]?
let nullableMapWithObject = list[11] as! [String?: Any?]?
Expand Down
8 changes: 4 additions & 4 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 @@ -51,7 +51,7 @@ struct AllTypes {
var a8ByteArray: FlutterStandardTypedData
var aFloatArray: FlutterStandardTypedData
var aList: [Any]
var aMap: [AnyHashable: Any?]
var aMap: [AnyHashable: Any]
var anEnum: AnEnum
var aString: String

Expand All @@ -64,7 +64,7 @@ struct AllTypes {
let a8ByteArray = list[5] as! FlutterStandardTypedData
let aFloatArray = list[6] as! FlutterStandardTypedData
let aList = list[7] as! [Any]
let aMap = list[8] as! [AnyHashable: 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 @@ -109,7 +109,7 @@ struct AllNullableTypes {
var aNullable8ByteArray: FlutterStandardTypedData? = nil
var aNullableFloatArray: FlutterStandardTypedData? = nil
var aNullableList: [Any]? = nil
var aNullableMap: [AnyHashable: Any?]? = nil
var aNullableMap: [AnyHashable: Any]? = nil
var nullableNestedList: [[Bool?]?]? = nil
var nullableMapWithAnnotations: [String?: String?]? = nil
var nullableMapWithObject: [String?: Any?]? = nil
Expand All @@ -125,7 +125,7 @@ struct AllNullableTypes {
let aNullable8ByteArray = list[5] as! FlutterStandardTypedData?
let aNullableFloatArray = list[6] as! FlutterStandardTypedData?
let aNullableList = list[7] as! [Any]?
let aNullableMap = list[8] as! [AnyHashable: Any?]?
let aNullableMap = list[8] as! [AnyHashable: Any]?
let nullableNestedList = list[9] as! [[Bool?]?]?
let nullableMapWithAnnotations = list[10] as! [String?: String?]?
let nullableMapWithObject = list[11] as! [String?: Any?]?
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/test/swift_generator_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ void main() {
generator.generate(swiftOptions, root, sink);
final String code = sink.toString();
expect(code, contains('struct Foobar'));
expect(code, contains('var field1: [AnyHashable: Any?]? = nil'));
expect(code, contains('var field1: [AnyHashable: Any]? = nil'));
});

test('gen nested', () {
Expand Down