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.

1 files changed, 150 insertions, 30 deletions

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index cdb4f29854..8a723cab9c 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -138,13 +138,25 @@ static GDScriptParser::DataType make_enum_type(const StringName &p_enum_name, co
 }
 
 static GDScriptParser::DataType make_native_enum_type(const StringName &p_enum_name, const StringName &p_native_class, const bool p_meta = true) {
-	GDScriptParser::DataType type = make_enum_type(p_enum_name, p_native_class, p_meta);
+	// Find out which base class declared the enum, so the name is always the same even when coming from other contexts.
+	StringName native_base = p_native_class;
+	while (true && native_base != StringName()) {
+		if (ClassDB::has_enum(native_base, p_enum_name, true)) {
+			break;
+		}
+		native_base = ClassDB::get_parent_class_nocheck(native_base);
+	}
+
+	GDScriptParser::DataType type = make_enum_type(p_enum_name, native_base, p_meta);
+	if (p_meta) {
+		type.builtin_type = Variant::NIL; // Native enum types are not Dictionaries
+	}
 
 	List<StringName> enum_values;
-	ClassDB::get_enum_constants(p_native_class, p_enum_name, &enum_values);
+	ClassDB::get_enum_constants(native_base, p_enum_name, &enum_values, true);
 
 	for (const StringName &E : enum_values) {
-		type.enum_values[E] = ClassDB::get_integer_constant(p_native_class, E);
+		type.enum_values[E] = ClassDB::get_integer_constant(native_base, E);
 	}
 
 	return type;
@@ -782,6 +794,22 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
 	resolving_datatype.kind = GDScriptParser::DataType::RESOLVING;
 
 	{
+#ifdef DEBUG_ENABLED
+		HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+		GDScriptParser::Node *member_node = member.get_source_node();
+		if (member_node && member_node->type != GDScriptParser::Node::ANNOTATION) {
+			// Apply @warning_ignore annotations before resolving member.
+			for (GDScriptParser::AnnotationNode *&E : member_node->annotations) {
+				if (E->name == SNAME("@warning_ignore")) {
+					resolve_annotation(E);
+					E->apply(parser, member.variable);
+				}
+			}
+			for (GDScriptWarning::Code ignored_warning : member_node->ignored_warnings) {
+				parser->ignored_warnings.insert(ignored_warning);
+			}
+		}
+#endif
 		switch (member.type) {
 			case GDScriptParser::ClassNode::Member::VARIABLE: {
 				check_class_member_name_conflict(p_class, member.variable->identifier->name, member.variable);
@@ -790,9 +818,48 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
 
 				// Apply annotations.
 				for (GDScriptParser::AnnotationNode *&E : member.variable->annotations) {
-					resolve_annotation(E);
-					E->apply(parser, member.variable);
+					if (E->name != SNAME("@warning_ignore")) {
+						resolve_annotation(E);
+						E->apply(parser, member.variable);
+					}
+				}
+#ifdef DEBUG_ENABLED
+				if (member.variable->exported && member.variable->onready) {
+					parser->push_warning(member.variable, GDScriptWarning::ONREADY_WITH_EXPORT);
 				}
+				if (member.variable->initializer) {
+					// Check if it is call to get_node() on self (using shorthand $ or not), so we can check if @onready is needed.
+					// This could be improved by traversing the expression fully and checking the presence of get_node at any level.
+					if (!member.variable->onready && member.variable->initializer && (member.variable->initializer->type == GDScriptParser::Node::GET_NODE || member.variable->initializer->type == GDScriptParser::Node::CALL || member.variable->initializer->type == GDScriptParser::Node::CAST)) {
+						GDScriptParser::Node *expr = member.variable->initializer;
+						if (expr->type == GDScriptParser::Node::CAST) {
+							expr = static_cast<GDScriptParser::CastNode *>(expr)->operand;
+						}
+						bool is_get_node = expr->type == GDScriptParser::Node::GET_NODE;
+						bool is_using_shorthand = is_get_node;
+						if (!is_get_node && expr->type == GDScriptParser::Node::CALL) {
+							is_using_shorthand = false;
+							GDScriptParser::CallNode *call = static_cast<GDScriptParser::CallNode *>(expr);
+							if (call->function_name == SNAME("get_node")) {
+								switch (call->get_callee_type()) {
+									case GDScriptParser::Node::IDENTIFIER: {
+										is_get_node = true;
+									} break;
+									case GDScriptParser::Node::SUBSCRIPT: {
+										GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(call->callee);
+										is_get_node = subscript->is_attribute && subscript->base->type == GDScriptParser::Node::SELF;
+									} break;
+									default:
+										break;
+								}
+							}
+						}
+						if (is_get_node) {
+							parser->push_warning(member.variable, GDScriptWarning::GET_NODE_DEFAULT_WITHOUT_ONREADY, is_using_shorthand ? "$" : "get_node()");
+						}
+					}
+				}
+#endif
 			} break;
 			case GDScriptParser::ClassNode::Member::CONSTANT: {
 				check_class_member_name_conflict(p_class, member.constant->identifier->name, member.constant);
@@ -878,6 +945,10 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
 				}
 			} break;
 			case GDScriptParser::ClassNode::Member::FUNCTION:
+				for (GDScriptParser::AnnotationNode *&E : member.function->annotations) {
+					resolve_annotation(E);
+					E->apply(parser, member.function);
+				}
 				resolve_function_signature(member.function, p_source);
 				break;
 			case GDScriptParser::ClassNode::Member::ENUM_VALUE: {
@@ -931,6 +1002,9 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
 				ERR_PRINT("Trying to resolve undefined member.");
 				break;
 		}
+#ifdef DEBUG_ENABLED
+		parser->ignored_warnings = previously_ignored_warnings;
+#endif
 	}
 
 	parser->current_class = previous_class;
@@ -1059,19 +1133,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
 				resolve_annotation(E);
 				E->apply(parser, member.function);
 			}
