Browse Source

Clean a bunch of C# warnings

- `[Obsolete]` tag generated in the middle of documentation comments
- Potential `null` values in generators
- Obsolete call to `GetEditorInterface()`
- We don't want `Godot.Collections.Array` to end with `Collection`
- A few culture specifications and use of `AsSpan` instead of `SubString` in `StringExtensions`
- Disable CA1716 in GodotSharp
Paul Joannon 1 year ago
parent
commit
46b3096570

+ 7 - 0
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/CodeAnalysisAttributes.cs

@@ -0,0 +1,7 @@
+namespace System.Diagnostics.CodeAnalysis
+{
+    [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue)]
+    public sealed class NotNullAttribute : Attribute
+    {
+    }
+}

+ 15 - 0
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Helper.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+
+namespace Godot.SourceGenerators
+{
+    public static class Helper
+    {
+        [Conditional("DEBUG")]
+        public static void ThrowIfNull([NotNull] object? value)
+        {
+            _ = value ?? throw new ArgumentNullException();
+        }
+    }
+}

+ 4 - 4
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/MustBeVariantAnalyzer.cs

@@ -1,5 +1,4 @@
 using System.Collections.Immutable;
 using System.Collections.Immutable;
-using System.Diagnostics;
 using System.Linq;
 using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp;
@@ -34,7 +33,7 @@ namespace Godot.SourceGenerators
 
 
             // Method invocation or variable declaration that contained the type arguments
             // Method invocation or variable declaration that contained the type arguments
             var parentSyntax = context.Node.Parent;
             var parentSyntax = context.Node.Parent;
-            Debug.Assert(parentSyntax != null);
+            Helper.ThrowIfNull(parentSyntax);
 
 
             var sm = context.SemanticModel;
             var sm = context.SemanticModel;
 
 
@@ -49,9 +48,10 @@ namespace Godot.SourceGenerators
                     continue;
                     continue;
 
 
                 var typeSymbol = sm.GetSymbolInfo(typeSyntax).Symbol as ITypeSymbol;
                 var typeSymbol = sm.GetSymbolInfo(typeSyntax).Symbol as ITypeSymbol;
-                Debug.Assert(typeSymbol != null);
+                Helper.ThrowIfNull(typeSymbol);
 
 
                 var parentSymbol = sm.GetSymbolInfo(parentSyntax).Symbol;
                 var parentSymbol = sm.GetSymbolInfo(parentSyntax).Symbol;
