Skip to content

Commit 7255e7e

Browse files
adamscottadamscott
committed
Fix attribute and enum matching
1 parent 734272a commit 7255e7e

File tree

7 files changed

+117
-37
lines changed

7 files changed

+117
-37
lines changed

‎modules/gdscript/gdscript_editor.cpp‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4538,7 +4538,7 @@ static Error _refactor_rename_symbol_match_from_class(GDScriptParser::RefactorRe
45384538
return OK;
45394539
}
45404540

4541-
static Error _refactor_rename_symbol_from_base(GDScriptParser::RefactorRenameContext p_context, const GDScriptParser::DataType &p_base, const String &p_symbol, const String &p_path, const String &p_class_path, Object *p_owner, const HashMap<String, String> &p_unsaved_scripts_source_code, ScriptLanguage::RefactorRenameSymbolResult &r_result) {
4541+
static Error _refactor_rename_symbol_from_base(GDScriptParser::RefactorRenameContext p_context, const GDScriptParser::DataType &p_base, const String &p_symbol, const String &p_path, const String &p_class_path, Object *p_owner, const HashMap<String, String> &p_unsaved_scripts_source_code, ScriptLanguage::RefactorRenameSymbolResult &r_result, RefactorRenameSymbolDefinintionType p_expected_definition_type = REFACTOR_RENAME_SYMBOL_DEFINITION_TYPE_UNDEFINED) {
45424542
#define OUTSIDE_REFACTOR(_type) \
45434543
r_result.type = ScriptLanguage::RefactorRenameSymbolResultType::_type; \
45444544
r_result.outside_refactor = true; \
@@ -4657,7 +4657,7 @@ static Error _refactor_rename_symbol_from_base(GDScriptParser::RefactorRenameCon
46574657
} break;
46584658
case GDScriptParser::DataType::ENUM: {
46594659
if (base_type.is_meta_type) {
4660-
if (base_type.enum_values.has(p_symbol)) {
4660+
if (base_type.enum_type == p_symbol || base_type.enum_values.has(p_symbol)) {
46614661
String type_name;
46624662
String enum_name;
46634663
GDScriptDocGen::doctype_from_gdtype(GDScriptAnalyzer::type_from_metatype(base_type), type_name, enum_name);
@@ -4668,10 +4668,14 @@ static Error _refactor_rename_symbol_from_base(GDScriptParser::RefactorRenameCon
46684668
const GDScriptParser::ClassNode::Member &member_enum = base_type.class_type->get_member(base_type.enum_type);
46694669
switch (member_enum.type) {
46704670
case GDScriptParser::ClassNode::Member::ENUM: {
4671+
RefactorRenameSymbolDefinintionType enum_type = RefactorRenameSymbolDefinintionType::REFACTOR_RENAME_SYMBOL_DEFINITION_TYPE_ENUM;
4672+
if (p_expected_definition_type != REFACTOR_RENAME_SYMBOL_DEFINITION_TYPE_UNDEFINED) {
4673+
enum_type = p_expected_definition_type;
4674+
}
46714675
return _refactor_rename_symbol_match_from_class(
46724676
p_context, p_symbol, p_path, p_class_path,
46734677
p_unsaved_scripts_source_code, r_result,
4674-
_refactor_rename_symbol_get_expected_type_from_classnode_member_type(member_enum.type),
4678+
enum_type,
46754679
member_enum.get_source_node());
46764680
} break;
46774681
default: {
@@ -5018,12 +5022,27 @@ ::Error GDScriptLanguage::refactor_rename_symbol_code(const String &p_code, cons
50185022
} break;
50195023
case GDScriptParser::REFACTOR_RENAME_TYPE_ATTRIBUTE_METHOD:
50205024
case GDScriptParser::REFACTOR_RENAME_TYPE_ATTRIBUTE: {
5025+
Error attribute_err;
5026+
5027+
if (context.identifier == nullptr) {
5028+
REFACTOR_RENAME_RETURN(ERR_CANT_RESOLVE);
5029+
}
50215030
if (context.node->type != GDScriptParser::Node::SUBSCRIPT) {
5022-
break;
5031+
REFACTOR_RENAME_RETURN(ERR_CANT_RESOLVE);
50235032
}
5033+
50245034
GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(context.node);
50255035
if (!subscript->is_attribute) {
5026-
break;
5036+
REFACTOR_RENAME_RETURN(ERR_BUG);
5037+
}
5038+
5039+
if (subscript->get_datatype().kind == GDScriptParser::DataType::ENUM) {
5040+
GDScriptParser::DataType base_type = subscript->get_datatype();
5041+
RefactorRenameSymbolDefinintionType enum_type = context.identifier->get_datatype().builtin_type == Variant::INT
5042+
? REFACTOR_RENAME_SYMBOL_DEFINITION_TYPE_ENUM_VALUE
5043+
: REFACTOR_RENAME_SYMBOL_DEFINITION_TYPE_ENUM;
5044+
attribute_err = _refactor_rename_symbol_from_base(context, subscript->get_datatype(), symbol, p_path, base_type.script_path, p_owner, p_unsaved_scripts_source_code, r_result, enum_type);
5045+
REFACTOR_RENAME_RETURN(attribute_err);
50275046
}
50285047

50295048
GDScriptParsingIdentifier base;
@@ -5032,9 +5051,8 @@ ::Error GDScriptLanguage::refactor_rename_symbol_code(const String &p_code, cons
50325051
break;
50335052
}
50345053

5035-
if (_refactor_rename_symbol_from_base(context, base.type, symbol, p_path, base.type.script_path, p_owner, p_unsaved_scripts_source_code, r_result) == OK) {
5036-
REFACTOR_RENAME_RETURN(OK);
5037-
}
5054+
attribute_err = _refactor_rename_symbol_from_base(context, base.type, symbol, p_path, base.type.script_path, p_owner, p_unsaved_scripts_source_code, r_result);
5055+
REFACTOR_RENAME_RETURN(attribute_err);
50385056
} break;
50395057
case GDScriptParser::REFACTOR_RENAME_TYPE_TYPE_ATTRIBUTE: {
50405058
if (context.node == nullptr || context.node->type != GDScriptParser::Node::TYPE) {

‎modules/gdscript/gdscript_parser.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,6 @@ bool GDScriptParser::refactor_rename_register(GDScriptParser::RefactorRenameType
454454
context.identifier = refactor_rename_context.identifier;
455455
context.identifier_is_enum_value = refactor_rename_context.identifier_is_enum_value;
456456
context.literal = refactor_rename_context.literal;
457-
} else {
458-
context.identifier = nullptr;
459-
context.literal = nullptr;
460457
}
461458
if (previous.has_cursor()) {
462459
context.token = previous;
@@ -3516,7 +3513,6 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_grouping(ExpressionNode *p
35163513

35173514
GDScriptParser::ExpressionNode *GDScriptParser::parse_attribute(ExpressionNode *p_previous_operand, bool p_can_assign) {
35183515
SubscriptNode *attribute = alloc_node<SubscriptNode>();
3519-
refactor_rename_register(REFACTOR_RENAME_TYPE_ATTRIBUTE, attribute);
35203516
reset_extents(attribute, p_previous_operand);
35213517
update_extents(attribute);
35223518

@@ -3540,6 +3536,13 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_attribute(ExpressionNode *
35403536
if (current.is_node_name()) {
35413537
current.type = GDScriptTokenizer::Token::IDENTIFIER;
35423538
}
3539+
3540+
if (is_for_refactor_rename()) {
3541+
if (current.has_cursor()) {
3542+
refactor_rename_register(REFACTOR_RENAME_TYPE_ATTRIBUTE, attribute, false);
3543+
}
3544+
}
3545+
35433546
if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected identifier after "." for attribute access.)")) {
35443547
complete_extents(attribute);
35453548
return attribute;
@@ -3668,6 +3671,7 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_call(ExpressionNode *p_pre
36683671
call->function_name = attribute->attribute->name;
36693672
}
36703673
make_completion_context(COMPLETION_ATTRIBUTE_METHOD, call->callee);
3674+
refactor_rename_register(REFACTOR_RENAME_TYPE_ATTRIBUTE_METHOD, attribute);
36713675
} else {
36723676
// TODO: The analyzer can see if this is actually a Callable and give better error message.
36733677
push_error(R"*(Cannot call on an expression. Use ".call()" if it's a Callable.)*");

‎modules/gdscript/gdscript_parser.h‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1886,7 +1886,7 @@ class GDScriptParser {
18861886
static HashMap<StringName, AnnotationInfo> valid_annotations;
18871887
List<AnnotationNode *> annotation_stack;
18881888

1889-
typedef ExpressionNode *(GDScriptParser::*ParseFunction)(ExpressionNode *p_previous_operand, bool p_can_assign);
1889+
typedef ExpressionNode *(GDScriptParser::*ParseFunction)(ExpressionNode *p_previous_operand, bool p_can_assignn);
18901890
// Higher value means higher precedence (i.e. is evaluated first).
18911891
enum Precedence {
18921892
PREC_NONE,
@@ -2057,30 +2057,53 @@ class GDScriptParser {
20572057
ExpressionNode *parse_expression(bool p_can_assign, bool p_stop_on_assign = false);
20582058
ExpressionNode *parse_precedence(Precedence p_precedence, bool p_can_assign, bool p_stop_on_assign = false);
20592059
ExpressionNode *parse_literal(ExpressionNode *p_previous_operand, bool p_can_assign);
2060+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_literal));
20602061
LiteralNode *parse_literal();
20612062
ExpressionNode *parse_self(ExpressionNode *p_previous_operand, bool p_can_assign);
2063+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_self));
20622064
ExpressionNode *parse_identifier(ExpressionNode *p_previous_operand, bool p_can_assign);
2065+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_identifier));
20632066
IdentifierNode *parse_identifier();
20642067
ExpressionNode *parse_builtin_constant(ExpressionNode *p_previous_operand, bool p_can_assign);
2068+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_builtin_constant));
20652069
ExpressionNode *parse_unary_operator(ExpressionNode *p_previous_operand, bool p_can_assign);
2070+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_unary_operator));
20662071
ExpressionNode *parse_binary_operator(ExpressionNode *p_previous_operand, bool p_can_assign);
2072+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_binary_operator));
20672073
ExpressionNode *parse_binary_not_in_operator(ExpressionNode *p_previous_operand, bool p_can_assign);
2074+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_binary_not_in_operator));
20682075
ExpressionNode *parse_ternary_operator(ExpressionNode *p_previous_operand, bool p_can_assign);
2076+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_ternary_operator));
20692077
ExpressionNode *parse_assignment(ExpressionNode *p_previous_operand, bool p_can_assign);
2078+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_assignment));
20702079
ExpressionNode *parse_array(ExpressionNode *p_previous_operand, bool p_can_assign);
2080+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_array));
20712081
ExpressionNode *parse_dictionary(ExpressionNode *p_previous_operand, bool p_can_assign);
2082+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_dictionary));
20722083
ExpressionNode *parse_call(ExpressionNode *p_previous_operand, bool p_can_assign);
2084+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_call));
20732085
ExpressionNode *parse_get_node(ExpressionNode *p_previous_operand, bool p_can_assign);
2086+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_get_node));
20742087
ExpressionNode *parse_preload(ExpressionNode *p_previous_operand, bool p_can_assign);
2088+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_preload));
20752089
ExpressionNode *parse_grouping(ExpressionNode *p_previous_operand, bool p_can_assign);
2090+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_preload));
20762091
ExpressionNode *parse_cast(ExpressionNode *p_previous_operand, bool p_can_assign);
2092+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_cast));
20772093
ExpressionNode *parse_await(ExpressionNode *p_previous_operand, bool p_can_assign);
2094+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_await));
20782095
ExpressionNode *parse_attribute(ExpressionNode *p_previous_operand, bool p_can_assign);
2096+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_attribute));
20792097
ExpressionNode *parse_subscript(ExpressionNode *p_previous_operand, bool p_can_assign);
2098+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_subscript));
20802099
ExpressionNode *parse_lambda(ExpressionNode *p_previous_operand, bool p_can_assign);
2100+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_lambda));
20812101
ExpressionNode *parse_type_test(ExpressionNode *p_previous_operand, bool p_can_assign);
2102+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_type_test));
20822103
ExpressionNode *parse_yield(ExpressionNode *p_previous_operand, bool p_can_assign);
2104+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_yield));
20832105
ExpressionNode *parse_invalid_token(ExpressionNode *p_previous_operand, bool p_can_assign);
2106+
static_assert(static_cast<ParseFunction>(&GDScriptParser::parse_invalid_token));
20842107
TypeNode *parse_type(bool p_allow_void = false);
20852108

