Analyzer & Fixer for redundant Prototype type strings (#5718)

* Add Prototype analyzer

* Add Prototype fixer

* Early return after finding prototype attribute

* Add PrototypeEndsWithPrototypeRule diagnostic

* Oops. Uncomment parallelizable.

* Rework to ignore redundancy for non-literal string values

* Allow redundancy when removal would expose class name not ending in "Prototype"

* Promote PrototypeEndsWithPrototypeRule from warning to error, since it causes a runtime error.

* No need to get the symbol to get the class identifier

* Minor cleanup

* A little more cleanup

* More specific location for redundant name

* Refactor redundant name fixer so argument order is no longer important

* Add failing test

* Use symbol analysis to fix alias handling

* Oops! We have to go back to the previous syntax-based approach.

Now it's a hybrid.

Also fixed tests to not copy the prototype definitions.

---------

Co-authored-by: PJB3005 <pieterjan.briers+git@gmail.com>

Robust.Analyzers.Tests/PrototypeAnalyzerTest.cs

@@ -0,0 +1,177 @@
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Testing;
+using NUnit.Framework;
+using VerifyCS =
+    Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<Robust.Analyzers.PrototypeAnalyzer, Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
+
+namespace Robust.Analyzers.Tests;
+
+[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)]
+[TestFixture]
+[TestOf(typeof(PrototypeAnalyzer))]
+public sealed class PrototypeAnalyzerTest
+{
+    private static Task Verifier(string code, params DiagnosticResult[] expected)
+    {
+        var test = new RTAnalyzerTest<PrototypeAnalyzer>()
+        {
+            TestState =
+            {
+                Sources = { code }
+            },
+        };
+
+        TestHelper.AddEmbeddedSources(
+            test.TestState,
+            "Robust.Shared.Prototypes.Attributes.cs",
+            "Robust.Shared.Prototypes.IPrototype.cs",
+            "Robust.Shared.Serialization.Manager.Attributes.DataFieldAttribute.cs"
+        );
+
+        // ExpectedDiagnostics cannot be set, so we need to AddRange here...
+        test.TestState.ExpectedDiagnostics.AddRange(expected);
+
+        return test.RunAsync();
+    }
+
+    [Test]
+    public async Task RedundantTypeTest()
+    {
+        const string code = """
+            using Robust.Shared.Prototypes;
+
+            [Prototype]
+            public sealed partial class GoodAutoPrototype : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype("someOtherName")]
+            public sealed partial class GoodUnmatchedPrototype : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype("badMatched")]
+            public sealed partial class BadMatchedPrototype : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype(ProtoName)]
+            public sealed partial class GoodNonLiteralMatchedPrototype : IPrototype
+            {
+                public const string ProtoName = "goodNonLiteralMatched";
+
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype(ProtoName)]
+            public sealed partial class GoodNonLiteralUnmatchedPrototype : IPrototype
+            {
+                public const string ProtoName = "someOtherNameEntirely";
+
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype("goodDoesNotEndWithPrototypeWord")]
+            public sealed partial class GoodDoesNotEndWithPrototypeWord : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+            """;
+
+        await Verifier(code,
+            // /0/Test0.cs(9,2): warning RA0033: Prototype BadMatchedPrototype has explicitly set type "badMatched" that matches autogenerated value
+            VerifyCS.Diagnostic(PrototypeAnalyzer.PrototypeRedundantTypeRule).WithSpan(17, 12, 17, 24).WithArguments("BadMatchedPrototype", "badMatched")
+        );
+    }
+
+    [Test]
+    public async Task AliasTest()
+    {
+        const string code = """
+            using Robust.Shared.Prototypes;
+            using PPrototypeAttribute = Robust.Shared.Prototypes.PrototypeAttribute;
+
+            [PPrototype("badMatched")]
+            public sealed partial class BadMatchedPrototype : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+            """;
+
+        await Verifier(code,
+            // /0/Test0.cs(4,2): warning RA0042: Prototype BadMatchedPrototype has explicitly set type "badMatched" that matches autogenerated value
+            VerifyCS.Diagnostic(PrototypeAnalyzer.PrototypeRedundantTypeRule).WithSpan(4, 13, 4, 25).WithArguments("BadMatchedPrototype", "badMatched")
+        );
+    }
+
+
+    [Test]
+    public async Task MoreAttributesTest()
+    {
+        const string code = """
+            using System;
+            using Robust.Shared.Prototypes;
+            using PPrototypeAttribute = Robust.Shared.Prototypes.PrototypeAttribute;
+
+            [FooBarAttribute]
+            [PPrototype("badMatched")]
+            public sealed partial class BadMatchedPrototype : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [AttributeUsage(AttributeTargets.Class, Inherited = false)]
+            public sealed class FooBarAttribute : Attribute;
+            """;
+
+        await Verifier(code,
+            // /0/Test0.cs(4,2): warning RA0042: Prototype BadMatchedPrototype has explicitly set type "badMatched" that matches autogenerated value
+            VerifyCS.Diagnostic(PrototypeAnalyzer.PrototypeRedundantTypeRule).WithSpan(6, 13, 6, 25).WithArguments("BadMatchedPrototype", "badMatched")
+        );
+    }
+
+    [Test]
+    public async Task NameEndsWithPrototypeTest()
+    {
+        const string code = """
+            using Robust.Shared.Prototypes;
+
+            [Prototype]
+            public sealed partial class GoodAutoPrototype : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype("ThisIsFine")]
+            public sealed partial class GoodManual : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+
+            [Prototype]
+            public sealed partial class BadAuto : IPrototype
+            {
+                [IdDataField]
+                public string ID { get; private set; } = default!;
+            }
+            """;
+
+        await Verifier(code,
+            // /0/Test0.cs(18,29): error RA0043: Prototype BadAuto does not end with the word Prototype
+            VerifyCS.Diagnostic(PrototypeAnalyzer.PrototypeEndsWithPrototypeRule).WithSpan(18, 29, 18, 36).WithArguments("BadAuto")
+        );
+    }
+}

