Skip to content

Commit

Permalink
Merge pull request #69590 from anvilfolk/enums
Browse files Browse the repository at this point in the history
GDScript enum fixes & refactor
  • Loading branch information
akien-mga committed Jan 9, 2023
2 parents 4d4fcd0 + 366ec89 commit 509da86
Show file tree
Hide file tree
Showing 61 changed files with 698 additions and 96 deletions.
207 changes: 137 additions & 70 deletions modules/gdscript/gdscript_analyzer.cpp

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_analyzer.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class GDScriptAnalyzer {

// Helpers.
GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);
GDScriptParser::DataType type_from_metatype(const GDScriptParser::DataType &p_meta_type) const;
static GDScriptParser::DataType type_from_metatype(const GDScriptParser::DataType &p_meta_type);
GDScriptParser::DataType type_from_property(const PropertyInfo &p_property) 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);
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ static bool _have_exact_arguments(const MethodBind *p_method, const Vector<GDScr
}

GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root, bool p_initializer, const GDScriptCodeGenerator::Address &p_index_addr) {
if (p_expression->is_constant) {
if (p_expression->is_constant && !p_expression->get_datatype().is_meta_type) {
return codegen.add_constant(p_expression->reduced_value);
}

Expand Down
19 changes: 13 additions & 6 deletions modules/gdscript/gdscript_editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -995,9 +995,8 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
ERR_FAIL_COND(p_recursion_depth > COMPLETION_RECURSION_LIMIT);

GDScriptParser::DataType base_type = p_base.type;
bool _static = base_type.is_meta_type;

if (_static && base_type.kind != GDScriptParser::DataType::BUILTIN) {
if (base_type.is_meta_type && base_type.kind != GDScriptParser::DataType::BUILTIN && base_type.kind != GDScriptParser::DataType::ENUM) {
ScriptLanguage::CodeCompletionOption option("new", ScriptLanguage::CODE_COMPLETION_KIND_FUNCTION, ScriptLanguage::LOCATION_LOCAL);
option.insert_text += "(";
r_result.insert(option.display, option);
Expand All @@ -1006,15 +1005,15 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
while (!base_type.has_no_type()) {
switch (base_type.kind) {
case GDScriptParser::DataType::CLASS: {
_find_identifiers_in_class(base_type.class_type, p_only_functions, _static, false, r_result, p_recursion_depth + 1);
_find_identifiers_in_class(base_type.class_type, p_only_functions, base_type.is_meta_type, false, r_result, p_recursion_depth + 1);
// This already finds all parent identifiers, so we are done.
base_type = GDScriptParser::DataType();
} break;
case GDScriptParser::DataType::SCRIPT: {
Ref<Script> scr = base_type.script_type;
if (scr.is_valid()) {
if (!p_only_functions) {
if (!_static) {
if (!base_type.is_meta_type) {
List<PropertyInfo> members;
scr->get_script_property_list(&members);
for (const PropertyInfo &E : members) {
Expand Down Expand Up @@ -1090,7 +1089,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
r_result.insert(option.display, option);
}

if (!_static || Engine::get_singleton()->has_singleton(type)) {
if (!base_type.is_meta_type || Engine::get_singleton()->has_singleton(type)) {
List<PropertyInfo> pinfo;
ClassDB::get_property_list(type, &pinfo);
for (const PropertyInfo &E : pinfo) {
Expand All @@ -1107,7 +1106,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
}
}

bool only_static = _static && !Engine::get_singleton()->has_singleton(type);
bool only_static = base_type.is_meta_type && !Engine::get_singleton()->has_singleton(type);

List<MethodInfo> methods;
ClassDB::get_method_list(type, &methods, false, true);
Expand All @@ -1129,6 +1128,7 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
}
return;
} break;
case GDScriptParser::DataType::ENUM:
case GDScriptParser::DataType::BUILTIN: {
Callable::CallError err;
Variant tmp;
Expand Down Expand Up @@ -1156,6 +1156,10 @@ static void _find_identifiers_in_base(const GDScriptCompletionIdentifier &p_base
List<MethodInfo> methods;
tmp.get_method_list(&methods);
for (const MethodInfo &E : methods) {
if (base_type.kind == GDScriptParser::DataType::ENUM && base_type.is_meta_type && !(E.flags & METHOD_FLAG_CONST)) {
// Enum types are static and cannot change, therefore we skip non-const dictionary methods.
continue;
}
ScriptLanguage::CodeCompletionOption option(E.name, ScriptLanguage::CODE_COMPLETION_KIND_FUNCTION);
if (E.arguments.size()) {
option.insert_text += "(";
Expand Down Expand Up @@ -1364,6 +1368,9 @@ static bool _guess_expression_type(GDScriptParser::CompletionContext &p_context,
if (p_expression->is_constant) {
// Already has a value, so just use that.
r_type = _type_from_variant(p_expression->reduced_value);
if (p_expression->get_datatype().kind == GDScriptParser::DataType::ENUM) {
r_type.type = p_expression->get_datatype();
}
found = true;
} else {
switch (p_expression->type) {
Expand Down
23 changes: 14 additions & 9 deletions modules/gdscript/gdscript_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1299,16 +1299,18 @@ GDScriptParser::EnumNode *GDScriptParser::parse_enum() {
EnumNode::Value item;
GDScriptParser::IdentifierNode *identifier = parse_identifier();
#ifdef DEBUG_ENABLED
for (MethodInfo &info : gdscript_funcs) {
if (info.name == identifier->name) {
if (!named) { // Named enum identifiers do not shadow anything since you can only access them with NamedEnum.ENUM_VALUE
for (MethodInfo &info : gdscript_funcs) {
if (info.name == identifier->name) {
push_warning(identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, "enum member", identifier->name, "built-in function");
}
}
if (Variant::has_utility_function(identifier->name)) {
push_warning(identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, "enum member", identifier->name, "built-in function");
} else if (ClassDB::class_exists(identifier->name)) {
push_warning(identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, "enum member", identifier->name, "global class");
}
}
if (Variant::has_utility_function(identifier->name)) {
push_warning(identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, "enum member", identifier->name, "built-in function");
} else if (ClassDB::class_exists(identifier->name)) {
push_warning(identifier, GDScriptWarning::SHADOWED_GLOBAL_IDENTIFIER, "enum member", identifier->name, "global class");
}
#endif
item.identifier = identifier;
item.parent_enum = enum_node;
Expand Down Expand Up @@ -4092,8 +4094,11 @@ String GDScriptParser::DataType::to_string() const {
}
return native_type.operator String();
}
case ENUM:
return enum_type.operator String() + " (enum)";
case ENUM: {
// native_type contains either the native class defining the enum
// or the fully qualified class name of the script defining the enum
return String(native_type).get_file(); // Remove path, keep filename
}
case RESOLVING:
case UNRESOLVED:
return "<unresolved type>";
Expand Down
4 changes: 2 additions & 2 deletions modules/gdscript/gdscript_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ class GDScriptParser {
case BUILTIN:
return builtin_type == p_other.builtin_type;
case NATIVE:
case ENUM:
return native_type == p_other.native_type && enum_type == p_other.enum_type;
case ENUM: // Enums use native_type to identify the enum and its base class.
return native_type == p_other.native_type;
case SCRIPT:
return script_type == p_other.script_type;
case CLASS:
Expand Down
3 changes: 3 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/assign_enum.gd
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
enum { V }
func test():
V = 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a new value to a constant.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
enum NamedEnum { V }
func test():
NamedEnum.V = 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a new value to a constant.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2, OTHER_ENUM_VALUE_3 }

func test():
print(MyOtherEnum.OTHER_ENUM_VALUE_3 as MyEnum)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid cast. Enum "cast_enum_bad_enum.gd::MyEnum" does not have value corresponding to "MyOtherEnum.OTHER_ENUM_VALUE_3" (2).
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }

func test():
print(2 as MyEnum)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid cast. Enum "cast_enum_bad_int.gd::MyEnum" does not have enum value 2.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
enum Enum {V1, V2}

func test():
Enum.clear()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call non-const Dictionary function "clear()" on enum "Enum".
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
enum Enum {V1, V2}

func test():
var bad = Enum.V3
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot find member "V3" in base "enum_bad_value.gd::Enum".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type "MyOtherEnum (enum)" to a target of type "MyEnum (enum)".
Value of type "enum_class_var_assign_with_wrong_enum_type.gd::MyOtherEnum" cannot be assigned to a variable of type "enum_class_var_assign_with_wrong_enum_type.gd::MyEnum".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type MyOtherEnum (enum) to variable "class_var" with specified type MyEnum (enum).
Cannot assign a value of type enum_class_var_init_with_wrong_enum_type.gd::MyOtherEnum to variable "class_var" with specified type enum_class_var_init_with_wrong_enum_type.gd::MyEnum.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
enum Enum {V1, V2}

func test():
var Enum2 = Enum
Enum2.clear()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot call non-const Dictionary function "clear()" on enum "Enum".
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }

func enum_func(e : MyEnum) -> void:
print(e)

func test():
enum_func(MyOtherEnum.OTHER_ENUM_VALUE_1)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid argument for "enum_func()" function: argument 1 should be "enum_function_parameter_wrong_type.gd::MyEnum" but is "enum_function_parameter_wrong_type.gd::MyOtherEnum".
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }

func enum_func() -> MyEnum:
return MyOtherEnum.OTHER_ENUM_VALUE_1

func test():
print(enum_func())
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot return value of type "enum_function_return_wrong_type.gd::MyOtherEnum" because the function return type is "enum_function_return_wrong_type.gd::MyEnum".
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }

class InnerClass:
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }

func test():
var local_var: MyEnum = MyEnum.ENUM_VALUE_1
print(local_var)
local_var = InnerClass.MyEnum.ENUM_VALUE_2
print(local_var)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Value of type "enum_local_var_assign_outer_with_wrong_enum_type.gd::InnerClass::MyEnum" cannot be assigned to a variable of type "enum_local_var_assign_outer_with_wrong_enum_type.gd::MyEnum".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type "MyOtherEnum (enum)" to a target of type "MyEnum (enum)".
Value of type "enum_local_var_assign_with_wrong_enum_type.gd::MyOtherEnum" cannot be assigned to a variable of type "enum_local_var_assign_with_wrong_enum_type.gd::MyEnum".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type MyOtherEnum (enum) to variable "local_var" with specified type MyEnum (enum).
Cannot assign a value of type enum_local_var_init_with_wrong_enum_type.gd::MyOtherEnum to variable "local_var" with specified type enum_local_var_init_with_wrong_enum_type.gd::MyEnum.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
func test():
var _bad = TileSet.TileShape.THIS_DOES_NOT_EXIST
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot find member "THIS_DOES_NOT_EXIST" in base "TileSet::TileShape".
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
enum MyEnum { VALUE_A, VALUE_B, VALUE_C = 42 }
func test():
const P = preload("../features/enum_value_from_parent.gd")
var local_var: MyEnum
local_var = P.VALUE_B
print(local_var)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Value of type "enum_value_from_parent.gd::<anonymous enum>" cannot be assigned to a variable of type "enum_preload_unnamed_assign_to_named.gd::MyEnum".
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class A:
enum { V }

class B extends A:
enum { V }

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
The member "V" already exists in parent class A.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
enum { V }

class InnerClass:
enum { V }

func test():
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_PARSER_ERROR
Name "V" is already used as a class enum value.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
enum { ENUM_VALUE_1, ENUM_VALUE_2 }

enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }

func test():
var local_var: MyEnum = ENUM_VALUE_1
print(local_var)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type enum_unnamed_assign_to_named.gd::<anonymous enum> to variable "local_var" with specified type enum_unnamed_assign_to_named.gd::MyEnum.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
func test():
TileSet.this_does_not_exist # Does not exist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot find member "this_does_not_exist" in base "TileSet".
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
enum LocalNamed { VALUE_A, VALUE_B, VALUE_C = 42 }

func test():
const P = preload("../features/enum_from_outer.gd")
var x : LocalNamed
x = P.Named.VALUE_A
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Value of type "enum_from_outer.gd::Named" cannot be assigned to a variable of type "preload_enum_error.gd::LocalNamed".
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type "String" to a target of type "int".
Value of type "String" cannot be assigned to a variable of type "int".
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
class_name EnumAccessOuterClass

class InnerClass:
enum MyEnum { V0, V2, V1 }

static func print_enums():
print("Inner - Inner")
print(MyEnum.V0, MyEnum.V1, MyEnum.V2)
print(InnerClass.MyEnum.V0, InnerClass.MyEnum.V1, InnerClass.MyEnum.V2)
print(EnumAccessOuterClass.InnerClass.MyEnum.V0, EnumAccessOuterClass.InnerClass.MyEnum.V1, EnumAccessOuterClass.InnerClass.MyEnum.V2)

print("Inner - Outer")
print(EnumAccessOuterClass.MyEnum.V0, EnumAccessOuterClass.MyEnum.V1, EnumAccessOuterClass.MyEnum.V2)


enum MyEnum { V0, V1, V2 }

func print_enums():
print("Outer - Outer")
print(MyEnum.V0, MyEnum.V1, MyEnum.V2)
print(EnumAccessOuterClass.MyEnum.V0, EnumAccessOuterClass.MyEnum.V1, EnumAccessOuterClass.MyEnum.V2)

print("Outer - Inner")
print(InnerClass.MyEnum.V0, InnerClass.MyEnum.V1, InnerClass.MyEnum.V2)
print(EnumAccessOuterClass.InnerClass.MyEnum.V0, EnumAccessOuterClass.InnerClass.MyEnum.V1, EnumAccessOuterClass.InnerClass.MyEnum.V2)

func test():
print_enums()
InnerClass.print_enums()
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
GDTEST_OK
Outer - Outer
012
012
Outer - Inner
021
021
Inner - Inner
021
021
021
Inner - Outer
012
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
enum Enum {V1, V2}

func test():
var enumAsDict : Dictionary = Enum.duplicate()
var enumAsVariant = Enum.duplicate()
print(Enum.has("V1"))
print(enumAsDict.has("V1"))
print(enumAsVariant.has("V1"))
enumAsDict.clear()
enumAsVariant.clear()
print(Enum.has("V1"))
print(enumAsDict.has("V1"))
print(enumAsVariant.has("V1"))
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
GDTEST_OK
true
true
true
true
false
false
Loading

0 comments on commit 509da86

Please sign in to comment.