[pigeon] Enable warnings as errors for remaining languages #3317
Conversation
|
I realized while fixing Kotlin warnings that it'll conflict with your change, so I'll reconcile once that's landed. It shouldn't change much, so this should still be fine to review. |
| @@ -180,12 +180,13 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |||
| final HostDatatype hostDatatype = _getHostDatatype(root, field); | |||
| String toWriteValue = ''; | |||
| final String fieldName = field.name; | |||
| final String safeCall = field.type.isNullable ? '?' : ''; | |||
There was a problem hiding this comment.
Fixes warnings about unnecessary safe calls on non-nullable values.
| @@ -458,7 +458,7 @@ class JavaGenerator extends StructuredGenerator<JavaOptions> { | |||
| '$returnType $output = channelReply == null ? null : ((Number) channelReply).longValue();'); | |||
| } else { | |||
| indent.writeln( | |||
| '$returnType $output = ($returnType) channelReply;'); | |||
| '$returnType $output = ${_cast('channelReply', javaType: returnType)};'); | |||
There was a problem hiding this comment.
The cast changes in Java and Kotlin fix uncessary casts (foo as Any?, foo as? Any in Kotlin, (Object) foo in Java).
| @@ -1,30 +0,0 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
I forgot to delete this file in my last consolidation; it's unused. I was getting warnings in my stale output copy and couldn't figure out why at first since the source file was still there.
(Also: you'll need to clean your generated files again before this lands if you haven't done so recently.)
| @@ -61,7 +61,7 @@ public void asyncSuccess() { | |||
| (bytes) -> { | |||
| bytes.rewind(); | |||
| @SuppressWarnings("unchecked") | |||
| ArrayList wrapped = (ArrayList) codec.decodeMessage(bytes); | |||
There was a problem hiding this comment.
I had to fix a bunch of raw type warnings in our tests, and suppress a bunch of unchecked casts for most of the same lines.
|
you have angered the kotlin unit tests |
|
I was focused on iterating on all the native tests and forgot to run the Dart unit tests. It was just an expectation that needed to be updated (based on the exact output of the |
|
Reconciled with #3284 @tarrinneal It seems like #3284 was incomplete for Kotlin, because I had fewer conflicts than I expected. I have several non-conflict areas where my new helper is being called with |
I'll take a look once this is merged |
stuartmorgan-g
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/packages, pr: 3317, due to - Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead. |
|
auto label is removed for flutter/packages, pr: 3317, due to Validations Fail. |
|
Oops, I thought you'd reviewed. Bot checks for the win! |
I thought I had too, lol. Working on it now. |
If this is something that should never happen, could we add an assert to enforce this? |
We should just remove the flag (unless there is a use case for it that I missed). Which is what I thought I would do when I reconciled my PR with yours, and was thus surprised when I couldn't :) |
stuartmorgan-g
added
the
autosubmit
Enables warnings-as-errors for remaining languages: Kotlin, Java, and Objective-C (it was already on for Swift and C++), and fix the resulting violations.
Fixes flutter/flutter#59116
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).