GDScript: Add warnings that are set to error by default

- Adds a list of default levels for all warning so they can be set
  individually.
- Add warnings set by default to error for:
  - Using `get_node()` without `@onready`.
  - Using `@onready` together with `@export`.
  - Inferring a static type with a Variant value.
  - Overriding a native engine method.
- Adjust how annotations to ignore warnings are treated so they also
  apply to method parameters.
- Clean up a bit how ignored warnings are set. There were two sets but
  only one was actually being used.
- Set all warnings to the `WARN` level for tests, so they they can be
  properly tested.
- Fix enum types in native methods signatures being set to `int`.
- Fix native enums being treated as Dictionary by mistake.
- Make name of native enum types use the class they are defined in, not
  the direct super class of the script. This ensures they are always
  equal even when coming from different sources.
- Fix error for signature mismatch that was only showing the first
  default argument as having a default. Now it shows for all.

doc/classes/ProjectSettings.xml

@@ -405,9 +405,15 @@
 		<member name="debug/gdscript/warnings/function_used_as_property" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when using a function as if it is a property.
 		</member>
+		<member name="debug/gdscript/warnings/get_node_default_without_onready" type="int" setter="" getter="" default="2">
+			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when [method Node.get_node] (or the shorthand [code]$[/code]) is used as default value of a class variable without the [code]@onready[/code] annotation.
+		</member>
 		<member name="debug/gdscript/warnings/incompatible_ternary" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a ternary operator may emit values with incompatible types.
 		</member>
+		<member name="debug/gdscript/warnings/inference_on_variant" type="int" setter="" getter="" default="2">
+			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a static inferred type uses a [Variant] as initial value, which makes the static type to also be Variant.
+		</member>
 		<member name="debug/gdscript/warnings/int_as_enum_without_cast" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when trying to use an integer as an enum without an explicit cast.
 		</member>
@@ -420,6 +426,12 @@
 		<member name="debug/gdscript/warnings/narrowing_conversion" type="int" setter="" getter="" default="1">
 			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).
 		</member>
+		<member name="debug/gdscript/warnings/native_method_override" type="int" setter="" getter="" default="2">
+			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when a method in the script overrides a native method, because it may not behave as expected.
+		</member>
+		<member name="debug/gdscript/warnings/onready_with_export" type="int" setter="" getter="" default="2">
+			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when the [code]@onready[/code] annotation is used together with the [code]@export[/code] annotation, since it may not behave as expected.
+		</member>
 		<member name="debug/gdscript/warnings/property_used_as_function" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when using a property as if it is a function.
 		</member>
@@ -477,7 +489,7 @@
 		<member name="debug/gdscript/warnings/unsafe_property_access" type="int" setter="" getter="" default="0">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when accessing a property whose presence is not guaranteed at compile-time in the class.
 		</member>
-		<member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="0">
+		<member name="debug/gdscript/warnings/unsafe_void_return" type="int" setter="" getter="" default="1">
 			When set to [code]warn[/code] or [code]error[/code], produces a warning or an error respectively when returning a call from a [code]void[/code] function when such call cannot be guaranteed to be also [code]void[/code].
 		</member>
 		<member name="debug/gdscript/warnings/unused_local_constant" type="int" setter="" getter="" default="1">

modules/gdscript/gdscript_analyzer.h

@@ -117,7 +117,7 @@ class GDScriptAnalyzer {
 	static GDScriptParser::DataType type_from_metatype(const GDScriptParser::DataType &p_meta_type);
 	GDScriptParser::DataType type_from_property(const PropertyInfo &p_property, bool p_is_arg = false) const;
 	GDScriptParser::DataType make_global_class_meta_type(const StringName &p_class_name, const GDScriptParser::Node *p_source);
-	bool get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg);
+	bool get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg, StringName *r_native_class = nullptr);
 	bool function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg);
 	void validate_call_arg(const List<GDScriptParser::DataType> &p_par_types, int p_default_args_count, bool p_is_vararg, const GDScriptParser::CallNode *p_call);
 	void validate_call_arg(const MethodInfo &p_method, const GDScriptParser::CallNode *p_call);