20862109
#ifdef TOOLS_ENABLED

‎modules/gdscript/tests/scripts/refactor/rename/tests/enum/enum_name/rename_EnumOfInnerInnerA.cfg‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,18 @@
22
[input]
33
refactor={
44
"file": "res://scripts/class_a.gd",
5-
"line": 8,
5+
"line": 7,
66
"column": 18,
77
"symbol": "EnumOfInnerInnerA",
88
}
99
[output]
1010
matches={
1111
"res://scripts/class_a.gd": [
1212
{
13-
"start_line": 8,
14-
"start_column": 13,
15-
"end_line": 8,
16-
"end_column": 25,
13+
"start_line": 7,
14+
"start_column": 14,
15+
"end_line": 7,
16+
"end_column": 31,
1717
},
1818
],
1919
}

‎modules/gdscript/tests/scripts/refactor/rename/tests/enum/enum_name/rename_EnumOfInnerInnerB.cfg‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
; This test renames InnerInnerB.EnumOfInnerInnerB.
2+
; This should only rename the enum and its use, but not the
3+
; other enum values sharing the same name.
4+
[input]
5+
refactor={
6+
"file": "res://scripts/class_b.gd",
7+
"line": 20,
8+
"column": 35,
9+
"symbol": "EnumOfInnerInnerB",
10+
}
11+
[output]
12+
matches={
13+
"res://scripts/class_b.gd": [
14+
{
15+
"start_line": 7,
16+
"start_column": 14,
17+
"end_line": 7,
18+
"end_column": 31,
19+
},
20+
{
21+
"start_line": 20,
22+
"start_column": 27,
23+
"end_line": 20,
24+
"end_column": 44,
25+
},
26+
],
27+
"res://scripts/class_a.gd": [
28+
{
29+
"start_line": 36,
30+
"start_column": 61,
31+
"end_line": 36,
32+
"end_column": 78,
33+
},
34+
]
35+
}

