New "must call base" analyzer. (#5266)

* New "must call base" analyzer.

This enforces that you actually call base when overriding stuff. This is intended for base methods like entity system's, where server/client systems overriding shared ones SHOULD call Initialize() and such.

* Add MustCallBase to entity system methods

Robust.Analyzers.Tests/MustCallBaseAnalyzerTest.cs

@@ -0,0 +1,92 @@
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CSharp.Testing;
+using Microsoft.CodeAnalysis.Testing;
+using Microsoft.CodeAnalysis.Testing.Verifiers;
+using NUnit.Framework;
+using VerifyCS =
+    Microsoft.CodeAnalysis.CSharp.Testing.NUnit.AnalyzerVerifier<Robust.Analyzers.MustCallBaseAnalyzer>;
+
+namespace Robust.Analyzers.Tests;
+
+[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)]
+[TestFixture]
+public sealed class MustCallBaseAnalyzerTest
+{
+    private static Task Verifier(string code, params DiagnosticResult[] expected)
+    {
+        var test = new CSharpAnalyzerTest<MustCallBaseAnalyzer, NUnitVerifier>()
+        {
+            TestState =
+            {
+                Sources = { code }
+            },
+        };
+
+        TestHelper.AddEmbeddedSources(
+            test.TestState,
+            "Robust.Shared.IoC.MustCallBaseAttribute.cs"
+        );
+
+        // ExpectedDiagnostics cannot be set, so we need to AddRange here...
+        test.TestState.ExpectedDiagnostics.AddRange(expected);
+
+        return test.RunAsync();
+    }
+
+    [Test]
+    public async Task Test()
+    {
+        const string code = """
+            using Robust.Shared.Analyzers;
+
+            public class Foo
+            {
+                [MustCallBase]
+                public virtual void Function()
+                {
+
+                }
+
+                [MustCallBase(true)]
+                public virtual void Function2()
+                {
+
+                }
+            }
+
+            public class Bar : Foo
+            {
+                public override void Function()
+                {
+
+                }
+
+                public override void Function2()
+                {
+
+                }
+            }
+
+            public class Baz : Foo
+            {
+                public override void Function()
+                {
+                    base.Function();
+                }
+            }
+
+            public class Bal : Bar
+            {
+                public override void Function2()
+                {
+                }
+            }
+            """;
+
+        await Verifier(code,
+            // /0/Test0.cs(20,26): warning RA0028: Overriders of this function must always call the base function
+            VerifyCS.Diagnostic().WithSpan(20, 26, 20, 34),
+            // /0/Test0.cs(41,26): warning RA0028: Overriders of this function must always call the base function
+            VerifyCS.Diagnostic().WithSpan(41, 26, 41, 35));
+    }
+}

Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj

@@ -10,6 +10,7 @@
   <ItemGroup>
     <EmbeddedResource Include="..\Robust.Shared\Analyzers\AccessAttribute.cs" LogicalName="Robust.Shared.Analyzers.AccessAttribute.cs" LinkBase="Implementations" />
     <EmbeddedResource Include="..\Robust.Shared\Analyzers\AccessPermissions.cs" LogicalName="Robust.Shared.Analyzers.AccessPermissions.cs" LinkBase="Implementations" />
+    <EmbeddedResource Include="..\Robust.Shared\Analyzers\MustCallBaseAttribute.cs" LogicalName="Robust.Shared.IoC.MustCallBaseAttribute.cs" LinkBase="Implementations" />
     <EmbeddedResource Include="..\Robust.Shared\IoC\DependencyAttribute.cs" LogicalName="Robust.Shared.IoC.DependencyAttribute.cs" LinkBase="Implementations" />
   </ItemGroup>
 

Robust.Analyzers/MustCallBaseAnalyzer.cs

