GDScript: support variable definition as condition in if-statement (godotengine/godot-proposals#2727)

Author: zjin123

modules/gdscript/editor/gdscript_translation_parser_plugin.cpp

@@ -135,6 +135,9 @@ void GDScriptEditorTranslationParserPlugin::_traverse_block(const GDScriptParser
 			} break;
 			case GDScriptParser::Node::IF: {
 				const GDScriptParser::IfNode *if_node = static_cast<const GDScriptParser::IfNode *>(statement);
+				if (if_node->variable) {
+					_assess_expression(if_node->variable->initializer);
+				}
 				_assess_expression(if_node->condition);
 				_traverse_block(if_node->true_block);
 				_traverse_block(if_node->false_block);

modules/gdscript/gdscript_analyzer.cpp

@@ -2096,6 +2096,9 @@ void GDScriptAnalyzer::resolve_parameter(GDScriptParser::ParameterNode *p_parame
 }
 
 void GDScriptAnalyzer::resolve_if(GDScriptParser::IfNode *p_if) {
+	if (p_if->variable) {
+		resolve_variable(p_if->variable, true);
+	}
 	reduce_expression(p_if->condition);
 
 	resolve_suite(p_if->true_block);

modules/gdscript/gdscript_compiler.cpp

@@ -251,6 +251,45 @@ static bool _can_use_validate_call(const MethodBind *p_method, const Vector<GDSc
 	return true;
 }
 
+Error GDScriptCompiler::_parse_variable(CodeGen &codegen, const GDScriptParser::VariableNode *p_variable, const GDScriptParser::SuiteNode *p_block) {
+	GDScriptCodeGenerator *gen = codegen.generator;
+
+	// Should be already in stack when the block began.
+	GDScriptCodeGenerator::Address local = codegen.locals[p_variable->identifier->name];
+	GDScriptDataType local_type = _gdtype_from_datatype(p_variable->get_datatype(), codegen.script);
+
+	Error err = OK;
+
+	bool initialized = false;
+	if (p_variable->initializer != nullptr) {
+		GDScriptCodeGenerator::Address src_address = _parse_expression(codegen, err, p_variable->initializer);
+		if (err) {
+			return err;
+		}
+		if (p_variable->use_conversion_assign) {
+			gen->write_assign_with_conversion(local, src_address);
+		} else {
+			gen->write_assign(local, src_address);
+		}
+		if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
+			codegen.generator->pop_temporary();
+		}
+		initialized = true;
+	} else if ((local_type.has_type && local_type.kind == GDScriptDataType::BUILTIN) || codegen.generator->is_local_dirty(local)) {
+		// Initialize with default for the type. Built-in types must always be cleared (they cannot be `null`).
+		// Objects and untyped variables are assigned to `null` only if the stack address has been re-used and not cleared.
+		codegen.generator->clear_address(local);
+		initialized = true;
+	}
+
+	// Don't check `is_local_dirty()` since the variable must be assigned to `null` **on each iteration**.
+	if (!initialized && p_block->is_in_loop) {
+		codegen.generator->clear_address(local);
+	}
+
+	return err;
+}
+
 GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root, bool p_initializer) {
 	if (p_expression->is_constant && !(p_expression->get_datatype().is_meta_type && p_expression->get_datatype().kind == GDScriptParser::DataType::CLASS)) {
 		return codegen.add_constant(p_expression->reduced_value);
@@ -1878,6 +1917,56 @@ void GDScriptCompiler::_clear_block_locals(CodeGen &codegen, const List<GDScript
 	}
 }
 
+Error GDScriptCompiler::_parse_if(CodeGen &codegen, const GDScriptParser::IfNode *if_n) {
+	Error err = OK;
+	GDScriptCodeGenerator *gen = codegen.generator;
+	List<GDScriptCodeGenerator::Address> block_locals;
+
+	gen->clear_temporaries();
+	codegen.start_block();
+	block_locals = _add_block_locals(codegen, if_n->condition_block);
+
+	if (if_n->variable) {
+		err = _parse_variable(codegen, if_n->variable, if_n->condition_block);
+		if (err) {
+			return err;
+		}
+
+		gen->clear_temporaries();
+	}
+
+	GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition);
+	if (err) {
+		return err;
+	}
+	gen->write_if(condition);
+
+	if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
+		codegen.generator->pop_temporary();
+	}
+
+	err = _parse_block(codegen, if_n->true_block);
+	if (err) {
+		return err;
+	}
+
+	_clear_block_locals(codegen, block_locals);
+	codegen.end_block();
+
+	if (if_n->false_block) {
+		gen->write_else();
+
+		err = _parse_block(codegen, if_n->false_block);
+		if (err) {
+			return err;
+		}
+	}
+
+	gen->write_endif();
+
+	return OK;
+}
+
 Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block, bool p_add_locals, bool p_clear_locals) {
 	Error err = OK;
 	GDScriptCodeGenerator *gen = codegen.generator;
@@ -2004,32 +2093,10 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
 			} break;
 			case GDScriptParser::Node::IF: {
 				const GDScriptParser::IfNode *if_n = static_cast<const GDScriptParser::IfNode *>(s);
-				GDScriptCodeGenerator::Address condition = _parse_expression(codegen, err, if_n->condition);
+				err = _parse_if(codegen, if_n);
 				if (err) {
 					return err;
 				}
-
-				gen->write_if(condition);
-
-				if (condition.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
-					codegen.generator->pop_temporary();
-				}
-
-				err = _parse_block(codegen, if_n->true_block);
-				if (err) {
-					return err;
-				}
-
-				if (if_n->false_block) {
-					gen->write_else();
-
-					err = _parse_block(codegen, if_n->false_block);
-					if (err) {
-						return err;
-					}
-				}
-
-				gen->write_endif();
 			} break;
 			case GDScriptParser::Node::FOR: {
 				const GDScriptParser::ForNode *for_n = static_cast<const GDScriptParser::ForNode *>(s);
@@ -2166,36 +2233,10 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
 #endif
 			} break;
 			case GDScriptParser::Node::VARIABLE: {
-				const GDScriptParser::VariableNode *lv = static_cast<const GDScriptParser::VariableNode *>(s);
-				// Should be already in stack when the block began.
-				GDScriptCodeGenerator::Address local = codegen.locals[lv->identifier->name];
-				GDScriptDataType local_type = _gdtype_from_datatype(lv->get_datatype(), codegen.script);
-
-				bool initialized = false;
-				if (lv->initializer != nullptr) {
-					GDScriptCodeGenerator::Address src_address = _parse_expression(codegen, err, lv->initializer);
-					if (err) {
-						return err;
-					}
-					if (lv->use_conversion_assign) {
-						gen->write_assign_with_conversion(local, src_address);
-					} else {
-						gen->write_assign(local, src_address);
-					}
-					if (src_address.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
-						codegen.generator->pop_temporary();
-					}
-					initialized = true;
-				} else if ((local_type.has_type && local_type.kind == GDScriptDataType::BUILTIN) || codegen.generator->is_local_dirty(local)) {
-					// Initialize with default for the type. Built-in types must always be cleared (they cannot be `null`).
-					// Objects and untyped variables are assigned to `null` only if the stack address has been re-used and not cleared.
-					codegen.generator->clear_address(local);
-					initialized = true;
-				}
-
-				// Don't check `is_local_dirty()` since the variable must be assigned to `null` **on each iteration**.
-				if (!initialized && p_block->is_in_loop) {
-					codegen.generator->clear_address(local);
+				const GDScriptParser::VariableNode *variable_n = static_cast<const GDScriptParser::VariableNode *>(s);
+				err = _parse_variable(codegen, variable_n, p_block);
+				if (err) {
+					return err;
 				}
 			} break;
 			case GDScriptParser::Node::CONSTANT: {

modules/gdscript/gdscript_compiler.h

@@ -155,6 +155,8 @@ class GDScriptCompiler {
 	GDScriptCodeGenerator::Address _parse_match_pattern(CodeGen &codegen, Error &r_error, const GDScriptParser::PatternNode *p_pattern, const GDScriptCodeGenerator::Address &p_value_addr, const GDScriptCodeGenerator::Address &p_type_addr, const GDScriptCodeGenerator::Address &p_previous_test, bool p_is_first, bool p_is_nested);
 	List<GDScriptCodeGenerator::Address> _add_block_locals(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block);
 	void _clear_block_locals(CodeGen &codegen, const List<GDScriptCodeGenerator::Address> &p_locals);
+	Error _parse_variable(CodeGen &codegen, const GDScriptParser::VariableNode *p_variable, const GDScriptParser::SuiteNode *p_block);
+	Error _parse_if(CodeGen &codegen, const GDScriptParser::IfNode *if_n);
 	Error _parse_block(CodeGen &codegen, const GDScriptParser::SuiteNode *p_block, bool p_add_locals = true, bool p_clear_locals = true);
 	GDScriptFunction *_parse_function(Error &r_error, GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false, bool p_for_lambda = false);
 	GDScriptFunction *_make_static_initializer(Error &r_error, GDScript *p_script, const GDScriptParser::ClassNode *p_class);

modules/gdscript/gdscript_parser.cpp

@@ -1076,7 +1076,7 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static) {
 	return parse_variable(p_is_static, true);
 }
 
-GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, bool p_allow_property) {
+GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, bool p_allow_property, bool p_is_if_condition) {
 	VariableNode *variable = alloc_node<VariableNode>();
 
 	if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected variable name after "var".)")) {
@@ -1135,7 +1135,10 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, b
 	}
 
 	complete_extents(variable);
