diff --git a/core/script_language.h b/core/script_language.h index 314b047027..6ba38399a1 100644 --- a/core/script_language.h +++ b/core/script_language.h @@ -294,7 +294,8 @@ public: /* EDITOR FUNCTIONS */ struct Warning { - int line; + int start_line = -1, end_line = -1; + int leftmost_column = -1, rightmost_column = -1; int code; String string_code; String message; diff --git a/editor/plugins/script_text_editor.cpp b/editor/plugins/script_text_editor.cpp index 1e03d9dfab..f4fdf8ccb0 100644 --- a/editor/plugins/script_text_editor.cpp +++ b/editor/plugins/script_text_editor.cpp @@ -494,7 +494,7 @@ void ScriptTextEditor::_validate_script() { ScriptLanguage::Warning w = E->get(); Dictionary ignore_meta; - ignore_meta["line"] = w.line; + ignore_meta["line"] = w.start_line; ignore_meta["code"] = w.string_code.to_lower(); warnings_panel->push_cell(); warnings_panel->push_meta(ignore_meta); @@ -506,9 +506,9 @@ void ScriptTextEditor::_validate_script() { warnings_panel->pop(); // Cell. warnings_panel->push_cell(); - warnings_panel->push_meta(w.line - 1); + warnings_panel->push_meta(w.start_line - 1); warnings_panel->push_color(warnings_panel->get_theme_color("warning_color", "Editor")); - warnings_panel->add_text(TTR("Line") + " " + itos(w.line)); + warnings_panel->add_text(TTR("Line") + " " + itos(w.start_line)); warnings_panel->add_text(" (" + w.string_code + "):"); warnings_panel->pop(); // Color. warnings_panel->pop(); // Meta goto. diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 65cd791ad9..dffea329b4 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -43,6 +43,7 @@ #include "gdscript_cache.h" #include "gdscript_compiler.h" #include "gdscript_parser.h" +#include "gdscript_warning.h" /////////////////////////// @@ -469,10 +470,9 @@ bool GDScript::_update_exports(bool *r_err, bool p_recursive_call) { } members_cache.push_back(member.variable->export_info); - // FIXME: Get variable's default value in non-literal cases. Variant default_value; - if (member.variable->initializer != nullptr && member.variable->initializer->type == GDScriptParser::Node::LITERAL) { - default_value = static_cast(member.variable->initializer)->value; + if (member.variable->initializer->is_constant) { + default_value = member.variable->initializer->reduced_value; } member_default_values_cache[member.variable->identifier->name] = default_value; } break; @@ -637,14 +637,13 @@ Error GDScript::reload(bool p_keep_state) { } } #ifdef DEBUG_ENABLED - // FIXME: Add warnings. - // for (const List::Element *E = parser.get_warnings().front(); E; E = E->next()) { - // const GDScriptWarning &warning = E->get(); - // if (EngineDebugger::is_active()) { - // Vector si; - // EngineDebugger::get_script_debugger()->send_error("", get_path(), warning.line, warning.get_name(), warning.get_message(), ERR_HANDLER_WARNING, si); - // } - // } + for (const List::Element *E = parser.get_warnings().front(); E; E = E->next()) { + const GDScriptWarning &warning = E->get(); + if (EngineDebugger::is_active()) { + Vector si; + EngineDebugger::get_script_debugger()->send_error("", get_path(), warning.start_line, warning.get_name(), warning.get_message(), ERR_HANDLER_WARNING, si); + } + } #endif valid = true; @@ -2044,7 +2043,7 @@ GDScriptLanguage::GDScriptLanguage() { GLOBAL_DEF("debug/gdscript/completion/autocomplete_setters_and_getters", false); for (int i = 0; i < (int)GDScriptWarning::WARNING_MAX; i++) { String warning = GDScriptWarning::get_name_from_code((GDScriptWarning::Code)i).to_lower(); - bool default_enabled = !warning.begins_with("unsafe_") && i != GDScriptWarning::UNUSED_CLASS_VARIABLE; + bool default_enabled = !warning.begins_with("unsafe_"); GLOBAL_DEF("debug/gdscript/warnings/" + warning, default_enabled); } #endif // DEBUG_ENABLED diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h index 4f2b1cadd3..8236464f15 100644 --- a/modules/gdscript/gdscript.h +++ b/modules/gdscript/gdscript.h @@ -303,52 +303,6 @@ public: ~GDScriptInstance(); }; -#ifdef DEBUG_ENABLED -struct GDScriptWarning { - enum Code { - UNASSIGNED_VARIABLE, // Variable used but never assigned - UNASSIGNED_VARIABLE_OP_ASSIGN, // Variable never assigned but used in an assignment operation (+=, *=, etc) - UNUSED_VARIABLE, // Local variable is declared but never used - SHADOWED_VARIABLE, // Variable name shadowed by other variable - UNUSED_CLASS_VARIABLE, // Class variable is declared but never used in the file - UNUSED_ARGUMENT, // Function argument is never used - UNREACHABLE_CODE, // Code after a return statement - STANDALONE_EXPRESSION, // Expression not assigned to a variable - VOID_ASSIGNMENT, // Function returns void but it's assigned to a variable - NARROWING_CONVERSION, // Float value into an integer slot, precision is lost - FUNCTION_MAY_YIELD, // Typed assign of function call that yields (it may return a function state) - VARIABLE_CONFLICTS_FUNCTION, // Variable has the same name of a function - FUNCTION_CONFLICTS_VARIABLE, // Function has the same name of a variable - FUNCTION_CONFLICTS_CONSTANT, // Function has the same name of a constant - INCOMPATIBLE_TERNARY, // Possible values of a ternary if are not mutually compatible - UNUSED_SIGNAL, // Signal is defined but never emitted - RETURN_VALUE_DISCARDED, // Function call returns something but the value isn't used - PROPERTY_USED_AS_FUNCTION, // Function not found, but there's a property with the same name - CONSTANT_USED_AS_FUNCTION, // Function not found, but there's a constant with the same name - FUNCTION_USED_AS_PROPERTY, // Property not found, but there's a function with the same name - INTEGER_DIVISION, // Integer divide by integer, decimal part is discarded - UNSAFE_PROPERTY_ACCESS, // Property not found in the detected type (but can be in subtypes) - UNSAFE_METHOD_ACCESS, // Function not found in the detected type (but can be in subtypes) - UNSAFE_CAST, // Cast used in an unknown type - UNSAFE_CALL_ARGUMENT, // Function call argument is of a supertype of the require argument - DEPRECATED_KEYWORD, // The keyword is deprecated and should be replaced - STANDALONE_TERNARY, // Return value of ternary expression is discarded - WARNING_MAX, - }; - - Code code = WARNING_MAX; - Vector symbols; - int line = -1; - - String get_name() const; - String get_message() const; - static String get_name_from_code(Code p_code); - static Code get_code_from_name(const String &p_name); - - GDScriptWarning() {} -}; -#endif // DEBUG_ENABLED - class GDScriptLanguage : public ScriptLanguage { friend class GDScriptFunctionState; diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 79690e2cf2..2dd84abc56 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -259,7 +259,7 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class, return OK; } -GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(const GDScriptParser::TypeNode *p_type) { +GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(GDScriptParser::TypeNode *p_type) { GDScriptParser::DataType result; if (p_type == nullptr) { @@ -274,12 +274,22 @@ GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(const GDScriptParser // void. result.kind = GDScriptParser::DataType::BUILTIN; result.builtin_type = Variant::NIL; + p_type->set_datatype(result); return result; } StringName first = p_type->type_chain[0]->name; - if (GDScriptParser::get_builtin_type(first) != Variant::NIL) { + if (first == "Variant") { + result.kind = GDScriptParser::DataType::VARIANT; + if (p_type->type_chain.size() > 1) { + push_error(R"("Variant" type don't contain nested types.)", p_type->type_chain[1]); + return GDScriptParser::DataType(); + } + return result; + } + + if (GDScriptParser::get_builtin_type(first) < Variant::VARIANT_MAX) { // Built-in types. if (p_type->type_chain.size() > 1) { push_error(R"(Built-in types don't contain nested types.)", p_type->type_chain[1]); @@ -363,6 +373,7 @@ GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(const GDScriptParser } } + p_type->set_datatype(result); return result; } @@ -385,17 +396,34 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas datatype.type_source = GDScriptParser::DataType::UNDETECTED; if (member.variable->initializer != nullptr) { + member.variable->set_datatype(datatype); // Allow recursive usage. reduce_expression(member.variable->initializer); datatype = member.variable->initializer->get_datatype(); } if (member.variable->datatype_specifier != nullptr) { datatype = resolve_datatype(member.variable->datatype_specifier); - } else if (member.variable->infer_datatype) { + if (member.variable->initializer != nullptr) { - push_error(vformat(R"(Cannot infer the type of "%s" variable because there's no default value.)", member.variable->identifier), member.variable->identifier); + if (!is_type_compatible(datatype, member.variable->initializer->get_datatype(), true)) { + push_error(vformat(R"(Value of type "%s" cannot be assigned to variable of type "%s".)", member.variable->initializer->get_datatype().to_string(), datatype.to_string()), member.variable->initializer); + } else if (datatype.builtin_type == Variant::INT && member.variable->initializer->get_datatype().builtin_type == Variant::FLOAT) { + parser->push_warning(member.variable->initializer, GDScriptWarning::NARROWING_CONVERSION); + } + if (member.variable->initializer->get_datatype().is_variant()) { + // TODO: Warn unsafe assign. + mark_node_unsafe(member.variable->initializer); + } + } + } else if (member.variable->infer_datatype) { + if (member.variable->initializer == nullptr) { + push_error(vformat(R"(Cannot infer the type of "%s" variable because there's no default value.)", member.variable->identifier->name), member.variable->identifier); } else if (!datatype.is_set() || datatype.has_no_type()) { - push_error(vformat(R"(Cannot infer the type of "%s" variable because the default value doesn't have a set type.)", member.variable->identifier), member.variable->initializer); + push_error(vformat(R"(Cannot infer the type of "%s" variable because the initial value doesn't have a set type.)", member.variable->identifier->name), member.variable->initializer); + } else if (datatype.is_variant()) { + push_error(vformat(R"(Cannot infer the type of "%s" variable because the initial value is Variant. Use explicit "Variant" type if this is intended.)", member.variable->identifier->name), member.variable->initializer); + } else if (datatype.builtin_type == Variant::NIL) { + push_error(vformat(R"(Cannot infer the type of "%s" variable because the initial value is "null".)", member.variable->identifier->name), member.variable->initializer); } } @@ -430,7 +458,22 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas if (!member.constant->initializer->is_constant) { push_error(R"(Initializer for a constant must be a constant expression.)", member.constant->initializer); } - member.constant->set_datatype(member.constant->initializer->get_datatype()); + + GDScriptParser::DataType datatype = member.constant->get_datatype(); + + if (member.constant->datatype_specifier != nullptr) { + datatype = resolve_datatype(member.constant->datatype_specifier); + + if (!is_type_compatible(datatype, member.constant->initializer->get_datatype(), true)) { + push_error(vformat(R"(Value of type "%s" cannot be initialized to constant of type "%s".)", member.constant->initializer->get_datatype().to_string(), datatype.to_string()), member.constant->initializer); + } else if (datatype.builtin_type == Variant::INT && member.constant->initializer->get_datatype().builtin_type == Variant::FLOAT) { + parser->push_warning(member.constant->initializer, GDScriptWarning::NARROWING_CONVERSION); + } + } + + datatype.is_constant = true; + + member.constant->set_datatype(datatype); } break; case GDScriptParser::ClassNode::Member::SIGNAL: { for (int j = 0; j < member.signal->parameters.size(); j++) { @@ -508,6 +551,17 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class) { resolve_class_body(member.m_class); } + + // Check unused variables. + 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.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); + } + } } void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node) { @@ -610,6 +664,11 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode * for (int i = 0; i < p_function->parameters.size(); i++) { resolve_pararameter(p_function->parameters[i]); + if (p_function->parameters[i]->usages == 0 && !String(p_function->parameters[i]->identifier->name).begins_with("_")) { + parser->push_warning(p_function->parameters[i]->identifier, GDScriptWarning::UNUSED_PARAMETER, p_function->identifier->name, p_function->parameters[i]->identifier->name); + } + + is_shadowing(p_function->parameters[i]->identifier, "function parameter"); } p_function->set_datatype(resolve_datatype(p_function->return_type)); @@ -630,10 +689,14 @@ void GDScriptAnalyzer::resolve_function_body(GDScriptParser::FunctionNode *p_fun GDScriptParser::DataType return_type = p_function->body->get_datatype(); - if (p_function->get_datatype().type_source == GDScriptParser::DataType::UNDETECTED && return_type.is_set()) { + if (p_function->get_datatype().has_no_type() && return_type.is_set()) { // Use the suite inferred type if return isn't explicitly set. return_type.type_source = GDScriptParser::DataType::INFERRED; p_function->set_datatype(p_function->body->get_datatype()); + } else if (p_function->get_datatype().is_hard_type() && (p_function->get_datatype().kind != GDScriptParser::DataType::BUILTIN || p_function->get_datatype().builtin_type != Variant::NIL)) { + if (!p_function->body->has_return) { + push_error(R"(Not all code paths return a value.)", p_function); + } } parser->current_function = previous_function; @@ -758,6 +821,8 @@ void GDScriptAnalyzer::resolve_for(GDScriptParser::ForNode *p_for) { resolve_suite(p_for->loop); p_for->set_datatype(p_for->loop->get_datatype()); + + is_shadowing(p_for->variable, R"("for" iterator variable)"); } void GDScriptAnalyzer::resolve_while(GDScriptParser::WhileNode *p_while) { @@ -769,6 +834,7 @@ void GDScriptAnalyzer::resolve_while(GDScriptParser::WhileNode *p_while) { void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable) { GDScriptParser::DataType type; + type.kind = GDScriptParser::DataType::VARIANT; // By default. if (p_variable->initializer != nullptr) { reduce_expression(p_variable->initializer); @@ -780,19 +846,49 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable if (type.has_no_type()) { push_error(vformat(R"(Could not infer the type of the variable "%s" because the initial value does not have a set type.)", p_variable->identifier->name), p_variable->initializer); } else if (type.is_variant()) { - push_error(vformat(R"(Could not infer the type of the variable "%s" because the initial value is a variant.)", p_variable->identifier->name), p_variable->initializer); + push_error(vformat(R"(Could not infer the type of the variable "%s" because the initial value is a variant. Use explicit "Variant" type if this is intended.)", p_variable->identifier->name), p_variable->initializer); + } else if (type.builtin_type == Variant::NIL) { + push_error(vformat(R"(Could not infer the type of the variable "%s" because the initial value is "null".)", p_variable->identifier->name), p_variable->initializer); } } else { type.type_source = GDScriptParser::DataType::INFERRED; } + if (p_variable->initializer->type == GDScriptParser::Node::CALL && type.kind == GDScriptParser::DataType::BUILTIN && type.builtin_type == Variant::NIL) { + parser->push_warning(p_variable->initializer, GDScriptWarning::VOID_ASSIGNMENT, static_cast(p_variable->initializer)->function_name); + } } if (p_variable->datatype_specifier != nullptr) { type = resolve_datatype(p_variable->datatype_specifier); + + if (p_variable->initializer != nullptr) { + if (!is_type_compatible(type, p_variable->initializer->get_datatype(), true)) { + push_error(vformat(R"(Value of type "%s" cannot be assigned to variable of type "%s".)", p_variable->initializer->get_datatype().to_string(), type.to_string()), p_variable->initializer); + } else if (type.builtin_type == Variant::INT && p_variable->initializer->get_datatype().builtin_type == Variant::FLOAT) { + parser->push_warning(p_variable->initializer, GDScriptWarning::NARROWING_CONVERSION); + } + if (p_variable->initializer->get_datatype().is_variant()) { + // TODO: Warn unsafe assign. + mark_node_unsafe(p_variable->initializer); + } + } + } else if (p_variable->infer_datatype) { + if (type.has_no_type()) { + push_error(vformat(R"(Cannot infer the type of variable "%s" because the initial value doesn't have a set type.)", p_variable->identifier->name), p_variable->identifier); + } + type.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED; } type.is_constant = false; p_variable->set_datatype(type); + + if (p_variable->usages == 0 && !String(p_variable->identifier->name).begins_with("_")) { + parser->push_warning(p_variable, GDScriptWarning::UNUSED_VARIABLE, p_variable->identifier->name); + } else if (p_variable->assignments == 0) { + parser->push_warning(p_variable, GDScriptWarning::UNASSIGNED_VARIABLE, p_variable->identifier->name); + } + + is_shadowing(p_variable->identifier, "variable"); } void GDScriptAnalyzer::resolve_constant(GDScriptParser::ConstantNode *p_constant) { @@ -806,25 +902,50 @@ void GDScriptAnalyzer::resolve_constant(GDScriptParser::ConstantNode *p_constant type = p_constant->initializer->get_datatype(); + if (p_constant->initializer->type == GDScriptParser::Node::CALL && type.kind == GDScriptParser::DataType::BUILTIN && type.builtin_type == Variant::NIL) { + parser->push_warning(p_constant->initializer, GDScriptWarning::VOID_ASSIGNMENT, static_cast(p_constant->initializer)->function_name); + } + if (p_constant->datatype_specifier != nullptr) { GDScriptParser::DataType explicit_type = resolve_datatype(p_constant->datatype_specifier); if (!is_type_compatible(explicit_type, type)) { push_error(vformat(R"(Assigned value for constant "%s" has type %s which is not compatible with defined type %s.)", p_constant->identifier->name, type.to_string(), explicit_type.to_string()), p_constant->initializer); + } else if (explicit_type.builtin_type == Variant::INT && type.builtin_type == Variant::FLOAT) { + parser->push_warning(p_constant->initializer, GDScriptWarning::NARROWING_CONVERSION); } type = explicit_type; + } else if (p_constant->infer_datatype) { + if (type.has_no_type()) { + push_error(vformat(R"(Cannot infer the type of constant "%s" because the initial value doesn't have a set type.)", p_constant->identifier->name), p_constant->identifier); + } + type.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED; } type.is_constant = true; p_constant->set_datatype(type); + + if (p_constant->usages == 0) { + parser->push_warning(p_constant, GDScriptWarning::UNUSED_LOCAL_CONSTANT, p_constant->identifier->name); + } + + is_shadowing(p_constant->identifier, "constant"); } void GDScriptAnalyzer::resolve_assert(GDScriptParser::AssertNode *p_assert) { reduce_expression(p_assert->condition); - reduce_literal(p_assert->message); + if (p_assert->message != nullptr) { + reduce_literal(p_assert->message); + } p_assert->set_datatype(p_assert->condition->get_datatype()); - // TODO: Warn when expression is constantly true or false. + if (p_assert->condition->is_constant) { + if (p_assert->condition->reduced_value.booleanize()) { + parser->push_warning(p_assert->condition, GDScriptWarning::ASSERT_ALWAYS_TRUE); + } else { + parser->push_warning(p_assert->condition, GDScriptWarning::ASSERT_ALWAYS_FALSE); + } + } } void GDScriptAnalyzer::resolve_match(GDScriptParser::MatchNode *p_match) { @@ -869,6 +990,10 @@ void GDScriptAnalyzer::resolve_match_pattern(GDScriptParser::PatternNode *p_matc result.kind = GDScriptParser::DataType::VARIANT; } p_match_pattern->bind->set_datatype(result); + is_shadowing(p_match_pattern->bind, "pattern bind"); + if (p_match_pattern->bind->usages == 0) { + parser->push_warning(p_match_pattern->bind, GDScriptWarning::UNASSIGNED_VARIABLE, p_match_pattern->bind->name); + } break; case GDScriptParser::PatternNode::PT_ARRAY: for (int i = 0; i < p_match_pattern->array.size(); i++) { @@ -904,9 +1029,6 @@ void GDScriptAnalyzer::resolve_pararameter(GDScriptParser::ParameterNode *p_para if (p_parameter->default_value != nullptr) { reduce_expression(p_parameter->default_value); - if (p_parameter->default_value->is_constant) { - push_error(vformat(R"(Default value for paramater "%s" is not a constant.)", p_parameter->identifier->name), p_parameter->default_value); - } result = p_parameter->default_value->get_datatype(); result.type_source = GDScriptParser::DataType::INFERRED; } @@ -916,7 +1038,7 @@ void GDScriptAnalyzer::resolve_pararameter(GDScriptParser::ParameterNode *p_para if (p_parameter->default_value != nullptr) { if (!is_type_compatible(p_parameter->datatype_specifier->get_datatype(), p_parameter->default_value->get_datatype())) { - push_error(vformat(R"(Type of default value for parameter "%s" is not compatible with paremeter type.)", p_parameter->identifier->name), p_parameter->default_value); + push_error(vformat(R"(Type of default value for parameter "%s" (%s) is not compatible with paremeter type (%s).)", p_parameter->identifier->name, p_parameter->default_value->get_datatype().to_string(), p_parameter->datatype_specifier->get_datatype().to_string()), p_parameter->default_value); } } @@ -940,6 +1062,23 @@ void GDScriptAnalyzer::resolve_return(GDScriptParser::ReturnNode *p_return) { result.is_constant = true; } + GDScriptParser::DataType function_type = parser->current_function->get_datatype(); + 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); + } + } 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); + } + } + p_return->set_datatype(result); } @@ -1112,10 +1251,22 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig break; } } + + GDScriptParser::DataType assignee_type = p_assignment->assignee->get_datatype(); + GDScriptParser::DataType assigned_type = p_assignment->assigned_value->get_datatype(); + if (p_assignment->assigned_value->type == GDScriptParser::Node::CALL && assigned_type.kind == GDScriptParser::DataType::BUILTIN && assigned_type.builtin_type == Variant::NIL) { + parser->push_warning(p_assignment->assigned_value, GDScriptWarning::VOID_ASSIGNMENT, static_cast(p_assignment->assigned_value)->function_name); + } else if (assignee_type.is_hard_type() && assignee_type.builtin_type == Variant::INT && assigned_type.builtin_type == Variant::FLOAT) { + parser->push_warning(p_assignment->assigned_value, GDScriptWarning::NARROWING_CONVERSION); + } } void GDScriptAnalyzer::reduce_await(GDScriptParser::AwaitNode *p_await) { - reduce_expression(p_await->to_await); + if (p_await->to_await->type == GDScriptParser::Node::CALL) { + reduce_call(static_cast(p_await->to_await), true); + } else { + reduce_expression(p_await->to_await); + } p_await->is_constant = p_await->to_await->is_constant; p_await->reduced_value = p_await->to_await->reduced_value; @@ -1124,7 +1275,9 @@ void GDScriptAnalyzer::reduce_await(GDScriptParser::AwaitNode *p_await) { p_await->set_datatype(awaiting_type); - // TODO: Add warning if await is redundant. + if (!awaiting_type.is_coroutine && awaiting_type.builtin_type != Variant::SIGNAL) { + parser->push_warning(p_await, GDScriptWarning::REDUNDANT_AWAIT); + } } void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_op) { @@ -1133,13 +1286,20 @@ void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_o // TODO: Right operand must be a valid type with the `is` operator. Need to check here. + GDScriptParser::DataType left_type = p_binary_op->left_operand->get_datatype(); + GDScriptParser::DataType right_type = p_binary_op->right_operand->get_datatype(); + + if (p_binary_op->variant_op == Variant::OP_DIVIDE && left_type.builtin_type == Variant::INT && right_type.builtin_type == Variant::INT) { + parser->push_warning(p_binary_op, GDScriptWarning::INTEGER_DIVISION); + } + if (p_binary_op->left_operand->is_constant && p_binary_op->right_operand->is_constant) { p_binary_op->is_constant = true; if (p_binary_op->variant_op < Variant::OP_MAX) { p_binary_op->reduced_value = Variant::evaluate(p_binary_op->variant_op, p_binary_op->left_operand->reduced_value, p_binary_op->right_operand->reduced_value); } else { if (p_binary_op->operation == GDScriptParser::BinaryOpNode::OP_TYPE_TEST) { - GDScriptParser::DataType test_type = p_binary_op->right_operand->get_datatype(); + GDScriptParser::DataType test_type = right_type; test_type.is_meta_type = false; if (!is_type_compatible(test_type, p_binary_op->left_operand->get_datatype(), false)) { @@ -1153,26 +1313,27 @@ void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_o } } p_binary_op->set_datatype(type_from_variant(p_binary_op->reduced_value)); + return; } GDScriptParser::DataType result; - if (p_binary_op->left_operand->get_datatype().is_variant() || p_binary_op->right_operand->get_datatype().is_variant()) { + if (left_type.is_variant() || right_type.is_variant()) { // Cannot infer type because one operand can be anything. result.kind = GDScriptParser::DataType::VARIANT; mark_node_unsafe(p_binary_op); } else { if (p_binary_op->variant_op < Variant::OP_MAX) { bool valid = false; - result = get_operation_type(p_binary_op->variant_op, p_binary_op->left_operand->get_datatype(), p_binary_op->right_operand->get_datatype(), valid); + result = get_operation_type(p_binary_op->variant_op, p_binary_op->left_operand->get_datatype(), right_type, valid); if (!valid) { - push_error(vformat(R"(Invalid operands "%s" and "%s" for "%s" operator.)", p_binary_op->left_operand->get_datatype().to_string(), p_binary_op->right_operand->get_datatype().to_string(), Variant::get_operator_name(p_binary_op->variant_op)), p_binary_op); + push_error(vformat(R"(Invalid operands "%s" and "%s" for "%s" operator.)", p_binary_op->left_operand->get_datatype().to_string(), right_type.to_string(), Variant::get_operator_name(p_binary_op->variant_op)), p_binary_op); } } else { if (p_binary_op->operation == GDScriptParser::BinaryOpNode::OP_TYPE_TEST) { - GDScriptParser::DataType test_type = p_binary_op->right_operand->get_datatype(); + GDScriptParser::DataType test_type = right_type; test_type.is_meta_type = false; if (!is_type_compatible(test_type, p_binary_op->left_operand->get_datatype(), false)) { @@ -1180,6 +1341,7 @@ void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_o push_error(vformat(R"(Expression is of type "%s" so it can't be of type "%s".)", p_binary_op->left_operand->get_datatype().to_string(), test_type.to_string()), p_binary_op->left_operand); } else { // TODO: Warning. + mark_node_unsafe(p_binary_op); } } @@ -1196,7 +1358,7 @@ void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_o p_binary_op->set_datatype(result); } -void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call) { +void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool is_await) { bool all_is_constant = true; for (int i = 0; i < p_call->arguments.size(); i++) { reduce_expression(p_call->arguments[i]); @@ -1300,7 +1462,9 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call) { types_match = false; break; } else { - // TODO: Check narrowing conversion. + if (par_type.builtin_type == Variant::INT && p_call->arguments[i]->get_datatype().builtin_type == Variant::FLOAT) { + parser->push_warning(p_call, GDScriptWarning::NARROWING_CONVERSION, p_call->function_name); + } } } @@ -1364,13 +1528,13 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call) { } else { validate_call_arg(function_info, p_call); } + p_call->set_datatype(type_from_property(function_info.return_val)); return; } } GDScriptParser::DataType base_type; - GDScriptParser::DataType result; - result.kind = GDScriptParser::DataType::VARIANT; + call_type.kind = GDScriptParser::DataType::VARIANT; bool is_self = false; if (p_call->is_super) { @@ -1382,9 +1546,9 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call) { } else if (p_call->callee->type == GDScriptParser::Node::SUBSCRIPT) { GDScriptParser::SubscriptNode *subscript = static_cast(p_call->callee); if (!subscript->is_attribute) { - // Invalid call. + // Invalid call. Error already sent in parser. // TODO: Could check if Callable here. - p_call->set_datatype(result); + p_call->set_datatype(call_type); mark_node_unsafe(p_call); return; } @@ -1392,8 +1556,9 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call) { base_type = subscript->base->get_datatype(); } else { + // Invalid call. Error already sent in parser. // TODO: Could check if Callable here too. - p_call->set_datatype(result); + p_call->set_datatype(call_type); mark_node_unsafe(p_call); return; } @@ -1411,13 +1576,43 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call) { push_error(vformat(R"*(Cannot call non-static function "%s()" from static function "%s()".)*", p_call->function_name, parser->current_function->identifier->name), p_call->callee); } - result = return_type; + call_type = return_type; } else { - String base_name = is_self && !p_call->is_super ? "self" : base_type.to_string(); - push_error(vformat(R"*(Function "%s()" not found in base %s.)*", p_call->function_name, base_name), p_call->is_super ? p_call : p_call->callee); + // Check if the name exists as something else. + bool found = false; + if (!p_call->is_super) { + GDScriptParser::IdentifierNode *callee_id; + if (p_call->callee->type == GDScriptParser::Node::IDENTIFIER) { + callee_id = static_cast(p_call->callee); + } else { + // Can only be attribute. + callee_id = static_cast(p_call->callee)->attribute; + } + reduce_identifier_from_base(callee_id, &base_type); + GDScriptParser::DataType callee_type = callee_id->get_datatype(); + if (callee_type.is_set() && !callee_type.is_variant()) { + found = true; + if (callee_type.builtin_type == Variant::CALLABLE) { + push_error(vformat(R"*(Name "%s" is a Callable. You can call it with "%s.call()" instead.)*", p_call->function_name, p_call->function_name), p_call->callee); + } else { + push_error(vformat(R"*(Name "%s" called as a function but is a "%s".)*", p_call->function_name, callee_type.to_string()), p_call->callee); + } + } else if (!is_self) { + parser->push_warning(p_call, GDScriptWarning::UNSAFE_METHOD_ACCESS, p_call->function_name, base_type.to_string()); + mark_node_unsafe(p_call); + } + } + if (!found && is_self) { + String base_name = is_self && !p_call->is_super ? "self" : base_type.to_string(); + push_error(vformat(R"*(Function "%s()" not found in base %s.)*", p_call->function_name, base_name), p_call->is_super ? p_call : p_call->callee); + } } - p_call->set_datatype(result); + if (call_type.is_coroutine && !is_await) { + push_error(vformat(R"*(Function "%s()" is a coroutine, so it must be called with "await".)*", p_call->function_name), p_call->callee); + } + + p_call->set_datatype(call_type); } void GDScriptAnalyzer::reduce_cast(GDScriptParser::CastNode *p_cast) { @@ -1449,11 +1644,13 @@ void GDScriptAnalyzer::reduce_cast(GDScriptParser::CastNode *p_cast) { if (!valid) { push_error(vformat(R"(Invalid cast. Cannot convert from "%s" to "%s".)", op_type.to_string(), cast_type.to_string()), p_cast->cast_type); } - } else { - mark_node_unsafe(p_cast); } } } + if (p_cast->operand->get_datatype().is_variant()) { + parser->push_warning(p_cast, GDScriptWarning::UNSAFE_CAST, cast_type.to_string()); + mark_node_unsafe(p_cast); + } // TODO: Perform cast on constants. } @@ -1559,6 +1756,8 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod } else if (member.type == GDScriptParser::ClassNode::Member::VARIABLE) { p_identifier->source = GDScriptParser::IdentifierNode::MEMBER_VARIABLE; p_identifier->variable_source = member.variable; + } else if (member.type == GDScriptParser::ClassNode::Member::FUNCTION) { + p_identifier->set_datatype(make_callable_type(member.function->info)); } return; } @@ -1612,8 +1811,6 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod p_identifier->set_datatype(type_from_variant(int_constant)); return; } - } else { - ERR_PRINT(vformat("GDScript parser bug: Class %s isn't a native exposed class.", native.operator String())); } } @@ -1624,7 +1821,7 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident switch (p_identifier->source) { case GDScriptParser::IdentifierNode::FUNCTION_PARAMETER: p_identifier->set_datatype(p_identifier->parameter_source->get_datatype()); - break; + return; case GDScriptParser::IdentifierNode::LOCAL_CONSTANT: case GDScriptParser::IdentifierNode::MEMBER_CONSTANT: p_identifier->set_datatype(p_identifier->constant_source->get_datatype()); @@ -1632,8 +1829,10 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident // TODO: Constant should have a value on the node itself. p_identifier->reduced_value = p_identifier->constant_source->initializer->reduced_value; return; - case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: + p_identifier->variable_source->usages++; + [[fallthrough]]; + case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: p_identifier->set_datatype(p_identifier->variable_source->get_datatype()); return; case GDScriptParser::IdentifierNode::LOCAL_ITERATOR: @@ -1693,7 +1892,12 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident } // Not found. - push_error(vformat(R"(Identifier "%s" not declared in the current scope.)", name), p_identifier); + // Check if it's a builtin function. + if (parser->get_builtin_function(name) < GDScriptFunctions::FUNC_MAX) { + push_error(vformat(R"(Built-in function "%s" cannot be used as an identifier.)", name), p_identifier); + } else { + push_error(vformat(R"(Identifier "%s" not declared in the current scope.)", name), p_identifier); + } GDScriptParser::DataType dummy; dummy.kind = GDScriptParser::DataType::VARIANT; p_identifier->set_datatype(dummy); // Just so type is set to something. @@ -1765,7 +1969,11 @@ void GDScriptAnalyzer::reduce_subscript(GDScriptParser::SubscriptNode *p_subscri p_subscript->is_constant = p_subscript->attribute->is_constant; p_subscript->reduced_value = p_subscript->attribute->reduced_value; } else { - push_error(vformat(R"(Cannot find member "%s" in base "%s".)", p_subscript->attribute->name, base_type.to_string()), p_subscript->attribute); + if (base_type.kind == GDScriptParser::DataType::BUILTIN) { + push_error(vformat(R"(Cannot find member "%s" in base "%s".)", p_subscript->attribute->name, base_type.to_string()), p_subscript->attribute); + } else { + parser->push_warning(p_subscript, GDScriptWarning::UNSAFE_PROPERTY_ACCESS, p_subscript->attribute->name, base_type.to_string()); + } result_type.kind = GDScriptParser::DataType::VARIANT; } } @@ -1970,9 +2178,10 @@ void GDScriptAnalyzer::reduce_ternary_op(GDScriptParser::TernaryOpNode *p_ternar if (!is_type_compatible(true_type, false_type)) { result = false_type; if (!is_type_compatible(false_type, true_type)) { - // TODO: Add warning here. Types of arms are not compatible with each other. result.type_source = GDScriptParser::DataType::UNDETECTED; result.kind = GDScriptParser::DataType::VARIANT; + + parser->push_warning(p_ternary_op, GDScriptWarning::INCOMPATIBLE_TERNARY); } } } @@ -2213,7 +2422,7 @@ bool GDScriptAnalyzer::validate_call_arg(const List &p GDScriptParser::DataType par_type = p_par_types[i]; GDScriptParser::DataType arg_type = p_call->arguments[i]->get_datatype(); - if (arg_type.kind == GDScriptParser::DataType::VARIANT) { + if (arg_type.is_variant()) { // Argument can be anything, so this is unsafe. mark_node_unsafe(p_call->arguments[i]); } else if (!is_type_compatible(par_type, arg_type, true)) { @@ -2227,13 +2436,55 @@ bool GDScriptAnalyzer::validate_call_arg(const List &p } } else { if (par_type.kind == GDScriptParser::DataType::BUILTIN && par_type.builtin_type == Variant::INT && arg_type.kind == GDScriptParser::DataType::BUILTIN && arg_type.builtin_type == Variant::FLOAT) { - // TODO: Add narrowing conversion warning. + parser->push_warning(p_call, GDScriptWarning::NARROWING_CONVERSION, p_call->function_name); } } } return valid; } +bool GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context) { + const StringName &name = p_local->name; + GDScriptParser::DataType base = parser->current_class->get_datatype(); + + GDScriptParser::ClassNode *base_class = base.class_type; + + while (base_class != nullptr) { + if (base_class->has_member(name)) { + parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE, p_context, p_local->name, base_class->get_member(name).get_type_name(), itos(base_class->get_member(name).get_line())); + return true; + } + base_class = base_class->base_type.class_type; + } + + StringName base_native = base.native_type; + + ERR_FAIL_COND_V_MSG(!class_exists(base_native), false, "Non-existent native base class."); + + StringName parent = base_native; + while (parent != StringName()) { + if (ClassDB::has_method(parent, name, true)) { + parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "method", parent); + return true; + } else if (ClassDB::has_signal(parent, name, true)) { + parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "signal", parent); + return true; + } else if (ClassDB::has_property(parent, name, true)) { + parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "property", parent); + return true; + } else if (ClassDB::has_integer_constant(parent, name, true)) { + parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "constant", parent); + return true; + } else if (ClassDB::has_enum(parent, name, true)) { + parser->push_warning(p_local, GDScriptWarning::SHADOWED_VARIABLE_BASE_CLASS, p_context, p_local->name, "enum", parent); + return true; + } + parent = ClassDB::get_parent_class(parent); + } + + return false; +} + GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid) { // This function creates dummy variant values and apply the operation to those. Less error-prone than keeping a table of valid operations. @@ -2485,6 +2736,13 @@ Error GDScriptAnalyzer::resolve_body() { Error GDScriptAnalyzer::resolve_program() { resolve_class_interface(parser->head); resolve_class_body(parser->head); + + List parser_keys; + depended_parsers.get_key_list(&parser_keys); + for (const List::Element *E = parser_keys.front(); E != nullptr; E = E->next()) { + depended_parsers[E->get()]->raise_status(GDScriptParserRef::FULLY_SOLVED); + } + depended_parsers.clear(); return parser->errors.empty() ? OK : ERR_PARSE_ERROR; } diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index 2794e55b3d..5de5c7ab85 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -42,7 +42,7 @@ class GDScriptAnalyzer { HashMap> depended_parsers; Error resolve_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive = true); - GDScriptParser::DataType resolve_datatype(const GDScriptParser::TypeNode *p_type); + GDScriptParser::DataType resolve_datatype(GDScriptParser::TypeNode *p_type); void decide_suite_type(GDScriptParser::Node *p_suite, GDScriptParser::Node *p_statement); @@ -74,7 +74,7 @@ class GDScriptAnalyzer { void reduce_assignment(GDScriptParser::AssignmentNode *p_assignment); void reduce_await(GDScriptParser::AwaitNode *p_await); void reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_op); - void reduce_call(GDScriptParser::CallNode *p_call); + void reduce_call(GDScriptParser::CallNode *p_call, bool is_await = false); void reduce_cast(GDScriptParser::CastNode *p_cast); void reduce_dictionary(GDScriptParser::DictionaryNode *p_dictionary); void reduce_get_node(GDScriptParser::GetNodeNode *p_get_node); @@ -96,6 +96,7 @@ class GDScriptAnalyzer { bool function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg); bool validate_call_arg(const List &p_par_types, int p_default_args_count, bool p_is_vararg, const GDScriptParser::CallNode *p_call); bool validate_call_arg(const MethodInfo &p_method, const GDScriptParser::CallNode *p_call); + bool is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context); GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid); bool is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false) const; void push_error(const String &p_message, const GDScriptParser::Node *p_origin); diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp index e5d72c31da..86ba16aaec 100644 --- a/modules/gdscript/gdscript_editor.cpp +++ b/modules/gdscript/gdscript_editor.cpp @@ -134,23 +134,25 @@ bool GDScriptLanguage::validate(const String &p_script, int &r_line_error, int & GDScriptAnalyzer analyzer(&parser); Error err = parser.parse(p_script, p_path, false); -#ifdef DEBUG_ENABLED - // FIXME: Warnings. - // if (r_warnings) { - // for (const List::Element *E = parser.get_warnings().front(); E; E = E->next()) { - // const GDScriptWarning &warn = E->get(); - // ScriptLanguage::Warning w; - // w.line = warn.line; - // w.code = (int)warn.code; - // w.string_code = GDScriptWarning::get_name_from_code(warn.code); - // w.message = warn.get_message(); - // r_warnings->push_back(w); - // } - // } -#endif if (err == OK) { err = analyzer.analyze(); } +#ifdef DEBUG_ENABLED + if (r_warnings) { + for (const List::Element *E = parser.get_warnings().front(); E; E = E->next()) { + const GDScriptWarning &warn = E->get(); + ScriptLanguage::Warning w; + w.start_line = warn.start_line; + w.end_line = warn.end_line; + w.leftmost_column = warn.leftmost_column; + w.rightmost_column = warn.rightmost_column; + w.code = (int)warn.code; + w.string_code = GDScriptWarning::get_name_from_code(warn.code); + w.message = warn.get_message(); + r_warnings->push_back(w); + } + } +#endif if (err) { GDScriptParser::ParserError parse_error = parser.get_errors().front()->get(); r_line_error = parse_error.line; diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index dadd97fbbd..f52d3100aa 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -33,6 +33,7 @@ #include "core/io/resource_loader.h" #include "core/math/math_defs.h" #include "core/os/file_access.h" +#include "core/project_settings.h" #include "gdscript.h" #ifdef DEBUG_ENABLED @@ -181,6 +182,61 @@ void GDScriptParser::push_error(const String &p_message, const Node *p_origin) { } } +void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_code, const String &p_symbol1, const String &p_symbol2, const String &p_symbol3, const String &p_symbol4) { + Vector symbols; + if (!p_symbol1.empty()) { + symbols.push_back(p_symbol1); + } + if (!p_symbol2.empty()) { + symbols.push_back(p_symbol2); + } + if (!p_symbol3.empty()) { + symbols.push_back(p_symbol3); + } + if (!p_symbol4.empty()) { + symbols.push_back(p_symbol4); + } + push_warning(p_source, p_code, symbols); +} + +void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_code, const Vector &p_symbols) { + if (is_ignoring_warnings) { + return; + } + if (GLOBAL_GET("debug/gdscript/warnings/exclude_addons").booleanize() && script_path.begins_with("res://addons/")) { + return; + } + + String warn_name = GDScriptWarning::get_name_from_code((GDScriptWarning::Code)p_code).to_lower(); + if (ignored_warnings.has(warn_name)) { + return; + } + if (!GLOBAL_GET("debug/gdscript/warnings/" + warn_name)) { + return; + } + + GDScriptWarning warning; + warning.code = p_code; + warning.symbols = p_symbols; + warning.start_line = p_source->start_line; + warning.end_line = p_source->end_line; + warning.leftmost_column = p_source->leftmost_column; + warning.rightmost_column = p_source->rightmost_column; + + List::Element *before = nullptr; + for (List::Element *E = warnings.front(); E != nullptr; E = E->next()) { + if (E->get().start_line > warning.start_line) { + break; + } + before = E; + } + if (before) { + warnings.insert_after(before, warning); + } else { + warnings.push_front(warning); + } +} + Error GDScriptParser::parse(const String &p_source_code, const String &p_script_path, bool p_for_completion) { clear(); tokenizer.set_source_code(p_source_code); @@ -601,6 +657,7 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_allow_proper if (match(GDScriptTokenizer::Token::EQUAL)) { // Initializer. variable->initializer = parse_expression(false); + variable->assignments++; } if (p_allow_property && match(GDScriptTokenizer::Token::COLON)) { @@ -1106,6 +1163,8 @@ GDScriptParser::SuiteNode *GDScriptParser::parse_suite(const String &p_context, GDScriptParser::Node *GDScriptParser::parse_statement() { Node *result = nullptr; + bool unreachable = current_suite->has_return && !current_suite->has_unreachable_code; + switch (current.type) { case GDScriptTokenizer::Token::PASS: advance(); @@ -1151,6 +1210,9 @@ GDScriptParser::Node *GDScriptParser::parse_statement() { n_return->return_value = parse_expression(false); } result = n_return; + + current_suite->has_return = true; + end_statement("return statement"); break; } @@ -1179,10 +1241,27 @@ GDScriptParser::Node *GDScriptParser::parse_statement() { } end_statement("expression"); result = expression; + + if (expression != nullptr) { + switch (expression->type) { + case Node::CALL: + case Node::ASSIGNMENT: + case Node::AWAIT: + // Fine. + break; + default: + push_warning(expression, GDScriptWarning::STANDALONE_EXPRESSION); + } + } break; } } + if (unreachable) { + current_suite->has_unreachable_code = true; + push_warning(result, GDScriptWarning::UNREACHABLE_CODE, current_function->identifier->name); + } + if (panic_mode) { synchronize(); } @@ -1232,6 +1311,7 @@ GDScriptParser::ContinueNode *GDScriptParser::parse_continue() { if (!can_continue) { push_error(R"(Cannot use "continue" outside of a loop or pattern matching block.)"); } + current_suite->has_continue = true; end_statement(R"("continue")"); return alloc_node(); } @@ -1281,6 +1361,10 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) { n_if->true_block = parse_suite(vformat(R"("%s" block)", p_token)); + if (n_if->true_block->has_continue) { + current_suite->has_continue = true; + } + if (match(GDScriptTokenizer::Token::ELIF)) { IfNode *elif = parse_if("elif"); @@ -1292,6 +1376,13 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) { n_if->false_block = parse_suite(R"("else" block)"); } + if (n_if->false_block != nullptr && n_if->false_block->has_return && n_if->true_block->has_return) { + current_suite->has_return = true; + } + if (n_if->false_block != nullptr && n_if->false_block->has_continue) { + current_suite->has_continue = true; + } + return n_if; } @@ -1310,16 +1401,43 @@ GDScriptParser::MatchNode *GDScriptParser::parse_match() { return match; } + bool all_have_return = true; + bool have_wildcard = false; + bool wildcard_has_return = false; + bool have_wildcard_without_continue = false; + bool have_unreachable_pattern = false; + while (!check(GDScriptTokenizer::Token::DEDENT) && !is_at_end()) { MatchBranchNode *branch = parse_match_branch(); if (branch == nullptr) { continue; } + + if (have_wildcard_without_continue && !have_unreachable_pattern) { + push_warning(branch->patterns[0], GDScriptWarning::UNREACHABLE_PATTERN); + } + + if (branch->has_wildcard) { + have_wildcard = true; + if (branch->block->has_return) { + wildcard_has_return = true; + } + if (!branch->block->has_continue) { + have_wildcard_without_continue = true; + } + } + if (!branch->block->has_return) { + all_have_return = false; + } match->branches.push_back(branch); } consume(GDScriptTokenizer::Token::DEDENT, R"(Expected an indented block after "match" statement.)"); + if (wildcard_has_return || (all_have_return && have_wildcard)) { + current_suite->has_return = true; + } + return match; } @@ -1341,6 +1459,8 @@ GDScriptParser::MatchBranchNode *GDScriptParser::parse_match_branch() { } if (pattern->pattern_type == PatternNode::PT_REST) { push_error(R"(Rest pattern can only be used inside array and dictionary patterns.)"); + } else if (pattern->pattern_type == PatternNode::PT_BIND || pattern->pattern_type == PatternNode::PT_WILDCARD) { + branch->has_wildcard = true; } branch->patterns.push_back(pattern); } while (match(GDScriptTokenizer::Token::COMMA)); @@ -1605,22 +1725,27 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_identifier(ExpressionNode case SuiteNode::Local::CONSTANT: identifier->source = IdentifierNode::LOCAL_CONSTANT; identifier->constant_source = declaration.constant; + declaration.constant->usages++; break; case SuiteNode::Local::VARIABLE: identifier->source = IdentifierNode::LOCAL_VARIABLE; identifier->variable_source = declaration.variable; + declaration.variable->usages++; break; case SuiteNode::Local::PARAMETER: identifier->source = IdentifierNode::FUNCTION_PARAMETER; identifier->parameter_source = declaration.parameter; + declaration.parameter->usages++; break; case SuiteNode::Local::FOR_VARIABLE: identifier->source = IdentifierNode::LOCAL_ITERATOR; identifier->bind_source = declaration.bind; + declaration.bind->usages++; break; case SuiteNode::Local::PATTERN_BIND: identifier->source = IdentifierNode::LOCAL_BIND; identifier->bind_source = declaration.bind; + declaration.bind->usages++; break; case SuiteNode::Local::UNDEFINED: ERR_FAIL_V_MSG(nullptr, "Undefined local found."); @@ -1836,8 +1961,33 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode return parse_expression(false); // Return the following expression. } + VariableNode *source_variable = nullptr; + switch (p_previous_operand->type) { - case Node::IDENTIFIER: + case Node::IDENTIFIER: { + // Get source to store assignment count. + // Also remove one usage since assignment isn't usage. + IdentifierNode *id = static_cast(p_previous_operand); + switch (id->source) { + case IdentifierNode::LOCAL_VARIABLE: + source_variable = id->variable_source; + id->variable_source->usages--; + break; + case IdentifierNode::LOCAL_CONSTANT: + id->constant_source->usages--; + break; + case IdentifierNode::FUNCTION_PARAMETER: + id->parameter_source->usages--; + break; + case IdentifierNode::LOCAL_ITERATOR: + case IdentifierNode::LOCAL_BIND: + id->bind_source->usages--; + break; + default: + break; + } + break; + } case Node::SUBSCRIPT: // Okay. break; @@ -1847,9 +1997,11 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode } AssignmentNode *assignment = alloc_node(); + bool has_operator = true; switch (previous.type) { case GDScriptTokenizer::Token::EQUAL: assignment->operation = AssignmentNode::OP_NONE; + has_operator = false; break; case GDScriptTokenizer::Token::PLUS_EQUAL: assignment->operation = AssignmentNode::OP_ADDITION; @@ -1887,6 +2039,10 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode assignment->assignee = p_previous_operand; assignment->assigned_value = parse_expression(false); + if (has_operator && source_variable != nullptr && source_variable->assignments == 0) { + push_warning(assignment, GDScriptWarning::UNASSIGNED_VARIABLE_OP_ASSIGN, source_variable->identifier->name); + } + return assignment; } diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 23b8fc249c..4e9f750a2d 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -44,6 +44,7 @@ #include "core/vector.h" #include "gdscript_functions.h" #include "gdscript_tokenizer.h" +#include "gdscript_warning.h" #ifdef DEBUG_ENABLED #include "core/string_builder.h" @@ -101,7 +102,7 @@ public: CLASS, // GDScript. VARIANT, // Can be any type. UNRESOLVED, - // TODO: Enum, Signal, Callable + // TODO: Enum }; Kind kind = UNRESOLVED; @@ -366,7 +367,7 @@ public: struct CallNode : public ExpressionNode { ExpressionNode *callee = nullptr; Vector arguments; - StringName function_name; // TODO: Set this. + StringName function_name; bool is_super = false; CallNode() { @@ -388,6 +389,9 @@ public: IdentifierNode *identifier = nullptr; LiteralNode *custom_value = nullptr; int value = 0; + int line = 0; + int leftmost_column = 0; + int rightmost_column = 0; }; IdentifierNode *identifier = nullptr; Vector values; @@ -444,6 +448,28 @@ public: return ""; } + int get_line() const { + switch (type) { + case CLASS: + return m_class->start_line; + case CONSTANT: + return constant->start_line; + case FUNCTION: + return function->start_line; + case VARIABLE: + return variable->start_line; + case ENUM_VALUE: + return enum_value.line; + case ENUM: + return m_enum->start_line; + case SIGNAL: + return signal->start_line; + case UNDEFINED: + ERR_FAIL_V_MSG(-1, "Reaching undefined member type."); + } + ERR_FAIL_V_MSG(-1, "Reaching unhandled type."); + } + DataType get_datatype() const { switch (type) { case CLASS: @@ -462,9 +488,16 @@ public: type.builtin_type = Variant::INT; return type; } + case SIGNAL: { + DataType type; + type.type_source = DataType::ANNOTATED_EXPLICIT; + type.kind = DataType::BUILTIN; + type.builtin_type = Variant::SIGNAL; + // TODO: Add parameter info. + return type; + } case ENUM: - case SIGNAL: - // TODO: Use special datatype kinds for these. + // TODO: Use special datatype kinds for this. return DataType(); case UNDEFINED: return DataType(); @@ -545,6 +578,7 @@ public: ExpressionNode *initializer = nullptr; TypeNode *datatype_specifier = nullptr; bool infer_datatype = false; + int usages = 0; ConstantNode() { type = CONSTANT; @@ -594,6 +628,7 @@ public: bool is_static = false; bool is_coroutine = false; MultiplayerAPI::RPCMode rpc_mode = MultiplayerAPI::RPC_MODE_DISABLED; + MethodInfo info; bool resolved_signature = false; bool resolved_body = false; @@ -634,6 +669,8 @@ public: IdentifierNode *bind_source; }; + int usages = 0; // Useful for binds/iterator variable. + IdentifierNode() { type = IDENTIFIER; } @@ -669,6 +706,7 @@ public: struct MatchBranchNode : public Node { Vector patterns; SuiteNode *block; + bool has_wildcard = false; MatchBranchNode() { type = MATCH_BRANCH; @@ -680,6 +718,7 @@ public: ExpressionNode *default_value = nullptr; TypeNode *datatype_specifier = nullptr; bool infer_datatype = false; + int usages = 0; ParameterNode() { type = PARAMETER; @@ -836,6 +875,10 @@ public: IfNode *parent_if = nullptr; PatternNode *parent_pattern = nullptr; + bool has_return = false; + bool has_continue = false; + bool has_unreachable_code = false; // Just so warnings aren't given more than once per block. + bool has_local(const StringName &p_name) const; const Local &get_local(const StringName &p_name) const; template @@ -916,6 +959,8 @@ public: bool onready = false; PropertyInfo export_info; MultiplayerAPI::RPCMode rpc_mode = MultiplayerAPI::RPC_MODE_DISABLED; + int assignments = 0; + int usages = 0; VariableNode() { type = VARIABLE; @@ -940,11 +985,15 @@ private: bool panic_mode = false; bool can_break = false; bool can_continue = false; + bool is_ignoring_warnings = false; List multiline_stack; ClassNode *head = nullptr; Node *list = nullptr; List errors; + List warnings; + Set ignored_warnings; + Set unsafe_lines; GDScriptTokenizer tokenizer; GDScriptTokenizer::Token previous; @@ -974,8 +1023,6 @@ private: HashMap valid_annotations; List annotation_stack; - Set unsafe_lines; - typedef ExpressionNode *(GDScriptParser::*ParseFunction)(ExpressionNode *p_previous_operand, bool p_can_assign); // Higher value means higher precedence (i.e. is evaluated first). enum Precedence { @@ -1015,6 +1062,8 @@ private: T *alloc_node(); void clear(); void push_error(const String &p_message, const Node *p_origin = nullptr); + void push_warning(const Node *p_source, GDScriptWarning::Code p_code, const String &p_symbol1 = String(), const String &p_symbol2 = String(), const String &p_symbol3 = String(), const String &p_symbol4 = String()); + void push_warning(const Node *p_source, GDScriptWarning::Code p_code, const Vector &p_symbols); GDScriptTokenizer::Token advance(); bool match(GDScriptTokenizer::Token::Type p_token_type); @@ -1104,6 +1153,7 @@ public: static GDScriptFunctions::Function get_builtin_function(const StringName &p_name); const List &get_errors() const { return errors; } + const List &get_warnings() const { return warnings; } const List get_dependencies() const { // TODO: Keep track of deps. return List(); diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index 9da16bb962..105facd9d0 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/modules/gdscript/gdscript_warning.cpp @@ -30,6 +30,8 @@ #include "gdscript_warning.h" +#include "core/variant.h" + #ifdef DEBUG_ENABLED String GDScriptWarning::get_message() const { @@ -48,22 +50,33 @@ String GDScriptWarning::get_message() const { CHECK_SYMBOLS(1); return "The local variable '" + symbols[0] + "' is declared but never used in the block. If this is intended, prefix it with an underscore: '_" + symbols[0] + "'"; } break; - case SHADOWED_VARIABLE: { - CHECK_SYMBOLS(2); - return "The local variable '" + symbols[0] + "' is shadowing an already-defined variable at line " + symbols[1] + "."; + case UNUSED_LOCAL_CONSTANT: { + CHECK_SYMBOLS(1); + return "The local constant '" + symbols[0] + "' is declared but never used in the block. If this is intended, prefix it with an underscore: '_" + symbols[0] + "'"; } break; - case UNUSED_CLASS_VARIABLE: { + case SHADOWED_VARIABLE: { + CHECK_SYMBOLS(4); + return vformat(R"(The local %s "%s" is shadowing an already-declared %s at line %s.)", symbols[0], symbols[1], symbols[2], symbols[3]); + } break; + case SHADOWED_VARIABLE_BASE_CLASS: { + CHECK_SYMBOLS(4); + return vformat(R"(The local %s "%s" is shadowing an already-declared %s at the base class "%s".)", symbols[0], symbols[1], symbols[2], symbols[3]); + } break; + case UNUSED_PRIVATE_CLASS_VARIABLE: { CHECK_SYMBOLS(1); return "The class variable '" + symbols[0] + "' is declared but never used in the script."; } break; - case UNUSED_ARGUMENT: { + case UNUSED_PARAMETER: { CHECK_SYMBOLS(2); - return "The argument '" + symbols[1] + "' is never used in the function '" + symbols[0] + "'. If this is intended, prefix it with an underscore: '_" + symbols[1] + "'"; + return "The parameter '" + symbols[1] + "' is never used in the function '" + symbols[0] + "'. If this is intended, prefix it with an underscore: '_" + symbols[1] + "'"; } break; case UNREACHABLE_CODE: { CHECK_SYMBOLS(1); return "Unreachable code (statement after return) in function '" + symbols[0] + "()'."; } break; + case UNREACHABLE_PATTERN: { + return "Unreachable pattern (pattern after wildcard or bind)."; + } break; case STANDALONE_EXPRESSION: { return "Standalone expression (the line has no effect)."; } break; @@ -74,22 +87,6 @@ String GDScriptWarning::get_message() const { case NARROWING_CONVERSION: { return "Narrowing conversion (float is converted to int and loses precision)."; } break; - case FUNCTION_MAY_YIELD: { - CHECK_SYMBOLS(1); - return "Assigned variable is typed but the function '" + symbols[0] + "()' may yield and return a GDScriptFunctionState instead."; - } break; - case VARIABLE_CONFLICTS_FUNCTION: { - CHECK_SYMBOLS(1); - return "Variable declaration of '" + symbols[0] + "' conflicts with a function of the same name."; - } break; - case FUNCTION_CONFLICTS_VARIABLE: { - CHECK_SYMBOLS(1); - return "Function declaration of '" + symbols[0] + "()' conflicts with a variable of the same name."; - } break; - case FUNCTION_CONFLICTS_CONSTANT: { - CHECK_SYMBOLS(1); - return "Function declaration of '" + symbols[0] + "()' conflicts with a constant of the same name."; - } break; case INCOMPATIBLE_TERNARY: { return "Values of the ternary conditional are not mutually compatible."; } break; @@ -139,6 +136,15 @@ String GDScriptWarning::get_message() const { case STANDALONE_TERNARY: { return "Standalone ternary conditional operator: the return value is being discarded."; } + case ASSERT_ALWAYS_TRUE: { + return "Assert statement is redundant because the expression is always true."; + } + case ASSERT_ALWAYS_FALSE: { + return "Assert statement will raise an error because the expression is always false."; + } + case REDUNDANT_AWAIT: { + return R"("await" keyword not needed in this case, because the expression isn't a coroutine nor a signal.)"; + } case WARNING_MAX: break; // Can't happen, but silences warning } @@ -158,17 +164,16 @@ String GDScriptWarning::get_name_from_code(Code p_code) { "UNASSIGNED_VARIABLE", "UNASSIGNED_VARIABLE_OP_ASSIGN", "UNUSED_VARIABLE", + "UNUSED_LOCAL_CONSTANT", "SHADOWED_VARIABLE", - "UNUSED_CLASS_VARIABLE", - "UNUSED_ARGUMENT", + "SHADOWED_VARIABLE_BASE_CLASS", + "UNUSED_PRIVATE_CLASS_VARIABLE", + "UNUSED_PARAMETER", "UNREACHABLE_CODE", + "UNREACHABLE_PATTERN", "STANDALONE_EXPRESSION", "VOID_ASSIGNMENT", "NARROWING_CONVERSION", - "FUNCTION_MAY_YIELD", - "VARIABLE_CONFLICTS_FUNCTION", - "FUNCTION_CONFLICTS_VARIABLE", - "FUNCTION_CONFLICTS_CONSTANT", "INCOMPATIBLE_TERNARY", "UNUSED_SIGNAL", "RETURN_VALUE_DISCARDED", @@ -182,9 +187,13 @@ String GDScriptWarning::get_name_from_code(Code p_code) { "UNSAFE_CALL_ARGUMENT", "DEPRECATED_KEYWORD", "STANDALONE_TERNARY", - nullptr + "ASSERT_ALWAYS_TRUE", + "ASSERT_ALWAYS_FALSE", + "REDUNDANT_AWAIT", }; + static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names."); + return names[(int)p_code]; } diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index 2b902b2df4..e183d6f302 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/modules/gdscript/gdscript_warning.h @@ -39,38 +39,41 @@ class GDScriptWarning { public: enum Code { - UNASSIGNED_VARIABLE, // Variable used but never assigned - UNASSIGNED_VARIABLE_OP_ASSIGN, // Variable never assigned but used in an assignment operation (+=, *=, etc) - UNUSED_VARIABLE, // Local variable is declared but never used - SHADOWED_VARIABLE, // Variable name shadowed by other variable - UNUSED_CLASS_VARIABLE, // Class variable is declared but never used in the file - UNUSED_ARGUMENT, // Function argument is never used - UNREACHABLE_CODE, // Code after a return statement - STANDALONE_EXPRESSION, // Expression not assigned to a variable - VOID_ASSIGNMENT, // Function returns void but it's assigned to a variable - NARROWING_CONVERSION, // Float value into an integer slot, precision is lost - FUNCTION_MAY_YIELD, // Typed assign of function call that yields (it may return a function state) - VARIABLE_CONFLICTS_FUNCTION, // Variable has the same name of a function - FUNCTION_CONFLICTS_VARIABLE, // Function has the same name of a variable - FUNCTION_CONFLICTS_CONSTANT, // Function has the same name of a constant - INCOMPATIBLE_TERNARY, // Possible values of a ternary if are not mutually compatible - UNUSED_SIGNAL, // Signal is defined but never emitted - RETURN_VALUE_DISCARDED, // Function call returns something but the value isn't used - PROPERTY_USED_AS_FUNCTION, // Function not found, but there's a property with the same name - CONSTANT_USED_AS_FUNCTION, // Function not found, but there's a constant with the same name - FUNCTION_USED_AS_PROPERTY, // Property not found, but there's a function with the same name - INTEGER_DIVISION, // Integer divide by integer, decimal part is discarded - UNSAFE_PROPERTY_ACCESS, // Property not found in the detected type (but can be in subtypes) - UNSAFE_METHOD_ACCESS, // Function not found in the detected type (but can be in subtypes) - UNSAFE_CAST, // Cast used in an unknown type - UNSAFE_CALL_ARGUMENT, // Function call argument is of a supertype of the require argument - DEPRECATED_KEYWORD, // The keyword is deprecated and should be replaced - STANDALONE_TERNARY, // Return value of ternary expression is discarded + UNASSIGNED_VARIABLE, // Variable used but never assigned. + UNASSIGNED_VARIABLE_OP_ASSIGN, // Variable never assigned but used in an assignment operation (+=, *=, etc). + UNUSED_VARIABLE, // Local variable is declared but never used. + UNUSED_LOCAL_CONSTANT, // Local constant is declared but never used. + SHADOWED_VARIABLE, // Variable name shadowed by other variable in same class. + SHADOWED_VARIABLE_BASE_CLASS, // Variable name shadowed by other variable in some base class. + UNUSED_PRIVATE_CLASS_VARIABLE, // Class variable is declared private ("_" prefix) but never used in the file. + UNUSED_PARAMETER, // Function parameter is never used. + UNREACHABLE_CODE, // Code after a return statement. + UNREACHABLE_PATTERN, // Pattern in a match statement after a catch all pattern (wildcard or bind). + STANDALONE_EXPRESSION, // Expression not assigned to a variable. + VOID_ASSIGNMENT, // Function returns void but it's assigned to a variable. + NARROWING_CONVERSION, // Float value into an integer slot, precision is lost. + INCOMPATIBLE_TERNARY, // Possible values of a ternary if are not mutually compatible. + UNUSED_SIGNAL, // Signal is defined but never emitted. + RETURN_VALUE_DISCARDED, // Function call returns something but the value isn't used. + PROPERTY_USED_AS_FUNCTION, // Function not found, but there's a property with the same name. + CONSTANT_USED_AS_FUNCTION, // Function not found, but there's a constant with the same name. + FUNCTION_USED_AS_PROPERTY, // Property not found, but there's a function with the same name. + INTEGER_DIVISION, // Integer divide by integer, decimal part is discarded. + UNSAFE_PROPERTY_ACCESS, // Property not found in the detected type (but can be in subtypes). + UNSAFE_METHOD_ACCESS, // Function not found in the detected type (but can be in subtypes). + UNSAFE_CAST, // Cast used in an unknown type. + UNSAFE_CALL_ARGUMENT, // Function call argument is of a supertype of the require argument. + DEPRECATED_KEYWORD, // The keyword is deprecated and should be replaced. + STANDALONE_TERNARY, // Return value of ternary expression is discarded. + ASSERT_ALWAYS_TRUE, // Expression for assert argument is always true. + ASSERT_ALWAYS_FALSE, // Expression for assert argument is always false. + REDUNDANT_AWAIT, // await is used but expression is synchronous (not a signal nor a coroutine). WARNING_MAX, }; Code code = WARNING_MAX; - int line = -1; + int start_line = -1, end_line = -1; + int leftmost_column = -1, rightmost_column = -1; Vector symbols; String get_name() const;