-
-#ifdef DEBUG_ENABLED
-			HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
-			for (uint32_t ignored_warning : member.function->ignored_warnings) {
-				parser->ignored_warning_codes.insert(ignored_warning);
-			}
-#endif // DEBUG_ENABLED
-
 			resolve_function_body(member.function);
-
-#ifdef DEBUG_ENABLED
-			parser->ignored_warning_codes = previously_ignored;
-#endif // DEBUG_ENABLED
 		} else if (member.type == GDScriptParser::ClassNode::Member::VARIABLE && member.variable->property != GDScriptParser::VariableNode::PROP_NONE) {
 			if (member.variable->property == GDScriptParser::VariableNode::PROP_INLINE) {
 				if (member.variable->getter != nullptr) {
@@ -1102,9 +1164,9 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
 		GDScriptParser::ClassNode::Member member = p_class->members[i];
 		if (member.type == GDScriptParser::ClassNode::Member::VARIABLE) {
 #ifdef DEBUG_ENABLED
-			HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
-			for (uint32_t ignored_warning : member.function->ignored_warnings) {
-				parser->ignored_warning_codes.insert(ignored_warning);
+			HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+			for (GDScriptWarning::Code ignored_warning : member.variable->ignored_warnings) {
+				parser->ignored_warnings.insert(ignored_warning);
 			}
 			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);
@@ -1179,7 +1241,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
 					}
 				}
 #ifdef DEBUG_ENABLED
-				parser->ignored_warning_codes = previously_ignored;
+				parser->ignored_warnings = previously_ignored_warnings;
 #endif // DEBUG_ENABLED
 			}
 		}
