Support renaming fields for JS object literals

This respects the `@JS()` annotation on parameters of external
factories in JS extension types to customize the keys of created object
literals.

Closes #55138

Change-Id: Idebeee922d2080356fd7449f639ecd2ae9ee5fc0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/445840
Reviewed-by: Nicholas Shahan <[email protected]>
Reviewed-by: Nate Biggs <[email protected]>
Reviewed-by: Srujan Gaddam <[email protected]>
Reviewed-by: Stephen Adams <[email protected]>
Commit-Queue: Nate Biggs <[email protected]>
Commit-Queue: Srujan Gaddam <[email protected]>

Author: simolus3

CHANGELOG.md

@@ -77,8 +77,12 @@ To learn more about the feature, check out the
   for more details.
 - `isA<JSBoxedDartObject>` now checks that the value was the result of a
   `toJSBox` operation instead of returning true for all objects.
+- For object literals created from extension type factories, the `@JS()`
+  annotation can now be used to change the name of keys in JavaScript. See
+  [#55138][] for more details.
 
 [#59830]: https://github.com/dart-lang/sdk/issues/59830
+[#55138]: https://github.com/dart-lang/sdk/issues/55138
 
 ## 3.9.0
 

pkg/_js_interop_checks/lib/src/js_interop.dart

@@ -53,12 +53,19 @@ bool hasPatchAnnotation(Annotatable a) => a.annotations.any(_isPatchAnnotation);
 /// If [a] has a `@JS('...')` annotation, returns the value inside the
 /// parentheses.
 ///
+/// This function only considers `JS` annotations defined in [interopLibraries]
+/// or, when that argument is not set, from both `package:js` and
+/// `dart:js_interop`.
+///
 /// If there is none or the class does not have a `@JS()` annotation, returns
 /// an empty String.
-String getJSName(Annotatable a) {
+String getJSName(Annotatable a, {Set<Uri>? interopLibraries}) {
   String jsClass = '';
   for (var annotation in a.annotations) {
-    if (_isJSInteropAnnotation(annotation)) {
+    if (_isJSInteropAnnotation(
+      annotation,
+      interopLibraries: interopLibraries,
+    )) {
       var jsClasses = stringAnnotationValues(annotation);
       if (jsClasses.isNotEmpty) {
         jsClass = jsClasses[0];
@@ -68,6 +75,14 @@ String getJSName(Annotatable a) {
   return jsClass;
 }
 
+/// If [a] has a `JS('...')` annotation from `dart:js_interop`, returns the
+/// value inside the parentheses.
+///
+/// If no such annotation exists, returns an empty string.
+String getDartJSInteropJSName(Annotatable a) {
+  return getJSName(a, interopLibraries: {_jsInterop});
+}
+
 /// If [a] has a `@Native('...')` annotation, returns the values inside the
 /// parentheses.
 ///
@@ -130,8 +145,8 @@ bool _isInteropAnnotation(
   return interopLibraries.contains(importUri);
 }
 
-bool _isJSInteropAnnotation(Expression value) =>
-    _isInteropAnnotation(value, 'JS');
+bool _isJSInteropAnnotation(Expression value, {Set<Uri>? interopLibraries}) =>
+    _isInteropAnnotation(value, 'JS', interopLibraries: interopLibraries);
 
 bool _isPackageJSAnnotation(Expression value) => _isInteropAnnotation(
   value,

pkg/compiler/lib/src/ssa/builder.dart

@@ -2,6 +2,9 @@
 // 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.
 
+// ignore: implementation_imports
+import 'package:_js_interop_checks/src/js_interop.dart'
+    show getDartJSInteropJSName;
 // ignore: implementation_imports
 import 'package:front_end/src/api_prototype/static_weak_references.dart'
     as ir
@@ -7016,7 +7019,11 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
         final argument = arguments[i];
         if (argument != null) {
           filteredArguments.add(argument);
-          var jsName = _nativeData.computeUnescapedJSInteropName(parameterName);
+          var customName = getDartJSInteropJSName(variable);
+
+          var jsName = (customName.isNotEmpty && isObjectLiteralConstructor)
+              ? customName
+              : _nativeData.computeUnescapedJSInteropName(parameterName);
           parameterNameMap[jsName] = js.InterpolatedExpression(positions++);
         }
         i++;

pkg/dart2wasm/lib/js/interop_specializer.dart

@@ -3,7 +3,11 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:_js_interop_checks/src/js_interop.dart'
-    show getJSName, hasAnonymousAnnotation, hasJSInteropAnnotation;
+    show
+        getDartJSInteropJSName,
+        getJSName,
+        hasAnonymousAnnotation,
+        hasJSInteropAnnotation;
 import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'
     show ExtensionIndex;
 import 'package:kernel/ast.dart';
@@ -343,9 +347,22 @@ class _ObjectLiteralSpecializer extends _InvocationSpecializer {
         .toList();
   }
 
+  /// The name to use in JavaScript for the Dart parameter [variable].
+  ///
+  /// This defaults to the name of the [variable], but can be changed with a
+  /// `@JS()` annotation.
+  String _jsKey(VariableDeclaration variable) {
+    // Only support `@JS` renaming on extension type object literal
+    // constructors.
+    final changedName = interopMethod.isExtensionTypeMember
+        ? getDartJSInteropJSName(variable)
+        : '';
+    return changedName.isEmpty ? variable.name! : changedName;
+  }
+
   @override
   String bodyString(String object, List<String> callArguments) {
-    final keys = parameters.map((named) => named.name!).toList();
+    final keys = parameters.map(_jsKey).toList();
     final keyValuePairs = <String>[];
     for (int i = 0; i < callArguments.length; i++) {
       keyValuePairs.add('${keys[i]}: ${callArguments[i]}');
@@ -362,8 +379,7 @@ class _ObjectLiteralSpecializer extends _InvocationSpecializer {
     // `Cons(a: 0)`, and `Cons(a: 1, b: 1)` only create two shapes:
     // `{a: value, b: value}` and `{a: value}`. Therefore, we only need two
     // methods to handle the `Cons` invocations.
-    final shape =
-        parameters.map((VariableDeclaration decl) => decl.name).join('|');
+    final shape = parameters.map(_jsKey).join('|');
     final interopProcedure = _jsObjectLiteralMethods
         .putIfAbsent(interopMethod, () => {})
         .putIfAbsent(shape, () => _getRawInteropProcedure());

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

@@ -8,7 +8,7 @@ import 'dart:io' as io;
 import 'dart:math' show max, min;
 
 import 'package:_js_interop_checks/src/js_interop.dart'
-    show hasDartJSInteropAnnotation;
+    show getDartJSInteropJSName, hasDartJSInteropAnnotation;
 import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'
     show ExtensionIndex;
 import 'package:front_end/src/api_unstable/ddc.dart';
@@ -6937,9 +6937,20 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       // JS interop checks assert that only external extension type constructors
       // and factories have named parameters.
       assert(target.function.positionalParameters.isEmpty);
-      var namedProperties = {
-        for (final named in node.arguments.named) named.name: named.value,
-      };
+      var namedProperties = <String, Expression>{};
+      for (var named in node.arguments.named) {
+        var name = named.name;
+        for (var parameter in target.function.namedParameters) {
+          if (parameter.name == named.name) {
+            var customName = getDartJSInteropJSName(parameter);
+            if (customName.isNotEmpty) {
+              name = customName;
+            }
+            break;
+          }
+        }
+        namedProperties[name] = named.value;
+      }
       return _emitObjectLiteral(namedProperties, isDartJsInterop: true);
     }
     if (target == _coreTypes.identicalProcedure) {

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

@@ -8,7 +8,7 @@ import 'dart:io' as io;
 import 'dart:math' show max, min;
 
 import 'package:_js_interop_checks/src/js_interop.dart'
-    show hasDartJSInteropAnnotation;
+    show getDartJSInteropJSName, hasDartJSInteropAnnotation;
 import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'
     show ExtensionIndex;
 import 'package:front_end/src/api_unstable/ddc.dart';
@@ -7620,9 +7620,21 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       // JS interop checks assert that only external extension type constructors
       // and factories have named parameters.
       assert(target.function.positionalParameters.isEmpty);
-      var namedProperties = {
-        for (final named in node.arguments.named) named.name: named.value,
-      };
+      var namedProperties = <String, Expression>{};
+      for (var named in node.arguments.named) {
+        var name = named.name;
+        for (var parameter in target.function.namedParameters) {
+          if (parameter.name == named.name) {
+            var customName = getDartJSInteropJSName(parameter);
+            if (customName.isNotEmpty) {
+              name = customName;
+            }
+            break;
+          }
+        }
+        namedProperties[name] = named.value;
+      }
+
       return _emitObjectLiteral(namedProperties, isDartJsInterop: true);
     }
     if (target == _coreTypes.identicalProcedure) {

tests/lib/js/static_interop_test/extension_type/object_literal_constructor_test.dart

@@ -13,6 +13,14 @@ extension type Literal._(JSObject _) implements JSObject {
   external factory Literal.fact({double? a, String b, bool? c});
 }
 
+extension type NamedKeysLiteral._(JSObject _) implements JSObject {
+  external NamedKeysLiteral({@JS('a') double? namedA, @JS('b') String namedB});
+}
+
+extension type OriginalNamesLiteral._(JSObject _) implements JSObject {
+  external OriginalNamesLiteral({@JS() double? a, @JS('') String b});
+}
+
 // Test that the properties we assumed to exist in `literal` actually exist and
 // that their values are as expected. If we assumed they don't exist, check that
 // they do not. Note that we don't check the order of the keys in the literal.
@@ -53,4 +61,10 @@ void main() {
   testProperties(Literal(a: 0.0, b: '', c: true), a: 0.0, b: '', c: true);
   // Test that passing in a different order doesn't change the values.
   testProperties(Literal.fact(c: true, a: 0.0, b: ''), a: 0.0, b: '', c: true);
+
+  testProperties(NamedKeysLiteral(namedA: 0.0), a: 0.0);
+  testProperties(NamedKeysLiteral(namedA: 1.0, namedB: 'b'), a: 1.0, b: 'b');
+
+  testProperties(OriginalNamesLiteral(a: 0.0), a: 0.0);
+  testProperties(OriginalNamesLiteral(a: 0.0, b: 'b'), a: 0.0, b: 'b');
 }