diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml
index ddcef3340e..4ffc925a3e 100644
--- a/doc/classes/ProjectSettings.xml
+++ b/doc/classes/ProjectSettings.xml
@@ -556,6 +556,9 @@
When set to [b]Warn[/b] or [b]Error[/b], produces a warning or an error respectively when an identifier that will be shadowed below in the block is used.
+
+ When set to [b]Warn[/b] or [b]Error[/b], produces a warning or an error respectively when a built-in property of type [code]Packed*Array[/code] is modified using a complex assignment chain or a non-[code]const[/code] method call. In this case, you are only modifying a temporary value, and the property's value remains unchanged.
+
When set to [b]Warn[/b] or [b]Error[/b], produces a warning or an error respectively when deprecated keywords are used.
[b]Note:[/b] There are currently no deprecated keywords, so this warning is never produced.
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 801776763c..25ca6f5f2c 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -2905,6 +2905,8 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
}
}
}
+
+ warn_confusable_temporary_modification(p_assignment);
#endif // DEBUG_ENABLED
if (p_assignment->assigned_value == nullptr || p_assignment->assignee == nullptr) {
@@ -3586,6 +3588,10 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
base_type = subscript->base->get_datatype();
is_self = subscript->base->type == GDScriptParser::Node::SELF;
}
+
+#ifdef DEBUG_ENABLED
+ warn_confusable_temporary_modification(subscript);
+#endif // DEBUG_ENABLED
} else {
// Invalid call. Error already sent in parser.
// TODO: Could check if Callable here too.
@@ -5127,6 +5133,10 @@ void GDScriptAnalyzer::reduce_subscript(GDScriptParser::SubscriptNode *p_subscri
}
p_subscript->set_datatype(result_type);
+
+#ifdef DEBUG_ENABLED
+ warn_confusable_temporary_modification(p_subscript);
+#endif // DEBUG_ENABLED
}
void GDScriptAnalyzer::reduce_ternary_op(GDScriptParser::TernaryOpNode *p_ternary_op, bool p_is_root) {
@@ -6137,6 +6147,7 @@ void GDScriptAnalyzer::validate_call_arg(const List &p
}
#ifdef DEBUG_ENABLED
+
void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope) {
const StringName &name = p_identifier->name;
@@ -6215,6 +6226,62 @@ void GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_identifier
native_base_class = ClassDB::get_parent_class(native_base_class);
}
}
+
+void GDScriptAnalyzer::warn_confusable_temporary_modification(GDScriptParser::ExpressionNode *p_expression) {
+ ERR_FAIL_NULL(p_expression);
+
+ GDScriptParser::ExpressionNode *current = nullptr;
+ if (p_expression->type == GDScriptParser::Node::ASSIGNMENT) {
+ current = static_cast(p_expression)->assignee;
+ } else if (p_expression->type == GDScriptParser::Node::SUBSCRIPT) {
+ current = static_cast(p_expression);
+ } else {
+ ERR_FAIL_MSG("GDScript bug: Invalid expression type.");
+ }
+
+ while (current && current->type == GDScriptParser::Node::SUBSCRIPT) {
+ GDScriptParser::SubscriptNode *subscript = static_cast(current);
+ GDScriptParser::ExpressionNode *base = subscript->base;
+ if (base && base->datatype.is_typed_container_type() && !base->datatype.is_meta_type) {
+ GDScriptParser::IdentifierNode *base_id = nullptr;
+ GDScriptParser::DataType scope_type;
+ if (base->type == GDScriptParser::Node::IDENTIFIER) {
+ base_id = static_cast(base);
+ scope_type = type_from_metatype(parser->current_class->datatype);
+ } else if (base->type == GDScriptParser::Node::SUBSCRIPT) {
+ GDScriptParser::SubscriptNode *base_subscript = static_cast(base);
+ if (base_subscript->is_attribute) {
+ base_id = base_subscript->attribute;
+ if (base_subscript->base) {
+ scope_type = base_subscript->base->datatype;
+ }
+ }
+ }
+
+ if (base_id && base_id->source == GDScriptParser::IdentifierNode::INHERITED_VARIABLE) {
+ const StringName &scope_native_type = scope_type.native_type;
+ if (class_exists(scope_native_type) && ClassDB::has_property(scope_native_type, base_id->name)) {
+ if (p_expression->type == GDScriptParser::Node::ASSIGNMENT) {
+ parser->push_warning(p_expression, GDScriptWarning::CONFUSABLE_TEMPORARY_MODIFICATION, scope_native_type, base_id->name);
+ } else if (p_expression->type == GDScriptParser::Node::SUBSCRIPT) {
+ StringName member;
+ if (subscript->is_attribute && subscript->attribute) {
+ member = subscript->attribute->name;
+ }
+
+ const Variant::Type builtin_type = base->datatype.builtin_type;
+ if (member && Variant::has_builtin_method(builtin_type, member) && !Variant::is_builtin_method_const(builtin_type, member)) {
+ parser->push_warning(subscript, GDScriptWarning::CONFUSABLE_TEMPORARY_MODIFICATION, scope_native_type, base_id->name, member);
+ }
+ }
+ }
+ }
+ }
+
+ current = base;
+ }
+}
+
#endif // DEBUG_ENABLED
GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, bool &r_valid, const GDScriptParser::Node *p_source) {
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index 94df53d521..aace74f847 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -157,9 +157,11 @@ class GDScriptAnalyzer {
Ref find_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref &p_dependant_parser);
Ref find_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser);
Ref get_depended_shallow_script(const String &p_path, Error &r_error);
+
#ifdef DEBUG_ENABLED
void is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope);
-#endif
+ void warn_confusable_temporary_modification(GDScriptParser::ExpressionNode *p_expression);
+#endif // DEBUG_ENABLED
public:
Error resolve_inheritance();
diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp
index 9dbabba8da..9270b8f09f 100644
--- a/modules/gdscript/gdscript_warning.cpp
+++ b/modules/gdscript/gdscript_warning.cpp
@@ -154,6 +154,12 @@ String GDScriptWarning::get_message() const {
case CONFUSABLE_CAPTURE_REASSIGNMENT:
CHECK_SYMBOLS(1);
return vformat(R"(Reassigning lambda capture does not modify the outer local variable "%s".)", symbols[0]);
+ case CONFUSABLE_TEMPORARY_MODIFICATION:
+ CHECK_SYMBOLS(2);
+ if (symbols.size() > 2) {
+ return vformat(R"*(The built-in property "%s" will not be modified as a result of calling the method "%s()". Consider assigning the desired value to the property instead, or check the "%s" class for a specialized API.)*", symbols[1], symbols[2], symbols[0]);
+ }
+ return vformat(R"(The built-in property "%s" will not be modified in the assignment chain. Consider using a temporary variable and assigning it back instead, or check the "%s" class for a specialized API.)", symbols[1], symbols[0]);
case INFERENCE_ON_VARIANT:
CHECK_SYMBOLS(1);
return vformat("The %s type is being inferred from a Variant value, so it will be typed as Variant.", symbols[0]);
@@ -238,6 +244,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
PNAME("CONFUSABLE_LOCAL_DECLARATION"),
PNAME("CONFUSABLE_LOCAL_USAGE"),
PNAME("CONFUSABLE_CAPTURE_REASSIGNMENT"),
+ PNAME("CONFUSABLE_TEMPORARY_MODIFICATION"),
PNAME("INFERENCE_ON_VARIANT"),
PNAME("NATIVE_METHOD_OVERRIDE"),
PNAME("GET_NODE_DEFAULT_WITHOUT_ONREADY"),
diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h
index ebb618240a..9a03b82108 100644
--- a/modules/gdscript/gdscript_warning.h
+++ b/modules/gdscript/gdscript_warning.h
@@ -87,6 +87,7 @@ public:
CONFUSABLE_LOCAL_DECLARATION, // The parent block declares an identifier with the same name below.
CONFUSABLE_LOCAL_USAGE, // The identifier will be shadowed below in the block.
CONFUSABLE_CAPTURE_REASSIGNMENT, // Reassigning lambda capture does not modify the outer local variable.
+ CONFUSABLE_TEMPORARY_MODIFICATION, // Modifying a temporary value in a complex assignment chain or calling a non-`const` method.
INFERENCE_ON_VARIANT, // The declaration uses type inference but the value is typed as Variant.
NATIVE_METHOD_OVERRIDE, // The script method overrides a native one, this may not work as intended.
GET_NODE_DEFAULT_WITHOUT_ONREADY, // A class variable uses `get_node()` (or the `$` notation) as its default value, but does not use the @onready annotation.
@@ -145,6 +146,7 @@ public:
WARN, // CONFUSABLE_LOCAL_DECLARATION
WARN, // CONFUSABLE_LOCAL_USAGE
WARN, // CONFUSABLE_CAPTURE_REASSIGNMENT
+ WARN, // CONFUSABLE_TEMPORARY_MODIFICATION
ERROR, // INFERENCE_ON_VARIANT // Most likely done by accident, usually inference is trying for a particular type.
ERROR, // NATIVE_METHOD_OVERRIDE // May not work as expected.
ERROR, // GET_NODE_DEFAULT_WITHOUT_ONREADY // May not work as expected.
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_temporary_modification.gd b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_temporary_modification.gd
new file mode 100644
index 0000000000..fa3af0c14f
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_temporary_modification.gd
@@ -0,0 +1,49 @@
+class TestNativeProperty extends Line2D:
+ func _ready() -> void:
+ points = PackedVector2Array([Vector2()])
+ points[0] = Vector2.ONE # Warn.
+ points[0].x = 2.0 # Warn.
+
+ var _size := points.size()
+ points.clear() # Warn.
+ var _size_callable := points.size
+ var _clear_callable := points.clear # Warn.
+
+ var base: TestNativeProperty = self
+
+ base.points = PackedVector2Array([Vector2()])
+ base.points[0] = Vector2.ONE # Warn.
+ base.points[0].x = 2.0 # Warn.
+
+ var _base_size := base.points.size()
+ base.points.clear() # Warn.
+ var _base_size_callable := base.points.size
+ var _base_clear_callable := base.points.clear # Warn.
+
+# No warnings for a custom property.
+class TestCustomProperty extends Node:
+ var points: PackedVector2Array
+
+ func _ready() -> void:
+ points = PackedVector2Array([Vector2()])
+ points[0] = Vector2.ONE
+ points[0].x = 2.0
+
+ var _size := points.size()
+ points.clear()
+ var _size_callable := points.size
+ var _clear_callable := points.clear
+
+ var base: TestCustomProperty = self
+
+ base.points = PackedVector2Array([Vector2()])
+ base.points[0] = Vector2.ONE
+ base.points[0].x = 2.0
+
+ var _base_size := base.points.size()
+ base.points.clear()
+ var _base_size_callable := base.points.size
+ var _base_clear_callable := base.points.clear
+
+func test():
+ pass
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/confusable_temporary_modification.out b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_temporary_modification.out
new file mode 100644
index 0000000000..cd44ac28cc
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/confusable_temporary_modification.out
@@ -0,0 +1,9 @@
+GDTEST_OK
+~~ WARNING at line 4: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified in the assignment chain. Consider using a temporary variable and assigning it back instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 5: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified in the assignment chain. Consider using a temporary variable and assigning it back instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 8: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified as a result of calling the method "clear()". Consider assigning the desired value to the property instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 10: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified as a result of calling the method "clear()". Consider assigning the desired value to the property instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 15: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified in the assignment chain. Consider using a temporary variable and assigning it back instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 16: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified in the assignment chain. Consider using a temporary variable and assigning it back instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 19: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified as a result of calling the method "clear()". Consider assigning the desired value to the property instead, or check the "Line2D" class for a specialized API.
+~~ WARNING at line 21: (CONFUSABLE_TEMPORARY_MODIFICATION) The built-in property "points" will not be modified as a result of calling the method "clear()". Consider assigning the desired value to the property instead, or check the "Line2D" class for a specialized API.