Package map processing abstraction.

Package map processing abstraction.

Babysteps towards a cleaner context creation story.

Comments welcome!

BUG=
R=brianwilkerson@google.com

Committed: https://github.com/dart-lang/sdk/commit/995a19b26f3514877e8938e82f114d84ad1c7bc8

[pquitslund, 2016-06-07 19:58:15]

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

[Brian Wilkerson, 2016-06-07 20:48:49]

Other than the lack of tests, lgtm.

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/lib/src/contex...
File pkg/analyzer/lib/src/context/context_factory.dart (right):

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/lib/src/contex...
pkg/analyzer/lib/src/context/context_factory.dart:44: for (String uri in
extendedLibraries.uris) {
It would be more efficient to add the extensions to the map first, then override
them from the embedded libraries.

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/context_factory_test.dart (right):

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/context_factory_test.dart:17:
group('PackageMapProcessor', () {
There are a lot of cases not tested here.
- What if the package map is empty?
- What if the package map doesn't have a package with an embedder file?
- What is there is no package with an ext?
- What if we don't find either one?
- What happens if one or both of those files are invalid?

Those are just the ones that come to mind quickly, you can probably think of
others.

(Also, it would be really nice to use reflectiveTest because it's much easier to
find the tests when there's a failure.)

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/context_factory_test.dart:50:
expect(proc.extendedLibraries.size(), 1);
We should test that the library map has the right mapping for at least one
library from each source.

[pquitslund, 2016-06-07 20:59:58]

Thanks!  Tests to follow.

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/lib/src/contex...
File pkg/analyzer/lib/src/context/context_factory.dart (right):

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/lib/src/contex...
pkg/analyzer/lib/src/context/context_factory.dart:44: for (String uri in
extendedLibraries.uris) {
On 2016/06/07 20:48:48, Brian Wilkerson wrote:
> It would be more efficient to add the extensions to the map first, then
override
> them from the embedded libraries.

Done.

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/test/src/conte...
File pkg/analyzer/test/src/context/context_factory_test.dart (right):

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/context_factory_test.dart:17:
group('PackageMapProcessor', () {
On 2016/06/07 20:48:48, Brian Wilkerson wrote:
> There are a lot of cases not tested here.
> - What if the package map is empty?
> - What if the package map doesn't have a package with an embedder file?
> - What is there is no package with an ext?
> - What if we don't find either one?
> - What happens if one or both of those files are invalid?
> 
> Those are just the ones that come to mind quickly, you can probably think of
> others.
> 
> (Also, it would be really nice to use reflectiveTest because it's much easier
to
> find the tests when there's a failure.)

Your wish is my command. :)  Done!

https://codereview.chromium.org/2043963004/diff/1/pkg/analyzer/test/src/conte...
pkg/analyzer/test/src/context/context_factory_test.dart:50:
expect(proc.extendedLibraries.size(), 1);
On 2016/06/07 20:48:48, Brian Wilkerson wrote:
> We should test that the library map has the right mapping for at least one
> library from each source.

Agreed.  I'll follow up with a test-full follow up CL.

[pquitslund, 2016-06-07 21:01:21]

Description was changed from

==========
Package map processing abstraction.

Babysteps towards a cleaner context creation story.

Comments welcome!

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

to

==========
Package map processing abstraction.

Babysteps towards a cleaner context creation story.

Comments welcome!

BUG=
R=brianwilkerson@google.com

Committed:
https://github.com/dart-lang/sdk/commit/995a19b26f3514877e8938e82f114d84ad1c7bc8
==========

[pquitslund, 2016-06-07 21:01:24]

Committed patchset #1 (id:1) manually as
995a19b26f3514877e8938e82f114d84ad1c7bc8 (presubmit successful).