Robust.Analyzers.Tests/PrototypeFixerTest.cs

@@ -0,0 +1,95 @@
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CSharp.Testing;
+using Microsoft.CodeAnalysis.Testing;
+using NUnit.Framework;
+using VerifyCS =
+    Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<Robust.Analyzers.PrototypeAnalyzer, Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
+
+
+namespace Robust.Analyzers.Tests;
+
+public sealed class PrototypeFixerTest
+{
+    private static Task Verifier(string code, string fixedCode, params DiagnosticResult[] expected)
+    {
+        var test = new CSharpCodeFixTest<PrototypeAnalyzer, PrototypeFixer, DefaultVerifier>()
+        {
+            TestState =
+            {
+                Sources = { code },
+            },
+            FixedState =
+            {
+                Sources = { fixedCode },
+            }
+        };
+
+        test.TestState.Sources.Add(("PrototypeAttribute.cs", PrototypeAttributeDef));
+        test.FixedState.Sources.Add(("PrototypeAttribute.cs", PrototypeAttributeDef));
+
+        test.TestState.ExpectedDiagnostics.AddRange(expected);
+
+        return test.RunAsync();
+    }
+
+    private const string PrototypeAttributeDef = """
+        using System;
+
+        namespace Robust.Shared.Prototypes
+        {
+            public class PrototypeAttribute : Attribute
+            {
+                public string? Type { get; internal set; }
+                public readonly int LoadPriority = 1;
+
+                public PrototypeAttribute(string? type = null, int loadPriority = 1)
+                {
+                    Type = type;
+                    LoadPriority = loadPriority;
+                }
+
+                public PrototypeAttribute(int loadPriority)
+                {
+                    Type = null;
+                    LoadPriority = loadPriority;
+                }
+            }
+            public interface IPrototype;
+        }
+    """;
+
+    [Test]
+    public async Task Test()
+    {
+        const string code = """
+            using Robust.Shared.Prototypes;
+
+            [Prototype]
+            public sealed partial class GoodAutoPrototype : IPrototype;
+
+            [Prototype("someOtherName")]
+            public sealed partial class GoodUnmatchedPrototype : IPrototype;
+
+            [Prototype("badMatched")]
+            public sealed partial class BadMatchedPrototype : IPrototype;
+            """;
+
+        const string fixedCode = """
+            using Robust.Shared.Prototypes;
+
+            [Prototype]
+            public sealed partial class GoodAutoPrototype : IPrototype;
+
+            [Prototype("someOtherName")]
+            public sealed partial class GoodUnmatchedPrototype : IPrototype;
+
+            [Prototype]
+            public sealed partial class BadMatchedPrototype : IPrototype;
+            """;
+
+        await Verifier(code, fixedCode,
+            // /0/Test0.cs(9,2): warning RA0033: Prototype BadMatchedPrototype has explicitly set type "badMatched" that matches autogenerated value
+            VerifyCS.Diagnostic(PrototypeAnalyzer.PrototypeRedundantTypeRule).WithSpan(9, 12, 9, 24).WithArguments("BadMatchedPrototype", "badMatched")
+        );
+    }
+}

