summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGeorge Marques <george@gmarqu.es>2023-01-31 22:34:21 -0300
committerGeorge Marques <george@gmarqu.es>2023-02-01 00:05:14 -0300
commita166833bfa23a21a7bff196a85a20b014e7c1396 (patch)
tree93b95bbf2d927fb80150c8807584b9d998c2ce59
parent0810ecaafdbee3ea747219e6ab3a8de5d2216a09 (diff)
GDScript: Add warnings that are set to error by default
- Adds a list of default levels for all warning so they can be set individually. - Add warnings set by default to error for: - Using `get_node()` without `@onready`. - Using `@onready` together with `@export`. - Inferring a static type with a Variant value. - Overriding a native engine method. - Adjust how annotations to ignore warnings are treated so they also apply to method parameters. - Clean up a bit how ignored warnings are set. There were two sets but only one was actually being used. - Set all warnings to the `WARN` level for tests, so they they can be properly tested. - Fix enum types in native methods signatures being set to `int`. - Fix native enums being treated as Dictionary by mistake. - Make name of native enum types use the class they are defined in, not the direct super class of the script. This ensures they are always equal even when coming from different sources. - Fix error for signature mismatch that was only showing the first default argument as having a default. Now it shows for all.
-rw-r--r--doc/classes/ProjectSettings.xml14
-rw-r--r--modules/gdscript/gdscript_analyzer.cpp173
-rw-r--r--modules/gdscript/gdscript_analyzer.h2
-rw-r--r--modules/gdscript/gdscript_parser.cpp14
-rw-r--r--modules/gdscript/gdscript_parser.h11
-rw-r--r--modules/gdscript/gdscript_warning.cpp31
-rw-r--r--modules/gdscript/gdscript_warning.h49
-rw-r--r--modules/gdscript/tests/gdscript_test_runner.cpp6
-rw-r--r--modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out2
-rw-r--r--modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd4
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.gd15
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.out14
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.gd6
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.out6
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.gd6
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.out6
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.gd5
-rw-r--r--modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.out6
18 files changed, 313 insertions, 57 deletions
diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml
index 95bd060fc6..736bdbd7f1 100644
--- a/doc/classes/ProjectSettings.xml
+++ b/doc/classes/ProjectSettings.xml
@@ -405,9 +405,15 @@
<member name="debug/gdscript/warnings/function_used_as_property" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when using a function as if it is a property.
</member>
+ <member name="debug/gdscript/warnings/get_node_default_without_onready" type="int" setter="" getter="" default="2">
+ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when [method Node.get_node] (or the shorthand [code]$[/code]) is used as default value of a class variable without the [code]@onready[/code] annotation.
+ </member>
<member name="debug/gdscript/warnings/incompatible_ternary" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a ternary operator may emit values with incompatible types.
</member>
+ <member name="debug/gdscript/warnings/inference_on_variant" type="int" setter="" getter="" default="2">
+ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a static inferred type uses a [Variant] as initial value, which makes the static type to also be Variant.
+ </member>
<member name="debug/gdscript/warnings/int_as_enum_without_cast" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when trying to use an integer as an enum without an explicit cast.
</member>
@@ -420,6 +426,12 @@
<member name="debug/gdscript/warnings/narrowing_conversion" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when passing a floating-point value to a function that expects an integer (it will be converted and lose precision).
</member>
+ <member name="debug/gdscript/warnings/native_method_override" type="int" setter="" getter="" default="2">
+ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a method in the script overrides a native method, because it may not behave as expected.
+ </member>
+ <member name="debug/gdscript/warnings/onready_with_export" type="int" setter="" getter="" default="2">
+ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when the [code]@onready[/code] annotation is used together with the [code]@export[/code] annotation, since it may not behave as expected.
+ </member>
<member name="debug/gdscript/warnings/property_used_as_function" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when using a property as if it is a function.
</member>
@@ -477,7 +489,7 @@
<member name="debug/gdscript/warnings/unsafe_property_access" type="int" setter="" getter="" default="0">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when accessing a property whose presence is not guaranteed at compile-time in the class.
</member>
- <member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="0">
+ <member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="1">
When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when returning a call from a [code]void[/code] function when such call cannot be guaranteed to be also [code]void[/code].
</member>
<member name="debug/gdscript/warnings/unused_local_constant" type="int" setter="" getter="" default="1">
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 1c2b743909..2f0336bc76 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -138,13 +138,25 @@ static GDScriptParser::DataType make_enum_type(const StringName &p_enum_name, co
}
static GDScriptParser::DataType make_native_enum_type(const StringName &p_enum_name, const StringName &p_native_class, const bool p_meta = true) {
- GDScriptParser::DataType type = make_enum_type(p_enum_name, p_native_class, p_meta);
+ // Find out which base class declared the enum, so the name is always the same even when coming from other contexts.
+ StringName native_base = p_native_class;
+ while (true && native_base != StringName()) {
+ if (ClassDB::has_enum(native_base, p_enum_name, true)) {
+ break;
+ }
+ native_base = ClassDB::get_parent_class_nocheck(native_base);
+ }
+
+ GDScriptParser::DataType type = make_enum_type(p_enum_name, native_base, p_meta);
+ if (p_meta) {
+ type.builtin_type = Variant::NIL; // Native enum types are not Dictionaries
+ }
List<StringName> enum_values;
- ClassDB::get_enum_constants(p_native_class, p_enum_name, &enum_values);
+ ClassDB::get_enum_constants(native_base, p_enum_name, &enum_values, true);
for (const StringName &E : enum_values) {
- type.enum_values[E] = ClassDB::get_integer_constant(p_native_class, E);
+ type.enum_values[E] = ClassDB::get_integer_constant(native_base, E);
}
return type;
@@ -782,6 +794,22 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
resolving_datatype.kind = GDScriptParser::DataType::RESOLVING;
{
+#ifdef DEBUG_ENABLED
+ HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+ GDScriptParser::Node *member_node = member.get_source_node();
+ if (member_node && member_node->type != GDScriptParser::Node::ANNOTATION) {
+ // Apply @warning_ignore annotations before resolving member.
+ for (GDScriptParser::AnnotationNode *&E : member_node->annotations) {
+ if (E->name == SNAME("@warning_ignore")) {
+ resolve_annotation(E);
+ E->apply(parser, member.variable);
+ }
+ }
+ for (GDScriptWarning::Code ignored_warning : member_node->ignored_warnings) {
+ parser->ignored_warnings.insert(ignored_warning);
+ }
+ }
+#endif
switch (member.type) {
case GDScriptParser::ClassNode::Member::VARIABLE: {
check_class_member_name_conflict(p_class, member.variable->identifier->name, member.variable);
@@ -790,9 +818,16 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
// Apply annotations.
for (GDScriptParser::AnnotationNode *&E : member.variable->annotations) {
- resolve_annotation(E);
- E->apply(parser, member.variable);
+ if (E->name != SNAME("@warning_ignore")) {
+ resolve_annotation(E);
+ E->apply(parser, member.variable);
+ }
+ }
+#ifdef DEBUG_ENABLED
+ if (member.variable->exported && member.variable->onready) {
+ parser->push_warning(member.variable, GDScriptWarning::ONREADY_WITH_EXPORT);
}
+#endif
} break;
case GDScriptParser::ClassNode::Member::CONSTANT: {
check_class_member_name_conflict(p_class, member.constant->identifier->name, member.constant);
@@ -878,6 +913,10 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
}
} break;
case GDScriptParser::ClassNode::Member::FUNCTION:
+ for (GDScriptParser::AnnotationNode *&E : member.function->annotations) {
+ resolve_annotation(E);
+ E->apply(parser, member.function);
+ }
resolve_function_signature(member.function, p_source);
break;
case GDScriptParser::ClassNode::Member::ENUM_VALUE: {
@@ -931,6 +970,9 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
ERR_PRINT("Trying to resolve undefined member.");
break;
}
+#ifdef DEBUG_ENABLED
+ parser->ignored_warnings = previously_ignored_warnings;
+#endif
}
parser->current_class = previous_class;
@@ -1059,19 +1101,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
resolve_annotation(E);
E->apply(parser, member.function);
}
-
-#ifdef DEBUG_ENABLED
- HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
- for (uint32_t ignored_warning : member.function->ignored_warnings) {
- parser->ignored_warning_codes.insert(ignored_warning);
- }
-#endif // DEBUG_ENABLED
-
resolve_function_body(member.function);
-
-#ifdef DEBUG_ENABLED
- parser->ignored_warning_codes = previously_ignored;
-#endif // DEBUG_ENABLED
} else if (member.type == GDScriptParser::ClassNode::Member::VARIABLE && member.variable->property != GDScriptParser::VariableNode::PROP_NONE) {
if (member.variable->property == GDScriptParser::VariableNode::PROP_INLINE) {
if (member.variable->getter != nullptr) {
@@ -1102,9 +1132,9 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
GDScriptParser::ClassNode::Member member = p_class->members[i];
if (member.type == GDScriptParser::ClassNode::Member::VARIABLE) {
#ifdef DEBUG_ENABLED
- HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
- for (uint32_t ignored_warning : member.function->ignored_warnings) {
- parser->ignored_warning_codes.insert(ignored_warning);
+ HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+ for (GDScriptWarning::Code ignored_warning : member.variable->ignored_warnings) {
+ parser->ignored_warnings.insert(ignored_warning);
}
if (member.variable->usages == 0 && String(member.variable->identifier->name).begins_with("_")) {
parser->push_warning(member.variable->identifier, GDScriptWarning::UNUSED_PRIVATE_CLASS_VARIABLE, member.variable->identifier->name);
@@ -1179,7 +1209,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
}
}
#ifdef DEBUG_ENABLED
- parser->ignored_warning_codes = previously_ignored;
+ parser->ignored_warnings = previously_ignored_warnings;
#endif // DEBUG_ENABLED
}
}
@@ -1289,6 +1319,11 @@ void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node, bool p_is_root
void GDScriptAnalyzer::resolve_annotation(GDScriptParser::AnnotationNode *p_annotation) {
ERR_FAIL_COND_MSG(!parser->valid_annotations.has(p_annotation->name), vformat(R"(Annotation "%s" not found to validate.)", p_annotation->name));
+ if (p_annotation->is_resolved) {
+ return;
+ }
+ p_annotation->is_resolved = true;
+
const MethodInfo &annotation_info = parser->valid_annotations[p_annotation->name].info;
const List<PropertyInfo>::Element *E = annotation_info.arguments.front();
@@ -1355,6 +1390,13 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
}
p_function->resolved_signature = true;
+#ifdef DEBUG_ENABLED
+ HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+ for (GDScriptWarning::Code ignored_warning : p_function->ignored_warnings) {
+ parser->ignored_warnings.insert(ignored_warning);
+ }
+#endif
+
GDScriptParser::FunctionNode *previous_function = parser->current_function;
parser->current_function = p_function;
@@ -1421,7 +1463,8 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
int default_par_count = 0;
bool is_static = false;
bool is_vararg = false;
- if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, is_static, is_vararg)) {
+ StringName native_base;
+ if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, is_static, is_vararg, &native_base)) {
bool valid = p_function->is_static == is_static;
valid = valid && parent_return_type == p_function->get_datatype();
@@ -1447,8 +1490,8 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
parameter = "Variant";
}
parent_signature += parameter;
- if (j == parameters_types.size() - default_par_count) {
- parent_signature += " = default";
+ if (j >= parameters_types.size() - default_par_count) {
+ parent_signature += " = <default>";
}
j++;
@@ -1464,6 +1507,11 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
push_error(vformat(R"(The function signature doesn't match the parent. Parent signature is "%s".)", parent_signature), p_function);
}
+#ifdef DEBUG_ENABLED
+ if (native_base != StringName()) {
+ parser->push_warning(p_function, GDScriptWarning::NATIVE_METHOD_OVERRIDE, function_name, native_base);
+ }
+#endif
}
#endif // TOOLS_ENABLED
}
@@ -1472,6 +1520,9 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
p_function->set_datatype(prev_datatype);
}
+#ifdef DEBUG_ENABLED
+ parser->ignored_warnings = previously_ignored_warnings;
+#endif
parser->current_function = previous_function;
}
@@ -1481,6 +1532,13 @@ void GDScriptAnalyzer::resolve_function_body(GDScriptParser::FunctionNode *p_fun
}
p_function->resolved_body = true;
+#ifdef DEBUG_ENABLED
+ HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+ for (GDScriptWarning::Code ignored_warning : p_function->ignored_warnings) {
+ parser->ignored_warnings.insert(ignored_warning);
+ }
+#endif
+
GDScriptParser::FunctionNode *previous_function = parser->current_function;
parser->current_function = p_function;
@@ -1498,6 +1556,9 @@ void GDScriptAnalyzer::resolve_function_body(GDScriptParser::FunctionNode *p_fun
}
}
+#ifdef DEBUG_ENABLED
+ parser->ignored_warnings = previously_ignored_warnings;
+#endif
parser->current_function = previous_function;
}
@@ -1538,16 +1599,16 @@ void GDScriptAnalyzer::resolve_suite(GDScriptParser::SuiteNode *p_suite) {
}
#ifdef DEBUG_ENABLED
- HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
- for (uint32_t ignored_warning : stmt->ignored_warnings) {
- parser->ignored_warning_codes.insert(ignored_warning);
+ HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+ for (GDScriptWarning::Code ignored_warning : stmt->ignored_warnings) {
+ parser->ignored_warnings.insert(ignored_warning);
}
#endif // DEBUG_ENABLED
resolve_node(stmt);
#ifdef DEBUG_ENABLED
- parser->ignored_warning_codes = previously_ignored;
+ parser->ignored_warnings = previously_ignored_warnings;
#endif // DEBUG_ENABLED
decide_suite_type(p_suite, stmt);
@@ -1599,6 +1660,11 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
} else if (initializer_type.kind == GDScriptParser::DataType::BUILTIN && initializer_type.builtin_type == Variant::NIL && !is_constant) {
push_error(vformat(R"(Cannot infer the type of "%s" %s because the value is "null".)", p_assignable->identifier->name, p_kind), p_assignable->initializer);
}
+#ifdef DEBUG_ENABLED
+ if (initializer_type.is_hard_type() && initializer_type.is_variant()) {
+ parser->push_warning(p_assignable, GDScriptWarning::INFERENCE_ON_VARIANT, p_kind);
+ }
+#endif
} else {
if (!initializer_type.is_set()) {
push_error(vformat(R"(Could not resolve type for %s "%s".)", p_kind, p_assignable->identifier->name), p_assignable->initializer);
@@ -1658,6 +1724,32 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
}
is_shadowing(p_variable->identifier, kind);
+ } else {
+ // Check if it is call to get_node() on self (using shorthand $ or not), so we can check if @onready is needed.
+ if (p_variable->initializer && (p_variable->initializer->type == GDScriptParser::Node::GET_NODE || p_variable->initializer->type == GDScriptParser::Node::CALL)) {
+ bool is_get_node = p_variable->initializer->type == GDScriptParser::Node::GET_NODE;
+ bool is_using_shorthand = is_get_node;
+ if (!is_get_node) {
+ is_using_shorthand = false;
+ GDScriptParser::CallNode *call = static_cast<GDScriptParser::CallNode *>(p_variable->initializer);
+ if (call->function_name == SNAME("get_node")) {
+ switch (call->get_callee_type()) {
+ case GDScriptParser::Node::IDENTIFIER: {
+ is_get_node = true;
+ } break;
+ case GDScriptParser::Node::SUBSCRIPT: {
+ GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(call->callee);
+ is_get_node = subscript->is_attribute && subscript->base->type == GDScriptParser::Node::SELF;
+ } break;
+ default:
+ break;
+ }
+ }
+ }
+ if (is_get_node) {
+ parser->push_warning(p_variable, GDScriptWarning::GET_NODE_DEFAULT_WITHOUT_ONREADY, is_using_shorthand ? "$" : "get_node()");
+ }
+ }
}
#endif
}
@@ -2931,7 +3023,11 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
// Enums do not have functions other than the built-in dictionary ones.
if (base_type.kind == GDScriptParser::DataType::ENUM && base_type.is_meta_type) {
- push_error(vformat(R"*(Enums only have Dictionary built-in methods. Function "%s()" does not exist for enum "%s".)*", p_call->function_name, base_type.enum_type), p_call->callee);
+ if (base_type.builtin_type == Variant::DICTIONARY) {
+ push_error(vformat(R"*(Enums only have Dictionary built-in methods. Function "%s()" does not exist for enum "%s".)*", p_call->function_name, base_type.enum_type), p_call->callee);
+ } else {
+ push_error(vformat(R"*(The native enum "%s" does not behave like Dictionary and does not have methods of its own.)*", base_type.enum_type), p_call->callee);
+ }
} else if (!p_call->is_super && callee_type != GDScriptParser::Node::NONE) { // Check if the name exists as something else.
GDScriptParser::IdentifierNode *callee_id;
if (callee_type == GDScriptParser::Node::IDENTIFIER) {
@@ -4302,15 +4398,26 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo
}
elem_type.is_constant = false;
result.set_container_element_type(elem_type);
+ } else if (p_property.type == Variant::INT) {
+ // Check if it's enum.
+ if (p_property.class_name != StringName()) {
+ Vector<String> names = String(p_property.class_name).split(".");
+ if (names.size() == 2) {
+ result = make_native_enum_type(names[1], names[0]);
+ }
+ }
}
}
return result;
}
-bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_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 GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_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, StringName *r_native_class) {
r_static = false;
r_vararg = false;
r_default_arg_count = 0;
+ if (r_native_class) {
+ *r_native_class = StringName();
+ }
StringName function_name = p_function;
bool was_enum = false;
@@ -4445,6 +4552,12 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo
if (valid && Engine::get_singleton()->has_singleton(base_native)) {
r_static = true;
}
+#ifdef DEBUG_ENABLED
+ MethodBind *native_method = ClassDB::get_method(base_native, function_name);
+ if (native_method && r_native_class) {
+ *r_native_class = native_method->get_instance_class();
+ }
+#endif
return valid;
}
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index b51564fb0a..ab2d2b3c6c 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -117,7 +117,7 @@ class GDScriptAnalyzer {
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 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 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, StringName *r_native_class = nullptr);
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);
void validate_call_arg(const List<GDScriptParser::DataType> &p_par_types, int p_default_args_count, bool p_is_vararg, const GDScriptParser::CallNode *p_call);
void validate_call_arg(const MethodInfo &p_method, const GDScriptParser::CallNode *p_call);
diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp
index 713ad3ed17..816be5a449 100644
--- a/modules/gdscript/gdscript_parser.cpp
+++ b/modules/gdscript/gdscript_parser.cpp
@@ -158,14 +158,10 @@ void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_
return;
}
- if (ignored_warning_codes.has(p_code)) {
+ if (ignored_warnings.has(p_code)) {
return;
}
- String warn_name = GDScriptWarning::get_name_from_code((GDScriptWarning::Code)p_code).to_lower();
- if (ignored_warnings.has(warn_name)) {
- return;
- }
int warn_level = (int)GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(p_code));
if (!warn_level) {
return;
@@ -180,7 +176,7 @@ void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_
warning.rightmost_column = p_source->rightmost_column;
if (warn_level == GDScriptWarning::WarnLevel::ERROR || bool(GLOBAL_GET("debug/gdscript/warnings/treat_warnings_as_errors"))) {
- push_error(warning.get_message(), p_source);
+ push_error(warning.get_message() + String(" (Warning treated as error.)"), p_source);
return;
}
@@ -3548,7 +3544,11 @@ const GDScriptParser::SuiteNode::Local &GDScriptParser::SuiteNode::get_local(con
return empty;
}
-bool GDScriptParser::AnnotationNode::apply(GDScriptParser *p_this, Node *p_target) const {
+bool GDScriptParser::AnnotationNode::apply(GDScriptParser *p_this, Node *p_target) {
+ if (is_applied) {
+ return true;
+ }
+ is_applied = true;
return (p_this->*(p_this->valid_annotations[name].apply))(this, p_target);
}
diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h
index 07dac25ec5..7255d21a6e 100644
--- a/modules/gdscript/gdscript_parser.h
+++ b/modules/gdscript/gdscript_parser.h
@@ -297,7 +297,9 @@ public:
int leftmost_column = 0, rightmost_column = 0;
Node *next = nullptr;
List<AnnotationNode *> annotations;
- Vector<uint32_t> ignored_warnings;
+#ifdef DEBUG_ENABLED
+ Vector<GDScriptWarning::Code> ignored_warnings;
+#endif
DataType datatype;
@@ -329,8 +331,10 @@ public:
AnnotationInfo *info = nullptr;
PropertyInfo export_info;
+ bool is_resolved = false;
+ bool is_applied = false;
- bool apply(GDScriptParser *p_this, Node *p_target) const;
+ bool apply(GDScriptParser *p_this, Node *p_target);
bool applies_to(uint32_t p_target_kinds) const;
AnnotationNode() {
@@ -1263,8 +1267,7 @@ private:
#ifdef DEBUG_ENABLED
bool is_ignoring_warnings = false;
List<GDScriptWarning> warnings;
- HashSet<String> ignored_warnings;
- HashSet<uint32_t> ignored_warning_codes;
+ HashSet<GDScriptWarning::Code> ignored_warnings;
HashSet<int> unsafe_lines;
#endif
diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp
index 9436146bed..ef59a07f1a 100644
--- a/modules/gdscript/gdscript_warning.cpp
+++ b/modules/gdscript/gdscript_warning.cpp
@@ -170,6 +170,21 @@ String GDScriptWarning::get_message() const {
case RENAMED_IN_GD4_HINT: {
break; // Renamed identifier hint is taken care of by the GDScriptAnalyzer. No message needed here.
}
+ 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]);
+ }
+ case NATIVE_METHOD_OVERRIDE: {
+ CHECK_SYMBOLS(2);
+ return vformat(R"(The method "%s" overrides a method from native class "%s". This won't be called by the engine and may not work as expected.)", symbols[0], symbols[1]);
+ }
+ case GET_NODE_DEFAULT_WITHOUT_ONREADY: {
+ CHECK_SYMBOLS(1);
+ return vformat(R"*(The default value is using "%s" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.)*", symbols[0]);
+ }
+ case ONREADY_WITH_EXPORT: {
+ return R"(The "@onready" annotation will make the default value to be set after the "@export" takes effect and will override it.)";
+ }
case WARNING_MAX:
break; // Can't happen, but silences warning
}
@@ -179,14 +194,8 @@ String GDScriptWarning::get_message() const {
}
int GDScriptWarning::get_default_value(Code p_code) {
- if (get_name_from_code(p_code).to_lower().begins_with("unsafe_")) {
- return WarnLevel::IGNORE;
- }
- // Too spammy by default on common cases (connect, Tween, etc.).
- if (p_code == RETURN_VALUE_DISCARDED) {
- return WarnLevel::IGNORE;
- }
- return WarnLevel::WARN;
+ ERR_FAIL_INDEX_V_MSG(p_code, WARNING_MAX, WarnLevel::IGNORE, "Getting default value of invalid warning code.");
+ return default_warning_levels[p_code];
}
PropertyInfo GDScriptWarning::get_property_info(Code p_code) {
@@ -240,7 +249,11 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
"INT_AS_ENUM_WITHOUT_MATCH",
"STATIC_CALLED_ON_INSTANCE",
"CONFUSABLE_IDENTIFIER",
- "RENAMED_IN_GODOT_4_HINT"
+ "RENAMED_IN_GODOT_4_HINT",
+ "INFERENCE_ON_VARIANT",
+ "NATIVE_METHOD_OVERRIDE",
+ "GET_NODE_DEFAULT_WITHOUT_ONREADY",
+ "ONREADY_WITH_EXPORT",
};
static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names.");
diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h
index fa2907cdae..f0123c518c 100644
--- a/modules/gdscript/gdscript_warning.h
+++ b/modules/gdscript/gdscript_warning.h
@@ -82,9 +82,58 @@ public:
STATIC_CALLED_ON_INSTANCE, // A static method was called on an instance of a class instead of on the class itself.
CONFUSABLE_IDENTIFIER, // The identifier contains misleading characters that can be confused. E.g. "usеr" (has Cyrillic "е" instead of Latin "e").
RENAMED_IN_GD4_HINT, // A variable or function that could not be found has been renamed in Godot 4
+ 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.
+ ONREADY_WITH_EXPORT, // The `@onready` annotation will set the value after `@export` which is likely not intended.
WARNING_MAX,
};
+ constexpr static WarnLevel default_warning_levels[] = {
+ WARN, // UNASSIGNED_VARIABLE
+ WARN, // UNASSIGNED_VARIABLE_OP_ASSIGN
+ WARN, // UNUSED_VARIABLE
+ WARN, // UNUSED_LOCAL_CONSTANT
+ WARN, // SHADOWED_VARIABLE
+ WARN, // SHADOWED_VARIABLE_BASE_CLASS
+ WARN, // UNUSED_PRIVATE_CLASS_VARIABLE
+ WARN, // UNUSED_PARAMETER
+ WARN, // UNREACHABLE_CODE
+ WARN, // UNREACHABLE_PATTERN
+ WARN, // STANDALONE_EXPRESSION
+ WARN, // NARROWING_CONVERSION
+ WARN, // INCOMPATIBLE_TERNARY
+ WARN, // UNUSED_SIGNAL
+ IGNORE, // RETURN_VALUE_DISCARDED // Too spammy by default on common cases (connect, Tween, etc.).
+ WARN, // PROPERTY_USED_AS_FUNCTION
+ WARN, // CONSTANT_USED_AS_FUNCTION
+ WARN, // FUNCTION_USED_AS_PROPERTY
+ WARN, // INTEGER_DIVISION
+ IGNORE, // UNSAFE_PROPERTY_ACCESS // Too common in untyped scenarios.
+ IGNORE, // UNSAFE_METHOD_ACCESS // Too common in untyped scenarios.
+ IGNORE, // UNSAFE_CAST // Too common in untyped scenarios.
+ IGNORE, // UNSAFE_CALL_ARGUMENT // Too common in untyped scenarios.
+ WARN, // UNSAFE_VOID_RETURN
+ WARN, // DEPRECATED_KEYWORD
+ WARN, // STANDALONE_TERNARY
+ WARN, // ASSERT_ALWAYS_TRUE
+ WARN, // ASSERT_ALWAYS_FALSE
+ WARN, // REDUNDANT_AWAIT
+ WARN, // EMPTY_FILE
+ WARN, // SHADOWED_GLOBAL_IDENTIFIER
+ WARN, // INT_AS_ENUM_WITHOUT_CAST
+ WARN, // INT_AS_ENUM_WITHOUT_MATCH
+ WARN, // STATIC_CALLED_ON_INSTANCE
+ WARN, // CONFUSABLE_IDENTIFIER
+ WARN, // RENAMED_IN_GD4_HINT
+ 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.
+ ERROR, // ONREADY_WITH_EXPORT // May not work as expected.
+ };
+
+ static_assert((sizeof(default_warning_levels) / sizeof(default_warning_levels[0])) == WARNING_MAX, "Amount of default levels does not match the amount of warnings.");
+
Code code = WARNING_MAX;
int start_line = -1, end_line = -1;
int leftmost_column = -1, rightmost_column = -1;
diff --git a/modules/gdscript/tests/gdscript_test_runner.cpp b/modules/gdscript/tests/gdscript_test_runner.cpp
index 5b8af0ff34..57405aa1ce 100644
--- a/modules/gdscript/tests/gdscript_test_runner.cpp
+++ b/modules/gdscript/tests/gdscript_test_runner.cpp
@@ -146,11 +146,11 @@ GDScriptTestRunner::GDScriptTestRunner(const String &p_source_dir, bool p_init_l
init_language(p_source_dir);
}
#ifdef DEBUG_ENABLED
- // Enable all warnings for GDScript, so we can test them.
+ // Set all warning levels to "Warn" in order to test them properly, even the ones that default to error.
ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/enable", true);
for (int i = 0; i < (int)GDScriptWarning::WARNING_MAX; i++) {
- String warning = GDScriptWarning::get_name_from_code((GDScriptWarning::Code)i).to_lower();
- ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/" + warning, true);
+ String warning_setting = GDScriptWarning::get_settings_path_from_code((GDScriptWarning::Code)i);
+ ProjectSettings::get_singleton()->set_setting(warning_setting, (int)GDScriptWarning::WARN);
}
#endif
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out
index c70a1df10d..508e46742f 100644
--- a/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out
+++ b/modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
-The function signature doesn't match the parent. Parent signature is "my_function(int = default) -> int".
+The function signature doesn't match the parent. Parent signature is "my_function(int = <default>) -> int".
diff --git a/modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd b/modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd
index 48a804ff54..b447180ea8 100644
--- a/modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd
+++ b/modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd
@@ -2,16 +2,18 @@ func variant() -> Variant: return null
var member_weak = variant()
var member_typed: Variant = variant()
+@warning_ignore("inference_on_variant")
var member_inferred := variant()
func param_weak(param = variant()) -> void: print(param)
func param_typed(param: Variant = variant()) -> void: print(param)
+@warning_ignore("inference_on_variant")
func param_inferred(param := variant()) -> void: print(param)
func return_untyped(): return variant()
func return_typed() -> Variant: return variant()
-@warning_ignore("unused_variable")
+@warning_ignore("unused_variable", "inference_on_variant")
func test() -> void:
var weak = variant()
var typed: Variant = variant()
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.gd b/modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.gd
new file mode 100644
index 0000000000..bf5ea241ec
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.gd
@@ -0,0 +1,15 @@
+extends Node
+
+var add_node = do_add_node() # Hack to have one node on init and not fail at runtime.
+
+var shorthand = $Node
+var with_self = self.get_node("Node")
+var without_self = get_node("Node")
+
+func test():
+ print("warn")
+
+func do_add_node():
+ var node = Node.new()
+ node.name = "Node"
+ add_child(node)
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.out b/modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.out
new file mode 100644
index 0000000000..11cd9c678b
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.out
@@ -0,0 +1,14 @@
+GDTEST_OK
+>> WARNING
+>> Line: 5
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "$" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+>> WARNING
+>> Line: 6
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "get_node()" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+>> WARNING
+>> Line: 7
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "get_node()" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+warn
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.gd b/modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.gd
new file mode 100644
index 0000000000..024e91b7c6
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.gd
@@ -0,0 +1,6 @@
+func test():
+ var inferred_with_variant := return_variant()
+ print(inferred_with_variant)
+
+func return_variant() -> Variant:
+ return "warn"
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.out b/modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.out
new file mode 100644
index 0000000000..1d4078d2f7
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.out
@@ -0,0 +1,6 @@
+GDTEST_OK
+>> WARNING
+>> Line: 2
+>> INFERENCE_ON_VARIANT
+>> The variable type is being inferred from a Variant value, so it will be typed as Variant.
+warn
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.gd b/modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.gd
new file mode 100644
index 0000000000..0b358ca5f2
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.gd
@@ -0,0 +1,6 @@
+extends Node
+
+@onready @export var conflict = ""
+
+func test():
+ print("warn")
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.out b/modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.out
new file mode 100644
index 0000000000..ff184f9f04
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.out
@@ -0,0 +1,6 @@
+GDTEST_OK
+>> WARNING
+>> Line: 3
+>> ONREADY_WITH_EXPORT
+>> The "@onready" annotation will make the default value to be set after the "@export" takes effect and will override it.
+warn
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.gd b/modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.gd
new file mode 100644
index 0000000000..19d40f8ec8
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.gd
@@ -0,0 +1,5 @@
+func test():
+ print("warn")
+
+func get(_property: StringName) -> Variant:
+ return null
diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.out b/modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.out
new file mode 100644
index 0000000000..793faa05d4
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.out
@@ -0,0 +1,6 @@
+GDTEST_OK
+>> WARNING
+>> Line: 4
+>> NATIVE_METHOD_OVERRIDE
+>> The method "get" overrides a method from native class "Object". This won't be called by the engine and may not work as expected.
+warn