Transition analyzer and analysis_server to new astFactory; remove old AST factory methods.

Transition analyzer and analysis_server to new astFactory; remove old AST factory methods.

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

Committed: https://github.com/dart-lang/sdk/commit/1d028eed8dfa00ac9d4ff685e61b01e6909dbeb4

[Paul Berry, 2016-11-30 19:32:16]

paulberry@google.com changed reviewers:
+ brianwilkerson@google.com, scheglov@google.com

[Brian Wilkerson, 2016-11-30 20:05:31]

Is it really safe to delete the factory methods? I'll be a little surprised if
none of the packages in the SDK make use of those methods.

Other than that concern, lgtm.

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/parser.dart (right):

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/parser.dart:397: return
astFactory.simpleIdentifier(syntheticToken,
I'm surprised by these changes. I guess I was expecting that the code would just
invoke the constructor on the implementation class directly rather than going
through the factory. I don't know that this is wrong, but the extra layer of
indirection seems unnecessary.

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/type_system.dart (right):

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/type_system.dart:587: DartType
tryPromoteToType(DartType to, DartType from) {
This appears to just be sorting changes. Let me know if that's not true.

[scheglov, 2016-11-30 20:08:40]

LGTM

[Paul Berry, 2016-11-30 20:21:42]

On 2016/11/30 20:05:31, Brian Wilkerson wrote:
> Is it really safe to delete the factory methods? I'll be a little surprised if
> none of the packages in the SDK make use of those methods.

I ran the analyzer over the entire SDK using tools/test.py, and found no issues,
so I believe this is safe.

[Paul Berry, 2016-11-30 20:21:57]

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/parser.dart (right):

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/parser.dart:397: return
astFactory.simpleIdentifier(syntheticToken,
On 2016/11/30 20:05:31, Brian Wilkerson wrote:
> I'm surprised by these changes. I guess I was expecting that the code would
just
> invoke the constructor on the implementation class directly rather than going
> through the factory. I don't know that this is wrong, but the extra layer of
> indirection seems unnecessary.

My long term plan is to make the AST factory an injectible dependency by storing
it as a field of the Parser class.  Using the factory here paves the way for
that.

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/type_system.dart (right):

https://codereview.chromium.org/2539243002/diff/20001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/type_system.dart:587: DartType
tryPromoteToType(DartType to, DartType from) {
On 2016/11/30 20:05:31, Brian Wilkerson wrote:
> This appears to just be sorting changes. Let me know if that's not true.

Whoops, you are correct.  Reverted this file to avoid confusion.

[Paul Berry, 2016-11-30 20:28:55]

Description was changed from

==========
Transition analyzer and analysis_server to new astFactory; remove old AST
factory methods.
==========

to

==========
Transition analyzer and analysis_server to new astFactory; remove old AST
factory methods.

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

Committed:
https://github.com/dart-lang/sdk/commit/1d028eed8dfa00ac9d4ff685e61b01e6909dbeb4
==========

[Paul Berry, 2016-11-30 20:28:56]

Committed patchset #2 (id:20001) manually as
1d028eed8dfa00ac9d4ff685e61b01e6909dbeb4 (presubmit successful).