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

GDScript: Consolidate behavior for assigning enum types 

This makes sure that assigning values to enum-typed variables are
consistent. Same enum is always valid, different enum is always
invalid (without casting) and assigning `int` creates a warning
if there is no casting.

There are new test cases to ensure this behavior doesn't break in
the future.
This commit is contained in:
George Marques 2022-01-27 11:34:33 -03:00
parent 82efb1d262
commit ad6e2e82a9
No known key found for this signature in database
GPG Key ID: 046BD46A3201E43D
27 changed files with 185 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
doc/classes/ProjectSettings.xml
View File

@ -356,6 +356,8 @@
<member name="debug/gdscript/warnings/incompatible_ternary" type="bool" setter="" getter="" default="true">
If [code]true[/code], enables warnings when a ternary operator may emit values with incompatible types.
</member>
<member name="debug/gdscript/warnings/int_assigned_to_enum" type="bool" setter="" getter="" default="true">
</member>
<member name="debug/gdscript/warnings/integer_division" type="bool" setter="" getter="" default="true">
If [code]true[/code], enables warnings when dividing an integer by another integer (the decimal part will be discarded).
</member>

46
modules/gdscript/gdscript_analyzer.cpp
View File

@ -650,9 +650,9 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
datatype = specified_type;
if (member.variable->initializer != nullptr) {
if (!is_type_compatible(datatype, member.variable->initializer->get_datatype(), true)) {
if (!is_type_compatible(datatype, member.variable->initializer->get_datatype(), true, member.variable->initializer)) {
// Try reverse test since it can be a masked subtype.
if (!is_type_compatible(member.variable->initializer->get_datatype(), datatype, true)) {
if (!is_type_compatible(member.variable->initializer->get_datatype(), datatype, true, member.variable->initializer)) {
push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", member.variable->initializer->get_datatype().to_string(), datatype.to_string()), member.variable->initializer);
} else {
// TODO: Add warning.
@ -1400,9 +1400,9 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
type.is_meta_type = false;
if (p_variable->initializer != nullptr) {
if (!is_type_compatible(type, p_variable->initializer->get_datatype(), true)) {
if (!is_type_compatible(type, p_variable->initializer->get_datatype(), true, p_variable->initializer)) {
// Try reverse test since it can be a masked subtype.
if (!is_type_compatible(p_variable->initializer->get_datatype(), type, true)) {
if (!is_type_compatible(p_variable->initializer->get_datatype(), type, true, p_variable->initializer)) {
push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", p_variable->initializer->get_datatype().to_string(), type.to_string()), p_variable->initializer);
} else {
// TODO: Add warning.
@ -1877,11 +1877,11 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
if (!assignee_type.is_variant() && assigned_value_type.is_hard_type()) {
if (compatible) {
compatible = is_type_compatible(assignee_type, op_type, true);
compatible = is_type_compatible(assignee_type, op_type, true, p_assignment->assigned_value);
if (!compatible) {
if (assignee_type.is_hard_type()) {
// Try reverse test since it can be a masked subtype.
if (!is_type_compatible(op_type, assignee_type, true)) {
if (!is_type_compatible(op_type, assignee_type, true, p_assignment->assigned_value)) {
push_error(vformat(R"(Cannot assign a value of type "%s" to a target of type "%s".)", assigned_value_type.to_string(), assignee_type.to_string()), p_assignment->assigned_value);
} else {
// TODO: Add warning.
@ -2474,6 +2474,7 @@ void GDScriptAnalyzer::reduce_cast(GDScriptParser::CastNode *p_cast) {
GDScriptParser::DataType cast_type = resolve_datatype(p_cast->cast_type);
if (!cast_type.is_set()) {
mark_node_unsafe(p_cast);
return;
}
@ -2484,7 +2485,13 @@ void GDScriptAnalyzer::reduce_cast(GDScriptParser::CastNode *p_cast) {
GDScriptParser::DataType op_type = p_cast->operand->get_datatype();
if (!op_type.is_variant()) {
bool valid = false;
if (op_type.kind == GDScriptParser::DataType::BUILTIN && cast_type.kind == GDScriptParser::DataType::BUILTIN) {
if (op_type.kind == GDScriptParser::DataType::ENUM && cast_type.kind == GDScriptParser::DataType::ENUM) {
// Enum types are compatible between each other, so it's a safe cast.
valid = true;
} else if (op_type.kind == GDScriptParser::DataType::BUILTIN && op_type.builtin_type == Variant::INT && cast_type.kind == GDScriptParser::DataType::ENUM) {
// Convertint int to enum is always valid.
valid = true;
} else if (op_type.kind == GDScriptParser::DataType::BUILTIN && cast_type.kind == GDScriptParser::DataType::BUILTIN) {
valid = Variant::can_convert(op_type.builtin_type, cast_type.builtin_type);
} else if (op_type.kind != GDScriptParser::DataType::BUILTIN && cast_type.kind != GDScriptParser::DataType::BUILTIN) {
valid = is_type_compatible(cast_type, op_type) || is_type_compatible(op_type, cast_type);
@ -2640,15 +2647,16 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
GDScriptParser::DataType result;
result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
result.kind = GDScriptParser::DataType::ENUM_VALUE;
result.kind = GDScriptParser::DataType::ENUM;
result.is_constant = true;
result.builtin_type = Variant::INT;
result.native_type = base.native_type;
result.enum_type = name;
result.enum_type = base.enum_type;
p_identifier->set_datatype(result);
} else {
// Consider as a Dictionary
// Consider as a Dictionary, so it can be anything.
GDScriptParser::DataType dummy;
dummy.type_source = GDScriptParser::DataType::UNDETECTED;
dummy.kind = GDScriptParser::DataType::VARIANT;
p_identifier->set_datatype(dummy);
}
@ -2793,7 +2801,7 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
if (element.identifier->name == p_identifier->name) {
GDScriptParser::DataType type;
type.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
type.kind = element.parent_enum->identifier ? GDScriptParser::DataType::ENUM_VALUE : GDScriptParser::DataType::BUILTIN;
type.kind = element.parent_enum->identifier ? GDScriptParser::DataType::ENUM : GDScriptParser::DataType::BUILTIN;
type.builtin_type = Variant::INT;
type.is_constant = true;
if (element.parent_enum->identifier) {
@ -3828,7 +3836,7 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
}
// TODO: Add safe/unsafe return variable (for variant cases)
bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion) const {
bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion, const GDScriptParser::Node *p_source_node) {
// These return "true" so it doesn't affect users negatively.
ERR_FAIL_COND_V_MSG(!p_target.is_set(), true, "Parser bug (please report): Trying to check compatibility of unset target type");
ERR_FAIL_COND_V_MSG(!p_source.is_set(), true, "Parser bug (please report): Trying to check compatibility of unset value type");
@ -3848,7 +3856,7 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
if (!valid && p_allow_implicit_conversion) {
valid = Variant::can_convert_strict(p_source.builtin_type, p_target.builtin_type);
}
if (!valid && p_target.builtin_type == Variant::INT && p_source.kind == GDScriptParser::DataType::ENUM_VALUE) {
if (!valid && p_target.builtin_type == Variant::INT && p_source.kind == GDScriptParser::DataType::ENUM && !p_source.is_meta_type) {
// Enum value is also integer.
valid = true;
}
@ -3869,6 +3877,11 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
if (p_target.kind == GDScriptParser::DataType::ENUM) {
if (p_source.kind == GDScriptParser::DataType::BUILTIN && p_source.builtin_type == Variant::INT) {
#ifdef DEBUG_ENABLED
if (p_source_node) {
parser->push_warning(p_source_node, GDScriptWarning::INT_ASSIGNED_TO_ENUM);
}
#endif
return true;
}
if (p_source.kind == GDScriptParser::DataType::ENUM) {
@ -3876,11 +3889,6 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
return true;
}
}
if (p_source.kind == GDScriptParser::DataType::ENUM_VALUE) {
if (p_source.native_type == p_target.native_type && p_target.enum_values.has(p_source.enum_type)) {
return true;
}
}
return false;
}
@ -3935,7 +3943,6 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
case GDScriptParser::DataType::VARIANT:
case GDScriptParser::DataType::BUILTIN:
case GDScriptParser::DataType::ENUM:
case GDScriptParser::DataType::ENUM_VALUE:
case GDScriptParser::DataType::UNRESOLVED:
break; // Already solved before.
}
@ -3972,7 +3979,6 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
case GDScriptParser::DataType::VARIANT:
case GDScriptParser::DataType::BUILTIN:
case GDScriptParser::DataType::ENUM:
case GDScriptParser::DataType::ENUM_VALUE:
case GDScriptParser::DataType::UNRESOLVED:
break; // Already solved before.
}

2
modules/gdscript/gdscript_analyzer.h
View File

@ -112,7 +112,7 @@ class GDScriptAnalyzer {
GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid, const GDScriptParser::Node *p_source);
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;
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 mark_node_unsafe(const GDScriptParser::Node *p_node);
bool class_exists(const StringName &p_class) const;

1
modules/gdscript/gdscript_compiler.cpp
View File

@ -141,7 +141,6 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
}
} break;
case GDScriptParser::DataType::ENUM:
case GDScriptParser::DataType::ENUM_VALUE:
result.has_type = true;
result.kind = GDScriptDataType::BUILTIN;
result.builtin_type = Variant::INT;

2
modules/gdscript/gdscript_editor.cpp
View File

@ -610,7 +610,7 @@ static String _make_arguments_hint(const GDScriptParser::FunctionNode *p_functio
case GDScriptParser::Node::SUBSCRIPT: {
const GDScriptParser::SubscriptNode *sub = static_cast<const GDScriptParser::SubscriptNode *>(par->default_value);
if (sub->is_constant) {
if (sub->datatype.kind == GDScriptParser::DataType::ENUM_VALUE) {
if (sub->datatype.kind == GDScriptParser::DataType::ENUM) {
def_val = sub->get_datatype().to_string();
} else if (sub->reduced) {
const Variant::Type vt = sub->reduced_value.get_type();

2
modules/gdscript/gdscript_parser.cpp
View File

@ -3737,8 +3737,6 @@ String GDScriptParser::DataType::to_string() const {
}
case ENUM:
return enum_type.operator String() + " (enum)";
case ENUM_VALUE:
return enum_type.operator String() + " (enum value)";
case UNRESOLVED:
return "<unresolved type>";
}

5
modules/gdscript/gdscript_parser.h
View File

@ -106,8 +106,7 @@ public:
NATIVE,
SCRIPT,
CLASS, // GDScript.
ENUM, // Full enumeration.
ENUM_VALUE, // Value from enumeration.
ENUM, // Enumeration.
VARIANT, // Can be any type.
UNRESOLVED,
};
@ -185,8 +184,6 @@ public:
return builtin_type == p_other.builtin_type;
case NATIVE:
case ENUM:
return native_type == p_other.native_type;
case ENUM_VALUE:
return native_type == p_other.native_type && enum_type == p_other.enum_type;
case SCRIPT:
return script_type == p_other.script_type;

4
modules/gdscript/gdscript_warning.cpp
View File

@ -152,6 +152,9 @@ String GDScriptWarning::get_message() const {
CHECK_SYMBOLS(3);
return vformat(R"(The %s '%s' has the same name as a %s.)", symbols[0], symbols[1], symbols[2]);
}
case INT_ASSIGNED_TO_ENUM: {
return "Integer used when an enum value is expected. If this is intended cast the integer to the enum type.";
}
case WARNING_MAX:
break; // Can't happen, but silences warning
}
@ -199,6 +202,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
"REDUNDANT_AWAIT",
"EMPTY_FILE",
"SHADOWED_GLOBAL_IDENTIFIER",
"INT_ASSIGNED_TO_ENUM",
};
static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names.");

1
modules/gdscript/gdscript_warning.h
View File

@ -70,6 +70,7 @@ public:
REDUNDANT_AWAIT, // await is used but expression is synchronous (not a signal nor a coroutine).
EMPTY_FILE, // A script file is empty.
SHADOWED_GLOBAL_IDENTIFIER, // A global class or function has the same name as variable.
INT_ASSIGNED_TO_ENUM, // An integer value was assigned to an enum-typed variable without casting.
WARNING_MAX,
};

10
modules/gdscript/tests/scripts/analyzer/errors/enum_class_var_assign_with_wrong_enum_type.gd Normal file
View File

@ -0,0 +1,10 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }
# Different enum types can't be assigned without casting.
var class_var: MyEnum = MyEnum.ENUM_VALUE_1
func test():
print(class_var)
class_var = MyOtherEnum.OTHER_ENUM_VALUE_2
print(class_var)

2
modules/gdscript/tests/scripts/analyzer/errors/enum_class_var_assign_with_wrong_enum_type.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type "MyOtherEnum (enum)" to a target of type "MyEnum (enum)".

8
modules/gdscript/tests/scripts/analyzer/errors/enum_class_var_init_with_wrong_enum_type.gd Normal file
View File

@ -0,0 +1,8 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }
# Different enum types can't be assigned without casting.
var class_var: MyEnum = MyOtherEnum.OTHER_ENUM_VALUE_1
func test():
print(class_var)

2
modules/gdscript/tests/scripts/analyzer/errors/enum_class_var_init_with_wrong_enum_type.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Value of type "MyOtherEnum (enum)" cannot be assigned to a variable of type "MyEnum (enum)".

8
modules/gdscript/tests/scripts/analyzer/errors/enum_local_var_assign_with_wrong_enum_type.gd Normal file
View File

@ -0,0 +1,8 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }
func test():
var local_var: MyEnum = MyEnum.ENUM_VALUE_1
print(local_var)
local_var = MyOtherEnum.OTHER_ENUM_VALUE_2
print(local_var)

2
modules/gdscript/tests/scripts/analyzer/errors/enum_local_var_assign_with_wrong_enum_type.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a value of type "MyOtherEnum (enum)" to a target of type "MyEnum (enum)".

6
modules/gdscript/tests/scripts/analyzer/errors/enum_local_var_init_with_wrong_enum_type.gd Normal file
View File

@ -0,0 +1,6 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }
func test():
var local_var: MyEnum = MyOtherEnum.OTHER_ENUM_VALUE_1
print(local_var)

2
modules/gdscript/tests/scripts/analyzer/errors/enum_local_var_init_with_wrong_enum_type.out Normal file
View File

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Value of type "MyOtherEnum (enum)" cannot be assigned to a variable of type "MyEnum (enum)".

13
modules/gdscript/tests/scripts/analyzer/features/enum_assign_enum_to_int_typed_var.gd Normal file
View File

@ -0,0 +1,13 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
var class_var: int = MyEnum.ENUM_VALUE_1
func test():
print(class_var)
class_var = MyEnum.ENUM_VALUE_2
print(class_var)
var local_var: int = MyEnum.ENUM_VALUE_1
print(local_var)
local_var = MyEnum.ENUM_VALUE_2
print(local_var)

5
modules/gdscript/tests/scripts/analyzer/features/enum_assign_enum_to_int_typed_var.out Normal file
View File

@ -0,0 +1,5 @@
GDTEST_OK
0
1
0
1

13
modules/gdscript/tests/scripts/analyzer/features/enum_assign_int_cast_to_same_enum.gd Normal file
View File

@ -0,0 +1,13 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
var class_var: MyEnum = 0 as MyEnum
func test():
print(class_var)
class_var = 1 as MyEnum
print(class_var)
var local_var: MyEnum = 0 as MyEnum
print(local_var)
local_var = 1 as MyEnum
print(local_var)

5
modules/gdscript/tests/scripts/analyzer/features/enum_assign_int_cast_to_same_enum.out Normal file
View File

@ -0,0 +1,5 @@
GDTEST_OK
0
1
0
1

14
modules/gdscript/tests/scripts/analyzer/features/enum_assign_other_enum_cast_to_same_enum.gd Normal file
View File

@ -0,0 +1,14 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
enum MyOtherEnum { OTHER_ENUM_VALUE_1, OTHER_ENUM_VALUE_2 }
var class_var: MyEnum = MyOtherEnum.OTHER_ENUM_VALUE_1 as MyEnum
func test():
print(class_var)
class_var = MyOtherEnum.OTHER_ENUM_VALUE_2 as MyEnum
print(class_var)
var local_var: MyEnum = MyOtherEnum.OTHER_ENUM_VALUE_1 as MyEnum
print(local_var)
local_var = MyOtherEnum.OTHER_ENUM_VALUE_2 as MyEnum
print(local_var)

5
modules/gdscript/tests/scripts/analyzer/features/enum_assign_other_enum_cast_to_same_enum.out Normal file
View File

@ -0,0 +1,5 @@
GDTEST_OK
0
1
0
1

13
modules/gdscript/tests/scripts/analyzer/features/enum_assign_same_enum.gd Normal file
View File

@ -0,0 +1,13 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
var class_var: MyEnum = MyEnum.ENUM_VALUE_1
func test():
print(class_var)
class_var = MyEnum.ENUM_VALUE_2
print(class_var)
var local_var: MyEnum = MyEnum.ENUM_VALUE_1
print(local_var)
local_var = MyEnum.ENUM_VALUE_2
print(local_var)

5
modules/gdscript/tests/scripts/analyzer/features/enum_assign_same_enum.out Normal file
View File

@ -0,0 +1,5 @@
GDTEST_OK
0
1
0
1

15
modules/gdscript/tests/scripts/parser/warnings/enum_assign_int_without_casting.gd Normal file
View File

@ -0,0 +1,15 @@
enum MyEnum { ENUM_VALUE_1, ENUM_VALUE_2 }
# Assigning int value to enum-typed variable without explicit cast causes a warning.
# While it is valid it may be a mistake in the assignment.
var class_var: MyEnum = 0
func test():
print(class_var)
class_var = 1
print(class_var)
var local_var: MyEnum = 0
print(local_var)
local_var = 1
print(local_var)

21
modules/gdscript/tests/scripts/parser/warnings/enum_assign_int_without_casting.out Normal file
View File

@ -0,0 +1,21 @@
GDTEST_OK
>> WARNING
>> Line: 5
>> INT_ASSIGNED_TO_ENUM
>> Integer used when an enum value is expected. If this is intended cast the integer to the enum type.
>> WARNING
>> Line: 9
>> INT_ASSIGNED_TO_ENUM
>> Integer used when an enum value is expected. If this is intended cast the integer to the enum type.
>> WARNING
>> Line: 12
>> INT_ASSIGNED_TO_ENUM
>> Integer used when an enum value is expected. If this is intended cast the integer to the enum type.
>> WARNING
>> Line: 14
>> INT_ASSIGNED_TO_ENUM
>> Integer used when an enum value is expected. If this is intended cast the integer to the enum type.
0
1
0
1