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

'BuildContext's across async gaps false positive? #4955

Closed
stephane-archer opened this issue Apr 24, 2024 · 8 comments
Closed

'BuildContext's across async gaps false positive? #4955

stephane-archer opened this issue Apr 24, 2024 · 8 comments
Assignees
Labels
false-positive P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@stephane-archer
Copy link

                      if (context.mounted) {
                        ScaffoldMessenger.of(context).showSnackBar(snackBar);
                      }

ok but:

                      if (context.mounted == false) {
                        return;
                      }
                      ScaffoldMessenger.of(context).showSnackBar(snackBar);

Don't use 'BuildContext's across async gaps. Try rewriting the code to not use the 'BuildContext', or guard the use with a 'mounted' check.

@srawlins
Copy link
Member

A possible gap in the rule! I think this doesn't come up because it is more idiomatic to write if (!context.mounted).

@srawlins
Copy link
Member

Actually I cannot reproduce this at HEAD. Can you say what version of Dart/Flutter you are using, and provide a more full example (just the full function body there should be sufficient).

@srawlins
Copy link
Member

Oops, apologies. I can reproduce. :D

@pq pq added false-positive P2 A bug or feature request we're likely to work on labels Apr 24, 2024
@stephane-archer
Copy link
Author

@srawlins no worries
The idiomatic way would fail silently or need an else close to handle the error which is quite verbose
I'm not sure why it's the idiomatic way :)

if (context.mounted == false) {
                        throw "I don't fail silently!";
 }
ScaffoldMessenger.of(context).showSnackBar(snackBar);

@srawlins
Copy link
Member

Hmm, I might have not been clear. I mean that the idiomatic way to write context.mounted == false is !context.mounted.

@stephane-archer
Copy link
Author

@srawlins ho I see, I misunderstood what you meant, sorry.
I hope to allow context.mounted == false would be easy to add.

@srawlins srawlins self-assigned this Apr 25, 2024
@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Apr 25, 2024
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Apr 29, 2024
Fixes dart-lang/linter#4955

This also moves the implementation of
`LinterContext.evaluateConstant()` to an extension getter on
Expression, `computeConstantValue`, as the implementation does not
need any information from LinterContext. An Expression can be
evaluated with just the data it has on its own fields. This helps
to simplify lint rules so that they may not need to pass around a
LinterContext object around, like in use_build_context_synchronously,
which doesn't use one currently.

Change-Id: I555ae32f57de0968fd30b051e9a816dba8005574
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364521
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
@srawlins
Copy link
Member

Fixed with dart-lang/sdk@d728ec3

@stephane-archer
Copy link
Author

@srawlins you are the best 🤯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants