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

C#: Fix warnings caught by new problem-matchers #99902

Merged
merged 1 commit into from
Dec 10, 2024
+133 −68
Merged

C#: Fix warnings caught by new problem-matchers #99902

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
3 changes: 0 additions & 3 deletions .github/workflows/linux_builds.yml
View file
Original file line number Diff line number Diff line change
@@ -155,9 +155,6 @@ jobs:
- name: Build .NET solutions
if: matrix.build-mono
run: |
# FIXME: C# warnings should be properly handled eventually, but we don't want to clutter
# the GitHub Actions annotations, so remove the associated problem matcher for now.
echo "::remove-matcher owner=msvc::"
./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform=linuxbsd
- name: Prepare artifact
2 changes: 1 addition & 1 deletion modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs
View file
Original file line number Diff line number Diff line change
@@ -309,7 +309,7 @@ void AppendPartialContainingTypeDeclarations(INamedTypeSymbol? containingType)
// Enums must be converted to the underlying type before they can be implicitly converted to Variant
if (paramSymbol.Type.TypeKind == TypeKind.Enum)
{
var underlyingType = ((INamedTypeSymbol)paramSymbol.Type).EnumUnderlyingType;
var underlyingType = ((INamedTypeSymbol)paramSymbol.Type).EnumUnderlyingType!;
source.Append($", ({underlyingType.FullQualifiedNameIncludeGlobal()})@{paramSymbol.Name}");
continue;
}
180 changes: 121 additions & 59 deletions modules/mono/editor/bindings_generator.cpp
View file
Original file line number Diff line number Diff line change
@@ -141,6 +141,11 @@ const Vector<String> prop_allowed_inherited_member_hiding = {
"MenuBar.TextDirection",
"RichTextLabel.TextDirection",
"TextEdit.TextDirection",
"VisualShaderNodeReroute.PortType",
// The following instances are uniquely egregious violations, hiding `GetType()` from `object`.
// Included for the sake of CI, with the understanding that they *deserve* warnings.
"GltfAccessor.GetType",
"GltfAccessor.MethodName.GetType",
};

