[dart:js_interop] Fix comparison operator return types

Fixes https://github.com/dart-lang/sdk/issues/55024

The patch files for these operators return a bool, whereas
the public API returns a JSBoolean. Since there's only one
possible return type, we should make them return bool for
convenience. Boolean conversion is also inexpensive on
dart2wasm, so that shouldn't be an issue.

Also adds helpers to operator_test to make sure any JS values
are converted before they're compared, adding additional type
checking through the conversion.

CoreLibraryReviewExempt: Fixes type mismatch in backend-specific library.
Change-Id: I7ff2e334e817e6e7d7d8d5091a4e5d570a496b03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354702
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
This commit is contained in:
Srujan Gaddam 2024-03-04 21:10:02 +00:00 committed by Commit Queue
parent 9cafa46290
commit e1834792c3
3 changed files with 80 additions and 55 deletions

View file

@ -32,6 +32,15 @@
[#53863]: https://github.com/dart-lang/sdk/issues/53863
#### `dart:js_interop`
- Fixes an issue with several comparison operators in `JSAnyOperatorExtension`
that were declared to return `JSBoolean` but really returned `bool`. This led
to runtime errors when trying to use the return values. Their return types are
now `bool` for convenience. See issues [#55024] for more details.
[#55024]: https://github.com/dart-lang/sdk/issues/55024
### Tools
#### Pub

View file

@ -1004,6 +1004,9 @@ extension StringToJSString on String {
/// ```
/// external operator int [](int key);
/// ```
///
/// Operators that always return number or boolean values use the Dart type
/// instead of a JS type for convenience as the conversions are inexpensive.
// TODO(srujzs): Add more as needed. For now, we just expose the ones needed to
// migrate from `dart:js_util`.
extension JSAnyOperatorExtension on JSAny? {
@ -1030,32 +1033,33 @@ extension JSAnyOperatorExtension on JSAny? {
// Comparison operators.
/// The result of <code>[this] > [any]</code> in JavaScript.
external JSBoolean greaterThan(JSAny? any);
external bool greaterThan(JSAny? any);
/// The result of <code>[this] >= [any]</code> in JavaScript.
external JSBoolean greaterThanOrEqualTo(JSAny? any);
external bool greaterThanOrEqualTo(JSAny? any);
/// The result of <code>[this] < [any]</code> in JavaScript.
external JSBoolean lessThan(JSAny? any);
external bool lessThan(JSAny? any);
/// The result of <code>[this] <= [any]</code> in JavaScript.
external JSBoolean lessThanOrEqualTo(JSAny? any);
external bool lessThanOrEqualTo(JSAny? any);
/// The result of <code>[this] == [any]</code> in JavaScript.
external JSBoolean equals(JSAny? any);
external bool equals(JSAny? any);
/// The result of <code>[this] != [any]</code> in JavaScript.
external JSBoolean notEquals(JSAny? any);
external bool notEquals(JSAny? any);
/// The result of <code>[this] === [any]</code> in JavaScript.
external JSBoolean strictEquals(JSAny? any);
external bool strictEquals(JSAny? any);
/// The result of <code>[this] !== [any]</code> in JavaScript.
external JSBoolean strictNotEquals(JSAny? any);
external bool strictNotEquals(JSAny? any);
// Bitwise operators.
/// The result of <code>[this] >>> [any]</code> in JavaScript.
// TODO(srujzs): This should return `num` or `double` instead.
external JSNumber unsignedRightShift(JSAny? any);
// Logical operators.

View file

@ -57,56 +57,68 @@ extension on int {
JSBigInt get toBigInt => BigInt(this.toString());
}
// Use a helper for `Expect.isTrue` and `Expect.isFalse` as those methods don't
// differentiate between `JSBoolean` and `bool`.
void expectTrue(bool value) {
Expect.isTrue(value);
}
void expectFalse(bool value) {
Expect.isFalse(value);
}
int toInt(JSAny any) => (any as JSNumber).toDartInt;
void dartJsInteropOperatorsTest() {
// Arithmetic.
final i10 = 10.toJS;
expect(i10.add(1.toJS), 11.toJS);
expect(i10.subtract(1.toJS), 9.toJS);
expect(i10.multiply(2.toJS), 20.toJS);
expect(i10.divide(10.toJS), 1.toJS);
expect(i10.modulo(5.toJS), 0.toJS);
expect(i10.exponentiate(2.toJS), 100.toJS);
expect(toInt(i10.add(1.toJS)), 11);
expect(toInt(i10.subtract(1.toJS)), 9);
expect(toInt(i10.multiply(2.toJS)), 20);
expect(toInt(i10.divide(10.toJS)), 1);
expect(toInt(i10.modulo(5.toJS)), 0);
expect(toInt(i10.exponentiate(2.toJS)), 100);
// Bitwise.
expect(i10.unsignedRightShift(1.toJS), 5.toJS);
expect(toInt(i10.unsignedRightShift(1.toJS)), 5);
// Comparison/relational.
final t = true.toJS;
final f = false.toJS;
// Equality attempts to coerce, whereas strict equality does not.
Expect.isTrue(t.equals(1.toJS));
Expect.isFalse(t.notEquals(1.toJS));
Expect.isFalse(t.strictEquals(1.toJS));
Expect.isTrue(t.strictNotEquals(1.toJS));
Expect.isFalse((t.and(f) as JSBoolean).toDart);
Expect.isTrue((t.or(f) as JSBoolean).toDart);
Expect.isFalse(t.not);
Expect.isTrue(t.isTruthy);
Expect.isFalse(i10.lessThan(i10));
Expect.isTrue(i10.lessThanOrEqualTo(i10));
Expect.isFalse(i10.greaterThan(i10));
Expect.isTrue(i10.greaterThanOrEqualTo(i10));
expectTrue(t.equals(1.toJS));
expectFalse(t.notEquals(1.toJS));
expectFalse(t.strictEquals(1.toJS));
expectTrue(t.strictNotEquals(1.toJS));
expectFalse((t.and(f) as JSBoolean).toDart);
expectTrue((t.or(f) as JSBoolean).toDart);
expectFalse(t.not);
expectTrue(t.isTruthy);
expectFalse(i10.lessThan(i10));
expectTrue(i10.lessThanOrEqualTo(i10));
expectFalse(i10.greaterThan(i10));
expectTrue(i10.greaterThanOrEqualTo(i10));
// Nulls.
expect(null.add(null), 0.toJS);
expect(null.subtract(null), 0.toJS);
expect(null.multiply(null), 0.toJS);
Expect.isTrue(isNaN(null.divide(null)));
Expect.isTrue(isNaN(null.modulo(null)));
expect(null.exponentiate(null), 1.toJS);
expect(null.unsignedRightShift(null), 0.toJS);
Expect.isTrue(null.equals(null));
Expect.isFalse(null.notEquals(null));
Expect.isTrue(null.strictEquals(null));
Expect.isFalse(null.strictNotEquals(null));
expect(toInt(null.add(null)), 0);
expect(toInt(null.subtract(null)), 0);
expect(toInt(null.multiply(null)), 0);
expectTrue(isNaN(null.divide(null)));
expectTrue(isNaN(null.modulo(null)));
expect(toInt(null.exponentiate(null)), 1);
expect(toInt(null.unsignedRightShift(null)), 0);
expectTrue(null.equals(null));
expectFalse(null.notEquals(null));
expectTrue(null.strictEquals(null));
expectFalse(null.strictNotEquals(null));
expect(null.and(null), null);
expect(null.or(null), null);
Expect.isTrue(null.not);
Expect.isFalse(null.isTruthy);
Expect.isFalse(null.lessThan(null));
Expect.isTrue(null.lessThanOrEqualTo(null));
Expect.isFalse(null.greaterThan(null));
Expect.isTrue(null.greaterThanOrEqualTo(null));
expectTrue(null.not);
expectFalse(null.isTruthy);
expectFalse(null.lessThan(null));
expectTrue(null.lessThanOrEqualTo(null));
expectFalse(null.greaterThan(null));
expectTrue(null.greaterThanOrEqualTo(null));
// Different types.
final b10 = 10.toBigInt;
@ -118,20 +130,20 @@ void dartJsInteropOperatorsTest() {
expect(b10.exponentiate(2.toBigInt), 100.toBigInt);
// Note that `unsignedRightShift` can not be used with BigInts and always
// returns a number.
expect(t.unsignedRightShift(f), 1.toJS);
expect(toInt(t.unsignedRightShift(f)), 1);
final b1 = 1.toBigInt;
Expect.isTrue(b1.equals(t));
Expect.isFalse(b1.notEquals(t));
Expect.isFalse(b1.strictEquals(t));
Expect.isTrue(b1.strictNotEquals(t));
expectTrue(b1.equals(t));
expectFalse(b1.notEquals(t));
expectFalse(b1.strictEquals(t));
expectTrue(b1.strictNotEquals(t));
expect(b10.and(b1), b1);
expect(b10.or(b1), b10);
Expect.isFalse(b10.not);
Expect.isTrue(b10.isTruthy);
Expect.isFalse(b10.lessThan(b10));
Expect.isTrue(b10.lessThanOrEqualTo(b10));
Expect.isFalse(b10.greaterThan(b10));
Expect.isTrue(b10.greaterThanOrEqualTo(b10));
expectFalse(b10.not);
expectTrue(b10.isTruthy);
expectFalse(b10.lessThan(b10));
expectTrue(b10.lessThanOrEqualTo(b10));
expectFalse(b10.greaterThan(b10));
expectTrue(b10.greaterThanOrEqualTo(b10));
}
void main() {