fixes detection of SDK libraries

fixes detection of SDK libraries

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/5746aaaa4808b9ef1f38b519e76a2621f1e1a4bb

[Jennifer Messerly, 2015-06-09 23:31:47]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[Jennifer Messerly, 2015-06-10 14:31:18]

Update: figured out why this CL "worked" and concluded _interceptors was not
loaded. Most of the private libraries are not considered part of the SDK (e.g.
LibraryElement.isInSdk -> false), because Analyzer just looks if the name starts
with "dart." ... aside from the issue here, it seems risky that any old library
can pretend to be part of the SDK. I'm going to see if I can change our logic to
use their Source URI instead.

[Jennifer Messerly, 2015-06-10 16:33:04]

PTAL. This may not fix your original problem anymore, but at least our compiler
knows what the SDK libraries actually are now :)

Fixes:
* was not validating the URI scheme is actually 'dart:'
* library names did not match corelibOrder, and naming of private libs was
inconsistent

[vsm, 2015-06-10 16:41:16]

lgtm

https://codereview.chromium.org/1170813008/diff/80001/lib/devc.dart
File lib/devc.dart (right):

https://codereview.chromium.org/1170813008/diff/80001/lib/devc.dart#newcode158
lib/devc.dart:158: if (!options.checkSdk && lib.source.uri.scheme == 'dart')
return;
Move _isDartUri to utils and call that instead?

https://codereview.chromium.org/1170813008/diff/80001/lib/src/codegen/js_modu...
File lib/src/codegen/js_module_item_order.dart (right):

https://codereview.chromium.org/1170813008/diff/80001/lib/src/codegen/js_modu...
lib/src/codegen/js_module_item_order.dart:234: static bool
_isDartUri(LibraryElement e) => e.source.uri.scheme == 'dart';
Do you want to assert e.isInSdk == _isDartUri(e)?  It looks like you also fixed
the names.

[Jennifer Messerly, 2015-06-10 16:46:04]

https://codereview.chromium.org/1170813008/diff/80001/lib/devc.dart
File lib/devc.dart (right):

https://codereview.chromium.org/1170813008/diff/80001/lib/devc.dart#newcode158
lib/devc.dart:158: if (!options.checkSdk && lib.source.uri.scheme == 'dart')
return;
On 2015/06/10 16:41:15, vsm wrote:
> Move _isDartUri to utils and call that instead?

funny, almost did that. The thing is, now it becomes an opaque call, and instead
of a simple isInSdk, you have to import 'utils.dart' just do a URI scheme check.
Overall, I'd prefer to keep it as is ... it's kind of my informal rule of
"straightforward check of a single property" that should be inlined.

(Contrast with isInlineJS, that needs to check 3 things, two of them unrelated)

https://codereview.chromium.org/1170813008/diff/80001/lib/src/codegen/js_modu...
File lib/src/codegen/js_module_item_order.dart (right):

https://codereview.chromium.org/1170813008/diff/80001/lib/src/codegen/js_modu...
lib/src/codegen/js_module_item_order.dart:234: static bool
_isDartUri(LibraryElement e) => e.source.uri.scheme == 'dart';
On 2015/06/10 16:41:15, vsm wrote:
> Do you want to assert e.isInSdk == _isDartUri(e)?  It looks like you also
fixed
> the names.

We can't assert it, because user can name their library "dart.my.awesome.lib".
Unless we intend to make that an error? Doesn't seem worth it to me.

[Jennifer Messerly, 2015-06-10 16:59:22]

Committed patchset #5 (id:80001) manually as
5746aaaa4808b9ef1f38b519e76a2621f1e1a4bb (presubmit successful).