Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GDScript: Add validation for @export_node_path annotation arguments #79935

Merged
merged 1 commit into from
Aug 1, 2023
Merged

GDScript: Add validation for @export_node_path annotation arguments #79935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/gdscript/doc_classes/@GDScript.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@
[codeblock]
@export_node_path("Button", "TouchScreenButton") var some_button
[/codeblock]
[b]Note:[/b] The type must be a native class or a globally registered script (using the [code]class_name[/code] keyword) that inherits [Node].
</description>
</annotation>
<annotation name="@export_placeholder">
Expand Down
18 changes: 15 additions & 3 deletions modules/gdscript/gdscript_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,16 +808,28 @@ static void _find_annotation_arguments(const GDScriptParser::AnnotationNode *p_a
ScriptLanguage::CodeCompletionOption node("Node", ScriptLanguage::CODE_COMPLETION_KIND_CLASS);
node.insert_text = node.display.quote(p_quote_style);
r_result.insert(node.display, node);
List<StringName> node_types;
ClassDB::get_inheriters_from_class("Node", &node_types);
for (const StringName &E : node_types) {

List<StringName> native_classes;
ClassDB::get_inheriters_from_class("Node", &native_classes);
for (const StringName &E : native_classes) {
if (!ClassDB::is_class_exposed(E)) {
continue;
}
ScriptLanguage::CodeCompletionOption option(E, ScriptLanguage::CODE_COMPLETION_KIND_CLASS);
option.insert_text = option.display.quote(p_quote_style);
r_result.insert(option.display, option);
}

List<StringName> global_script_classes;
ScriptServer::get_global_class_list(&global_script_classes);
for (const StringName &E : global_script_classes) {
if (!ClassDB::is_parent_class(ScriptServer::get_global_class_native_base(E), "Node")) {
continue;
}
ScriptLanguage::CodeCompletionOption option(E, ScriptLanguage::CODE_COMPLETION_KIND_CLASS);
option.insert_text = option.display.quote(p_quote_style);
r_result.insert(option.display, option);
}
} else if (p_annotation->name == SNAME("@warning_ignore")) {
for (int warning_code = 0; warning_code < GDScriptWarning::WARNING_MAX; warning_code++) {
ScriptLanguage::CodeCompletionOption warning(GDScriptWarning::get_name_from_code((GDScriptWarning::Code)warning_code).to_lower(), ScriptLanguage::CODE_COMPLETION_KIND_PLAIN_TEXT);
Expand Down
18 changes: 16 additions & 2 deletions modules/gdscript/gdscript_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3867,6 +3867,7 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node
}
}

// WARNING: Do not merge with the previous `if` because there `!=`, not `==`!
if (p_annotation->name == SNAME("@export_flags")) {
const int64_t max_flags = 32;
Vector<String> t = arg_string.split(":", true, 1);
Expand All @@ -3892,6 +3893,18 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node
push_error(vformat(R"(Invalid argument %d of annotation "@export_flags": Starting from argument %d, the flag value must be specified explicitly.)", i + 1, max_flags + 1), p_annotation->arguments[i]);
return false;
}
} else if (p_annotation->name == SNAME("@export_node_path")) {
String native_class = arg_string;
if (ScriptServer::is_global_class(arg_string)) {
native_class = ScriptServer::get_global_class_native_base(arg_string);
}
if (!ClassDB::class_exists(native_class)) {
push_error(vformat(R"(Invalid argument %d of annotation "@export_node_path": The class "%s" was not found in the global scope.)", i + 1, arg_string), p_annotation->arguments[i]);
return false;
} else if (!ClassDB::is_parent_class(native_class, SNAME("Node"))) {
push_error(vformat(R"(Invalid argument %d of annotation "@export_node_path": The class "%s" does not inherit "Node".)", i + 1, arg_string), p_annotation->arguments[i]);
return false;
}
}

if (i > 0) {
Expand All @@ -3909,8 +3922,7 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node
if (export_type.builtin_type == Variant::INT) {
variable->export_info.type = Variant::INT;
}
}
if (p_annotation->name == SNAME("@export_multiline")) {
} else if (p_annotation->name == SNAME("@export_multiline")) {
if (export_type.builtin_type == Variant::ARRAY && export_type.has_container_element_type()) {
DataType inner_type = export_type.get_container_element_type();
if (inner_type.builtin_type != Variant::STRING) {
Expand Down Expand Up @@ -3938,6 +3950,8 @@ bool GDScriptParser::export_annotations(const AnnotationNode *p_annotation, Node
}
}

// WARNING: Do not merge with the previous `else if`! Otherwise `else` (default variable type check)
// will not work for the above annotations. `@export` and `@export_enum` validate the type separately.
if (p_annotation->name == SNAME("@export")) {
if (variable->datatype_specifier == nullptr && variable->initializer == nullptr) {
push_error(R"(Cannot use simple "@export" annotation with variable without type or initializer, since type can't be inferred.)", p_annotation);
Expand Down
Loading