@@ -1289,6 +1351,11 @@ void GDScriptAnalyzer::resolve_node(GDScriptParser::Node *p_node, bool p_is_root
 void GDScriptAnalyzer::resolve_annotation(GDScriptParser::AnnotationNode *p_annotation) {
 	ERR_FAIL_COND_MSG(!parser->valid_annotations.has(p_annotation->name), vformat(R"(Annotation "%s" not found to validate.)", p_annotation->name));
 
+	if (p_annotation->is_resolved) {
+		return;
+	}
+	p_annotation->is_resolved = true;
+
 	const MethodInfo &annotation_info = parser->valid_annotations[p_annotation->name].info;
 
 	const List<PropertyInfo>::Element *E = annotation_info.arguments.front();
@@ -1355,6 +1422,13 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 	}
 	p_function->resolved_signature = true;
 
+#ifdef DEBUG_ENABLED
+	HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+	for (GDScriptWarning::Code ignored_warning : p_function->ignored_warnings) {
+		parser->ignored_warnings.insert(ignored_warning);
+	}
+#endif
+
 	GDScriptParser::FunctionNode *previous_function = parser->current_function;
 	parser->current_function = p_function;
 
@@ -1421,7 +1495,8 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 		int default_par_count = 0;
 		bool is_static = false;
 		bool is_vararg = false;
-		if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, is_static, is_vararg)) {
+		StringName native_base;
+		if (!p_is_lambda && get_function_signature(p_function, false, base_type, function_name, parent_return_type, parameters_types, default_par_count, is_static, is_vararg, &native_base)) {
 			bool valid = p_function->is_static == is_static;
 			valid = valid && parent_return_type == p_function->get_datatype();
 
@@ -1447,8 +1522,8 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 						parameter = "Variant";
 					}
 					parent_signature += parameter;
-					if (j == parameters_types.size() - default_par_count) {
-						parent_signature += " = default";
+					if (j >= parameters_types.size() - default_par_count) {
+						parent_signature += " = <default>";
 					}
 
 					j++;
@@ -1464,6 +1539,11 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 
 				push_error(vformat(R"(The function signature doesn't match the parent. Parent signature is "%s".)", parent_signature), p_function);
 			}
+#ifdef DEBUG_ENABLED
+			if (native_base != StringName()) {
+				parser->push_warning(p_function, GDScriptWarning::NATIVE_METHOD_OVERRIDE, function_name, native_base);
+			}
+#endif
 		}
 #endif // TOOLS_ENABLED
 	}
@@ -1472,6 +1552,9 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
 		p_function->set_datatype(prev_datatype);
 	}
 
+#ifdef DEBUG_ENABLED
+	parser->ignored_warnings = previously_ignored_warnings;
+#endif
 	parser->current_function = previous_function;
 }
 
@@ -1481,6 +1564,13 @@ void GDScriptAnalyzer::resolve_function_body(GDScriptParser::FunctionNode *p_fun
 	}
 	p_function->resolved_body = true;
 
+#ifdef DEBUG_ENABLED
+	HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+	for (GDScriptWarning::Code ignored_warning : p_function->ignored_warnings) {
+		parser->ignored_warnings.insert(ignored_warning);
+	}
+#endif
+
 	GDScriptParser::FunctionNode *previous_function = parser->current_function;
 	parser->current_function = p_function;
 
@@ -1498,6 +1588,9 @@ void GDScriptAnalyzer::resolve_function_body(GDScriptParser::FunctionNode *p_fun
 		}
 	}
 
+#ifdef DEBUG_ENABLED
+	parser->ignored_warnings = previously_ignored_warnings;
+#endif
 	parser->current_function = previous_function;
 }
 
@@ -1538,16 +1631,16 @@ void GDScriptAnalyzer::resolve_suite(GDScriptParser::SuiteNode *p_suite) {
 		}
 
 #ifdef DEBUG_ENABLED
