Explorar o código

LibJS: Improve error messages for primitive strict mode property access

Using ErrorType::ReferencePrimitiveSetProperty the errors for primitives
now look like "Cannot set property 'foo' of number '123'".

The strict-mode-errors test has been adjusted and re-enabled.
Simon Wanner hai 1 ano
pai
achega
eaf8c2e398

+ 10 - 2
Userland/Libraries/LibJS/Bytecode/CommonImplementations.cpp

@@ -147,6 +147,11 @@ ThrowCompletionOr<Value> get_global(Bytecode::Interpreter& interpreter, Identifi
 
 ThrowCompletionOr<void> put_by_property_key(VM& vm, Value base, Value this_value, Value value, PropertyKey name, Op::PropertyKind kind)
 {
+    // Better error message than to_object would give
+    if (vm.in_strict_mode() && base.is_nullish())
+        return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishSetProperty, name, base.to_string_without_side_effects());
+
+    // a. Let baseObj be ? ToObject(V.[[Base]]).
     auto object = TRY(base.to_object(vm));
     if (kind == Op::PropertyKind::Getter || kind == Op::PropertyKind::Setter) {
         // The generator should only pass us functions for getters and setters.
@@ -169,8 +174,11 @@ ThrowCompletionOr<void> put_by_property_key(VM& vm, Value base, Value this_value
     }
     case Op::PropertyKind::KeyValue: {
         bool succeeded = TRY(object->internal_set(name, value, this_value));
-        if (!succeeded && vm.in_strict_mode())
-            return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishSetProperty, name, base.to_string_without_side_effects());
+        if (!succeeded && vm.in_strict_mode()) {
+            if (base.is_object())
+                return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishSetProperty, name, base.to_string_without_side_effects());
+            return vm.throw_completion<TypeError>(ErrorType::ReferencePrimitiveSetProperty, name, base.typeof(), base.to_string_without_side_effects());
+        }
         break;
     }
     case Op::PropertyKind::DirectKeyValue:

+ 1 - 1
Userland/Libraries/LibJS/Tests/builtins/Symbol/Symbol.js

@@ -23,5 +23,5 @@ test("setting new properties on a symbol is an error in strict mode", () => {
     var symbol = Symbol("foo");
     expect(() => {
         symbol.bar = 42;
-    }).toThrowWithMessage(TypeError, "Cannot set property 'bar' of Symbol(foo)");
+    }).toThrowWithMessage(TypeError, "Cannot set property 'bar' of symbol 'Symbol(foo)'");
 });

+ 8 - 12
Userland/Libraries/LibJS/Tests/strict-mode-errors.js

@@ -1,23 +1,19 @@
 "use strict";
 
-test.xfail("basic functionality", () => {
-    [true, false, "foo", 123].forEach(primitive => {
+test("basic functionality", () => {
+    [true, false, "foo", 123, 123n, null, undefined].forEach(primitive => {
+        let description = `${typeof primitive} '${primitive}${
+            typeof primitive == "bigint" ? "n" : ""
+        }'`;
+        if (primitive == null) description = String(primitive);
         expect(() => {
             primitive.foo = "bar";
-        }).toThrowWithMessage(TypeError, `Cannot set property 'foo' of ${primitive}`);
+        }).toThrowWithMessage(TypeError, `Cannot set property 'foo' of ${description}`);
         expect(() => {
             primitive[Symbol.hasInstance] = 123;
         }).toThrowWithMessage(
             TypeError,
-            `Cannot set property 'Symbol(Symbol.hasInstance)' of ${primitive}`
+            `Cannot set property 'Symbol(Symbol.hasInstance)' of ${description}`
         );
     });
-    [null, undefined].forEach(primitive => {
-        expect(() => {
-            primitive.foo = "bar";
-        }).toThrowWithMessage(TypeError, `${primitive} cannot be converted to an object`);
-        expect(() => {
-            primitive[Symbol.hasInstance] = 123;
-        }).toThrowWithMessage(TypeError, `${primitive} cannot be converted to an object`);
-    });
 });