From 3f52871f70bbdae05047a4da366641dadcbc7d99 Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Thu, 4 Jul 2024 10:23:44 +0300 Subject: [PATCH] GDScript: Add warning if non-`@tool` class extends `@tool` class --- doc/classes/ProjectSettings.xml | 3 +++ modules/gdscript/gdscript_analyzer.cpp | 20 +++++++++++++++++++ modules/gdscript/gdscript_warning.cpp | 3 +++ modules/gdscript/gdscript_warning.h | 2 ++ .../warnings/non_tool_extends_tool.gd | 7 +++++++ .../warnings/non_tool_extends_tool.notest.gd | 1 + .../warnings/non_tool_extends_tool.out | 9 +++++++++ .../warnings/non_tool_extends_tool_ignored.gd | 9 +++++++++ .../non_tool_extends_tool_ignored.out | 1 + 9 files changed, 55 insertions(+) create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.notest.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.out create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.out diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index b1556783d52..41a5eb8dc12 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -533,6 +533,9 @@ When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when dividing an integer by another integer (the decimal part will be discarded). + + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when the base class script has the [code]@tool[/code] annotation, but the current class script does not have it. + When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when passing a floating-point value to a function that expects an integer (it will be converted and lose precision). diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index a6b4bce000d..508f4284ad5 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -411,6 +411,12 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c return err; } +#ifdef DEBUG_ENABLED + if (!parser->_is_tool && ext_parser->get_parser()->_is_tool) { + parser->push_warning(p_class, GDScriptWarning::MISSING_TOOL); + } +#endif + base = ext_parser->get_parser()->head->get_datatype(); } else { if (p_class->extends.is_empty()) { @@ -438,6 +444,13 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c push_error(vformat(R"(Could not resolve super class inheritance from "%s".)", name), id); return err; } + +#ifdef DEBUG_ENABLED + if (!parser->_is_tool && base_parser->get_parser()->_is_tool) { + parser->push_warning(p_class, GDScriptWarning::MISSING_TOOL); + } +#endif + base = base_parser->get_parser()->head->get_datatype(); } } else if (ProjectSettings::get_singleton()->has_autoload(name) && ProjectSettings::get_singleton()->get_autoload(name).is_singleton) { @@ -458,6 +471,13 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c push_error(vformat(R"(Could not resolve super class inheritance from "%s".)", name), id); return err; } + +#ifdef DEBUG_ENABLED + if (!parser->_is_tool && info_parser->get_parser()->_is_tool) { + parser->push_warning(p_class, GDScriptWarning::MISSING_TOOL); + } +#endif + base = info_parser->get_parser()->head->get_datatype(); } else if (class_exists(name)) { if (Engine::get_singleton()->has_singleton(name)) { diff --git a/modules/gdscript/gdscript_warning.cpp b/modules/gdscript/gdscript_warning.cpp index e8fb1d94b35..4ffb4bd9d13 100644 --- a/modules/gdscript/gdscript_warning.cpp +++ b/modules/gdscript/gdscript_warning.cpp @@ -109,6 +109,8 @@ String GDScriptWarning::get_message() const { case STATIC_CALLED_ON_INSTANCE: CHECK_SYMBOLS(2); return vformat(R"*(The function "%s()" is a static function but was called from an instance. Instead, it should be directly called from the type: "%s.%s()".)*", symbols[0], symbols[1], symbols[0]); + case MISSING_TOOL: + return R"(The base class script has the "@tool" annotation, but this script does not have it.)"; case REDUNDANT_STATIC_UNLOAD: return R"(The "@static_unload" annotation is redundant because the file does not have a class with static variables.)"; case REDUNDANT_AWAIT: @@ -219,6 +221,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) { "UNSAFE_VOID_RETURN", "RETURN_VALUE_DISCARDED", "STATIC_CALLED_ON_INSTANCE", + "MISSING_TOOL", "REDUNDANT_STATIC_UNLOAD", "REDUNDANT_AWAIT", "ASSERT_ALWAYS_TRUE", diff --git a/modules/gdscript/gdscript_warning.h b/modules/gdscript/gdscript_warning.h index 1c806bb4e20..ffcf00a8301 100644 --- a/modules/gdscript/gdscript_warning.h +++ b/modules/gdscript/gdscript_warning.h @@ -70,6 +70,7 @@ public: UNSAFE_VOID_RETURN, // Function returns void but returned a call to a function that can't be type checked. RETURN_VALUE_DISCARDED, // Function call returns something but the value isn't used. STATIC_CALLED_ON_INSTANCE, // A static method was called on an instance of a class instead of on the class itself. + MISSING_TOOL, // The base class script has the "@tool" annotation, but this script does not have it. REDUNDANT_STATIC_UNLOAD, // The `@static_unload` annotation is used but the class does not have static data. REDUNDANT_AWAIT, // await is used but expression is synchronous (not a signal nor a coroutine). ASSERT_ALWAYS_TRUE, // Expression for assert argument is always true. @@ -123,6 +124,7 @@ public: WARN, // UNSAFE_VOID_RETURN IGNORE, // RETURN_VALUE_DISCARDED // Too spammy by default on common cases (connect, Tween, etc.). WARN, // STATIC_CALLED_ON_INSTANCE + WARN, // MISSING_TOOL WARN, // REDUNDANT_STATIC_UNLOAD WARN, // REDUNDANT_AWAIT WARN, // ASSERT_ALWAYS_TRUE diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.gd b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.gd new file mode 100644 index 00000000000..95d497c3f3f --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.gd @@ -0,0 +1,7 @@ +extends "./non_tool_extends_tool.notest.gd" + +class InnerClass extends "./non_tool_extends_tool.notest.gd": + pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.notest.gd b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.notest.gd new file mode 100644 index 00000000000..07427846d1e --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.notest.gd @@ -0,0 +1 @@ +@tool diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.out b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.out new file mode 100644 index 00000000000..f65caf52227 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool.out @@ -0,0 +1,9 @@ +GDTEST_OK +>> WARNING +>> Line: 1 +>> MISSING_TOOL +>> The base class script has the "@tool" annotation, but this script does not have it. +>> WARNING +>> Line: 3 +>> MISSING_TOOL +>> The base class script has the "@tool" annotation, but this script does not have it. diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.gd b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.gd new file mode 100644 index 00000000000..a452307d991 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.gd @@ -0,0 +1,9 @@ +@warning_ignore("missing_tool") +extends "./non_tool_extends_tool.notest.gd" + +@warning_ignore("missing_tool") +class InnerClass extends "./non_tool_extends_tool.notest.gd": + pass + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.out b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.out new file mode 100644 index 00000000000..d73c5eb7cde --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/warnings/non_tool_extends_tool_ignored.out @@ -0,0 +1 @@ +GDTEST_OK