[dart:js_interop_unsafe] Rename extensions and make []/[]= take String properties

Strings are the most likely use case of getting and setting
properties, and therefore we should make that easier to use.
This CL also renames the extensions in dart:js_interop_unsafe
to a more relevant name and to avoid conflicts in dart:js_interop.

CoreLibraryReviewExempt: Backend-specific library.
Change-Id: Ia8ce6593167c648f9710b47cfe27f80c854be407
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324572
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>

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

@@ -39,7 +39,7 @@ class ExportCreator extends Transformer {
       this._typeEnvironment, this._diagnosticReporter, this._exportChecker)
       : _callMethodVarArgs = _typeEnvironment.coreTypes.index
             .getTopLevelProcedure('dart:js_interop_unsafe',
-                'JSObjectUtilExtension|callMethodVarArgs'),
+                'JSObjectUnsafeUtilExtension|callMethodVarArgs'),
         _createDartExport = _typeEnvironment.coreTypes.index
             .getTopLevelProcedure('dart:js_util', 'createDartExport'),
         _createStaticInteropMock = _typeEnvironment.coreTypes.index
@@ -47,7 +47,7 @@ class ExportCreator extends Transformer {
         _functionToJS = _typeEnvironment.coreTypes.index.getTopLevelProcedure(
             'dart:js_interop', 'FunctionToJSExportedDartFunction|get#toJS'),
         _getProperty = _typeEnvironment.coreTypes.index.getTopLevelProcedure(
-            'dart:js_interop_unsafe', 'JSObjectUtilExtension|[]'),
+            'dart:js_interop_unsafe', 'JSObjectUnsafeUtilExtension|[]'),
         _globalContext = _typeEnvironment.coreTypes.index
             .getTopLevelProcedure('dart:js_interop', 'get:globalContext'),
         _jsAny = _typeEnvironment.coreTypes.index
@@ -55,7 +55,7 @@ class ExportCreator extends Transformer {
         _jsObject = _typeEnvironment.coreTypes.index
             .getClass('dart:_js_types', 'JSObject'),
         _setProperty = _typeEnvironment.coreTypes.index.getTopLevelProcedure(
-            'dart:js_interop_unsafe', 'JSObjectUtilExtension|[]='),
+            'dart:js_interop_unsafe', 'JSObjectUnsafeUtilExtension|[]='),
         _stringToJS = _typeEnvironment.coreTypes.index.getTopLevelProcedure(
             'dart:js_interop', 'StringToJSString|get#toJS'),
         _staticInteropMockValidator = StaticInteropMockValidator(
@@ -201,7 +201,7 @@ class ExportCreator extends Transformer {
 
     // Get the global 'Object' property.
     Expression getObjectProperty() => asJSObject(StaticInvocation(_getProperty,
-        Arguments([StaticGet(_globalContext), toJSString('Object')])))
+        Arguments([StaticGet(_globalContext), StringLiteral('Object')])))
       ..fileOffset = node.fileOffset;
 
     // Get a fresh object literal, using the proto to create it if one was
@@ -240,9 +240,9 @@ class ExportCreator extends Transformer {
       var exports = exportMap[exportName]!;
       ExpressionStatement setProperty(
           VariableGet jsObject, String propertyName, StaticInvocation jsValue) {
-        // `jsObject[propertyName.toJS] = jsValue`
+        // `jsObject[propertyName] = jsValue`
         return ExpressionStatement(StaticInvocation(_setProperty,
-            Arguments([jsObject, toJSString(propertyName), jsValue])))
+            Arguments([jsObject, StringLiteral(propertyName), jsValue])))
           ..fileOffset = node.fileOffset
           ..parent = node.parent;
       }
