Skip to content

[pigeon] re-enable "treat warnings as errors" #126006

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.

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

Closed
2 tasks done
hellohuanlin opened this issue May 3, 2023 · 3 comments · Fixed by flutter/packages#3901
Closed
2 tasks done

[pigeon] re-enable "treat warnings as errors" #126006

hellohuanlin opened this issue May 3, 2023 · 3 comments · Fixed by flutter/packages#3901
Labels
a: plugins Support for writing, building, and running plugin packages p: pigeon related to pigeon messaging codegen tool package flutter/packages repository. See also p: labels. platform-ios iOS applications specifically

Comments

@hellohuanlin
Copy link
Contributor

hellohuanlin commented May 3, 2023

Is there an existing issue for this?

Use case

We enabled "treat warnings as errors" for Swift pigeon's unit test in this PR, which got reverted, because our recent migration to Xcode 14.3 that introduced a new warning about type inference for "Any collection".

Background

The original diagnosis was introduced in swiftlang/swift@1d21b4e, which checks against type inference for heterogeneous collection:

let foo = ["Hello", 999] // (1)
// Error: Heterogeneous collection literal could only be inferred to '[Any]'; add explicit type annotation if this is intentional. 

let bar = [nil] // (2)
// Error: Heterogeneous collection literal could only be inferred to '[Any?]'; add explicit type annotation if this is intentional. 

The fix is simply to add type annotation:

let foo: [Any] = ["Hello", 999] // (1)
let bar: [Any?] = [nil] // (2)

This is reasonable diagnosis, since the heterogenous collection loses the concrete type info for the elements, so we need to explicitly indicate this is indeed what we want.

Though for (2), it could use better error message - it's not exactly a "heterogeneous" collection. But anyways, this diagnosis is basically saying, if a collection can only be inferred to [Any] or [Any?] (either heterogeneous collection or [nil]), you have to explicitly tell compiler this is indeed what you want.

Changes in Xcode 14.3 / Swift 5.8

Before Swift 5.8, when passing a literal into a function, the type annotation requirement is considered as satisfied:

func takeFoo(_ foo: [Any?]) {
  print(foo)
}
takeFoo(["Hello", 999])
takeFoo([nil])

This was fine, because the argument explicitly indicate that we want [Any?] type, hence no warnings or errors. However, in Swift 5.8, the above will trigger the same diagnosis (tho as a warning not an error).

NullableReturnsTests.swift:43:53: error: heterogeneous collection literal could only be inferred to '[Any?]'; add explicit type annotation if this is intentional
    let inputEncoded = binaryMessenger.codec.encode([nil])
                                                    ^~~~~
                                                          as [Any?]

This change seems to be introduced by mistake, and already reverted in main branch and will be in Swift 5.9, so possibly Xcode 14.3.1 or 14.4.

Proposal

The fix should be easy:

takeFoo([nil] as [Any?])

or:

let foo: [Any?] = [nil]
takeFoo(foo)
@hellohuanlin hellohuanlin added platform-ios iOS applications specifically p: pigeon related to pigeon messaging codegen tool a: plugins Support for writing, building, and running plugin packages labels May 3, 2023
@stuartmorgan-g
Copy link
Contributor

We enabled "treat warnings as errors" for Swift pigeon flutter/packages#3889, which got reverted

You enable warnings as errors for the unit test code itself in that PR. The plugin, which contains the generated code, already had that setting enabled, and wasn't reverted.

@stuartmorgan-g stuartmorgan-g added the package flutter/packages repository. See also p: labels. label May 3, 2023
@hellohuanlin
Copy link
Contributor Author

You enable warnings as errors for the unit test code itself in that PR.

updated. thx for clarification.

auto-submit bot pushed a commit to flutter/packages that referenced this issue May 4, 2023
…t test (#3901)

`as [Any?]` here is actually not a "cast", but a "type annotation". An equivalent fix can be: 

```
let arg: [Any?] = [nil]
let inputEncoded = binaryMessenger.codec.encode(arg)
```

Fixes
```
heterogeneous collection literal could only be inferred to '[Any?]'; add explicit type annotation if this is intentional
```

Which was introduced in #3889 and later got reverted. 

See flutter/flutter#126006 for more details about some interesting research on this warning. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#126006

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 2023
nploi pushed a commit to nploi/packages that referenced this issue Jul 16, 2023
…t test (flutter#3901)

`as [Any?]` here is actually not a "cast", but a "type annotation". An equivalent fix can be: 

```
let arg: [Any?] = [nil]
let inputEncoded = binaryMessenger.codec.encode(arg)
```

Fixes
```
heterogeneous collection literal could only be inferred to '[Any?]'; add explicit type annotation if this is intentional
```

Which was introduced in flutter#3889 and later got reverted. 

See flutter/flutter#126006 for more details about some interesting research on this warning. 

*List which issues are fixed by this PR. You must list at least one issue.*

Fixes flutter/flutter#126006

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: plugins Support for writing, building, and running plugin packages p: pigeon related to pigeon messaging codegen tool package flutter/packages repository. See also p: labels. platform-ios iOS applications specifically
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants