From ef3390075fd3600d965ba3a441446fd4cba85bc0 Mon Sep 17 00:00:00 2001 From: Tayrtahn Date: Wed, 25 Mar 2026 15:51:48 -0400 Subject: [PATCH] Add exclusivity check for virtual + analyzer test --- .../ExplicitVirtualAnalyzerTest.cs | 111 ++++++++++++++++++ .../Robust.Analyzers.Tests.csproj | 1 + Robust.Analyzers/ExplicitVirtualAnalyzer.cs | 58 ++++++--- Robust.Analyzers/Robust.Analyzers.csproj | 5 + Robust.Roslyn.Shared/Diagnostics.cs | 1 + 5 files changed, 158 insertions(+), 18 deletions(-) create mode 100644 Robust.Analyzers.Tests/ExplicitVirtualAnalyzerTest.cs diff --git a/Robust.Analyzers.Tests/ExplicitVirtualAnalyzerTest.cs b/Robust.Analyzers.Tests/ExplicitVirtualAnalyzerTest.cs new file mode 100644 index 00000000000..fc5b88179b7 --- /dev/null +++ b/Robust.Analyzers.Tests/ExplicitVirtualAnalyzerTest.cs @@ -0,0 +1,111 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; +using NUnit.Framework; +using VerifyCS = + Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier; + +namespace Robust.Analyzers.Tests; + +[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)] +[TestFixture] +[TestOf(typeof(ExplicitVirtualAnalyzer))] +public sealed class ExplicitVirtualAnalyzerTest +{ + private static Task Verifier(string code, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest() + { + TestState = + { + Sources = { code }, + }, + }; + + TestHelper.AddEmbeddedSources( + test.TestState, + "Robust.Shared.Analyzers.VirtualAttribute.cs" + ); + + // ExpectedDiagnostics cannot be set, so we need to AddRange here... + test.TestState.ExpectedDiagnostics.AddRange(expected); + + return test.RunAsync(); + } + + [Test] + [Description("Ensures that a non-sealed/abstract/static class not marked as [Virtual] raises a warning.")] + public async Task NoVirtualOrOther() + { + const string code = """ + public class Foo { } + """; + + await Verifier(code, + // /0/Test0.cs(1,8): warning RA0003: Class must be explicitly marked as [Virtual], abstract, static, or sealed + VerifyCS.Diagnostic(ExplicitVirtualAnalyzer.ExplicitVirtualRule).WithSpan(1, 8, 1, 13) + ); + } + + [Test] + [Description("Ensures that a non-sealed/abstract/static class explicitly marked as [Virtual] does not raise a warning.")] + public async Task OnlyVirtual() + { + const string code = """ + using Robust.Shared.Analyzers; + + [Virtual] + public class Foo { } + """; + + await Verifier(code, []); + } + + [Test] + [Description("Ensures that a sealed class marked as [Virtual] raises an error.")] + public async Task SealedVirtual() + { + const string code = """ + using Robust.Shared.Analyzers; + + [Virtual] + public sealed class Foo { } + """; + + await Verifier(code, + // /0/Test0.cs(4,15): error RA0048: A class marked as [Virtual] cannot be abstract, static, or sealed + VerifyCS.Diagnostic(ExplicitVirtualAnalyzer.ExclusiveRule).WithSpan(4, 15, 4, 20)); + } + + [Test] + [Description("Ensures that an abstract class marked as [Virtual] raises an error.")] + public async Task AbstractVirtual() + { + const string code = """ + using Robust.Shared.Analyzers; + + [Virtual] + public abstract class Foo { } + """; + + await Verifier(code, + // /0/Test0.cs(4,17): error RA0048: A class marked as [Virtual] cannot be abstract, static, or sealed + VerifyCS.Diagnostic(ExplicitVirtualAnalyzer.ExclusiveRule).WithSpan(4, 17, 4, 22)); + } + + [Test] + [Description("Ensures that a static class marked as [Virtual] raises an error.")] + public async Task StaticVirtual() + { + const string code = """ + using Robust.Shared.Analyzers; + + [Virtual] + public static class Foo { } + """; + + await Verifier(code, + // /0/Test0.cs(4,15): error RA0048: A class marked as [Virtual] cannot be abstract, static, or sealed + VerifyCS.Diagnostic(ExplicitVirtualAnalyzer.ExclusiveRule).WithSpan(4, 15, 4, 20)); + } +} diff --git a/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj b/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj index 2c1750469b0..f4bf86a58ea 100644 --- a/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj +++ b/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj @@ -18,6 +18,7 @@ + diff --git a/Robust.Analyzers/ExplicitVirtualAnalyzer.cs b/Robust.Analyzers/ExplicitVirtualAnalyzer.cs index 1b4a2a986b5..1dbc2abff2c 100644 --- a/Robust.Analyzers/ExplicitVirtualAnalyzer.cs +++ b/Robust.Analyzers/ExplicitVirtualAnalyzer.cs @@ -1,6 +1,5 @@ using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -15,45 +14,68 @@ public sealed class ExplicitVirtualAnalyzer : DiagnosticAnalyzer internal const string Attribute = "Robust.Shared.Analyzers.VirtualAttribute"; [SuppressMessage("ReSharper", "RS2008")] - private static readonly DiagnosticDescriptor Rule = new( + public static readonly DiagnosticDescriptor ExplicitVirtualRule = new( Diagnostics.IdExplicitVirtual, - "Class must be explicitly marked as [Virtual], abstract, static or sealed", - "Class must be explicitly marked as [Virtual], abstract, static or sealed", + "Class must be explicitly marked as [Virtual], abstract, static, or sealed", + "Class must be explicitly marked as [Virtual], abstract, static, or sealed", "Usage", DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: "Class must be explicitly marked as [Virtual], abstract, static or sealed."); + description: "Class must be explicitly marked as [Virtual], abstract, static, or sealed."); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + public static readonly DiagnosticDescriptor ExclusiveRule = new( + Diagnostics.IdExclusiveVirtual, + "A class marked as [Virtual] cannot be abstract, static, or sealed", + "A class marked as [Virtual] cannot be abstract, static, or sealed", + "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "A class marked as [Virtual] cannot be abstract, static, or sealed."); + + public override ImmutableArray SupportedDiagnostics => + [ + ExplicitVirtualRule, + ExclusiveRule, + ]; public override void Initialize(AnalysisContext context) { context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); - context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.ClassDeclaration); - } + context.RegisterCompilationStartAction(ctx => + { + if (ctx.Compilation.GetTypeByMetadataName(Attribute) is not INamedTypeSymbol attrSymbol) + return; - private static bool HasAttribute(INamedTypeSymbol namedTypeSymbol, INamedTypeSymbol attrSymbol) - { - return namedTypeSymbol.GetAttributes() - .Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, attrSymbol)); + ctx.RegisterSyntaxNodeAction(nodeContext => AnalyzeNode(nodeContext, attrSymbol), SyntaxKind.ClassDeclaration); + }); } - private static void AnalyzeNode(SyntaxNodeAnalysisContext context) + private static void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol attrSymbol) { - var attrSymbol = context.Compilation.GetTypeByMetadataName(Attribute); var classDecl = (ClassDeclarationSyntax)context.Node; var classSymbol = context.SemanticModel.GetDeclaredSymbol(classDecl); if (classSymbol == null) return; - if (classSymbol.IsSealed || classSymbol.IsAbstract || classSymbol.IsStatic) - return; + var hasKeyword = classSymbol.IsSealed || classSymbol.IsAbstract || classSymbol.IsStatic; + var hasAttribute = AttributeHelper.HasAttribute(classSymbol, attrSymbol, out _); + + if (hasAttribute && hasKeyword) + { + // Having both [Virtual] and sealed/abstract/static doesn't make sense. + context.ReportDiagnostic(Diagnostic.Create( + ExclusiveRule, + classDecl.Keyword.GetLocation() + )); + } - if (HasAttribute(classSymbol, attrSymbol)) + // Having just [Virtual] or sealed/abstract/static is fine. + if (hasKeyword || hasAttribute) return; - var diag = Diagnostic.Create(Rule, classDecl.Keyword.GetLocation()); + // Having neither is bad. + var diag = Diagnostic.Create(ExplicitVirtualRule, classDecl.Keyword.GetLocation()); context.ReportDiagnostic(diag); } } diff --git a/Robust.Analyzers/Robust.Analyzers.csproj b/Robust.Analyzers/Robust.Analyzers.csproj index 082dca23eec..d05d8052e22 100644 --- a/Robust.Analyzers/Robust.Analyzers.csproj +++ b/Robust.Analyzers/Robust.Analyzers.csproj @@ -38,6 +38,11 @@ + + + + + diff --git a/Robust.Roslyn.Shared/Diagnostics.cs b/Robust.Roslyn.Shared/Diagnostics.cs index d2ea2c6d3cd..459d701a18f 100644 --- a/Robust.Roslyn.Shared/Diagnostics.cs +++ b/Robust.Roslyn.Shared/Diagnostics.cs @@ -51,6 +51,7 @@ public static class Diagnostics public const string IdPreferProxy = "RA0045"; public const string IdProxyForRedundantMethodName = "RA0046"; public const string IdProxyForTargetMethodNotFound = "RA0047"; + public const string IdExclusiveVirtual = "RA0048"; public static SuppressionDescriptor MeansImplicitAssignment => new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");