From b6548c870cffad2fa55e9125670c16078cbeeb67 Mon Sep 17 00:00:00 2001 From: Tayrtahn Date: Tue, 23 Jul 2024 13:01:43 -0400 Subject: [PATCH] Add analyzer/fixer for replacing ProtoId with EntProtoId (#5312) * Add PreferOtherTypeAttribute, analyzer, and test. * nullable enable * Add nuget package for CodeFix verifier * Add fixer for PreferOtherType * Rename arguments * Adjust diagnostic message * Move attribute lookup --- Directory.Packages.props | 1 + .../PreferOtherTypeAnalyzerTest.cs | 62 ++++++++++++ .../PreferOtherTypeFixerTest.cs | 81 ++++++++++++++++ .../Robust.Analyzers.Tests.csproj | 2 + Robust.Analyzers/PreferOtherTypeAnalyzer.cs | 75 ++++++++++++++ Robust.Analyzers/PreferOtherTypeFixer.cs | 97 +++++++++++++++++++ Robust.Analyzers/Robust.Analyzers.csproj | 5 + Robust.Roslyn.Shared/Diagnostics.cs | 1 + .../Analyzers/PreferOtherTypeAttribute.cs | 18 ++++ Robust.Shared/Prototypes/ProtoId.cs | 1 + 10 files changed, 343 insertions(+) create mode 100644 Robust.Analyzers.Tests/PreferOtherTypeAnalyzerTest.cs create mode 100644 Robust.Analyzers.Tests/PreferOtherTypeFixerTest.cs create mode 100644 Robust.Analyzers/PreferOtherTypeAnalyzer.cs create mode 100644 Robust.Analyzers/PreferOtherTypeFixer.cs create mode 100644 Robust.Shared/Analyzers/PreferOtherTypeAttribute.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 0145e680a..a709466be 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -19,6 +19,7 @@ + diff --git a/Robust.Analyzers.Tests/PreferOtherTypeAnalyzerTest.cs b/Robust.Analyzers.Tests/PreferOtherTypeAnalyzerTest.cs new file mode 100644 index 000000000..f774c0f37 --- /dev/null +++ b/Robust.Analyzers.Tests/PreferOtherTypeAnalyzerTest.cs @@ -0,0 +1,62 @@ +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; + +namespace Robust.Analyzers.Tests; + +[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)] +[TestFixture] +public sealed class PreferOtherTypeAnalyzerTest +{ + private static Task Verifier(string code, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest() + { + TestState = + { + Sources = { code }, + }, + }; + + TestHelper.AddEmbeddedSources( + test.TestState, + "Robust.Shared.Analyzers.PreferOtherTypeAttribute.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 EntityPrototype { }; + public class EntProtoId { }; + public class ReagentPrototype { }; + + [PreferOtherType(typeof(EntityPrototype), typeof(EntProtoId))] + public class ProtoId { }; + + public class Test + { + public ProtoId Bad = new(); + + public ProtoId Good = new(); + } + """; + + await Verifier(code, + // /0/Test0.cs(12,12): warning RA0031: Use the specific type EntProtoId instead of ProtoId when the type argument is EntityPrototype + VerifyCS.Diagnostic().WithSpan(12, 12, 12, 48).WithArguments("EntProtoId", "ProtoId", "EntityPrototype") + ); + } +} diff --git a/Robust.Analyzers.Tests/PreferOtherTypeFixerTest.cs b/Robust.Analyzers.Tests/PreferOtherTypeFixerTest.cs new file mode 100644 index 000000000..b475c51bd --- /dev/null +++ b/Robust.Analyzers.Tests/PreferOtherTypeFixerTest.cs @@ -0,0 +1,81 @@ +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; + +namespace Robust.Analyzers.Tests; + +public sealed class PreferOtherTypeFixerTest +{ + private static Task Verifier(string code, string fixedCode, params DiagnosticResult[] expected) + { + var test = new CSharpCodeFixTest() + { + TestState = + { + Sources = { code }, + }, + FixedState = + { + Sources = { fixedCode }, + } + }; + + TestHelper.AddEmbeddedSources( + test.TestState, + "Robust.Shared.Analyzers.PreferOtherTypeAttribute.cs" + ); + + TestHelper.AddEmbeddedSources( + test.FixedState, + "Robust.Shared.Analyzers.PreferOtherTypeAttribute.cs" + ); + + test.TestState.ExpectedDiagnostics.AddRange(expected); + + return test.RunAsync(); + } + + [Test] + public async Task Test() + { + const string code = """ + using Robust.Shared.Analyzers; + + public class EntityPrototype { }; + public class EntProtoId { }; + public class ReagentPrototype { }; + + [PreferOtherType(typeof(EntityPrototype), typeof(EntProtoId))] + public class ProtoId { }; + + public class Test + { + public ProtoId Foo = new(); + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + + public class EntityPrototype { }; + public class EntProtoId { }; + public class ReagentPrototype { }; + + [PreferOtherType(typeof(EntityPrototype), typeof(EntProtoId))] + public class ProtoId { }; + + public class Test + { + public EntProtoId Foo = new(); + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(12,12): error RA0031: Use the specific type EntProtoId instead of ProtoId when the type argument is EntityPrototype + VerifyCS.Diagnostic().WithSpan(12, 12, 12, 48).WithArguments("EntProtoId", "ProtoId", "EntityPrototype")); + } +} diff --git a/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj b/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj index 10d7d5499..3eb6b7c1e 100644 --- a/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj +++ b/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj @@ -12,6 +12,7 @@ + @@ -27,6 +28,7 @@ + diff --git a/Robust.Analyzers/PreferOtherTypeAnalyzer.cs b/Robust.Analyzers/PreferOtherTypeAnalyzer.cs new file mode 100644 index 000000000..fbc5b93f1 --- /dev/null +++ b/Robust.Analyzers/PreferOtherTypeAnalyzer.cs @@ -0,0 +1,75 @@ +#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; + +namespace Robust.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class PreferOtherTypeAnalyzer : DiagnosticAnalyzer +{ + private const string AttributeType = "Robust.Shared.Analyzers.PreferOtherTypeAttribute"; + + private static readonly DiagnosticDescriptor PreferOtherTypeDescriptor = new( + Diagnostics.IdPreferOtherType, + "Use the specific type", + "Use the specific type {0} instead of {1} when the type argument is {2}", + "Usage", + DiagnosticSeverity.Error, + true, + "Use the specific type."); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( + PreferOtherTypeDescriptor + ); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.ReportDiagnostics | GeneratedCodeAnalysisFlags.Analyze); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeField, SyntaxKind.VariableDeclaration); + } + + private void AnalyzeField(SyntaxNodeAnalysisContext context) + { + if (context.Node is not VariableDeclarationSyntax node) + return; + + // Get the type of the generic being used + if (node.Type is not GenericNameSyntax genericName) + return; + var genericSyntax = genericName.TypeArgumentList.Arguments[0]; + if (context.SemanticModel.GetSymbolInfo(genericSyntax).Symbol is not { } genericType) + return; + + // Look for the PreferOtherTypeAttribute + var symbolInfo = context.SemanticModel.GetSymbolInfo(node.Type); + if (symbolInfo.Symbol?.GetAttributes() is not { } attributes) + return; + + var preferOtherTypeAttribute = context.Compilation.GetTypeByMetadataName(AttributeType); + + foreach (var attribute in attributes) + { + if (!SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, preferOtherTypeAttribute)) + continue; + + // See if the generic type argument matches the type the attribute specifies + if (attribute.ConstructorArguments[0].Value is not ITypeSymbol checkedType) + return; + if (!SymbolEqualityComparer.Default.Equals(checkedType, genericType)) + continue; + + if (attribute.ConstructorArguments[1].Value is not ITypeSymbol replacementType) + continue; + context.ReportDiagnostic(Diagnostic.Create(PreferOtherTypeDescriptor, + context.Node.GetLocation(), + replacementType.Name, + symbolInfo.Symbol.Name, + genericType.Name)); + } + } +} diff --git a/Robust.Analyzers/PreferOtherTypeFixer.cs b/Robust.Analyzers/PreferOtherTypeFixer.cs new file mode 100644 index 000000000..90b1f5724 --- /dev/null +++ b/Robust.Analyzers/PreferOtherTypeFixer.cs @@ -0,0 +1,97 @@ +#nullable enable +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using static Robust.Roslyn.Shared.Diagnostics; + +namespace Robust.Analyzers; + +[ExportCodeFixProvider(LanguageNames.CSharp)] +public sealed class PreferOtherTypeFixer : CodeFixProvider +{ + private const string PreferOtherTypeAttributeName = "PreferOtherTypeAttribute"; + + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create( + IdPreferOtherType + ); + + public override FixAllProvider GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + foreach (var diagnostic in context.Diagnostics) + { + switch (diagnostic.Id) + { + case IdPreferOtherType: + return RegisterReplaceType(context, diagnostic); + } + } + + return Task.CompletedTask; + } + + private static async Task RegisterReplaceType(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().First(); + + if (token == null) + return; + + context.RegisterCodeFix(CodeAction.Create( + "Replace type", + c => ReplaceType(context.Document, token, c), + "Replace type" + ), diagnostic); + } + + private static async Task ReplaceType(Document document, VariableDeclarationSyntax syntax, CancellationToken cancellation) + { + var root = (CompilationUnitSyntax?) await document.GetSyntaxRootAsync(cancellation); + var model = await document.GetSemanticModelAsync(cancellation); + + if (model == null) + return document; + + if (syntax.Type is not GenericNameSyntax genericNameSyntax) + return document; + var genericTypeSyntax = genericNameSyntax.TypeArgumentList.Arguments[0]; + if (model.GetSymbolInfo(genericTypeSyntax).Symbol is not {} genericTypeSymbol) + return document; + + var symbolInfo = model.GetSymbolInfo(syntax.Type); + if (symbolInfo.Symbol?.GetAttributes() is not { } attributes) + return document; + + foreach (var attribute in attributes) + { + if (attribute.AttributeClass?.Name != PreferOtherTypeAttributeName) + continue; + + if (attribute.ConstructorArguments[0].Value is not ITypeSymbol checkedTypeSymbol) + continue; + + if (!SymbolEqualityComparer.Default.Equals(checkedTypeSymbol, genericTypeSymbol)) + continue; + + if (attribute.ConstructorArguments[1].Value is not ITypeSymbol replacementTypeSymbol) + continue; + + var replacementIdentifier = SyntaxFactory.IdentifierName(replacementTypeSymbol.Name); + var replacementSyntax = syntax.WithType(replacementIdentifier); + + root = root!.ReplaceNode(syntax, replacementSyntax); + return document.WithSyntaxRoot(root); + } + + return document; + } +} diff --git a/Robust.Analyzers/Robust.Analyzers.csproj b/Robust.Analyzers/Robust.Analyzers.csproj index 1d0783481..f6f5023a1 100644 --- a/Robust.Analyzers/Robust.Analyzers.csproj +++ b/Robust.Analyzers/Robust.Analyzers.csproj @@ -21,6 +21,11 @@ + + + + + diff --git a/Robust.Roslyn.Shared/Diagnostics.cs b/Robust.Roslyn.Shared/Diagnostics.cs index 6d27bdfa3..ec0664fbb 100644 --- a/Robust.Roslyn.Shared/Diagnostics.cs +++ b/Robust.Roslyn.Shared/Diagnostics.cs @@ -34,6 +34,7 @@ public static class Diagnostics public const string IdMustCallBase = "RA0028"; public const string IdDataFieldNoVVReadWrite = "RA0029"; public const string IdUseNonGenericVariant = "RA0030"; + public const string IdPreferOtherType = "RA0031"; public static SuppressionDescriptor MeansImplicitAssignment => new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned."); diff --git a/Robust.Shared/Analyzers/PreferOtherTypeAttribute.cs b/Robust.Shared/Analyzers/PreferOtherTypeAttribute.cs new file mode 100644 index 000000000..67e96c88f --- /dev/null +++ b/Robust.Shared/Analyzers/PreferOtherTypeAttribute.cs @@ -0,0 +1,18 @@ +using System; + +#if ROBUST_ANALYZERS_IMPL +namespace Robust.Shared.Analyzers.Implementation; +#else +namespace Robust.Shared.Analyzers; +#endif + +/// +/// Marks that use of a generic Type should be replaced with a specific other Type +/// when the type argument T is a certain Type. +/// +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct)] +public sealed class PreferOtherTypeAttribute(Type genericType, Type replacementType) : Attribute +{ + public readonly Type GenericArgument = genericType; + public readonly Type ReplacementType = replacementType; +} diff --git a/Robust.Shared/Prototypes/ProtoId.cs b/Robust.Shared/Prototypes/ProtoId.cs index 4871f8fe6..18ff7e2ca 100644 --- a/Robust.Shared/Prototypes/ProtoId.cs +++ b/Robust.Shared/Prototypes/ProtoId.cs @@ -13,6 +13,7 @@ namespace Robust.Shared.Prototypes; /// /// for an alias. [Serializable] +[PreferOtherType(typeof(EntityPrototype), typeof(EntProtoId))] public readonly record struct ProtoId(string Id) : IEquatable, IComparable> where T : class, IPrototype { public static implicit operator string(ProtoId protoId)