Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[google_sign_in_ios] Fix "callee requires a non-null parameter" analyzer warning #7513

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Aug 26, 2024

The normal Cocoa pattern is to check a result, and only if it's nil then do something with the error.

https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ErrorHandlingCocoa/CreateCustomizeNSError/CreateCustomizeNSError.html

Important: Success or failure is indicated by the return value of the method. Although Cocoa methods that indirectly return error objects in the Cocoa error domain are guaranteed to return such objects if the method indicates failure by directly returning nil or NO, you should always check that the return value is nil or NO before attempting to do anything with the NSError object.

Use this pattern with the result and error from the sign in completion block. This means -didSignInForUser is only called for non-nil users (the possibility of a nullable parameter was kicking off the analyzer warning).

Screenshot 2024-08-26 at 4 24 04 PM

Fixes flutter/flutter#153587

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Comment on lines -306 to -309
NSString *idToken;
if (user.idToken) {
idToken = user.idToken.tokenString;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed, if user.idToken is nil then user.idToken.tokenString will also be nil. This is another normal Cocoa pattern, don't check nils since calling a selector (method) on nil will harmlessly also return nil.

completion([FSIUserData makeWithDisplayName:user.profile.name
email:user.profile.email
userId:user.userID
photoUrl:photoUrl.absoluteString
Copy link
Member Author

Choose a reason for hiding this comment

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

Swap to dot-notation because absoluteString is a property. This is just a convention, there's no functional difference.
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CocoaFundamentals/CocoaObjects/CocoaObjects.html

@jmagman jmagman marked this pull request as ready for review August 27, 2024 18:18
@jmagman jmagman requested a review from loic-sharma as a code owner August 27, 2024 18:18
@jmagman jmagman requested a review from stuartmorgan August 27, 2024 18:18
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

@jmagman jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 27, 2024
@auto-submit auto-submit bot merged commit ce17b4b into flutter:main Aug 27, 2024
76 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 29, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 29, 2024
flutter/packages@d862279...2a0f254

2024-08-28 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Adds internal wrapper for remaining methods of the Android native `AdsManager`  (flutter/packages#7437)
2024-08-28 brackenavaron@gmail.com [shared_preferences] Fix typo in changelog (flutter/packages#7523)
2024-08-27 matanlurey@users.noreply.github.com Remove matan from codeowners (flutter/packages#7511)
2024-08-27 tarrinneal@gmail.com [shared_preferences] Add test to enforce mutable lists (flutter/packages#7369)
2024-08-27 magder@google.com [google_sign_in_ios] Fix "callee requires a non-null parameter" analyzer warning (flutter/packages#7513)
2024-08-27 louisehsu@google.com [in_app_purchase_storekit] Allows 'localizedDescription' to be nullable to handle occasional nulls in Storekit objects (flutter/packages#7515)
2024-08-27 31859944+LongCatIsLooong@users.noreply.github.com Cupertino icons golden tests (flutter/packages#7421)
2024-08-26 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 2.0.10 to 2.0.20 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7500)
2024-08-26 zezohassam@gmail.com [video_player] Updates minimum supported SDK (flutter/packages#7498)
2024-08-26 paulberry@google.com [url_launcher] Ignore new `unreachable_switch_default` warning. (flutter/packages#7487)
2024-08-26 linxunfeng@yeah.net Revert "[video_player] Relands #6456: Uses SurfaceProducer, this time with setCallback for suspend/resume lifecycles" (flutter/packages#7497)
2024-08-24 matanlurey@users.noreply.github.com [video_player] Relands #6456: Uses `SurfaceProducer`, this time with `setCallback` for suspend/resume lifecycles. (flutter/packages#6989)
2024-08-23 109111084+yaakovschectman@users.noreply.github.com [google_maps_flutter_android] Convert `PlatformTileOverlay` to Pigeon (flutter/packages#7467)
2024-08-23 47866232+chunhtai@users.noreply.github.com [go_router] Fixes issue so that the parseRouteInformationWithContext � (flutter/packages#7337)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Aug 29, 2024
Bump Xcode analyze minimum check version iOS 13 -> iOS 14 and macOS 12.3 -> macOS 13.

- Hit StoreKit2 deprecations. Exclude `in_app_purchase_storekit` to work around StoreKit 1 deprecation warnings.  Added a TODO to remove the exclusion when StoreKit2 is adopted (this should happen in the next few weeks) flutter/flutter#116383.
- Hit flutter/flutter#153587, fixed in #7513
- Hit flutter/flutter#125901 which was `#pragma`'d out in #6221

See related flutter/plugins#5778 and flutter/flutter#102835
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
flutter/packages@d862279...2a0f254

2024-08-28 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Adds internal wrapper for remaining methods of the Android native `AdsManager`  (flutter/packages#7437)
2024-08-28 brackenavaron@gmail.com [shared_preferences] Fix typo in changelog (flutter/packages#7523)
2024-08-27 matanlurey@users.noreply.github.com Remove matan from codeowners (flutter/packages#7511)
2024-08-27 tarrinneal@gmail.com [shared_preferences] Add test to enforce mutable lists (flutter/packages#7369)
2024-08-27 magder@google.com [google_sign_in_ios] Fix "callee requires a non-null parameter" analyzer warning (flutter/packages#7513)
2024-08-27 louisehsu@google.com [in_app_purchase_storekit] Allows 'localizedDescription' to be nullable to handle occasional nulls in Storekit objects (flutter/packages#7515)
2024-08-27 31859944+LongCatIsLooong@users.noreply.github.com Cupertino icons golden tests (flutter/packages#7421)
2024-08-26 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 2.0.10 to 2.0.20 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7500)
2024-08-26 zezohassam@gmail.com [video_player] Updates minimum supported SDK (flutter/packages#7498)
2024-08-26 paulberry@google.com [url_launcher] Ignore new `unreachable_switch_default` warning. (flutter/packages#7487)
2024-08-26 linxunfeng@yeah.net Revert "[video_player] Relands flutter#6456: Uses SurfaceProducer, this time with setCallback for suspend/resume lifecycles" (flutter/packages#7497)
2024-08-24 matanlurey@users.noreply.github.com [video_player] Relands flutter#6456: Uses `SurfaceProducer`, this time with `setCallback` for suspend/resume lifecycles. (flutter/packages#6989)
2024-08-23 109111084+yaakovschectman@users.noreply.github.com [google_maps_flutter_android] Convert `PlatformTileOverlay` to Pigeon (flutter/packages#7467)
2024-08-23 47866232+chunhtai@users.noreply.github.com [go_router] Fixes issue so that the parseRouteInformationWithContext � (flutter/packages#7337)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_sign_in platform-ios platform-macos
Projects
None yet
2 participants