Merge pull request #69471 from rune-scape/rune-out-of-order

GDScript: Out of order member resolution

modules/gdscript/gdscript_analyzer.h

@@ -52,18 +52,20 @@ class GDScriptAnalyzer {
 
 	void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list);
 
-	Error resolve_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive = true);
+	Error resolve_class_inheritance(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source = nullptr);
+	Error resolve_class_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive);
 	GDScriptParser::DataType resolve_datatype(GDScriptParser::TypeNode *p_type);
 
 	void decide_suite_type(GDScriptParser::Node *p_suite, GDScriptParser::Node *p_statement);
 
-	// This traverses the tree to resolve all TypeNodes.
-	Error resolve_program();
-
 	void resolve_annotation(GDScriptParser::AnnotationNode *p_annotation);
-	void resolve_class_interface(GDScriptParser::ClassNode *p_class);
+	void resolve_class_member(GDScriptParser::ClassNode *p_class, StringName p_name, const GDScriptParser::Node *p_source = nullptr);
-	void resolve_class_body(GDScriptParser::ClassNode *p_class);
+	void resolve_class_member(GDScriptParser::ClassNode *p_class, int p_index, const GDScriptParser::Node *p_source = nullptr);
-	void resolve_function_signature(GDScriptParser::FunctionNode *p_function);
+	void resolve_class_interface(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source = nullptr);
+	void resolve_class_interface(GDScriptParser::ClassNode *p_class, bool p_recursive);
+	void resolve_class_body(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source = nullptr);
+	void resolve_class_body(GDScriptParser::ClassNode *p_class, bool p_recursive);
+	void resolve_function_signature(GDScriptParser::FunctionNode *p_function, const GDScriptParser::Node *p_source = nullptr);
 	void resolve_function_body(GDScriptParser::FunctionNode *p_function);
 	void resolve_node(GDScriptParser::Node *p_node, bool p_is_root = true);
 	void resolve_suite(GDScriptParser::SuiteNode *p_suite);
@@ -115,7 +117,7 @@ class GDScriptAnalyzer {
 	GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, bool &r_valid, const GDScriptParser::Node *p_source);
 	void update_array_literal_element_type(const GDScriptParser::DataType &p_base_type, GDScriptParser::ArrayNode *p_array_literal);
 	bool is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false, const GDScriptParser::Node *p_source_node = nullptr);
-	void push_error(const String &p_message, const GDScriptParser::Node *p_origin);
+	void push_error(const String &p_message, const GDScriptParser::Node *p_origin = nullptr);
 	void mark_node_unsafe(const GDScriptParser::Node *p_node);
 	void mark_lambda_use_self();
 	bool class_exists(const StringName &p_class) const;
@@ -128,6 +130,7 @@ public:
 	Error resolve_inheritance();
 	Error resolve_interface();
 	Error resolve_body();
+	Error resolve_dependencies();
 	Error analyze();
 
 	GDScriptAnalyzer(GDScriptParser *p_parser);

modules/gdscript/gdscript_cache.cpp

@@ -50,6 +50,13 @@ GDScriptParser *GDScriptParserRef::get_parser() const {
 	return parser;
 }
 
