GDScript: Improve usability of setter chains

- Consider PackedArrays non-shared since they are copied on C++/script
  boundaries.
- Add error messages in the analyzer when assigning to read-only
  properties.
- Add specific error message at runtime when assignment fails because
  the property is read-only.

12 files changed, 91 insertions, 9 deletions

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index cdb4f29854..fd04d3c660 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -2240,6 +2240,28 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 	if (assignee_type.is_constant || (p_assignment->assignee->type == GDScriptParser::Node::SUBSCRIPT && static_cast<GDScriptParser::SubscriptNode *>(p_assignment->assignee)->base->is_constant)) {
 		push_error("Cannot assign a new value to a constant.", p_assignment->assignee);
+		return;
+	} else if (assignee_type.is_read_only) {
+		push_error("Cannot assign a new value to a read-only property.", p_assignment->assignee);
+		return;
+	} else if (p_assignment->assignee->type == GDScriptParser::Node::SUBSCRIPT) {
+		GDScriptParser::SubscriptNode *sub = static_cast<GDScriptParser::SubscriptNode *>(p_assignment->assignee);
+		while (sub) {
+			const GDScriptParser::DataType &base_type = sub->base->datatype;
+			if (base_type.is_hard_type() && base_type.is_read_only) {
+				if (base_type.kind == GDScriptParser::DataType::BUILTIN && !Variant::is_type_shared(base_type.builtin_type)) {
+					push_error("Cannot assign a new value to a read-only property.", p_assignment->assignee);
+					return;
+				}
+			} else {
+				break;
+			}
+			if (sub->base->type == GDScriptParser::Node::SUBSCRIPT) {
+				sub = static_cast<GDScriptParser::SubscriptNode *>(sub->base);
+			} else {
+				sub = nullptr;
+			}
+		}
 	}
 
 	// Check if assigned value is an array literal, so we can make it a typed array too if appropriate.
@@ -3329,7 +3351,8 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
 			StringName getter_name = ClassDB::get_property_getter(native, name);
 			MethodBind *getter = ClassDB::get_method(native, getter_name);
 			if (getter != nullptr) {
-				p_identifier->set_datatype(type_from_property(getter->get_return_info()));
+				bool has_setter = ClassDB::get_property_setter(native, name) != StringName();
+				p_identifier->set_datatype(type_from_property(getter->get_return_info(), false, !has_setter));
 				p_identifier->source = GDScriptParser::IdentifierNode::INHERITED_VARIABLE;
 			}
 			return;
@@ -4268,8 +4291,9 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_metatype(const GDScriptPars
 	return result;
 }
 
-GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo &p_property, bool p_is_arg) const {
+GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo &p_property, bool p_is_arg, bool p_is_readonly) const {
 	GDScriptParser::DataType result;
+	result.is_read_only = p_is_readonly;
 	result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
 	if (p_property.type == Variant::NIL && (p_is_arg || (p_property.usage & PROPERTY_USAGE_NIL_IS_VARIANT))) {
 		// Variant
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index b51564fb0a..75d52509a4 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -115,7 +115,7 @@ class GDScriptAnalyzer {
 	Array make_array_from_element_datatype(const GDScriptParser::DataType &p_element_datatype, const GDScriptParser::Node *p_source_node = nullptr);
 	GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);
 	static GDScriptParser::DataType type_from_metatype(const GDScriptParser::DataType &p_meta_type);
-	GDScriptParser::DataType type_from_property(const PropertyInfo &p_property, bool p_is_arg = false) const;
+	GDScriptParser::DataType type_from_property(const PropertyInfo &p_property, bool p_is_arg = false, bool p_is_readonly = false) const;
 	GDScriptParser::DataType make_global_class_meta_type(const StringName &p_class_name, const GDScriptParser::Node *p_source);
 	bool get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg);
 	bool function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg);
diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h
index 07dac25ec5..bc0fe58fa7 100644
--- a/modules/gdscript/gdscript_parser.h
+++ b/modules/gdscript/gdscript_parser.h
@@ -122,6 +122,7 @@ public:
 		TypeSource type_source = UNDETECTED;
 
 		bool is_constant = false;
+		bool is_read_only = false;
 		bool is_meta_type = false;
 		bool is_coroutine = false; // For function calls.
 
