Merge pull request #72454 from dalexeev/gds-fix-icon-annotation

GDScript: Fix `@icon` annotation
godotengine · Jan 31, 2023 · 925784d · 925784d
2 parents 1c42e14 + 83cb968
commit 925784d
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 10 deletions.
diff --git a/modules/gdscript/doc_classes/@GDScript.xml b/modules/gdscript/doc_classes/@GDScript.xml
@@ -563,6 +563,7 @@
 				[/codeblock]
 				[b]Note:[/b] Only the script can have a custom icon. Inner classes are not supported.
 				[b]Note:[/b] As annotations describe their subject, the [code]@icon[/code] annotation must be placed before the class definition and inheritance.
+				[b]Note:[/b] Unlike other annotations, the argument of the [code]@icon[/code] annotation must be a string literal (constant expressions are not supported).
 			</description>
 		</annotation>
 		<annotation name="@onready">

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
@@ -4847,11 +4847,6 @@ Ref<GDScriptParserRef> GDScriptAnalyzer::get_parser_for(const String &p_path) {
 }
 
 Error GDScriptAnalyzer::resolve_inheritance() {
-	// Apply annotations.
-	for (GDScriptParser::AnnotationNode *&E : parser->head->annotations) {
-		resolve_annotation(E);
-		E->apply(parser, parser->head);
-	}
 	return resolve_class_inheritance(parser->head, true);
 }
 
@@ -4885,6 +4880,12 @@ Error GDScriptAnalyzer::analyze() {
 		return err;
 	}
 
+	// Apply annotations.
+	for (GDScriptParser::AnnotationNode *&E : parser->head->annotations) {
+		resolve_annotation(E);
+		E->apply(parser, parser->head);
+	}
+
 	resolve_interface();
 	resolve_body();
 	if (!parser->errors.is_empty()) {

diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp
@@ -491,7 +491,12 @@ void GDScriptParser::parse_program() {
 		AnnotationNode *annotation = parse_annotation(AnnotationInfo::SCRIPT | AnnotationInfo::STANDALONE | AnnotationInfo::CLASS_LEVEL);
 		if (annotation != nullptr) {
 			if (annotation->applies_to(AnnotationInfo::SCRIPT)) {
-				head->annotations.push_back(annotation);
+				// `@icon` needs to be applied in the parser. See GH-72444.
+				if (annotation->name == SNAME("@icon")) {
+					annotation->apply(this, head);
+				} else {
+					head->annotations.push_back(annotation);
+				}
 			} else {
 				annotation_stack.push_back(annotation);
 				// This annotation must appear after script-level annotations
@@ -809,7 +814,7 @@ void GDScriptParser::parse_class_body(bool p_is_multiline) {
 						if (previous.type != GDScriptTokenizer::Token::NEWLINE) {
 							push_error(R"(Expected newline after a standalone annotation.)");
 						}
-						if (annotation->name == "@export_category" || annotation->name == "@export_group" || annotation->name == "@export_subgroup") {
+						if (annotation->name == SNAME("@export_category") || annotation->name == SNAME("@export_group") || annotation->name == SNAME("@export_subgroup")) {
 							current_class->add_member_group(annotation);
 						} else {
 							// For potential non-group standalone annotations.
@@ -1436,7 +1441,7 @@ GDScriptParser::AnnotationNode *GDScriptParser::parse_annotation(uint32_t p_vali
 	match(GDScriptTokenizer::Token::NEWLINE); // Newline after annotation is optional.
 
 	if (valid) {
-		valid = validate_annotation_argument_count(annotation);
+		valid = validate_annotation_arguments(annotation);
 	}
 
 	return valid ? annotation : nullptr;
@@ -3551,7 +3556,7 @@ bool GDScriptParser::AnnotationNode::applies_to(uint32_t p_target_kinds) const {
 	return (info->target_kind & p_target_kinds) > 0;
 }
 
-bool GDScriptParser::validate_annotation_argument_count(AnnotationNode *p_annotation) {
+bool GDScriptParser::validate_annotation_arguments(AnnotationNode *p_annotation) {
 	ERR_FAIL_COND_V_MSG(!valid_annotations.has(p_annotation->name), false, vformat(R"(Annotation "%s" not found to validate.)", p_annotation->name));
 
 	const MethodInfo &info = valid_annotations[p_annotation->name].info;
@@ -3566,6 +3571,27 @@ bool GDScriptParser::validate_annotation_argument_count(AnnotationNode *p_annota
 		return false;
 	}
 
+	// `@icon`'s argument needs to be resolved in the parser. See GH-72444.
+	if (p_annotation->name == SNAME("@icon")) {
+		ExpressionNode *argument = p_annotation->arguments[0];
+
+		if (argument->type != Node::LITERAL) {
+			push_error(R"(Argument 1 of annotation "@icon" must be a string literal.)", argument);
+			return false;
+		}
+
+		Variant value = static_cast<LiteralNode *>(argument)->value;
+
+		if (value.get_type() != Variant::STRING) {
+			push_error(R"(Argument 1 of annotation "@icon" must be a string literal.)", argument);
+			return false;
+		}
+
+		p_annotation->resolved_arguments.push_back(value);
+	}
+
+	// For other annotations, see `GDScriptAnalyzer::resolve_annotation()`.
+
 	return true;
 }
 

diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h
@@ -1402,7 +1402,7 @@ class GDScriptParser {
 	// Annotations
 	AnnotationNode *parse_annotation(uint32_t p_valid_targets);
 	bool register_annotation(const MethodInfo &p_info, uint32_t p_target_kinds, AnnotationAction p_apply, const Vector<Variant> &p_default_arguments = Vector<Variant>(), bool p_is_vararg = false);
-	bool validate_annotation_argument_count(AnnotationNode *p_annotation);
+	bool validate_annotation_arguments(AnnotationNode *p_annotation);
 	void clear_unused_annotations();
 	bool tool_annotation(const AnnotationNode *p_annotation, Node *p_target);
 	bool icon_annotation(const AnnotationNode *p_annotation, Node *p_target);