Skip to content

Commit

Permalink
Revert "GDScript: Add warnings that are set to error by default"
Browse files Browse the repository at this point in the history 
This reverts commit a166833.

This caused multiple regressions.
Needs to be redone with more testing before merge.

Fixes #72501.
  • Loading branch information
@akien-mga
akien-mga committed Feb 1, 2023
1 parent f7397a5 commit afe3b94
Show file tree
Hide file tree
Showing 18 changed files with 57 additions and 313 deletions.
14 changes: 1 addition & 13 deletions doc/classes/ProjectSettings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,15 +405,9 @@
<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>
Expand All @@ -426,12 +420,6 @@
<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>
Expand Down Expand Up @@ -489,7 +477,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="1">
<member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="0">
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">
Expand Down
173 changes: 30 additions & 143 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,13 @@ 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) {
// 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
}
GDScriptParser::DataType type = make_enum_type(p_enum_name, p_native_class, p_meta);

List<StringName> enum_values;
ClassDB::get_enum_constants(native_base, p_enum_name, &enum_values, true);
ClassDB::get_enum_constants(p_native_class, p_enum_name, &enum_values);

for (const StringName &E : enum_values) {
type.enum_values[E] = ClassDB::get_integer_constant(native_base, E);
type.enum_values[E] = ClassDB::get_integer_constant(p_native_class, E);
}

return type;
Expand Down Expand Up @@ -794,22 +782,6 @@ 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);
Expand All @@ -818,16 +790,9 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,

// Apply annotations.
for (GDScriptParser::AnnotationNode *&E : member.variable->annotations) {
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);
resolve_annotation(E);
E->apply(parser, member.variable);
}
#endif
} break;
case GDScriptParser::ClassNode::Member::CONSTANT: {
check_class_member_name_conflict(p_class, member.constant->identifier->name, member.constant);
Expand Down Expand Up @@ -913,10 +878,6 @@ 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: {
Expand Down Expand Up @@ -970,9 +931,6 @@ 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;
Expand Down Expand Up @@ -1101,7 +1059,19 @@ 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) {
Expand Down Expand Up @@ -1132,9 +1102,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<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
for (GDScriptWarning::Code ignored_warning : member.variable->ignored_warnings) {
parser->ignored_warnings.insert(ignored_warning);
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);
}
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);
Expand Down Expand Up @@ -1209,7 +1179,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
}
}
#ifdef DEBUG_ENABLED
parser->ignored_warnings = previously_ignored_warnings;
parser->ignored_warning_codes = previously_ignored;
#endif // DEBUG_ENABLED
}
}
Expand Down Expand Up @@ -1319,11 +1289,6 @@ 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();
Expand Down Expand Up @@ -1390,13 +1355,6 @@ 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;

Expand Down Expand Up @@ -1463,8 +1421,7 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
int default_par_count = 0;
bool is_static = false;
bool is_vararg = false;
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)) {
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)) {
bool valid = p_function->is_static == is_static;
valid = valid && parent_return_type == p_function->get_datatype();

Expand All @@ -1490,8 +1447,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++;
Expand All @@ -1507,11 +1464,6 @@ 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
}
Expand All @@ -1520,9 +1472,6 @@ 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;
}

Expand All @@ -1532,13 +1481,6 @@ 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;

Expand All @@ -1556,9 +1498,6 @@ void GDScriptAnalyzer::resolve_function_body(GDScriptParser::FunctionNode *p_fun
}
}

#ifdef DEBUG_ENABLED
parser->ignored_warnings = previously_ignored_warnings;
#endif
parser->current_function = previous_function;
}

Expand Down Expand Up @@ -1599,16 +1538,16 @@ void GDScriptAnalyzer::resolve_suite(GDScriptParser::SuiteNode *p_suite) {
}

#ifdef DEBUG_ENABLED
HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
for (GDScriptWarning::Code ignored_warning : stmt->ignored_warnings) {
parser->ignored_warnings.insert(ignored_warning);
HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
for (uint32_t ignored_warning : stmt->ignored_warnings) {
parser->ignored_warning_codes.insert(ignored_warning);
}
#endif // DEBUG_ENABLED

resolve_node(stmt);

#ifdef DEBUG_ENABLED
parser->ignored_warnings = previously_ignored_warnings;
parser->ignored_warning_codes = previously_ignored;
#endif // DEBUG_ENABLED

decide_suite_type(p_suite, stmt);
Expand Down Expand Up @@ -1660,11 +1599,6 @@ 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);
Expand Down Expand Up @@ -1724,32 +1658,6 @@ 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
}
Expand Down Expand Up @@ -3023,11 +2931,7 @@ 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) {
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);
}
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 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) {
Expand Down Expand Up @@ -4398,26 +4302,15 @@ 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, StringName *r_native_class) {
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) {
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;
Expand Down Expand Up @@ -4552,12 +4445,6 @@ 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;
}

Expand Down
Loading

0 comments on commit afe3b94

Please sign in to comment.