[ci+various] Partially enable javac warning checks #3293

stuartmorgan-g · 2023-02-24T18:46:30Z

Since our existing gradlew lint check doesn't catch all warnings (notably, deprecation warnings, but also others like raw values), this:

Configures our plugin example apps to enable -Xlint:all -Werror for the javac for the plugin project, so that we also get javac lint coverage in CI. This is done in the example app so that it won't affect plugin clients.
Adds a check to lint-android that the example is configured this way, so that we don't forget to set it up when adding new plugins (or re-generating plugin examples from template).

Where it was trivial for me to just fix existing violations, I did so in this PR. For the rest that had violations, I commented out the enabling of the flags, with a TODO. Normally we would do this kind of thing with a script/configs file to opt packages out of a new check, but since this is part of an existing check rather than a whole new command, doing it that way would disable gradlew lint for those packages as well. Making a whole new command seemed more complex for the long term, so this seemed like the best short term option. We should try to remove as many of these opt-outs as possible ASAP.

Part of flutter/flutter#91868

Pre-launch Checklist

packages/camera/camera_android/example/android/build.gradle

stuartmorgan-g · 2023-02-24T18:47:40Z

...merax/android/src/main/java/io/flutter/plugins/camerax/ProcessCameraProviderHostApiImpl.java

-    Camera camera =
-        processCameraProvider.bindToLifecycle(
-            (LifecycleOwner) lifecycleOwner, cameraSelector, useCases);
+    Camera camera = processCameraProvider.bindToLifecycle(lifecycleOwner, cameraSelector, useCases);


Fixes an unnecessary cast warning.

stuartmorgan-g · 2023-02-24T18:48:26Z

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java

@@ -25,7 +25,6 @@
 import com.google.android.gms.maps.model.RoundCap;
 import com.google.android.gms.maps.model.SquareCap;
 import com.google.android.gms.maps.model.Tile;
-import io.flutter.view.FlutterMain;


Quick research suggested that there's no good way to suppress the deprecation warning this line creates, so I fully qualified the names instead.

stuartmorgan-g · 2023-02-24T18:50:42Z

...picker_android/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerDelegate.java

@@ -29,12 +29,6 @@
 import java.util.Map;
 import java.util.UUID;

-enum CameraDevice {


Fixes a warning about referencing a class other that the main class (I forget the actual term in the warning) from another file.

stuartmorgan-g · 2023-02-24T18:51:12Z

...picker_android/android/src/main/java/io/flutter/plugins/imagepicker/ImagePickerDelegate.java

-        Double maxWidth = (Double) resultMap.get(cache.MAP_KEY_MAX_WIDTH);
-        Double maxHeight = (Double) resultMap.get(cache.MAP_KEY_MAX_HEIGHT);
+        Double maxWidth = (Double) resultMap.get(ImagePickerCache.MAP_KEY_MAX_WIDTH);
+        Double maxHeight = (Double) resultMap.get(ImagePickerCache.MAP_KEY_MAX_HEIGHT);


Fixes a warning about referencing static constants via instance.

stuartmorgan-g · 2023-02-24T18:51:28Z

...h/local_auth_android/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java

@@ -26,7 +26,6 @@
 import io.flutter.plugin.common.MethodChannel.MethodCallHandler;
 import io.flutter.plugin.common.MethodChannel.Result;
 import io.flutter.plugin.common.PluginRegistry;
-import io.flutter.plugin.common.PluginRegistry.Registrar;


Same deprecation issue here.

stuartmorgan-g · 2023-02-24T20:22:32Z

Oops, I forgot to check the native unit tests for warnings. Looks like I'll need to turn it off for more packages for now (but I'll see if any are easily fixable).

auto-submit · 2023-03-01T21:42:00Z

auto label is removed for flutter/packages, pr: 3293, due to - The status or check suite android-platform_tests CHANNEL:master PACKAGE_SHARDING:--shardIndex 1 --shardCount 5 has failed. Please fix the issues identified (or deflake) before re-applying this label.

stuartmorgan-g · 2023-03-01T22:00:06Z

Looks like I'll need to fix some new warnings from the image_picker PR that just landed; I'll take a look tomorrow.

