[Reland][pkg:js] Lower @staticInterop non-anonymous constructors

This is a reland of https://dart-review.googlesource.com/c/sdk/+/279180.
This removes the anonymous constructor lowering, as the jsify
semantics are not the same as what we have today, since we do no
conversions today. This avoids the breakage in Flutter where we
convert a Uint8List in jsify.

Change-Id: I7eb4ffbd3258abdf6c1aea2035f7dab0336d4851
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279231
Reviewed-by: Riley Porter <rileyporter@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>

pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart

@@ -10,6 +10,7 @@ import 'package:kernel/type_environment.dart';
 import '../js_interop.dart'
     show
         getJSName,
+        hasAnonymousAnnotation,
         hasInternalJSInteropAnnotation,
         hasStaticInteropAnnotation,
         hasTrustTypesAnnotation;
@@ -133,54 +134,30 @@ class JsUtilOptimizer extends Transformer {
             transformedBody = _getExternalMethodBody(node, shouldTrustType);
           }
         }
-      } else if (node.isStatic &&
-          ((node.enclosingClass != null &&
-                  hasStaticInteropAnnotation(node.enclosingClass!)) ||
-              // We only lower top-levels if we're using the
-              // `dart:_js_annotations`' `@JS` annotation to avoid a breaking
-              // change for `package:js` users. There are some internal
-              // libraries that already use this library, so we exclude them
-              // here.
-              // TODO(srujzs): When they're ready to migrate to sound semantics,
-              // we should remove this exception.
-              ((hasInternalJSInteropAnnotation(node) ||
-                      hasInternalJSInteropAnnotation(node.enclosingLibrary)) &&
-                  !_existingJsAnnotationsUsers
-                      .contains(node.enclosingLibrary.importUri.toString())))) {
-        // Fetch the dotted prefix of the member.
-        var libraryName = getJSName(node.enclosingLibrary);
-        var dottedPrefix = libraryName;
-        var enclosingClass = node.enclosingClass;
-        var shouldTrustType = false;
-        if (enclosingClass == null) {
-          // Top-level. If the `@JS` value of the node has any '.'s, we take the
-          // entries before the last '.' to determine the dotted prefix name.
-          var jsName = getJSName(node);
-          if (jsName.isNotEmpty) {
-            var lastDotIndex = jsName.lastIndexOf('.');
-            if (lastDotIndex >= 0) {
-              dottedPrefix = _getCombinedJSName(
-                  dottedPrefix, jsName.substring(0, lastDotIndex));
+      } else {
+        // Do the lowerings for top-levels, static class members, and factories.
+        var dottedPrefix = _getDottedPrefixForNonInstanceMember(node);
+        if (dottedPrefix != null) {
+          var receiver = _getObjectOffGlobalThis(
+              node, dottedPrefix.isEmpty ? [] : dottedPrefix.split('.'));
+          var shouldTrustType = node.enclosingClass != null &&
+              hasTrustTypesAnnotation(node.enclosingClass!);
+          if (node.kind == ProcedureKind.Getter) {
+            transformedBody =
+                _getExternalGetterBody(node, shouldTrustType, receiver);
+          } else if (node.kind == ProcedureKind.Setter) {
+            transformedBody = _getExternalSetterBody(node, receiver);
+          } else if (node.kind == ProcedureKind.Method) {
+            transformedBody =
+                _getExternalMethodBody(node, shouldTrustType, receiver);
+          } else if (node.kind == ProcedureKind.Factory) {
+            if (!hasAnonymousAnnotation(node.enclosingClass!)) {
+              transformedBody = _getExternalConstructorBody(
+                  node,
+                  // Get the constructor object using the class name.
+                  _getObjectOffGlobalThis(node, dottedPrefix.split('.')));
             }
           }
-        } else if (hasStaticInteropAnnotation(enclosingClass)) {
-          // Class static member, use the class name as part of the dotted
-          // prefix.
-          var className = getJSName(enclosingClass);
-          if (className.isEmpty) className = enclosingClass.name;
-          dottedPrefix = _getCombinedJSName(dottedPrefix, className);
-          shouldTrustType = hasTrustTypesAnnotation(enclosingClass);
-        }
-        var receiver = _getObjectOffGlobalThis(
-            node, dottedPrefix.isEmpty ? [] : dottedPrefix.split('.'));
-        if (node.kind == ProcedureKind.Getter) {
-          transformedBody =
-              _getExternalGetterBody(node, shouldTrustType, receiver);
-        } else if (node.kind == ProcedureKind.Setter) {
-          transformedBody = _getExternalSetterBody(node, receiver);
-        } else if (node.kind == ProcedureKind.Method) {
-          transformedBody =
-              _getExternalMethodBody(node, shouldTrustType, receiver);
         }
       }
     }
@@ -195,10 +172,55 @@ class JsUtilOptimizer extends Transformer {
     return node;
   }
 
-  /// Given two `@JS` values, combines them into a qualified name using '.'.
+  /// Returns the prefixed JS name for the given [node] using the enclosing
+  /// library's, enclosing class' (if any), and member's `@JS` values.
+  ///
+  /// Returns null if [node] is not external and a top-level member, a
+  /// `@staticInterop` factory, or a `@staticInterop` static member.
+  String? _getDottedPrefixForNonInstanceMember(Procedure node) {
+    if (!node.isExternal || (!node.isFactory && !node.isStatic)) return null;
+    var enclosingClass = node.enclosingClass;
+    var dottedPrefix = getJSName(node.enclosingLibrary);
+
+    if (enclosingClass == null &&
+        ((hasInternalJSInteropAnnotation(node) ||
+                hasInternalJSInteropAnnotation(node.enclosingLibrary)) &&
+            !_existingJsAnnotationsUsers
+                .contains(node.enclosingLibrary.importUri.toString()))) {
+      // Top-level external member. We only lower top-levels if we're using the
+      // `dart:_js_annotations`' `@JS` annotation to avoid a breaking change for
+      // `package:js` users. There are some internal libraries that already use
+      // this library, so we exclude them here.
+      // TODO(srujzs): When they're ready to migrate to sound semantics, we
+      // should remove this exception.
+
+      // If the `@JS` value of the node has any '.'s, we take the entries
+      // before the last '.' to determine the dotted prefix name.
+      var jsName = getJSName(node);
+      if (jsName.isNotEmpty) {
+        var lastDotIndex = jsName.lastIndexOf('.');
+        if (lastDotIndex != -1) {
+          dottedPrefix = _concatenateJSNames(
+              dottedPrefix, jsName.substring(0, lastDotIndex));
+        }
+      }
+    } else if (enclosingClass != null &&
+        hasStaticInteropAnnotation(enclosingClass)) {
+      // `@staticInterop` factory or static member, use the class name as part
+      // of the dotted prefix.
+      var className = getJSName(enclosingClass);
+      if (className.isEmpty) className = enclosingClass.name;
+      dottedPrefix = _concatenateJSNames(dottedPrefix, className);
+    } else {
+      return null;
+    }
+    return dottedPrefix;
+  }
+
+  /// Given two `@JS` values, combines them into a concatenated name using '.'.
   ///
   /// If either parameters are empty, returns the other.
-  String _getCombinedJSName(String prefix, String suffix) {
+  String _concatenateJSNames(String prefix, String suffix) {
     if (prefix.isEmpty) return suffix;
     if (suffix.isEmpty) return prefix;
     return '$prefix.$suffix';
@@ -327,6 +349,30 @@ class JsUtilOptimizer extends Transformer {
         shouldTrustType: shouldTrustType));
   }
 
+  /// Returns a new function body for the given [node] external non-object
+  /// literal factory.
+  ///
+  /// The new function body will call the optimized version of
+  /// `js_util.callConstructor` using the given [constructor] and the arguments
+  /// of the provided external factory.
+  ReturnStatement _getExternalConstructorBody(
+      Procedure node, Expression constructor) {
+    var function = node.function;
+    assert(function.namedParameters.isEmpty);
+    var callConstructorInvocation = StaticInvocation(
+        _callConstructorTarget,
+        Arguments([
+          constructor,
+          ListLiteral(function.positionalParameters
+              .map<Expression>((argument) => VariableGet(argument))
+              .toList())
+        ], types: [
+          function.returnType
+        ]))
+      ..fileOffset = node.fileOffset;
+    return ReturnStatement(_lowerCallConstructor(callConstructorInvocation));
+  }
+
   /// Return whether [node] is an extension member that's declared as
   /// non-`static`.
   bool _isInstanceExtensionMember(Member node) =>

pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.strong.expect

@@ -14,13 +14,15 @@ library static_interop /*isNonNullableByDefault*/;
 import self as sta;
 import "package:js/js.dart" as js;
 import "dart:core" as core;
+import "dart:js_util" as js_;
 
 import "package:js/js.dart";
 
 @#C4
 @#C5
 class StaticJSClass extends core::Object {
-  external static factory •() → sta::StaticJSClass;
+  static factory •() → sta::StaticJSClass
+    return js_::_callConstructorUnchecked0<sta::StaticJSClass>(js_::_getPropertyTrustType<core::Object>(js_::globalThis, "JSClass"));
   static method _#new#tearOff() → sta::StaticJSClass
     return sta::StaticJSClass::•();
   static factory factory() → sta::StaticJSClass {

pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.strong.transformed.expect

@@ -14,13 +14,15 @@ library static_interop /*isNonNullableByDefault*/;
 import self as sta;
 import "package:js/js.dart" as js;
 import "dart:core" as core;
+import "dart:js_util" as js_;
 
 import "package:js/js.dart";
 
 @#C4
 @#C5
 class StaticJSClass extends core::Object {
-  external static factory •() → sta::StaticJSClass;
+  static factory •() → sta::StaticJSClass
+    return js_::_callConstructorUnchecked0<sta::StaticJSClass>(js_::_getPropertyTrustType<core::Object>(js_::globalThis, "JSClass"));
   static method _#new#tearOff() → sta::StaticJSClass
     return sta::StaticJSClass::•();
   static factory factory() → sta::StaticJSClass {

pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.weak.expect

@@ -14,13 +14,15 @@ library static_interop /*isNonNullableByDefault*/;
 import self as sta;
 import "package:js/js.dart" as js;
 import "dart:core" as core;
+import "dart:js_util" as js_;
 
 import "package:js/js.dart";
 
 @#C4
 @#C5
 class StaticJSClass extends core::Object {
-  external static factory •() → sta::StaticJSClass;
+  static factory •() → sta::StaticJSClass
+    return js_::_callConstructorUnchecked0<sta::StaticJSClass>(js_::_getPropertyTrustType<core::Object>(js_::globalThis, "JSClass"));
   static method _#new#tearOff() → sta::StaticJSClass
     return sta::StaticJSClass::•();
   static factory factory() → sta::StaticJSClass {

pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.weak.outline.expect

@@ -11,13 +11,15 @@ library static_interop /*isNonNullableByDefault*/;
 import self as self2;
 import "package:js/js.dart" as js;
 import "dart:core" as core;
+import "dart:js_util" as js_;
 
 import "package:js/js.dart";
 
 @#C4
 @#C5
 class StaticJSClass extends core::Object {
-  external static factory •() → self2::StaticJSClass;
+  static factory •() → self2::StaticJSClass
+    return js_::_callConstructorUnchecked0<self2::StaticJSClass>(js_::_getPropertyTrustType<core::Object>(js_::globalThis, "JSClass"));
   static method _#new#tearOff() → self2::StaticJSClass
     return self2::StaticJSClass::•();
   static factory factory() → self2::StaticJSClass

pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.weak.transformed.expect

@@ -14,13 +14,15 @@ library static_interop /*isNonNullableByDefault*/;
 import self as sta;
 import "package:js/js.dart" as js;
 import "dart:core" as core;
+import "dart:js_util" as js_;
 
 import "package:js/js.dart";
 
 @#C4
 @#C5
 class StaticJSClass extends core::Object {
-  external static factory •() → sta::StaticJSClass;
+  static factory •() → sta::StaticJSClass
+    return js_::_callConstructorUnchecked0<sta::StaticJSClass>(js_::_getPropertyTrustType<core::Object>(js_::globalThis, "JSClass"));
   static method _#new#tearOff() → sta::StaticJSClass
     return sta::StaticJSClass::•();
   static factory factory() → sta::StaticJSClass {

pkg/front_end/testcases/incremental/js_interop_change.yaml.world.1.expect

@@ -53,7 +53,8 @@ library static_interop from "org-dartlang-test:///lib2.dart" as sta {
   @#C5
   @#C2
   class StaticJSClass extends dart.core::Object {
-    external static factory •() → sta::StaticJSClass;
+    static factory •() → sta::StaticJSClass
+      return dart.js_util::_callConstructorUnchecked0<sta::StaticJSClass>(dart.js_util::_getPropertyTrustType<dart.core::Object>(dart.js_util::globalThis, "JSClass"));
     static method _#new#tearOff() → sta::StaticJSClass
       return sta::StaticJSClass::•();
     static factory factory() → sta::StaticJSClass {

pkg/front_end/testcases/incremental/js_interop_change.yaml.world.2.expect

@@ -53,7 +53,8 @@ library static_interop from "org-dartlang-test:///lib2.dart" as sta {
   @#C5
   @#C2
   class StaticJSClass extends dart.core::Object {
-    external static factory •() → sta::StaticJSClass;
+    static factory •() → sta::StaticJSClass
+      return dart.js_util::_callConstructorUnchecked0<sta::StaticJSClass>(dart.js_util::_getPropertyTrustType<dart.core::Object>(dart.js_util::globalThis, "JSClass"));
     static method _#new#tearOff() → sta::StaticJSClass
       return sta::StaticJSClass::•();
     static factory factory() → sta::StaticJSClass {

tests/lib/js/static_interop_test/external_static_member_lowerings_test.dart

@@ -7,10 +7,9 @@ library external_static_member_lowerings_test;
 
 // ignore: IMPORT_INTERNAL_LIBRARY
 import 'dart:_js_interop';
-import 'dart:js_util' as js_util;
 
 import 'package:expect/minitest.dart';
-import 'package:js/js.dart' show trustTypes, staticInterop, anonymous;
+import 'package:js/js.dart' show trustTypes, staticInterop;
 
 @JS()
 external dynamic eval(String code);
@@ -18,6 +17,11 @@ external dynamic eval(String code);
 @JS()
 @staticInterop
 class ExternalStatic {
+  external factory ExternalStatic(String initialValue);
+  external factory ExternalStatic.named(
+      [String initialValue = 'uninitialized']);
+  // External redirecting factories are not allowed.
+
   external static String field;
   @JS('field')
   external static String renamedField;
@@ -36,6 +40,10 @@ class ExternalStatic {
   external static String renamedMethod();
 }
 
+extension on ExternalStatic {
+  external String get initialValue;
+}
+
 @JS('ExternalStatic')
 @staticInterop
 @trustTypes
@@ -67,7 +75,9 @@ external String renamedMethod();
 
 void main() {
   eval('''
-    globalThis.ExternalStatic = function ExternalStatic() {}
+    globalThis.ExternalStatic = function ExternalStatic(initialValue) {
+      this.initialValue = initialValue;
+    }
     globalThis.ExternalStatic.method = function() {
       return 'method';
     }
@@ -90,6 +100,7 @@ void main() {
   ''');
   testClassStaticMembers();
   testTopLevelMembers();
+  testFactories();
 }
 
 void testClassStaticMembers() {
@@ -154,3 +165,19 @@ void testTopLevelMembers() {
   expect(renamedMethod(), 'method');
   expect((renamedMethod)(), 'method');
 }
+
+void testFactories() {
+  // Non-object literal factories and their tear-offs.
+  var initialized = 'initialized';
+  var uninitialized = 'uninitialized';
+
+  var externalStatic = ExternalStatic(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = ExternalStatic.named();
+  expect(externalStatic.initialValue, uninitialized);
+
+  externalStatic = (ExternalStatic.new)(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = (ExternalStatic.named)(initialized);
+  expect(externalStatic.initialValue, initialized);
+}

tests/lib/js/static_interop_test/external_static_member_lowerings_with_namespaces_test.dart

@@ -18,6 +18,11 @@ import 'package:js/js.dart' show trustTypes, staticInterop;
 @JS('library3.ExternalStatic')
 @staticInterop
 class ExternalStatic {
+  external factory ExternalStatic(String initialValue);
+  external factory ExternalStatic.named(
+      [String initialValue = 'uninitialized']);
+  // External redirecting factories are not allowed.
+
   external static String field;
   @JS('field')
   external static String renamedField;
@@ -36,6 +41,10 @@ class ExternalStatic {
   external static String renamedMethod();
 }
 
+extension on ExternalStatic {
+  external String get initialValue;
+}
+
 @JS('library3.ExternalStatic')
 @staticInterop
 @trustTypes
@@ -55,7 +64,9 @@ void main() {
     var library1 = {library2: library2};
     globalThis.library1 = library1;
 
-    library3.ExternalStatic = function ExternalStatic() {}
+    library3.ExternalStatic = function ExternalStatic(initialValue) {
+      this.initialValue = initialValue;
+    }
     library3.ExternalStatic.method = function() {
       return 'method';
     }
@@ -84,6 +95,7 @@ void main() {
   ]);
   testClassStaticMembers();
   testTopLevelMembers();
+  testFactories();
 }
 
 // Top-level fields.
@@ -172,3 +184,19 @@ void testTopLevelMembers() {
   expect(namespacedMethod(), 'namespacedMethod');
   expect((namespacedMethod)(), 'namespacedMethod');
 }
+
+void testFactories() {
+  // Non-object literal factories and their tear-offs.
+  var initialized = 'initialized';
+  var uninitialized = 'uninitialized';
+
+  var externalStatic = ExternalStatic(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = ExternalStatic.named();
+  expect(externalStatic.initialValue, uninitialized);
+
+  externalStatic = (ExternalStatic.new)(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = (ExternalStatic.named)(initialized);
+  expect(externalStatic.initialValue, initialized);
+}

tests/lib_2/js/static_interop_test/external_static_member_lowerings_test.dart

@@ -7,10 +7,9 @@ library external_static_member_lowerings_test;
 
 // ignore: IMPORT_INTERNAL_LIBRARY
 import 'dart:_js_interop';
-import 'dart:js_util' as js_util;
 
 import 'package:expect/minitest.dart';
-import 'package:js/js.dart' show trustTypes, staticInterop, anonymous;
+import 'package:js/js.dart' show trustTypes, staticInterop;
 
 @JS()
 external dynamic eval(String code);
@@ -18,6 +17,11 @@ external dynamic eval(String code);
 @JS()
 @staticInterop
 class ExternalStatic {
+  external factory ExternalStatic(String initialValue);
+  external factory ExternalStatic.named(
+      [String initialValue = 'uninitialized']);
+  // External redirecting factories are not allowed.
+
   external static String field;
   @JS('field')
   external static String renamedField;
@@ -36,6 +40,10 @@ class ExternalStatic {
   external static String renamedMethod();
 }
 
+extension on ExternalStatic {
+  external String get initialValue;
+}
+
 @JS('ExternalStatic')
 @staticInterop
 @trustTypes
@@ -67,7 +75,9 @@ external String renamedMethod();
 
 void main() {
   eval('''
-    globalThis.ExternalStatic = function ExternalStatic() {}
+    globalThis.ExternalStatic = function ExternalStatic(initialValue) {
+      this.initialValue = initialValue;
+    }
     globalThis.ExternalStatic.method = function() {
       return 'method';
     }
@@ -90,6 +100,7 @@ void main() {
   ''');
   testClassStaticMembers();
   testTopLevelMembers();
+  testFactories();
 }
 
 void testClassStaticMembers() {
@@ -154,3 +165,19 @@ void testTopLevelMembers() {
   expect(renamedMethod(), 'method');
   expect((renamedMethod)(), 'method');
 }
+
+void testFactories() {
+  // Non-object literal factories and their tear-offs.
+  var initialized = 'initialized';
+  var uninitialized = 'uninitialized';
+
+  var externalStatic = ExternalStatic(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = ExternalStatic.named();
+  expect(externalStatic.initialValue, uninitialized);
+
+  externalStatic = (ExternalStatic.new)(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = (ExternalStatic.named)(initialized);
+  expect(externalStatic.initialValue, initialized);
+}

tests/lib_2/js/static_interop_test/external_static_member_lowerings_with_namespaces_test.dart

@@ -18,6 +18,11 @@ import 'package:js/js.dart' show trustTypes, staticInterop;
 @JS('library3.ExternalStatic')
 @staticInterop
 class ExternalStatic {
+  external factory ExternalStatic(String initialValue);
+  external factory ExternalStatic.named(
+      [String initialValue = 'uninitialized']);
+  // External redirecting factories are not allowed.
+
   external static String field;
   @JS('field')
   external static String renamedField;
@@ -36,6 +41,10 @@ class ExternalStatic {
   external static String renamedMethod();
 }
 
+extension on ExternalStatic {
+  external String get initialValue;
+}
+
 @JS('library3.ExternalStatic')
 @staticInterop
 @trustTypes
@@ -55,7 +64,9 @@ void main() {
     var library1 = {library2: library2};
     globalThis.library1 = library1;
 
-    library3.ExternalStatic = function ExternalStatic() {}
+    library3.ExternalStatic = function ExternalStatic(initialValue) {
+      this.initialValue = initialValue;
+    }
     library3.ExternalStatic.method = function() {
       return 'method';
     }
@@ -84,6 +95,7 @@ void main() {
   ]);
   testClassStaticMembers();
   testTopLevelMembers();
+  testFactories();
 }
 
 // Top-level fields.
@@ -172,3 +184,19 @@ void testTopLevelMembers() {
   expect(namespacedMethod(), 'namespacedMethod');
   expect((namespacedMethod)(), 'namespacedMethod');
 }
+
+void testFactories() {
+  // Non-object literal factories and their tear-offs.
+  var initialized = 'initialized';
+  var uninitialized = 'uninitialized';
+
+  var externalStatic = ExternalStatic(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = ExternalStatic.named();
+  expect(externalStatic.initialValue, uninitialized);
+
+  externalStatic = (ExternalStatic.new)(initialized);
+  expect(externalStatic.initialValue, initialized);
+  externalStatic = (ExternalStatic.named)(initialized);
+  expect(externalStatic.initialValue, initialized);
+}