From 6590e1fd8a0770894626d012c2391f2ae1efa497 Mon Sep 17 00:00:00 2001 From: Aaron Franke Date: Sat, 22 Oct 2022 18:32:21 -0500 Subject: [PATCH] Make script annotations placed before class_name and extends --- modules/gdscript/gdscript_parser.cpp | 54 ++++--------------- .../errors/class_name_after_annotation.gd | 4 +- .../errors/class_name_after_annotation.out | 2 +- .../scripts/parser/features/class_name.gd | 2 +- 4 files changed, 15 insertions(+), 47 deletions(-) diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 103269f0f5c..9b55d6663c2 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -540,43 +540,28 @@ void GDScriptParser::parse_program() { head = alloc_node(); head->fqcn = script_path; current_class = head; + bool can_have_class_or_extends = true; - // If we happen to parse an annotation before extends or class_name keywords, track it. - // @tool is allowed, but others should fail. - AnnotationNode *premature_annotation = nullptr; - - if (match(GDScriptTokenizer::Token::ANNOTATION)) { - // Check for @tool, script-level, or standalone annotation. + while (match(GDScriptTokenizer::Token::ANNOTATION)) { AnnotationNode *annotation = parse_annotation(AnnotationInfo::SCRIPT | AnnotationInfo::STANDALONE | AnnotationInfo::CLASS_LEVEL); if (annotation != nullptr) { - if (annotation->name == SNAME("@tool")) { - // TODO: don't allow @tool anywhere else. (Should all script annotations be the first thing?). - _is_tool = true; - if (previous.type != GDScriptTokenizer::Token::NEWLINE) { - push_error(R"(Expected newline after "@tool" annotation.)"); - } - // @tool annotation has no specific target. - annotation->apply(this, nullptr); - } else if (annotation->applies_to(AnnotationInfo::SCRIPT | AnnotationInfo::STANDALONE)) { - premature_annotation = annotation; - if (previous.type != GDScriptTokenizer::Token::NEWLINE) { - push_error(R"(Expected newline after a standalone annotation.)"); - } + if (annotation->applies_to(AnnotationInfo::SCRIPT)) { annotation->apply(this, head); } else { - premature_annotation = annotation; annotation_stack.push_back(annotation); + // This annotation must appear after script-level annotations + // and class_name/extends (ex: could be @onready or @export), + // so we stop looking for script-level stuff. + can_have_class_or_extends = false; + break; } } } - for (bool should_break = false; !should_break;) { + while (can_have_class_or_extends) { // Order here doesn't matter, but there should be only one of each at most. switch (current.type) { case GDScriptTokenizer::Token::CLASS_NAME: - if (premature_annotation != nullptr) { - push_error(R"("class_name" should be used before annotations (except @tool).)"); - } advance(); if (head->identifier != nullptr) { push_error(R"("class_name" can only be used once.)"); @@ -585,9 +570,6 @@ void GDScriptParser::parse_program() { } break; case GDScriptTokenizer::Token::EXTENDS: - if (premature_annotation != nullptr) { - push_error(R"("extends" should be used before annotations (except @tool).)"); - } advance(); if (head->extends_used) { push_error(R"("extends" can only be used once.)"); @@ -597,7 +579,8 @@ void GDScriptParser::parse_program() { } break; default: - should_break = true; + // No tokens are allowed between script annotations and class/extends. + can_have_class_or_extends = false; break; } @@ -606,21 +589,6 @@ void GDScriptParser::parse_program() { } } - if (match(GDScriptTokenizer::Token::ANNOTATION)) { - // Check for a script-level, or standalone annotation. - AnnotationNode *annotation = parse_annotation(AnnotationInfo::SCRIPT | AnnotationInfo::STANDALONE | AnnotationInfo::CLASS_LEVEL); - if (annotation != nullptr) { - if (annotation->applies_to(AnnotationInfo::SCRIPT | AnnotationInfo::STANDALONE)) { - if (previous.type != GDScriptTokenizer::Token::NEWLINE) { - push_error(R"(Expected newline after a standalone annotation.)"); - } - annotation->apply(this, head); - } else { - annotation_stack.push_back(annotation); - } - } - } - parse_class_body(true); complete_extents(head); diff --git a/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.gd b/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.gd index ada60301329..ab8b056f99d 100644 --- a/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.gd +++ b/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.gd @@ -1,6 +1,6 @@ -# Error here. `class_name` should be used *before* annotations, not after (except @tool). -@icon("res://path/to/optional/icon.svg") +# Error here. Annotations should be used before `class_name`, not after. class_name HelloWorld +@icon("res://path/to/optional/icon.svg") func test(): pass diff --git a/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.out b/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.out index 02b33c86923..a598ff84240 100644 --- a/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.out +++ b/modules/gdscript/tests/scripts/parser/errors/class_name_after_annotation.out @@ -1,2 +1,2 @@ GDTEST_PARSER_ERROR -"class_name" should be used before annotations (except @tool). +Annotation "@icon" is not allowed in this level. diff --git a/modules/gdscript/tests/scripts/parser/features/class_name.gd b/modules/gdscript/tests/scripts/parser/features/class_name.gd index 8bd188e2477..19009e433dc 100644 --- a/modules/gdscript/tests/scripts/parser/features/class_name.gd +++ b/modules/gdscript/tests/scripts/parser/features/class_name.gd @@ -1,5 +1,5 @@ -class_name HelloWorld @icon("res://path/to/optional/icon.svg") +class_name HelloWorld func test(): pass