LibJS: Make put_own_property_by_index closer to spec

Most of the code is taken from put_own_property however the attributes
  need to be handled slightly differently it seems

4 files changed, 113 insertions, 10 deletions

diff --git a/Userland/Libraries/LibJS/CMakeLists.txt b/Userland/Libraries/LibJS/CMakeLists.txt
index 8aeb7bbf63..f5361080ab 100644
--- a/Userland/Libraries/LibJS/CMakeLists.txt
+++ b/Userland/Libraries/LibJS/CMakeLists.txt
@@ -96,6 +96,7 @@ set(SOURCES
     Runtime/PromisePrototype.cpp
     Runtime/PromiseReaction.cpp
     Runtime/PromiseResolvingFunction.cpp
+    Runtime/PropertyAttributes.cpp
     Runtime/ProxyConstructor.cpp
     Runtime/ProxyObject.cpp
     Runtime/Reference.cpp
diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp
index 2997928da3..9ba3728e07 100644
--- a/Userland/Libraries/LibJS/Runtime/Object.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Object.cpp
@@ -742,23 +742,72 @@ bool Object::put_own_property_by_index(u32 property_index, Value value, Property
             attributes.set_has_setter();
     }
 
-    PropertyAttributes existing_attributes = new_property ? 0 : existing_property.value().attributes;
+    if (new_property) {
+        if (!is_extensible()) {
+            dbgln_if(OBJECT_DEBUG, "Disallow define_property of non-extensible object");
+            if (throw_exceptions)
+                vm().throw_exception<TypeError>(global_object(), ErrorType::NonExtensibleDefine, property_index);
+            return false;
+        }
 
-    if (!new_property && mode == PutOwnPropertyMode::DefineProperty && !existing_attributes.is_configurable() && attributes != existing_attributes) {
-        dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
-        if (throw_exceptions)
-            vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_index);
-        return false;
+        m_indexed_properties.put(this, property_index, value, attributes, mode == PutOwnPropertyMode::Put ? AllowSideEffects::Yes : AllowSideEffects::No);
+
+        return true;
     }
 
-    auto value_here = new_property ? Value() : existing_property.value().value;
-    if (!new_property && mode == PutOwnPropertyMode::Put && !value_here.is_accessor() && !existing_attributes.is_writable()) {
+    if (attributes == 0 && value.is_empty())
+        return true;
+
+    PropertyAttributes existing_attributes = existing_property.value().attributes;
+    auto value_here = existing_property.value().value;
+
+    if (mode == PutOwnPropertyMode::DefineProperty && !existing_attributes.is_configurable()) {
+        if ((attributes.has_configurable() && attributes.is_configurable()) || (attributes.has_enumerable() && attributes.is_enumerable() != existing_attributes.is_enumerable())) {
+            dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
+            if (throw_exceptions)
+                vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_index);
+            return false;
+        }
+
+        if (value_here.is_accessor() != value.is_accessor()) {
+            dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
+            if (throw_exceptions)
+                vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_index);
+            return false;
+        }
+
+        if (!value_here.is_accessor() && !existing_attributes.is_writable() && ((attributes.has_writable() && attributes.is_writable()) || (!value.is_empty() && !same_value(value, value_here)))) {
+            dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
+            if (throw_exceptions)
+                vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_index);
+            return false;
+        }
+
+        if (value_here.is_accessor() && ((attributes.has_setter() && value.as_accessor().setter() != value_here.as_accessor().setter()) || (attributes.has_getter() && value.as_accessor().getter() != value_here.as_accessor().getter()))) {
+            dbgln_if(OBJECT_DEBUG, "Disallow reconfig of non-configurable property");
+            if (throw_exceptions)
+                vm().throw_exception<TypeError>(global_object(), ErrorType::DescChangeNonConfigurable, property_index);
+            return false;
+        }
+    }
+
+    if (mode == PutOwnPropertyMode::Put && !value_here.is_accessor() && !existing_attributes.is_writable()) {
         dbgln_if(OBJECT_DEBUG, "Disallow write to non-writable property");
+        if (throw_exceptions)
+            vm().throw_exception<TypeError>(global_object(), ErrorType::DescWriteNonWritable, property_index);
         return false;
     }
 
-    if (value.is_empty())
-        return true;
+    PropertyAttributes combined_attributes = existing_attributes.overwrite(attributes);
+
+    if (value.is_empty()) {
+        if (combined_attributes == existing_attributes) {
+            return true;
+        }
+        value = value_here.value_or(js_undefined());
+    }
+
+    attributes = combined_attributes;
 
     if (value_here.is_native_property()) {
         call_native_property_setter(value_here.as_native_property(), this, value);
diff --git a/Userland/Libraries/LibJS/Runtime/PropertyAttributes.cpp b/Userland/Libraries/LibJS/Runtime/PropertyAttributes.cpp
new file mode 100644
index 0000000000..5a68b567d3
--- /dev/null
+++ b/Userland/Libraries/LibJS/Runtime/PropertyAttributes.cpp
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2021, David Tuin <david.tuin@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include "PropertyAttributes.h"
+
+namespace JS {
+
+PropertyAttributes PropertyAttributes::overwrite(PropertyAttributes attr) const
+{
+    PropertyAttributes combined = m_bits;
+    if (attr.has_configurable()) {
+        if (attr.is_configurable()) {
+            combined.set_configurable();
+        } else {
+            combined.m_bits &= ~Attribute::Configurable;
+        }
+        combined.set_has_configurable();
+    }
+
+    if (attr.has_enumerable()) {
+        if (attr.is_enumerable()) {
+            combined.set_enumerable();
+        } else {
+            combined.m_bits &= ~Attribute::Enumerable;
+        }
+        combined.set_has_configurable();
+    }
+
+    if (attr.has_writable()) {
+        if (attr.is_writable()) {
+            combined.set_writable();
+        } else {
+            combined.m_bits &= ~Attribute::Writable;
+        }
+        combined.set_has_writable();
+    }
+
+    if (attr.has_getter()) {
+        combined.set_has_getter();
+    }
+
+    if (attr.has_setter()) {
+        combined.set_has_setter();
+    }
+    return combined;
+}
+
+}
diff --git a/Userland/Libraries/LibJS/Runtime/PropertyAttributes.h b/Userland/Libraries/LibJS/Runtime/PropertyAttributes.h
index 161cd8522b..44792ee9a7 100644
--- a/Userland/Libraries/LibJS/Runtime/PropertyAttributes.h
+++ b/Userland/Libraries/LibJS/Runtime/PropertyAttributes.h
@@ -61,6 +61,8 @@ public:
     bool operator==(const PropertyAttributes& other) const { return m_bits == other.m_bits; }
     bool operator!=(const PropertyAttributes& other) const { return m_bits != other.m_bits; }
 
+    PropertyAttributes overwrite(PropertyAttributes attr) const;
+
     u8 bits() const { return m_bits; }
 
 private: