[ddc] Add BigInt and Symbol interceptors

Adds the same interceptors from dart2js as JsPeerInterfaces
and handles typeof differences in getReifiedType. These must
be JsPeerInterfaces so that Object members can be stored in
their prototype. Tests are added so that dart2js and ddc are
consistent.

Change-Id: Iadc3dd26957c0a21b4039c49c1c1ff162ae286e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/321748
Reviewed-by: Stephen Adams <sra@google.com>

CHANGELOG.md

@@ -85,6 +85,23 @@
 
 - Applications compiled by DDC will no longer add members to the native
   JavaScript Object prototype.
+- **Breaking change for JS interop with Symbols and BigInts**:
+  JavaScript `Symbol`s and `BigInt`s are now associated with their own
+  interceptor and should not be used with `package:js` classes. These types were
+  being intercepted with the assumption that they are a subtype of JavaScript's
+  `Object`, but this is incorrect. This lead to erroneous behavior when using
+  these types as Dart `Object`s. See [#53106][] for more details.
+
+#### Dart2js
+
+- **Breaking change for JS interop with Symbols and BigInts**:
+  JavaScript `Symbol`s and `BigInt`s are now associated with their own
+  interceptor and should not be used with `package:js` classes. These types were
+  being intercepted with the assumption that they are a subtype of JavaScript's
+  `Object`, but this is incorrect. This lead to erroneous behavior when using
+  these types as Dart `Object`s. See [#53106][] for more details.
+
+[#53106]: https://github.com/dart-lang/sdk/issues/53106
 
 ## 3.1.0
 

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/rtti.dart

@@ -189,6 +189,9 @@ getReifiedType(obj) {
       case "string":
         return typeRep<String>();
       case "symbol":
+        return typeRep<JavaScriptSymbol>();
+      case "bigint":
+        return typeRep<JavaScriptBigInt>();
       default:
         return typeRep<LegacyJavaScriptObject>();
     }
@@ -219,6 +222,9 @@ getReifiedType(obj) {
       case "string":
         return JS('', '#', String);
       case "symbol":
+        return typeRep<JavaScriptSymbol>();
+      case "bigint":
+        return typeRep<JavaScriptBigInt>();
       default:
         return typeRep<LegacyJavaScriptObject>();
     }

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart

@@ -21,14 +21,16 @@ import 'dart:_foreign_helper'
         spread;
 import 'dart:_interceptors'
     show
+        JavaScriptBigInt,
         JavaScriptFunction,
+        JavaScriptObject,
+        JavaScriptSymbol,
         JSArray,
+        JSFunction,
         JSInt,
         jsNull,
         JSNumNotInt,
-        JSFunction,
         LegacyJavaScriptObject,
-        JavaScriptObject,
         NativeError;
 
 import 'dart:_internal' as internal show LateError, Symbol;

sdk/lib/_internal/js_dev_runtime/private/interceptors.dart

@@ -318,3 +318,43 @@ findInterceptorForType(Type? type) {}
 /// A reference to this class is only used by `getInterceptor()` to return to
 /// the dart:rti library because stores information used for type checks.
 class JavaScriptFunction extends LegacyJavaScriptObject implements Function {}
+
+/// Interceptor for JavaScript BigInt primitive values, i.e. values `x` for
+/// which `typeof x == "bigint"`.
+@JsPeerInterface(name: 'BigInt')
+final class JavaScriptBigInt extends Interceptor {
+  const JavaScriptBigInt();
+
+  // JavaScript BigInt objects don't have any operations that look efficient
+  // enough to generate a good hash code.
+  //
+  // TODO(https://dartbug.com/53162): "bigint" primitive values might be used as
+  // keys without using the `hashCode`, but that will not help for compound hash
+  // values, e.g. produced by `Object.hash`.
+  int get hashCode => 0;
+
+  /// Returns the result of the JavaScript BigInt's `toString` method.
+  String toString() => JS('String', 'String(#)', this);
+}
+
+/// Interceptor for JavaScript Symbol primitive values, i.e. values `x` for
+/// which `typeof x == "symbol"`.
+@JsPeerInterface(name: 'Symbol')
+final class JavaScriptSymbol extends Interceptor {
+  const JavaScriptSymbol();
+
+  // It is not clear what to do for a Symbol's hashCode.  Registered Symbols
+  // [can't be keys of WeakMaps][1].  Using a property won't work because the
+  // property will be added to the ephmeral Object wrapper, either being an
+  // incorrect operation or an error in strict mode.
+  //
+  // TODO(https://dartbug.com/53162): "symbol" primitive values might be used as
+  // keys without using the `hashCode`, but that will not help for compound hash
+  // values, e.g. produced by `Object.hash`.
+  //
+  // [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol#shared_symbols_in_the_global_symbol_registry
+  int get hashCode => 0;
+
+  /// Returns the result of the JavaScript Symbol's `toString` method.
+  String toString() => JS('String', 'String(#)', this);
+}

tests/web/internal/javascript_bigint_test.dart

@@ -0,0 +1,83 @@
+// Copyright (c) 2023, 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 'dart:_interceptors';
+import 'package:js/js.dart';
+import 'package:expect/expect.dart';
+
+@JS()
+external JavaScriptBigInt bigInt;
+
+@JS('bigInt')
+external dynamic bigIntDynamic;
+
+@JS('BigInt')
+external JavaScriptBigInt makeBigInt(String value);
+
+@JS()
+main() {
+  const s = '9876543210000000000000123456789';
+  bigInt = makeBigInt(s);
+
+  /* toString */
+  Expect.equals(s, bigInt.toString());
+  Expect.equals(s, bigIntDynamic.toString());
+  // String interpolation
+  Expect.equals(s, '$bigInt');
+  Expect.equals(s, '$bigIntDynamic');
+  // toString tear-offs
+  var toStringTearoff = bigInt.toString;
+  Expect.type<String Function()>(toStringTearoff);
+  Expect.equals(bigInt.toString, toStringTearoff);
+  Expect.equals(bigInt.toString(), toStringTearoff());
+  toStringTearoff = bigIntDynamic.toString;
+  Expect.type<String Function()>(toStringTearoff);
+  Expect.equals(bigInt.toString, toStringTearoff);
+  Expect.equals(bigInt.toString(), toStringTearoff());
+
+  /* hashCode */
+  // This value is allowed to change, but for lack of a better existing option,
+  // we return 0.
+  Expect.equals(0, bigInt.hashCode);
+  Expect.equals(0, bigIntDynamic.hashCode);
+
+  /* == */
+  // Prefer `==` over `Expect.equals` so we can check dynamic vs non-dynamic
+  // calls.
+  Expect.isTrue(bigInt == bigInt);
+  Expect.isTrue(bigIntDynamic == bigInt);
+  final differentBigInt = makeBigInt('1234567890000000000000987654321');
+  Expect.isFalse(bigInt == differentBigInt);
+  Expect.isFalse(bigIntDynamic == differentBigInt);
+
+  /* noSuchMethod */
+  final methodName = 'testMethod';
+  final invocation = Invocation.method(Symbol(methodName), null);
+  void testNoSuchMethodResult(noSuchMethodResult) {
+    Expect.type<NoSuchMethodError>(noSuchMethodResult);
+    Expect.contains(methodName, noSuchMethodResult.toString());
+  }
+
+  testNoSuchMethodResult(Expect.throws(() => bigInt.noSuchMethod(invocation)));
+  testNoSuchMethodResult(
+      Expect.throws(() => bigIntDynamic.noSuchMethod(invocation)));
+
+  var noSuchMethodTearoff = bigInt.noSuchMethod;
+  Expect.type<dynamic Function(Invocation)>(noSuchMethodTearoff);
+  Expect.equals(bigInt.noSuchMethod, noSuchMethodTearoff);
+  testNoSuchMethodResult(Expect.throws(() => noSuchMethodTearoff(invocation)));
+  noSuchMethodTearoff = bigIntDynamic.noSuchMethod;
+  Expect.type<dynamic Function(Invocation)>(noSuchMethodTearoff);
+  Expect.equals(bigIntDynamic.noSuchMethod, noSuchMethodTearoff);
+  testNoSuchMethodResult(Expect.throws(
+      () => noSuchMethodTearoff(Invocation.method(Symbol(methodName), null))));
+
+  /* runtimeType */
+  var runtimeTypeResult = bigInt.runtimeType;
+  Expect.type<Type>(runtimeTypeResult);
+  Expect.equals(bigInt.runtimeType, runtimeTypeResult);
+  runtimeTypeResult = bigIntDynamic.runtimeType;
+  Expect.type<Type>(runtimeTypeResult);
+  Expect.equals(bigIntDynamic.runtimeType, runtimeTypeResult);
+}

tests/web/internal/javascript_symbol_test.dart

@@ -0,0 +1,88 @@
+// Copyright (c) 2023, 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 'dart:_interceptors';
+import 'package:js/js.dart';
+import 'package:expect/expect.dart';
+
+@JS()
+external void eval(String code);
+
+@JS()
+external JavaScriptSymbol symbol;
+
+@JS('symbol')
+external dynamic symbolDynamic;
+
+@JS('Symbol')
+external JavaScriptSymbol makeSymbol(String value);
+
+@JS()
+main() {
+  const s = 'symbolValue';
+  symbol = makeSymbol(s);
+
+  /* toString */
+  final toStringVal = 'Symbol($s)';
+  Expect.equals(toStringVal, symbol.toString());
+  Expect.equals(toStringVal, symbolDynamic.toString());
+  // String interpolation
+  Expect.equals(toStringVal, '$symbol');
+  Expect.equals(toStringVal, '$symbolDynamic');
+  // toString tear-offs
+  var toStringTearoff = symbol.toString;
+  Expect.type<String Function()>(toStringTearoff);
+  Expect.equals(symbol.toString, toStringTearoff);
+  Expect.equals(symbol.toString(), toStringTearoff());
+  toStringTearoff = symbolDynamic.toString;
+  Expect.type<String Function()>(toStringTearoff);
+  Expect.equals(symbol.toString, toStringTearoff);
+  Expect.equals(symbol.toString(), toStringTearoff());
+
+  /* hashCode */
+  // This value is allowed to change, but for lack of a better existing option,
+  // we return 0.
+  Expect.equals(0, symbol.hashCode);
+  Expect.equals(0, symbolDynamic.hashCode);
+
+  /* == */
+  // Prefer `==` over `Expect.equals` so we can check dynamic vs non-dynamic
+  // calls.
+  Expect.isTrue(symbol == symbol);
+  Expect.isTrue(symbolDynamic == symbol);
+  // Different symbols with the same values are not equal.
+  final differentSymbol = makeSymbol(s);
+  Expect.isFalse(symbol == differentSymbol);
+  Expect.isFalse(symbolDynamic == differentSymbol);
+
+  /* noSuchMethod */
+  final methodName = 'testMethod';
+  final invocation = Invocation.method(Symbol(methodName), null);
+  void testNoSuchMethodResult(noSuchMethodResult) {
+    Expect.type<NoSuchMethodError>(noSuchMethodResult);
+    Expect.contains(methodName, noSuchMethodResult.toString());
+  }
+
+  testNoSuchMethodResult(Expect.throws(() => symbol.noSuchMethod(invocation)));
+  testNoSuchMethodResult(
+      Expect.throws(() => symbolDynamic.noSuchMethod(invocation)));
+
+  var noSuchMethodTearoff = symbol.noSuchMethod;
+  Expect.type<dynamic Function(Invocation)>(noSuchMethodTearoff);
+  Expect.equals(symbol.noSuchMethod, noSuchMethodTearoff);
+  testNoSuchMethodResult(Expect.throws(() => noSuchMethodTearoff(invocation)));
+  noSuchMethodTearoff = symbolDynamic.noSuchMethod;
+  Expect.type<dynamic Function(Invocation)>(noSuchMethodTearoff);
+  Expect.equals(symbolDynamic.noSuchMethod, noSuchMethodTearoff);
+  testNoSuchMethodResult(Expect.throws(
+      () => noSuchMethodTearoff(Invocation.method(Symbol(methodName), null))));
+
+  /* runtimeType */
+  var runtimeTypeResult = symbol.runtimeType;
+  Expect.type<Type>(runtimeTypeResult);
+  Expect.equals(symbol.runtimeType, runtimeTypeResult);
+  runtimeTypeResult = symbolDynamic.runtimeType;
+  Expect.type<Type>(runtimeTypeResult);
+  Expect.equals(symbolDynamic.runtimeType, runtimeTypeResult);
+}

tests/web/native/javascript_bigint_test.dart

@@ -1,32 +0,0 @@
-// Copyright (c) 2023, 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 'native_testing.dart';
-
-import 'dart:_interceptors';
-
-dynamic makeBigIntDynamic(String name) native;
-JavaScriptBigInt makeBigInt(String name) native;
-
-void setup() {
-  JS('', r"""
-(function(){
-  self.makeBigInt = function(name){return BigInt(name)};
-  self.makeBigIntDynamic = function(name){return BigInt(name)};
-})()""");
-}
-
-main() {
-  nativeTesting();
-  setup();
-  const s = '9876543210000000000000123456789';
-
-  Expect.notEquals(s, makeBigInt(s));
-  Expect.notEquals(s, makeBigIntDynamic(s));
-
-  Expect.equals(s, makeBigInt(s).toString());
-  Expect.equals(s, makeBigIntDynamic(s).toString());
-  Expect.equals(s, '${makeBigInt(s)}');
-  Expect.equals(s, '${makeBigIntDynamic(s)}');
-}

tests/web/native/javascript_symbol_test.dart

@@ -1,31 +0,0 @@
-// Copyright (c) 2023, 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 'native_testing.dart';
-
-import 'dart:_interceptors';
-
-dynamic makeSymbolDynamic(String name) native;
-JavaScriptSymbol makeSymbol(String name) native;
-
-void setup() {
-  JS('', r"""
-(function(){
-  self.makeSymbol = function(name){return Symbol(name)};
-  self.makeSymbolDynamic = function(name){return Symbol(name)};
-})()""");
-}
-
-main() {
-  nativeTesting();
-  setup();
-
-  Expect.notEquals('Symbol(foo)', makeSymbol('foo'));
-  Expect.notEquals('Symbol(foo)', makeSymbolDynamic('foo'));
-
-  Expect.equals('Symbol(foo)', makeSymbol('foo').toString());
-  Expect.equals('Symbol(foo)', makeSymbolDynamic('foo').toString());
-  Expect.equals('Symbol(foo)', '${makeSymbol('foo')}');
-  Expect.equals('Symbol(foo)', '${makeSymbolDynamic('foo')}');
-}