diff options
17 files changed, 291 insertions, 106 deletions
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 2dafc85f45..fd7664049a 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -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 + } } } diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 34b1e45cdf..1f9aad40af 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -2089,77 +2089,18 @@ GDScriptFunction *GDScriptCompiler::_parse_function(Error &r_error, GDScript *p_ Error GDScriptCompiler::_parse_setter_getter(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::VariableNode *p_variable, bool p_is_setter) { Error error = OK; - CodeGen codegen; - codegen.generator = memnew(GDScriptByteCodeGenerator); - - codegen.class_node = p_class; - codegen.script = p_script; - StringName func_name; + GDScriptParser::FunctionNode *function; if (p_is_setter) { - func_name = "@" + p_variable->identifier->name + "_setter"; + function = p_variable->setter; } else { - func_name = "@" + p_variable->identifier->name + "_getter"; + function = p_variable->getter; } - codegen.function_name = func_name; - - GDScriptDataType return_type; - if (p_is_setter) { - return_type.has_type = true; - return_type.kind = GDScriptDataType::BUILTIN; - return_type.builtin_type = Variant::NIL; - } else { - return_type = _gdtype_from_datatype(p_variable->get_datatype(), p_script); - } + _parse_function(error, p_script, p_class, function); - codegen.generator->write_start(p_script, func_name, false, Multiplayer::RPCConfig(), return_type); - - if (p_is_setter) { - uint32_t par_addr = codegen.generator->add_parameter(p_variable->setter_parameter->name, false, _gdtype_from_datatype(p_variable->get_datatype())); - codegen.parameters[p_variable->setter_parameter->name] = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::FUNCTION_PARAMETER, par_addr, _gdtype_from_datatype(p_variable->get_datatype())); - } - - error = _parse_block(codegen, p_is_setter ? p_variable->setter : p_variable->getter); - if (error) { - memdelete(codegen.generator); - return error; - } - - GDScriptFunction *gd_function = codegen.generator->write_end(); - - p_script->member_functions[func_name] = gd_function; - -#ifdef DEBUG_ENABLED - if (EngineDebugger::is_active()) { - String signature; - //path - if (p_script->get_path() != String()) { - signature += p_script->get_path(); - } - //loc - signature += "::" + itos(p_is_setter ? p_variable->setter->start_line : p_variable->getter->start_line); - - //function and class - - if (p_class->identifier) { - signature += "::" + String(p_class->identifier->name) + "." + String(func_name); - } else { - signature += "::" + String(func_name); - } - - codegen.generator->set_signature(signature); - } -#endif - codegen.generator->set_initial_line(p_is_setter ? p_variable->setter->start_line : p_variable->getter->start_line); - -#ifdef TOOLS_ENABLED - p_script->member_lines[func_name] = p_is_setter ? p_variable->setter->start_line : p_variable->getter->start_line; -#endif - memdelete(codegen.generator); - - return OK; + return error; } Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index d3b57fbe18..7a03774118 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -947,7 +947,7 @@ GDScriptParser::VariableNode *GDScriptParser::parse_property(VariableNode *p_var void GDScriptParser::parse_property_setter(VariableNode *p_variable) { switch (p_variable->property) { - case VariableNode::PROP_INLINE: + case VariableNode::PROP_INLINE: { consume(GDScriptTokenizer::Token::PARENTHESIS_OPEN, R"(Expected "(" after "set".)"); if (consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected parameter name after "(".)")) { p_variable->setter_parameter = parse_identifier(); @@ -955,9 +955,30 @@ void GDScriptParser::parse_property_setter(VariableNode *p_variable) { consume(GDScriptTokenizer::Token::PARENTHESIS_CLOSE, R"*(Expected ")" after parameter name.)*"); consume(GDScriptTokenizer::Token::COLON, R"*(Expected ":" after ")".)*"); - p_variable->setter = parse_suite("setter definition"); - break; + IdentifierNode *identifier = alloc_node<IdentifierNode>(); + identifier->name = "@" + p_variable->identifier->name + "_setter"; + + FunctionNode *function = alloc_node<FunctionNode>(); + function->identifier = identifier; + + FunctionNode *previous_function = current_function; + current_function = function; + + ParameterNode *parameter = alloc_node<ParameterNode>(); + parameter->identifier = p_variable->setter_parameter; + + function->parameters_indices[parameter->identifier->name] = 0; + function->parameters.push_back(parameter); + + SuiteNode *body = alloc_node<SuiteNode>(); + body->add_local(parameter, function); + + function->body = parse_suite("setter declaration", body); + p_variable->setter = function; + current_function = previous_function; + break; + } case VariableNode::PROP_SETGET: consume(GDScriptTokenizer::Token::EQUAL, R"(Expected "=" after "set")"); make_completion_context(COMPLETION_PROPERTY_METHOD, p_variable); @@ -972,11 +993,25 @@ void GDScriptParser::parse_property_setter(VariableNode *p_variable) { void GDScriptParser::parse_property_getter(VariableNode *p_variable) { switch (p_variable->property) { - case VariableNode::PROP_INLINE: + case VariableNode::PROP_INLINE: { consume(GDScriptTokenizer::Token::COLON, R"(Expected ":" after "get".)"); - p_variable->getter = parse_suite("getter definition"); + IdentifierNode *identifier = alloc_node<IdentifierNode>(); + identifier->name = "@" + p_variable->identifier->name + "_getter"; + + FunctionNode *function = alloc_node<FunctionNode>(); + function->identifier = identifier; + + FunctionNode *previous_function = current_function; + current_function = function; + + SuiteNode *body = alloc_node<SuiteNode>(); + function->body = parse_suite("getter declaration", body); + + p_variable->getter = function; + current_function = previous_function; break; + } case VariableNode::PROP_SETGET: consume(GDScriptTokenizer::Token::EQUAL, R"(Expected "=" after "get")"); make_completion_context(COMPLETION_PROPERTY_METHOD, p_variable); @@ -4437,7 +4472,7 @@ void GDScriptParser::TreePrinter::print_variable(VariableNode *p_variable) { if (p_variable->property == VariableNode::PROP_INLINE) { push_line(":"); increase_indent(); - print_suite(p_variable->getter); + print_suite(p_variable->getter->body); decrease_indent(); } else { push_line(" ="); @@ -4457,7 +4492,7 @@ void GDScriptParser::TreePrinter::print_variable(VariableNode *p_variable) { } push_line("):"); increase_indent(); - print_suite(p_variable->setter); + print_suite(p_variable->setter->body); decrease_indent(); } else { push_line(" ="); diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 593fb0cc5e..af9b973ada 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -1109,12 +1109,12 @@ public: PropertyStyle property = PROP_NONE; union { - SuiteNode *setter = nullptr; + FunctionNode *setter = nullptr; IdentifierNode *setter_pointer; }; IdentifierNode *setter_parameter = nullptr; union { - SuiteNode *getter = nullptr; + FunctionNode *getter = nullptr; IdentifierNode *getter_pointer; }; diff --git a/modules/gdscript/language_server/gdscript_extend_parser.cpp b/modules/gdscript/language_server/gdscript_extend_parser.cpp index 730e554476..f4c0c4d9bb 100644 --- a/modules/gdscript/language_server/gdscript_extend_parser.cpp +++ b/modules/gdscript/language_server/gdscript_extend_parser.cpp @@ -165,7 +165,7 @@ void ExtendGDScriptParser::parse_class_symbol(const GDScriptParser::ClassNode *p case ClassNode::Member::VARIABLE: { lsp::DocumentSymbol symbol; symbol.name = m.variable->identifier->name; - symbol.kind = m.variable->property == VariableNode::PropertyStyle::PROP_NONE ? lsp::SymbolKind::Variable : lsp::SymbolKind::Property; + symbol.kind = m.variable->property == VariableNode::PROP_NONE ? lsp::SymbolKind::Variable : lsp::SymbolKind::Property; symbol.deprecated = false; symbol.range.start.line = LINE_NUMBER_TO_INDEX(m.variable->start_line); symbol.range.start.character = LINE_NUMBER_TO_INDEX(m.variable->start_column); diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_function_get_type_error.gd b/modules/gdscript/tests/scripts/analyzer/errors/property_function_get_type_error.gd new file mode 100644 index 0000000000..f1be6aaa0c --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_function_get_type_error.gd @@ -0,0 +1,11 @@ +var _prop : int + +# Getter function has wrong return type. +var prop : String: + get = get_prop + +func get_prop(): + return _prop + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_function_get_type_error.out b/modules/gdscript/tests/scripts/analyzer/errors/property_function_get_type_error.out new file mode 100644 index 0000000000..29eec51ef2 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_function_get_type_error.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Function with return type "int" cannot be used as getter for a property of type "String". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_function_set_type_error.gd b/modules/gdscript/tests/scripts/analyzer/errors/property_function_set_type_error.gd new file mode 100644 index 0000000000..dd190157a1 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_function_set_type_error.gd @@ -0,0 +1,11 @@ +var _prop : int + +# Setter function has wrong argument type. +var prop : String: + set = set_prop + +func set_prop(value : int): + _prop = value + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_function_set_type_error.out b/modules/gdscript/tests/scripts/analyzer/errors/property_function_set_type_error.out new file mode 100644 index 0000000000..7a25280d55 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_function_set_type_error.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Function with argument type "int" cannot be used as setter for a property of type "String". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_inline_get_type_error.gd b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_get_type_error.gd new file mode 100644 index 0000000000..7f2b29222a --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_get_type_error.gd @@ -0,0 +1,9 @@ +var _prop : int + +# Inline getter returns int instead of String. +var prop : String: + get: + return _prop + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_inline_get_type_error.out b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_get_type_error.out new file mode 100644 index 0000000000..e0adef1bf8 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_get_type_error.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Cannot return value of type "int" because the function return type is "String". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_inline_set_type_error.gd b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_set_type_error.gd new file mode 100644 index 0000000000..0ce239dbbd --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_set_type_error.gd @@ -0,0 +1,9 @@ +var _prop : int + +# Inline setter assigns String to int. +var prop : String: + set(value): + _prop = value + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/property_inline_set_type_error.out b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_set_type_error.out new file mode 100644 index 0000000000..bbadf1ce27 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/property_inline_set_type_error.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Cannot assign a value of type "String" to a target of type "int". diff --git a/modules/gdscript/tests/scripts/analyzer/features/property_functions.gd b/modules/gdscript/tests/scripts/analyzer/features/property_functions.gd new file mode 100644 index 0000000000..1706087f82 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/property_functions.gd @@ -0,0 +1,16 @@ +var _prop = 1 +var prop: + get = get_prop, set = set_prop + +func get_prop(): + return _prop + +func set_prop(value): + _prop = value + +func test(): + print(prop) + + prop = 2 + + print(prop) diff --git a/modules/gdscript/tests/scripts/analyzer/features/property_functions.out b/modules/gdscript/tests/scripts/analyzer/features/property_functions.out new file mode 100644 index 0000000000..f1253ca57e --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/property_functions.out @@ -0,0 +1,3 @@ +GDTEST_OK +1 +2 diff --git a/modules/gdscript/tests/scripts/analyzer/features/property_inline.gd b/modules/gdscript/tests/scripts/analyzer/features/property_inline.gd new file mode 100644 index 0000000000..23eb011b23 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/property_inline.gd @@ -0,0 +1,46 @@ +# Untyped inline property +var prop1: + get: + return prop1 + set(value): + prop1 = value + +# Typed inline property +var prop2 : int: + get: + return prop2 + set(value): + prop2 = value + +# Typed inline property with default value +var prop3 : int = 1: + get: + return prop3 + set(value): + prop3 = value + +# Typed inline property with backing variable +var _prop4 : int = 2 +var prop4: int: + get: + return _prop4 + set(value): + _prop4 = value + +func test(): + print(prop1) + print(prop2) + print(prop3) + print(prop4) + + print() + + prop1 = 1 + prop2 = 2 + prop3 = 3 + prop4 = 4 + + print(prop1) + print(prop2) + print(prop3) + print(prop4) diff --git a/modules/gdscript/tests/scripts/analyzer/features/property_inline.out b/modules/gdscript/tests/scripts/analyzer/features/property_inline.out new file mode 100644 index 0000000000..5482592e90 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/property_inline.out @@ -0,0 +1,10 @@ +GDTEST_OK +null +0 +1 +2 + +1 +2 +3 +4 |