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.

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

GDScript: Add @warning_ignore_start and @warning_ignore_restore annotations #76020

Merged
merged 1 commit into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 28 additions & 1 deletion modules/gdscript/doc_classes/@GDScript.xml
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,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 [annotation @icon] annotation must be placed before the class definition and inheritance.
[b]Note:[/b] Unlike other annotations, the argument of the [annotation @icon] annotation must be a string literal (constant expressions are not supported).
[b]Note:[/b] Unlike most other annotations, the argument of the [annotation @icon] annotation must be a string literal (constant expressions are not supported).
</description>
</annotation>
<annotation name="@onready">
Expand Down Expand Up @@ -794,6 +794,33 @@
@warning_ignore("unreachable_code")
print("unreachable")
[/codeblock]
See also [annotation @warning_ignore_start] and [annotation @warning_ignore_restore].
</description>
</annotation>
<annotation name="@warning_ignore_restore" qualifiers="vararg">
<return type="void" />
<param index="0" name="warning" type="String" />
<description>
Stops ignoring the listed warning types after [annotation @warning_ignore_start]. Ignoring the specified warning types will be reset to Project Settings. This annotation can be omitted to ignore the warning types until the end of the file.
[b]Note:[/b] Unlike most other annotations, arguments of the [annotation @warning_ignore_restore] annotation must be string literals (constant expressions are not supported).
</description>
</annotation>
<annotation name="@warning_ignore_start" qualifiers="vararg">
<return type="void" />
<param index="0" name="warning" type="String" />
<description>
Starts ignoring the listed warning types until the end of the file or the [annotation @warning_ignore_restore] annotation with the given warning type.
[codeblock]
func test():
var a = 1 # Warning (if enabled in the Project Settings).
@warning_ignore_start("unused_variable")
var b = 2 # No warning.
var c = 3 # No warning.
@warning_ignore_restore("unused_variable")
var d = 4 # Warning (if enabled in the Project Settings).
[/codeblock]
[b]Note:[/b] To suppress a single warning, use [annotation @warning_ignore] instead.
[b]Note:[/b] Unlike most other annotations, arguments of the [annotation @warning_ignore_start] annotation must be string literals (constant expressions are not supported).
</description>
</annotation>
</annotations>
Expand Down
5 changes: 4 additions & 1 deletion modules/gdscript/gdscript_editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,11 @@ static void _find_annotation_arguments(const GDScriptParser::AnnotationNode *p_a
}
} break;
}
} else if (p_annotation->name == SNAME("@warning_ignore")) {
} else if (p_annotation->name == SNAME("@warning_ignore") || p_annotation->name == SNAME("@warning_ignore_start") || p_annotation->name == SNAME("@warning_ignore_restore")) {
for (int warning_code = 0; warning_code < GDScriptWarning::WARNING_MAX; warning_code++) {
if (warning_code == GDScriptWarning::RENAMED_IN_GODOT_4_HINT) {
continue;
}
ScriptLanguage::CodeCompletionOption warning(GDScriptWarning::get_name_from_code((GDScriptWarning::Code)warning_code).to_lower(), ScriptLanguage::CODE_COMPLETION_KIND_PLAIN_TEXT);
warning.insert_text = warning.display.quote(p_quote_style);
r_result.insert(warning.display, warning);
Expand Down
148 changes: 110 additions & 38 deletions modules/gdscript/gdscript_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ bool GDScriptParser::annotation_exists(const String &p_annotation_name) const {
GDScriptParser::GDScriptParser() {
// Register valid annotations.
if (unlikely(valid_annotations.is_empty())) {
// Script annotations.
register_annotation(MethodInfo("@tool"), AnnotationInfo::SCRIPT, &GDScriptParser::tool_annotation);
register_annotation(MethodInfo("@icon", PropertyInfo(Variant::STRING, "icon_path")), AnnotationInfo::SCRIPT, &GDScriptParser::icon_annotation);
register_annotation(MethodInfo("@static_unload"), AnnotationInfo::SCRIPT, &GDScriptParser::static_unload_annotation);

// Onready annotation.
register_annotation(MethodInfo("@onready"), AnnotationInfo::VARIABLE, &GDScriptParser::onready_annotation);
// Export annotations.
register_annotation(MethodInfo("@export"), AnnotationInfo::VARIABLE, &GDScriptParser::export_annotations<PROPERTY_HINT_NONE, Variant::NIL>);
Expand Down Expand Up @@ -128,13 +129,18 @@ GDScriptParser::GDScriptParser() {
register_annotation(MethodInfo("@export_group", PropertyInfo(Variant::STRING, "name"), PropertyInfo(Variant::STRING, "prefix")), AnnotationInfo::STANDALONE, &GDScriptParser::export_group_annotations<PROPERTY_USAGE_GROUP>, varray(""));
register_annotation(MethodInfo("@export_subgroup", PropertyInfo(Variant::STRING, "name"), PropertyInfo(Variant::STRING, "prefix")), AnnotationInfo::STANDALONE, &GDScriptParser::export_group_annotations<PROPERTY_USAGE_SUBGROUP>, varray(""));
// Warning annotations.
register_annotation(MethodInfo("@warning_ignore", PropertyInfo(Variant::STRING, "warning")), AnnotationInfo::CLASS_LEVEL | AnnotationInfo::STATEMENT, &GDScriptParser::warning_annotations, varray(), true);
register_annotation(MethodInfo("@warning_ignore", PropertyInfo(Variant::STRING, "warning")), AnnotationInfo::CLASS_LEVEL | AnnotationInfo::STATEMENT, &GDScriptParser::warning_ignore_annotation, varray(), true);
register_annotation(MethodInfo("@warning_ignore_start", PropertyInfo(Variant::STRING, "warning")), AnnotationInfo::STANDALONE, &GDScriptParser::warning_ignore_region_annotations, varray(), true);
register_annotation(MethodInfo("@warning_ignore_restore", PropertyInfo(Variant::STRING, "warning")), AnnotationInfo::STANDALONE, &GDScriptParser::warning_ignore_region_annotations, varray(), true);
// Networking.
register_annotation(MethodInfo("@rpc", PropertyInfo(Variant::STRING, "mode"), PropertyInfo(Variant::STRING, "sync"), PropertyInfo(Variant::STRING, "transfer_mode"), PropertyInfo(Variant::INT, "transfer_channel")), AnnotationInfo::FUNCTION, &GDScriptParser::rpc_annotation, varray("authority", "call_remote", "unreliable", 0));
}

#ifdef DEBUG_ENABLED
is_ignoring_warnings = !(bool)GLOBAL_GET("debug/gdscript/warnings/enable");
for (int i = 0; i < GDScriptWarning::WARNING_MAX; i++) {
warning_ignore_start_lines[i] = INT_MAX;
}
#endif

#ifdef TOOLS_ENABLED
Expand Down Expand Up @@ -214,6 +220,9 @@ void GDScriptParser::apply_pending_warnings() {
if (warning_ignored_lines[pw.code].has(pw.source->start_line)) {
continue;
}
if (warning_ignore_start_lines[pw.code] <= pw.source->start_line) {
continue;
}

GDScriptWarning warning;
warning.code = pw.code;
Expand Down Expand Up @@ -625,7 +634,7 @@ void GDScriptParser::parse_program() {
} else if (annotation->applies_to(AnnotationInfo::SCRIPT)) {
PUSH_PENDING_ANNOTATIONS_TO_HEAD;
if (annotation->name == SNAME("@tool") || annotation->name == SNAME("@icon")) {
// Some annotations need to be resolved in the parser.
// Some annotations need to be resolved and applied in the parser.
annotation->apply(this, head, nullptr); // `head->outer == nullptr`.
} else {
head->annotations.push_back(annotation);
Expand All @@ -640,8 +649,10 @@ void GDScriptParser::parse_program() {
// so we stop looking for script-level stuff.
can_have_class_or_extends = false;
break;
} else if (annotation->name == SNAME("@warning_ignore_start") || annotation->name == SNAME("@warning_ignore_restore")) {
// Some annotations need to be resolved and applied in the parser.
annotation->apply(this, nullptr, nullptr);
} else {
// For potential non-group standalone annotations.
push_error(R"(Unexpected standalone annotation.)");
}
} else {
Expand Down Expand Up @@ -1030,8 +1041,10 @@ void GDScriptParser::parse_class_body(bool p_is_multiline) {
}
if (annotation->name == SNAME("@export_category") || annotation->name == SNAME("@export_group") || annotation->name == SNAME("@export_subgroup")) {
current_class->add_member_group(annotation);
} else if (annotation->name == SNAME("@warning_ignore_start") || annotation->name == SNAME("@warning_ignore_restore")) {
// Some annotations need to be resolved and applied in the parser.
annotation->apply(this, nullptr, nullptr);
} else {
// For potential non-group standalone annotations.
push_error(R"(Unexpected standalone annotation.)");
}
} else { // `AnnotationInfo::CLASS_LEVEL`.
Expand Down Expand Up @@ -1896,9 +1909,21 @@ GDScriptParser::Node *GDScriptParser::parse_statement() {
break;
case GDScriptTokenizer::Token::ANNOTATION: {
advance();
AnnotationNode *annotation = parse_annotation(AnnotationInfo::STATEMENT);
AnnotationNode *annotation = parse_annotation(AnnotationInfo::STATEMENT | AnnotationInfo::STANDALONE);
if (annotation != nullptr) {
annotation_stack.push_back(annotation);
if (annotation->applies_to(AnnotationInfo::STANDALONE)) {
if (previous.type != GDScriptTokenizer::Token::NEWLINE) {
push_error(R"(Expected newline after a standalone annotation.)");
}
if (annotation->name == SNAME("@warning_ignore_start") || annotation->name == SNAME("@warning_ignore_restore")) {
// Some annotations need to be resolved and applied in the parser.
annotation->apply(this, nullptr, nullptr);
} else {
push_error(R"(Unexpected standalone annotation.)");
}
} else {
annotation_stack.push_back(annotation);
}
}
break;
}
Expand Down Expand Up @@ -4096,23 +4121,25 @@ bool GDScriptParser::validate_annotation_arguments(AnnotationNode *p_annotation)
return false;
}

// Some annotations need to be resolved in the parser.
if (p_annotation->name == SNAME("@icon")) {
ExpressionNode *argument = p_annotation->arguments[0];
// Some annotations need to be resolved and applied in the parser.
if (p_annotation->name == SNAME("@icon") || p_annotation->name == SNAME("@warning_ignore_start") || p_annotation->name == SNAME("@warning_ignore_restore")) {
for (int i = 0; i < p_annotation->arguments.size(); i++) {
ExpressionNode *argument = p_annotation->arguments[i];

if (argument->type != Node::LITERAL) {
push_error(R"(Argument 1 of annotation "@icon" must be a string literal.)", argument);
return false;
}
if (argument->type != Node::LITERAL) {
push_error(vformat(R"(Argument %d of annotation "%s" must be a string literal.)", i + 1, p_annotation->name), argument);
return false;
}

Variant value = static_cast<LiteralNode *>(argument)->value;
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;
}
if (value.get_type() != Variant::STRING) {
push_error(vformat(R"(Argument %d of annotation "%s" must be a string literal.)", i + 1, p_annotation->name), argument);
return false;
}

p_annotation->resolved_arguments.push_back(value);
p_annotation->resolved_arguments.push_back(value);
}
}

// For other annotations, see `GDScriptAnalyzer::resolve_annotation()`.
Expand Down Expand Up @@ -4162,6 +4189,17 @@ bool GDScriptParser::icon_annotation(AnnotationNode *p_annotation, Node *p_targe
return true;
}

bool GDScriptParser::static_unload_annotation(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 *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;
}
class_node->annotated_static_unload = true;
return true;
}

bool GDScriptParser::onready_annotation(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.)");

Expand Down Expand Up @@ -4756,11 +4794,8 @@ bool GDScriptParser::export_group_annotations(AnnotationNode *p_annotation, Node
return true;
}

bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
#ifndef DEBUG_ENABLED
// Only available in debug builds.
return true;
#else // DEBUG_ENABLED
bool GDScriptParser::warning_ignore_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
#ifdef DEBUG_ENABLED
if (is_ignoring_warnings) {
return true; // We already ignore all warnings, let's optimize it.
}
Expand Down Expand Up @@ -4805,8 +4840,14 @@ bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_t
} break;

case Node::FUNCTION: {
// `@warning_ignore` on function has a controversial feature that is used in tests.
// It's better not to remove it for now, while there is no way to mass-ignore warnings.
FunctionNode *function = static_cast<FunctionNode *>(p_target);
end_line = function->start_line;
for (int i = 0; i < function->parameters.size(); i++) {
end_line = MAX(end_line, function->parameters[i]->end_line);
if (function->parameters[i]->initializer != nullptr) {
end_line = MAX(end_line, function->parameters[i]->initializer->end_line);
}
}
} break;

case Node::MATCH_BRANCH: {
Expand All @@ -4828,6 +4869,48 @@ bool GDScriptParser::warning_annotations(AnnotationNode *p_annotation, Node *p_t
}
}
return !has_error;
#else // !DEBUG_ENABLED
// Only available in debug builds.
return true;
#endif // DEBUG_ENABLED
}

bool GDScriptParser::warning_ignore_region_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
#ifdef DEBUG_ENABLED
bool has_error = false;
const bool is_start = p_annotation->name == SNAME("@warning_ignore_start");
for (const Variant &warning_name : p_annotation->resolved_arguments) {
GDScriptWarning::Code warning_code = GDScriptWarning::get_code_from_name(String(warning_name).to_upper());
if (warning_code == GDScriptWarning::WARNING_MAX) {
push_error(vformat(R"(Invalid warning name: "%s".)", warning_name), p_annotation);
has_error = true;
continue;
}
if (is_start) {
if (warning_ignore_start_lines[warning_code] != INT_MAX) {
push_error(vformat(R"(Warning "%s" is already being ignored by "@warning_ignore_start" at line %d.)", String(warning_name).to_upper(), warning_ignore_start_lines[warning_code]), p_annotation);
has_error = true;
continue;
}
warning_ignore_start_lines[warning_code] = p_annotation->start_line;
} else {
if (warning_ignore_start_lines[warning_code] == INT_MAX) {
push_error(vformat(R"(Warning "%s" is not being ignored by "@warning_ignore_start".)", String(warning_name).to_upper()), p_annotation);
has_error = true;
continue;
}
const int start_line = warning_ignore_start_lines[warning_code];
const int end_line = MAX(start_line, p_annotation->start_line); // Prevent infinite loop.
for (int i = start_line; i <= end_line; i++) {
warning_ignored_lines[warning_code].insert(i);
}
warning_ignore_start_lines[warning_code] = INT_MAX;
}
}
return !has_error;
#else // !DEBUG_ENABLED
// Only available in debug builds.
return true;
#endif // DEBUG_ENABLED
}

Expand Down Expand Up @@ -4892,17 +4975,6 @@ bool GDScriptParser::rpc_annotation(AnnotationNode *p_annotation, Node *p_target
return true;
}

bool GDScriptParser::static_unload_annotation(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 *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;
}
class_node->annotated_static_unload = true;
return true;
}

GDScriptParser::DataType GDScriptParser::SuiteNode::Local::get_datatype() const {
switch (type) {
case CONSTANT:
Expand Down
6 changes: 4 additions & 2 deletions modules/gdscript/gdscript_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,7 @@ class GDScriptParser {
List<GDScriptWarning> warnings;
List<PendingWarning> pending_warnings;
HashSet<int> warning_ignored_lines[GDScriptWarning::WARNING_MAX];
int warning_ignore_start_lines[GDScriptWarning::WARNING_MAX];
HashSet<int> unsafe_lines;
#endif

Expand Down Expand Up @@ -1506,6 +1507,7 @@ class GDScriptParser {
void clear_unused_annotations();
bool tool_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool icon_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool static_unload_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool onready_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
template <PropertyHint t_hint, Variant::Type t_type>
bool export_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
Expand All @@ -1514,9 +1516,9 @@ class GDScriptParser {
bool export_tool_button_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
template <PropertyUsageFlags t_usage>
bool export_group_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool warning_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool warning_ignore_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool warning_ignore_region_annotations(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool rpc_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
bool static_unload_annotation(AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
// Statements.
Node *parse_statement();
VariableNode *parse_variable(bool p_is_static);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func param_inferred(param := variant()) -> void: print(param)
func return_untyped(): return variant()
func return_typed() -> Variant: return variant()

@warning_ignore("unused_variable", "inference_on_variant")
@warning_ignore_start("unused_variable", "inference_on_variant")
func test() -> void:
var weak = variant()
var typed: Variant = variant()
Expand All @@ -32,4 +32,4 @@ func test() -> void:
if typed != null: pass
if typed is Node: pass

print('ok')
print("ok")
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,4 @@ func test():
Utils.check((const_null is A) == false)
Utils.check(is_instance_of(const_null, A) == false)

print('ok')
print("ok")
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ class Members:
Utils.check(str(two) == '[486]')
return true


@warning_ignore("unsafe_method_access")
@warning_ignore("return_value_discarded")
@warning_ignore_start('unsafe_method_access', 'return_value_discarded')
func test():
var untyped_basic = [459]
Utils.check(str(untyped_basic) == '[459]')
Expand Down Expand Up @@ -207,7 +205,7 @@ func test():

var a := A.new()
var typed_natives: Array[RefCounted] = [a]
var typed_scripts = Array(typed_natives, TYPE_OBJECT, "RefCounted", A)
var typed_scripts = Array(typed_natives, TYPE_OBJECT, 'RefCounted', A)
Utils.check(typed_scripts[0] == a)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ class Members:
return true


@warning_ignore("unsafe_method_access")
@warning_ignore("assert_always_true")
@warning_ignore("return_value_discarded")
@warning_ignore_start("unsafe_method_access", "return_value_discarded")
func test():
var untyped_basic = { 459: 954 }
Utils.check(str(untyped_basic) == '{ 459: 954 }')
Expand Down
Loading