Skip to content

Commit

Permalink
Merge pull request #82843 from dalexeev/gds-export-node-only-in-nodes
Browse files Browse the repository at this point in the history 
GDScript: Add error when exporting node in non `Node`-derived classes
  • Loading branch information
@akien-mga
akien-mga committed Oct 5, 2023
2 parents af232e6 + 9e2273a commit cd7c50f
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 51 deletions.
27 changes: 26 additions & 1 deletion modules/gdscript/doc_classes/@GDScript.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,36 @@
<description>
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.
</description>
</annotation>
<annotation name="@export_category">
Expand Down
20 changes: 10 additions & 10 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
64 changes: 35 additions & 29 deletions modules/gdscript/gdscript_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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<ClassNode *>(p_node);
ClassNode *class_node = static_cast<ClassNode *>(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;
}
Expand All @@ -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<VariableNode *>(p_node);
VariableNode *variable = static_cast<VariableNode *>(p_target);
if (variable->is_static) {
push_error(R"("@onready" annotation cannot be applied to a static variable.)", p_annotation);
return false;
Expand All @@ -3910,10 +3910,11 @@ bool GDScriptParser::onready_annotation(const AnnotationNode *p_annotation, Node
}

template <PropertyHint t_hint, Variant::Type t_type>
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<VariableNode *>(p_node);
VariableNode *variable = static_cast<VariableNode *>(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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -4173,7 +4179,7 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node
}

template <PropertyUsageFlags t_usage>
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<AnnotationNode *>(p_annotation);

if (annotation->resolved_arguments.is_empty()) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}

Expand All @@ -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<FunctionNode *>(p_node);
FunctionNode *function = static_cast<FunctionNode *>(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;
Expand Down Expand Up @@ -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<ClassNode *>(p_target);
if (p_class->annotated_static_unload) {
ClassNode *class_node = static_cast<ClassNode *>(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;
}

Expand Down
20 changes: 10 additions & 10 deletions modules/gdscript/gdscript_parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1461,16 +1461,16 @@ class GDScriptParser {
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_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 <PropertyHint t_hint, Variant::Type t_type>
bool export_annotations(const AnnotationNode *p_annotation, Node *p_target);
bool export_annotations(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
template <PropertyUsageFlags t_usage>
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);
Expand Down
8 changes: 8 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.gd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# GH-82809

extends Resource

@export var node: Node

func test():
pass
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_1.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Node export is only supported in Node-derived classes, but the current class inherits "Resource".
9 changes: 9 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.gd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# GH-82809

extends Node

class Inner:
@export var node: Node

func test():
pass
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_2.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Node export is only supported in Node-derived classes, but the current class inherits "RefCounted".
8 changes: 8 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.gd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# GH-82809

extends Resource

@export var node_array: Array[Node]

func test():
pass
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/export_node_in_non_node_derived_class_3.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Node export is only supported in Node-derived classes, but the current class inherits "Resource".
Loading

0 comments on commit cd7c50f

Please sign in to comment.