Robust.Analyzers/PrototypeAnalyzer.cs

@@ -0,0 +1,139 @@
+#nullable enable
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Robust.Roslyn.Shared;
+using Robust.Shared.Prototypes;
+
+namespace Robust.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class PrototypeAnalyzer : DiagnosticAnalyzer
+{
+    public static readonly DiagnosticDescriptor PrototypeRedundantTypeRule = new(
+        Diagnostics.IdPrototypeRedundantType,
+        "Redundant Prototype Type specification",
+        "Prototype {0} has explicitly set type \"{1}\" that matches autogenerated value",
+        "Usage",
+        DiagnosticSeverity.Warning,
+        true,
+        "Remove the redundant type specification."
+    );
+
+    public static readonly DiagnosticDescriptor PrototypeEndsWithPrototypeRule = new(
+        Diagnostics.IdPrototypeEndsWithPrototype,
+        "Prototype name must end with the word Prototype",
+        "Prototype {0} does not end with the word Prototype",
+        "Usage",
+        DiagnosticSeverity.Error,
+        true,
+        "Add the word Prototype to the end of the class name or manually specify a name in the Prototype attribute."
+    );
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+        [PrototypeRedundantTypeRule, PrototypeEndsWithPrototypeRule];
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze |
+                                               GeneratedCodeAnalysisFlags.ReportDiagnostics);
+        context.EnableConcurrentExecution();
+
+        context.RegisterCompilationStartAction(static ctx =>
+        {
+            var prototypeAttribute =
+                ctx.Compilation.GetTypeByMetadataName("Robust.Shared.Prototypes.PrototypeAttribute");
+
+            // No attribute, no analyzer.
+            if (prototypeAttribute is null)
+                return;
+
+            ctx.RegisterSyntaxNodeAction(
+                symCtx => AnalyzePrototype(symCtx, prototypeAttribute),
+                SyntaxKind.ClassDeclaration);
+        });
+    }
+
+    private static void AnalyzePrototype(SyntaxNodeAnalysisContext context, INamedTypeSymbol prototypeAttributeSymbol)
+    {
+        if (context.Node is not ClassDeclarationSyntax classDeclarationSyntax)
+            return;
+
+        var classSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclarationSyntax);
+        if (classSymbol is null)
+            return;
+
+        var className = classSymbol.Name;
+
+        if (!AttributeHelper.HasAttribute(classSymbol, prototypeAttributeSymbol, out var attributeData))
+            return;
+
+        var prototypeAttribute = GetAttributeSyntax(attributeData, classDeclarationSyntax);
+        if (prototypeAttribute == null)
+            return;
+
+        // Check for autogenerated type
+        if (prototypeAttribute.ArgumentList?.Arguments[0] is not { } argumentSyntax)
+        {
+            if (!className.EndsWith(PrototypeUtility.PrototypeNameEnding))
+            {
+                context.ReportDiagnostic(Diagnostic.Create(PrototypeEndsWithPrototypeRule,
+                    classDeclarationSyntax.Identifier.GetLocation(),
+                    className));
+            }
+
+            return;
+        }
+
+        // We only care about redundancy if the argument is a string literal.
+        // Passing in a value that resolves to a redundant string is fine.
+        if (argumentSyntax.Expression is not LiteralExpressionSyntax literalSyntax)
+            return;
+
+        var literalValue = context.SemanticModel.GetConstantValue(literalSyntax);
+        if (literalValue.Value is not string specifiedName)
+            return;
+
+        var autoName = PrototypeUtility.CalculatePrototypeName(className);
+
+        // Check for name redundancy
+        if (autoName == specifiedName)
+        {
+            // If the class name does not end with "Prototype", allow the redundancy
+            if (!className.EndsWith(PrototypeUtility.PrototypeNameEnding))
+                return;
+
+            var location = argumentSyntax.GetLocation();
+            context.ReportDiagnostic(Diagnostic.Create(PrototypeRedundantTypeRule,
+                location,
+                className,
+                specifiedName));
+        }
+    }
+
+    private static AttributeSyntax? GetAttributeSyntax(
+        AttributeData attributeData,
+        ClassDeclarationSyntax classSyntax)
+    {
+        if (attributeData.ApplicationSyntaxReference is not { } syntaxReference)
+            return null;
+
+        foreach (var attributeList in classSyntax.AttributeLists)
+        {
+            foreach (var attribute in attributeList.Attributes)
+            {
+                if (syntaxReference.SyntaxTree != attribute.SyntaxTree)
+                    continue;
+
+                if (!syntaxReference.Span.OverlapsWith(attribute.Span))
+                    continue;
+
+                return attribute;
+            }
+        }
+
+        return null;
+    }
+}

