remove dependencies to analyzer from lib/src/fasta

loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

front_end/src/fasta/testing/* continues to have some shared testing logic,
 - logic to compute the patched sdk is moved up one level
 - TestContext, Run, and suite.dart (which depend on analyzer) moved to pkg/front_end/test/fasta/testing/
 - only a copy of TestContext is duplicated under pkg/kernel/test/closure/.

R=ahe@google.com, paulberry@google.com

Committed: https://github.com/dart-lang/sdk/commit/06189b271a4a0bd87c469c33168279c68051c724

[Siggi Cherem (dart-lang), 2017-04-19 17:21:18]

Description was changed from

==========
(in-progress) 1. move fe/src/fasta/analyzer elsewhere

BUG=
==========

to

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

TestContext and Run mvoed out of front_end/src/fasta/testing/* and some of it is
duplicated now in pkg/front_end/test/fasta/testing/ and
pkg/kernel/test/closure/.
==========

[Siggi Cherem (dart-lang), 2017-04-19 17:23:02]

Description was changed from

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

TestContext and Run mvoed out of front_end/src/fasta/testing/* and some of it is
duplicated now in pkg/front_end/test/fasta/testing/ and
pkg/kernel/test/closure/.
==========

to

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

front_end/src/fasta/testing/* continues to have some shared testing logic,
 - logic to compute the patched sdk is moved up one level
 - TestContext and Run (which dependeds on analyzer) moved to
pkg/front_end/test/fasta/testing/ 
 - a copy of TestContext is duplicated under pkg/kernel/test/closure/.
==========

[Siggi Cherem (dart-lang), 2017-04-19 19:46:51]

Description was changed from

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

front_end/src/fasta/testing/* continues to have some shared testing logic,
 - logic to compute the patched sdk is moved up one level
 - TestContext and Run (which dependeds on analyzer) moved to
pkg/front_end/test/fasta/testing/ 
 - a copy of TestContext is duplicated under pkg/kernel/test/closure/.
==========

to

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

front_end/src/fasta/testing/* continues to have some shared testing logic,
 - logic to compute the patched sdk is moved up one level
 - TestContext, Run, and suite.dart (which depend on analyzer) moved to
pkg/front_end/test/fasta/testing/
 - only a copy of TestContext is duplicated under pkg/kernel/test/closure/.
==========

[Siggi Cherem (dart-lang), 2017-04-19 20:30:23]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2017-04-19 20:30:29]

Patchset #1 (id:20001) has been deleted

[Siggi Cherem (dart-lang), 2017-04-19 20:30:34]

Patchset #1 (id:40001) has been deleted

[Siggi Cherem (dart-lang), 2017-04-19 20:46:17]

sigmund@google.com changed reviewers:
+ ahe@google.com, paulberry@google.com

[Siggi Cherem (dart-lang), 2017-04-19 20:46:18]

more details on the email I sent to the team offline.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
File pkg/front_end/lib/src/fasta/patched_sdk_location.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:7: library
front_end.src.fasta.patched_sdk_location;
I extracted this code out of kernel_chain in the process, probably unnecessary
now that I see where everything settled, but it still feels like a separate
piece.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
File pkg/front_end/test/fasta/testing/kernel_chain.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
pkg/front_end/test/fasta/testing/kernel_chain.dart:5: library
front_end.test.fasta.testing.kernel_chain;
this is the subset from lib/src/fasta/testing/kernel_chain that depends on
analyzer. Once we build platform directly from fasta, we should be able to move
all of this back.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
File pkg/front_end/test/fasta/testing/suite.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
pkg/front_end/test/fasta/testing/suite.dart:5: library fasta.testing.suite;
I just moved this file - it's only used by tests here anyways.

The dependency on analyzer will also go away once we change how we build
platform. At that point I might still be inclined to keep this here in the test
folder.

https://codereview.chromium.org/2828583003/diff/100001/pkg/kernel/test/closur...
File pkg/kernel/test/closures/suite.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/kernel/test/closur...
pkg/kernel/test/closures/suite.dart:118: Uri sdk = await computePatchedSdk();
The changes in this file (and mainly these lines here) are the main duplication
I ended up adding with my refactor.

Let me know what you think, but it seems small enough that I'm inclined to go
for it until we rewrite how to generate the platform file.

[Paul Berry, 2017-04-20 07:36:10]

lgtm

I'm curious to hear what Peter thinks.

[ahe, 2017-04-20 09:24:50]

Can we go over this together in our 1:1?

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
File pkg/front_end/lib/src/fasta/patched_sdk_location.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:7: library
front_end.src.fasta.patched_sdk_location;
On 2017/04/19 20:46:18, Siggi Cherem (dart-lang) wrote:
> I extracted this code out of kernel_chain in the process, probably unnecessary
> now that I see where everything settled, but it still feels like a separate
> piece.

Agreed, but I think it should be moved to the testing subpackage.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:7: library
front_end.src.fasta.patched_sdk_location;
As for the library name, I'd prefer to shorten "front_end.src.fasta" to "fasta".

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:12: import
'environment_variable.dart' show EnvironmentVariable;
We should now be able to move this to testing as well.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
File pkg/front_end/test/fasta/testing.json (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
pkg/front_end/test/fasta/testing.json:54: "/test/fasta/",
We don't need to exclude anything from /test/ anymore.

[ahe, 2017-04-20 09:37:14]

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
File pkg/front_end/lib/src/fasta/run.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/run.dart:5: library fasta.run;
We can probably remove this and use kernel-service instead.

[Siggi Cherem (dart-lang), 2017-04-21 04:21:43]

Patchset #4 (id:120001) has been deleted

[Siggi Cherem (dart-lang), 2017-04-21 04:26:55]

Patchset #4 (id:140001) has been deleted

[Siggi Cherem (dart-lang), 2017-04-21 04:29:23]

PTAL

I did what we talked about offline: I made the tests load the platform.dill file
that is pregenerated by build.py, that gets rid of all dependencies to analyzer
in kernel/closure_test and almost all of them in the fasta testing suite. We
still have one dependency in using the ASTKind of analyzer. I assume we want to
keep that, but if not, we can get rid of the dependency entirely.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
File pkg/front_end/lib/src/fasta/patched_sdk_location.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:7: library
front_end.src.fasta.patched_sdk_location;
On 2017/04/20 09:24:50, ahe wrote:
> As for the library name, I'd prefer to shorten "front_end.src.fasta" to
"fasta".

Done.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:7: library
front_end.src.fasta.patched_sdk_location;
On 2017/04/20 09:24:50, ahe wrote:
> On 2017/04/19 20:46:18, Siggi Cherem (dart-lang) wrote:
> > I extracted this code out of kernel_chain in the process, probably
unnecessary
> > now that I see where everything settled, but it still feels like a separate
> > piece.
> 
> Agreed, but I think it should be moved to the testing subpackage.

Done.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/patched_sdk_location.dart:12: import
'environment_variable.dart' show EnvironmentVariable;
On 2017/04/20 09:24:50, ahe wrote:
> We should now be able to move this to testing as well.

Done.

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
File pkg/front_end/lib/src/fasta/run.dart (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/lib/src/...
pkg/front_end/lib/src/fasta/run.dart:5: library fasta.run;
On 2017/04/20 09:37:14, ahe wrote:
> We can probably remove this and use kernel-service instead.

good to know - I'll tackle this on a separate CL

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
File pkg/front_end/test/fasta/testing.json (right):

https://codereview.chromium.org/2828583003/diff/100001/pkg/front_end/test/fas...
pkg/front_end/test/fasta/testing.json:54: "/test/fasta/",
On 2017/04/20 09:24:50, ahe wrote:
> We don't need to exclude anything from /test/ anymore.

Done. Thanks!

[Siggi Cherem (dart-lang), 2017-04-21 05:30:23]

Patchset #4 (id:160001) has been deleted

[ahe, 2017-04-25 13:55:05]

lgtm

[ahe, 2017-04-25 13:58:05]

On 2017/04/21 04:29:23, Siggi Cherem (dart-lang) wrote:
> I did what we talked about offline: I made the tests load the platform.dill
file
> that is pregenerated by build.py, that gets rid of all dependencies to
analyzer
> in kernel/closure_test and almost all of them in the fasta testing suite. We
> still have one dependency in using the ASTKind of analyzer. I assume we want
to
> keep that, but if not, we can get rid of the dependency entirely.

Can we move AstKind to suite.dart and remove the dependency on the analyzer?

[Siggi Cherem (dart-lang), 2017-04-25 15:59:02]

On 2017/04/25 13:58:05, ahe wrote:
> On 2017/04/21 04:29:23, Siggi Cherem (dart-lang) wrote:
> > I did what we talked about offline: I made the tests load the platform.dill
> file
> > that is pregenerated by build.py, that gets rid of all dependencies to
> analyzer
> > in kernel/closure_test and almost all of them in the fasta testing suite. We
> > still have one dependency in using the ASTKind of analyzer. I assume we want
> to
> > keep that, but if not, we can get rid of the dependency entirely.
> 
> Can we move AstKind to suite.dart and remove the dependency on the analyzer?

Sorry I meant that by using AstKind.Analyzer we have a dependency on the
analyzer target to get "komplile" coverage. The actual definition of AstKind is
in suite.dart though, so there isn't any extra dependency to package:analyzer
from that enum. We could move the kompile tests to the analyzer package and that
might let us get rid of the dev_dependency on package:analyzer in the future. I
don't believe it is worth it at this time, though

[Siggi Cherem (dart-lang), 2017-04-25 20:16:20]

Description was changed from

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

front_end/src/fasta/testing/* continues to have some shared testing logic,
 - logic to compute the patched sdk is moved up one level
 - TestContext, Run, and suite.dart (which depend on analyzer) moved to
pkg/front_end/test/fasta/testing/
 - only a copy of TestContext is duplicated under pkg/kernel/test/closure/.
==========

to

==========
loader, target, diet_listener moved
   from front_end/lib/src/fasta/analyzer/
   to   analyzer/lib/src/fasta/

analyzer_compile and compile_platform_dartk moved
   from front_end/lib/src/fasta/analyzer/
   to   front_end/tool/_fasta/

front_end/src/fasta/testing/* continues to have some shared testing logic,
 - logic to compute the patched sdk is moved up one level
 - TestContext, Run, and suite.dart (which depend on analyzer) moved to
pkg/front_end/test/fasta/testing/
 - only a copy of TestContext is duplicated under pkg/kernel/test/closure/.

R=ahe@google.com, paulberry@google.com

Committed:
https://github.com/dart-lang/sdk/commit/06189b271a4a0bd87c469c33168279c68051c724
==========

[Siggi Cherem (dart-lang), 2017-04-25 20:16:21]

Committed patchset #5 (id:200001) manually as
06189b271a4a0bd87c469c33168279c68051c724 (presubmit successful).