Skip to content

Commit

Permalink
Merge pull request #77324 from anvilfolk/oh-no-its-virtual
Browse files Browse the repository at this point in the history 
GDScript: add errors when calling unimplemented virtual functions
  • Loading branch information
@akien-mga
akien-mga committed Jun 15, 2023
2 parents 3b4504a + 861743c commit 49b774e
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 32 deletions.
66 changes: 36 additions & 30 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1611,11 +1611,10 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
GDScriptParser::DataType parent_return_type;
List<GDScriptParser::DataType> parameters_types;
int default_par_count = 0;
bool is_static = false;
bool is_vararg = false;
BitField<MethodFlags> method_flags;
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;
if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, method_flags, &native_base)) {
bool valid = p_function->is_static == method_flags.has_flag(METHOD_FLAG_STATIC);
valid = valid && parent_return_type == p_function->get_datatype();

int par_count_diff = p_function->parameters.size() - parameters_types.size();
Expand Down Expand Up @@ -2030,9 +2029,8 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) {
GDScriptParser::DataType return_type;
List<GDScriptParser::DataType> par_types;
int default_arg_count = 0;
bool is_static = false;
bool is_vararg = false;
if (get_function_signature(p_for->list, false, list_type, CoreStringNames::get_singleton()->_iter_get, return_type, par_types, default_arg_count, is_static, is_vararg)) {
BitField<MethodFlags> method_flags;
if (get_function_signature(p_for->list, false, list_type, CoreStringNames::get_singleton()->_iter_get, return_type, par_types, default_arg_count, method_flags)) {
variable_type = return_type;
variable_type.type_source = list_type.type_source;
} else if (!list_type.is_hard_type()) {
Expand Down Expand Up @@ -3085,30 +3083,39 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
return;
}

bool is_static = false;
bool is_vararg = false;
int default_arg_count = 0;
BitField<MethodFlags> method_flags;
GDScriptParser::DataType return_type;
List<GDScriptParser::DataType> par_types;

bool is_constructor = (base_type.is_meta_type || (p_call->callee && p_call->callee->type == GDScriptParser::Node::IDENTIFIER)) && p_call->function_name == SNAME("new");

if (get_function_signature(p_call, is_constructor, base_type, p_call->function_name, return_type, par_types, default_arg_count, is_static, is_vararg)) {
if (get_function_signature(p_call, is_constructor, base_type, p_call->function_name, return_type, par_types, default_arg_count, method_flags)) {
// If the method is implemented in the class hierarchy, the virtual flag will not be set for that MethodInfo and the search stops there.
// MethodInfo's above the class that defines the method might still have the virtual flag set.
if (method_flags.has_flag(METHOD_FLAG_VIRTUAL)) {
if (p_call->is_super) {
push_error(vformat(R"*(Cannot call the parent class' virtual function "%s()" because it hasn't been defined.)*", p_call->function_name), p_call);
} else {
push_error(vformat(R"*(Cannot call virtual function "%s()" because it hasn't been defined.)*", p_call->function_name), p_call);
}
}

// If the function require typed arrays we must make literals be typed.
for (const KeyValue<int, GDScriptParser::ArrayNode *> &E : arrays) {
int index = E.key;
if (index < par_types.size() && par_types[index].has_container_element_type()) {
update_array_literal_element_type(E.value, par_types[index].get_container_element_type());
}
}
validate_call_arg(par_types, default_arg_count, is_vararg, p_call);
validate_call_arg(par_types, default_arg_count, method_flags.has_flag(METHOD_FLAG_VARARG), p_call);

if (base_type.kind == GDScriptParser::DataType::ENUM && base_type.is_meta_type) {
// Enum type is treated as a dictionary value for function calls.
base_type.is_meta_type = false;
}

if (is_self && static_context && !is_static) {
if (is_self && static_context && !method_flags.has_flag(METHOD_FLAG_STATIC)) {
if (parser->current_function) {
// Get the parent function above any lambda.
GDScriptParser::FunctionNode *parent_function = parser->current_function;
Expand All @@ -3119,10 +3126,10 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
} else {
push_error(vformat(R"*(Cannot call non-static function "%s()" for static variable initializer.)*", p_call->function_name), p_call);
}
} else if (!is_self && base_type.is_meta_type && !is_static) {
} else if (!is_self && base_type.is_meta_type && !method_flags.has_flag(METHOD_FLAG_STATIC)) {
base_type.is_meta_type = false; // For `to_string()`.
push_error(vformat(R"*(Cannot call non-static function "%s()" on the class "%s" directly. Make an instance instead.)*", p_call->function_name, base_type.to_string()), p_call);
} else if (is_self && !is_static) {
} else if (is_self && !method_flags.has_flag(METHOD_FLAG_STATIC)) {
mark_lambda_use_self();
}

Expand All @@ -3136,7 +3143,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
parser->push_warning(p_call, GDScriptWarning::RETURN_VALUE_DISCARDED, p_call->function_name);
}

if (is_static && !is_constructor && !base_type.is_meta_type && !(is_self && static_context)) {
if (method_flags.has_flag(METHOD_FLAG_STATIC) && !is_constructor && !base_type.is_meta_type && !(is_self && static_context)) {
String caller_type = String(base_type.native_type);

if (caller_type.is_empty()) {
Expand Down Expand Up @@ -4652,9 +4659,8 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo
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) {
r_static = false;
r_vararg = false;
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, BitField<MethodFlags> &r_method_flags, StringName *r_native_class) {
r_method_flags = METHOD_FLAGS_DEFAULT;
r_default_arg_count = 0;
if (r_native_class) {
*r_native_class = StringName();
Expand Down Expand Up @@ -4687,10 +4693,9 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo

for (const MethodInfo &E : methods) {
if (E.name == p_function) {
function_signature_from_info(E, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg);
r_static = Variant::is_builtin_method_static(p_base_type.builtin_type, function_name);
function_signature_from_info(E, r_return_type, r_par_types, r_default_arg_count, r_method_flags);
// Cannot use non-const methods on enums.
if (!r_static && was_enum && !(E.flags & METHOD_FLAG_CONST)) {
if (!r_method_flags.has_flag(METHOD_FLAG_STATIC) && was_enum && !(E.flags & METHOD_FLAG_CONST)) {
push_error(vformat(R"*(Cannot call non-const Dictionary function "%s()" on enum "%s".)*", p_function, p_base_type.enum_type), p_source);
}
return true;
Expand Down Expand Up @@ -4721,7 +4726,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo

if (p_is_constructor) {
function_name = GDScriptLanguage::get_singleton()->strings._init;
r_static = true;
r_method_flags.set_flag(METHOD_FLAG_STATIC);
}

GDScriptParser::ClassNode *base_class = p_base_type.class_type;
Expand All @@ -4744,7 +4749,9 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo
}

if (found_function != nullptr) {
r_static = p_is_constructor || found_function->is_static;
if (p_is_constructor || found_function->is_static) {
r_method_flags.set_flag(METHOD_FLAG_STATIC);
}
for (int i = 0; i < found_function->parameters.size(); i++) {
r_par_types.push_back(found_function->parameters[i]->get_datatype());
if (found_function->parameters[i]->initializer != nullptr) {
Expand All @@ -4764,7 +4771,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo
MethodInfo info = base_script->get_method_info(function_name);

if (!(info == MethodInfo())) {
return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg);
return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_method_flags);
}
base_script = base_script->get_base_script();
}
Expand All @@ -4775,7 +4782,7 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo
StringName script_class = p_base_type.kind == GDScriptParser::DataType::SCRIPT ? p_base_type.script_type->get_class_name() : StringName(GDScript::get_class_static());

if (ClassDB::get_method_info(script_class, function_name, &info)) {
return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg);
return function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_method_flags);
}
}

Expand All @@ -4789,9 +4796,9 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo

MethodInfo info;
if (ClassDB::get_method_info(base_native, function_name, &info)) {
bool valid = function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_static, r_vararg);
bool valid = function_signature_from_info(info, r_return_type, r_par_types, r_default_arg_count, r_method_flags);
if (valid && Engine::get_singleton()->has_singleton(base_native)) {
r_static = true;
r_method_flags.set_flag(METHOD_FLAG_STATIC);
}
#ifdef DEBUG_ENABLED
MethodBind *native_method = ClassDB::get_method(base_native, function_name);
Expand All @@ -4805,11 +4812,10 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo
return false;
}

bool GDScriptAnalyzer::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) {
bool GDScriptAnalyzer::function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, BitField<MethodFlags> &r_method_flags) {
r_return_type = type_from_property(p_info.return_val);
r_default_arg_count = p_info.default_arguments.size();
r_vararg = (p_info.flags & METHOD_FLAG_VARARG) != 0;
r_static = (p_info.flags & METHOD_FLAG_STATIC) != 0;
r_method_flags = p_info.flags;

for (const PropertyInfo &E : p_info.arguments) {
r_par_types.push_back(type_from_property(E, true));
Expand Down
4 changes: 2 additions & 2 deletions modules/gdscript/gdscript_analyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ 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, 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);
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, BitField<MethodFlags> &r_method_flags, 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, BitField<MethodFlags> &r_method_flags);
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);
GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid, const GDScriptParser::Node *p_source);
Expand Down
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/virtual_method_not_implemented.gd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
func test():
_get_property_list()
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/virtual_method_not_implemented.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call virtual function "_get_property_list()" because it hasn't been defined.
5 changes: 5 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/virtual_super_not_implemented.gd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
func _init():
super()

func test():
pass
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/virtual_super_not_implemented.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call the parent class' virtual function "_init()" because it hasn't been defined.
11 changes: 11 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/virtual_method_implemented.gd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class TestOne:
func _get_property_list():
return {}

class TestTwo extends TestOne:
func _init():
var _x = _get_property_list()

func test():
var x = TestTwo.new()
var _x = x._get_property_list()
1 change: 1 addition & 0 deletions modules/gdscript/tests/scripts/analyzer/features/virtual_method_implemented.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GDTEST_OK

0 comments on commit 49b774e

Please sign in to comment.