Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker][android] suppress unchecked warning #4408

Merged
merged 3 commits into from
Oct 19, 2021
Merged

[image_picker][android] suppress unchecked warning #4408

merged 3 commits into from
Oct 19, 2021

Conversation

lukaskurz
Copy link
Contributor

@lukaskurz lukaskurz commented Oct 1, 2021

Casting a Object to an ArrayList from a map causes an unchecked cast warning, which shows up during builds of the image_picker plugin. Looking at the code, we can verify that the only times the value belonging to cache.MAP_KEY_PATH_LIST is set, it is an ArrayList.

Map<String, Object> resultMap = new HashMap<>();
    ArrayList<String> pathList = new ArrayList<>();
    boolean hasData = false;

    if (prefs.contains(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY)) {
      final Set<String> imagePathList =
          prefs.getStringSet(FLUTTER_IMAGE_PICKER_IMAGE_PATH_KEY, null);
      if (imagePathList != null) {
        pathList.addAll(imagePathList);
        resultMap.put(MAP_KEY_PATH_LIST, pathList); // here
        hasData = true;
      }
    }

and

ArrayList<String> pathList = (ArrayList<String>) resultMap.get(cache.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);
        int imageQuality =
            resultMap.get(cache.MAP_KEY_IMAGE_QUALITY) == null
                ? 100
                : (int) resultMap.get(cache.MAP_KEY_IMAGE_QUALITY);

        newPathList.add(imageResizer.resizeImageIfNeeded(path, maxWidth, maxHeight, imageQuality));
      }
      resultMap.put(cache.MAP_KEY_PATH_LIST, newPathList); // and here
      resultMap.put(cache.MAP_KEY_PATH, newPathList.get(newPathList.size() - 1));
    }

Since instanceof checks cannot be done with generics (afaik, not a java pro here 😅 ) and a ClassCastException has to be thrown either way, if the value is not an ArrayList, I think it can be suppressed.

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#90811

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter]. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Oct 1, 2021
@lukaskurz lukaskurz changed the title [image_picker][v0.8.4+2] suppress unchecked warning [image_picker][android] suppress unchecked warning Oct 1, 2021
Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a small nit

packages/image_picker/image_picker/CHANGELOG.md Outdated Show resolved Hide resolved
@lukaskurz
Copy link
Contributor Author

BTW, I wasn't sure where to ask this, but is the repo participating in the hacktoberfest ?

@bparrishMines bparrishMines added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Oct 19, 2021
@bparrishMines
Copy link
Contributor

@lukaskurz From my knowledge, we aren't participating in hacktoberfest. Unless @stuartmorgan has heard something different.

@stuartmorgan
Copy link
Contributor

BTW, I wasn't sure where to ask this, but is the repo participating in the hacktoberfest ?

No. The page you linked to explains how to tell; we don't have that topic on the repo.

@lukaskurz
Copy link
Contributor Author

@stuartmorgan To be honest, I was kinda hoping for the hacktoberfest-accepted label, since that does not require the topic to be added 😅 But it's not a problem, I can totally understand why you would not want to pollute the existing labels with another label just for some FOSS event.

Thanks for accepting the PR 👍

@stuartmorgan
Copy link
Contributor

It's not a question of label pollution. We are not participating because our experience with the event (which in the past was not opt-in) has not been positive.

@lukaskurz
Copy link
Contributor Author

lukaskurz commented Oct 21, 2021

@stuartmorgan Oh, i did not know that. May I ask what that bad experience entailed ? I'm guessing there probably is an influx of low-quality/low-effort PRs.

Edit: Admittedly such as adding a single @SuppressWarning(...) 😅

@stuartmorgan
Copy link
Contributor

I'm guessing there probably is an influx of low-quality/low-effort PRs.

Yes.

Edit: Admittedly such as adding a single @SuppressWarning(...) 😅

No, fixing warnings is useful. It was mostly completely pointless README changes.