@@ -0,0 +1,111 @@
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Robust.Roslyn.Shared;
+
+namespace Robust.Analyzers;
+
+#nullable enable
+
+/// <summary>
+/// Enforces <c>MustCallBaseAttribute</c>.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class MustCallBaseAnalyzer : DiagnosticAnalyzer
+{
+    private const string Attribute = "Robust.Shared.Analyzers.MustCallBaseAttribute";
+
+    private static readonly DiagnosticDescriptor Rule = new(
+        Diagnostics.IdMustCallBase,
+        "No base call in overriden function",
+        "Overriders of this function must always call the base function",
+        "Usage",
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+        context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.Method);
+    }
+
+    private static void AnalyzeSymbol(SymbolAnalysisContext context)
+    {
+        if (context.Symbol is not IMethodSymbol { IsOverride: true } method)
+            return;
+
+        var attrSymbol = context.Compilation.GetTypeByMetadataName(Attribute);
+        if (attrSymbol == null)
+            return;
+
+        if (DoesMethodOverriderHaveAttribute(method, attrSymbol) is not { } data)
+            return;
+
+        if (data is { onlyOverrides: true, depth: < 2 })
+            return;
+
+        var syntax = (MethodDeclarationSyntax) method.DeclaringSyntaxReferences[0].GetSyntax();
+        if (HasBaseCall(syntax))
+            return;
+
+        var diag = Diagnostic.Create(Rule, syntax.Identifier.GetLocation());
+        context.ReportDiagnostic(diag);
+    }
+
+    private static (int depth, bool onlyOverrides)? DoesMethodOverriderHaveAttribute(
+        IMethodSymbol method,
+        INamedTypeSymbol attributeSymbol)
+    {
+        var depth = 0;
+        while (method.OverriddenMethod != null)
+        {
+            depth += 1;
+            method = method.OverriddenMethod;
+            if (GetAttribute(method, attributeSymbol) is not { } attribute)
+                continue;
+
+            var onlyOverrides = attribute.ConstructorArguments is [{Kind: TypedConstantKind.Primitive, Value: true}];
+            return (depth, onlyOverrides);
+        }
+
+        return null;
+    }
+
+    private static bool HasBaseCall(MethodDeclarationSyntax syntax)
+    {
+        return syntax.Accept(new BaseCallLocator());
+    }
+
+    private static AttributeData? GetAttribute(ISymbol namedTypeSymbol, INamedTypeSymbol attrSymbol)
+    {
+        return namedTypeSymbol.GetAttributes()
+            .SingleOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, attrSymbol));
+    }
+
+    private sealed class BaseCallLocator : CSharpSyntaxVisitor<bool>
+    {
+        public override bool VisitBaseExpression(BaseExpressionSyntax node)
+        {
+            return true;
+        }
+
+        public override bool DefaultVisit(SyntaxNode node)
+        {
+            foreach (var childNode in node.ChildNodes())
+            {
+                if (childNode is not CSharpSyntaxNode cSharpSyntax)
+                    continue;
+
+                if (cSharpSyntax.Accept(this))
+                    return true;
+            }
+
+            return false;
+        }
+    }
+}

Robust.Roslyn.Shared/Diagnostics.cs

@@ -31,6 +31,7 @@ public static class Diagnostics
     public const string IdDependencyFieldAssigned = "RA0025";
     public const string IdUncachedRegex = "RA0026";
     public const string IdDataFieldRedundantTag = "RA0027";
+    public const string IdMustCallBase = "RA0028";
 
     public static SuppressionDescriptor MeansImplicitAssignment =>
         new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");

Robust.Shared/Analyzers/MustCallBaseAttribute.cs

@@ -0,0 +1,17 @@
+using System;
+
+namespace Robust.Shared.Analyzers;
+
+/// <summary>
+/// Indicates that overriders of this method must always call the base function.
+/// </summary>
+/// <param name="onlyOverrides">
+/// If true, only base calls to *overrides* are necessary.
+/// This is intended for base classes where the base function is always empty,
+/// so a base call from the first override may be ommitted.
+/// </param>
+[AttributeUsage(AttributeTargets.Method)]
+public sealed class MustCallBaseAttribute(bool onlyOverrides = false) : Attribute
+{
+    public bool OnlyOverrides { get; } = onlyOverrides;
+}

Robust.Shared/GameObjects/EntitySystem.cs

@@ -74,6 +74,7 @@ namespace Robust.Shared.GameObjects
         }
 
         /// <inheritdoc />
+        [MustCallBase(true)]
         public virtual void Initialize() { }
 
         /// <inheritdoc />
@@ -81,12 +82,15 @@ namespace Robust.Shared.GameObjects
         /// Not ran on the client if prediction is disabled and
         /// <see cref="UpdatesOutsidePrediction"/> is false (the default).
         /// </remarks>
+        [MustCallBase(true)]
         public virtual void Update(float frameTime) { }
 
         /// <inheritdoc />
+        [MustCallBase(true)]
         public virtual void FrameUpdate(float frameTime) { }
 
         /// <inheritdoc />
+        [MustCallBase(true)]
         public virtual void Shutdown()
         {
             ShutdownSubscriptions();