Merge pull request #93889 from dalexeev/gds-exclude-addons-exceptions-setting

GDScript: Add `debug/gdscript/warnings/directory_rules` project setting

Author: Repiteo

doc/classes/ProjectSettings.xml

@@ -131,6 +131,9 @@ void PluginConfigDialog::_on_required_text_changed() {
 	if ((!script_edit->get_text().get_extension().is_empty() && script_edit->get_text().get_extension() != ext) || script_edit->get_text().ends_with(".")) {
 		validation_panel->set_message(MSG_ID_SCRIPT, vformat(TTR("Script extension must match chosen language extension (.%s)."), ext), EditorValidationPanel::MSG_ERROR);
 	}
+	if (language->get_name() == "GDScript") {
+		validation_panel->set_message(MSG_ID_ENABLE_WARNINGS, TTR("Consider enabling GDScript warnings for this plugin by adding an entry for it to the project setting Debug > GDScript > Warnings > Directory Rules."), EditorValidationPanel::MSG_INFO);
+	}
 }
 
 String PluginConfigDialog::_get_subfolder() {
@@ -317,6 +320,7 @@ PluginConfigDialog::PluginConfigDialog() {
 	validation_panel->add_line(MSG_ID_SCRIPT, TTR("Script extension is valid."));
 	validation_panel->add_line(MSG_ID_SUBFOLDER, TTR("Subfolder name is valid."));
 	validation_panel->add_line(MSG_ID_ACTIVE, "");
+	validation_panel->add_line(MSG_ID_ENABLE_WARNINGS, "");
 	validation_panel->set_update_callback(callable_mp(this, &PluginConfigDialog::_on_required_text_changed));
 	validation_panel->set_accept_button(get_ok_button());
 

editor/plugins/plugin_config_dialog.cpp

@@ -47,6 +47,7 @@ class PluginConfigDialog : public ConfirmationDialog {
 		MSG_ID_SUBFOLDER,
 		MSG_ID_SCRIPT,
 		MSG_ID_ACTIVE,
+		MSG_ID_ENABLE_WARNINGS,
 	};
 
 	LineEdit *name_edit = nullptr;

editor/plugins/plugin_config_dialog.h

@@ -2282,11 +2282,18 @@ void GDScriptLanguage::init() {
 		GDExtensionManager::get_singleton()->connect("extension_loaded", callable_mp(this, &GDScriptLanguage::_extension_loaded));
 		GDExtensionManager::get_singleton()->connect("extension_unloading", callable_mp(this, &GDScriptLanguage::_extension_unloading));
 	}
-#endif
+#endif // TOOLS_ENABLED
+
+#ifdef DEBUG_ENABLED
+	GDScriptParser::update_project_settings();
+	if (!ProjectSettings::get_singleton()->is_connected("settings_changed", callable_mp_static(&GDScriptParser::update_project_settings))) {
+		ProjectSettings::get_singleton()->connect("settings_changed", callable_mp_static(&GDScriptParser::update_project_settings));
+	}
+#endif // DEBUG_ENABLED
 
 #ifdef TESTS_ENABLED
 	GDScriptTests::GDScriptTestRunner::handle_cmdline();
-#endif
+#endif // TESTS_ENABLED
 }
 
 #ifdef TOOLS_ENABLED
@@ -2950,7 +2957,7 @@ GDScriptLanguage::GDScriptLanguage() {
 	profiling = false;
 	profile_native_calls = false;
 	script_frame_time = 0;
-#endif
+#endif // DEBUG_ENABLED
 
 	_debug_max_call_stack = GLOBAL_DEF_RST(PropertyInfo(Variant::INT, "debug/settings/gdscript/max_call_stack", PROPERTY_HINT_RANGE, "512," + itos(GDScriptFunction::MAX_CALL_DEPTH - 1) + ",1"), 1024);
 	track_call_stack = GLOBAL_DEF_RST("debug/settings/gdscript/always_track_call_stacks", false);
@@ -2961,20 +2968,29 @@ GDScriptLanguage::GDScriptLanguage() {
 	track_locals = track_locals || EngineDebugger::is_active();
 
 	GLOBAL_DEF("debug/gdscript/warnings/enable", true);
-	GLOBAL_DEF("debug/gdscript/warnings/exclude_addons", true);
-	GLOBAL_DEF("debug/gdscript/warnings/renamed_in_godot_4_hint", true);
+
+	GLOBAL_DEF(PropertyInfo(Variant::DICTIONARY,
+					   "debug/gdscript/warnings/directory_rules",
+					   PROPERTY_HINT_TYPE_STRING,
+					   vformat("%d/%d:;%d/%d:Exclude,Include", Variant::STRING, PROPERTY_HINT_DIR, Variant::INT, PROPERTY_HINT_ENUM)),
+			Dictionary({ { "res://addons", GDScriptParser::WarningDirectoryRule::DECISION_EXCLUDE } }));
+
 	for (int i = 0; i < (int)GDScriptWarning::WARNING_MAX; i++) {
-		GDScriptWarning::Code code = (GDScriptWarning::Code)i;
-		Variant default_enabled = GDScriptWarning::get_default_value(code);
-		String path = GDScriptWarning::get_settings_path_from_code(code);
-		GLOBAL_DEF(GDScriptWarning::get_property_info(code), default_enabled);
-	}
+		const GDScriptWarning::Code code = (GDScriptWarning::Code)i;
+		const Variant default_value = GDScriptWarning::get_default_value(code);
+		GLOBAL_DEF(GDScriptWarning::get_property_info(code), default_value);
 
 #ifndef DISABLE_DEPRECATED
-	ProjectSettings::get_singleton()->set_as_internal("debug/gdscript/warnings/property_used_as_function", true);
-	ProjectSettings::get_singleton()->set_as_internal("debug/gdscript/warnings/constant_used_as_function", true);
-	ProjectSettings::get_singleton()->set_as_internal("debug/gdscript/warnings/function_used_as_property", true);
-#endif
+		if (i >= GDScriptWarning::FIRST_DEPRECATED_WARNING) {
+			const String setting_path = GDScriptWarning::get_setting_path_from_code(code);
+			ProjectSettings::get_singleton()->set_as_internal(setting_path, true);
+		}
+#endif // DISABLE_DEPRECATED
+	}
+
+	// TODO: This setting has nothing to do with warnings. It should be moved at the next compatibility breakage,
+	// if the setting is still relevant at that time.
+	GLOBAL_DEF("debug/gdscript/warnings/renamed_in_godot_4_hint", true);
 #endif // DEBUG_ENABLED
 }
 

modules/gdscript/gdscript.cpp

@@ -1010,7 +1010,7 @@ static void _find_annotation_arguments(const GDScriptParser::AnnotationNode *p_a
 			if (warning_code >= GDScriptWarning::FIRST_DEPRECATED_WARNING) {
 				break; // Don't suggest deprecated warnings as they are never produced.
 			}
-#endif
+#endif // DISABLE_DEPRECATED
 			ScriptLanguage::CodeCompletionOption warning(GDScriptWarning::get_name_from_code((GDScriptWarning::Code)warning_code).to_lower(), ScriptLanguage::CODE_COMPLETION_KIND_PLAIN_TEXT);
 			warning.insert_text = warning.display.quote(p_quote_style);
 			r_result.insert(warning.display, warning);

modules/gdscript/gdscript_editor.cpp

@@ -68,9 +68,15 @@ Variant::Type GDScriptParser::get_builtin_type(const StringName &p_type) {
 	return Variant::VARIANT_MAX;
 }
 
+#ifdef DEBUG_ENABLED
+bool GDScriptParser::is_project_ignoring_warnings = false;
+GDScriptWarning::WarnLevel GDScriptParser::warning_levels[GDScriptWarning::WARNING_MAX];
+LocalVector<GDScriptParser::WarningDirectoryRule> GDScriptParser::warning_directory_rules;
+#endif // DEBUG_ENABLED
+
 #ifdef TOOLS_ENABLED
 HashMap<String, String> GDScriptParser::theme_color_names;
-#endif
+#endif // TOOLS_ENABLED
 
 HashMap<StringName, GDScriptParser::AnnotationInfo> GDScriptParser::valid_annotations;
 
@@ -89,6 +95,53 @@ bool GDScriptParser::annotation_exists(const String &p_annotation_name) const {
 	return valid_annotations.has(p_annotation_name);
 }
 
+#ifdef DEBUG_ENABLED
+void GDScriptParser::update_project_settings() {
+	is_project_ignoring_warnings = !GLOBAL_GET("debug/gdscript/warnings/enable").booleanize();
+
+	for (int i = 0; i < GDScriptWarning::WARNING_MAX; i++) {
+		const String setting_path = GDScriptWarning::get_setting_path_from_code((GDScriptWarning::Code)i);
+		warning_levels[i] = (GDScriptWarning::WarnLevel)(int)GLOBAL_GET(setting_path);
+	}
+
+#ifndef DISABLE_DEPRECATED
+	// We do not use `GLOBAL_GET`, since we check without taking overrides into account. We leave the migration of non-trivial configurations to the user.
+	if (unlikely(ProjectSettings::get_singleton()->has_setting("debug/gdscript/warnings/exclude_addons"))) {
+		const bool is_excluding_addons = ProjectSettings::get_singleton()->get_setting("debug/gdscript/warnings/exclude_addons", true).booleanize();
+		ProjectSettings::get_singleton()->clear("debug/gdscript/warnings/exclude_addons");
+
+		Dictionary rules = ProjectSettings::get_singleton()->get_setting("debug/gdscript/warnings/directory_rules");
+		rules["res://addons"] = is_excluding_addons ? WarningDirectoryRule::DECISION_EXCLUDE : WarningDirectoryRule::DECISION_INCLUDE;
+		ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/directory_rules", rules);
+	}
+#endif // DISABLE_DEPRECATED
+
+	warning_directory_rules.clear();
+
+	const Dictionary rules = GLOBAL_GET("debug/gdscript/warnings/directory_rules");
+	for (const KeyValue<Variant, Variant> &kv : rules) {
+		String dir = kv.key.operator String().simplify_path();
+		ERR_CONTINUE_MSG(!dir.begins_with("res://"), R"(Paths in the project setting "debug/gdscript/warnings/directory_rules" keys must start with the "res://" prefix.)");
+		if (!dir.ends_with("/")) {
+			dir += '/';
+		}
+
+		const int decision = kv.value;
+		ERR_CONTINUE(decision < 0 || decision >= WarningDirectoryRule::DECISION_MAX);
+
+		warning_directory_rules.push_back({ dir, (WarningDirectoryRule::Decision)decision });
+	}
+
+	struct RuleSort {
+		bool operator()(const WarningDirectoryRule &p_a, const WarningDirectoryRule &p_b) const {
+			return p_a.directory_path.count("/") > p_b.directory_path.count("/");
+		}
+	};
+
+	warning_directory_rules.sort_custom<RuleSort>();
+}
+#endif // DEBUG_ENABLED
+
 GDScriptParser::GDScriptParser() {
 	// Register valid annotations.
 	if (unlikely(valid_annotations.is_empty())) {
@@ -137,11 +190,10 @@ GDScriptParser::GDScriptParser() {
 	}
 
 #ifdef DEBUG_ENABLED
-	is_ignoring_warnings = !(bool)GLOBAL_GET("debug/gdscript/warnings/enable");
 	for (int i = 0; i < GDScriptWarning::WARNING_MAX; i++) {
 		warning_ignore_start_lines[i] = INT_MAX;
 	}
-#endif
+#endif // DEBUG_ENABLED
 
 #ifdef TOOLS_ENABLED
 	if (unlikely(theme_color_names.is_empty())) {
@@ -161,7 +213,7 @@ GDScriptParser::GDScriptParser() {
 		theme_color_names.insert("a", "axis_w_color");
 		theme_color_names.insert("a8", "axis_w_color");
 	}
-#endif
+#endif // TOOLS_ENABLED
 }
 
 GDScriptParser::~GDScriptParser() {
@@ -195,13 +247,11 @@ void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_
 	ERR_FAIL_NULL(p_source);
 	ERR_FAIL_INDEX(p_code, GDScriptWarning::WARNING_MAX);
 
-	if (is_ignoring_warnings) {
-		return;
-	}
-	if (GLOBAL_GET_CACHED(bool, "debug/gdscript/warnings/exclude_addons") && script_path.begins_with("res://addons/")) {
+	if (is_project_ignoring_warnings || is_script_ignoring_warnings) {
 		return;
 	}
-	GDScriptWarning::WarnLevel warn_level = (GDScriptWarning::WarnLevel)(int)GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(p_code));
+
+	const GDScriptWarning::WarnLevel warn_level = warning_levels[p_code];
 	if (warn_level == GDScriptWarning::IGNORE) {
 		return;
 	}
@@ -251,6 +301,24 @@ void GDScriptParser::apply_pending_warnings() {
 
 	pending_warnings.clear();
 }
+
+void GDScriptParser::evaluate_warning_directory_rules_for_script_path() {
+	is_script_ignoring_warnings = false;
+	for (const WarningDirectoryRule &rule : warning_directory_rules) {
+		if (script_path.begins_with(rule.directory_path)) {
+			switch (rule.decision) {
+				case WarningDirectoryRule::DECISION_EXCLUDE:
+					is_script_ignoring_warnings = true;
+					return; // Stop checking rules.
+				case WarningDirectoryRule::DECISION_INCLUDE:
+					is_script_ignoring_warnings = false;
+					return; // Stop checking rules.
+				case WarningDirectoryRule::DECISION_MAX:
+					return; // Unreachable.
+			}
+		}
+	}
+}
 #endif // DEBUG_ENABLED
 
 void GDScriptParser::override_completion_context(const Node *p_for_node, CompletionType p_type, Node *p_node, int p_argument) {
@@ -391,9 +459,14 @@ Error GDScriptParser::parse(const String &p_source_code, const String &p_script_
 	text_tokenizer->set_source_code(source);
 
 	tokenizer = text_tokenizer;
-
 	tokenizer->set_cursor_position(cursor_line, cursor_column);
+
 	script_path = p_script_path.simplify_path();
+
+#ifdef DEBUG_ENABLED
+	evaluate_warning_directory_rules_for_script_path();
+#endif // DEBUG_ENABLED
+
 	current = tokenizer->scan();
 	// Avoid error or newline as the first token.
 	// The latter can mess with the parser when opening files filled exclusively with comments and newlines.
@@ -414,15 +487,15 @@ Error GDScriptParser::parse(const String &p_source_code, const String &p_script_
 		nd->end_line = 1;
 		push_warning(nd, GDScriptWarning::EMPTY_FILE);
 	}
-#endif
+#endif // DEBUG_ENABLED
 
 	push_multiline(false); // Keep one for the whole parsing.
 	parse_program();
 	pop_multiline();
 
 #ifdef TOOLS_ENABLED
 	comment_data = tokenizer->get_comments();
-#endif
+#endif // TOOLS_ENABLED
 
 	memdelete(text_tokenizer);
 	tokenizer = nullptr;
@@ -431,7 +504,7 @@ Error GDScriptParser::parse(const String &p_source_code, const String &p_script_
 	if (multiline_stack.size() > 0) {
 		ERR_PRINT("Parser bug: Imbalanced multiline stack.");
 	}
-#endif
+#endif // DEBUG_ENABLED
 
 	if (errors.is_empty()) {
 		return OK;
@@ -450,7 +523,13 @@ Error GDScriptParser::parse_binary(const Vector<uint8_t> &p_binary, const String
 	}
 
 	tokenizer = buffer_tokenizer;
+
 	script_path = p_script_path.simplify_path();
+
+#ifdef DEBUG_ENABLED
+	evaluate_warning_directory_rules_for_script_path();
+#endif // DEBUG_ENABLED
+
 	current = tokenizer->scan();
 	// Avoid error or newline as the first token.
 	// The latter can mess with the parser when opening files filled exclusively with comments and newlines.
@@ -4997,10 +5076,6 @@ bool GDScriptParser::export_group_annotations(AnnotationNode *p_annotation, Node
 
 bool GDScriptParser::warning_ignore_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
 #ifdef DEBUG_ENABLED
-	if (is_ignoring_warnings) {
-		return true; // We already ignore all warnings, let's optimize it.
-	}
-
 	bool has_error = false;
 	for (const Variant &warning_name : p_annotation->resolved_arguments) {
 		GDScriptWarning::Code warning_code = GDScriptWarning::get_code_from_name(String(warning_name).to_upper());

modules/gdscript/gdscript_parser.cpp

@@ -1349,20 +1349,37 @@ class GDScriptParser {
 	List<ParserError> errors;
 
 #ifdef DEBUG_ENABLED
+public:
+	struct WarningDirectoryRule {
+		enum Decision {
+			DECISION_EXCLUDE,
+			DECISION_INCLUDE,
+			DECISION_MAX,
+		};
+
+		String directory_path; // With a trailing slash.
+		Decision decision = DECISION_EXCLUDE;
+	};
+
+private:
 	struct PendingWarning {
 		const Node *source = nullptr;
 		GDScriptWarning::Code code = GDScriptWarning::WARNING_MAX;
 		bool treated_as_error = false;
 		Vector<String> symbols;
 	};
 
-	bool is_ignoring_warnings = false;
+	static bool is_project_ignoring_warnings;
+	static GDScriptWarning::WarnLevel warning_levels[GDScriptWarning::WARNING_MAX];
+	static LocalVector<WarningDirectoryRule> warning_directory_rules;
+
 	List<GDScriptWarning> warnings;
 	List<PendingWarning> pending_warnings;
+	bool is_script_ignoring_warnings = false;
 	HashSet<int> warning_ignored_lines[GDScriptWarning::WARNING_MAX];
 	int warning_ignore_start_lines[GDScriptWarning::WARNING_MAX];
 	HashSet<int> unsafe_lines;
-#endif
+#endif // DEBUG_ENABLED
 
 	GDScriptTokenizer *tokenizer = nullptr;
 	GDScriptTokenizer::Token previous;
@@ -1473,6 +1490,7 @@ class GDScriptParser {
 	}
 
 	void clear();
+
 	void push_error(const String &p_message, const Node *p_origin = nullptr);
 #ifdef DEBUG_ENABLED
 	void push_warning(const Node *p_source, GDScriptWarning::Code p_code, const Vector<String> &p_symbols);
@@ -1481,7 +1499,9 @@ class GDScriptParser {
 		push_warning(p_source, p_code, Vector<String>{ p_symbols... });
 	}
 	void apply_pending_warnings();
-#endif
+	void evaluate_warning_directory_rules_for_script_path();
+#endif // DEBUG_ENABLED
+
 	// Setting p_force to false will prevent the completion context from being update if a context was already set before.
 	// This should only be done when we push context before we consumed any tokens for the corresponding structure.
 	// See parse_precedence for an example.
@@ -1615,11 +1635,13 @@ class GDScriptParser {
 		// TODO: Keep track of deps.
 		return List<String>();
 	}
+
 #ifdef DEBUG_ENABLED
+	static void update_project_settings();
 	const List<GDScriptWarning> &get_warnings() const { return warnings; }
 	const HashSet<int> &get_unsafe_lines() const { return unsafe_lines; }
 	int get_last_line_number() const { return current.end_line; }
-#endif
+#endif // DEBUG_ENABLED
 
 #ifdef TOOLS_ENABLED
 	static HashMap<String, String> theme_color_names;