summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGeorge Marques <george@gmarqu.es>2024-04-09 14:15:51 -0300
committerGeorge Marques <george@gmarqu.es>2024-04-10 11:59:57 -0300
commit877802e2520e03593d2e5cf76cfa7659899b1aa4 (patch)
treefd39354ece46ff416f468babd8e34d5c6f61bdd7
parenta7b860250f305f6cbaf61c30f232ff3bbdfdda0b (diff)
downloadredot-engine-877802e2520e03593d2e5cf76cfa7659899b1aa4.tar.gz
GDScript: Don't warn on unassigned for builtin-typed variables
If the type of a variable is a built-in Variant type, then it will automatically be assigned a default value based on the type. This means that the explicit initialization may be unnecessary. Thus this commit removes the warning in such case. This also changes the meaning of the unassigned warning to happen when the variable is used before being assigned, not when it has zero assignments.
-rw-r--r--modules/gdscript/gdscript_analyzer.cpp29
-rw-r--r--modules/gdscript/gdscript_parser.cpp22
-rw-r--r--modules/gdscript/gdscript_warning.cpp2
-rw-r--r--modules/gdscript/tests/scripts/analyzer/features/enum_typecheck_inner_class.gd3
-rw-r--r--modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.gd7
-rw-r--r--modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.out2
-rw-r--r--modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.gd2
-rw-r--r--modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.gd11
-rw-r--r--modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.out15
-rw-r--r--modules/gdscript/tests/scripts/runtime/features/reset_unassigned_variables_in_loops.gd2
10 files changed, 65 insertions, 30 deletions
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index f67f4913c3..a51b5f90f9 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -1973,8 +1973,6 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
if (p_is_local) {
if (p_variable->usages == 0 && !String(p_variable->identifier->name).begins_with("_")) {
parser->push_warning(p_variable, GDScriptWarning::UNUSED_VARIABLE, p_variable->identifier->name);
- } else if (p_variable->assignments == 0) {
- parser->push_warning(p_variable, GDScriptWarning::UNASSIGNED_VARIABLE, p_variable->identifier->name);
}
}
is_shadowing(p_variable->identifier, kind, p_is_local);
@@ -2615,9 +2613,21 @@ void GDScriptAnalyzer::update_array_literal_element_type(GDScriptParser::ArrayNo
}
void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assignment) {
- reduce_expression(p_assignment->assignee);
reduce_expression(p_assignment->assigned_value);
+#ifdef DEBUG_ENABLED
+ // Increment assignment count for local variables.
+ // Before we reduce the assignee because we don't want to warn about not being assigned when performing the assignment.
+ if (p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
+ GDScriptParser::IdentifierNode *id = static_cast<GDScriptParser::IdentifierNode *>(p_assignment->assignee);
+ if (id->source == GDScriptParser::IdentifierNode::LOCAL_VARIABLE && id->variable_source) {
+ id->variable_source->assignments++;
+ }
+ }
+#endif
+
+ reduce_expression(p_assignment->assignee);
+
if (p_assignment->assigned_value == nullptr || p_assignment->assignee == nullptr) {
return;
}
@@ -2754,6 +2764,14 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
if (assignee_type.is_hard_type() && assignee_type.builtin_type == Variant::INT && assigned_value_type.builtin_type == Variant::FLOAT) {
parser->push_warning(p_assignment->assigned_value, GDScriptWarning::NARROWING_CONVERSION);
}
+ // Check for assignment with operation before assignment.
+ if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE && p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
+ GDScriptParser::IdentifierNode *id = static_cast<GDScriptParser::IdentifierNode *>(p_assignment->assignee);
+ // Use == 1 here because this assignment was already counted in the beginning of the function.
+ if (id->source == GDScriptParser::IdentifierNode::LOCAL_VARIABLE && id->variable_source && id->variable_source->assignments == 1) {
+ parser->push_warning(p_assignment, GDScriptWarning::UNASSIGNED_VARIABLE_OP_ASSIGN, id->name);
+ }
+ }
#endif
}
@@ -3926,6 +3944,11 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
case GDScriptParser::IdentifierNode::LOCAL_VARIABLE:
p_identifier->set_datatype(p_identifier->variable_source->get_datatype());
found_source = true;
+#ifdef DEBUG_ENABLED
+ if (p_identifier->variable_source && p_identifier->variable_source->assignments == 0 && !(p_identifier->get_datatype().is_hard_type() && p_identifier->get_datatype().kind == GDScriptParser::DataType::BUILTIN)) {
+ parser->push_warning(p_identifier, GDScriptWarning::UNASSIGNED_VARIABLE, p_identifier->name);
+ }
+#endif
break;
case GDScriptParser::IdentifierNode::LOCAL_ITERATOR:
p_identifier->set_datatype(p_identifier->bind_source->get_datatype());
diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp
index d706f4e9a3..173bff575a 100644
--- a/modules/gdscript/gdscript_parser.cpp
+++ b/modules/gdscript/gdscript_parser.cpp
@@ -2769,10 +2769,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
return parse_expression(false); // Return the following expression.
}
-#ifdef DEBUG_ENABLED
- VariableNode *source_variable = nullptr;
-#endif
-
switch (p_previous_operand->type) {
case Node::IDENTIFIER: {
#ifdef DEBUG_ENABLED
@@ -2781,8 +2777,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
IdentifierNode *id = static_cast<IdentifierNode *>(p_previous_operand);
switch (id->source) {
case IdentifierNode::LOCAL_VARIABLE:
-
- source_variable = id->variable_source;
id->variable_source->usages--;
break;
case IdentifierNode::LOCAL_CONSTANT:
@@ -2813,16 +2807,10 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
update_extents(assignment);
make_completion_context(COMPLETION_ASSIGN, assignment);
-#ifdef DEBUG_ENABLED
- bool has_operator = true;
-#endif
switch (previous.type) {
case GDScriptTokenizer::Token::EQUAL:
assignment->operation = AssignmentNode::OP_NONE;
assignment->variant_op = Variant::OP_MAX;
-#ifdef DEBUG_ENABLED
- has_operator = false;
-#endif
break;
case GDScriptTokenizer::Token::PLUS_EQUAL:
assignment->operation = AssignmentNode::OP_ADDITION;
@@ -2878,16 +2866,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
}
complete_extents(assignment);
-#ifdef DEBUG_ENABLED
- if (source_variable != nullptr) {
- if (has_operator && source_variable->assignments == 0) {
- push_warning(assignment, GDScriptWarning::UNASSIGNED_VARIABLE_OP_ASSIGN, source_variable->identifier->name);
- }
-
- source_variable->assignments += 1;
- }
-#endif
-
return assignment;
}
diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp
index e7d9787eab..8549525ced 100644
--- a/modules/gdscript/gdscript_warning.cpp
+++ b/modules/gdscript/gdscript_warning.cpp
@@ -40,7 +40,7 @@ String GDScriptWarning::get_message() const {
switch (code) {
case UNASSIGNED_VARIABLE:
CHECK_SYMBOLS(1);
- return vformat(R"(The variable "%s" was used but never assigned a value.)", symbols[0]);
+ return vformat(R"(The variable "%s" was used before being assigned a value.)", symbols[0]);
case UNASSIGNED_VARIABLE_OP_ASSIGN:
CHECK_SYMBOLS(1);
return vformat(R"(Using assignment with operation but the variable "%s" was not previously assigned a value.)", symbols[0]);
diff --git a/modules/gdscript/tests/scripts/analyzer/features/enum_typecheck_inner_class.gd b/modules/gdscript/tests/scripts/analyzer/features/enum_typecheck_inner_class.gd
index 1c4b19d8e0..0444051831 100644
--- a/modules/gdscript/tests/scripts/analyzer/features/enum_typecheck_inner_class.gd
+++ b/modules/gdscript/tests/scripts/analyzer/features/enum_typecheck_inner_class.gd
@@ -11,6 +11,7 @@ class InnerClass:
var e2: InnerClass.MyEnum
var e3: EnumTypecheckOuterClass.InnerClass.MyEnum
+ @warning_ignore("unassigned_variable")
print("Self ", e1, e2, e3)
e1 = MyEnum.V1
e2 = MyEnum.V1
@@ -48,6 +49,7 @@ func test_outer_from_outer():
var e1: MyEnum
var e2: EnumTypecheckOuterClass.MyEnum
+ @warning_ignore("unassigned_variable")
print("Self ", e1, e2)
e1 = MyEnum.V1
e2 = MyEnum.V1
@@ -66,6 +68,7 @@ func test_inner_from_outer():
var e1: InnerClass.MyEnum
var e2: EnumTypecheckOuterClass.InnerClass.MyEnum
+ @warning_ignore("unassigned_variable")
print("Inner ", e1, e2)
e1 = InnerClass.MyEnum.V1
e2 = InnerClass.MyEnum.V1
diff --git a/modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.gd b/modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.gd
new file mode 100644
index 0000000000..8099b366f3
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.gd
@@ -0,0 +1,7 @@
+# GH-88117, GH-85796
+
+func test():
+ var array: Array
+ # Should not emit unassigned warning because the Array type has a default value.
+ array.assign([1, 2, 3])
+ print(array)
diff --git a/modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.out b/modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.out
new file mode 100644
index 0000000000..6d85a6cc07
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/features/unassigned_builtin_typed.out
@@ -0,0 +1,2 @@
+GDTEST_OK
+[1, 2, 3]
diff --git a/modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.gd b/modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.gd
index 8a1ab6f406..333950d64e 100644
--- a/modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.gd
+++ b/modules/gdscript/tests/scripts/analyzer/features/warning_ignore_warnings.gd
@@ -31,8 +31,8 @@ func int_func() -> int:
func test_warnings(unused_private_class_variable):
var t = 1
- @warning_ignore("unassigned_variable")
var unassigned_variable
+ @warning_ignore("unassigned_variable")
print(unassigned_variable)
var _unassigned_variable_op_assign
diff --git a/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.gd b/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.gd
index afb5059eea..b38cffb754 100644
--- a/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.gd
+++ b/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.gd
@@ -1,2 +1,11 @@
func test():
- var __
+ var unassigned
+ print(unassigned)
+ unassigned = "something" # Assigned only after use.
+
+ var a
+ print(a) # Unassigned, warn.
+ if a: # Still unassigned, warn.
+ a = 1
+ print(a) # Assigned (dead code), don't warn.
+ print(a) # "Maybe" assigned, don't warn.
diff --git a/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.out b/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.out
index 10f89be132..36db304ef4 100644
--- a/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.out
+++ b/modules/gdscript/tests/scripts/parser/warnings/unassigned_variable.out
@@ -1,5 +1,16 @@
GDTEST_OK
>> WARNING
->> Line: 2
+>> Line: 3
>> UNASSIGNED_VARIABLE
->> The variable "__" was used but never assigned a value.
+>> The variable "unassigned" was used before being assigned a value.
+>> WARNING
+>> Line: 7
+>> UNASSIGNED_VARIABLE
+>> The variable "a" was used before being assigned a value.
+>> WARNING
+>> Line: 8
+>> UNASSIGNED_VARIABLE
+>> The variable "a" was used before being assigned a value.
+<null>
+<null>
+<null>
diff --git a/modules/gdscript/tests/scripts/runtime/features/reset_unassigned_variables_in_loops.gd b/modules/gdscript/tests/scripts/runtime/features/reset_unassigned_variables_in_loops.gd
index c45f8dce48..2bd5362f2a 100644
--- a/modules/gdscript/tests/scripts/runtime/features/reset_unassigned_variables_in_loops.gd
+++ b/modules/gdscript/tests/scripts/runtime/features/reset_unassigned_variables_in_loops.gd
@@ -7,6 +7,7 @@ func test():
var b
if true:
var c
+ @warning_ignore("unassigned_variable")
prints("Begin:", i, a, b, c)
a = 1
b = 1
@@ -20,6 +21,7 @@ func test():
var b
if true:
var c
+ @warning_ignore("unassigned_variable")
prints("Begin:", j, a, b, c)
a = 1
b = 1