[ddc] Fix issues with duplicate library name aliases.

The attached bug shows an issue users have been encountering where a constructor seems to be undefined. It turns out this is because DDC is trying to read the constructor from the wrong library.

This is happening because both 'package:dio' and a sister package 'package:dio_web_adapter' both contain a library with the same path: 'src/adapter.dart'. 'BrowserHttpClientAdapter' the class they are trying to reference is defined in 'package:dio/src/adapter.dart'. However, due to a naming collision, their import is referencing 'package:dio_web_adapter/src/adapter.dart'.

This naming collision happens because of the logic in '_jsLibraryAlias'. By truncating the start of the import URI (i.e. 'dio/' and 'dio_web_adapter/') the two libraries map to the same alias. This alias is then used to as the key in the AMD module export object and since both libraries are in the same module, only the second one gets exported.

This code may have been written with the assumption that libraries from different packages would always be in different modules (in which case the shortened paths shouldn't collide) but this is not the case. The fix is to use the full import URI including the package name.

In writing the attached modular test I discovered another issue that only affects es6 imports. The ScopedId resolver was not considering NameSpecifier as a declaration point for variables. This lead to a similar name collision since the import alias's name was also being derived from a truncated import URI. In the test, both 'f1/foo.dart' and 'f2/foo.dart' were being imported 'as foo'. Now one is 'as foo' and the other is 'as foo$'.

The first issue affects both AMD and es6 while the second issue only affects es6. The modular tests run with es6 so the new test fails if either of these fixes is not in place.

The new DDC module system is not affected by either issue since it doesn't use NameSpecifiers and it uses the full import URI as a string to register libraries rather than a shortened alias.

Tested on TGP and with a local Flutter application.

Bug: #56498
Change-Id: I5bdb945cfbe615874b40e2fc4ebba31b661cf3b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410260
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Nate Biggs <[email protected]>

Author: natebiggs

pkg/dev_compiler/lib/src/compiler/module_builder.dart

@@ -567,13 +567,31 @@ bool isSdkInternalRuntimeUri(Uri importUri) {
   return importUri.isScheme('dart') && importUri.path == '_runtime';
 }
 
+/// Returns a name that can be used to represent a library within the context
+/// of a module. This name is not globally unique and therefore should not be
+/// used as an import/export name for the library as this can lead to naming
+/// collisions. Use [libraryUriToImportName] to ensure global uniqueness.
+///
+/// The name should be given to a [ScopedId] to ensure there are no local
+/// collisions.
 String libraryUriToJsIdentifier(Uri importUri) {
   if (importUri.isScheme('dart')) {
     return isSdkInternalRuntimeUri(importUri) ? 'dart' : importUri.path;
   }
   return pathToJSIdentifier(p.withoutExtension(importUri.pathSegments.last));
 }
 
+/// Returns a globally unique name that can be used to represent a library.
+/// Since this name is unique, it can safely be used for imports and exports
+/// to/from JS modules. If global uniqueness is not necessary, use
+/// [libraryUriToJsIdentifier] which produces shorter names.
+String libraryUriToImportName(Uri importUri) {
+  if (importUri.isScheme('dart')) {
+    return isSdkInternalRuntimeUri(importUri) ? 'dart' : importUri.path;
+  }
+  return pathToJSIdentifier(p.withoutExtension(importUri.path));
+}
+
 /// Creates function name given [moduleName].
 String loadFunctionName(String moduleName) =>
     'load__${pathToJSIdentifier(moduleName.replaceAll('.', '_'))}';

pkg/dev_compiler/lib/src/js_ast/printer.dart

@@ -1760,6 +1760,8 @@ class MinifyRenamer implements LocalNamer {
 /// Like [BaseVisitor], but calls [declare] for [Identifier] declarations, and
 /// [visitIdentifier] otherwise.
 abstract class VariableDeclarationVisitor extends BaseVisitorVoid {
+  bool _inImportDeclaration = false;
+
   void declare(Identifier node);
 
   @override
@@ -1827,4 +1829,29 @@ abstract class VariableDeclarationVisitor extends BaseVisitorVoid {
       element.accept(this);
     }
   }
+
+  @override
+  void visitImportDeclaration(ImportDeclaration node) {
+    if (node.defaultBinding != null) {
+      declare(node.defaultBinding!);
+    }
+    if (node.namedImports != null) {
+      _inImportDeclaration = true;
+      for (var namedImport in node.namedImports!) {
+        namedImport.accept(this);
+      }
+      _inImportDeclaration = false;
+    }
+  }
+
+  @override
+  void visitNameSpecifier(NameSpecifier node) {
+    final asName = node.asName;
+    // The specified 'as' name only declares a local name in the context of an
+    // import.
+    if (_inImportDeclaration && asName != null) {
+      declare(asName);
+    }
+    node.name?.accept(this);
+  }
 }

pkg/dev_compiler/lib/src/kernel/compiler.dart

@@ -19,14 +19,16 @@ import 'package:kernel/library_index.dart';
 import 'package:kernel/src/dart_type_equivalence.dart';
 import 'package:kernel/type_algebra.dart';
 import 'package:kernel/type_environment.dart';
-import 'package:path/path.dart' as p;
 import 'package:source_span/source_span.dart' show SourceLocation;
 
 import '../command/options.dart' show Options;
 import '../compiler/js_names.dart' as js_ast;
 import '../compiler/js_utils.dart' as js_ast;
 import '../compiler/module_builder.dart'
-    show isSdkInternalRuntimeUri, libraryUriToJsIdentifier;
+    show
+        isSdkInternalRuntimeUri,
+        libraryUriToImportName,
+        libraryUriToJsIdentifier;
 import '../compiler/module_containers.dart' show ModuleItemContainer;
 import '../compiler/rewrite_async.dart';
 import '../js_ast/js_ast.dart' as js_ast;
@@ -818,11 +820,6 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     return module;
   }
 