-		HashSet<uint32_t> previously_ignored = parser->ignored_warning_codes;
-		for (uint32_t ignored_warning : stmt->ignored_warnings) {
-			parser->ignored_warning_codes.insert(ignored_warning);
+		HashSet<GDScriptWarning::Code> previously_ignored_warnings = parser->ignored_warnings;
+		for (GDScriptWarning::Code ignored_warning : stmt->ignored_warnings) {
+			parser->ignored_warnings.insert(ignored_warning);
 		}
 #endif // DEBUG_ENABLED
 
 		resolve_node(stmt);
 
 #ifdef DEBUG_ENABLED
-		parser->ignored_warning_codes = previously_ignored;
+		parser->ignored_warnings = previously_ignored_warnings;
 #endif // DEBUG_ENABLED
 
 		decide_suite_type(p_suite, stmt);
@@ -1599,6 +1692,11 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
 			} else if (initializer_type.kind == GDScriptParser::DataType::BUILTIN && initializer_type.builtin_type == Variant::NIL && !is_constant) {
 				push_error(vformat(R"(Cannot infer the type of "%s" %s because the value is "null".)", p_assignable->identifier->name, p_kind), p_assignable->initializer);
 			}
+#ifdef DEBUG_ENABLED
+			if (initializer_type.is_hard_type() && initializer_type.is_variant()) {
+				parser->push_warning(p_assignable, GDScriptWarning::INFERENCE_ON_VARIANT, p_kind);
+			}
+#endif
 		} else {
 			if (!initializer_type.is_set()) {
 				push_error(vformat(R"(Could not resolve type for %s "%s".)", p_kind, p_assignable->identifier->name), p_assignable->initializer);
@@ -2931,7 +3029,11 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
 
 		// Enums do not have functions other than the built-in dictionary ones.
 		if (base_type.kind == GDScriptParser::DataType::ENUM && base_type.is_meta_type) {
-			push_error(vformat(R"*(Enums only have Dictionary built-in methods. Function "%s()" does not exist for enum "%s".)*", p_call->function_name, base_type.enum_type), p_call->callee);
+			if (base_type.builtin_type == Variant::DICTIONARY) {
+				push_error(vformat(R"*(Enums only have Dictionary built-in methods. Function "%s()" does not exist for enum "%s".)*", p_call->function_name, base_type.enum_type), p_call->callee);
+			} else {
+				push_error(vformat(R"*(The native enum "%s" does not behave like Dictionary and does not have methods of its own.)*", base_type.enum_type), p_call->callee);
+			}
 		} else if (!p_call->is_super && callee_type != GDScriptParser::Node::NONE) { // Check if the name exists as something else.
 			GDScriptParser::IdentifierNode *callee_id;
 			if (callee_type == GDScriptParser::Node::IDENTIFIER) {
@@ -4310,15 +4412,27 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo
 			}
 			elem_type.is_constant = false;
 			result.set_container_element_type(elem_type);
+		} else if (p_property.type == Variant::INT) {
+			// Check if it's enum.
+			if (p_property.class_name != StringName()) {
+				Vector<String> names = String(p_property.class_name).split(".");
+				if (names.size() == 2) {
+					result = make_native_enum_type(names[1], names[0], false);
+					result.is_constant = false;
+				}
+			}
 		}
 	}
 	return result;
 }
 
-bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_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 GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType p_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) {
 	r_static = false;
 	r_vararg = false;
 	r_default_arg_count = 0;
+	if (r_native_class) {
+		*r_native_class = StringName();
+	}
 	StringName function_name = p_function;
 
 	bool was_enum = false;
@@ -4453,6 +4567,12 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::Node *p_source, bo
 		if (valid && Engine::get_singleton()->has_singleton(base_native)) {
 			r_static = true;
 		}
+#ifdef DEBUG_ENABLED
+		MethodBind *native_method = ClassDB::get_method(base_native, function_name);
+		if (native_method && r_native_class) {
+			*r_native_class = native_method->get_instance_class();
+		}
+#endif
 		return valid;
 	}