From d97624e2954dc5180f2ae28d8aab9c8eaaff2954 Mon Sep 17 00:00:00 2001 From: George Marques Date: Tue, 21 Aug 2018 13:14:11 -0300 Subject: [PATCH 1/5] GDSCript: Fix cyclic class dependency detection --- modules/gdscript/gdscript_compiler.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index fe393957dbe..368601127da 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -1994,8 +1994,11 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner p_script->_signals[name] = p_class->_signals[i].arguments; } - if (!p_class->owner) { + if (p_class->owner) { parsed_classes.insert(p_class->name); + if (parsing_classes.has(p_class->name)) { + parsing_classes.erase(p_class->name); + } } //parse sub-classes @@ -2011,7 +2014,6 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner Error err = _parse_class_level(subclass.ptr(), p_script, p_class->subclasses[i], p_keep_state); if (err) return err; - parsing_classes.erase(name); } #ifdef TOOLS_ENABLED From abbdb9d9514ff1299a3a7cec044eaf735273cad3 Mon Sep 17 00:00:00 2001 From: George Marques Date: Tue, 21 Aug 2018 13:38:18 -0300 Subject: [PATCH 2/5] GDScript: Forbid invalid identifiers in match bindings Also forbid shadowing a variable from an upper scope. --- modules/gdscript/gdscript_parser.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index bec314866d4..841bb3e610c 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -2024,12 +2024,20 @@ GDScriptParser::PatternNode *GDScriptParser::_parse_pattern(bool p_static) { // bind case GDScriptTokenizer::TK_PR_VAR: { tokenizer->advance(); + if (!tokenizer->is_token_literal()) { + _set_error("Expected identifier for binding variable name."); + return NULL; + } pattern->pt_type = GDScriptParser::PatternNode::PT_BIND; pattern->bind = tokenizer->get_token_identifier(); - // Check if binding is already used - if (current_block->variables.has(pattern->bind)) { - _set_error("Binding name of '" + pattern->bind.operator String() + "' was already used in the pattern."); - return NULL; + // Check if variable name is already used + BlockNode *bl = current_block; + while (bl) { + if (bl->variables.has(pattern->bind)) { + _set_error("Binding name of '" + pattern->bind.operator String() + "' is already declared in this scope."); + return NULL; + } + bl = bl->parent_block; } // Create local variable for proper identifier detection later LocalVarNode *lv = alloc_node(); From fdf18a41985701aafb20d2f6d35f481fee22e6cc Mon Sep 17 00:00:00 2001 From: George Marques Date: Tue, 21 Aug 2018 13:54:52 -0300 Subject: [PATCH 3/5] GDScript: Fix undefined behavior on GDScriptTokenizerBuffer --- modules/gdscript/gdscript_tokenizer.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/gdscript/gdscript_tokenizer.h b/modules/gdscript/gdscript_tokenizer.h index 28a08bfaf8d..11a291cb2e2 100644 --- a/modules/gdscript/gdscript_tokenizer.h +++ b/modules/gdscript/gdscript_tokenizer.h @@ -283,8 +283,14 @@ public: virtual String get_token_error(int p_offset = 0) const; virtual void advance(int p_amount = 1); #ifdef DEBUG_ENABLED - virtual const Vector > &get_warning_skips() const { return Vector >(); } - virtual const Set &get_warning_global_skips() const { return Set(); } + virtual const Vector > &get_warning_skips() const { + static Vector > v; + return v; + } + virtual const Set &get_warning_global_skips() const { + static Set s; + return s; + } virtual const bool is_ignoring_warnings() const { return true; } #endif // DEBUG_ENABLED GDScriptTokenizerBuffer(); From 4d2b9a4b352a86cb78331b136c05ec2538b2928b Mon Sep 17 00:00:00 2001 From: George Marques Date: Tue, 21 Aug 2018 15:21:51 -0300 Subject: [PATCH 4/5] GDScript: Show warning messages only on debugger Don't show on console/output anymore. --- modules/gdscript/gdscript.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index b987d2897ff..5125b58b41f 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -606,9 +606,11 @@ Error GDScript::reload(bool p_keep_state) { } #if DEBUG_ENABLED for (const List::Element *E = parser.get_warnings().front(); E; E = E->next()) { - String msg = "Script warning: " + E->get().get_name() + " (" + path + ") line " + itos(E->get().line) + ": "; - msg += E->get().get_message(); - WARN_PRINTS(msg); + const GDScriptWarning &warning = E->get(); + if (ScriptDebugger::get_singleton()) { + Vector si; + ScriptDebugger::get_singleton()->send_error("", get_path(), warning.line, warning.get_name(), warning.get_message(), ERR_HANDLER_WARNING, si); + } } #endif From 94d662ad558c80b0b04c85ad3daadb24ce653cd9 Mon Sep 17 00:00:00 2001 From: George Marques Date: Tue, 21 Aug 2018 20:19:35 -0300 Subject: [PATCH 5/5] GDScript: Ignore unused arguments/local vars that start with _ Makes it simple to ignore particular arguments without adding special comments, especially in engine-defined functions. --- modules/gdscript/gdscript_parser.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 841bb3e610c..2c0d541d8f5 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -7397,7 +7397,7 @@ void GDScriptParser::_check_function_types(FunctionNode *p_function) { } } #ifdef DEBUG_ENABLED - if (p_function->arguments_usage[i] == 0) { + if (p_function->arguments_usage[i] == 0 && !p_function->arguments[i].operator String().begins_with("_")) { _add_warning(GDScriptWarning::UNUSED_ARGUMENT, p_function->line, p_function->name, p_function->arguments[i].operator String()); } #endif // DEBUG_ENABLED @@ -7855,10 +7855,12 @@ void GDScriptParser::_check_block_types(BlockNode *p_block) { // Warnings check for (Map::Element *E = p_block->variables.front(); E; E = E->next()) { LocalVarNode *lv = E->get(); - if (lv->usages == 0) { - _add_warning(GDScriptWarning::UNUSED_VARIABLE, lv->line, lv->name); - } else if (lv->assignments == 0) { - _add_warning(GDScriptWarning::UNASSIGNED_VARIABLE, lv->line, lv->name); + if (!lv->name.operator String().begins_with("_")) { + if (lv->usages == 0) { + _add_warning(GDScriptWarning::UNUSED_VARIABLE, lv->line, lv->name); + } else if (lv->assignments == 0) { + _add_warning(GDScriptWarning::UNASSIGNED_VARIABLE, lv->line, lv->name); + } } } #endif // DEBUG_ENABLED