Robust.Analyzers/PrototypeFixer.cs

@@ -0,0 +1,77 @@
+#nullable enable
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using static Robust.Roslyn.Shared.Diagnostics;
+
+namespace Robust.Analyzers;
+
+[ExportCodeFixProvider(LanguageNames.CSharp)]
+public sealed class PrototypeFixer : CodeFixProvider
+{
+    public override ImmutableArray<string> FixableDiagnosticIds => [IdPrototypeRedundantType];
+
+    public override FixAllProvider GetFixAllProvider()
+    {
+        return WellKnownFixAllProviders.BatchFixer;
+    }
+
+    public override Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        foreach (var diagnostic in context.Diagnostics)
+        {
+            switch (diagnostic.Id)
+            {
+                case IdPrototypeRedundantType:
+                    return RegisterRemoveType(context, diagnostic);
+            }
+        }
+
+        return Task.CompletedTask;
+    }
+
+    private static async Task RegisterRemoveType(CodeFixContext context, Diagnostic diagnostic)
+    {
+        var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken);
+        var span = diagnostic.Location.SourceSpan;
+        var token = root?.FindToken(span.Start).Parent?.AncestorsAndSelf().OfType<AttributeArgumentSyntax>().First();
+
+        if (token == null)
+            return;
+
+        context.RegisterCodeFix(CodeAction.Create(
+            "Remove explicitly set type",
+            c => RemoveType(context.Document, token, c),
+            "Remove explicitly set type"
+        ), diagnostic);
+    }
+
+    private static async Task<Document> RemoveType(Document document, AttributeArgumentSyntax syntax, CancellationToken cancellation)
+    {
+        var root = (CompilationUnitSyntax?) await document.GetSyntaxRootAsync(cancellation);
+
+        if (syntax.Parent is not AttributeArgumentListSyntax argListSyntax)
+            return document;
+
+        if (argListSyntax.Arguments.Count == 1)
+        {
+            // If this is the only argument, remove the whole argument list so we don't leave empty parentheses
+            if (argListSyntax.Parent is not AttributeSyntax attributeSyntax)
+                return document;
+
+            var newAttributeSyntax = attributeSyntax.RemoveNode(argListSyntax, SyntaxRemoveOptions.KeepNoTrivia);
+            root = root!.ReplaceNode(attributeSyntax, newAttributeSyntax!);
+        }
+        else
+        {
+            // Otherwise just remove the argument
+            var newArgListSyntax = argListSyntax.WithArguments(argListSyntax.Arguments.Remove(syntax));
+            root = root!.ReplaceNode(argListSyntax, newArgListSyntax);
+        }
+
+
+        return document.WithSyntaxRoot(root);
+    }
+}

