Skip to content

Try to make returning void within dynamic an error #32161

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
4 of 5 tasks
MichaelRFairhurst opened this issue Feb 14, 2018 · 7 comments
Closed
4 of 5 tasks

Try to make returning void within dynamic an error #32161

MichaelRFairhurst opened this issue Feb 14, 2018 · 7 comments
Assignees
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on
Milestone

Comments

@MichaelRFairhurst
Copy link
Contributor

MichaelRFairhurst commented Feb 14, 2018

Work to do:

  • investigate if plausible to land via internal code changes
  • land updates internally
  • land (and ship) updates externally
  • land external updates internally
  • enable in analyzer

Split out from #30177

@kevmoo kevmoo added the legacy-area-analyzer Use area-devexp instead. label Feb 14, 2018
MichaelRFairhurst added a commit to MichaelRFairhurst/ansicolor-dart that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which *may* be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/charted that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which *may* be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/cli_util that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/dart2js_info that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/fake_async that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to dart-archive/http2 that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/mustache that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/observable that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to MichaelRFairhurst/usage that referenced this issue Feb 19, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
MichaelRFairhurst added a commit to dart-archive/cli_util that referenced this issue Feb 19, 2018
MichaelRFairhurst added a commit to dart-archive/usage that referenced this issue Feb 20, 2018
jtmcdole added a commit to google/ansicolor-dart that referenced this issue Feb 20, 2018
jakobr-google pushed a commit to dart-archive/http2 that referenced this issue Feb 20, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
harryterkelsen pushed a commit to dart-archive/dart2js_info that referenced this issue Feb 20, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
dart-bot pushed a commit that referenced this issue Feb 22, 2018
Change-Id: Ifee22c00bd9384faef8ee48c1d7fd92879609ce4
Reviewed-on: https://dart-review.googlesource.com/42321
Commit-Queue: Mike Fairhurst <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
dart-bot pushed a commit that referenced this issue Feb 22, 2018
Change-Id: Iea314e7a23513eef1823dbfe8690494f45af9185
Reviewed-on: https://dart-review.googlesource.com/42323
Reviewed-by: Dmitry Stefantsov <[email protected]>
Commit-Queue: Mike Fairhurst <[email protected]>
alorenzen pushed a commit to angulardart/angular that referenced this issue Feb 22, 2018
… function that returns dynamic. dart-lang/sdk#32161

This commonly appears in the form `f() => g()` where `g()` returns void, and `f()` is a top-level or a method, so it gets an implicit dynamic rather than type inference. Even if the restriction is not added to dart 2, this migration creates cleaner code that more often expresses the true intent.

#codehealth

Tested:
    TAP --sample for global presubmit queue
    http://test/OCL:186244698:BASE:186239924:1519083758984:829050d9
PiperOrigin-RevId: 186623288
dart-bot pushed a commit that referenced this issue Feb 22, 2018
Change-Id: Ic986525d61bc36fb5ad25cacb2c9233b75c38bf7
Reviewed-on: https://dart-review.googlesource.com/42322
Commit-Queue: Mike Fairhurst <[email protected]>
Reviewed-by: Dmitry Stefantsov <[email protected]>
alorenzen pushed a commit to angulardart/angular that referenced this issue Feb 23, 2018
… function that returns dynamic. dart-lang/sdk#32161

This commonly appears in the form `f() => g()` where `g()` returns void, and `f()` is a top-level or a method, so it gets an implicit dynamic rather than type inference. Even if the restriction is not added to dart 2, this migration creates cleaner code that more often expresses the true intent.

#codehealth

Tested:
    TAP --sample for global presubmit queue
    http://test/OCL:186244698:BASE:186239924:1519083758984:829050d9
PiperOrigin-RevId: 186623288
alorenzen pushed a commit to angulardart/angular that referenced this issue Feb 24, 2018
… function that returns dynamic. dart-lang/sdk#32161

This commonly appears in the form `f() => g()` where `g()` returns void, and `f()` is a top-level or a method, so it gets an implicit dynamic rather than type inference. Even if the restriction is not added to dart 2, this migration creates cleaner code that more often expresses the true intent.

#codehealth

Tested:
    TAP --sample for global presubmit queue
    http://test/OCL:186244698:BASE:186239924:1519083758984:829050d9
PiperOrigin-RevId: 186623288
alorenzen pushed a commit to angulardart/angular that referenced this issue Feb 24, 2018
… function that returns dynamic. dart-lang/sdk#32161

This commonly appears in the form `f() => g()` where `g()` returns void, and `f()` is a top-level or a method, so it gets an implicit dynamic rather than type inference. Even if the restriction is not added to dart 2, this migration creates cleaner code that more often expresses the true intent.

#codehealth

Tested:
    TAP --sample for global presubmit queue
    http://test/OCL:186244698:BASE:186239924:1519083758984:829050d9
PiperOrigin-RevId: 186623288
alorenzen pushed a commit to angulardart/angular that referenced this issue Feb 24, 2018
… function that returns dynamic. dart-lang/sdk#32161