modules/gdscript/gdscript_parser.cpp

@@ -158,14 +158,10 @@ void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_
 		return;
 	}
 
-	if (ignored_warning_codes.has(p_code)) {
+	if (ignored_warnings.has(p_code)) {
 		return;
 	}
 
-	String warn_name = GDScriptWarning::get_name_from_code((GDScriptWarning::Code)p_code).to_lower();
-	if (ignored_warnings.has(warn_name)) {
-		return;
-	}
 	int warn_level = (int)GLOBAL_GET(GDScriptWarning::get_settings_path_from_code(p_code));
 	if (!warn_level) {
 		return;
@@ -180,7 +176,7 @@ void GDScriptParser::push_warning(const Node *p_source, GDScriptWarning::Code p_
 	warning.rightmost_column = p_source->rightmost_column;
 
 	if (warn_level == GDScriptWarning::WarnLevel::ERROR || bool(GLOBAL_GET("debug/gdscript/warnings/treat_warnings_as_errors"))) {
-		push_error(warning.get_message(), p_source);
+		push_error(warning.get_message() + String(" (Warning treated as error.)"), p_source);
 		return;
 	}
 
@@ -3548,7 +3544,11 @@ const GDScriptParser::SuiteNode::Local &GDScriptParser::SuiteNode::get_local(con
 	return empty;
 }
 
-bool GDScriptParser::AnnotationNode::apply(GDScriptParser *p_this, Node *p_target) const {
+bool GDScriptParser::AnnotationNode::apply(GDScriptParser *p_this, Node *p_target) {
+	if (is_applied) {
+		return true;
+	}
+	is_applied = true;
 	return (p_this->*(p_this->valid_annotations[name].apply))(this, p_target);
 }
 

modules/gdscript/gdscript_parser.h

@@ -297,7 +297,9 @@ class GDScriptParser {
 		int leftmost_column = 0, rightmost_column = 0;
 		Node *next = nullptr;
 		List<AnnotationNode *> annotations;
-		Vector<uint32_t> ignored_warnings;
+#ifdef DEBUG_ENABLED
+		Vector<GDScriptWarning::Code> ignored_warnings;
+#endif
 
 		DataType datatype;
 
@@ -329,8 +331,10 @@ class GDScriptParser {
 
 		AnnotationInfo *info = nullptr;
 		PropertyInfo export_info;
+		bool is_resolved = false;
+		bool is_applied = false;
 
-		bool apply(GDScriptParser *p_this, Node *p_target) const;
+		bool apply(GDScriptParser *p_this, Node *p_target);
 		bool applies_to(uint32_t p_target_kinds) const;
 
 		AnnotationNode() {
@@ -1263,8 +1267,7 @@ class GDScriptParser {
 #ifdef DEBUG_ENABLED
 	bool is_ignoring_warnings = false;
 	List<GDScriptWarning> warnings;
-	HashSet<String> ignored_warnings;
-	HashSet<uint32_t> ignored_warning_codes;
+	HashSet<GDScriptWarning::Code> ignored_warnings;
 	HashSet<int> unsafe_lines;
 #endif
 

modules/gdscript/gdscript_warning.cpp

@@ -170,6 +170,21 @@ String GDScriptWarning::get_message() const {
 		case RENAMED_IN_GD4_HINT: {
 			break; // Renamed identifier hint is taken care of by the GDScriptAnalyzer. No message needed here.
 		}
+		case INFERENCE_ON_VARIANT: {
+			CHECK_SYMBOLS(1);
+			return vformat("The %s type is being inferred from a Variant value, so it will be typed as Variant.", symbols[0]);
+		}
+		case NATIVE_METHOD_OVERRIDE: {
+			CHECK_SYMBOLS(2);
+			return vformat(R"(The method "%s" overrides a method from native class "%s". This won't be called by the engine and may not work as expected.)", symbols[0], symbols[1]);
+		}
+		case GET_NODE_DEFAULT_WITHOUT_ONREADY: {
+			CHECK_SYMBOLS(1);
+			return vformat(R"*(The default value is using "%s" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.)*", symbols[0]);
+		}
+		case ONREADY_WITH_EXPORT: {
+			return R"(The "@onready" annotation will make the default value to be set after the "@export" takes effect and will override it.)";
+		}
 		case WARNING_MAX:
 			break; // Can't happen, but silences warning
 	}
@@ -179,14 +194,8 @@ String GDScriptWarning::get_message() const {
 }
 
 int GDScriptWarning::get_default_value(Code p_code) {
-	if (get_name_from_code(p_code).to_lower().begins_with("unsafe_")) {
-		return WarnLevel::IGNORE;
-	}
-	// Too spammy by default on common cases (connect, Tween, etc.).
-	if (p_code == RETURN_VALUE_DISCARDED) {
-		return WarnLevel::IGNORE;
-	}
-	return WarnLevel::WARN;
+	ERR_FAIL_INDEX_V_MSG(p_code, WARNING_MAX, WarnLevel::IGNORE, "Getting default value of invalid warning code.");
+	return default_warning_levels[p_code];
 }
 
 PropertyInfo GDScriptWarning::get_property_info(Code p_code) {
@@ -240,7 +249,11 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
 		"INT_AS_ENUM_WITHOUT_MATCH",
 		"STATIC_CALLED_ON_INSTANCE",
 		"CONFUSABLE_IDENTIFIER",
-		"RENAMED_IN_GODOT_4_HINT"
+		"RENAMED_IN_GODOT_4_HINT",
+		"INFERENCE_ON_VARIANT",
+		"NATIVE_METHOD_OVERRIDE",
+		"GET_NODE_DEFAULT_WITHOUT_ONREADY",
+		"ONREADY_WITH_EXPORT",
 	};
 
 	static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names.");

modules/gdscript/gdscript_warning.h

@@ -82,9 +82,58 @@ class GDScriptWarning {
 		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
+		INFERENCE_ON_VARIANT, // The declaration uses type inference but the value is typed as Variant.
+		NATIVE_METHOD_OVERRIDE, // The script method overrides a native one, this may not work as intended.
+		GET_NODE_DEFAULT_WITHOUT_ONREADY, // A class variable uses `get_node()` (or the `$` notation) as its default value, but does not use the @onready annotation.
+		ONREADY_WITH_EXPORT, // The `@onready` annotation will set the value after `@export` which is likely not intended.
 		WARNING_MAX,
 	};
 
+	constexpr static WarnLevel default_warning_levels[] = {
+		WARN, // UNASSIGNED_VARIABLE
+		WARN, // UNASSIGNED_VARIABLE_OP_ASSIGN
+		WARN, // UNUSED_VARIABLE
+		WARN, // UNUSED_LOCAL_CONSTANT
+		WARN, // SHADOWED_VARIABLE
+		WARN, // SHADOWED_VARIABLE_BASE_CLASS
+		WARN, // UNUSED_PRIVATE_CLASS_VARIABLE
+		WARN, // UNUSED_PARAMETER
+		WARN, // UNREACHABLE_CODE
+		WARN, // UNREACHABLE_PATTERN
+		WARN, // STANDALONE_EXPRESSION
+		WARN, // NARROWING_CONVERSION
+		WARN, // INCOMPATIBLE_TERNARY
+		WARN, // UNUSED_SIGNAL
+		IGNORE, // RETURN_VALUE_DISCARDED // Too spammy by default on common cases (connect, Tween, etc.).
+		WARN, // PROPERTY_USED_AS_FUNCTION
+		WARN, // CONSTANT_USED_AS_FUNCTION
+		WARN, // FUNCTION_USED_AS_PROPERTY
+		WARN, // INTEGER_DIVISION
+		IGNORE, // UNSAFE_PROPERTY_ACCESS // Too common in untyped scenarios.
+		IGNORE, // UNSAFE_METHOD_ACCESS // Too common in untyped scenarios.
+		IGNORE, // UNSAFE_CAST // Too common in untyped scenarios.
+		IGNORE, // UNSAFE_CALL_ARGUMENT // Too common in untyped scenarios.
+		WARN, // UNSAFE_VOID_RETURN
+		WARN, // DEPRECATED_KEYWORD
+		WARN, // STANDALONE_TERNARY
+		WARN, // ASSERT_ALWAYS_TRUE
+		WARN, // ASSERT_ALWAYS_FALSE
+		WARN, // REDUNDANT_AWAIT
+		WARN, // EMPTY_FILE
+		WARN, // SHADOWED_GLOBAL_IDENTIFIER
+		WARN, // INT_AS_ENUM_WITHOUT_CAST
+		WARN, // INT_AS_ENUM_WITHOUT_MATCH
+		WARN, // STATIC_CALLED_ON_INSTANCE
+		WARN, // CONFUSABLE_IDENTIFIER
+		WARN, // RENAMED_IN_GD4_HINT
+		ERROR, // INFERENCE_ON_VARIANT // Most likely done by accident, usually inference is trying for a particular type.
+		ERROR, // NATIVE_METHOD_OVERRIDE // May not work as expected.
+		ERROR, // GET_NODE_DEFAULT_WITHOUT_ONREADY // May not work as expected.
+		ERROR, // ONREADY_WITH_EXPORT // May not work as expected.
+	};
+
+	static_assert((sizeof(default_warning_levels) / sizeof(default_warning_levels[0])) == WARNING_MAX, "Amount of default levels does not match the amount of warnings.");
+
 	Code code = WARNING_MAX;
 	int start_line = -1, end_line = -1;
 	int leftmost_column = -1, rightmost_column = -1;

modules/gdscript/tests/gdscript_test_runner.cpp

@@ -146,11 +146,11 @@ GDScriptTestRunner::GDScriptTestRunner(const String &p_source_dir, bool p_init_l
 		init_language(p_source_dir);
 	}
 #ifdef DEBUG_ENABLED
-	// Enable all warnings for GDScript, so we can test them.
+	// Set all warning levels to "Warn" in order to test them properly, even the ones that default to error.
 	ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/enable", true);
 	for (int i = 0; i < (int)GDScriptWarning::WARNING_MAX; i++) {
-		String warning = GDScriptWarning::get_name_from_code((GDScriptWarning::Code)i).to_lower();
-		ProjectSettings::get_singleton()->set_setting("debug/gdscript/warnings/" + warning, true);
+		String warning_setting = GDScriptWarning::get_settings_path_from_code((GDScriptWarning::Code)i);
+		ProjectSettings::get_singleton()->set_setting(warning_setting, (int)GDScriptWarning::WARN);
 	}
 #endif
 

modules/gdscript/tests/scripts/analyzer/errors/function_dont_match_parent_signature_parameter_default_values.out

@@ -1,2 +1,2 @@
 GDTEST_ANALYZER_ERROR
-The function signature doesn't match the parent. Parent signature is "my_function(int = default) -> int".
+The function signature doesn't match the parent. Parent signature is "my_function(int = <default>) -> int".

modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.gd

@@ -0,0 +1,18 @@
+extends Node
+
+@onready var shorthand = $Node
+@onready var call = get_node(^"Node")
+@onready var shorthand_with_cast = $Node as Node
+@onready var call_with_cast = get_node(^"Node") as Node
+
+func _init():
+	var node := Node.new()
+	node.name = "Node"
+	add_child(node)
+
+func test():
+	# Those are expected to be `null` since `_ready()` is never called on tests.
+	prints("shorthand", shorthand)
+	prints("call", call)
+	prints("shorthand_with_cast", shorthand_with_cast)
+	prints("call_with_cast", call_with_cast)

modules/gdscript/tests/scripts/analyzer/features/allow_get_node_with_onready.out

@@ -0,0 +1,5 @@
+GDTEST_OK
+shorthand <null>
+call <null>
+shorthand_with_cast <null>
+call_with_cast <null>

modules/gdscript/tests/scripts/analyzer/features/assign_to_native_enum_property.gd

@@ -0,0 +1,13 @@
+# https://github.com/godotengine/godot/issues/72501
+extends Node
+
+func test():
+	prints("before", process_mode)
+	process_mode = PROCESS_MODE_PAUSABLE
+	prints("after", process_mode)
+
+	var node := Node.new()
+	add_child(node)
+	prints("before", node.process_mode)
+	node.process_mode = PROCESS_MODE_PAUSABLE
+	prints("after", node.process_mode)

modules/gdscript/tests/scripts/analyzer/features/assign_to_native_enum_property.out

@@ -0,0 +1,5 @@
+GDTEST_OK
+before 0
+after 1
+before 0
+after 1

modules/gdscript/tests/scripts/analyzer/features/hard_variants.gd

@@ -2,16 +2,18 @@ func variant() -> Variant: return null
 
 var member_weak = variant()
 var member_typed: Variant = variant()
+@warning_ignore("inference_on_variant")
 var member_inferred := variant()
 
 func param_weak(param = variant()) -> void: print(param)
 func param_typed(param: Variant = variant()) -> void: print(param)
+@warning_ignore("inference_on_variant")
 func param_inferred(param := variant()) -> void: print(param)
 
 func return_untyped(): return variant()
 func return_typed() -> Variant: return variant()
 
-@warning_ignore("unused_variable")
+@warning_ignore("unused_variable", "inference_on_variant")
 func test() -> void:
 	var weak = variant()
 	var typed: Variant = variant()

modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.gd

@@ -0,0 +1,17 @@
+extends Node
+
+var add_node = do_add_node() # Hack to have one node on init and not fail at runtime.
+
+var shorthand = $Node
+var with_self = self.get_node(^"Node")
+var without_self = get_node(^"Node")
+var with_cast = get_node(^"Node") as Node
+var shorthand_with_cast = $Node as Node
+
+func test():
+	print("warn")
+
+func do_add_node():
+	var node = Node.new()
+	node.name = "Node"
+	add_child(node)

modules/gdscript/tests/scripts/analyzer/warnings/get_node_without_onready.out

@@ -0,0 +1,22 @@
+GDTEST_OK
+>> WARNING
+>> Line: 5
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "$" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+>> WARNING
+>> Line: 6
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "get_node()" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+>> WARNING
+>> Line: 7
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "get_node()" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+>> WARNING
+>> Line: 8
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "get_node()" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+>> WARNING
+>> Line: 9
+>> GET_NODE_DEFAULT_WITHOUT_ONREADY
+>> The default value is using "$" which won't return nodes in the scene tree before "_ready()" is called. Use the "@onready" annotation to solve this.
+warn

modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.gd

@@ -0,0 +1,6 @@
+func test():
+	var inferred_with_variant := return_variant()
+	print(inferred_with_variant)
+
+func return_variant() -> Variant:
+	return "warn"

modules/gdscript/tests/scripts/analyzer/warnings/inference_with_variant.out

@@ -0,0 +1,6 @@
+GDTEST_OK
+>> WARNING
+>> Line: 2
+>> INFERENCE_ON_VARIANT
+>> The variable type is being inferred from a Variant value, so it will be typed as Variant.
+warn

modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.gd

@@ -0,0 +1,6 @@
+extends Node
+
+@onready @export var conflict = ""
+
+func test():
+	print("warn")

modules/gdscript/tests/scripts/analyzer/warnings/onready_with_export.out

@@ -0,0 +1,6 @@
+GDTEST_OK
+>> WARNING
+>> Line: 3
+>> ONREADY_WITH_EXPORT
+>> The "@onready" annotation will make the default value to be set after the "@export" takes effect and will override it.
+warn

modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.gd

@@ -0,0 +1,5 @@
+func test():
+	print("warn")
+
+func get(_property: StringName) -> Variant:
+	return null

modules/gdscript/tests/scripts/analyzer/warnings/overriding_native_method.out

@@ -0,0 +1,6 @@
+GDTEST_OK
+>> WARNING
+>> Line: 4
+>> NATIVE_METHOD_OVERRIDE
+>> The method "get" overrides a method from native class "Object". This won't be called by the engine and may not work as expected.
+warn