+GDScriptAnalyzer *GDScriptParserRef::get_analyzer() {
+	if (analyzer == nullptr) {
+		analyzer = memnew(GDScriptAnalyzer(parser));
+	}
+	return analyzer;
+}
+
 Error GDScriptParserRef::raise_status(Status p_new_status) {
 	ERR_FAIL_COND_V(parser == nullptr, ERR_INVALID_DATA);
 
@@ -64,23 +71,22 @@ Error GDScriptParserRef::raise_status(Status p_new_status) {
 				result = parser->parse(GDScriptCache::get_source_code(path), path, false);
 				break;
 			case PARSED: {
-				analyzer = memnew(GDScriptAnalyzer(parser));
 				status = INHERITANCE_SOLVED;
-				Error inheritance_result = analyzer->resolve_inheritance();
+				Error inheritance_result = get_analyzer()->resolve_inheritance();
 				if (result == OK) {
 					result = inheritance_result;
 				}
 			} break;
 			case INHERITANCE_SOLVED: {
 				status = INTERFACE_SOLVED;
-				Error interface_result = analyzer->resolve_interface();
+				Error interface_result = get_analyzer()->resolve_interface();
 				if (result == OK) {
 					result = interface_result;
 				}
 			} break;
 			case INTERFACE_SOLVED: {
 				status = FULLY_SOLVED;
-				Error body_result = analyzer->resolve_body();
+				Error body_result = get_analyzer()->resolve_body();
 				if (result == OK) {
 					result = body_result;
 				}

modules/gdscript/gdscript_cache.h

@@ -65,6 +65,7 @@ public:
 	bool is_valid() const;
 	Status get_status() const;
 	GDScriptParser *get_parser() const;
+	GDScriptAnalyzer *get_analyzer();
 	Error raise_status(Status p_new_status);
 	void clear();
 

modules/gdscript/gdscript_compiler.cpp

@@ -157,6 +157,7 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
 				result.builtin_type = Variant::INT;
 			}
 			break;
+		case GDScriptParser::DataType::RESOLVING:
 		case GDScriptParser::DataType::UNRESOLVED: {
 			ERR_PRINT("Parser bug: converting unresolved type.");
 			return GDScriptDataType();

modules/gdscript/gdscript_parser.cpp

@@ -642,6 +642,53 @@ void GDScriptParser::parse_program() {
 	clear_unused_annotations();
 }
 
+GDScriptParser::ClassNode *GDScriptParser::find_class(const String &p_qualified_name) const {
+	String first = p_qualified_name.get_slice("::", 0);
+
+	Vector<String> class_names;
+	GDScriptParser::ClassNode *result = nullptr;
+	// Empty initial name means start at the head.
+	if (first.is_empty() || (head->identifier && first == head->identifier->name)) {
+		class_names = p_qualified_name.split("::");
+		result = head;
+	} else if (p_qualified_name.begins_with(script_path)) {
+		// Script path could have a class path separator("::") in it.
+		class_names = p_qualified_name.trim_prefix(script_path).split("::");
+		result = head;
+	} else if (head->has_member(first)) {
+		class_names = p_qualified_name.split("::");
+		GDScriptParser::ClassNode::Member member = head->get_member(first);
+		if (member.type == GDScriptParser::ClassNode::Member::CLASS) {
+			result = member.m_class;
+		}
+	}
+
+	// Starts at index 1 because index 0 was handled above.
+	for (int i = 1; result != nullptr && i < class_names.size(); i++) {
+		String current_name = class_names[i];
+		GDScriptParser::ClassNode *next = nullptr;
+		if (result->has_member(current_name)) {
+			GDScriptParser::ClassNode::Member member = result->get_member(current_name);
+			if (member.type == GDScriptParser::ClassNode::Member::CLASS) {
+				next = member.m_class;
+			}
+		}
+		result = next;
+	}
+
+	return result;
+}
+
+bool GDScriptParser::has_class(const GDScriptParser::ClassNode *p_class) const {
+	if (head->fqcn.is_empty() && p_class->fqcn.get_slice("::", 0).is_empty()) {
+		return p_class == head;
+	} else if (p_class->fqcn.begins_with(head->fqcn)) {
+		return find_class(p_class->fqcn.trim_prefix(head->fqcn)) == p_class;
+	}
+
+	return false;
+}
+
 GDScriptParser::ClassNode *GDScriptParser::parse_class() {
 	ClassNode *n_class = alloc_node<ClassNode>();
 
@@ -2240,7 +2287,7 @@ GDScriptParser::IdentifierNode *GDScriptParser::parse_identifier() {
 
 GDScriptParser::ExpressionNode *GDScriptParser::parse_identifier(ExpressionNode *p_previous_operand, bool p_can_assign) {
 	if (!previous.is_identifier()) {
-		ERR_FAIL_V_MSG(nullptr, "Parser bug: parsing literal node without literal token.");
+		ERR_FAIL_V_MSG(nullptr, "Parser bug: parsing identifier node without identifier token.");
 	}
 	IdentifierNode *identifier = alloc_node<IdentifierNode>();
 	complete_extents(identifier);
@@ -4042,11 +4089,12 @@ String GDScriptParser::DataType::to_string() const {
 		}
 		case ENUM:
 			return enum_type.operator String() + " (enum)";
+		case RESOLVING:
 		case UNRESOLVED:
 			return "<unresolved type>";
 	}
 
-	ERR_FAIL_V_MSG("<unresolved type", "Kind set outside the enum range.");
+	ERR_FAIL_V_MSG("<unresolved type>", "Kind set outside the enum range.");
 }
 
 static Variant::Type _variant_type_to_typed_array_element_type(Variant::Type p_type) {

modules/gdscript/gdscript_parser.h

@@ -107,6 +107,7 @@ public:
 			CLASS, // GDScript.
 			ENUM, // Enumeration.
 			VARIANT, // Can be any type.
+			RESOLVING, // Currently resolving.
 			UNRESOLVED,
 		};
 		Kind kind = UNRESOLVED;
@@ -133,9 +134,10 @@ public:
 		MethodInfo method_info; // For callable/signals.
 		HashMap<StringName, int64_t> enum_values; // For enums.
 
-		_FORCE_INLINE_ bool is_set() const { return kind != UNRESOLVED; }
+		_FORCE_INLINE_ bool is_set() const { return kind != RESOLVING && kind != UNRESOLVED; }
+		_FORCE_INLINE_ bool is_resolving() const { return kind == RESOLVING; }
 		_FORCE_INLINE_ bool has_no_type() const { return type_source == UNDETECTED; }
-		_FORCE_INLINE_ bool is_variant() const { return kind == VARIANT || kind == UNRESOLVED; }
+		_FORCE_INLINE_ bool is_variant() const { return kind == VARIANT || kind == RESOLVING || kind == UNRESOLVED; }
 		_FORCE_INLINE_ bool is_hard_type() const { return type_source > INFERRED; }
 		String to_string() const;
 
@@ -188,6 +190,7 @@ public:
 					return script_type == p_other.script_type;
 				case CLASS:
 					return class_type == p_other.class_type;
+				case RESOLVING:
 				case UNRESOLVED:
 					break;
 			}
@@ -516,6 +519,32 @@ public:
 			};
 			EnumNode::Value enum_value;
 
+			String get_name() const {
+				switch (type) {
+					case UNDEFINED:
+						return "<undefined member>";
+					case CLASS:
+						// All class-type members have an id.
+						return m_class->identifier->name;
+					case CONSTANT:
+						return constant->identifier->name;
+					case FUNCTION:
+						return function->identifier->name;
+					case SIGNAL:
+						return signal->identifier->name;
+					case VARIABLE:
+						return variable->identifier->name;
+					case ENUM:
+						// All enum-type members have an id.
+						return m_enum->identifier->name;
+					case ENUM_VALUE:
+						return enum_value.identifier->name;
+					case GROUP:
+						return annotation->export_info.name;
+				}
+				return "";
+			}
+
 			String get_type_name() const {
 				switch (type) {
 					case UNDEFINED:
@@ -576,31 +605,42 @@ public:
 						return variable->get_datatype();
 					case ENUM:
 						return m_enum->get_datatype();
-					case ENUM_VALUE: {
+					case ENUM_VALUE:
-						// Always integer.
+						return enum_value.identifier->get_datatype();
-						DataType out_type;
+					case SIGNAL:
-						out_type.type_source = DataType::ANNOTATED_EXPLICIT;
+						return signal->get_datatype();
-						out_type.kind = DataType::BUILTIN;
+					case GROUP:
-						out_type.builtin_type = Variant::INT;
-						return out_type;
-					}
-					case SIGNAL: {
-						DataType out_type;
-						out_type.type_source = DataType::ANNOTATED_EXPLICIT;
-						out_type.kind = DataType::BUILTIN;
-						out_type.builtin_type = Variant::SIGNAL;
-						// TODO: Add parameter info.
-						return out_type;
-					}
-					case GROUP: {
 						return DataType();
-					}
 					case UNDEFINED:
 						return DataType();
 				}
 				ERR_FAIL_V_MSG(DataType(), "Reaching unhandled type.");
 			}
 
+			Node *get_source_node() const {
+				switch (type) {
+					case CLASS:
+						return m_class;
+					case CONSTANT:
+						return constant;
+					case FUNCTION:
+						return function;
+					case VARIABLE:
+						return variable;
+					case ENUM:
+						return m_enum;
+					case ENUM_VALUE:
+						return enum_value.identifier;
+					case SIGNAL:
+						return signal;
+					case GROUP:
+						return annotation;
+					case UNDEFINED:
+						return nullptr;
+				}
+				ERR_FAIL_V_MSG(nullptr, "Reaching unhandled type.");
+			}
+
 			Member() {}
 
 			Member(ClassNode *p_class) {
@@ -1430,6 +1470,8 @@ public:
 	Error parse(const String &p_source_code, const String &p_script_path, bool p_for_completion);
 	ClassNode *get_tree() const { return head; }
 	bool is_tool() const { return _is_tool; }
+	ClassNode *find_class(const String &p_qualified_name) const;
+	bool has_class(const GDScriptParser::ClassNode *p_class) const;
 	static Variant::Type get_builtin_type(const StringName &p_type);
 
 	CompletionContext get_completion_context() const { return completion_context; }

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

@@ -1,2 +1,2 @@
 GDTEST_ANALYZER_ERROR
-The member "Vector2" cannot have the same name as a builtin type.
+Class "Vector2" hides a built-in type.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_inheritance.gd

@@ -0,0 +1,8 @@
+func test():
+	print(InnerA.new())
+
+class InnerA extends InnerB:
+	pass
+
+class InnerB extends InnerA:
+	pass

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Cyclic inheritance.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_const.gd

@@ -0,0 +1,5 @@
+func test():
+	print(c1)
+
+const c1 = c2
+const c2 = c1

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "c1": Cyclic reference.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_enum.gd

@@ -0,0 +1,5 @@
+func test():
+	print(E1.V)
+
+enum E1 {V = E2.V}
+enum E2 {V = E1.V}

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "E1": Cyclic reference.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_enum_value.gd

@@ -0,0 +1,5 @@
+func test():
+	print(EV1)
+
+enum {EV1 = EV2}
+enum {EV2 = EV1}

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "EV1": Cyclic reference.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_external.gd

@@ -0,0 +1,6 @@
+func test():
+	print(v)
+
+var v = A.v
+
+const A = preload("cyclic_ref_external_a.notest.gd")

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "v".

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_external_a.notest.gd

@@ -0,0 +1,3 @@
+const B = preload("cyclic_ref_external.gd")
+
+var v = B.v

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_func.gd

@@ -0,0 +1,9 @@
+func test():
+	print(f1())
+	print(f2())
+
+static func f1(p := f2()) -> int:
+	return 1
+
+static func f2(p := f1()) -> int:
+	return 2

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "f1": Cyclic reference.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_override.gd

@@ -0,0 +1,12 @@
+func test():
+	print(v)
+
+var v := InnerA.new().f()
+
+class InnerA:
+	func f(p := InnerB.new().f()) -> int:
+		return 1
+
+class InnerB extends InnerA:
+	func f(p := 1) -> int:
+		return super.f()

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "f": Cyclic reference.

modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_var.gd

@@ -0,0 +1,5 @@
+func test():
+	print(v1)
+
+var v1 := v2
+var v2 := v1

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

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Could not resolve member "v1": Cyclic reference.

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

@@ -0,0 +1,50 @@
+func test():
+	print("v1: ", v1)
+	print("v1 is String: ", v1 is String)
+	print("v2: ", v2)
+	print("v2 is bool: ", v2 is bool)
+	print("c1: ", c1)
+	print("c1 is int: ", c1 is int)
+	print("c2: ", c2)
+	print("c2 is int: ", c2 is int)
+	print("E1.V1: ", E1.V1)
+	print("E1.V2: ", E1.V2)
+	print("E2.V: ", E2.V)
+	print("EV1: ", EV1)
+	print("EV2: ", EV2)
+	print("EV3: ", EV3)
+
+var v1 := InnerA.new().fn()
+
+class InnerA extends InnerAB:
+	func fn(p2 := E1.V2) -> String:
+		return "%s, p2=%s" % [super.fn(), p2]
+
+	class InnerAB:
+		func fn(p1 := c1) -> String:
+			return "p1=%s" % p1
+
+var v2 := f()
+
+func f() -> bool:
+	return true
+
+const c1 := E1.V1
+
+enum E1 {
+	V1 = E2.V + 2,
+	V2 = V1 - 1
+}
+
+enum E2 {V = 2}
+
+const c2 := EV2
+
+enum {
+	EV1 = 42,
+	EV2 = EV3 + 1
+}
+
+enum {
+	EV3 = EV1 + 1
+}

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

@@ -0,0 +1,15 @@
+GDTEST_OK
+v1: p1=4, p2=3
+v1 is String: true
+v2: true
+v2 is bool: true
+c1: 4
+c1 is int: true
+c2: 44
+c2 is int: true
+E1.V1: 4
+E1.V2: 3
+E2.V: 2
+EV1: 42
+EV2: 44
+EV3: 43

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

@@ -0,0 +1,39 @@
+const B = preload("out_of_order_external_a.notest.gd")
+
+func test():
+	print("v1: ", v1)
+	print("v1 is String: ", v1 is String)
+	print("v2: ", v2)
+	print("v2 is bool: ", v2 is bool)
+	print("c1: ", c1)
+	print("c1 is int: ", c1 is int)
+	print("c2: ", c2)
+	print("c2 is int: ", c2 is int)
+	print("E1.V1: ", E1.V1)
+	print("E1.V2: ", E1.V2)
+	print("B.E2.V: ", B.E2.V)
+	print("EV1: ", EV1)
+	print("EV2: ", EV2)
+	print("B.EV3: ", B.EV3)
+
+var v1 := Inner.new().fn()
+
+class Inner extends B.Inner:
+	func fn(p2 := E1.V2) -> String:
+		return "%s, p2=%s" % [super.fn(), p2]
+
+var v2 := B.new().f()
+
+const c1 := E1.V1
+
+enum E1 {
+	V1 = B.E2.V + 2,
+	V2 = V1 - 1
+}
+
+const c2 := EV2
+
+enum {
+	EV1 = 42,
+	EV2 = B.EV3 + 1
+}

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

@@ -0,0 +1,15 @@
+GDTEST_OK
+v1: p1=4, p2=3
+v1 is String: true
+v2: true
+v2 is bool: true
+c1: 4
+c1 is int: true
+c2: 44
+c2 is int: true
+E1.V1: 4
+E1.V2: 3
+B.E2.V: 2
+EV1: 42
+EV2: 44
+B.EV3: 43

modules/gdscript/tests/scripts/analyzer/features/out_of_order_external_a.notest.gd

@@ -0,0 +1,12 @@
+const A = preload("out_of_order_external.gd")
+
+class Inner:
+	func fn(p1 := A.c1) -> String:
+		return "p1=%s" % p1
+
+func f(p := A.c1) -> bool:
+	return p is int
+
+enum E2 {V = 2}
+
+enum {EV3 = A.EV1 + 1}