Merge pull request #957 from Spartan322/4.3.1-cherry-pick/bugs-gdscript

[4.3] Cherry-picks for the 4.3 (4.3.1) branch - 2nd gdscript bugs batch

Author: Spartan322

modules/gdscript/gdscript_editor.cpp

@@ -862,7 +862,8 @@ static void _get_directory_contents(EditorFileSystemDirectory *p_dir, HashMap<St
 	}
 }
 
-static void _find_annotation_arguments(const GDScriptParser::AnnotationNode *p_annotation, int p_argument, const String p_quote_style, HashMap<String, ScriptLanguage::CodeCompletionOption> &r_result) {
+static void _find_annotation_arguments(const GDScriptParser::AnnotationNode *p_annotation, int p_argument, const String p_quote_style, HashMap<String, ScriptLanguage::CodeCompletionOption> &r_result, String &r_arghint) {
+	r_arghint = _make_arguments_hint(p_annotation->info->info, p_argument, true);
 	if (p_annotation->name == SNAME("@export_range")) {
 		if (p_argument == 3 || p_argument == 4 || p_argument == 5) {
 			// Slider hint.
@@ -2127,7 +2128,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 	// Look in blocks first.
 	int last_assign_line = -1;
 	const GDScriptParser::ExpressionNode *last_assigned_expression = nullptr;
-	GDScriptParser::DataType id_type;
+	GDScriptCompletionIdentifier id_type;
 	GDScriptParser::SuiteNode *suite = p_context.current_suite;
 	bool is_function_parameter = false;
 
@@ -2149,7 +2150,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 	if (can_be_local && suite && suite->has_local(p_identifier->name)) {
 		const GDScriptParser::SuiteNode::Local &local = suite->get_local(p_identifier->name);
 
-		id_type = local.get_datatype();
+		id_type.type = local.get_datatype();
 
 		// Check initializer as the first assignment.
 		switch (local.type) {
@@ -2187,7 +2188,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 			base.type.is_meta_type = p_context.current_function && p_context.current_function->is_static;
 
 			if (_guess_identifier_type_from_base(p_context, base, p_identifier->name, base_identifier)) {
-				id_type = base_identifier.type;
+				id_type = base_identifier;
 			}
 		}
 	}
@@ -2227,7 +2228,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 				c.current_line = type_test->operand->start_line;
 				c.current_suite = suite;
 				if (type_test->test_datatype.is_hard_type()) {
-					id_type = type_test->test_datatype;
+					id_type.type = type_test->test_datatype;
 					if (last_assign_line < c.current_line) {
 						// Override last assignment.
 						last_assign_line = c.current_line;
@@ -2245,10 +2246,10 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 		c.current_line = last_assign_line;
 		GDScriptCompletionIdentifier assigned_type;
 		if (_guess_expression_type(c, last_assigned_expression, assigned_type)) {
-			if (id_type.is_set() && assigned_type.type.is_set() && !GDScriptAnalyzer::check_type_compatibility(id_type, assigned_type.type)) {
+			if (id_type.type.is_set() && assigned_type.type.is_set() && !GDScriptAnalyzer::check_type_compatibility(id_type.type, assigned_type.type)) {
 				// The assigned type is incompatible. The annotated type takes priority.
+				r_type = id_type;
 				r_type.assigned_expression = last_assigned_expression;
-				r_type.type = id_type;
 			} else {
 				r_type = assigned_type;
 			}
@@ -2266,8 +2267,8 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 						GDScriptParser::FunctionNode *parent_function = base_type.class_type->get_member(p_context.current_function->identifier->name).function;
 						if (parent_function->parameters_indices.has(p_identifier->name)) {
 							const GDScriptParser::ParameterNode *parameter = parent_function->parameters[parent_function->parameters_indices[p_identifier->name]];
-							if ((!id_type.is_set() || id_type.is_variant()) && parameter->get_datatype().is_hard_type()) {
-								id_type = parameter->get_datatype();
+							if ((!id_type.type.is_set() || id_type.type.is_variant()) && parameter->get_datatype().is_hard_type()) {
+								id_type.type = parameter->get_datatype();
 							}
 							if (parameter->initializer) {
 								GDScriptParser::CompletionContext c = p_context;
@@ -2283,7 +2284,7 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 					base_type = base_type.class_type->base_type;
 					break;
 				case GDScriptParser::DataType::NATIVE: {
-					if (id_type.is_set() && !id_type.is_variant()) {
+					if (id_type.type.is_set() && !id_type.type.is_variant()) {
 						base_type = GDScriptParser::DataType();
 						break;
 					}
@@ -2304,8 +2305,8 @@ static bool _guess_identifier_type(GDScriptParser::CompletionContext &p_context,
 		}
 	}
 
-	if (id_type.is_set() && !id_type.is_variant()) {
-		r_type.type = id_type;
+	if (id_type.type.is_set() && !id_type.type.is_variant()) {
+		r_type = id_type;
 		return true;
 	}
 
@@ -3197,7 +3198,7 @@ ::Error GDScriptLanguage::complete_code(const String &p_code, const String &p_pa
 				break;
 			}
 			const GDScriptParser::AnnotationNode *annotation = static_cast<const GDScriptParser::AnnotationNode *>(completion_context.node);
-			_find_annotation_arguments(annotation, completion_context.current_argument, quote_style, options);
+			_find_annotation_arguments(annotation, completion_context.current_argument, quote_style, options, r_call_hint);
 			r_forced = true;
 		} break;
 		case GDScriptParser::COMPLETION_BUILT_IN_TYPE_CONSTANT_OR_STATIC_METHOD: {
@@ -3321,11 +3322,36 @@ ::Error GDScriptLanguage::complete_code(const String &p_code, const String &p_pa
 		case GDScriptParser::COMPLETION_SUBSCRIPT: {
 			const GDScriptParser::SubscriptNode *subscript = static_cast<const GDScriptParser::SubscriptNode *>(completion_context.node);
 			GDScriptCompletionIdentifier base;
-			if (!_guess_expression_type(completion_context, subscript->base, base)) {
-				break;
-			}
+			const bool res = _guess_expression_type(completion_context, subscript->base, base);
+
+			// If the type is not known, we assume it is BUILTIN, since indices on arrays is the most common use case.
+			if (!subscript->is_attribute && (!res || base.type.kind == GDScriptParser::DataType::BUILTIN || base.type.is_variant())) {
+				if (base.value.get_type() == Variant::DICTIONARY) {
+					List<PropertyInfo> members;
+					base.value.get_property_list(&members);
 
-			_find_identifiers_in_base(base, false, false, options, 0);
+					for (const PropertyInfo &E : members) {
+						ScriptLanguage::CodeCompletionOption option(E.name.quote(quote_style), ScriptLanguage::CODE_COMPLETION_KIND_MEMBER, ScriptLanguage::LOCATION_LOCAL);
+						options.insert(option.display, option);
+					}
+				}
+				if (!subscript->index || subscript->index->type != GDScriptParser::Node::LITERAL) {
+					_find_identifiers(completion_context, false, options, 0);
+				}
+			} else if (res) {
+				if (!subscript->is_attribute) {
+					// Quote the options if they are not accessed as attribute.
+
+					HashMap<String, ScriptLanguage::CodeCompletionOption> opt;
+					_find_identifiers_in_base(base, false, false, opt, 0);
+					for (const KeyValue<String, CodeCompletionOption> &E : opt) {
+						ScriptLanguage::CodeCompletionOption option(E.value.insert_text.quote(quote_style), E.value.kind, E.value.location);
+						options.insert(option.display, option);
+					}
+				} else {
+					_find_identifiers_in_base(base, false, false, options, 0);
+				}
+			}
 		} break;
 		case GDScriptParser::COMPLETION_TYPE_ATTRIBUTE: {
 			if (!completion_context.current_class) {

modules/gdscript/gdscript_parser.cpp

@@ -251,7 +251,7 @@ void GDScriptParser::override_completion_context(const Node *p_for_node, Complet
 	if (!for_completion) {
 		return;
 	}
-	if (completion_context.node != p_for_node) {
+	if (p_for_node == nullptr || completion_context.node != p_for_node) {
 		return;
 	}
 	CompletionContext context;
@@ -262,11 +262,12 @@ void GDScriptParser::override_completion_context(const Node *p_for_node, Complet
 	context.current_line = tokenizer->get_cursor_line();
 	context.current_argument = p_argument;
 	context.node = p_node;
+	context.parser = this;
 	completion_context = context;
 }
 
-void GDScriptParser::make_completion_context(CompletionType p_type, Node *p_node, int p_argument) {
-	if (!for_completion) {
+void GDScriptParser::make_completion_context(CompletionType p_type, Node *p_node, int p_argument, bool p_force) {
+	if (!for_completion || (!p_force && completion_context.type != COMPLETION_NONE)) {
 		return;
 	}
 	if (previous.cursor_place != GDScriptTokenizerText::CURSOR_MIDDLE && previous.cursor_place != GDScriptTokenizerText::CURSOR_END && current.cursor_place == GDScriptTokenizerText::CURSOR_NONE) {
@@ -284,8 +285,8 @@ void GDScriptParser::make_completion_context(CompletionType p_type, Node *p_node
 	completion_context = context;
 }
 
-void GDScriptParser::make_completion_context(CompletionType p_type, Variant::Type p_builtin_type) {
-	if (!for_completion) {
+void GDScriptParser::make_completion_context(CompletionType p_type, Variant::Type p_builtin_type, bool p_force) {
+	if (!for_completion || (!p_force && completion_context.type != COMPLETION_NONE)) {
 		return;
 	}
 	if (previous.cursor_place != GDScriptTokenizerText::CURSOR_MIDDLE && previous.cursor_place != GDScriptTokenizerText::CURSOR_END && current.cursor_place == GDScriptTokenizerText::CURSOR_NONE) {
@@ -1640,23 +1641,29 @@ GDScriptParser::AnnotationNode *GDScriptParser::parse_annotation(uint32_t p_vali
 		advance();
 		// Arguments.
 		push_completion_call(annotation);
-		make_completion_context(COMPLETION_ANNOTATION_ARGUMENTS, annotation, 0);
 		int argument_index = 0;
 		do {
+			make_completion_context(COMPLETION_ANNOTATION_ARGUMENTS, annotation, argument_index);
+			set_last_completion_call_arg(argument_index);
 			if (check(GDScriptTokenizer::Token::PARENTHESIS_CLOSE)) {
 				// Allow for trailing comma.
 				break;
 			}
 
-			make_completion_context(COMPLETION_ANNOTATION_ARGUMENTS, annotation, argument_index);
-			set_last_completion_call_arg(argument_index++);
 			ExpressionNode *argument = parse_expression(false);
+
 			if (argument == nullptr) {
 				push_error("Expected expression as the annotation argument.");
 				valid = false;
-				continue;
+			} else {
+				annotation->arguments.push_back(argument);
+
+				if (argument->type == Node::LITERAL) {
+					override_completion_context(argument, COMPLETION_ANNOTATION_ARGUMENTS, annotation, argument_index);
+				}
 			}
-			annotation->arguments.push_back(argument);
+
+			argument_index++;
 		} while (match(GDScriptTokenizer::Token::COMMA) && !is_at_end());
 
 		pop_multiline();
@@ -2472,7 +2479,7 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_precedence(Precedence p_pr
 	}
 
 	// Completion can appear whenever an expression is expected.
-	make_completion_context(COMPLETION_IDENTIFIER, nullptr);
+	make_completion_context(COMPLETION_IDENTIFIER, nullptr, -1, false);
 
 	GDScriptTokenizer::Token token = current;
 	GDScriptTokenizer::Token::Type token_type = token.type;
@@ -2489,8 +2496,17 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_precedence(Precedence p_pr
 
 	advance(); // Only consume the token if there's a valid rule.
 
+	// After a token was consumed, update the completion context regardless of a previously set context.
+
 	ExpressionNode *previous_operand = (this->*prefix_rule)(nullptr, p_can_assign);
 
+#ifdef TOOLS_ENABLED
+	// HACK: We can't create a context in parse_identifier since it is used in places were we don't want completion.
+	if (previous_operand != nullptr && previous_operand->type == GDScriptParser::Node::IDENTIFIER && prefix_rule == static_cast<ParseFunction>(&GDScriptParser::parse_identifier)) {
+		make_completion_context(COMPLETION_IDENTIFIER, previous_operand);
+	}
+#endif
+
 	while (p_precedence <= get_rule(current.type)->precedence) {
 		if (previous_operand == nullptr || (p_stop_on_assign && current.type == GDScriptTokenizer::Token::EQUAL) || lambda_ended) {
 			return previous_operand;
@@ -2925,6 +2941,11 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_assignment(ExpressionNode
 	}
 	assignment->assignee = p_previous_operand;
 	assignment->assigned_value = parse_expression(false);
+#ifdef TOOLS_ENABLED
+	if (assignment->assigned_value != nullptr && assignment->assigned_value->type == GDScriptParser::Node::IDENTIFIER) {
+		override_completion_context(assignment->assigned_value, COMPLETION_ASSIGN, assignment);
+	}
+#endif
 	if (assignment->assigned_value == nullptr) {
 		push_error(R"(Expected an expression after "=".)");
 	}
@@ -3124,6 +3145,12 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_subscript(ExpressionNode *
 	subscript->base = p_previous_operand;
 	subscript->index = parse_expression(false);
 
+#ifdef TOOLS_ENABLED
+	if (subscript->index != nullptr && subscript->index->type == Node::LITERAL) {
+		override_completion_context(subscript->index, COMPLETION_SUBSCRIPT, subscript);
+	}
+#endif
+
 	if (subscript->index == nullptr) {
 		push_error(R"(Expected expression after "[".)");
 	}

modules/gdscript/gdscript_parser.h

@@ -1457,8 +1457,11 @@ class GDScriptParser {
 	}
 	void apply_pending_warnings();
 #endif
-	void make_completion_context(CompletionType p_type, Node *p_node, int p_argument = -1);
-	void make_completion_context(CompletionType p_type, Variant::Type p_builtin_type);
+	// Setting p_force to false will prevent the completion context from being update if a context was already set before.
+	// This should only be done when we push context before we consumed any tokens for the corresponding structure.
+	// See parse_precedence for an example.
+	void make_completion_context(CompletionType p_type, Node *p_node, int p_argument = -1, bool p_force = true);
+	void make_completion_context(CompletionType p_type, Variant::Type p_builtin_type, bool p_force = true);
 	// In some cases it might become necessary to alter the completion context after parsing a subexpression.
 	// For example to not override COMPLETE_CALL_ARGUMENTS with COMPLETION_NONE from string literals.
 	void override_completion_context(const Node *p_for_node, CompletionType p_type, Node *p_node, int p_argument = -1);

modules/gdscript/tests/scripts/completion/assignment_options/enum_attribute.cfg

@@ -0,0 +1,19 @@
+[output]
+include=[
+    {"display": "new(…)"},
+    {"display": "SIZE_EXPAND"},
+    {"display": "SIZE_EXPAND_FILL"},
+    {"display": "SIZE_FILL"},
+    {"display": "SIZE_SHRINK_BEGIN"},
+    {"display": "SIZE_SHRINK_CENTER"},
+    {"display": "SIZE_SHRINK_END"},
+]
+exclude=[
+    {"display": "Control.SIZE_EXPAND"},
+    {"display": "Control.SIZE_EXPAND_FILL"},
+    {"display": "Control.SIZE_FILL"},
+    {"display": "Control.SIZE_SHRINK_BEGIN"},
+    {"display": "Control.SIZE_SHRINK_CENTER"},
+    {"display": "Control.SIZE_SHRINK_END"},
+    {"display": "contro_var"}
+]

modules/gdscript/tests/scripts/completion/assignment_options/enum_attribute.gd

@@ -0,0 +1,7 @@
+extends Control
+
+var contro_var
+
+func _ready():
+    size_flags_horizontal = Control.➡
+    pass

modules/gdscript/tests/scripts/completion/assignment_options/enum_attribute_identifier.cfg

@@ -0,0 +1,19 @@
+[output]
+include=[
+    {"display": "new(…)"},
+    {"display": "SIZE_EXPAND"},
+    {"display": "SIZE_EXPAND_FILL"},
+    {"display": "SIZE_FILL"},
+    {"display": "SIZE_SHRINK_BEGIN"},
+    {"display": "SIZE_SHRINK_CENTER"},
+    {"display": "SIZE_SHRINK_END"},
+]
+exclude=[
+    {"display": "Control.SIZE_EXPAND"},
+    {"display": "Control.SIZE_EXPAND_FILL"},
+    {"display": "Control.SIZE_FILL"},
+    {"display": "Control.SIZE_SHRINK_BEGIN"},
+    {"display": "Control.SIZE_SHRINK_CENTER"},
+    {"display": "Control.SIZE_SHRINK_END"},
+    {"display": "contro_var"}
+]

modules/gdscript/tests/scripts/completion/assignment_options/enum_attribute_identifier.gd

@@ -0,0 +1,7 @@
+extends Control
+
+var contro_var
+
+func _ready():
+    size_flags_horizontal = Control.SIZE➡
+    pass

modules/gdscript/tests/scripts/completion/assignment_options/enum_identifier.cfg

@@ -0,0 +1,12 @@
+[output]
+include=[
+    {"display": "Control.SIZE_EXPAND"},
+    {"display": "Control.SIZE_EXPAND_FILL"},
+    {"display": "Control.SIZE_FILL"},
+    {"display": "Control.SIZE_SHRINK_BEGIN"},
+    {"display": "Control.SIZE_SHRINK_CENTER"},
+    {"display": "Control.SIZE_SHRINK_END"},
+]
+exclude=[
+    {"display": "contro_var"}
+]

modules/gdscript/tests/scripts/completion/assignment_options/enum_identifier.gd

@@ -0,0 +1,7 @@
+extends Control
+
+var contro_var
+
+func _ready():
+    size_flags_horizontal = Con➡
+    pass

modules/gdscript/tests/scripts/completion/assignment_options/enum_no_identifier.cfg

@@ -0,0 +1,12 @@
+[output]
+include=[
+    {"display": "Control.SIZE_EXPAND"},
+    {"display": "Control.SIZE_EXPAND_FILL"},
+    {"display": "Control.SIZE_FILL"},
+    {"display": "Control.SIZE_SHRINK_BEGIN"},
+    {"display": "Control.SIZE_SHRINK_CENTER"},
+    {"display": "Control.SIZE_SHRINK_END"},
+]
+exclude=[
+    {"display": "contro_var"}
+]