Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix constants scope in extended or inner GDScript classes #69587

Merged
merged 1 commit into from
Dec 10, 2022
Merged

Fix constants scope in extended or inner GDScript classes #69587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 57 additions & 44 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,22 @@ 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) {
if (p_list->find(p_node) != nullptr) {
return;
}
p_list->push_back(p_node);

// Prioritize node base type over its outer class
adamscott marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure @anvilfolk, this prioritizes the node base type over its outer class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right! And it should prioritize all base classes before even the first outer class.

And then it prioritizes the first outer class's base classes (all the way up) before moving on to first outer class's outer class, and so forth.

It gets pretty confusing, but honestly if somebody is looking this deep into the class hierarchy for names that are reused everywhere, they should just refactor their code, hehehe :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should also add native/builtin classes and everything from what I can tell! :)

if (p_node->base_type.class_type != nullptr) {
get_class_node_current_scope_classes(p_node->base_type.class_type, p_list);
}

if (p_node->outer != nullptr) {
get_class_node_current_scope_classes(p_node->outer, p_list);
}
}

Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive) {
if (p_class->base_type.is_set()) {
// Already resolved
Expand Down Expand Up @@ -327,9 +343,10 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
base.native_type = name;
} else {
// Look for other classes in script.
GDScriptParser::ClassNode *look_class = p_class;
bool found = false;
while (look_class != nullptr) {
List<GDScriptParser::ClassNode *> script_classes;
get_class_node_current_scope_classes(p_class, &script_classes);
for (GDScriptParser::ClassNode *look_class : script_classes) {
if (look_class->identifier && look_class->identifier->name == name) {
if (!look_class->get_datatype().is_set()) {
Error err = resolve_inheritance(look_class, false);
Expand All @@ -353,7 +370,6 @@ Error GDScriptAnalyzer::resolve_inheritance(GDScriptParser::ClassNode *p_class,
found = true;
break;
}
look_class = look_class->outer;
}

if (!found) {
Expand Down Expand Up @@ -517,30 +533,26 @@ GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(GDScriptParser::Type
result = make_native_enum_type(parser->current_class->base_type.native_type, first);
} else {
// Classes in current scope.
GDScriptParser::ClassNode *script_class = parser->current_class;
bool found = false;
while (!found && script_class != nullptr) {
List<GDScriptParser::ClassNode *> script_classes;
get_class_node_current_scope_classes(parser->current_class, &script_classes);
for (GDScriptParser::ClassNode *script_class : script_classes) {
if (script_class->identifier && script_class->identifier->name == first) {
result = script_class->get_datatype();
found = true;
break;
}
if (script_class->members_indices.has(first)) {
GDScriptParser::ClassNode::Member member = script_class->members[script_class->members_indices[first]];
switch (member.type) {
case GDScriptParser::ClassNode::Member::CLASS:
result = member.m_class->get_datatype();
found = true;
break;
case GDScriptParser::ClassNode::Member::ENUM:
result = member.m_enum->get_datatype();
found = true;
break;
case GDScriptParser::ClassNode::Member::CONSTANT:
if (member.constant->get_datatype().is_meta_type) {
result = member.constant->get_datatype();
result.is_meta_type = false;
found = true;
break;
} else if (Ref<Script>(member.constant->initializer->reduced_value).is_valid()) {
Ref<GDScript> gdscript = member.constant->initializer->reduced_value;
Expand Down Expand Up @@ -569,7 +581,6 @@ GDScriptParser::DataType GDScriptAnalyzer::resolve_datatype(GDScriptParser::Type
return GDScriptParser::DataType();
}
}
script_class = script_class->outer;
}
}
if (!result.is_set()) {
Expand Down Expand Up @@ -2891,41 +2902,43 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
}
// Check outer constants.
// TODO: Allow outer static functions.
GDScriptParser::ClassNode *outer = base_class->outer;
while (outer != nullptr) {
if (outer->has_member(name)) {
const GDScriptParser::ClassNode::Member &member = outer->get_member(name);
switch (member.type) {
case GDScriptParser::ClassNode::Member::CONSTANT: {
// TODO: Make sure loops won't cause problem. And make special error message for those.
// For out-of-order resolution:
reduce_expression(member.constant->initializer);
p_identifier->set_datatype(member.get_datatype());
p_identifier->is_constant = true;
p_identifier->reduced_value = member.constant->initializer->reduced_value;
return;
} break;
case GDScriptParser::ClassNode::Member::ENUM_VALUE: {
p_identifier->set_datatype(member.get_datatype());
p_identifier->is_constant = true;
p_identifier->reduced_value = member.enum_value.value;
return;
} break;
case GDScriptParser::ClassNode::Member::ENUM: {
p_identifier->set_datatype(member.get_datatype());
p_identifier->is_constant = false;
return;
} break;
case GDScriptParser::ClassNode::Member::CLASS: {
resolve_class_interface(member.m_class);
p_identifier->set_datatype(member.m_class->get_datatype());
return;
} break;
default:
break;
if (base_class->outer != nullptr) {
List<GDScriptParser::ClassNode *> script_classes;
get_class_node_current_scope_classes(parser->current_class, &script_classes);
for (GDScriptParser::ClassNode *script_class : script_classes) {
if (script_class->has_member(name)) {
const GDScriptParser::ClassNode::Member &member = script_class->get_member(name);
switch (member.type) {
case GDScriptParser::ClassNode::Member::CONSTANT: {
// TODO: Make sure loops won't cause problem. And make special error message for those.
// For out-of-order resolution:
reduce_expression(member.constant->initializer);
p_identifier->set_datatype(member.get_datatype());
p_identifier->is_constant = true;
p_identifier->reduced_value = member.constant->initializer->reduced_value;
return;
} break;
case GDScriptParser::ClassNode::Member::ENUM_VALUE: {
p_identifier->set_datatype(member.get_datatype());
p_identifier->is_constant = true;
p_identifier->reduced_value = member.enum_value.value;
return;
} break;
case GDScriptParser::ClassNode::Member::ENUM: {
p_identifier->set_datatype(member.get_datatype());
p_identifier->is_constant = false;
return;
} break;
case GDScriptParser::ClassNode::Member::CLASS: {
resolve_class_interface(member.m_class);
p_identifier->set_datatype(member.m_class->get_datatype());
return;
} break;
default:
break;
}
}
}
outer = outer->outer;
}

base_class = base_class->base_type.class_type;
Expand Down
2 changes: 2 additions & 0 deletions modules/gdscript/gdscript_analyzer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ 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);

Error resolve_inheritance(GDScriptParser::ClassNode *p_class, bool p_recursive = true);
GDScriptParser::DataType resolve_datatype(GDScriptParser::TypeNode *p_type);

Expand Down
14 changes: 14 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const A: = preload("base_outer_resolution_a.notest.gd")
const B: = preload("base_outer_resolution_b.notest.gd")
const C: = preload("base_outer_resolution_c.notest.gd")

const Extend: = preload("base_outer_resolution_extend.notest.gd")

func test() -> void:
Extend.test_a(A.new())
Extend.test_b(B.new())
Extend.InnerClass.test_c(C.new())
Extend.InnerClass.InnerInnerClass.test_a_b_c(A.new(), B.new(), C.new())
Extend.InnerClass.InnerInnerClass.test_enum(C.TestEnum.HELLO_WORLD)
Extend.InnerClass.InnerInnerClass.test_a_prime(A.APrime.new())

7 changes: 7 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
GDTEST_OK
true
true
true
true
true
true
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution_a.notest.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class APrime:
pass
Empty file added modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution_b.notest.gd
View file
Open in desktop
Empty file.
4 changes: 4 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution_base.notest.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const A: = preload("base_outer_resolution_a.notest.gd")

class InnerClassInBase:
const C: = preload("base_outer_resolution_c.notest.gd")
3 changes: 3 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution_c.notest.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
enum TestEnum {
HELLO_WORLD
}
23 changes: 23 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/base_outer_resolution_extend.notest.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
extends "base_outer_resolution_base.notest.gd"

const B: = preload("base_outer_resolution_b.notest.gd")

static func test_a(a: A) -> void:
print(a is A)

static func test_b(b: B) -> void:
print(b is B)

class InnerClass extends InnerClassInBase:
static func test_c(c: C) -> void:
print(c is C)

class InnerInnerClass:
static func test_a_b_c(a: A, b: B, c: C) -> void:
print(a is A and b is B and c is C)

static func test_enum(test_enum: C.TestEnum) -> void:
print(test_enum == C.TestEnum.HELLO_WORLD)

static func test_a_prime(a_prime: A.APrime) -> void:
print(a_prime is A.APrime)