From bb9a00889ac9ce6f33adb34a583208e3843c3f92 Mon Sep 17 00:00:00 2001 From: jordi Date: Wed, 23 Mar 2022 12:54:41 -0500 Subject: [PATCH] Add hint for identifiers renamed since Godot 3 --- doc/classes/ProjectSettings.xml | 3 + editor/project_converter_3_to_4.cpp | 32 +++---- editor/project_converter_3_to_4.h | 16 ++++ modules/gdscript/gdscript_analyzer.cpp | 118 ++++++++++++++++++++++++- modules/gdscript/gdscript_analyzer.h | 11 +++ modules/gdscript/gdscript_warning.cpp | 7 ++ modules/gdscript/gdscript_warning.h | 1 + 7 files changed, 172 insertions(+), 16 deletions(-) diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index 84df014fa4e..304400bebe2 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -423,6 +423,9 @@ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a function that is not a coroutine is called with await. + + When enabled, using a property, enum, or function that was renamed since Godot 3 will produce a hint if an error occurs. + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when calling a function without using its return value (by assigning it to a variable or using it as a function argument). Such return values are sometimes used to denote possible errors using the [enum Error] enum. diff --git a/editor/project_converter_3_to_4.cpp b/editor/project_converter_3_to_4.cpp index 727b2c9b4d8..f8d23b2dfcb 100644 --- a/editor/project_converter_3_to_4.cpp +++ b/editor/project_converter_3_to_4.cpp @@ -32,10 +32,11 @@ #include "modules/modules_enabled.gen.h" -const int ERROR_CODE = 77; - +#ifndef DISABLE_DEPRECATED #ifdef MODULE_REGEX_ENABLED +const int ERROR_CODE = 77; + #include "modules/regex/regex.h" #include "core/io/dir_access.h" @@ -44,7 +45,7 @@ const int ERROR_CODE = 77; #include "core/templates/list.h" #include "core/templates/local_vector.h" -static const char *enum_renames[][2] = { +const char *ProjectConverter3To4::enum_renames[][2] = { //// constants { "TYPE_COLOR_ARRAY", "TYPE_PACKED_COLOR_ARRAY" }, { "TYPE_FLOAT64_ARRAY", "TYPE_PACKED_FLOAT64_ARRAY" }, @@ -164,7 +165,7 @@ static const char *enum_renames[][2] = { { nullptr, nullptr }, }; -static const char *gdscript_function_renames[][2] = { +const char *ProjectConverter3To4::gdscript_function_renames[][2] = { // { "_set_name", "get_tracker_name"}, // XRPositionalTracker - CameraFeed use this // { "_unhandled_input", "_unhandled_key_input"}, // BaseButton, ViewportContainer broke Node, FileDialog,SubViewportContainer // { "create_gizmo", "_create_gizmo"}, // EditorNode3DGizmoPlugin - may be used @@ -620,7 +621,7 @@ static const char *gdscript_function_renames[][2] = { }; // gdscript_function_renames clone with CamelCase -static const char *csharp_function_renames[][2] = { +const char *ProjectConverter3To4::csharp_function_renames[][2] = { // { "_SetName", "GetTrackerName"}, // XRPositionalTracker - CameraFeed use this // { "_UnhandledInput", "_UnhandledKeyInput"}, // BaseButton, ViewportContainer broke Node, FileDialog,SubViewportContainer // { "CreateGizmo", "_CreateGizmo"}, // EditorNode3DGizmoPlugin - may be used @@ -1057,7 +1058,7 @@ static const char *csharp_function_renames[][2] = { }; // Some needs to be disabled, because users can use this names as variables -static const char *gdscript_properties_renames[][2] = { +const char *ProjectConverter3To4::gdscript_properties_renames[][2] = { // // { "d", "distance" }, //WorldMarginShape2D - TODO, looks that polish letters ą ę are treaten as space, not as letter, so `będą` are renamed to `będistanceą` // // {"alt","alt_pressed"}, // This may broke a lot of comments and user variables // // {"command","command_pressed"},// This may broke a lot of comments and user variables @@ -1173,7 +1174,7 @@ static const char *gdscript_properties_renames[][2] = { }; // Some needs to be disabled, because users can use this names as variables -static const char *csharp_properties_renames[][2] = { +const char *ProjectConverter3To4::csharp_properties_renames[][2] = { // // { "D", "Distance" }, //WorldMarginShape2D - TODO, looks that polish letters ą ę are treaten as space, not as letter, so `będą` are renamed to `będistanceą` // // {"Alt","AltPressed"}, // This may broke a lot of comments and user variables // // {"Command","CommandPressed"},// This may broke a lot of comments and user variables @@ -1278,7 +1279,7 @@ static const char *csharp_properties_renames[][2] = { { nullptr, nullptr }, }; -static const char *gdscript_signals_renames[][2] = { +const char *ProjectConverter3To4::gdscript_signals_renames[][2] = { // {"instantiate","instance"}, // FileSystemDock // { "hide", "hidden" }, // CanvasItem - function with same name exists // { "tween_all_completed","loop_finished"}, // Tween - TODO, not sure @@ -1302,7 +1303,7 @@ static const char *gdscript_signals_renames[][2] = { { nullptr, nullptr }, }; -static const char *csharp_signals_renames[][2] = { +const char *ProjectConverter3To4::csharp_signals_renames[][2] = { // {"Instantiate","Instance"}, // FileSystemDock // { "Hide", "Hidden" }, // CanvasItem - function with same name exists // { "TweenAllCompleted","LoopFinished"}, // Tween - TODO, not sure @@ -1326,7 +1327,7 @@ static const char *csharp_signals_renames[][2] = { }; -static const char *project_settings_renames[][2] = { +const char *ProjectConverter3To4::project_settings_renames[][2] = { { "audio/channel_disable_threshold_db", "audio/buses/channel_disable_threshold_db" }, { "audio/channel_disable_time", "audio/buses/channel_disable_time" }, { "audio/default_bus_layout", "audio/buses/default_bus_layout" }, @@ -1370,7 +1371,7 @@ static const char *project_settings_renames[][2] = { { nullptr, nullptr }, }; -static const char *input_map_renames[][2] = { +const char *ProjectConverter3To4::input_map_renames[][2] = { { ",\"alt\":", ",\"alt_pressed\":" }, { ",\"shift\":", ",\"shift_pressed\":" }, { ",\"control\":", ",\"ctrl_pressed\":" }, @@ -1382,7 +1383,7 @@ static const char *input_map_renames[][2] = { { nullptr, nullptr }, }; -static const char *builtin_types_renames[][2] = { +const char *ProjectConverter3To4::builtin_types_renames[][2] = { { "PoolByteArray", "PackedByteArray" }, { "PoolColorArray", "PackedColorArray" }, { "PoolIntArray", "PackedInt32Array" }, @@ -1396,7 +1397,7 @@ static const char *builtin_types_renames[][2] = { { nullptr, nullptr }, }; -static const char *shaders_renames[][2] = { +const char *ProjectConverter3To4::shaders_renames[][2] = { { "ALPHA_SCISSOR", "ALPHA_SCISSOR_THRESHOLD" }, { "CAMERA_MATRIX", "INV_VIEW_MATRIX" }, { "INV_CAMERA_MATRIX", "VIEW_MATRIX" }, @@ -1414,7 +1415,7 @@ static const char *shaders_renames[][2] = { { nullptr, nullptr }, }; -static const char *class_renames[][2] = { +const char *ProjectConverter3To4::class_renames[][2] = { // { "BulletPhysicsDirectBodyState", "BulletPhysicsDirectBodyState3D" }, // Class is not visible in ClassDB // { "BulletPhysicsServer", "BulletPhysicsServer3D" }, // Class is not visible in ClassDB // { "GDScriptFunctionState", "Node3D" }, // TODO - not sure to which should be changed @@ -1639,7 +1640,7 @@ static const char *class_renames[][2] = { { nullptr, nullptr }, }; -static const char *color_renames[][2] = { +const char *ProjectConverter3To4::ProjectConverter3To4::color_renames[][2] = { { "aliceblue", "ALICE_BLUE" }, { "antiquewhite", "ANTIQUE_WHITE" }, { "aqua", "AQUA" }, @@ -4348,3 +4349,4 @@ int ProjectConverter3To4::validate_conversion() { } #endif // MODULE_REGEX_ENABLED +#endif // DISABLE_DEPRECATED diff --git a/editor/project_converter_3_to_4.h b/editor/project_converter_3_to_4.h index b3aa52f1e35..6ec2dd188d8 100644 --- a/editor/project_converter_3_to_4.h +++ b/editor/project_converter_3_to_4.h @@ -29,6 +29,7 @@ /**************************************************************************/ #ifndef PROJECT_CONVERTER_3_TO_4_H +#ifndef DISABLE_DEPRECATED #define PROJECT_CONVERTER_3_TO_4_H #include "core/io/file_access.h" @@ -41,6 +42,19 @@ class RegEx; class ProjectConverter3To4 { public: class RegExContainer; + static const char *enum_renames[][2]; + static const char *gdscript_function_renames[][2]; + static const char *csharp_function_renames[][2]; + static const char *gdscript_properties_renames[][2]; + static const char *csharp_properties_renames[][2]; + static const char *gdscript_signals_renames[][2]; + static const char *csharp_signals_renames[][2]; + static const char *project_settings_renames[][2]; + static const char *input_map_renames[][2]; + static const char *builtin_types_renames[][2]; + static const char *shaders_renames[][2]; + static const char *class_renames[][2]; + static const char *color_renames[][2]; private: uint64_t maximum_file_size; @@ -97,4 +111,6 @@ public: int convert(); }; +#endif // DISABLE_DEPRECATED + #endif // PROJECT_CONVERTER_3_TO_4_H diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index e04a962dcb0..b1ba6d2f330 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -2364,6 +2364,62 @@ void GDScriptAnalyzer::reduce_binary_op(GDScriptParser::BinaryOpNode *p_binary_o p_binary_op->set_datatype(result); } +#ifdef TOOLS_ENABLED +#ifndef DISABLE_DEPRECATED +const char *GDScriptAnalyzer::get_rename_from_map(const char *map[][2], String key) { + for (int index = 0; map[index][0]; index++) { + if (map[index][0] == key) { + return map[index][1]; + } + } + return nullptr; +} + +// Checks if an identifier/function name has been renamed in Godot 4, uses ProjectConverter3To4 for rename map. +// Returns the new name if found, nullptr otherwise. +const char *GDScriptAnalyzer::check_for_renamed_identifier(String identifier, GDScriptParser::Node::Type type) { + switch (type) { + case GDScriptParser::Node::IDENTIFIER: { + // Check properties + const char *result = get_rename_from_map(ProjectConverter3To4::gdscript_properties_renames, identifier); + if (result) { + return result; + } + // Check enum values + result = get_rename_from_map(ProjectConverter3To4::enum_renames, identifier); + if (result) { + return result; + } + // Check color constants + result = get_rename_from_map(ProjectConverter3To4::color_renames, identifier); + if (result) { + return result; + } + // Check type names + result = get_rename_from_map(ProjectConverter3To4::class_renames, identifier); + if (result) { + return result; + } + return get_rename_from_map(ProjectConverter3To4::builtin_types_renames, identifier); + } + case GDScriptParser::Node::CALL: { + const char *result = get_rename_from_map(ProjectConverter3To4::gdscript_function_renames, identifier); + if (result) { + return result; + } + // Built-in Types are mistaken for function calls when the built-in type is not found. + // Check built-in types if function rename not found + return get_rename_from_map(ProjectConverter3To4::builtin_types_renames, identifier); + } + // Signal references don't get parsed through the GDScriptAnalyzer. No support for signal rename hints. + default: + // No rename found, return null + return nullptr; + } +} +#endif // DISABLE_DEPRECATED +#endif // TOOLS_ENABLED + void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_await, bool p_is_root) { bool all_is_constant = true; HashMap arrays; // For array literal to potentially type when passing. @@ -2776,7 +2832,22 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a } if (!found && (is_self || (base_type.is_hard_type() && base_type.kind == GDScriptParser::DataType::BUILTIN))) { String base_name = is_self && !p_call->is_super ? "self" : base_type.to_string(); +#ifdef TOOLS_ENABLED +#ifndef DISABLE_DEPRECATED + String rename_hint = String(); + if (GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(GDScriptWarning::Code::RENAMED_IN_GD4_HINT)).booleanize()) { + const char *renamed_function_name = check_for_renamed_identifier(p_call->function_name, p_call->type); + if (renamed_function_name) { + rename_hint = " " + vformat(R"(Did you mean to use "%s"?)", String(renamed_function_name) + "()"); + } + } + push_error(vformat(R"*(Function "%s()" not found in base %s.%s)*", p_call->function_name, base_name, rename_hint), p_call->is_super ? p_call : p_call->callee); +#else // !DISABLE_DEPRECATED 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); +#endif // DISABLE_DEPRECATED +#else + 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); +#endif } else if (!found && (!p_call->is_super && base_type.is_hard_type() && base_type.kind == GDScriptParser::DataType::NATIVE && base_type.is_meta_type)) { push_error(vformat(R"*(Static function "%s()" not found in base "%s".)*", p_call->function_name, base_type.native_type), p_call); } @@ -2988,7 +3059,22 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod p_identifier->reduced_value = result; p_identifier->set_datatype(type_from_variant(result, p_identifier)); } else if (base.is_hard_type()) { - push_error(vformat(R"(Cannot find constant "%s" on type "%s".)", name, base.to_string()), p_identifier); +#ifdef TOOLS_ENABLED +#ifndef DISABLE_DEPRECATED + String rename_hint = String(); + if (GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(GDScriptWarning::Code::RENAMED_IN_GD4_HINT)).booleanize()) { + const char *renamed_identifier_name = check_for_renamed_identifier(name, p_identifier->type); + if (renamed_identifier_name) { + rename_hint = " " + vformat(R"(Did you mean to use "%s"?)", renamed_identifier_name); + } + } + push_error(vformat(R"(Cannot find constant "%s" on base "%s".%s)", name, base.to_string(), rename_hint), p_identifier); +#else // !DISABLE_DEPRECATED + push_error(vformat(R"(Cannot find constant "%s" on base "%s".)", name, base.to_string()), p_identifier); +#endif // DISABLE_DEPRECATED +#else + push_error(vformat(R"(Cannot find constant "%s" on base "%s".)", name, base.to_string()), p_identifier); +#endif } } else { switch (base.builtin_type) { @@ -3017,7 +3103,22 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod } } if (base.is_hard_type()) { +#ifdef TOOLS_ENABLED +#ifndef DISABLE_DEPRECATED + String rename_hint = String(); + if (GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(GDScriptWarning::Code::RENAMED_IN_GD4_HINT)).booleanize()) { + const char *renamed_identifier_name = check_for_renamed_identifier(name, p_identifier->type); + if (renamed_identifier_name) { + rename_hint = " " + vformat(R"(Did you mean to use "%s"?)", renamed_identifier_name); + } + } + push_error(vformat(R"(Cannot find property "%s" on base "%s".%s)", name, base.to_string(), rename_hint), p_identifier); +#else // !DISABLE_DEPRECATED push_error(vformat(R"(Cannot find property "%s" on base "%s".)", name, base.to_string()), p_identifier); +#endif // DISABLE_DEPRECATED +#else + push_error(vformat(R"(Cannot find property "%s" on base "%s".)", name, base.to_string()), p_identifier); +#endif } } } @@ -3356,7 +3457,22 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident if (GDScriptUtilityFunctions::function_exists(name)) { push_error(vformat(R"(Built-in function "%s" cannot be used as an identifier.)", name), p_identifier); } else { +#ifdef TOOLS_ENABLED +#ifndef DISABLE_DEPRECATED + String rename_hint = String(); + if (GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(GDScriptWarning::Code::RENAMED_IN_GD4_HINT)).booleanize()) { + const char *renamed_identifier_name = check_for_renamed_identifier(name, p_identifier->type); + if (renamed_identifier_name) { + rename_hint = " " + vformat(R"(Did you mean to use "%s"?)", renamed_identifier_name); + } + } + push_error(vformat(R"(Identifier "%s" not declared in the current scope.%s)", name, rename_hint), p_identifier); +#else // !DISABLE_DEPRECATED push_error(vformat(R"(Identifier "%s" not declared in the current scope.)", name), p_identifier); +#endif // DISABLE_DEPRECATED +#else + push_error(vformat(R"(Identifier "%s" not declared in the current scope.)", name), p_identifier); +#endif } GDScriptParser::DataType dummy; dummy.kind = GDScriptParser::DataType::VARIANT; diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index b22d47982fa..017c87c141a 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -37,6 +37,10 @@ #include "gdscript_cache.h" #include "gdscript_parser.h" +#ifdef TOOLS_ENABLED +#include "editor/project_converter_3_to_4.h" +#endif + class GDScriptAnalyzer { GDScriptParser *parser = nullptr; HashMap> depended_parsers; @@ -129,6 +133,13 @@ class GDScriptAnalyzer { bool is_shadowing(GDScriptParser::IdentifierNode *p_local, const String &p_context); #endif +#ifdef TOOLS_ENABLED +#ifndef DISABLE_DEPRECATED + const char *get_rename_from_map(const char *map[][2], String key); + const char *check_for_renamed_identifier(String identifier, GDScriptParser::Node::Type type); +#endif // DISABLE_DEPRECATED +#endif // TOOLS_ENABLED + public: Error resolve_inheritance(); Error resolve_interface(); diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index a6cbb7f6aec..ec2ea9ba4d8 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/modules/gdscript/gdscript_warning.cpp @@ -159,6 +159,9 @@ String GDScriptWarning::get_message() const { CHECK_SYMBOLS(1); return vformat(R"(The identifier "%s" has misleading characters and might be confused with something else.)", symbols[0]); } + case RENAMED_IN_GD4_HINT: { + break; // Renamed identifier hint is taken care of by the GDScriptAnalyzer. No message needed here. + } case WARNING_MAX: break; // Can't happen, but silences warning } @@ -180,6 +183,9 @@ int GDScriptWarning::get_default_value(Code p_code) { PropertyInfo GDScriptWarning::get_property_info(Code p_code) { // Making this a separate function in case a warning needs different PropertyInfo in the future. + if (p_code == Code::RENAMED_IN_GD4_HINT) { + return PropertyInfo(Variant::BOOL, get_settings_path_from_code(p_code)); + } return PropertyInfo(Variant::INT, get_settings_path_from_code(p_code), PROPERTY_HINT_ENUM, "Ignore,Warn,Error"); } @@ -224,6 +230,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) { "INT_ASSIGNED_TO_ENUM", "STATIC_CALLED_ON_INSTANCE", "CONFUSABLE_IDENTIFIER", + "RENAMED_IN_GODOT_4_HINT" }; static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names."); diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index b485f02b9c5..96db3ed19c0 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/modules/gdscript/gdscript_warning.h @@ -79,6 +79,7 @@ public: INT_ASSIGNED_TO_ENUM, // An integer value was assigned to an enum-typed variable without casting. STATIC_CALLED_ON_INSTANCE, // A static method was called on an instance of a class instead of on the class itself. CONFUSABLE_IDENTIFIER, // The identifier contains misleading characters that can be confused. E.g. "usеr" (has Cyrillic "е" instead of Latin "e"). + RENAMED_IN_GD4_HINT, // A variable or function that could not be found has been renamed in Godot 4 WARNING_MAX, };