assert isAsynchronous and isGenerator false if parseAsync is false

assert isAsynchronous and isGenerator false if parseAsync is false
and fix ASYNC_NOT_SUPPORTED error code

R=brianwilkerson@google.com, scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/559fb01244e3fd05a171d1506dab224e59e647bb

[danrubel, 2016-01-13 21:24:06]

danrubel@google.com changed reviewers:
+ scheglov@google.com

[Brian Wilkerson, 2016-01-13 21:38:01]

brianwilkerson@google.com changed reviewers:
+ brianwilkerson@google.com

[Brian Wilkerson, 2016-01-13 21:38:01]

https://codereview.chromium.org/1584823003/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/parser.dart (right):

https://codereview.chromium.org/1584823003/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/parser.dart:6104: keyword = null;
I don't think we want to make these changes to the parser (other than the error
code fix). As a general principle we capture the actual content of the file even
if it's invalid. More specifically, there are UX implications:
 - If we've already told the user that async isn't supported, then that should
be enough. Do we really want them to get an error every time they used an async
'for', 'await' or 'yield'?
 - If we have quick assists to help them convert their code it might be harder
to implement correctly if we've lost information.

Is there a problem that you're seeing that this fixes?

[danrubel, 2016-01-13 22:02:20]

https://codereview.chromium.org/1584823003/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/parser.dart (right):

https://codereview.chromium.org/1584823003/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/parser.dart:6104: keyword = null;
On 2016/01/13 21:38:01, Brian Wilkerson wrote:
> I don't think we want to make these changes to the parser (other than the
error
> code fix). As a general principle we capture the actual content of the file
even
> if it's invalid. More specifically, there are UX implications:
>  - If we've already told the user that async isn't supported, then that should
> be enough. Do we really want them to get an error every time they used an
async
> 'for', 'await' or 'yield'?
>  - If we have quick assists to help them convert their code it might be harder
> to implement correctly if we've lost information.
> 
> Is there a problem that you're seeing that this fixes?

Yes. There are a number of places in the resolver and elsewhere that first check
isAsynchronous and then access resolver.futureType, resolver.streamType, etc.
These will all be null if there is no dart:async library. This seemed the best
way to fix resolution when async is not supported.

An alternate approach is to revert this change and have the resolver define
placeholder/mock declarations and elements for Future and Stream when dart:async
is not available. Would that be a better approach?

[danrubel, 2016-01-14 14:55:55]

On 2016/01/13 22:02:20, danrubel wrote:
>
https://codereview.chromium.org/1584823003/diff/1/pkg/analyzer/lib/src/genera...
> File pkg/analyzer/lib/src/generated/parser.dart (right):
> 
>
https://codereview.chromium.org/1584823003/diff/1/pkg/analyzer/lib/src/genera...
> pkg/analyzer/lib/src/generated/parser.dart:6104: keyword = null;
> On 2016/01/13 21:38:01, Brian Wilkerson wrote:
> > I don't think we want to make these changes to the parser (other than the
> error
> > code fix). As a general principle we capture the actual content of the file
> even
> > if it's invalid. More specifically, there are UX implications:
> >  - If we've already told the user that async isn't supported, then that
should
> > be enough. Do we really want them to get an error every time they used an
> async
> > 'for', 'await' or 'yield'?
> >  - If we have quick assists to help them convert their code it might be
harder
> > to implement correctly if we've lost information.
> > 
> > Is there a problem that you're seeing that this fixes?
> 
> Yes. There are a number of places in the resolver and elsewhere that first
check
> isAsynchronous and then access resolver.futureType, resolver.streamType, etc.
> These will all be null if there is no dart:async library. This seemed the best
> way to fix resolution when async is not supported.
> 
> An alternate approach is to revert this change and have the resolver define
> placeholder/mock declarations and elements for Future and Stream when
dart:async
> is not available. Would that be a better approach?

I've updated this CL per discussion and will address resolver issues via mocks
in a separate CL.

PTAL.

[Brian Wilkerson, 2016-01-14 15:08:33]

LGTM

[scheglov, 2016-01-14 16:09:12]

LGTM

[danrubel, 2016-01-14 19:52:26]

Description was changed from

==========
assert isAsynchronous and isGenerator false if parseAsync is false
and fix ASYNC_NOT_SUPPORTED error code
==========

to

==========
assert isAsynchronous and isGenerator false if parseAsync is false
and fix ASYNC_NOT_SUPPORTED error code

R=brianwilkerson@google.com, scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/559fb01244e3fd05a171d1506dab224e59e647bb
==========

[danrubel, 2016-01-14 19:52:27]

Committed patchset #3 (id:40001) manually as
559fb01244e3fd05a171d1506dab224e59e647bb (presubmit successful).