godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #94871 from rune-scape/fix-mismatched-parsers2 

GDScript: Fix common mismatched external parser errors (second try)
This commit is contained in:
Rémi Verschelde 2024-07-31 22:33:03 +02:00
parent 9559330187 6e8fa6dd50
commit 8460a72f94
No known key found for this signature in database
GPG Key ID: C3336907360768E1
15 changed files with 225 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

198
modules/gdscript/gdscript_analyzer.cpp
View File

@ -285,7 +285,7 @@ Error GDScriptAnalyzer::check_class_member_name_conflict(const GDScriptParser::C
return OK;
}
void GDScriptAnalyzer::get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list) {
void GDScriptAnalyzer::get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list, GDScriptParser::Node *p_source) {
ERR_FAIL_NULL(p_node);
ERR_FAIL_NULL(p_list);
@ -299,11 +299,13 @@ void GDScriptAnalyzer::get_class_node_current_scope_classes(GDScriptParser::Clas
// Prioritize node base type over its outer class
if (p_node->base_type.class_type != nullptr) {
get_class_node_current_scope_classes(p_node->base_type.class_type, p_list);
ensure_cached_parser_for_class(p_node->base_type.class_type, p_node, vformat(R"(Trying to fetch classes in the current scope: base class of "%s")", p_node->fqcn), p_source);
get_class_node_current_scope_classes(p_node->base_type.class_type, p_list, p_source);
}
if (p_node->outer != nullptr) {
get_class_node_current_scope_classes(p_node->outer, p_list);
ensure_cached_parser_for_class(p_node->outer, p_node, vformat(R"(Trying to fetch classes in the current scope: outer class of "%s")", p_node->fqcn), p_source);
get_class_node_current_scope_classes(p_node->outer, p_list, p_source);
}
}
@ -312,6 +314,13 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c
p_source = p_class;
}
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class inheritance of "%s")", p_class->fqcn), p_source);
Finally finally([&]() {
for (GDScriptParser::ClassNode *look_class = p_class; look_class != nullptr; look_class = look_class->base_type.class_type) {
ensure_cached_parser_for_class(look_class->base_type.class_type, look_class, vformat(R"(Trying to resolve class inheritance of "%s")", p_class->fqcn), p_source);
}
});
if (p_class->base_type.is_resolving()) {
push_error(vformat(R"(Could not resolve class "%s": Cyclic reference.)", type_from_metatype(p_class->get_datatype()).to_string()), p_source);
return ERR_PARSE_ERROR;
@ -323,21 +332,17 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c
}
if (!parser->has_class(p_class)) {
String script_path = p_class->get_datatype().script_path;
Ref<GDScriptParserRef> parser_ref = parser->get_depended_parser_for(script_path);
if (parser_ref.is_null()) {
push_error(vformat(R"(Could not find script "%s".)", script_path), p_source);
// Error already pushed.
return ERR_PARSE_ERROR;
}
Error err = parser_ref->raise_status(GDScriptParserRef::PARSED);
if (err) {
push_error(vformat(R"(Could not parse script "%s": %s.)", script_path, error_names[err]), p_source);
push_error(vformat(R"(Could not parse script "%s": %s.)", p_class->get_datatype().script_path, error_names[err]), p_source);
return ERR_PARSE_ERROR;
}
ERR_FAIL_COND_V_MSG(!parser_ref->get_parser()->has_class(p_class), ERR_PARSE_ERROR, R"(Parser bug: Mismatched external parser.)");
GDScriptAnalyzer *other_analyzer = parser_ref->get_analyzer();
GDScriptParser *other_parser = parser_ref->get_parser();
@ -471,7 +476,7 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c
// Look for other classes in script.
bool found = false;
List<GDScriptParser::ClassNode *> script_classes;
get_class_node_current_scope_classes(p_class, &script_classes);
get_class_node_current_scope_classes(p_class, &script_classes, id);
for (GDScriptParser::ClassNode *look_class : script_classes) {
if (look_class->identifier && look_class->identifier->name == name) {
if (!look_class->get_datatype().is_set()) {
@ -763,7 +768,7 @@ GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(GDScriptParser::Type
// Classes in current scope.
List<GDScriptParser::ClassNode *> script_classes;
bool found = false;
get_class_node_current_scope_classes(parser->current_class, &script_classes);
get_class_node_current_scope_classes(parser->current_class, &script_classes, p_type);
for (GDScriptParser::ClassNode *script_class : script_classes) {
if (found) {
break;
@ -883,6 +888,15 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
p_source = member.get_source_node();
}
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class member "%s" of "%s")", member.get_name(), p_class->fqcn), p_source);
Finally finally([&]() {
ensure_cached_parser_for_class(member.get_datatype().class_type, p_class, vformat(R"(Trying to resolve datatype of class member "%s" of "%s")", member.get_name(), p_class->fqcn), p_source);
GDScriptParser::DataType member_type = member.get_datatype();
if (member_type.has_container_element_type(0)) {
ensure_cached_parser_for_class(member_type.get_container_element_type(0).class_type, p_class, vformat(R"(Trying to resolve datatype of class member "%s" of "%s")", member.get_name(), p_class->fqcn), p_source);
}
});
if (member.get_datatype().is_resolving()) {
push_error(vformat(R"(Could not resolve member "%s": Cyclic reference.)", member.get_name()), p_source);
return;
@ -892,42 +906,39 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
return;
}
// If it's already resolving, that's ok.
if (!p_class->base_type.is_resolving()) {
Error err = resolve_class_inheritance(p_class);
if (err) {
return;
}
}
if (!parser->has_class(p_class)) {
String script_path = p_class->get_datatype().script_path;
Ref<GDScriptParserRef> parser_ref = parser->get_depended_parser_for(script_path);
if (parser_ref.is_null()) {
push_error(vformat(R"(Could not find script "%s" (While resolving "%s").)", script_path, member.get_name()), p_source);
// Error already pushed.
return;
}
Error err = parser_ref->raise_status(GDScriptParserRef::PARSED);
if (err) {
push_error(vformat(R"(Could not resolve script "%s": %s (While resolving "%s").)", script_path, error_names[err], member.get_name()), p_source);
push_error(vformat(R"(Could not parse script "%s": %s (While resolving external class member "%s").)", p_class->get_datatype().script_path, error_names[err], member.get_name()), p_source);
return;
}
ERR_FAIL_COND_MSG(!parser_ref->get_parser()->has_class(p_class), R"(Parser bug: Mismatched external parser.)");
GDScriptAnalyzer *other_analyzer = parser_ref->get_analyzer();
GDScriptParser *other_parser = parser_ref->get_parser();
int error_count = other_parser->errors.size();
other_analyzer->resolve_class_member(p_class, p_index);
if (other_parser->errors.size() > error_count) {
push_error(vformat(R"(Could not resolve member "%s".)", member.get_name()), p_source);
push_error(vformat(R"(Could not resolve external class member "%s".)", member.get_name()), p_source);
return;
}
return;
}
// If it's already resolving, that's ok.
if (!p_class->base_type.is_resolving()) {
Error err = resolve_class_inheritance(p_class);
if (err) {
return;
}
}
GDScriptParser::ClassNode *previous_class = parser->current_class;
parser->current_class = p_class;
@ -1170,27 +1181,25 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
p_source = p_class;
}
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class interface of "%s")", p_class->fqcn), p_source);
if (!p_class->resolved_interface) {
#ifdef DEBUG_ENABLED
bool has_static_data = p_class->has_static_data;
#endif
if (!parser->has_class(p_class)) {
String script_path = p_class->get_datatype().script_path;
Ref<GDScriptParserRef> parser_ref = parser->get_depended_parser_for(script_path);
if (parser_ref.is_null()) {
push_error(vformat(R"(Could not find script "%s".)", script_path), p_source);
// Error already pushed.
return;
}
Error err = parser_ref->raise_status(GDScriptParserRef::PARSED);
if (err) {
push_error(vformat(R"(Could not resolve script "%s": %s.)", script_path, error_names[err]), p_source);
push_error(vformat(R"(Could not parse script "%s": %s.)", p_class->get_datatype().script_path, error_names[err]), p_source);
return;
}
ERR_FAIL_COND_MSG(!parser_ref->get_parser()->has_class(p_class), R"(Parser bug: Mismatched external parser.)");
GDScriptAnalyzer *other_analyzer = parser_ref->get_analyzer();
GDScriptParser *other_parser = parser_ref->get_parser();
@ -1198,6 +1207,7 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
other_analyzer->resolve_class_interface(p_class);
if (other_parser->errors.size() > error_count) {
push_error(vformat(R"(Could not resolve class "%s".)", p_class->fqcn), p_source);
return;
}
return;
@ -1261,26 +1271,24 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
p_source = p_class;
}
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class body of "%s")", p_class->fqcn), p_source);
if (p_class->resolved_body) {
return;
}
if (!parser->has_class(p_class)) {
String script_path = p_class->get_datatype().script_path;
Ref<GDScriptParserRef> parser_ref = parser->get_depended_parser_for(script_path);
if (parser_ref.is_null()) {
push_error(vformat(R"(Could not find script "%s".)", script_path), p_source);
// Error already pushed.
return;
}
Error err = parser_ref->raise_status(GDScriptParserRef::PARSED);
if (err) {
push_error(vformat(R"(Could not resolve script "%s": %s.)", script_path, error_names[err]), p_source);
push_error(vformat(R"(Could not parse script "%s": %s.)", p_class->get_datatype().script_path, error_names[err]), p_source);
return;
}
ERR_FAIL_COND_MSG(!parser_ref->get_parser()->has_class(p_class), R"(Parser bug: Mismatched external parser.)");
GDScriptAnalyzer *other_analyzer = parser_ref->get_analyzer();
GDScriptParser *other_parser = parser_ref->get_parser();
@ -1288,6 +1296,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
other_analyzer->resolve_class_body(p_class);
if (other_parser->errors.size() > error_count) {
push_error(vformat(R"(Could not resolve class "%s".)", p_class->fqcn), p_source);
return;
}
return;
@ -3645,12 +3654,116 @@ GDScriptParser::DataType GDScriptAnalyzer::make_global_class_meta_type(const Str
}
}
Ref<GDScriptParserRef> GDScriptAnalyzer::ensure_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const String &p_context, const GDScriptParser::Node *p_source) {
if (p_class == nullptr) {
return nullptr;
}
if (parser->has_class(p_class)) {
return nullptr;
}
if (HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>>::Iterator E = external_class_parser_cache.find(p_class)) {
return E->value;
}
if (p_from_class == nullptr) {
p_from_class = parser->head;
}
String script_path = p_class->get_datatype().script_path;
Ref<GDScriptParserRef> parser_ref;
for (const GDScriptParser::ClassNode *look_class = p_from_class; look_class != nullptr; look_class = look_class->base_type.class_type) {
if (parser->has_class(look_class)) {
parser_ref = find_cached_parser_for_class(p_class, parser);
if (parser_ref.is_valid()) {
break;
}
}
if (HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>>::Iterator E = external_class_parser_cache.find(look_class)) {
parser_ref = find_cached_parser_for_class(p_class, E->value);
if (parser_ref.is_valid()) {
break;
}
}
String look_class_script_path = look_class->get_datatype().script_path;
if (HashMap<String, Ref<GDScriptParserRef>>::Iterator E = parser->depended_parsers.find(look_class_script_path)) {
parser_ref = find_cached_parser_for_class(p_class, E->value);
if (parser_ref.is_valid()) {
break;
}
}
}
if (parser_ref.is_null()) {
push_error(vformat(R"(Parser bug: Could not find external parser. (%s))", p_context), p_source);
// A null parser will be inserted into the cache and this error won't spam for the same class.
// This is ok, the values of external_class_parser_cache are not assumed to be valid references.
}
external_class_parser_cache.insert(p_class, parser_ref);
return parser_ref;
}
Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser) {
if (p_dependant_parser.is_null()) {
return nullptr;
}
if (HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>>::Iterator E = p_dependant_parser->get_analyzer()->external_class_parser_cache.find(p_class)) {
if (E->value.is_valid()) {
// Silently ensure it's parsed.
E->value->raise_status(GDScriptParserRef::PARSED);
if (E->value->get_parser()->has_class(p_class)) {
return E->value;
}
}
}
if (p_dependant_parser->get_parser()->has_class(p_class)) {
return p_dependant_parser;
}
// Silently ensure it's parsed.
p_dependant_parser->raise_status(GDScriptParserRef::PARSED);
return find_cached_parser_for_class(p_class, p_dependant_parser->get_parser());
}
Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser) {
if (p_dependant_parser == nullptr) {
return nullptr;
}
String script_path = p_class->get_datatype().script_path;
if (HashMap<String, Ref<GDScriptParserRef>>::Iterator E = p_dependant_parser->depended_parsers.find(script_path)) {
if (E->value.is_valid()) {
// Silently ensure it's parsed.
E->value->raise_status(GDScriptParserRef::PARSED);
if (E->value->get_parser()->has_class(p_class)) {
return E->value;
}
}
}
return nullptr;
}
Ref<GDScript> GDScriptAnalyzer::get_depended_shallow_script(const String &p_path, Error &r_error) {
// To keep a local cache of the parser for resolving external nodes later.
parser->get_depended_parser_for(p_path);
Ref<GDScript> scr = GDScriptCache::get_shallow_script(p_path, r_error, parser->script_path);
return scr;
}
void GDScriptAnalyzer::reduce_identifier_from_base_set_class(GDScriptParser::IdentifierNode *p_identifier, GDScriptParser::DataType p_identifier_datatype) {
ERR_FAIL_NULL(p_identifier);
p_identifier->set_datatype(p_identifier_datatype);
Error err = OK;
Ref<GDScript> scr = GDScriptCache::get_shallow_script(p_identifier_datatype.script_path, err, parser->script_path);
Ref<GDScript> scr = get_depended_shallow_script(p_identifier_datatype.script_path, err);
if (err) {
push_error(vformat(R"(Error while getting cache for script "%s".)", p_identifier_datatype.script_path), p_identifier);
return;
@ -3767,7 +3880,7 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
bool is_base = true;
if (base_class != nullptr) {
get_class_node_current_scope_classes(base_class, &script_classes);
get_class_node_current_scope_classes(base_class, &script_classes, p_identifier);
}
bool is_constructor = base.is_meta_type && p_identifier->name == SNAME("new");
@ -4340,7 +4453,7 @@ void GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode *p_preload) {
const String &res_type = ResourceLoader::get_resource_type(p_preload->resolved_path);
if (res_type == "GDScript") {
Error err = OK;
Ref<GDScript> res = GDScriptCache::get_shallow_script(p_preload->resolved_path, err, parser->script_path);
Ref<GDScript> res = get_depended_shallow_script(p_preload->resolved_path, err);
p_preload->resource = res;
if (err != OK) {
push_error(vformat(R"(Could not preload resource script "%s".)", p_preload->resolved_path), p_preload->path);
@ -4361,6 +4474,7 @@ void GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode *p_preload) {
p_preload->is_constant = true;
p_preload->reduced_value = p_preload->resource;
p_preload->set_datatype(type_from_variant(p_preload->reduced_value, p_preload));
ensure_cached_parser_for_class(p_preload->get_datatype().class_type, nullptr, vformat(R"(Trying to resolve preload of "%s")", p_preload->resolved_path), p_preload);
}
void GDScriptAnalyzer::reduce_self(GDScriptParser::SelfNode *p_self) {
@ -4916,7 +5030,7 @@ Array GDScriptAnalyzer::make_array_from_element_datatype(const GDScriptParser::D
Ref<Script> script_type = p_element_datatype.script_type;
if (p_element_datatype.kind == GDScriptParser::DataType::CLASS && script_type.is_null()) {
Error err = OK;
Ref<GDScript> scr = GDScriptCache::get_shallow_script(p_element_datatype.script_path, err, parser->script_path);
Ref<GDScript> scr = get_depended_shallow_script(p_element_datatype.script_path, err);
if (err) {
push_error(vformat(R"(Error while getting cache for script "%s".)", p_element_datatype.script_path), p_source_node);
return array;

19
modules/gdscript/gdscript_analyzer.h
View File

@ -41,9 +41,22 @@
class GDScriptAnalyzer {
GDScriptParser *parser = nullptr;
template <typename Fn>
class Finally {
Fn fn;
public:
Finally(Fn p_fn) :
fn(p_fn) {}
~Finally() {
fn();
}
};
const GDScriptParser::EnumNode *current_enum = nullptr;
GDScriptParser::LambdaNode *current_lambda = nullptr;
List<GDScriptParser::LambdaNode *> pending_body_resolution_lambdas;
HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>> external_class_parser_cache;
bool static_context = false;
// Tests for detecting invalid overloading of script members
@ -52,7 +65,7 @@ class GDScriptAnalyzer {
Error check_native_member_name_conflict(const StringName &p_member_name, const GDScriptParser::Node *p_member_node, const StringName &p_native_type_string);
Error check_class_member_name_conflict(const GDScriptParser::ClassNode *p_class_node, const StringName &p_member_name, const GDScriptParser::Node *p_member_node);
void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list);
void get_class_node_current_scope_classes(GDScriptParser::ClassNode *p_node, List<GDScriptParser::ClassNode *> *p_list, GDScriptParser::Node *p_source);
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);
@ -132,6 +145,10 @@ class GDScriptAnalyzer {
void resolve_pending_lambda_bodies();
bool class_exists(const StringName &p_class) const;
void reduce_identifier_from_base_set_class(GDScriptParser::IdentifierNode *p_identifier, GDScriptParser::DataType p_identifier_datatype);
Ref<GDScriptParserRef> ensure_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const String &p_context, const GDScriptParser::Node *p_source);
Ref<GDScriptParserRef> find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser);
Ref<GDScriptParserRef> find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser);
Ref<GDScript> get_depended_shallow_script(const String &p_path, Error &r_error);
#ifdef DEBUG_ENABLED
void is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope);
#endif

2
modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_external.out
View File

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

17
modules/gdscript/tests/scripts/analyzer/features/external_parser.gd Normal file
View File

@ -0,0 +1,17 @@
extends "external_parser_base1.notest.gd"
const External1 = preload("external_parser_script1.notest.gd")
func baz(e1: External1) -> void:
print(e1.e1c.bar)
print(e1.baz)
func test_external_base_parser_type_resolve(_v: TypeFromBase):
pass
func test():
var ext := External1.new()
print(ext.array[0].test2)
print(ext.get_external2().get_external3().test3)
# TODO: This actually produces a runtime error, but we're testing the analyzer here
#baz(ext)

3
modules/gdscript/tests/scripts/analyzer/features/external_parser.out Normal file
View File

@ -0,0 +1,3 @@
GDTEST_OK
test2
test3

1
modules/gdscript/tests/scripts/analyzer/features/external_parser_base1.notest.gd Normal file
View File

@ -0,0 +1 @@
extends "external_parser_base2.notest.gd"

2
modules/gdscript/tests/scripts/analyzer/features/external_parser_base2.notest.gd Normal file
View File

@ -0,0 +1,2 @@
class TypeFromBase:
pass

12
modules/gdscript/tests/scripts/analyzer/features/external_parser_script1.notest.gd Normal file
View File

@ -0,0 +1,12 @@
extends "external_parser_script1_base.notest.gd"
const External2 = preload("external_parser_script2.notest.gd")
const External1c = preload("external_parser_script1c.notest.gd")
@export var e1c: External1c
var array: Array[External2] = [ External2.new() ]
var baz: int
func get_external2() -> External2:
return External2.new()

1
modules/gdscript/tests/scripts/analyzer/features/external_parser_script1_base.notest.gd Normal file
View File

@ -0,0 +1 @@
extends Resource

1
modules/gdscript/tests/scripts/analyzer/features/external_parser_script1c.notest.gd Normal file
View File

@ -0,0 +1 @@
extends "external_parser_script1d.notest.gd"

5
modules/gdscript/tests/scripts/analyzer/features/external_parser_script1d.notest.gd Normal file
View File

@ -0,0 +1,5 @@
extends "external_parser_script1f.notest.gd"
const External1e = preload("external_parser_script1e.notest.gd")
var bar: Array[External1e]

0
modules/gdscript/tests/scripts/analyzer/features/external_parser_script1e.notest.gd Normal file
View File

1
modules/gdscript/tests/scripts/analyzer/features/external_parser_script1f.notest.gd Normal file
View File

@ -0,0 +1 @@
extends Resource

6
modules/gdscript/tests/scripts/analyzer/features/external_parser_script2.notest.gd Normal file
View File

@ -0,0 +1,6 @@
const External3 = preload("external_parser_script3.notest.gd")
var test2 := "test2"
func get_external3() -> External3:
return External3.new()

1
modules/gdscript/tests/scripts/analyzer/features/external_parser_script3.notest.gd Normal file
View File

@ -0,0 +1 @@
var test3 := "test3"