Robust.Analyzers/Robust.Analyzers.csproj

@@ -33,6 +33,11 @@
     <Compile Include="..\Robust.Shared\Serialization\NetSerializableAttribute.cs" LinkBase="Implementations" />
   </ItemGroup>
 
+  <ItemGroup>
+    <!-- Needed for PrototypeAnalyzer. -->
+    <Compile Include="..\Robust.Shared\Prototypes\PrototypeUtility.cs" LinkBase="Implementations" />
+  </ItemGroup>
+
   <Import Project="../Robust.Roslyn.Shared/Robust.Roslyn.Shared.props" />
 
   <PropertyGroup>

Robust.Roslyn.Shared/Diagnostics.cs

@@ -45,6 +45,8 @@ public static class Diagnostics
     public const string IdPrototypeInstantiation = "RA0039";
     public const string IdAutoGenStateAttributeMissing = "RA0040";
     public const string IdAutoGenStateParamMissing = "RA0041";
+    public const string IdPrototypeRedundantType = "RA0042";
+    public const string IdPrototypeEndsWithPrototype = "RA0043";
 
     public static SuppressionDescriptor MeansImplicitAssignment =>
         new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");

Robust.Shared/Prototypes/PrototypeManager.cs

@@ -1005,12 +1005,7 @@ namespace Robust.Shared.Prototypes
 
         static string CalculatePrototypeName(Type type)
         {
-            const string prototype = "Prototype";
-            if (!type.Name.EndsWith(prototype))
-                throw new InvalidPrototypeNameException($"Prototype {type} must end with the word Prototype");
-
-            var name = type.Name.AsSpan();
-            return $"{char.ToLowerInvariant(name[0])}{name[1..^prototype.Length]}";
+            return PrototypeUtility.CalculatePrototypeName(type.Name);
         }
 
         /// <inheritdoc />
@@ -1274,11 +1269,4 @@ namespace Robust.Shared.Prototypes
             throw new ArgumentOutOfRangeException($"Unable to pick valid prototype for {typeof(T)}?");
         }
     }
-
-    public sealed class InvalidPrototypeNameException : Exception
-    {
-        public InvalidPrototypeNameException(string message) : base(message)
-        {
-        }
-    }
 }

Robust.Shared/Prototypes/PrototypeUtility.cs

@@ -0,0 +1,23 @@
+using System;
+
+namespace Robust.Shared.Prototypes;
+
+public static class PrototypeUtility
+{
+    /// <summary>
+    /// Prototypes using autogenerated names are required to end in this string.
+    /// </summary>
+    public const string PrototypeNameEnding = "Prototype";
+
+    /// <summary>
+    /// Given the type name of a Prototype, returns an autogenerated name for it.
+    /// </summary>
+    public static string CalculatePrototypeName(string type)
+    {
+        var name = type.AsSpan();
+        if (!type.EndsWith(PrototypeNameEnding))
+            return $"{char.ToLowerInvariant(name[0])}{name.Slice(1).ToString()}";
+
+        return $"{char.ToLowerInvariant(name[0])}{name.Slice(1, name.Length - PrototypeNameEnding.Length - 1).ToString()}";
+    }
+}