-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[share] MethodCallHandler.java uses unchecked or unsafe operations #3004
Conversation
Hi @cyanglaz |
I'm confused, what issue does this solve exactly? |
Hi @PixelToast A message is displayed when building for android:
|
Ah, I understand the concern, thanks for the PR! Why not use the generic type parameter of MethodCall.argument to do the casting? Serializing and deserializing definitely isn't the right approach in my opinion. |
Casting Argument to Object will be fine but Casting to List it's unsafe operation. |
I don't have a dev environment available at the moment, but would |
Thank you for your reply @PixelToast 🥇 I just pushed the new code
|
paths = call.argument("paths"); | ||
mimeTypes = call.argument("mimeTypes"); | ||
text = call.argument("text"); | ||
subject = call.argument("subject"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: move this out of the try block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnfield PR updated . Thank you
@@ -21,23 +21,27 @@ | |||
|
|||
@Override | |||
public void onMethodCall(MethodCall call, MethodChannel.Result result) { | |||
String text, subject; | |||
List<String> paths, mimeTypes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: move to "shareFiles"
case, since that's the only place it's used.
Also for these, please use one var per declaration, per https://google.github.io/styleguide/javaguide.html#s4.8.2-variable-declarations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnfield PR updated . Thank you
String text; | ||
String subject; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not saying this on the last round, but I think this is going to be a bit more readable if we don't declare these up here, since we don't expect to use them again outside of the block. We can just do String text =
and String subject =
in the cases that need them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Handle deprecation & unchecked warning as error
Avoiding uses unchecked or unsafe Object Type Casting in MethodCallHandler.java
Related issue : flutter/flutter#65970
A message is displayed when building for android:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?