You've already forked godot
mirror of
https://github.com/godotengine/godot.git
synced 2025-11-05 12:10:55 +00:00
GDScript: Report property type errors
Inline getters & setters are now FunctionNodes. Their names are set in the parser, not in the compiler. GDScript-Analyzer will now run through getter and setter. Also report wrong type or signature errors regarding getset properties. Added GDScript tests for getters and setters. #53102
This commit is contained in:
@@ -875,18 +875,32 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class) {
|
||||
GDScriptParser::ClassNode *previous_class = parser->current_class;
|
||||
parser->current_class = p_class;
|
||||
|
||||
// Do functions now.
|
||||
// Do functions and properties now.
|
||||
for (int i = 0; i < p_class->members.size(); i++) {
|
||||
GDScriptParser::ClassNode::Member member = p_class->members[i];
|
||||
if (member.type != GDScriptParser::ClassNode::Member::FUNCTION) {
|
||||
continue;
|
||||
}
|
||||
if (member.type == GDScriptParser::ClassNode::Member::FUNCTION) {
|
||||
resolve_function_body(member.function);
|
||||
|
||||
resolve_function_body(member.function);
|
||||
// Apply annotations.
|
||||
for (GDScriptParser::AnnotationNode *&E : member.function->annotations) {
|
||||
E->apply(parser, member.function);
|
||||
}
|
||||
} 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) {
|
||||
member.variable->getter->set_datatype(member.variable->datatype);
|
||||
|
||||
// Apply annotations.
|
||||
for (GDScriptParser::AnnotationNode *&E : member.function->annotations) {
|
||||
E->apply(parser, member.function);
|
||||
resolve_function_body(member.variable->getter);
|
||||
}
|
||||
if (member.variable->setter != nullptr) {
|
||||
if (member.variable->setter->parameters.size() > 0) {
|
||||
member.variable->setter->parameters[0]->datatype_specifier = member.variable->datatype_specifier;
|
||||
}
|
||||
|
||||
resolve_function_signature(member.variable->setter);
|
||||
resolve_function_body(member.variable->setter);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -902,17 +916,80 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class) {
|
||||
resolve_class_body(member.m_class);
|
||||
}
|
||||
|
||||
// Check unused variables.
|
||||
// Check unused variables and datatypes of property getters and setters.
|
||||
for (int i = 0; i < p_class->members.size(); i++) {
|
||||
GDScriptParser::ClassNode::Member member = p_class->members[i];
|
||||
if (member.type != GDScriptParser::ClassNode::Member::VARIABLE) {
|
||||
continue;
|
||||
}
|
||||
if (member.type == GDScriptParser::ClassNode::Member::VARIABLE) {
|
||||
#ifdef DEBUG_ENABLED
|
||||
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);
|
||||
}
|
||||
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);
|
||||
}
|
||||
#endif
|
||||
|
||||
if (member.variable->property == GDScriptParser::VariableNode::PROP_SETGET) {
|
||||
GDScriptParser::FunctionNode *getter_function = nullptr;
|
||||
GDScriptParser::FunctionNode *setter_function = nullptr;
|
||||
|
||||
bool has_valid_getter = false;
|
||||
bool has_valid_setter = false;
|
||||
|
||||
if (member.variable->getter_pointer != nullptr) {
|
||||
if (p_class->has_function(member.variable->getter_pointer->name)) {
|
||||
getter_function = p_class->get_member(member.variable->getter_pointer->name).function;
|
||||
}
|
||||
|
||||
if (getter_function == nullptr) {
|
||||
push_error(vformat(R"(Getter "%s" not found.)", member.variable->getter_pointer->name), member.variable);
|
||||
|
||||
} else if (getter_function->parameters.size() != 0 || getter_function->datatype.has_no_type()) {
|
||||
push_error(vformat(R"(Function "%s" cannot be used as getter because of its signature.)", getter_function->identifier->name), member.variable);
|
||||
|
||||
} else if (!is_type_compatible(member.variable->datatype, getter_function->datatype, true)) {
|
||||
push_error(vformat(R"(Function with return type "%s" cannot be used as getter for a property of type "%s".)", getter_function->datatype.to_string(), member.variable->datatype.to_string()), member.variable);
|
||||
|
||||
} else {
|
||||
has_valid_getter = true;
|
||||
|
||||
#ifdef DEBUG_ENABLED
|
||||
if (member.variable->datatype.builtin_type == Variant::INT && getter_function->datatype.builtin_type == Variant::FLOAT) {
|
||||
parser->push_warning(member.variable, GDScriptWarning::NARROWING_CONVERSION);
|
||||
}
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
if (member.variable->setter_pointer != nullptr) {
|
||||
if (p_class->has_function(member.variable->setter_pointer->name)) {
|
||||
setter_function = p_class->get_member(member.variable->setter_pointer->name).function;
|
||||
}
|
||||
|
||||
if (setter_function == nullptr) {
|
||||
push_error(vformat(R"(Setter "%s" not found.)", member.variable->setter_pointer->name), member.variable);
|
||||
|
||||
} else if (setter_function->parameters.size() != 1) {
|
||||
push_error(vformat(R"(Function "%s" cannot be used as setter because of its signature.)", setter_function->identifier->name), member.variable);
|
||||
|
||||
} else if (!is_type_compatible(member.variable->datatype, setter_function->parameters[0]->datatype, true)) {
|
||||
push_error(vformat(R"(Function with argument type "%s" cannot be used as setter for a property of type "%s".)", setter_function->parameters[0]->datatype.to_string(), member.variable->datatype.to_string()), member.variable);
|
||||
|
||||
} else {
|
||||
has_valid_setter = true;
|
||||
|
||||
#ifdef DEBUG_ENABLED
|
||||
if (member.variable->datatype.builtin_type == Variant::INT && setter_function->return_type->datatype.builtin_type == Variant::FLOAT) {
|
||||
parser->push_warning(member.variable, GDScriptWarning::NARROWING_CONVERSION);
|
||||
}
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
if (member.variable->datatype.is_variant() && has_valid_getter && has_valid_setter) {
|
||||
if (!is_type_compatible(getter_function->datatype, setter_function->parameters[0]->datatype, true)) {
|
||||
push_error(vformat(R"(Getter with type "%s" cannot be used along with setter of type "%s".)", getter_function->datatype.to_string(), setter_function->parameters[0]->datatype.to_string()), member.variable);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1529,12 +1606,20 @@ void GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode *p_parame
|
||||
void GDScriptAnalyzer::resolve_return(GDScriptParser::ReturnNode *p_return) {
|
||||
GDScriptParser::DataType result;
|
||||
|
||||
GDScriptParser::DataType expected_type;
|
||||
bool has_expected_type = false;
|
||||
|
||||
if (parser->current_function != nullptr) {
|
||||
expected_type = parser->current_function->get_datatype();
|
||||
has_expected_type = true;
|
||||
}
|
||||
|
||||
if (p_return->return_value != nullptr) {
|
||||
reduce_expression(p_return->return_value);
|
||||
if (p_return->return_value->type == GDScriptParser::Node::ARRAY) {
|
||||
// Check if assigned value is an array literal, so we can make it a typed array too if appropriate.
|
||||
if (parser->current_function->get_datatype().has_container_element_type() && p_return->return_value->type == GDScriptParser::Node::ARRAY) {
|
||||
update_array_literal_element_type(parser->current_function->get_datatype(), static_cast<GDScriptParser::ArrayNode *>(p_return->return_value));
|
||||
if (has_expected_type && expected_type.has_container_element_type() && p_return->return_value->type == GDScriptParser::Node::ARRAY) {
|
||||
update_array_literal_element_type(expected_type, static_cast<GDScriptParser::ArrayNode *>(p_return->return_value));
|
||||
}
|
||||
}
|
||||
result = p_return->return_value->get_datatype();
|
||||
@@ -1546,23 +1631,24 @@ void GDScriptAnalyzer::resolve_return(GDScriptParser::ReturnNode *p_return) {
|
||||
result.is_constant = true;
|
||||
}
|
||||
|
||||
GDScriptParser::DataType function_type = parser->current_function->get_datatype();
|
||||
function_type.is_meta_type = false;
|
||||
if (function_type.is_hard_type()) {
|
||||
if (!is_type_compatible(function_type, result)) {
|
||||
// Try other way. Okay but not safe.
|
||||
if (!is_type_compatible(result, function_type)) {
|
||||
push_error(vformat(R"(Cannot return value of type "%s" because the function return type is "%s".)", result.to_string(), function_type.to_string()), p_return);
|
||||
} else {
|
||||
// TODO: Add warning.
|
||||
mark_node_unsafe(p_return);
|
||||
}
|
||||
if (has_expected_type) {
|
||||
expected_type.is_meta_type = false;
|
||||
if (expected_type.is_hard_type()) {
|
||||
if (!is_type_compatible(expected_type, result)) {
|
||||
// Try other way. Okay but not safe.
|
||||
if (!is_type_compatible(result, expected_type)) {
|
||||
push_error(vformat(R"(Cannot return value of type "%s" because the function return type is "%s".)", result.to_string(), expected_type.to_string()), p_return);
|
||||
} else {
|
||||
// TODO: Add warning.
|
||||
mark_node_unsafe(p_return);
|
||||
}
|
||||
#ifdef DEBUG_ENABLED
|
||||
} else if (function_type.builtin_type == Variant::INT && result.builtin_type == Variant::FLOAT) {
|
||||
parser->push_warning(p_return, GDScriptWarning::NARROWING_CONVERSION);
|
||||
} else if (result.is_variant()) {
|
||||
mark_node_unsafe(p_return);
|
||||
} else if (expected_type.builtin_type == Variant::INT && result.builtin_type == Variant::FLOAT) {
|
||||
parser->push_warning(p_return, GDScriptWarning::NARROWING_CONVERSION);
|
||||
} else if (result.is_variant()) {
|
||||
mark_node_unsafe(p_return);
|
||||
#endif
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user