@@ -251,7 +251,7 @@ class ExportCreator extends Transformer {
       // With methods, there's only one export per export name.
       if (firstExport is Procedure &&
           firstExport.kind == ProcedureKind.Method) {
-        // `jsExport[jsName.toJS] = dartMock.tearoffMethod.toJS`
+        // `jsExport[jsName] = dartMock.tearoffMethod.toJS`
         block.add(setProperty(
             VariableGet(jsExporter),
             exportName,
@@ -276,7 +276,7 @@ class ExportCreator extends Transformer {
         // The AST code looks like:
         //
         // ```
-        // getSetMap['get'.toJS] = () {
+        // getSetMap['get'] = () {
         //   return dartInstance.getter;
         // }.toJS;
         // ```
@@ -284,7 +284,7 @@ class ExportCreator extends Transformer {
         // in the case of a getter and:
         //
         // ```
-        // getSetMap['set'.toJS] = (val) {
+        // getSetMap['set'] = (val) {
         //  dartInstance.setter = val;
         // }.toJS;
         // ```

sdk/lib/_internal/js_shared/lib/js_interop_unsafe_patch.dart

@@ -8,7 +8,7 @@ import 'dart:js_interop' hide JS;
 import 'dart:js_util' as js_util;
 
 @patch
-extension JSObjectUtilExtension on JSObject {
+extension JSObjectUnsafeUtilExtension on JSObject {
   @patch
   @pragma('dart2js:prefer-inline')
   JSBoolean hasProperty(JSAny property) =>
@@ -16,12 +16,12 @@ extension JSObjectUtilExtension on JSObject {
 
   @patch
   @pragma('dart2js:prefer-inline')
-  JSAny? operator [](JSAny property) =>
+  T getProperty<T extends JSAny?>(JSAny property) =>
       JS<dynamic>('Object|Null', '#[#]', this, property);
 
   @patch
   @pragma('dart2js:prefer-inline')
-  void operator []=(JSAny property, JSAny? value) =>
+  void setProperty(JSAny property, JSAny? value) =>
       JS<void>('', '#[#] = #', this, property, value);
 
   // TODO(joshualitt): Specialize at callsites.
@@ -57,7 +57,7 @@ extension JSObjectUtilExtension on JSObject {
 }
 
 @patch
-extension JSFunctionUtilExtension on JSFunction {
+extension JSFunctionUnsafeUtilExtension on JSFunction {
   // TODO(joshualitt): Specialize `callAsConstructor`.
   @patch
   @pragma('dart2js:prefer-inline')

sdk/lib/_internal/wasm/lib/js_helper.dart

@@ -10,6 +10,7 @@ import 'dart:_js_annotations' as js;
 import 'dart:_js_types' as js_types;
 import 'dart:_wasm';
 import 'dart:js_interop';
+import 'dart:js_interop_unsafe';
 import 'dart:typed_data';
 
 part 'regexp_helper.dart';
@@ -98,11 +99,6 @@ extension JSArrayExtension on JSArray {
   external JSNumber get length;
 }
 
-extension JSObjectExtension on JSObject {
-  external JSAny? operator [](JSString key);
-  external void operator []=(JSString key, JSAny? value);
-}
-
 class JSArrayIteratorAdapter<T> implements Iterator<T> {
   final JSArray array;
   int index = -1;

sdk/lib/_internal/wasm/lib/js_interop_patch.dart

@@ -473,25 +473,12 @@ JSArray _createJSProxyOfList(List<JSAny?> list) {
   // Needed for `concat` to spread the contents of the current array instead of
   // prepending.
   // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/isConcatSpreadable
-  unsafe.JSObjectUtilExtension(jsExportWrapper)[_Symbol.isConcatSpreadable] =
-      true.toJS;
+  jsExportWrapper.setProperty(_Symbol.isConcatSpreadable, true.toJS);
 
-  final getIndex =
-      (unsafe.JSObjectUtilExtension(jsExportWrapper)['_getIndex'.toJS]
-              as JSFunction)
-          .toExternRef;
-  final setIndex =
-      (unsafe.JSObjectUtilExtension(jsExportWrapper)['_setIndex'.toJS]
-              as JSFunction)
-          .toExternRef;
-  final hasIndex =
-      (unsafe.JSObjectUtilExtension(jsExportWrapper)['_hasIndex'.toJS]
-              as JSFunction)
-          .toExternRef;
-  final deleteIndex =
-      (unsafe.JSObjectUtilExtension(jsExportWrapper)['_deleteIndex'.toJS]
-              as JSFunction)
-          .toExternRef;
+  final getIndex = jsExportWrapper['_getIndex']!.toExternRef;
+  final setIndex = jsExportWrapper['_setIndex']!.toExternRef;
+  final hasIndex = jsExportWrapper['_hasIndex']!.toExternRef;
+  final deleteIndex = jsExportWrapper['_deleteIndex']!.toExternRef;
 
   final proxy = _box<JSArray>(js_helper.JS<WasmExternRef?>('''
     (wrapper, getIndex, setIndex, hasIndex, deleteIndex) => new Proxy(wrapper, {

sdk/lib/_internal/wasm/lib/js_interop_unsafe_patch.dart

@@ -14,17 +14,17 @@ import 'dart:js_interop' hide JS;
 T _box<T>(WasmExternRef? ref) => JSValue.box(ref) as T;
 
 @patch
-extension JSObjectUtilExtension on JSObject {
+extension JSObjectUnsafeUtilExtension on JSObject {
   @patch
   JSBoolean hasProperty(JSAny property) => _box<JSBoolean>(JS<WasmExternRef?>(
       '(o, p) => p in o', toExternRef, property.toExternRef));
 
   @patch
-  JSAny? operator [](JSAny property) => _box<JSAny?>(
+  T getProperty<T extends JSAny?>(JSAny property) => _box<T>(
       JS<WasmExternRef?>('(o, p) => o[p]', toExternRef, property.toExternRef));
 
   @patch
-  void operator []=(JSAny property, JSAny? value) => JS<void>(
+  void setProperty(JSAny property, JSAny? value) => JS<void>(
       '(o, p, v) => o[p] = v',
       toExternRef,
       property.toExternRef,
@@ -57,7 +57,7 @@ extension JSObjectUtilExtension on JSObject {
 }
 
 @patch
-extension JSFunctionUtilExtension on JSFunction {
+extension JSFunctionUnsafeUtilExtension on JSFunction {
   @patch
   JSObject _callAsConstructor(
           [JSAny? arg1, JSAny? arg2, JSAny? arg3, JSAny? arg4]) =>

sdk/lib/_internal/wasm/lib/regexp_helper.dart

@@ -202,7 +202,7 @@ class _MatchImplementation implements RegExpMatch {
   String? namedGroup(String name) {
     JSObject? groups = _match.groups;
     if (groups.isDefinedAndNotNull) {
-      Object? result = dartifyRaw(groups![name.toJS]?.toExternRef);
+      Object? result = dartifyRaw(groups![name]?.toExternRef);
       if (result != null ||
           hasPropertyRaw(groups.toExternRef, name.toExternRef)) {
         return result?.toString();

sdk/lib/js_interop_unsafe/js_interop_unsafe.dart

@@ -25,18 +25,22 @@ library dart.js_interop_unsafe;
 
 import 'dart:js_interop';
 
-extension JSObjectUtilExtension on JSObject {
+extension JSObjectUnsafeUtilExtension on JSObject {
   /// Whether or not this [JSObject] has a given property.
   external JSBoolean hasProperty(JSAny property);
 
-  /// Equivalent to invoking operator `[]` in JS.
-  external JSAny? operator [](JSAny property);
+  /// Shorthand helper to get String properties.
+  JSAny? operator [](String property) => getProperty(property.toJS);
 
-  /// Gets a given property from this [JSObject].
-  T getProperty<T extends JSAny?>(JSAny property) => this[property] as T;
+  /// Gets a given [property] from this [JSObject].
+  external T getProperty<T extends JSAny?>(JSAny property);
 
-  /// Equivalent to invoking `[]=` in JS.
-  external void operator []=(JSAny property, JSAny? value);
+  /// Shorthand helper to set String properties.
+  void operator []=(String property, JSAny? value) =>
+      setProperty(property.toJS, value);
+
+  /// Sets a given [property] with [value] on this [JSObject].
+  external void setProperty(JSAny property, JSAny? value);
 
   /// Calls a method on this [JSObject] with up to four arguments and returns
   /// the result.
@@ -57,7 +61,7 @@ extension JSObjectUtilExtension on JSObject {
   external JSBoolean delete(JSAny property);
 }
 
-extension JSFunctionUtilExtension on JSFunction {
+extension JSFunctionUnsafeUtilExtension on JSFunction {
   /// Calls this [JSFunction] as a constructor with up to four arguments and
   /// returns the constructed [JSObject].
   external JSObject _callAsConstructor(

tests/lib/js_interop_unsafe/basic_test.dart

@@ -17,9 +17,9 @@ external void eval(String code);
 void createObjectTest() {
   JSObject o = newObject<JSObject>();
   Expect.isFalse(o.hasProperty('foo'.toJS).toDart);
-  o['foo'.toJS] = 'bar'.toJS;
+  o['foo'] = 'bar'.toJS;
   Expect.isTrue(o.hasProperty('foo'.toJS).toDart);
-  Expect.equals('bar', (o['foo'.toJS] as JSString).toDart);
+  Expect.equals('bar', (o['foo'] as JSString).toDart);
 }
 
 void equalTest() {
@@ -46,9 +46,9 @@ void equalTest() {
     ''');
     JSObject gc = globalContext;
     void test(String propertyName, bool testCanonicalization) {
-      Expect.equals(gc[propertyName.toJS], gc[propertyName.toJS]);
+      Expect.equals(gc[propertyName], gc[propertyName]);
       if (testCanonicalization) {
-        Expect.equals(gc[propertyName.toJS], gc[(propertyName + "2").toJS]);
+        Expect.equals(gc[propertyName], gc[propertyName + "2"]);
       }
     }
 
@@ -83,11 +83,10 @@ void typeofTest() {
   };
   void test(String property, String expectedType) {
     Expect.isTrue(
-        globalContext[property.toJS]?.typeofEquals(expectedType.toJS).toDart);
+        globalContext[property]?.typeofEquals(expectedType.toJS).toDart);
     for (final type in types) {
       if (type != expectedType) {
-        Expect.isFalse(
-            globalContext[property.toJS]?.typeofEquals(type.toJS).toDart);
+        Expect.isFalse(globalContext[property]?.typeofEquals(type.toJS).toDart);
       }
     }
   }
@@ -110,9 +109,9 @@ void instanceOfTest() {
       globalThis.obj = new JSClass1();
     ''');
   JSObject gc = globalContext;
-  JSObject obj = gc['obj'.toJS] as JSObject;
-  JSFunction jsClass1Constructor = gc['JSClass1'.toJS] as JSFunction;
-  JSFunction jsClass2Constructor = gc['JSClass2'.toJS] as JSFunction;
+  JSObject obj = gc['obj'] as JSObject;
+  JSFunction jsClass1Constructor = gc['JSClass1'] as JSFunction;
+  JSFunction jsClass2Constructor = gc['JSClass2'] as JSFunction;
   Expect.isTrue(obj.instanceof(jsClass1Constructor).toDart);
   Expect.isFalse(obj.instanceof(jsClass2Constructor).toDart);
 }
@@ -152,7 +151,7 @@ void evalAndConstructTest() {
     globalThis.JSClass = JSClass;
   ''');
   JSObject gc = globalContext;
-  JSFunction constructor = gc['JSClass'.toJS] as JSFunction;
+  JSFunction constructor = gc['JSClass'] as JSFunction;
 
   // Var args
   JSObject jsObj1 =
@@ -268,7 +267,7 @@ void deepConversionsTest() {
     globalThis.keyObject2 = keyObject;
   ''');
   JSObject gc = globalContext;
-  Expect.isNull(gc['a'.toJS]);
+  Expect.isNull(gc['a']);
   Expect.equals('foo', gc.getProperty<JSString>('b'.toJS).toDart);
   _expectRecEquals(
       ['a', 'b', 'c'],
@@ -320,7 +319,7 @@ void deepConversionsTest() {
       gc.getProperty<JSDataView>('dataView'.toJS).toDart.buffer.asUint8List());
 
   // Confirm a function that takes a roundtrip remains a function.
-  JSFunction foo = gc['f'.toJS].dartify() as JSFunction;
+  JSFunction foo = gc['f'].dartify() as JSFunction;
   Expect.equals(
       'hello world', gc.callMethod<JSString>('invoke'.toJS, foo).toDart);
 
@@ -334,12 +333,12 @@ void deepConversionsTest() {
       3,
       {'baz': 'boo'}
     ],
-  ], gc['implicitExplicit'.toJS].dartify() as Iterable);
+  ], gc['implicitExplicit'].dartify() as Iterable);
 
   // Test that JS objects behave as expected in Map / Set.
   Set<Object?> set = {};
-  JSAny? key1 = gc['keyObject1'.toJS];
-  JSAny? key2 = gc['keyObject2'.toJS];
+  JSAny? key1 = gc['keyObject1'];
+  JSAny? key2 = gc['keyObject2'];
   Expect.isTrue(set.add(key1));
   Expect.isTrue(set.contains(key1));
   Expect.isFalse(set.add(key2));