Skip to content

Commit

Permalink
[ci+various] Partially enable javac warning checks (flutter#3293)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
stuartmorgan committed Mar 2, 2023
1 parent b7812d9 commit 2f21321
Show file tree
Hide file tree
Showing 39 changed files with 436 additions and 70 deletions.
14 changes: 14 additions & 0 deletions packages/camera/camera_android/example/android/build.gradle
Expand Up @@ -27,3 +27,17 @@ subprojects {
task clean(type: Delete) {
delete rootProject.buildDir
}

// Build the plugin project with warnings enabled. This is here rather than
// in the plugin itself to avoid breaking clients that have different
// warnings (e.g., deprecation warnings from a newer SDK than this project
// builds with).
gradle.projectsEvaluated {
project(":camera_android") {
tasks.withType(JavaCompile) {
// TODO(stuartmorgan): Enable this. See
// https://github.com/flutter/flutter/issues/91868
//options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
Expand Up @@ -120,9 +120,7 @@ public Long bindToLifecycle(
instanceManager.getInstance(((Number) useCaseIds.get(i)).longValue()));
}

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

final CameraFlutterApiImpl cameraFlutterApi =
new CameraFlutterApiImpl(binaryMessenger, instanceManager);
Expand Down
Expand Up @@ -30,6 +30,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
Expand Down Expand Up @@ -107,7 +108,6 @@ public void setSurfaceProviderTest_createsSurfaceProviderAndReturnsTextureEntryI
final ArgumentCaptor<Preview.SurfaceProvider> surfaceProviderCaptor =
ArgumentCaptor.forClass(Preview.SurfaceProvider.class);
final ArgumentCaptor<Surface> surfaceCaptor = ArgumentCaptor.forClass(Surface.class);
final ArgumentCaptor<Consumer> consumerCaptor = ArgumentCaptor.forClass(Consumer.class);

// Test that surface provider was set and the surface texture ID was returned.
assertEquals(previewHostApi.setSurfaceProvider(previewIdentifier), surfaceTextureEntryId);
Expand Down Expand Up @@ -136,7 +136,9 @@ public void createSurfaceProvider_createsExpectedPreviewSurfaceProvider() {
.thenReturn(mockSystemServicesFlutterApi);

final ArgumentCaptor<Surface> surfaceCaptor = ArgumentCaptor.forClass(Surface.class);
final ArgumentCaptor<Consumer> consumerCaptor = ArgumentCaptor.forClass(Consumer.class);
@SuppressWarnings("unchecked")
final ArgumentCaptor<Consumer<SurfaceRequest.Result>> consumerCaptor =
ArgumentCaptor.forClass(Consumer.class);

Preview.SurfaceProvider previewSurfaceProvider =
previewHostApi.createSurfaceProvider(mockSurfaceTexture);
Expand Down Expand Up @@ -183,7 +185,8 @@ public void createSurfaceProvider_createsExpectedPreviewSurfaceProvider() {
.thenReturn(SurfaceRequest.Result.RESULT_INVALID_SURFACE);
capturedConsumer.accept(mockSurfaceRequestResult);
verify(mockSurface).release();
verify(mockSystemServicesFlutterApi).sendCameraError(anyString(), any(Reply.class));
verify(mockSystemServicesFlutterApi)
.sendCameraError(anyString(), ArgumentMatchers.<Reply<Void>>any());
}

@Test
Expand Down
Expand Up @@ -66,6 +66,7 @@ public void getInstanceTest() {
new ProcessCameraProviderHostApiImpl(mockBinaryMessenger, testInstanceManager, context);
final ListenableFuture<ProcessCameraProvider> processCameraProviderFuture =
spy(Futures.immediateFuture(processCameraProvider));
@SuppressWarnings("unchecked")
final GeneratedCameraXLibrary.Result<Long> mockResult =
mock(GeneratedCameraXLibrary.Result.class);

Expand Down
Expand Up @@ -24,6 +24,7 @@
import org.junit.Rule;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
Expand All @@ -43,6 +44,7 @@ public void requestCameraPermissionsTest() {
mock(CameraPermissionsManager.class);
final Activity mockActivity = mock(Activity.class);
final PermissionsRegistry mockPermissionsRegistry = mock(PermissionsRegistry.class);
@SuppressWarnings("unchecked")
final Result<CameraPermissionsErrorData> mockResult = mock(Result.class);
final Boolean enableAudio = false;

Expand All @@ -65,7 +67,7 @@ public void requestCameraPermissionsTest() {
eq(enableAudio),
resultCallbackCaptor.capture());

ResultCallback resultCallback = (ResultCallback) resultCallbackCaptor.getValue();
ResultCallback resultCallback = resultCallbackCaptor.getValue();

// Test no error data is sent upon permissions request success.
resultCallback.onResult(null, null);
Expand Down Expand Up @@ -130,7 +132,7 @@ public void deviceOrientationChangeTest() {
deviceOrientationChangeCallback.onChange(DeviceOrientation.PORTRAIT_DOWN);
verify(systemServicesFlutterApi)
.sendDeviceOrientationChangedEvent(
eq(DeviceOrientation.PORTRAIT_DOWN.toString()), any(Reply.class));
eq(DeviceOrientation.PORTRAIT_DOWN.toString()), ArgumentMatchers.<Reply<Void>>any());

// Test that the DeviceOrientationManager starts listening for device orientation changes.
verify(mockDeviceOrientationManager).start();
Expand Down
Expand Up @@ -29,3 +29,15 @@ subprojects {
task clean(type: Delete) {
delete rootProject.buildDir
}

// Build the plugin project with warnings enabled. This is here rather than
// in the plugin itself to avoid breaking clients that have different
// warnings (e.g., deprecation warnings from a newer SDK than this project
// builds with).
gradle.projectsEvaluated {
project(":camera_android_camerax") {
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
14 changes: 14 additions & 0 deletions packages/espresso/example/android/build.gradle
Expand Up @@ -27,3 +27,17 @@ subprojects {
task clean(type: Delete) {
delete rootProject.buildDir
}

// Build the plugin project with warnings enabled. This is here rather than
// in the plugin itself to avoid breaking clients that have different
// warnings (e.g., deprecation warnings from a newer SDK than this project
// builds with).
gradle.projectsEvaluated {
project(":espresso") {
tasks.withType(JavaCompile) {
// TODO(stuartmorgan): Enable this. See
// https://github.com/flutter/flutter/issues/91868
//options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
Expand Up @@ -11,6 +11,7 @@
import androidx.lifecycle.Lifecycle;
import io.flutter.embedding.engine.plugins.activity.ActivityPluginBinding;
import io.flutter.plugin.common.PluginRegistry;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
Expand All @@ -19,9 +20,16 @@
public class FlutterLifecycleAdapterTest {
@Mock Lifecycle lifecycle;

AutoCloseable mockCloseable;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
mockCloseable = MockitoAnnotations.openMocks(this);
}

@After
public void tearDown() throws Exception {
mockCloseable.close();
}

@Test
Expand Down
Expand Up @@ -27,3 +27,15 @@ subprojects {
task clean(type: Delete) {
delete rootProject.buildDir
}

// Build the plugin project with warnings enabled. This is here rather than
// in the plugin itself to avoid breaking clients that have different
// warnings (e.g., deprecation warnings from a newer SDK than this project
// builds with).
gradle.projectsEvaluated {
project(":flutter_plugin_android_lifecycle") {
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
@@ -1,3 +1,7 @@
## 2.4.8

* Fixes compilation warnings.

## 2.4.7

* Updates annotation dependency.
Expand Down
Expand Up @@ -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;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand All @@ -50,15 +49,16 @@ private static BitmapDescriptor toBitmapDescriptor(Object o) {
case "fromAsset":
if (data.size() == 2) {
return BitmapDescriptorFactory.fromAsset(
FlutterMain.getLookupKeyForAsset(toString(data.get(1))));
io.flutter.view.FlutterMain.getLookupKeyForAsset(toString(data.get(1))));
} else {
return BitmapDescriptorFactory.fromAsset(
FlutterMain.getLookupKeyForAsset(toString(data.get(1)), toString(data.get(2))));
io.flutter.view.FlutterMain.getLookupKeyForAsset(
toString(data.get(1)), toString(data.get(2))));
}
case "fromAssetImage":
if (data.size() == 3) {
return BitmapDescriptorFactory.fromAsset(
FlutterMain.getLookupKeyForAsset(toString(data.get(1))));
io.flutter.view.FlutterMain.getLookupKeyForAsset(toString(data.get(1))));
} else {
throw new IllegalArgumentException(
"'fromAssetImage' Expected exactly 3 arguments, got: " + data.size());
Expand Down
Expand Up @@ -27,3 +27,17 @@ subprojects {
task clean(type: Delete) {
delete rootProject.buildDir
}

// Build the plugin project with warnings enabled. This is here rather than
// in the plugin itself to avoid breaking clients that have different
// warnings (e.g., deprecation warnings from a newer SDK than this project
// builds with).
gradle.projectsEvaluated {
project(":google_maps_flutter_android") {
tasks.withType(JavaCompile) {
// TODO(stuartmorgan): Enable this. See
// https://github.com/flutter/flutter/issues/91868
//options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
Expand Up @@ -2,7 +2,7 @@ name: google_maps_flutter_android
description: Android implementation of the google_maps_flutter plugin.
repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
version: 2.4.7
version: 2.4.8

environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down
Expand Up @@ -29,6 +29,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -41,18 +42,23 @@ public class GoogleSignInTest {
@Mock Context mockContext;
@Mock Resources mockResources;
@Mock Activity mockActivity;
@Mock PluginRegistry.Registrar mockRegistrar;
@Mock BinaryMessenger mockMessenger;
@Spy MethodChannel.Result result;
@Mock GoogleSignInWrapper mockGoogleSignIn;
@Mock GoogleSignInAccount account;
@Mock GoogleSignInClient mockClient;
@Mock Task<GoogleSignInAccount> mockSignInTask;

@SuppressWarnings("deprecation")
@Mock
PluginRegistry.Registrar mockRegistrar;

private GoogleSignInPlugin plugin;
private AutoCloseable mockCloseable;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
mockCloseable = MockitoAnnotations.openMocks(this);
when(mockRegistrar.messenger()).thenReturn(mockMessenger);
when(mockRegistrar.context()).thenReturn(mockContext);
when(mockRegistrar.activity()).thenReturn(mockActivity);
Expand All @@ -62,6 +68,11 @@ public void setUp() {
plugin.setUpRegistrar(mockRegistrar);
}

@After
public void tearDown() throws Exception {
mockCloseable.close();
}

@Test
public void requestScopes_ResultErrorIfAccountIsNull() {
MethodCall methodCall = new MethodCall("requestScopes", null);
Expand Down
Expand Up @@ -27,3 +27,15 @@ subprojects {
task clean(type: Delete) {
delete rootProject.buildDir
}

// Build the plugin project with warnings enabled. This is here rather than
// in the plugin itself to avoid breaking clients that have different
// warnings (e.g., deprecation warnings from a newer SDK than this project
// builds with).
gradle.projectsEvaluated {
project(":google_sign_in_android") {
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror"
}
}
}
3 changes: 2 additions & 1 deletion packages/image_picker/image_picker_android/CHANGELOG.md
@@ -1,5 +1,6 @@
## NEXT
## 0.8.5+9

* Fixes compilation warnings.
* Updates compileSdkVersion to 33.

## 0.8.5+8
Expand Down
Expand Up @@ -31,12 +31,6 @@
import java.util.Map;
import java.util.UUID;

enum CameraDevice {
REAR,

FRONT
}

/**
* A delegate class doing the heavy lifting for the plugin.
*
Expand Down Expand Up @@ -86,6 +80,11 @@ public class ImagePickerDelegate
@VisibleForTesting static final int REQUEST_CODE_TAKE_VIDEO_WITH_CAMERA = 2353;
@VisibleForTesting static final int REQUEST_CAMERA_VIDEO_PERMISSION = 2355;

public enum CameraDevice {
REAR,
FRONT
}

@VisibleForTesting final String fileProviderName;

private final Activity activity;
Expand Down Expand Up @@ -222,21 +221,22 @@ void saveStateBeforeResult() {
void retrieveLostImage(MethodChannel.Result result) {
Map<String, Object> resultMap = cache.getCacheMap();
@SuppressWarnings("unchecked")
ArrayList<String> pathList = (ArrayList<String>) resultMap.get(cache.MAP_KEY_PATH_LIST);
ArrayList<String> pathList =
(ArrayList<String>) resultMap.get(ImagePickerCache.MAP_KEY_PATH_LIST);
ArrayList<String> newPathList = new ArrayList<>();
if (pathList != null) {
for (String path : pathList) {
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);
int imageQuality =
resultMap.get(cache.MAP_KEY_IMAGE_QUALITY) == null
resultMap.get(ImagePickerCache.MAP_KEY_IMAGE_QUALITY) == null
? 100
: (int) resultMap.get(cache.MAP_KEY_IMAGE_QUALITY);
: (int) resultMap.get(ImagePickerCache.MAP_KEY_IMAGE_QUALITY);

newPathList.add(imageResizer.resizeImageIfNeeded(path, maxWidth, maxHeight, imageQuality));
}
resultMap.put(cache.MAP_KEY_PATH_LIST, newPathList);
resultMap.put(cache.MAP_KEY_PATH, newPathList.get(newPathList.size() - 1));
resultMap.put(ImagePickerCache.MAP_KEY_PATH_LIST, newPathList);
resultMap.put(ImagePickerCache.MAP_KEY_PATH, newPathList.get(newPathList.size() - 1));
}
if (resultMap.isEmpty()) {
result.success(null);
Expand Down Expand Up @@ -450,6 +450,8 @@ private File createTemporaryWritableFile(String suffix) {

private void grantUriPermissions(Intent intent, Uri imageUri) {
PackageManager packageManager = activity.getPackageManager();
// TODO(stuartmorgan): Add new codepath: https://github.com/flutter/flutter/issues/121816
@SuppressWarnings("deprecation")
List<ResolveInfo> compatibleActivities =
packageManager.queryIntentActivities(intent, PackageManager.MATCH_DEFAULT_ONLY);

Expand Down
Expand Up @@ -342,12 +342,12 @@ public void onMethodCall(MethodCall call, MethodChannel.Result rawResult) {
int imageSource;
ImagePickerDelegate delegate = activityState.getDelegate();
if (call.argument("cameraDevice") != null) {
CameraDevice device;
ImagePickerDelegate.CameraDevice device;
int deviceIntValue = call.argument("cameraDevice");
if (deviceIntValue == CAMERA_DEVICE_FRONT) {
device = CameraDevice.FRONT;
device = ImagePickerDelegate.CameraDevice.FRONT;
} else {
device = CameraDevice.REAR;
device = ImagePickerDelegate.CameraDevice.REAR;
}
delegate.setCameraDevice(device);
}
Expand Down

0 comments on commit 2f21321

Please sign in to comment.