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

Null-assert the value given to a Completer expecting a non-null value #49053

Merged
merged 11 commits into from
Feb 28, 2024

Conversation

srawlins
Copy link
Contributor

@srawlins srawlins commented Dec 14, 2023

The code with out the null assertion is legal as per the type signature, but will throw a runtime exception if the nullable value is null. To make this exception more explicit, the value must be null-checked before completing the completer with the value.

The analyzer will soon enforce such checks. See dart-lang/sdk#53253.

This PR is behaviorally a no-op.

Fixes flutter/flutter#136775

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 Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

The code with out the null assertion is legal as per the type signature, but will throw a runtime exception if the nullable value is null. To make this exception more explicit, the value must be null-checked before completing the completer with the value.

The analyzer will soon enforce such checks. See dart-lang/sdk#53253.

This PR is behaviorally a no-op.

Fixes flutter/flutter#136775
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Dec 14, 2023
@srawlins
Copy link
Contributor Author

👋

@srawlins
Copy link
Contributor Author

🕺

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@srawlins srawlins merged commit 5c09604 into main Feb 28, 2024
25 checks passed
@srawlins srawlins deleted the srawlins-patch-1 branch February 28, 2024 01:27
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 28, 2024
8acc96d405 Roll Dart SDK from 1e6b559fae88 to 44af84628b90 (1 revision) (flutter/engine#51035)
632f9d742d Roll Skia from 84c60d70fbbd to a673ab63fccc (1 revision) (flutter/engine#51034)
5c09604bd6 Null-assert the value given to a Completer expecting a non-null value (flutter/engine#49053)
055ede728c Roll Dart SDK from a0e3c16eea3e to 1e6b559fae88 (1 revision) (flutter/engine#51025)
d6bca2a382 [Windows] Reduce log level of unsupported accessibility event message (flutter/engine#51024)
8fa1141b46 Roll Skia from c8703c91675e to 84c60d70fbbd (4 revisions) (flutter/engine#51021)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlutterVmService completes a non-nullable Completer with a nullable value
2 participants