‎modules/gdscript/tests/scripts/refactor/rename/tests/enum/enum_value/rename_enum_with_same_name_as_its_enum_value.cfg‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,32 @@
22
[input]
33
refactor={
44
"file": "res://scripts/enum.gd",
5-
"line": 2,
6-
"column": 11,
5+
"line": 1,
6+
"column": 9,
77
"symbol": "ENUM_VALUE_1",
88
}
99
[output]
1010
matches={
11+
"res://scripts/class_c.gd": [
12+
{
13+
"start_line": 12,
14+
"start_column": 21,
15+
"end_line": 12,
16+
"end_column": 33,
17+
},
18+
],
1119
"res://scripts/enum.gd": [
1220
{
13-
"start_line": 2,
14-
"start_column": 5,
15-
"end_line": 2,
16-
"end_column": 17,
21+
"start_line": 1,
22+
"start_column": 6,
23+
"end_line": 1,
24+
"end_column": 18,
1725
},
1826
{
1927
"start_line": 8,
20-
"start_column": 24,
28+
"start_column": 11,
2129
"end_line": 8,
22-
"end_column": 36,
30+
"end_column": 23,
2331
},
24-
],
25-
"res://scripts/class_c.gd": [
26-
{
27-
"start_line": 12,
28-
"start_column": 34,
29-
"end_line": 12,
30-
"end_column": 46,
31-
},
32-
],
32+
]
3333
}

‎modules/gdscript/tests/scripts/refactor/rename/tests/enum_change_inner_inner_enum_value.cfg‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ matches={
2626
],
2727
"res://scripts/class_a.gd": [
2828
{
29-
"start_line": 34,
29+
"start_line": 36,
3030
"start_column": 79,
31-
"end_line": 34,
31+
"end_line": 36,
3232
"end_column": 91,
3333
},
3434
]

0 commit comments

Comments
 (0)