Version 3.7.0-244.0.dev

Merge 2616eb2 into dev

pkg/analysis_server/analyzer_use_new_elements.txt

@@ -26,7 +26,6 @@ lib/src/services/search/element_visitors.dart
 lib/src/services/search/search_engine.dart
 lib/src/services/search/search_engine_internal.dart
 test/abstract_single_unit.dart
-test/services/correction/status_test.dart
 test/services/refactoring/legacy/abstract_rename.dart
 test/services/refactoring/legacy/rename_import_test.dart
 test/services/search/element_visitors_test.dart

pkg/analysis_server/lib/src/services/search/search_engine_internal.dart

@@ -293,10 +293,10 @@ class SearchMatchImpl implements SearchMatch {
   final Source unitSource;
 
   @override
-  final LibraryElement libraryElement;
+  final LibraryElement2 libraryElement2;
 
   @override
-  final Element element;
+  final Element2 element2;
 
   @override
   final bool isResolved;
@@ -314,19 +314,19 @@ class SearchMatchImpl implements SearchMatch {
     this.file,
     this.librarySource,
     this.unitSource,
-    this.libraryElement,
-    this.element,
+    this.libraryElement2,
+    this.element2,
     this.isResolved,
     this.isQualified,
     this.kind,
     this.sourceRange,
   );
 
   @override
-  Element2 get element2 => element.asElement2!;
+  Element get element => element2.asElement!;
 
   @override
-  LibraryElement2 get libraryElement2 => libraryElement.asElement2;
+  LibraryElement get libraryElement => libraryElement2.asElement;
 
   @override
   String toString() {
@@ -352,8 +352,8 @@ class SearchMatchImpl implements SearchMatch {
       element.source!.fullName,
       element.librarySource!,
       element.source!,
-      element.library!,
-      element,
+      element.library!.asElement2,
+      element.asElement2!,
       true,
       true,
       MatchKind.DECLARATION,
@@ -367,8 +367,8 @@ class SearchMatchImpl implements SearchMatch {
       enclosingElement.source!.fullName,
       enclosingElement.librarySource!,
       enclosingElement.source!,
-      enclosingElement.library!,
-      enclosingElement,
+      enclosingElement.library!.asElement2,
+      enclosingElement.asElement2!,
       result.isResolved,
       result.isQualified,
       toMatchKind(result.kind),

pkg/analysis_server/test/services/correction/status_test.dart

@@ -24,9 +24,9 @@ void main() {
 class RefactoringLocationTest extends AbstractSingleUnitTest {
   Future<void> test_createLocation_forElement() async {
     await resolveTestCode('class MyClass {}');
-    var element = findElement.class_('MyClass');
+    var element = findElement2.class_('MyClass');
     // check
-    var location = newLocation_fromElement(element)!;
+    var location = newLocation_fromElement2(element)!;
     expect(location.file, testFile.path);
     expect(location.offset, 6);
     expect(location.length, 7);
@@ -36,13 +36,15 @@ class RefactoringLocationTest extends AbstractSingleUnitTest {
 
   Future<void> test_createLocation_forMatch() async {
     await resolveTestCode('class MyClass {}');
-    var element = findElement.class_('MyClass');
-    var sourceRange = range.elementName(element);
+    var element = findElement2.class_('MyClass');
+    var firstFragment = element.firstFragment;
+    var libraryFragment = firstFragment.libraryFragment;
+    var sourceRange = range.fragmentName(firstFragment)!;
     SearchMatch match = SearchMatchImpl(
-      element.source.fullName,
-      element.library.source,
-      element.source,
-      element.library,
+      libraryFragment.source.fullName,
+      element.library2.firstFragment.source,
+      libraryFragment.source,
+      element.library2,
       element,
       true,
       false,

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

@@ -24,31 +24,31 @@ abstract class FixedNames {
 }
 
 /// Unique instance for temporary variables. Will be renamed consistently
-/// across the entire file. Different instances will be named differently
-/// even if they have the same name, this makes it safe to use in code
-/// generation without needing global knowledge. See [TemporaryNamer].
+/// across the entire file. Instances with different IDs will be named
+/// differently even if they have the same name, this makes it safe to use in
+/// code generation without needing global knowledge. See [TemporaryNamer].
 // TODO(jmesserly): move into js_ast? add a boolean to Identifier?
 class TemporaryId extends Identifier {
-  // TODO(jmesserly): by design, temporary identifier nodes are shared
-  // throughout the AST, so any source information we attach in one location
-  // be incorrect for another location (and overwrites previous data).
-  //
-  // If we want to track source information for temporary variables, we'll
-  // need to separate the identity of the variable from its Identifier.
-  //
-  // In practice that makes temporaries more difficult to use: they're no longer
-  // JS AST nodes, so `toIdentifier()` is required to put them in the JS AST.
-  // And anywhere we currently use type `Identifier` to hold Identifier or
-  // TemporaryId, those types would need to change to `Identifier Function()`.
-  //
-  // However we may need to fix this if we want hover to work well for things
-  // like library prefixes and field-initializing formals.
+  final int _id;
+
   @override
-  dynamic get sourceInformation => null;
+  TemporaryId withSourceInformation(dynamic sourceInformation) =>
+      TemporaryId.from(this)..sourceInformation = sourceInformation;
+
+  static int _idCounter = 0;
+
+  TemporaryId(super.name) : _id = _idCounter++;
+  TemporaryId.from(TemporaryId other)
+      : _id = other._id,
+        super(other.name);
+
   @override
-  set sourceInformation(Object? obj) {}
+  int get hashCode => _id;
 
-  TemporaryId(super.name);
+  @override
+  bool operator ==(Object other) {
+    return other is TemporaryId && other._id == _id;
+  }
 }
 
 /// Creates a qualified identifier, without determining for sure if it needs to

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

@@ -97,6 +97,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   @override
   final variableIdentifiers = <VariableDeclaration, js_ast.Identifier>{};
 
+  /// Identifiers for kernel variables with an analgous identifier in JS.
+  ///
+  /// [VariableDeclaration.name] is not necessarily a safe identifier for JS
+  /// transpiled code. The same name can be used in shadowing contexts. We map
+  /// each kernel variable to a [js_ast.TemporaryId] so that at code emission
+  /// time, declarations that would be shadowed are given a unique name. If
+  /// there is no risk of shadowing, the original name will be used.
+  final Map<VariableDeclaration, js_ast.TemporaryId> _variableTempIds = {};
+
   /// Maps a library URI import, that is not in [_libraries], to the
   /// corresponding Kernel summary module we imported it with.
   ///
@@ -4930,7 +4939,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     var v = node.variable;
     var id = _emitVariableRef(v);
     if (id.name == v.name) {
-      id.sourceInformation = _variableSpan(node.fileOffset, v.name!.length);
+      id = id.withSourceInformation(
+          _variableSpan(node.fileOffset, v.name!.length));
     }
     return id;
   }
@@ -4964,7 +4974,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     return null;
   }
 
-  js_ast.Identifier _emitVariableRef(VariableDeclaration v) {
+  js_ast.TemporaryId _emitVariableRef(VariableDeclaration v) {
     if (_isTemporaryVariable(v)) {
       var name = _debuggerFriendlyTemporaryVariableName(v);
       name ??= 't\$${_tempVariables.length}';
@@ -4978,7 +4988,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       // See https://github.com/dart-lang/sdk/issues/55918
       name = extractLocalNameFromLateLoweredLocal(name);
     }
-    return _emitIdentifier(name);
+    return js_ast.TemporaryId.from(
+        _variableTempIds[v] ??= _emitTemporaryId(name));
   }
 
   /// Emits the declaration of a variable.

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

@@ -248,6 +248,15 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   Map<VariableDeclaration, js_ast.Identifier> get variableIdentifiers =>
       _symbolData.variableIdentifiers;
 
+  /// Identifiers for kernel variables with an analgous identifier in JS.
+  ///
+  /// [VariableDeclaration.name] is not necessarily a safe identifier for JS
+  /// transpiled code. The same name can be used in shadowing contexts. We map
+  /// each kernel variable to a [js_ast.TemporaryId] so that at code emission
+  /// time, references that would be shadowed are given a unique name. If there
+  /// is no risk of shadowing, the original name will be used.
+  final Map<VariableDeclaration, js_ast.TemporaryId> _variableTempIds = {};
+
   /// Maps a library URI import, that is not in [_libraries], to the
   /// corresponding Kernel summary module we imported it with.
   ///
@@ -5250,7 +5259,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     var v = node.variable;
     var id = _emitVariableRef(v);
     if (id.name == v.name) {
-      id.sourceInformation = _variableSpan(node.fileOffset, v.name!.length);
+      id = id.withSourceInformation(
+          _variableSpan(node.fileOffset, v.name!.length));
     }
     return id;
   }
@@ -5284,7 +5294,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     return null;
   }
 
-  js_ast.Identifier _emitVariableRef(VariableDeclaration v) {
+  js_ast.TemporaryId _emitVariableRef(VariableDeclaration v) {
     if (_isTemporaryVariable(v)) {
       var name = _debuggerFriendlyTemporaryVariableName(v);
       name ??= 't\$${_tempVariables.length}';
@@ -5298,7 +5308,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       // See https://github.com/dart-lang/sdk/issues/55918
       name = extractLocalNameFromLateLoweredLocal(name);
     }
-    return _emitIdentifier(name);
+    return js_ast.TemporaryId.from(
+        _variableTempIds[v] ??= _emitTemporaryId(name));
   }
 
   /// Emits the declaration of a variable.

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

@@ -115,10 +115,80 @@ class ExpressionCompiler {
         dartScope.definitions[extractLocalName(localName)] =
             dartScope.definitions.remove(localName)!;
       }
+
+      // Create a mapping from Dart variable names in scope to the corresponding
+      // JS variable names. The Dart variable may have had a suffix of the
+      // form '$N' added to it where N is either the empty string or an
+      // integer >= 0.
+      final dartNameToJsName = <String, String>{};
+
+      int nameCompare(String a, String b) {
+        final lengthCmp = b.length.compareTo(a.length);
+        if (lengthCmp != 0) return lengthCmp;
+        return b.compareTo(a);
+      }
+
+      // Sort Dart names in case a user-defined name includes a '$'. The
+      // resulting normalized JS name might seem like a suffixed version of a
+      // shorter Dart name. Since longer Dart names can't incorrectly match a
+      // shorter JS name (JS names are always at least as long as the Dart
+      // name), we process them from longest to shortest.
+      final dartNames = [...dartScope.definitions.keys]..sort(nameCompare);
+
+      // Sort JS names so that the highest suffix value gets assigned to the
+      // corresponding Dart name first. Since names are suffixed in ascending
+      // order as inner scopes are visited, the highest suffix value will be
+      // the one that matches the visible Dart name in a given scope.
+      final jsNames = [...jsScope.keys]..sort(nameCompare);
+
+      const removedSentinel = '';
+      const thisJsName = r'$this';
+
+      for (final dartName in dartNames) {
+        if (isExtensionThisName(dartName)) {
+          if (jsScope.containsKey(thisJsName)) {
+            dartNameToJsName[dartName] = thisJsName;
+          }
+          continue;
+        }
+        // Any name containing a '$' symbol will have that symbol expanded to
+        // '$36' in JS. We do a similar expansion here to normalize the names.
+        final jsNamePrefix =
+            js_ast.toJSIdentifier(dartName).replaceAll('\$', '\\\$');
+        print(jsNamePrefix);
+        final regexp = RegExp(r'^' + jsNamePrefix + r'(\$[0-9]*)?$');
+        for (var i = 0; i < jsNames.length; i++) {
+          final jsName = jsNames[i];
+          if (jsName == removedSentinel) continue;
+          if (jsName.length < dartName.length) break;
+          if (regexp.hasMatch(jsName)) {
+            dartNameToJsName[dartName] = jsName;
+            jsNames[i] = removedSentinel;
+
+            // Remove any additional JS names that match this name as these will
+            // correspond to shadowed Dart variables that are not visible in the
+            // current scope.
+            //
+            // Note: In some extreme cases this can match the wrong variable.
+            // This would require a combination of 36 nested variables with the
+            // same name and a similarly named variable with a $ in its name.
+            for (var j = i; j < jsNames.length; j++) {
+              final jsName = jsNames[j];
+              if (jsName == removedSentinel) continue;
+              if (jsName.length < dartName.length) break;
+              if (regexp.hasMatch(jsNames[j])) {
+                jsNames[j] = removedSentinel;
+              }
+            }
+            break;
+          }
+        }
+      }
+
       // remove undefined js variables (this allows us to get a reference error
       // from chrome on evaluation)
-      dartScope.definitions.removeWhere((variable, type) =>
-          !jsScope.containsKey(_dartNameToJsName(variable)));
+      dartScope.definitions.removeWhere(
+          (variable, type) => !dartNameToJsName.containsKey(variable));
 
       dartScope.typeParameters
           .removeWhere((parameter) => !jsScope.containsKey(parameter.name));
@@ -128,7 +198,7 @@ class ExpressionCompiler {
       var localJsScope = [
         ...dartScope.typeParameters.map((parameter) => jsScope[parameter.name]),
         ...dartScope.definitions.keys
-            .map((variable) => jsScope[_dartNameToJsName(variable)])
+            .map((variable) => jsScope[dartNameToJsName[variable]])
       ];
 
       _log('Performed scope substitutions for expression');
@@ -177,12 +247,6 @@ class ExpressionCompiler {
     }
   }
 
-  String? _dartNameToJsName(String? dartName) {
-    if (dartName == null) return dartName;
-    if (isExtensionThisName(dartName)) return r'$this';
-    return dartName;
-  }
-
   DartScope? _findScopeAt(
       Uri libraryUri, Uri? scriptFileUri, int line, int column) {
     if (line < 0) {