+                Helper.ThrowIfNull(parentSymbol);
 
 
                 if (!ShouldCheckTypeArgument(context, parentSyntax, parentSymbol, typeSyntax, typeSymbol, i))
                 if (!ShouldCheckTypeArgument(context, parentSyntax, parentSymbol, typeSyntax, typeSymbol, i))
                 {
                 {
@@ -69,7 +69,7 @@ namespace Godot.SourceGenerators
 
 
                 var marshalType = MarshalUtils.ConvertManagedTypeToMarshalType(typeSymbol, typeCache);
                 var marshalType = MarshalUtils.ConvertManagedTypeToMarshalType(typeSymbol, typeCache);
 
 
-                if (marshalType == null)
+                if (marshalType is null)
                 {
                 {
                     Common.ReportGenericTypeArgumentMustBeVariant(context, typeSyntax, typeSymbol);
                     Common.ReportGenericTypeArgumentMustBeVariant(context, typeSyntax, typeSymbol);
                     continue;
                     continue;

+ 1 - 1
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptMethodsGenerator.cs

@@ -144,7 +144,7 @@ namespace Godot.SourceGenerators
                 .Append("    /// </summary>\n");
                 .Append("    /// </summary>\n");
 
 
             source.Append(
             source.Append(
-                $"    public new class MethodName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.MethodName {{\n");
+                $"    public new class MethodName : {symbol.BaseType!.FullQualifiedNameIncludeGlobal()}.MethodName {{\n");
 
 
             // Generate cached StringNames for methods and properties, for fast lookup
             // Generate cached StringNames for methods and properties, for fast lookup
 
 

+ 1 - 1
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertiesGenerator.cs

@@ -133,7 +133,7 @@ namespace Godot.SourceGenerators
                 .Append("    /// </summary>\n");
                 .Append("    /// </summary>\n");
 
 
             source.Append(
             source.Append(
-                $"    public new class PropertyName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.PropertyName {{\n");
+                $"    public new class PropertyName : {symbol.BaseType!.FullQualifiedNameIncludeGlobal()}.PropertyName {{\n");
 
 
             // Generate cached StringNames for methods and properties, for fast lookup
             // Generate cached StringNames for methods and properties, for fast lookup
 
 

+ 1 - 1
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs

@@ -185,7 +185,7 @@ namespace Godot.SourceGenerators
                 .Append("    /// </summary>\n");
                 .Append("    /// </summary>\n");
 
 
             source.Append(
             source.Append(
-                $"    public new class SignalName : {symbol.BaseType.FullQualifiedNameIncludeGlobal()}.SignalName {{\n");
+                $"    public new class SignalName : {symbol.BaseType!.FullQualifiedNameIncludeGlobal()}.SignalName {{\n");
 
 
             // Generate cached StringNames for methods and properties, for fast lookup
             // Generate cached StringNames for methods and properties, for fast lookup
 
 

+ 2 - 2
modules/mono/editor/GodotTools/GodotTools/Build/BuildProblemsView.cs

@@ -516,7 +516,7 @@ namespace GodotTools.Build
 
 
         public override void _Ready()
         public override void _Ready()
         {
         {
-            var editorSettings = GodotSharpEditor.Instance.GetEditorInterface().GetEditorSettings();
+            var editorSettings = EditorInterface.Singleton.GetEditorSettings();
             _layout = editorSettings.GetSetting(GodotSharpEditor.Settings.ProblemsLayout).As<ProblemsLayout>();
             _layout = editorSettings.GetSetting(GodotSharpEditor.Settings.ProblemsLayout).As<ProblemsLayout>();
 
 
             Name = "Problems".TTR();
             Name = "Problems".TTR();
@@ -655,7 +655,7 @@ namespace GodotTools.Build
             switch ((long)what)
             switch ((long)what)
             {
             {
                 case EditorSettings.NotificationEditorSettingsChanged:
                 case EditorSettings.NotificationEditorSettingsChanged:
-                    var editorSettings = GodotSharpEditor.Instance.GetEditorInterface().GetEditorSettings();
+                    var editorSettings = EditorInterface.Singleton.GetEditorSettings();
                     _layout = editorSettings.GetSetting(GodotSharpEditor.Settings.ProblemsLayout).As<ProblemsLayout>();
                     _layout = editorSettings.GetSetting(GodotSharpEditor.Settings.ProblemsLayout).As<ProblemsLayout>();
                     _toggleLayoutButton.ButtonPressed = GetToggleLayoutPressedState();
                     _toggleLayoutButton.ButtonPressed = GetToggleLayoutPressedState();
                     UpdateProblemsView();
                     UpdateProblemsView();

+ 2 - 4
modules/mono/editor/bindings_generator.cpp

@@ -2237,10 +2237,6 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
 
 
 				p_output.append(INDENT1 "/// </summary>");
 				p_output.append(INDENT1 "/// </summary>");
 			}
 			}
-
-			if (p_imethod.method_doc->is_deprecated) {
-				p_output.append(MEMBER_BEGIN "[Obsolete(\"This method is deprecated.\")]");
-			}
 		}
 		}
 
 
 		if (default_args_doc.get_string_length()) {
 		if (default_args_doc.get_string_length()) {
@@ -2255,6 +2251,8 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
 			p_output.append(MEMBER_BEGIN "[Obsolete(\"");
 			p_output.append(MEMBER_BEGIN "[Obsolete(\"");
 			p_output.append(p_imethod.deprecation_message);
 			p_output.append(p_imethod.deprecation_message);
 			p_output.append("\")]");
 			p_output.append("\")]");
+		} else if (p_imethod.method_doc && p_imethod.method_doc->is_deprecated) {
+			p_output.append(MEMBER_BEGIN "[Obsolete(\"This method is deprecated.\")]");
 		}
 		}
 
 
 		if (p_imethod.is_compat) {
 		if (p_imethod.is_compat) {

+ 3 - 0
modules/mono/glue/GodotSharp/.editorconfig

@@ -6,6 +6,9 @@ dotnet_diagnostic.CA1062.severity = error
 dotnet_diagnostic.CA1069.severity = none
 dotnet_diagnostic.CA1069.severity = none
 # CA1708: Identifiers should differ by more than case
 # CA1708: Identifiers should differ by more than case
 dotnet_diagnostic.CA1708.severity = none
 dotnet_diagnostic.CA1708.severity = none
+# CA1716: Identifiers should not match keywords
+# This is suppressed, because it will report `@event` as well as `event`
+dotnet_diagnostic.CA1716.severity = none
 # CS1591: Missing XML comment for publicly visible type or member
 # CS1591: Missing XML comment for publicly visible type or member
 dotnet_diagnostic.CS1591.severity = none
 dotnet_diagnostic.CS1591.severity = none
 # CS1573: Parameter has no matching param tag in the XML comment
 # CS1573: Parameter has no matching param tag in the XML comment

+ 2 - 0
modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs

@@ -16,7 +16,9 @@ namespace Godot.Collections
     /// interfacing with the engine. Otherwise prefer .NET collections
     /// interfacing with the engine. Otherwise prefer .NET collections
     /// such as <see cref="System.Array"/> or <see cref="List{T}"/>.
     /// such as <see cref="System.Array"/> or <see cref="List{T}"/>.
     /// </summary>
     /// </summary>
+#pragma warning disable CA1710 // Identifiers should have correct suffix
     public sealed class Array :
     public sealed class Array :
+#pragma warning restore CA1710
         IList<Variant>,
         IList<Variant>,
         IReadOnlyList<Variant>,
         IReadOnlyList<Variant>,
         ICollection,
         ICollection,

+ 12 - 13
modules/mono/glue/GodotSharp/GodotSharp/Core/StringExtensions.cs

@@ -1,7 +1,6 @@
 using System;
 using System;
 using System.Collections.Generic;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Globalization;
-using System.IO;
 using System.Security;
 using System.Security;
 using System.Security.Cryptography;
 using System.Security.Cryptography;
 using System.Text;
 using System.Text;
@@ -220,7 +219,7 @@ namespace Godot
                 {
                 {
                     if (hasText)
                     if (hasText)
                     {
                     {
-                        sb.Append(instance.Substring(indentStop, i - indentStop));
+                        sb.Append(instance.AsSpan(indentStop, i - indentStop));
                     }
                     }
                     sb.Append('\n');
                     sb.Append('\n');
                     hasText = false;
                     hasText = false;
@@ -252,7 +251,7 @@ namespace Godot
 
 
             if (hasText)
             if (hasText)
             {
             {
-                sb.Append(instance.Substring(indentStop, instance.Length - indentStop));
+                sb.Append(instance.AsSpan(indentStop, instance.Length - indentStop));
             }
             }
 
 
             return sb.ToString();
             return sb.ToString();
@@ -323,7 +322,7 @@ namespace Godot
                 string slice = aux.GetSliceCharacter(' ', i);
                 string slice = aux.GetSliceCharacter(' ', i);
                 if (slice.Length > 0)
                 if (slice.Length > 0)
                 {
                 {
-                    slice = char.ToUpper(slice[0]) + slice.Substring(1);
+                    slice = char.ToUpperInvariant(slice[0]) + slice.Substring(1);
                     if (i > 0)
                     if (i > 0)
                         cap += " ";
                         cap += " ";
                     cap += slice;
                     cap += slice;
@@ -408,13 +407,13 @@ namespace Godot
                 bool shouldSplit = condA || condB || condC || canBreakNumberLetter || canBreakLetterNumber;
                 bool shouldSplit = condA || condB || condC || canBreakNumberLetter || canBreakLetterNumber;
                 if (shouldSplit)
                 if (shouldSplit)
                 {
                 {
-                    newString += instance.Substring(startIndex, i - startIndex) + "_";
+                    newString += string.Concat(instance.AsSpan(startIndex, i - startIndex), "_");
                     startIndex = i;
                     startIndex = i;
                 }
                 }
             }
             }
 
 
             newString += instance.Substring(startIndex, instance.Length - startIndex);
             newString += instance.Substring(startIndex, instance.Length - startIndex);
-            return lowerCase ? newString.ToLower() : newString;
+            return lowerCase ? newString.ToLowerInvariant() : newString;
         }
         }
 
 
         /// <summary>
         /// <summary>
@@ -479,9 +478,9 @@ namespace Godot
                         return -1; // If this is empty, and the other one is not, then we're less... I think?
                         return -1; // If this is empty, and the other one is not, then we're less... I think?
                     if (to[toIndex] == 0)
                     if (to[toIndex] == 0)
                         return 1; // Otherwise the other one is smaller..
                         return 1; // Otherwise the other one is smaller..
-                    if (char.ToUpper(instance[instanceIndex]) < char.ToUpper(to[toIndex])) // More than
+                    if (char.ToUpperInvariant(instance[instanceIndex]) < char.ToUpperInvariant(to[toIndex])) // More than
                         return -1;
                         return -1;
-                    if (char.ToUpper(instance[instanceIndex]) > char.ToUpper(to[toIndex])) // Less than
+                    if (char.ToUpperInvariant(instance[instanceIndex]) > char.ToUpperInvariant(to[toIndex])) // Less than
                         return 1;
                         return 1;
 
 
                     instanceIndex++;
                     instanceIndex++;
@@ -853,7 +852,7 @@ namespace Godot
                     else
                     else
                     {
                     {
                         sb.Append(prefix);
                         sb.Append(prefix);
-                        sb.Append(instance.Substring(lineStart, i - lineStart + 1));
+                        sb.Append(instance.AsSpan(lineStart, i - lineStart + 1));
                     }
                     }
                     lineStart = i + 1;
                     lineStart = i + 1;
                 }
                 }
@@ -861,7 +860,7 @@ namespace Godot
             if (lineStart != instance.Length)
             if (lineStart != instance.Length)
             {
             {
                 sb.Append(prefix);
                 sb.Append(prefix);
-                sb.Append(instance.Substring(lineStart));
+                sb.Append(instance.AsSpan(lineStart));
             }
             }
             return sb.ToString();
             return sb.ToString();
         }
         }
@@ -925,8 +924,8 @@ namespace Godot
 
 
                 if (!caseSensitive)
                 if (!caseSensitive)
                 {
                 {
-                    char sourcec = char.ToLower(instance[source]);
-                    char targetc = char.ToLower(text[target]);
+                    char sourcec = char.ToLowerInvariant(instance[source]);
+                    char targetc = char.ToLowerInvariant(text[target]);
                     match = sourcec == targetc;
                     match = sourcec == targetc;
                 }
                 }
                 else
                 else
@@ -1234,7 +1233,7 @@ namespace Godot
                         return false;
                         return false;
                     if (caseSensitive)
                     if (caseSensitive)
                         return instance[0] == expr[0];
                         return instance[0] == expr[0];
-                    return (char.ToUpper(instance[0]) == char.ToUpper(expr[0])) &&
+                    return (char.ToUpperInvariant(instance[0]) == char.ToUpperInvariant(expr[0])) &&
                            ExprMatch(instance.Substring(1), expr.Substring(1), caseSensitive);
                            ExprMatch(instance.Substring(1), expr.Substring(1), caseSensitive);
             }
             }
         }
         }