From 9e2273abc7f24a7652889a1936b0d8ff71353d60 Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Thu, 5 Oct 2023 13:50:26 +0300 Subject: [PATCH] GDScript: Add error when exporting node in non [Node]-derived classes --- modules/gdscript/doc_classes/@GDScript.xml | 27 +++++++- modules/gdscript/gdscript_analyzer.cpp | 20 +++--- modules/gdscript/gdscript_parser.cpp | 64 ++++++++++--------- modules/gdscript/gdscript_parser.h | 20 +++--- ...export_node_in_non_node_derived_class_1.gd | 8 +++ ...xport_node_in_non_node_derived_class_1.out | 2 + ...export_node_in_non_node_derived_class_2.gd | 9 +++ ...xport_node_in_non_node_derived_class_2.out | 2 + ...export_node_in_non_node_derived_class_3.gd | 8 +++ ...xport_node_in_non_node_derived_class_3.out | 2 + .../parser/features/export_variable.gd | 7 +- .../parser/features/export_variable.out | 2 + 12 files changed, 120 insertions(+), 51 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.out diff --git a/modules/gdscript/doc_classes/@GDScript.xml b/modules/gdscript/doc_classes/@GDScript.xml index 4fec745999c6..3da6bcf10c89 100644 --- a/modules/gdscript/doc_classes/@GDScript.xml +++ b/modules/gdscript/doc_classes/@GDScript.xml @@ -285,11 +285,36 @@ Mark the following property as exported (editable in the Inspector dock and saved to disk). To control the type of the exported property, use the type hint notation. [codeblock] + extends Node + + enum Direction {LEFT, RIGHT, UP, DOWN} + + # Built-in types. @export var string = "" @export var int_number = 5 @export var float_number: float = 5 + + # Enums. + @export var type: Variant.Type + @export var format: Image.Format + @export var direction: Direction + + # Resources. @export var image: Image - [/codeblock] + @export var custom_resource: CustomResource + + # Nodes. + @export var node: Node + @export var custom_node: CustomNode + + # Typed arrays. + @export var int_array: Array[int] + @export var direction_array: Array[Direction] + @export var image_array: Array[Image] + @export var node_array: Array[Node] + [/codeblock] + [b]Note:[/b] Custom resources and nodes must be registered as global classes using [code]class_name[/code]. + [b]Note:[/b] Node export is only supported in [Node]-derived classes and has a number of other limitations. diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index cdeaa70e5f6d..831971c3f3d7 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -910,7 +910,7 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class, for (GDScriptParser::AnnotationNode *&E : member_node->annotations) { if (E->name == SNAME("@warning_ignore")) { resolve_annotation(E); - E->apply(parser, member.variable); + E->apply(parser, member.variable, p_class); } } for (GDScriptWarning::Code ignored_warning : member_node->ignored_warnings) { @@ -933,7 +933,7 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class, for (GDScriptParser::AnnotationNode *&E : member.variable->annotations) { if (E->name != SNAME("@warning_ignore")) { resolve_annotation(E); - E->apply(parser, member.variable); + E->apply(parser, member.variable, p_class); } } @@ -985,7 +985,7 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class, // Apply annotations. for (GDScriptParser::AnnotationNode *&E : member.constant->annotations) { resolve_annotation(E); - E->apply(parser, member.constant); + E->apply(parser, member.constant, p_class); } } break; case GDScriptParser::ClassNode::Member::SIGNAL: { @@ -1015,7 +1015,7 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class, // Apply annotations. for (GDScriptParser::AnnotationNode *&E : member.signal->annotations) { resolve_annotation(E); - E->apply(parser, member.signal); + E->apply(parser, member.signal, p_class); } } break; case GDScriptParser::ClassNode::Member::ENUM: { @@ -1063,13 +1063,13 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class, // Apply annotations. for (GDScriptParser::AnnotationNode *&E : member.m_enum->annotations) { resolve_annotation(E); - E->apply(parser, member.m_enum); + E->apply(parser, member.m_enum, p_class); } } break; case GDScriptParser::ClassNode::Member::FUNCTION: for (GDScriptParser::AnnotationNode *&E : member.function->annotations) { resolve_annotation(E); - E->apply(parser, member.function); + E->apply(parser, member.function, p_class); } resolve_function_signature(member.function, p_source); break; @@ -1279,7 +1279,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co // Apply annotations. for (GDScriptParser::AnnotationNode *&E : member.function->annotations) { resolve_annotation(E); - E->apply(parser, member.function); + E->apply(parser, member.function, p_class); } resolve_function_body(member.function); } else if (member.type == GDScriptParser::ClassNode::Member::VARIABLE && member.variable->property != GDScriptParser::VariableNode::PROP_NONE) { @@ -1301,7 +1301,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co } else if (member.type == GDScriptParser::ClassNode::Member::GROUP) { // Apply annotation (`@export_{category,group,subgroup}`). resolve_annotation(member.annotation); - member.annotation->apply(parser, nullptr); + member.annotation->apply(parser, nullptr, p_class); } } @@ -1837,7 +1837,7 @@ void GDScriptAnalyzer::resolve_suite(GDScriptParser::SuiteNode *p_suite) { // Apply annotations. for (GDScriptParser::AnnotationNode *&E : stmt->annotations) { resolve_annotation(E); - E->apply(parser, stmt); + E->apply(parser, stmt, nullptr); } #ifdef DEBUG_ENABLED @@ -5544,7 +5544,7 @@ Error GDScriptAnalyzer::analyze() { // Apply annotations. for (GDScriptParser::AnnotationNode *&E : parser->head->annotations) { resolve_annotation(E); - E->apply(parser, parser->head); + E->apply(parser, parser->head, nullptr); } resolve_interface(); diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 9fb1030d12c0..057c2b49ab3b 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -519,7 +519,7 @@ void GDScriptParser::parse_program() { if (annotation->applies_to(AnnotationInfo::SCRIPT)) { // `@icon` needs to be applied in the parser. See GH-72444. if (annotation->name == SNAME("@icon")) { - annotation->apply(this, head); + annotation->apply(this, head, nullptr); } else { head->annotations.push_back(annotation); } @@ -3795,12 +3795,12 @@ const GDScriptParser::SuiteNode::Local &GDScriptParser::SuiteNode::get_local(con return empty; } -bool GDScriptParser::AnnotationNode::apply(GDScriptParser *p_this, Node *p_target) { +bool GDScriptParser::AnnotationNode::apply(GDScriptParser *p_this, Node *p_target, ClassNode *p_class) { if (is_applied) { return true; } is_applied = true; - return (p_this->*(p_this->valid_annotations[name].apply))(this, p_target); + return (p_this->*(p_this->valid_annotations[name].apply))(this, p_target, p_class); } bool GDScriptParser::AnnotationNode::applies_to(uint32_t p_target_kinds) const { @@ -3846,7 +3846,7 @@ bool GDScriptParser::validate_annotation_arguments(AnnotationNode *p_annotation) return true; } -bool GDScriptParser::tool_annotation(const AnnotationNode *p_annotation, Node *p_node) { +bool GDScriptParser::tool_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { #ifdef DEBUG_ENABLED if (this->_is_tool) { push_error(R"("@tool" annotation can only be used once.)", p_annotation); @@ -3857,15 +3857,15 @@ bool GDScriptParser::tool_annotation(const AnnotationNode *p_annotation, Node *p return true; } -bool GDScriptParser::icon_annotation(const AnnotationNode *p_annotation, Node *p_node) { - ERR_FAIL_COND_V_MSG(p_node->type != Node::CLASS, false, R"("@icon" annotation can only be applied to classes.)"); +bool GDScriptParser::icon_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { + ERR_FAIL_COND_V_MSG(p_target->type != Node::CLASS, false, R"("@icon" annotation can only be applied to classes.)"); ERR_FAIL_COND_V(p_annotation->resolved_arguments.is_empty(), false); - ClassNode *p_class = static_cast(p_node); + ClassNode *class_node = static_cast(p_target); String path = p_annotation->resolved_arguments[0]; #ifdef DEBUG_ENABLED - if (!p_class->icon_path.is_empty()) { + if (!class_node->icon_path.is_empty()) { push_error(R"("@icon" annotation can only be used once.)", p_annotation); return false; } @@ -3875,27 +3875,27 @@ bool GDScriptParser::icon_annotation(const AnnotationNode *p_annotation, Node *p } #endif // DEBUG_ENABLED - p_class->icon_path = path; + class_node->icon_path = path; if (path.is_empty() || path.is_absolute_path()) { - p_class->simplified_icon_path = path.simplify_path(); + class_node->simplified_icon_path = path.simplify_path(); } else if (path.is_relative_path()) { - p_class->simplified_icon_path = script_path.get_base_dir().path_join(path).simplify_path(); + class_node->simplified_icon_path = script_path.get_base_dir().path_join(path).simplify_path(); } else { - p_class->simplified_icon_path = path; + class_node->simplified_icon_path = path; } return true; } -bool GDScriptParser::onready_annotation(const AnnotationNode *p_annotation, Node *p_node) { - ERR_FAIL_COND_V_MSG(p_node->type != Node::VARIABLE, false, R"("@onready" annotation can only be applied to class variables.)"); +bool GDScriptParser::onready_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { + ERR_FAIL_COND_V_MSG(p_target->type != Node::VARIABLE, false, R"("@onready" annotation can only be applied to class variables.)"); if (current_class && !ClassDB::is_parent_class(current_class->get_datatype().native_type, SNAME("Node"))) { push_error(R"("@onready" can only be used in classes that inherit "Node".)", p_annotation); } - VariableNode *variable = static_cast(p_node); + VariableNode *variable = static_cast(p_target); if (variable->is_static) { push_error(R"("@onready" annotation cannot be applied to a static variable.)", p_annotation); return false; @@ -3910,10 +3910,11 @@ bool GDScriptParser::onready_annotation(const AnnotationNode *p_annotation, Node } template -bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node *p_node) { - ERR_FAIL_COND_V_MSG(p_node->type != Node::VARIABLE, false, vformat(R"("%s" annotation can only be applied to variables.)", p_annotation->name)); +bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { + ERR_FAIL_COND_V_MSG(p_target->type != Node::VARIABLE, false, vformat(R"("%s" annotation can only be applied to variables.)", p_annotation->name)); + ERR_FAIL_NULL_V(p_class, false); - VariableNode *variable = static_cast(p_node); + VariableNode *variable = static_cast(p_target); if (variable->is_static) { push_error(vformat(R"(Annotation "%s" cannot be applied to a static variable.)", p_annotation->name), p_annotation); return false; @@ -4128,7 +4129,12 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node } break; default: push_error(R"(Export type can only be built-in, a resource, a node, or an enum.)", variable); - break; + return false; + } + + if (variable->export_info.hint == PROPERTY_HINT_NODE_TYPE && !ClassDB::is_parent_class(p_class->base_type.native_type, SNAME("Node"))) { + push_error(vformat(R"(Node export is only supported in Node-derived classes, but the current class inherits "%s".)", p_class->base_type.to_string()), variable); + return false; } if (is_array) { @@ -4173,7 +4179,7 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node } template -bool GDScriptParser::export_group_annotations(const AnnotationNode *p_annotation, Node *p_node) { +bool GDScriptParser::export_group_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { AnnotationNode *annotation = const_cast(p_annotation); if (annotation->resolved_arguments.is_empty()) { @@ -4205,7 +4211,7 @@ bool GDScriptParser::export_group_annotations(const AnnotationNode *p_annotation return true; } -bool GDScriptParser::warning_annotations(const AnnotationNode *p_annotation, Node *p_node) { +bool GDScriptParser::warning_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { #ifdef DEBUG_ENABLED bool has_error = false; for (const Variant &warning_name : p_annotation->resolved_arguments) { @@ -4214,7 +4220,7 @@ bool GDScriptParser::warning_annotations(const AnnotationNode *p_annotation, Nod push_error(vformat(R"(Invalid warning name: "%s".)", warning_name), p_annotation); has_error = true; } else { - p_node->ignored_warnings.push_back(warning); + p_target->ignored_warnings.push_back(warning); } } @@ -4226,10 +4232,10 @@ bool GDScriptParser::warning_annotations(const AnnotationNode *p_annotation, Nod #endif // DEBUG_ENABLED } -bool GDScriptParser::rpc_annotation(const AnnotationNode *p_annotation, Node *p_node) { - ERR_FAIL_COND_V_MSG(p_node->type != Node::FUNCTION, false, vformat(R"("%s" annotation can only be applied to functions.)", p_annotation->name)); +bool GDScriptParser::rpc_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { + ERR_FAIL_COND_V_MSG(p_target->type != Node::FUNCTION, false, vformat(R"("%s" annotation can only be applied to functions.)", p_annotation->name)); - FunctionNode *function = static_cast(p_node); + FunctionNode *function = static_cast(p_target); if (function->rpc_config.get_type() != Variant::NIL) { push_error(R"(RPC annotations can only be used once per function.)", p_annotation); return false; @@ -4287,14 +4293,14 @@ bool GDScriptParser::rpc_annotation(const AnnotationNode *p_annotation, Node *p_ return true; } -bool GDScriptParser::static_unload_annotation(const AnnotationNode *p_annotation, Node *p_target) { +bool GDScriptParser::static_unload_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) { ERR_FAIL_COND_V_MSG(p_target->type != Node::CLASS, false, vformat(R"("%s" annotation can only be applied to classes.)", p_annotation->name)); - ClassNode *p_class = static_cast(p_target); - if (p_class->annotated_static_unload) { + ClassNode *class_node = static_cast(p_target); + if (class_node->annotated_static_unload) { push_error(vformat(R"("%s" annotation can only be used once per script.)", p_annotation->name), p_annotation); return false; } - p_class->annotated_static_unload = true; + class_node->annotated_static_unload = true; return true; } diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 2daadd7e6aee..62a2f4f98c0b 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -363,7 +363,7 @@ class GDScriptParser { bool is_resolved = false; bool is_applied = false; - bool apply(GDScriptParser *p_this, Node *p_target); + bool apply(GDScriptParser *p_this, Node *p_target, ClassNode *p_class); bool applies_to(uint32_t p_target_kinds) const; AnnotationNode() { @@ -1340,7 +1340,7 @@ class GDScriptParser { bool in_lambda = false; bool lambda_ended = false; // Marker for when a lambda ends, to apply an end of statement if needed. - typedef bool (GDScriptParser::*AnnotationAction)(const AnnotationNode *p_annotation, Node *p_target); + typedef bool (GDScriptParser::*AnnotationAction)(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); struct AnnotationInfo { enum TargetKind { NONE = 0, @@ -1461,16 +1461,16 @@ class GDScriptParser { bool register_annotation(const MethodInfo &p_info, uint32_t p_target_kinds, AnnotationAction p_apply, const Vector &p_default_arguments = Vector(), bool p_is_vararg = false); 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); - bool onready_annotation(const AnnotationNode *p_annotation, Node *p_target); + bool tool_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); + bool icon_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); + bool onready_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); template - bool export_annotations(const AnnotationNode *p_annotation, Node *p_target); + bool export_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); template - bool export_group_annotations(const AnnotationNode *p_annotation, Node *p_target); - bool warning_annotations(const AnnotationNode *p_annotation, Node *p_target); - bool rpc_annotation(const AnnotationNode *p_annotation, Node *p_target); - bool static_unload_annotation(const AnnotationNode *p_annotation, Node *p_target); + bool export_group_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); + bool warning_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); + bool rpc_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); + bool static_unload_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class); // Statements. Node *parse_statement(); VariableNode *parse_variable(bool p_is_static); diff --git a/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.gd b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.gd new file mode 100644 index 000000000000..05aa726a050c --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.gd @@ -0,0 +1,8 @@ +# GH-82809 + +extends Resource + +@export var node: Node + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.out b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.out new file mode 100644 index 000000000000..9a45bbb515aa --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Node export is only supported in Node-derived classes, but the current class inherits "Resource". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.gd b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.gd new file mode 100644 index 000000000000..c210e7c043b2 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.gd @@ -0,0 +1,9 @@ +# GH-82809 + +extends Node + +class Inner: + @export var node: Node + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.out b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.out new file mode 100644 index 000000000000..3da6d6d7ac23 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Node export is only supported in Node-derived classes, but the current class inherits "RefCounted". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.gd b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.gd new file mode 100644 index 000000000000..6f2c10464373 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.gd @@ -0,0 +1,8 @@ +# GH-82809 + +extends Resource + +@export var node_array: Array[Node] + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.out b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.out new file mode 100644 index 000000000000..9a45bbb515aa --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +Node export is only supported in Node-derived classes, but the current class inherits "Resource". diff --git a/modules/gdscript/tests/scripts/parser/features/export_variable.gd b/modules/gdscript/tests/scripts/parser/features/export_variable.gd index acf9ff2e215c..c9d05a7e684d 100644 --- a/modules/gdscript/tests/scripts/parser/features/export_variable.gd +++ b/modules/gdscript/tests/scripts/parser/features/export_variable.gd @@ -1,3 +1,5 @@ +extends Node + @export var example = 99 @export_range(0, 100) var example_range = 100 @export_range(0, 100, 1) var example_range_step = 101 @@ -6,7 +8,8 @@ @export var color: Color @export_color_no_alpha var color_no_alpha: Color @export_node_path("Sprite2D", "Sprite3D", "Control", "Node") var nodepath := ^"hello" - +@export var node: Node +@export var node_array: Array[Node] func test(): print(example) @@ -16,3 +19,5 @@ func test(): print(color) print(color_no_alpha) print(nodepath) + print(node) + print(var_to_str(node_array)) diff --git a/modules/gdscript/tests/scripts/parser/features/export_variable.out b/modules/gdscript/tests/scripts/parser/features/export_variable.out index bae35e75c69b..5430c975f4de 100644 --- a/modules/gdscript/tests/scripts/parser/features/export_variable.out +++ b/modules/gdscript/tests/scripts/parser/features/export_variable.out @@ -6,3 +6,5 @@ GDTEST_OK (0, 0, 0, 1) (0, 0, 0, 1) hello + +Array[Node]([])