-	end_statement("variable declaration");
+
+	if (p_is_if_condition == false) {
+		end_statement("variable declaration");
+	}
 
 	return variable;
 }
@@ -2087,21 +2090,68 @@ GDScriptParser::ForNode *GDScriptParser::parse_for() {
 }
 
 GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) {
-	IfNode *n_if = alloc_node<IfNode>();
+	SuiteNode *condition_block = alloc_node<SuiteNode>();
+	condition_block->parent_block = current_suite;
+	condition_block->parent_function = current_function;
+	SuiteNode *saved_suite = current_suite;
+	current_suite = condition_block;
+
+	VariableNode *variable = nullptr;
+	ExpressionNode *condition = nullptr;
+	if (match(GDScriptTokenizer::Token::VAR)) {
+		// Variable declaration
+		variable = parse_variable(false, false, true);
+		if (variable == nullptr) {
+			push_error(vformat(R"(Expected variable definition after "%s".)", p_token));
+		} else if (variable->initializer == nullptr) {
+			push_error(R"(Expected expression for variable initial value.)");
+		} else {
+			const SuiteNode::Local &local = current_suite->get_local(variable->identifier->name);
+			if (local.type != SuiteNode::Local::UNDEFINED) {
+				push_error(vformat(R"(There is already a %s named "%s" declared in this scope.)", local.get_name(), variable->identifier->name), variable->identifier);
+			}
+			condition_block->add_local(variable, current_function);
+
+			IdentifierNode *identifier = alloc_node<IdentifierNode>();
+			identifier->name = variable->identifier->name;
+			identifier->suite = condition_block;
+			identifier->source = IdentifierNode::Source::LOCAL_VARIABLE;
+			const SuiteNode::Local &declaration = condition_block->get_local(identifier->name);
+			identifier->variable_source = declaration.variable;
+			declaration.variable->usages++;
+			complete_extents(identifier);
+
+			condition_block->statements.push_back(variable);
 
-	n_if->condition = parse_expression(false);
-	if (n_if->condition == nullptr) {
-		push_error(vformat(R"(Expected conditional expression after "%s".)", p_token));
+			condition = identifier;
+		}
+	} else {
+		// Expression
+		ExpressionNode *expression = parse_expression(false);
+		if (expression == nullptr) {
+			push_error(vformat(R"(Expected conditional expression after "%s".)", p_token));
+		} else {
+			condition = expression;
+		}
 	}
 
 	consume(GDScriptTokenizer::Token::COLON, vformat(R"(Expected ":" after "%s" condition.)", p_token));
 
-	n_if->true_block = parse_suite(vformat(R"("%s" block)", p_token));
-	n_if->true_block->parent_if = n_if;
+	SuiteNode *true_block = parse_suite(vformat(R"("%s" block)", p_token));
 
-	if (n_if->true_block->has_continue) {
-		current_suite->has_continue = true;
-	}
+	complete_extents(condition_block);
+	current_suite = saved_suite;
+
+	IfNode *n_if = alloc_node<IfNode>();
+
+	true_block->parent_function = current_function;
+	true_block->parent_block = condition_block;
+	true_block->parent_if = n_if;
+
+	n_if->variable = variable;
+	n_if->condition = condition;
+	n_if->condition_block = condition_block;
+	n_if->true_block = true_block;
 
 	if (match(GDScriptTokenizer::Token::ELIF)) {
 		SuiteNode *else_block = alloc_node<SuiteNode>();
@@ -2123,10 +2173,10 @@ GDScriptParser::IfNode *GDScriptParser::parse_if(const String &p_token) {
 	}
 	complete_extents(n_if);
 
-	if (n_if->false_block != nullptr && n_if->false_block->has_return && n_if->true_block->has_return) {
+	if ((n_if->false_block != nullptr && n_if->false_block->has_return) && n_if->true_block->has_return) {
 		current_suite->has_return = true;
 	}
-	if (n_if->false_block != nullptr && n_if->false_block->has_continue) {
+	if ((n_if->false_block != nullptr && n_if->false_block->has_continue) || n_if->true_block->has_continue) {
 		current_suite->has_continue = true;
 	}
 
@@ -4787,11 +4837,25 @@ bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_t
 
 				// Contain bodies.
 				SIMPLE_CASE(Node::FOR, ForNode, list)
-				SIMPLE_CASE(Node::IF, IfNode, condition)
 				SIMPLE_CASE(Node::MATCH, MatchNode, test)
 				SIMPLE_CASE(Node::WHILE, WhileNode, condition)
 #undef SIMPLE_CASE
 
+				case Node::IF: {
+					IfNode *if_n = static_cast<IfNode *>(p_target);
+					if (if_n->variable) {
+						if (if_n->variable->initializer) {
+							end_line = if_n->variable->initializer->end_line;
+						} else {
+							end_line = if_n->start_line;
+						}
+					} else if (if_n->condition) {
+						end_line = if_n->condition->end_line;
+					} else {
+						end_line = if_n->start_line;
+					}
+				} break;
+
 				case Node::CLASS: {
 					end_line = p_target->start_line;
 					for (const AnnotationNode *annotation : p_target->annotations) {
@@ -5744,7 +5808,11 @@ void GDScriptParser::TreePrinter::print_if(IfNode *p_if, bool p_is_elif) {
 	} else {
 		push_text("If ");
 	}
-	print_expression(p_if->condition);
+	if (p_if->variable) {
+		print_variable(p_if->variable);
+	} else {
+		print_expression(p_if->condition);
+	}
 	push_line(" :");
 
 	increase_indent();

modules/gdscript/gdscript_parser.h

@@ -918,7 +918,9 @@ class GDScriptParser {
 	};
 
 	struct IfNode : public Node {
+		VariableNode *variable = nullptr;
 		ExpressionNode *condition = nullptr;
+		SuiteNode *condition_block = nullptr;
 		SuiteNode *true_block = nullptr;
 		SuiteNode *false_block = nullptr;
 
@@ -1516,7 +1518,7 @@ class GDScriptParser {
 	// Statements.
 	Node *parse_statement();
 	VariableNode *parse_variable(bool p_is_static);
-	VariableNode *parse_variable(bool p_is_static, bool p_allow_property);
+	VariableNode *parse_variable(bool p_is_static, bool p_allow_property, bool p_is_if_condition = false);
 	VariableNode *parse_property(VariableNode *p_variable, bool p_need_indent);
 	void parse_property_getter(VariableNode *p_variable);
 	void parse_property_setter(VariableNode *p_variable);

modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.gd

@@ -0,0 +1,5 @@
+func test():
+    if var x = 100:
+        print("t")
+    elif x > 0:
+        print("f")

modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_elif.out

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Identifier "x" not declared in the current scope.

modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.gd

@@ -0,0 +1,5 @@
+func test():
+    if var x = 100:
+        print("t")
+    else:
+        print(x)

modules/gdscript/tests/scripts/analyzer/errors/if_var_use_in_else.out

@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Identifier "x" not declared in the current scope.