stuartmorgan-g · 2023-03-02T14:55:40Z

It turns out that what caused the new warnings was the update to SDK 33, not warnings in the new code. Which means #3271 is probably going to create a lot of new deprecation warnings.

@reidbaker Are you okay with me adding a bunch of suppressions for new deprecation warnings, with TODOs pointing to new issues about writing new code paths for the deprecated code?

reidbaker · 2023-03-02T14:57:40Z

Yes I am ok with that but it means that when we update to 34 any new warning there will also be a pain :/

stuartmorgan-g · 2023-03-02T15:04:19Z

Isn't that to some extend a feature though? It means we'll find out about deprecated code earlier, and can track what will need to be updated before it becomes an emergency later.

reidbaker · 2023-03-02T16:13:08Z

Yes it is a feature but it also means that updating compileSdkVersion is no longer "strictly better" if you treat warnings as errors.

stuartmorgan-g · 2023-03-02T19:46:46Z

Landing manually; the two in-progress runs look hung, but they are iOS tests so can't be affected by this PR, and this PR is prone to collisions so I'd like to get it landed.

…ackages#3293)

* main: (3910 commits) [various] Align Flutter and Dart SDK constraints (flutter#3349) Roll Flutter from c590086 to f2f8005 (14 revisions) (flutter#3373) [webview_flutter] Enable warnings-as-errors on Android (flutter#3356) [ci] Increase Android platform test sharding (flutter#3365) Roll Flutter from f032a4d to c590086 (69 revisions) (flutter#3366) [Espresso] Update truth package to 1.1.3 (flutter#3358) [google_maps] Relax the Android renderer requset test (flutter#3364) [pigeon] Only check generated files on master (flutter#3357) [webview]: Bump androidx.webkit:webkit from 1.5.0 to 1.6.0 in /packages/webview_flutter/webview_flutter_android/android (flutter#3243) [ci+various] Partially enable javac warning checks (flutter#3293) [webview_flutter] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3336) [local_auth] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3335) [google_sign_in] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3330) [google_maps_flutter] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3329) [video_player] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3328) [file_selector] Update minimum Flutter version to 3.3 and iOS 11 (flutter#3325) [go_router_builder] Fix the example for default values in the README (flutter#3231) Update annotation and espresso dependencies (flutter#3271) [tool] Provide a --base-branch flag (flutter#3322) [image_picker_android] Adds Android 13 photo picker functionality (flutter#3267) ...

Since our existing `gradlew lint` check doesn't catch all warnings (notably, deprecation warnings, but also others like raw values), this: - Configures our plugin example apps to enable `-Xlint:all -Werror` for the javac for the plugin project, so that we also get javac lint coverage in CI. This is done in the example app so that it won't affect plugin clients. - Adds a check to `lint-android` that the example is configured this way, so that we don't forget to set it up when adding new plugins (or re-generating plugin examples from template). Where it was trivial for me to just fix existing violations, I did so in this PR. For the rest that had violations, I commented out the enabling of the flags, with a TODO. Normally we would do this kind of thing with a `script/configs` file to opt packages out of a new check, but since this is part of an existing check rather than a whole new command, doing it that way would disable `gradlew lint` for those packages as well. Making a whole new command seemed more complex for the long term, so this seemed like the best short term option. We should try to remove as many of these opt-outs as possible ASAP. Part of flutter/flutter#91868

stuartmorgan-g added 5 commits February 24, 2023 12:21

Add new check and tests

91c68e5

Enable werror everywhere

734eb54

Fix or temporarily disable to get everything green

88d046c

Revert accidental change

d7fe1bd

Autoformat

cea94d5

stuartmorgan-g requested review from camsim99, gmackall, reidbaker and bparrishMines as code owners February 24, 2023 18:46

stuartmorgan-g commented Feb 24, 2023

View reviewed changes

stuartmorgan-g mentioned this pull request Feb 24, 2023

Migrate packages repo to use flutter.compileSdkVersion flutter/flutter#121425

Closed

analyzer fix

ff90733

stuartmorgan-g added 2 commits February 24, 2023 19:19

Fix unit tests

bad3270

Merge branch 'main' into tool-android-javac-warnings

13905dd

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 1, 2023

Merge branch 'main' into tool-android-javac-warnings

23ed9d9

stuartmorgan-g added 2 commits March 2, 2023 12:35

Revert metadata

52c5f02

Merge branch 'main' into tool-android-javac-warnings

f388008

stuartmorgan-g mentioned this pull request Mar 2, 2023

[image_picker] Replace deprecated methods on Android flutter/flutter#121816

Closed

stuartmorgan-g added 2 commits March 2, 2023 13:21

Suppress new deprecation warnings

532ffb9

Re-bump versions

c1a68e2

stuartmorgan-g force-pushed the tool-android-javac-warnings branch from 2af1488 to c1a68e2 Compare March 2, 2023 18:23

Re-fix web-based change

7dfd1e4

stuartmorgan-g mentioned this pull request Mar 2, 2023

Consider warnings-as-errors for plugin CI builds flutter/flutter#91868

Closed

stuartmorgan-g merged commit 2f21321 into flutter:main Mar 2, 2023

stuartmorgan-g deleted the tool-android-javac-warnings branch March 2, 2023 19:47

engine-flutter-autoroll mentioned this pull request Mar 3, 2023

Roll Packages from 875a4032544f to 2f21321980de (8 revisions) flutter/flutter#121871

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 3, 2023

2f21321 [ci+various] Partially enable javac warning checks (flutter/p…

634ff43

…ackages#3293)

engine-flutter-autoroll mentioned this pull request Mar 3, 2023

Roll Packages from 875a4032544f to da635f0fee3c (9 revisions) flutter/flutter#121876

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 3, 2023

2f21321 [ci+various] Partially enable javac warning checks (flutter/p…

bc57ed8

…ackages#3293)

engine-flutter-autoroll mentioned this pull request Mar 3, 2023

Roll Packages from 875a4032544f to ca7205b9c541 (10 revisions) flutter/flutter#121879

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 3, 2023

2f21321 [ci+various] Partially enable javac warning checks (flutter/p…

6d8cb8b

…ackages#3293)

reidbaker mentioned this pull request Mar 20, 2023

[Many] i123017 integration annotation documentation #3495

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ci+various] Partially enable javac warning checks #3293

[ci+various] Partially enable javac warning checks #3293

Uh oh!

stuartmorgan-g commented Feb 24, 2023

Uh oh!

Uh oh!

stuartmorgan-g Feb 24, 2023

Uh oh!

stuartmorgan-g Feb 24, 2023

Uh oh!

stuartmorgan-g Feb 24, 2023

Uh oh!

stuartmorgan-g Feb 24, 2023

Uh oh!

stuartmorgan-g Feb 24, 2023

Uh oh!

stuartmorgan-g commented Feb 24, 2023

Uh oh!

auto-submit bot commented Mar 1, 2023

Uh oh!

stuartmorgan-g commented Mar 1, 2023

Uh oh!

stuartmorgan-g commented Mar 2, 2023

Uh oh!

reidbaker commented Mar 2, 2023

Uh oh!

stuartmorgan-g commented Mar 2, 2023

Uh oh!

reidbaker commented Mar 2, 2023

Uh oh!

stuartmorgan-g commented Mar 2, 2023

Uh oh!

Uh oh!

[ci+various] Partially enable javac warning checks #3293

[ci+various] Partially enable javac warning checks #3293

Uh oh!

Conversation

stuartmorgan-g commented Feb 24, 2023

Pre-launch Checklist

Uh oh!

Uh oh!

stuartmorgan-g Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g Feb 24, 2023

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Feb 24, 2023

Uh oh!

auto-submit bot commented Mar 1, 2023

Uh oh!

stuartmorgan-g commented Mar 1, 2023

Uh oh!

stuartmorgan-g commented Mar 2, 2023

Uh oh!

reidbaker commented Mar 2, 2023

Uh oh!

stuartmorgan-g commented Mar 2, 2023

Uh oh!

reidbaker commented Mar 2, 2023

Uh oh!

stuartmorgan-g commented Mar 2, 2023

Uh oh!

Uh oh!