Skip to content

No analyzer strong mode error for void sync* and async* functions #32192

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

Closed
sgrekhov opened this issue Feb 16, 2018 · 6 comments
Closed

No analyzer strong mode error for void sync* and async* functions #32192

sgrekhov opened this issue Feb 16, 2018 · 6 comments
Assignees
Labels
dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-analyzer Use area-devexp instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Milestone

Comments

@sgrekhov
Copy link
Contributor

sgrekhov commented Feb 16, 2018

Consider the following code

void h() sync* { }

main() {
  h();
}

Alanyzer in nostrong mode produces warning
warning - Functions marked 'sync*' must have a return type assignable to 'Iterable'
I'm expecting an error in a strong mode but there is, surprisingly, no issues!

The same is true for async* functions

void h() async* { }

main() {
  h();
}

warning - Functions marked 'async*' must have a return type assignable to 'Stream'
in a nostrong mode and no issues in a strong one

Tested on dartanalyzer version 2.0.0-dev.26.0 on Windows

@eernstg
Copy link
Member

eernstg commented Feb 16, 2018

Note that in 9f9987f we updated the feature specification generalized-void.md to specify the following:

It is a static warning (in Dart 2: a compile-time error) if a function marked
`async*` or `sync*` has return type `void`.

So the declarations of h are compile-time errors specifically because they use void as their return type, not because of a missing assignability relationship.

Assignability has changed because of a different change, though, so that's probably the reason why the strong mode analysis does not flag this declaration: void used to be related to dynamic and the bottom type, only, but in Dart 2 it is a top type (like Object and dynamic), and this means that it is assignable to Iterable today, but wasn't previously.

So the Dart 2 static analysis needs to implement the (newly added) specific rule about void, and these situations will then again be flagged as errors.

@bwilkerson bwilkerson added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed analyzer-strong-mode labels Sep 2, 2018
@bwilkerson bwilkerson added this to the Dart2.1 milestone Sep 2, 2018
@bwilkerson bwilkerson modified the milestones: Dart2.1, PostDart2.1 Sep 4, 2018
@aadilmaan aadilmaan modified the milestones: Future, D25 Release Jun 4, 2019
@srawlins srawlins added the dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec label Jun 17, 2020
@srawlins
Copy link
Member

srawlins commented Jul 2, 2020

@eernstg The text you refer to:

It is a static warning (in Dart 2: a compile-time error) if a function
marked async*, or sync* has return type void.

does not seem to have made it out of the generalized-void into the language spec.

I see that CFE enforces this rule; the analyzer still does not. Should it?

@eernstg
Copy link
Member

eernstg commented Jul 2, 2020

Good catch, thanks!

We do indeed not have any warnings or errors for return type void on a sync* function (or async* for that matter), we only require that the return type is a supertype of Iterable<T> for some T (Stream<T> for async*). I believe this is a mistake, and I created PR dart-lang/language#1060 to fix it.

@srawlins
Copy link
Member

srawlins commented Jul 2, 2020

Thanks much!

@scheglov scheglov self-assigned this Jul 10, 2020
@scheglov
Copy link
Contributor

@eernstg
Copy link
Member

eernstg commented Jul 10, 2020

Thanks! The spec update dart-lang/language#1060 has landed.

dart-bot pushed a commit that referenced this issue Jul 10, 2020
Bug: #32192
Change-Id: Ib31205207def253dc89802898a899b3555ecc28d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153953
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
dart-bot pushed a commit that referenced this issue Jul 11, 2020
Fixes #32192

This has been illegal for some time. This change updates the analyzer
to match the behavior of CFE.

Also move the relevant tests to diagnostics/

Change-Id: I4539aeb65708e2594c52288a6b4584ba7e5455e4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153066
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-analyzer Use area-devexp instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

7 participants