-  /// Choose a canonical name from the [library] element.
-  String _jsLibraryName(Library library) {
-    return libraryUriToJsIdentifier(library.importUri);
-  }
-
   /// Choose a module-unique name from the [library] element.
   ///
   /// Returns null if no alias exists or there are multiple output paths
@@ -834,17 +831,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     var uri = library.importUri.normalizePath();
     if (uri.isScheme('dart')) return null;
 
-    Iterable<String> segments;
-    if (uri.isScheme('package')) {
-      // Strip the package name.
-      segments = uri.pathSegments.skip(1);
-    } else {
-      segments = uri.pathSegments;
-    }
-
-    var qualifiedPath =
-        js_ast.pathToJSIdentifier(p.withoutExtension(segments.join('/')));
-    return qualifiedPath == _jsLibraryName(library) ? null : qualifiedPath;
+    return libraryUriToImportName(uri);
   }
 
   /// Debugger friendly name for a Dart [library].
@@ -7751,7 +7738,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
   void _setEmitIfIncrementalLibrary(Library library) {
     if (_incrementalMode) {
-      _setEmitIfIncremental(_libraryToModule(library), _jsLibraryName(library));
+      _setEmitIfIncremental(_libraryToModule(library),
+          libraryUriToJsIdentifier(library.importUri));
     }
   }
 
@@ -7994,7 +7982,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       }
       var libraryId = _isBuildingSdk && _isDartLibrary(library, '_rti')
           ? _rtiLibraryId
-          : js_ast.ScopedId(_jsLibraryName(library));
+          : js_ast.ScopedId(libraryUriToJsIdentifier(library.importUri));
 
       _libraries[library] = libraryId;
       var alias = _jsLibraryAlias(library);
@@ -8048,8 +8036,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
     // It's either one of the libraries in this module, or it's an import.
     return _libraries[library] ??
-        _imports.putIfAbsent(
-            library, () => js_ast.ScopedId(_jsLibraryName(library)));
+        _imports.putIfAbsent(library,
+            () => js_ast.ScopedId(libraryUriToJsIdentifier(library.importUri)));
   }
 
   /// Emits imports into [items].
@@ -8080,7 +8068,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         var imports = <js_ast.NameSpecifier>[];
         for (var library in libraries) {
           if (!_incrementalMode ||
-              usedLibraries!.contains(_jsLibraryName(library))) {
+              usedLibraries!
+                  .contains(libraryUriToJsIdentifier(library.importUri))) {
             var alias = _jsLibraryAlias(library);
             if (alias != null) {
               var aliasId = js_ast.ScopedId(alias);
@@ -8180,7 +8169,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
     if (usedLibraries.isNotEmpty) {
       _libraries.forEach((library, libraryId) {
-        if (usedLibraries.contains(_jsLibraryName(library))) {
+        if (usedLibraries
+            .contains(libraryUriToJsIdentifier(library.importUri))) {
           var alias = _jsLibraryAlias(library);
           var aliasId = alias == null ? libraryId : js_ast.ScopedId(alias);
           var asName = alias == null ? null : libraryId;

pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart

@@ -171,7 +171,8 @@ class ExpressionEvaluationTestDriver {
     htmlBootstrapper = testDir.uri.resolve('bootstrapper.html');
     var bootstrapFile = File(htmlBootstrapper.toFilePath())..createSync();
     var moduleName = compiler.metadata!.name;
-    var mainLibraryName = compiler.metadataForLibraryUri(input).name;
+    var mainLibraryName = libraryUriToImportName(
+        Uri.parse(compiler.metadataForLibraryUri(input).importUri));
     var appName = p.relative(
         p.withoutExtension(compiler.metadataForLibraryUri(input).importUri));
 

tests/modular/issue56498/f1/foo.dart

@@ -0,0 +1,7 @@
+// Copyright (c) 2025, 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.
+
+export 'package:f2/foo.dart';
+
+class F1 {}

tests/modular/issue56498/f2/foo.dart

@@ -0,0 +1,5 @@
+// Copyright (c) 2025, 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.
+
+class F2 {}

tests/modular/issue56498/main.dart

@@ -0,0 +1,9 @@
+// Copyright (c) 2025, 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 'package:f1/foo.dart';
+
+main() {
+  print(F1());
+  print(F2());
+}

tests/modular/issue56498/modules.yaml

@@ -0,0 +1,16 @@
+# Copyright (c) 2025, 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.
+#
+# Test ensuring that the modular compiler works properly with `package:`
+# imports. This test also ensures that the dart2js implementation of the modular
+# test pipeline works as intended. The test is not designed to cover any
+# compiler or language feature explicitly.
+dependencies:
+  main: [f1, expect]
+  f1: f2
+
+packages:
+  f0: .
+  f1: f1
+  f2: f2