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

[linter] throw synchronously when you return a Future 👿 #59956

Open
stephane-archer opened this issue Jan 22, 2025 · 4 comments
Open

[linter] throw synchronously when you return a Future 👿 #59956

stephane-archer opened this issue Jan 22, 2025 · 4 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal

Comments

@stephane-archer
Copy link

stephane-archer commented Jan 22, 2025

Future<int> foo() {
     throw "my caller will try catch where the future is await 👿";
}
@stephane-archer stephane-archer added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jan 22, 2025
@bwilkerson
Copy link
Member

My understanding is that the example is valid code, but it does require the caller to use onError to catch the exception. Given that, I'm concerned about false positives. If I think of this as a lint focused on 'style' rather than 'correctness' then I suppose it's ok, though I don't know how likely we are to add it.

@bwilkerson bwilkerson added analyzer-linter Issues with the analyzer's support for the linter package linter-lint-proposal labels Jan 22, 2025
@stephane-archer
Copy link
Author

#59639 (comment) by @lrhn

I'll re-open as a reminder to change it to an asynchronrous error.

you have that kind of error in your codebase so I think you should consider seriously this lint

@stephane-archer
Copy link
Author

@bwilkerson From my understanding if you throw synchronously you can only try catch on the function call. I don't think onError is of any use here.

@stephane-archer
Copy link
Author

error should be thrown asynchronrously if you return a future. while it is technically valid to do it synchronously the caller will never expect that and if you throw synchronously in the middle of some async code the behavior of the code become extremely hard to understand.

I think this is 80% of the time a mistake

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal
Projects
None yet
Development

No branches or pull requests

2 participants