godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #72608 from vnen/gdscript-warning-default-error 

GDScript: Add warnings that are set to error by default (take 2)
This commit is contained in:
Yuri Sizov 2023-02-05 16:00:26 +03:00 committed by GitHub
parent e13e4b7b6d 273bf7210f
commit 13f0158e49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 371 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
doc/classes/ProjectSettings.xml
View File

@ -417,9 +417,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>
@ -432,6 +438,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>
@ -489,7 +501,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">

180
modules/gdscript/gdscript_analyzer.cpp
View File

@ -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,48 @@ 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);
}
if (member.variable->initializer) {
// Check if it is call to get_node() on self (using shorthand $ or not), so we can check if @onready is needed.
// This could be improved by traversing the expression fully and checking the presence of get_node at any level.
if (!member.variable->onready && member.variable->initializer && (member.variable->initializer->type == GDScriptParser::Node::GET_NODE || member.variable->initializer->type == GDScriptParser::Node::CALL || member.variable->initializer->type == GDScriptParser::Node::CAST)) {
GDScriptParser::Node *expr = member.variable->initializer;
if (expr->type == GDScriptParser::Node::CAST) {
expr = static_cast<GDScriptParser::CastNode *>(expr)->operand;
}
bool is_get_node = expr->type == GDScriptParser::Node::GET_NODE;
bool is_using_shorthand = is_get_node;
if (!is_get_node && expr->type == GDScriptParser::Node::CALL) {
is_using_shorthand = false;
GDScriptParser::CallNode *call = static_cast<GDScriptParser::CallNode *>(expr);
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(member.variable, GDScriptWarning::GET_NODE_DEFAULT_WITHOUT_ONREADY, is_using_shorthand ? "$" : "get_node()");
}
}
}
#endif
} break;
case GDScriptParser::ClassNode::Member::CONSTANT: {
check_class_member_name_conflict(p_class, member.constant->identifier->name, member.constant);
@ -878,6 +945,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 +1002,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 +1133,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 +1164,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 +1241,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 +1351,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 +1422,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 +1495,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 +1522,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 +1539,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 +1552,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 +1564,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 +1588,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 +1631,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 +1692,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);
@ -2953,7 +3051,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) {
@ -4334,15 +4436,27 @@ 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], false);
result.is_constant = false;
}
}
}
}
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;
@ -4477,6 +4591,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;
}

2
modules/gdscript/gdscript_analyzer.h
View File

@ -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, 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 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);

14
modules/gdscript/gdscript_parser.cpp
View File

@ -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);
}

11
modules/gdscript/gdscript_parser.h
View File

@ -299,7 +299,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;
@ -331,8 +333,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() {
@ -1265,8 +1269,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

31
modules/gdscript/gdscript_warning.cpp
View File

@ -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.");

49
modules/gdscript/gdscript_warning.h
View File

@ -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;

6
modules/gdscript/tests/gdscript_test_runner.cpp
View File

@ -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

2
modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out
View File

@ -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".

18
modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.gd Normal file
View File

@ -0,0 +1,18 @@
extends Node
@onready var shorthand = $Node
@onready var call = get_node(^"Node")
@onready var shorthand_with_cast = $Node as Node
@onready var call_with_cast = get_node(^"Node") as Node
func _init():
var node := Node.new()
node.name = "Node"
add_child(node)
func test():
# Those are expected to be `null` since `_ready()` is never called on tests.
prints("shorthand", shorthand)
prints("call", call)
prints("shorthand_with_cast", shorthand_with_cast)
prints("call_with_cast", call_with_cast)

5
modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.out Normal file
View File

@ -0,0 +1,5 @@
GDTEST_OK
shorthand <null>
call <null>
shorthand_with_cast <null>
call_with_cast <null>

13
modules/gdscript/tests/scripts/analyzer/features/assign_to_native_enum_property.gd Normal file
View File

@ -0,0 +1,13 @@
# https://github.com/godotengine/godot/issues/72501
extends Node
func test():
prints("before", process_mode)
process_mode = PROCESS_MODE_PAUSABLE
prints("after", process_mode)
var node := Node.new()
add_child(node)
prints("before", node.process_mode)
node.process_mode = PROCESS_MODE_PAUSABLE
prints("after", node.process_mode)

5
modules/gdscript/tests/scripts/analyzer/features/assign_to_native_enum_property.out Normal file
View File

@ -0,0 +1,5 @@
GDTEST_OK
before 0
after 1
before 0
after 1

4
modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd
View File

@ -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()

17
modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.gd Normal file
View File

@ -0,0 +1,17 @@
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")
var with_cast = get_node(^"Node") as Node
var shorthand_with_cast = $Node as Node
func test():
print("warn")
func do_add_node():
var node = Node.new()
node.name = "Node"
add_child(node)

22
modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.out Normal file
View File

@ -0,0 +1,22 @@
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.
>> WARNING
>> Line: 8
>> 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: 9
>> 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.
warn

6
modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.gd Normal file
View File

@ -0,0 +1,6 @@
func test():
var inferred_with_variant := return_variant()
print(inferred_with_variant)
func return_variant() -> Variant:
return "warn"

6
modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.out Normal file
View File

@ -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

6
modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.gd Normal file
View File

@ -0,0 +1,6 @@
extends Node
@onready @export var conflict = ""
func test():
print("warn")

6
modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.out Normal file
View File

@ -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

5
modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.gd Normal file
View File

@ -0,0 +1,5 @@
func test():
print("warn")
func get(_property: StringName) -> Variant:
return null

6
modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.out Normal file
View File

@ -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