void BindingsGenerator::TypeInterface::postsetup_enum_type(BindingsGenerator::TypeInterface &r_enum_itype) {
@@ -583,19 +588,19 @@ String BindingsGenerator::bbcode_to_xml(const String &p_bbcode, const TypeInterf
}

if (link_tag == "method") {
_append_xml_method(xml_output, target_itype, target_cname, link_target, link_target_parts);
_append_xml_method(xml_output, target_itype, target_cname, link_target, link_target_parts, p_itype);
} else if (link_tag == "constructor") {
// TODO: Support constructors?
_append_xml_undeclared(xml_output, link_target);
} else if (link_tag == "operator") {
// TODO: Support operators?
_append_xml_undeclared(xml_output, link_target);
} else if (link_tag == "member") {
_append_xml_member(xml_output, target_itype, target_cname, link_target, link_target_parts);
_append_xml_member(xml_output, target_itype, target_cname, link_target, link_target_parts, p_itype);
} else if (link_tag == "signal") {
_append_xml_signal(xml_output, target_itype, target_cname, link_target, link_target_parts);
_append_xml_signal(xml_output, target_itype, target_cname, link_target, link_target_parts, p_itype);
} else if (link_tag == "enum") {
_append_xml_enum(xml_output, target_itype, target_cname, link_target, link_target_parts);
_append_xml_enum(xml_output, target_itype, target_cname, link_target, link_target_parts, p_itype);
} else if (link_tag == "constant") {
_append_xml_constant(xml_output, target_itype, target_cname, link_target, link_target_parts);
} else if (link_tag == "param") {
@@ -662,14 +667,8 @@ String BindingsGenerator::bbcode_to_xml(const String &p_bbcode, const TypeInterf
}

if (target_itype) {
if ((!p_itype || p_itype->api_type == ClassDB::API_CORE) && target_itype->api_type == ClassDB::API_EDITOR) {
// Editor references in core documentation cannot be resolved,
// handle as standard codeblock.
_log("Cannot reference editor type '%s' in documentation for core type '%s'\n",
target_itype->proxy_name.utf8().get_data(), p_itype ? p_itype->proxy_name.utf8().get_data() : "@GlobalScope");
xml_output.append("<c>");
xml_output.append(target_itype->proxy_name);
xml_output.append("</c>");
if (!_validate_api_type(target_itype, p_itype)) {
_append_xml_undeclared(xml_output, target_itype->proxy_name);
} else {
xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
xml_output.append(target_itype->proxy_name);
@@ -1086,7 +1085,7 @@ void BindingsGenerator::_append_text_undeclared(StringBuilder &p_output, const S
p_output.append("'" + p_link_target + "'");
}

void BindingsGenerator::_append_xml_method(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts) {
void BindingsGenerator::_append_xml_method(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype) {
if (p_link_target_parts[0] == name_cache.type_at_GlobalScope) {
if (OS::get_singleton()->is_stdout_verbose()) {
OS::get_singleton()->print("Cannot resolve @GlobalScope method reference in documentation: %s\n", p_link_target.utf8().get_data());
@@ -1117,35 +1116,38 @@ void BindingsGenerator::_append_xml_method(StringBuilder &p_xml_output, const Ty
const MethodInterface *target_imethod = p_target_itype->find_method_by_name(p_target_cname);

if (target_imethod) {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(p_target_itype->proxy_name);
p_xml_output.append(".");
p_xml_output.append(target_imethod->proxy_name);
p_xml_output.append("(");
bool first_key = true;
for (const ArgumentInterface &iarg : target_imethod->arguments) {
const TypeInterface *arg_type = _get_type_or_null(iarg.type);

if (first_key) {
first_key = false;
} else {
p_xml_output.append(", ");
}
if (!arg_type) {
ERR_PRINT("Cannot resolve argument type in documentation: '" + p_link_target + "'.");
p_xml_output.append(iarg.type.cname);
continue;
}
if (iarg.def_param_mode == ArgumentInterface::NULLABLE_VAL) {
p_xml_output.append("Nullable{");
}
String arg_cs_type = arg_type->cs_type + _get_generic_type_parameters(*arg_type, iarg.type.generic_type_parameters);
p_xml_output.append(arg_cs_type.replacen("<", "{").replacen(">", "}").replacen("params ", ""));
if (iarg.def_param_mode == ArgumentInterface::NULLABLE_VAL) {
p_xml_output.append("}");
const String method_name = p_target_itype->proxy_name + "." + target_imethod->proxy_name;
if (!_validate_api_type(p_target_itype, p_source_itype)) {
_append_xml_undeclared(p_xml_output, method_name);
} else {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(method_name);
p_xml_output.append("(");
bool first_key = true;
for (const ArgumentInterface &iarg : target_imethod->arguments) {
const TypeInterface *arg_type = _get_type_or_null(iarg.type);

if (first_key) {
first_key = false;
} else {
p_xml_output.append(", ");
}
if (!arg_type) {
ERR_PRINT("Cannot resolve argument type in documentation: '" + p_link_target + "'.");
p_xml_output.append(iarg.type.cname);
continue;
}
if (iarg.def_param_mode == ArgumentInterface::NULLABLE_VAL) {
p_xml_output.append("Nullable{");
}
String arg_cs_type = arg_type->cs_type + _get_generic_type_parameters(*arg_type, iarg.type.generic_type_parameters);
p_xml_output.append(arg_cs_type.replacen("<", "{").replacen(">", "}").replacen("params ", ""));
if (iarg.def_param_mode == ArgumentInterface::NULLABLE_VAL) {
p_xml_output.append("}");
}
}
p_xml_output.append(")\"/>");
}
p_xml_output.append(")\"/>");
} else {
if (!p_target_itype->is_intentionally_ignored(p_link_target)) {
ERR_PRINT("Cannot resolve method reference in documentation: '" + p_link_target + "'.");
@@ -1157,7 +1159,7 @@ void BindingsGenerator::_append_xml_method(StringBuilder &p_xml_output, const Ty
}
}

void BindingsGenerator::_append_xml_member(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts) {
void BindingsGenerator::_append_xml_member(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype) {
if (p_link_target.contains_char('/')) {
// Properties with '/' (slash) in the name are not declared in C#, so there is nothing to reference.
_append_xml_undeclared(p_xml_output, p_link_target);
@@ -1184,11 +1186,14 @@ void BindingsGenerator::_append_xml_member(StringBuilder &p_xml_output, const Ty
}

if (target_iprop) {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(current_itype->proxy_name);
p_xml_output.append(".");
p_xml_output.append(target_iprop->proxy_name);
p_xml_output.append("\"/>");
const String member_name = current_itype->proxy_name + "." + target_iprop->proxy_name;
if (!_validate_api_type(p_target_itype, p_source_itype)) {
_append_xml_undeclared(p_xml_output, member_name);
} else {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(member_name);
p_xml_output.append("\"/>");
}
} else {
if (!p_target_itype->is_intentionally_ignored(p_link_target)) {
ERR_PRINT("Cannot resolve member reference in documentation: '" + p_link_target + "'.");
@@ -1199,7 +1204,7 @@ void BindingsGenerator::_append_xml_member(StringBuilder &p_xml_output, const Ty
}
}

void BindingsGenerator::_append_xml_signal(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts) {
void BindingsGenerator::_append_xml_signal(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype) {
if (!p_target_itype || !p_target_itype->is_object_type) {
if (OS::get_singleton()->is_stdout_verbose()) {
if (p_target_itype) {
@@ -1215,11 +1220,14 @@ void BindingsGenerator::_append_xml_signal(StringBuilder &p_xml_output, const Ty
const SignalInterface *target_isignal = p_target_itype->find_signal_by_name(p_target_cname);

if (target_isignal) {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(p_target_itype->proxy_name);
p_xml_output.append(".");
p_xml_output.append(target_isignal->proxy_name);
p_xml_output.append("\"/>");
const String signal_name = p_target_itype->proxy_name + "." + target_isignal->proxy_name;
if (!_validate_api_type(p_target_itype, p_source_itype)) {
_append_xml_undeclared(p_xml_output, signal_name);
} else {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(signal_name);
p_xml_output.append("\"/>");
}
} else {
if (!p_target_itype->is_intentionally_ignored(p_link_target)) {
ERR_PRINT("Cannot resolve signal reference in documentation: '" + p_link_target + "'.");
@@ -1230,7 +1238,7 @@ void BindingsGenerator::_append_xml_signal(StringBuilder &p_xml_output, const Ty
}
}

void BindingsGenerator::_append_xml_enum(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts) {
void BindingsGenerator::_append_xml_enum(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype) {
const StringName search_cname = !p_target_itype ? p_target_cname : StringName(p_target_itype->name + "." + (String)p_target_cname);

HashMap<StringName, TypeInterface>::ConstIterator enum_match = enum_types.find(search_cname);
@@ -1242,9 +1250,13 @@ void BindingsGenerator::_append_xml_enum(StringBuilder &p_xml_output, const Type
if (enum_match) {
const TypeInterface &target_enum_itype = enum_match->value;

p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(target_enum_itype.proxy_name); // Includes nesting class if any
p_xml_output.append("\"/>");
if (!_validate_api_type(p_target_itype, p_source_itype)) {
_append_xml_undeclared(p_xml_output, target_enum_itype.proxy_name);
} else {
p_xml_output.append("<see cref=\"" BINDINGS_NAMESPACE ".");
p_xml_output.append(target_enum_itype.proxy_name); // Includes nesting class if any
p_xml_output.append("\"/>");
}
} else {
if (!p_target_itype->is_intentionally_ignored(p_link_target)) {
ERR_PRINT("Cannot resolve enum reference in documentation: '" + p_link_target + "'.");
@@ -1379,6 +1391,46 @@ void BindingsGenerator::_append_xml_undeclared(StringBuilder &p_xml_output, cons
p_xml_output.append("</c>");
}

bool BindingsGenerator::_validate_api_type(const TypeInterface *p_target_itype, const TypeInterface *p_source_itype) {
static constexpr const char *api_types[5] = {
"Core",
"Editor",
"Extension",
"Editor Extension",
"None",
};

const ClassDB::APIType target_api = p_target_itype ? p_target_itype->api_type : ClassDB::API_NONE;
ERR_FAIL_INDEX_V((int)target_api, 5, false);
const ClassDB::APIType source_api = p_source_itype ? p_source_itype->api_type : ClassDB::API_NONE;
ERR_FAIL_INDEX_V((int)source_api, 5, false);
bool validate = false;

switch (target_api) {
case ClassDB::API_NONE:
case ClassDB::API_CORE:
default:
validate = true;
break;
case ClassDB::API_EDITOR:
validate = source_api == ClassDB::API_EDITOR || source_api == ClassDB::API_EDITOR_EXTENSION;
break;
case ClassDB::API_EXTENSION:
validate = source_api == ClassDB::API_EXTENSION || source_api == ClassDB::API_EDITOR_EXTENSION;
break;
case ClassDB::API_EDITOR_EXTENSION:
validate = source_api == ClassDB::API_EDITOR_EXTENSION;
break;
}
if (!validate) {
const String target_name = p_target_itype ? p_target_itype->proxy_name : "@GlobalScope";
const String source_name = p_source_itype ? p_source_itype->proxy_name : "@GlobalScope";
WARN_PRINT(vformat("Type '%s' has API level '%s'; it cannot be referenced by type '%s' with API level '%s'.",
target_name, api_types[target_api], source_name, api_types[source_api]));
}
return validate;
}

int BindingsGenerator::_determine_enum_prefix(const EnumInterface &p_ienum) {
CRASH_COND(p_ienum.constants.is_empty());

@@ -2585,7 +2637,9 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
output << INDENT2 "/// <summary>\n"
<< INDENT2 "/// Cached name for the '" << iprop.cname << "' property.\n"
<< INDENT2 "/// </summary>\n"
<< INDENT2 "public static readonly StringName " << iprop.proxy_name << " = \"" << iprop.cname << "\";\n";
<< INDENT2 "public static "
<< (prop_allowed_inherited_member_hiding.has(itype.proxy_name + ".PropertyName." + iprop.proxy_name) ? "new " : "")
<< "readonly StringName " << iprop.proxy_name << " = \"" << iprop.cname << "\";\n";
}
output << INDENT1 "}\n";
//MethodName
@@ -2609,7 +2663,9 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
output << INDENT2 "/// <summary>\n"
<< INDENT2 "/// Cached name for the '" << imethod.cname << "' method.\n"
<< INDENT2 "/// </summary>\n"
<< INDENT2 "public static readonly StringName " << imethod.proxy_name << " = \"" << imethod.cname << "\";\n";
<< INDENT2 "public static "
<< (prop_allowed_inherited_member_hiding.has(itype.proxy_name + ".MethodName." + imethod.proxy_name) ? "new " : "")
<< "readonly StringName " << imethod.proxy_name << " = \"" << imethod.cname << "\";\n";
}
output << INDENT1 "}\n";
//SignalName
@@ -2627,7 +2683,9 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
output << INDENT2 "/// <summary>\n"
<< INDENT2 "/// Cached name for the '" << isignal.cname << "' signal.\n"
<< INDENT2 "/// </summary>\n"
<< INDENT2 "public static readonly StringName " << isignal.proxy_name << " = \"" << isignal.cname << "\";\n";
<< INDENT2 "public static "
<< (prop_allowed_inherited_member_hiding.has(itype.proxy_name + ".SignalName." + isignal.proxy_name) ? "new " : "")
<< "readonly StringName " << isignal.proxy_name << " = \"" << isignal.cname << "\";\n";
}
output << INDENT1 "}\n";

@@ -3030,6 +3088,10 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
p_output.append(MEMBER_BEGIN);
p_output.append(p_imethod.is_internal ? "internal " : "public ");

if (prop_allowed_inherited_member_hiding.has(p_itype.proxy_name + "." + p_imethod.proxy_name)) {
p_output.append("new ");
}

if (p_itype.is_singleton || p_imethod.is_static) {
p_output.append("static ");
} else if (p_imethod.is_virtual) {
10 changes: 6 additions & 4 deletions modules/mono/editor/bindings_generator.h
View file
Original file line number Diff line number Diff line change
@@ -804,15 +804,17 @@ class BindingsGenerator {
void _append_text_param(StringBuilder &p_output, const String &p_link_target);
void _append_text_undeclared(StringBuilder &p_output, const String &p_link_target);

void _append_xml_method(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts);
void _append_xml_member(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts);
void _append_xml_signal(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts);
void _append_xml_enum(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts);
void _append_xml_method(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype);
void _append_xml_member(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype);
void _append_xml_signal(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype);
void _append_xml_enum(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts, const TypeInterface *p_source_itype);
void _append_xml_constant(StringBuilder &p_xml_output, const TypeInterface *p_target_itype, const StringName &p_target_cname, const String &p_link_target, const Vector<String> &p_link_target_parts);
void _append_xml_constant_in_global_scope(StringBuilder &p_xml_output, const String &p_target_cname, const String &p_link_target);
void _append_xml_param(StringBuilder &p_xml_output, const String &p_link_target, bool p_is_signal);
void _append_xml_undeclared(StringBuilder &p_xml_output, const String &p_link_target);

bool _validate_api_type(const TypeInterface *p_target_itype, const TypeInterface *p_source_itype);

int _determine_enum_prefix(const EnumInterface &p_ienum);
void _apply_prefix_to_enum_constants(EnumInterface &p_ienum, int p_prefix_length);

2 changes: 2 additions & 0 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs
View file
Original file line number Diff line number Diff line change
@@ -1046,6 +1046,8 @@ internal interface IGenericGodotArray
[DebuggerTypeProxy(typeof(ArrayDebugView<>))]
[DebuggerDisplay("Count = {Count}")]
[SuppressMessage("ReSharper", "RedundantExtendsListEntry")]
[SuppressMessage("Design", "CA1001", MessageId = "Types that own disposable fields should be disposable",
Justification = "Known issue. Requires explicit refcount management to not dispose untyped collections.")]
[SuppressMessage("Naming", "CA1710", MessageId = "Identifiers should have correct suffix")]
public sealed class Array<[MustBeVariant] T> :
IList<T>,
2 changes: 2 additions & 0 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs
View file
Original file line number Diff line number Diff line change
@@ -485,6 +485,8 @@ internal interface IGenericGodotDictionary
/// <typeparam name="TValue">The type of the dictionary's values.</typeparam>
[DebuggerTypeProxy(typeof(DictionaryDebugView<,>))]
[DebuggerDisplay("Count = {Count}")]
[SuppressMessage("Design", "CA1001", MessageId = "Types that own disposable fields should be disposable",
Justification = "Known issue. Requires explicit refcount management to not dispose untyped collections.")]
public class Dictionary<[MustBeVariant] TKey, [MustBeVariant] TValue> :
IDictionary<TKey, TValue>,
IReadOnlyDictionary<TKey, TValue>,
2 changes: 1 addition & 1 deletion modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
View file
Original file line number Diff line number Diff line change
@@ -109,7 +109,7 @@ static void AppendArray(StringBuilder sb, Type type)
// Append brackets.
AppendArrayBrackets(sb, type);

static void AppendArrayBrackets(StringBuilder sb, Type type)
static void AppendArrayBrackets(StringBuilder sb, Type? type)
{
while (type != null && type.IsArray)
{