@@ -206,6 +207,7 @@ public:
 		void operator=(const DataType &p_other) {
 			kind = p_other.kind;
 			type_source = p_other.type_source;
+			is_read_only = p_other.is_read_only;
 			is_constant = p_other.is_constant;
 			is_meta_type = p_other.is_meta_type;
 			is_coroutine = p_other.is_coroutine;
diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp
index e18a4a6190..b99f5d2685 100644
--- a/modules/gdscript/gdscript_vm.cpp
+++ b/modules/gdscript/gdscript_vm.cpp
@@ -811,13 +811,22 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 #ifdef DEBUG_ENABLED
 				if (!valid) {
+					Object *obj = dst->get_validated_object();
 					String v = index->operator String();
-					if (!v.is_empty()) {
-						v = "'" + v + "'";
+					bool read_only_property = false;
+					if (obj) {
+						read_only_property = ClassDB::has_property(obj->get_class_name(), v) && (ClassDB::get_property_setter(obj->get_class_name(), v) == StringName());
+					}
+					if (read_only_property) {
+						err_text = vformat(R"(Cannot set value into property "%s" (on base "%s") because it is read-only.)", v, _get_var_type(dst));
 					} else {
-						v = "of type '" + _get_var_type(index) + "'";
+						if (!v.is_empty()) {
+							v = "'" + v + "'";
+						} else {
+							v = "of type '" + _get_var_type(index) + "'";
+						}
+						err_text = "Invalid set index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'";
 					}
-					err_text = "Invalid set index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'";
 					OPCODE_BREAK;
 				}
 #endif
@@ -1003,8 +1012,16 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 #ifdef DEBUG_ENABLED
 				if (!valid) {
-					String err_type;
-					err_text = "Invalid set index '" + String(*index) + "' (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'.";
+					Object *obj = dst->get_validated_object();
+					bool read_only_property = false;
+					if (obj) {
+						read_only_property = ClassDB::has_property(obj->get_class_name(), *index) && (ClassDB::get_property_setter(obj->get_class_name(), *index) == StringName());
+					}
+					if (read_only_property) {
+						err_text = vformat(R"(Cannot set value into property "%s" (on base "%s") because it is read-only.)", String(*index), _get_var_type(dst));
+					} else {
+						err_text = "Invalid set index '" + String(*index) + "' (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'.";
+					}
 					OPCODE_BREAK;
 				}
 #endif
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property.gd b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property.gd
new file mode 100644
index 0000000000..2b1c4c9594
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property.gd
@@ -0,0 +1,4 @@
+func test():
+	var tree := SceneTree.new()
+	tree.root = Window.new()
+	tree.free()
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property.out b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property.out
new file mode 100644
index 0000000000..b236d70ec8
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Cannot assign a new value to a read-only property.
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property_indirectly.gd b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property_indirectly.gd
new file mode 100644
index 0000000000..c97ee0ea69
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property_indirectly.gd
@@ -0,0 +1,4 @@
+func test():
+	var state := PhysicsDirectBodyState3DExtension.new()
+	state.center_of_mass.x += 1.0
+	state.free()
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property_indirectly.out b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property_indirectly.out
new file mode 100644
index 0000000000..b236d70ec8
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/assign_to_read_only_property_indirectly.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Cannot assign a new value to a read-only property.
diff --git a/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property.gd b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property.gd
new file mode 100644
index 0000000000..19c4186622
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property.gd
@@ -0,0 +1,7 @@
+func test():
+	var state = PhysicsDirectBodyState3DExtension.new()
+	assign(state)
+	state.free()
+
+func assign(state):
+	state.center_of_mass.x -= 1.0
diff --git a/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property.out b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property.out
new file mode 100644
index 0000000000..c181c5dd02
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property.out
@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: assign()
+>> runtime/assign_to_read_only_property.gd
+>> 7
+>> Cannot set value into property "center_of_mass" (on base "PhysicsDirectBodyState3DExtension") because it is read-only.
diff --git a/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property_with_variable_index.gd b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property_with_variable_index.gd
new file mode 100644
index 0000000000..f15f580272
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property_with_variable_index.gd
@@ -0,0 +1,8 @@
+func test():
+	var state = PhysicsDirectBodyState3DExtension.new()
+	var prop = &"center_of_mass"
+	assign(state, prop)
+	state.free()
+
+func assign(state, prop):
+	state[prop].x = 1.0
diff --git a/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property_with_variable_index.out b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property_with_variable_index.out
new file mode 100644
index 0000000000..2cdc81aacc
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/assign_to_read_only_property_with_variable_index.out
@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: assign()
+>> runtime/assign_to_read_only_property_with_variable_index.gd
+>> 8
+>> Cannot set value into property "center_of_mass" (on base "PhysicsDirectBodyState3DExtension") because it is read-only.