From 1a231787b05ff910cc54a1e4ba6887302a58893a 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 1d687c9f0..dd7e255d5 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -1093,14 +1093,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; @@ -2146,6 +2147,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 c42a41347..42e3b7277 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -600,6 +600,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);