This commonly appears in the form `f() => g()` where `g()` returns void, and `f()` is a top-level or a method, so it gets an implicit dynamic rather than type inference. Even if the restriction is not added to dart 2, this migration creates cleaner code that more often expresses the true intent.

#codehealth

Tested:
    TAP --sample for global presubmit queue
    http://test/OCL:186244698:BASE:186239924:1519083758984:829050d9
PiperOrigin-RevId: 186623288
matanlurey pushed a commit to angulardart/angular that referenced this issue Feb 25, 2018
… function that returns dynamic. dart-lang/sdk#32161

This commonly appears in the form `f() => g()` where `g()` returns void, and `f()` is a top-level or a method, so it gets an implicit dynamic rather than type inference. Even if the restriction is not added to dart 2, this migration creates cleaner code that more often expresses the true intent.

#codehealth

Tested:
    TAP --sample for global presubmit queue
    http://test/OCL:186244698:BASE:186239924:1519083758984:829050d9
PiperOrigin-RevId: 186623288
psunkari added a commit to google/charted that referenced this issue Feb 26, 2018
MichaelRFairhurst added a commit to google/observable that referenced this issue Feb 27, 2018
@bwilkerson bwilkerson added the P2 A bug or feature request we're likely to work on label Mar 29, 2018
@bwilkerson bwilkerson added this to the Dart2 Beta 4 milestone Mar 29, 2018
@bwilkerson
Copy link
Member

@MichaelRFairhurst How is work on this issue going?

@MichaelRFairhurst
Copy link
Contributor Author

fixes are all landed, but @leafpetersen decided it might not be best for Dart to land this in the analyzer.

@leafpetersen is that decided for sure yet? Can we close this?

@eernstg
Copy link
Member

eernstg commented Apr 3, 2018

The language team has agreed to make one change that would be quite important for this issue: An => function which has return type void and an => function whose returned expression has type void are syntactic sugar for an expansion that does not return anything (so we get the null return value from reaching the end of the function):

void foo() => 42; // Sugar for `void foo() { 42; }`.
foo() => print('Hello!'); // Sugar for `void foo() { print('Hello!'); }`.

I suspect that this decision is sitting in the pipeline (so you might not have had a chance to hear about it yet, and it isn't in any feature spec), but it should change the situation around a substantial number of functions which are "void with =>". The remaining ones (stuff like foo() { ... return print(''); }) would remain an error, so those fixes would still be needed.

I believe that almost all the other fixes (presumably they would be "add an explicit void return type to an => function") would be perfectly fine, though, because they could be claimed to be more readable than the ones that rely on this new way to get a void return type. So I believe there's no need to remove any of the fixes, no matter what.

Of course, the semantics of the targeted => functions changes and all compilers must be changed accordingly, but for the analyzer I guess it might be a relatively small change (it just needs to consider those functions to have return type void).

@MichaelRFairhurst
Copy link
Contributor Author

foo() => print('Hello!'); // Sugar for `void foo() { print('Hello!'); }`.

Is this for top levels or just local closures?

@eernstg
Copy link
Member

eernstg commented Apr 4, 2018

Obviously, we need to specify this, like right now. However, let me try to give an estimated answer. ;-)

In all cases, it's about => functions and only about => functions.

The discussion was specifically about top-level functions, so the rule would apply for them.

For static functions, we might want to consider class members together, in order to keep the number of rules/exceptions low. In isolation, it seems likely that static functions should be able to use this rule, just like top-level functions.

For instance methods there is a clash between omitting parts of the method signature and getting them implicitly from a supertype, and applying this rule when the return type is omitted. So considering void foo() => 42; as an instance method declaration to mean void foo() { 42; } would make sense, but considering bar() => print("Hello!"); to mean void bar() { print("Hello!"); } is problematic, in particular if you would otherwise have "inherited" some T as the return type, and the fact that the "returned expression" is of type void was a mistake. In that case it makes more sense to say "print("Hello!") is not assignable to T" than it does to say "Invalid override of return type T with return type void".

For local functions there is no ambiguity: They already get an inferred return type if the return type is omitted. The reason why this is possible is that they have no dependency cycles, so the complexity cannot explode.

Function literals have inference, too.

So we need to carefully consider the locations where this rule is applied, starting with the safe bet which is the top level.

@MichaelRFairhurst
Copy link
Contributor Author

Closing, we can have a new ticket made once the change is specced, and that may be after the front end/analyzer integration anyways. Sound good @eernstg ?

@eernstg
Copy link
Member

eernstg commented May 8, 2018

Sounds good @MichaelRFairhurst, thanks!

Specification work is handled via #33069.

xxgreg pushed a commit to xxgreg/mustache that referenced this issue Jun 3, 2018
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
mosuem pushed a commit to dart-lang/http that referenced this issue Oct 17, 2024
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
mosuem pushed a commit to dart-lang/tools that referenced this issue Oct 25, 2024
Add void declarations to methods with implicit dynamic returning void
values, which may be illegal in dart 2, but in either case, expresses
the current intent better.
mosuem pushed a commit to dart-lang/tools that referenced this issue Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

4 participants