Skip to content

Dartdoc crashes on "Future<void> foo()" #1625

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
whesse opened this issue Mar 5, 2018 · 4 comments
Closed

Dartdoc crashes on "Future<void> foo()" #1625

whesse opened this issue Mar 5, 2018 · 4 comments
Assignees
Labels
P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@whesse
Copy link
Contributor

whesse commented Mar 5, 2018

The Dart language now allows Future<void>, and the type of dart:io RandomAccessFile.close() has been changed to that in commit
44aa9a17fd8ca5278dfe12590ad48212fefda4dd

Future<void> close(); instead of
Future<RandomAccessFile> close();

This causes a crash in line 176 of src/element_type.dart, where f.element.library is tested against null, to special-case A<dynamic>. But with A<void>, f.element is itself null. If we change to f.element?.library != null, we get a crash in the call to ModelElement.from(f.element, lib), again, where there is special casing for dynamic, but not for void.

Flutter code is being changed to use Future<void>, and this IO change should also land, so we need to handle this case.

Reverting 44aa9a17fd8ca5278dfe12590ad48212fefda4dd until this is fixed and rolled to Dart SDK

The builder with the failure is:
https://ci.chromium.org/buildbot/client.dart/dart-sdk-linux-be/19230

@jcollins-g
@jcollins-g: edited to escape types

@whesse whesse added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) P0 A serious issue requiring immediate resolution Type: bug labels Mar 5, 2018
dart-bot pushed a commit to dart-lang/sdk that referenced this issue Mar 5, 2018
This reverts commit 44aa9a1.

Reason for revert: The dartdoc API documentation generator cannot handle Future<void> yet.  This is filed as a P0 issue, and the change should be relanded when it is fixed: dart-lang/dartdoc#1625

Original change's description:
> Do not return this on RandomAccessFile.close
> 
> Bug: 32015
> Change-Id: I98508bdad569201afeed91f1287f061b5bb39a31
> Reviewed-on: https://dart-review.googlesource.com/44060
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
> Commit-Queue: Vyacheslav Egorov <vegorov@google.com>

TBR=lrn@google.com,vegorov@google.com,zra@google.com,sigmund@google.com,lexa.knyazev@gmail.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 32015
Change-Id: I658b997cdc62e23cfb4fec8921e19239d534e272
Reviewed-on: https://dart-review.googlesource.com/44980
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: William Hesse <whesse@google.com>
@jcollins-g jcollins-g self-assigned this Mar 5, 2018
@jcollins-g
Copy link
Contributor

f.element is null because VoidTypeImpls don't have elements, which is by design. Types without elements aren't known to dartdoc yet and I'll have to wire this in. May take a bit to turn around as there are a lot of assumptions in dartdoc that any type has an element.

@jcollins-g
Copy link
Contributor

jcollins-g commented Mar 5, 2018

Hacking it up so it doesn't crash for Future<void> in this one instance is fairly straightforward. However, the general case of handling void as a type parameter looks to be suprisingly hard. Dartdoc's entire type resolution system assumes that all valid types from the analyzer have non-null elements (and therefore, can have ModelElements associated with them). This will have to be ripped out.

@isoos
Copy link
Contributor

isoos commented Mar 13, 2018

Is there any estimate on the fix (or at least a partial fix)? url_launcher is a popular Flutter package, it would be nice to generate docs for it...

@jcollins-g
Copy link
Contributor

jcollins-g commented Mar 13, 2018

@isoos #1629 should fix those issues and it's just gotten through review. Hopefully I'll be able to test and publish a package with it today, but if not, tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants