Qualify library names in packages

lib/src/compiler/code_generator.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:collection' show HashMap, HashSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/ast.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/token.dart' show Token, TokenType;
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/ast/token.dart' show StringToken;
+
+// TODO(vsm): Delete
+import 'package:analyzer/src/context/source.dart' show SourceFactoryImpl;
+
 //TODO(leafp): Remove deprecated dependency
 //ignore: DEPRECATED_MEMBER_USE
 import 'package:analyzer/src/generated/element.dart'
     show DynamicTypeImpl, LocalVariableElementImpl;
 import 'package:analyzer/src/generated/engine.dart' show AnalysisContext;
 import 'package:analyzer/src/generated/resolver.dart'
     show TypeProvider, NamespaceBuilder;
 import 'package:analyzer/src/generated/type_system.dart'
     show StrongTypeSystemImpl;
 import 'package:analyzer/src/summary/summarize_elements.dart'
@@ skipping 160 matching lines @@
     compilationUnits = CoercionReifier.reify(compilationUnits);
 
     // Initialize our library variables.
     var items = <JS.ModuleItem>[];
     for (var unit in compilationUnits) {
       var library = unit.element.library;
       if (unit.element != library.definingCompilationUnit) continue;
 
       var libraryTemp = _isDartRuntime(library)
           ? _runtimeLibVar
-          : new JS.TemporaryId(jsLibraryName(library));
+          : new JS.TemporaryId(jsLibraryName(context, library));
       _libraries[library] = libraryTemp;
       items.add(new JS.ExportDeclaration(
           js.call('const # = Object.create(null)', [libraryTemp])));
 
       // dart:_runtime has a magic module that holds extenstion method symbols.
       // TODO(jmesserly): find a cleaner design for this.
       if (_isDartRuntime(library)) {
         items.add(new JS.ExportDeclaration(
             js.call('const # = Object.create(null)', [_dartxVar])));
       }
@@ skipping 3473 matching lines @@
       node.externalKeyword != null || _functionBody(node) is NativeFunctionBody;
 
   FunctionBody _functionBody(node) =>
       node is FunctionDeclaration ? node.functionExpression.body : node.body;
 
   /// Returns the canonical name to refer to the Dart library.
   JS.Identifier emitLibraryName(LibraryElement library) {
     // It's either one of the libraries in this module, or it's an import.
     return _libraries[library] ??
         _imports.putIfAbsent(
-            library, () => new JS.TemporaryId(jsLibraryName(library)));
+            library, () => new JS.TemporaryId(jsLibraryName(context, library)));
   }
 
   JS.Node annotate(JS.Node node, AstNode original, [Element element]) {
     if (options.closure && element != null) {
       node = node.withClosureAnnotation(closureAnnotationFor(
           node, original, element, namedArgumentTemp.name));
     }
     return node..sourceInformation = original;
   }
 
@@ skipping 55 matching lines @@
       // TODO(leafp): The above only handles the case where the return type
       // is exactly Future/Stream/Iterable.  Handle the subtype case.
       return DynamicTypeImpl.instance;
     }
   }
 }
 
 /// Choose a canonical name from the library element.
 /// This never uses the library's name (the identifier in the `library`
 /// declaration) as it doesn't have any meaningful rules enforced.
-String jsLibraryName(LibraryElement library) {
+String jsLibraryName(AnalysisContext context, LibraryElement library) {
+  var sourceFactory = context.sourceFactory as SourceFactoryImpl;

[vsm, 2016/04/27 18:24:35]

sourceFactory has it's own restoreAbsolute that walks the resolvers in order -
but it's preferring the file resolver to the package resolver.

[Jennifer Messerly, 2016/04/27 18:30:05]

Oh crazy! Is that hard coded? You could change the order we pass the resolvers
in. (I would guess we already pass in package resolver first though.... code is
in lib/src/analyzer/context.dart in DDC)

[Paul Berry, 2016/04/27 19:19:57]

Are you sure about this?  Looking at the definition in
package:analyzer/src/context/source.dart, it looks like it just visits the
resolvers in order.

[vsm, 2016/04/27 20:56:08]

yep, just need to set in the right order!  will fix and get rid of the unneeded
import.

+  for (var resolver in sourceFactory.resolvers) {
+    var uri = resolver.restoreAbsolute(library.source);

[Paul Berry, 2016/04/27 19:19:57]

Why not just:

var uri = library.source.uri;

This retrieves the URI that was used to find the package when it was first
resolved, which I think gives you the information you want and saves a lot of
legwork.

[Jennifer Messerly, 2016/04/27 19:24:21]

ah, thanks, I wondered that too! Figured maybe I was missing something.

Yeah in my experience, library.source.uri always has the right thing. :)

[vsm, 2016/04/27 20:56:08]

that should actually work for the bazel use case.  according to siggi, we'll
always be given a package:uri for anything that could be a package uri.

it is handy though for compiling from the command line - i can glob in a list of
files under packages/collection rather than listing each explicitly.

i can remove from this cl though if y'all would prefer.

[Paul Berry, 2016/04/27 21:13:49]

Ah, fair point.  Ok, I see why this is a use case that is worth supporting.  And
I see why in order to support it we need to implement the multi-package
resolver's restoreAbsolute() method.  However, I still believe we shouldn't be
calling that method from here (nor should we be calling
sourceFactory.restoreUri() from here).  Instead, we need to ensure that
library.source.uri is correct before passing the sources into the analyzer in
the first place (otherwise things will go wrong elsewhere in the analyzer).

For an example of how we do this in analyzer_cli, see
https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_cli/lib/src/driver.....
 Note that we create the Source object, then use SourceFactory.restoreUri() to
see if a better URI can be found for it, and if so, we recreate the source based
on the better URI.  This is all done prior to feeding the sources into the
context, so the analyzer always sees correct URIs.

It's particularly important that we get this right for DDC, because DDC outputs
a summary, and the URIs contained in the summary are the URIs from
library.source.uri.  So if those URIs aren't correct, we'll run into trouble
when someone tries to use the summary as a dependency of another package.

[Jennifer Messerly, 2016/04/27 21:52:51]

yeah +1 to all that

@Vijay -- the DDC spot to fix this is probably
https://github.com/dart-lang/dev_compiler/blob/ec5180eb173c09b62109195025bc98...

that code there to map file relative paths to absolute file URIs never made me
happy so this may fix it :)

+    if (uri?.scheme == 'package') {
+      // Strip the package name.
+      // TODO(vsm): This is not unique if an escaped '/'appears in a filename.
+      // E.g., "foo/bar.dart" and "foo$47bar.dart" would collide.
+      var packageRelativePath = uri.pathSegments.skip(1).join('/');
+      return pathToJSIdentifier(packageRelativePath);
+    }
+  }
+  // TODO(vsm): This is not unique, but we're not in
+  // a package.  We need a mechanism for creating a unique library
+  // name.
   return pathToJSIdentifier(library.source.uri.pathSegments.last);
 }
 
 /// Shorthand for identifier-like property names.
 /// For now, we emit them as strings and the printer restores them to
 /// identifiers if it can.
 // TODO(jmesserly): avoid the round tripping through quoted form.
 JS.LiteralString _propertyName(String name) => js.string(name, "'");
 
 // TODO(jacobr): we would like to do something like the following
@@ skipping 14 matching lines @@
 }
 
 bool isLibraryPrefix(Expression node) =>
     node is SimpleIdentifier && node.staticElement is PrefixElement;
 
 LibraryElement _getLibrary(AnalysisContext c, String uri) =>
     c.computeLibraryElement(c.sourceFactory.forUri(uri));
 
 bool _isDartRuntime(LibraryElement l) =>
     l.isInSdk && l.source.uri.toString() == 'dart:_runtime';