From 52507443d3f1edc86daabb8a63a9ae688141e1ee Mon Sep 17 00:00:00 2001 From: Kirill Diduk Date: Fri, 1 Jul 2022 00:32:49 +0200 Subject: [PATCH] Check duplicate keys in dictionary literals: enums and const variables Check identifiers (const variables and unnamed enums) and named enums when parsing dictionary literals whether the keys are not duplicated. In case of duplicate key is encountered, highlight the line with it and print error message: `Duplicate key "foo" found in Dictionary literal` This commit is a logical continuation of the commit dab73c7 which implemented such checks only for literal keys (which fixed #7034). Apart from that, this commit also fixes the issue with the error message itself, which was shown one line below the duplicated key in case it was the last one in the dictionary literal and there was no hanging comma. Also, the format of the error message has been changed so that now the error message also contains the value of the key which is duplicated. Instead of `Duplicate key found in Dictionary literal`, it now prints `Duplicate key "" found in Dictionary literal` Fixes #50971 --- modules/gdscript/gdscript_parser.cpp | 56 +++++++++++++++++++++++++--- modules/gdscript/gdscript_parser.h | 1 + 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 9e410645bea..e396c8db0b0 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -1127,14 +1127,15 @@ GDScriptParser::Node *GDScriptParser::_parse_expression(Node *p_parent, bool p_s } expecting = DICT_EXPECT_COMMA; - if (key->type == GDScriptParser::Node::TYPE_CONSTANT) { - Variant const &keyName = static_cast(key)->value; - - if (keys.has(keyName)) { - _set_error("Duplicate key found in Dictionary literal"); + const Variant *key_value = _try_to_find_constant_value_for_expression(key); + if (key_value) { + if (keys.has(*key_value)) { + _set_error("Duplicate key \"" + String(*key_value) + "\" found in Dictionary literal", + key->line, + key->column); return nullptr; } - keys.insert(keyName); + keys.insert(*key_value); } DictionaryNode::Pair pair; @@ -2180,6 +2181,49 @@ bool GDScriptParser::_reduce_export_var_type(Variant &p_value, int p_line) { return false; } +const Variant *GDScriptParser::_try_to_find_constant_value_for_expression(const Node *p_expr) const { + if (p_expr->type == Node::TYPE_CONSTANT) { + return &(static_cast(p_expr)->value); + } else if (p_expr->type == Node::TYPE_IDENTIFIER) { + const StringName &name = static_cast(p_expr)->name; + const Map::Element *element = + current_class->constant_expressions.find(name); + if (element) { + Node *cn_exp = element->value().expression; + if (cn_exp->type == Node::TYPE_CONSTANT) { + return &(static_cast(cn_exp)->value); + } + } + } else if (p_expr->type == Node::TYPE_OPERATOR) { + // Check if expression `p_expr` is a named enum (e.g. `State.IDLE`). + const OperatorNode *op_node = static_cast(p_expr); + if (op_node->op == GDScriptParser::OperatorNode::OP_INDEX_NAMED) { + const Vector &op_args = op_node->arguments; + if (op_args.size() < 2) { + return nullptr; // Invalid expression. + } + + if (op_args[0]->type != Node::TYPE_IDENTIFIER || op_args[1]->type != Node::TYPE_IDENTIFIER) { + return nullptr; // Not an enum expression. + } + + const StringName &enum_name = static_cast(op_args[0])->name; + const StringName &const_name = static_cast(op_args[1])->name; + Map::Element *element = + current_class->constant_expressions.find(enum_name); + if (element) { + Node *cn_exp = element->value().expression; + if (cn_exp->type == Node::TYPE_CONSTANT) { + const Dictionary &enum_dict = static_cast(cn_exp)->value; + return enum_dict.getptr(const_name); + } + } + } + } + + return nullptr; +} + bool GDScriptParser::_recover_from_completion() { if (!completion_found) { return false; //can't recover if no completion diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 3880041d82a..674d67ba9ad 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -607,6 +607,7 @@ private: Node *_reduce_expression(Node *p_node, bool p_to_const = false); Node *_parse_and_reduce_expression(Node *p_parent, bool p_static, bool p_reduce_const = false, bool p_allow_assign = false); bool _reduce_export_var_type(Variant &p_value, int p_line = 0); + const Variant *_try_to_find_constant_value_for_expression(const Node *p_expr) const; PatternNode *_parse_pattern(bool p_static); void _parse_pattern_block(BlockNode *p_block, Vector &p_branches, bool p_static);