NickalasB added a commit to NickalasB/plugins that referenced this pull request Oct 21, 2021
* master:
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  [ci] Replace Firebase Test Lab deprecated Pixel 4 device with Pixel 5 (flutter#4436)
  [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality  (flutter#4389)
  Bump compileSdkVersion to 31 (flutter#4432)
  [camera] Update [CameraOrientationTests testOrientationNotifications] unit test to work on Xcode 13 (flutter#4426)
  Update integration_test README  (flutter#3824)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)
NickalasB added a commit to NickalasB/plugins that referenced this pull request Oct 21, 2021
* master:
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  [ci] Replace Firebase Test Lab deprecated Pixel 4 device with Pixel 5 (flutter#4436)
  [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality  (flutter#4389)
  Bump compileSdkVersion to 31 (flutter#4432)
  [camera] Update [CameraOrientationTests testOrientationNotifications] unit test to work on Xcode 13 (flutter#4426)
  Update integration_test README  (flutter#3824)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)

# Conflicts:
#	packages/webview_flutter/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/webview_flutter/pubspec.yaml
yasargil added a commit to yasargil/plugins that referenced this pull request Oct 25, 2021
* master: (364 commits)
  Use OpenJDK 11 in CI jobs  (flutter#4419)
  [google_sign_in] remove the commented out code in tests (flutter#4442)
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  [ci] Replace Firebase Test Lab deprecated Pixel 4 device with Pixel 5 (flutter#4436)
  [image_picker_for_web] Added support for maxWidth, maxHeight and imageQuality  (flutter#4389)
  Bump compileSdkVersion to 31 (flutter#4432)
  [camera] Update [CameraOrientationTests testOrientationNotifications] unit test to work on Xcode 13 (flutter#4426)
  Update integration_test README  (flutter#3824)
  [webview_flutter] Adjust test URLs again (flutter#4407)
  [google_sign_in] Add serverAuthCode attribute to google_sign_in_platform_interface user data (flutter#4179)
  [camera] Add filter for unsupported cameras on Android (flutter#4418)
  ...

# Conflicts:
#	packages/google_maps_flutter/google_maps_flutter/android/src/main/java/io/flutter/plugins/googlemaps/GoogleMapController.java
hofmannfelix pushed a commit to hofmannfelix/plugins that referenced this pull request Oct 30, 2021
…ideo_src_on_same_controller

* commit '76e84c0679dbab7bfaaaa553b17bb0dbdb9a3c33': (537 commits)
  [video_player] Initialize player when size and duration become available (flutter#4438)
  [webview_flutter] Implement zoom enabled for ios and android (flutter#4417)
  Partial revert of "upgraded usage of BinaryMessenger (flutter#4451)" (flutter#4453)
  Implement Android WebView api with pigeon (Java portion) (flutter#4441)
  [in_app_purchase] Update to the latest pkg:json_serializable (flutter#4434)
  Implement Android WebView api with pigeon (Dart portion)  (flutter#4435)
  upgraded usage of BinaryMessenger (flutter#4451)
  [flutter_plugin_tools] Fix pubspec-check on Windows (flutter#4428)
  Use OpenJDK 11 in CI jobs  (flutter#4419)
  [google_sign_in] remove the commented out code in tests (flutter#4442)
  [webview] Fix typos in the README (flutter#4249)
  [google_sign_in] add serverAuthCode to GoogleSignInAccount (flutter#4180)
  [ci] Update macOS Cirrus image to Xcode 13 (flutter#4429)
  [shared_preferences] Switch to new analysis options (flutter#4384)
  [flutter_plugin_android_lifecycle] remove placeholder dart file (flutter#4413)
  [camera] Run iOS methods on UI thread by default (flutter#4140)
  [ci] Always run all `format` steps (flutter#4427)
  [flutter_plugin_tools] Fix license-check on Windows (flutter#4425)
  [google_maps_flutter] Clean Java test, consolidate Marker example. (flutter#4400)
  [image_picker][android] suppress unchecked warning (flutter#4408)
  ...

# Conflicts:
#	packages/video_player/video_player/pubspec.yaml
#	packages/video_player/video_player_web/lib/video_player_web.dart
#	packages/video_player/video_player_web/pubspec.yaml
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: image_picker platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants