diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 3bf2c622d..320547e84 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -35,7 +35,9 @@ END TEMPLATE--> ### Breaking changes -*None yet* +* A new analyzer has been added that will error if you attempt to subscribe to `AfterAutoHandleStateEvent` on a + component that doesn't have the `AutoGenerateComponentState` attribute, or doesn't have the first argument of that + attribute set to `true`. In most cases you will want to set said argument to `true`. ### New features @@ -43,6 +45,7 @@ END TEMPLATE--> * added **IConfigurationManager**.*SubscribeMultiple* ext. method to provide simpler way to unsubscribe from multiple cvar at once * Added `SharedMapSystem.QueueDeleteMap`, which deletes a map with the specified MapId in the next tick. * Added generic version of `ComponentRegistry.TryGetComponent`. +* `AttributeHelper.HasAttribute` has had an overload's type signature loosened from `INamedTypeSymbol` to `ITypeSymbol`. ### Bugfixes diff --git a/Robust.Analyzers.Tests/AfterAutoHandleStateAnalyzerTest.cs b/Robust.Analyzers.Tests/AfterAutoHandleStateAnalyzerTest.cs new file mode 100644 index 000000000..8bbafe314 --- /dev/null +++ b/Robust.Analyzers.Tests/AfterAutoHandleStateAnalyzerTest.cs @@ -0,0 +1,117 @@ +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(AfterAutoHandleStateAnalyzer))] +public sealed class AfterAutoHandleStateAnalyzerTest +{ + private const string SubscribeEventDef = """ + using System; + namespace Robust.Shared.GameObjects; + + public readonly struct EntityUid; + + public abstract class EntitySystem + { + public void SubscribeLocalEvent() where TEvent : notnull { } + } + + public interface IComponent; + public interface IComponentState; + """; + + // A rare case for block-scoped namespace, I thought. Then I realized this + // only needed the one type definition. + private const string OtherTypeDefs = """ + using System; + + namespace JetBrains.Annotations + { + public sealed class BaseTypeRequiredAttribute(Type baseType) : Attribute; + } + """; + + private static Task Verifier(string code, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest + { + TestState = { Sources = { code } } + }; + + TestHelper.AddEmbeddedSources(test.TestState, + "Robust.Shared.Analyzers.ComponentNetworkGeneratorAuxiliary.cs", + "Robust.Shared.GameObjects.EventBusAttributes.cs"); + + test.TestState.Sources.Add(("EntitySystem.Subscriptions.cs", SubscribeEventDef)); + test.TestState.Sources.Add(("Types.cs", OtherTypeDefs)); + + test.TestState.ExpectedDiagnostics.AddRange(expected); + + return test.RunAsync(); + } + + [Test] + public async Task Test() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + [AutoGenerateComponentState(true)] + public sealed class AutoGenTrue; + [AutoGenerateComponentState(true, true)] + public sealed class AutoGenTrueTrue; + + public sealed class NotAutoGen; + [AutoGenerateComponentState] + public sealed class AutoGenNoArgs; + [AutoGenerateComponentState(false)] + public sealed class AutoGenFalse; + [AutoGenerateComponentState(RaiseAfterAutoHandleState = true)] + public sealed class AutoGenIncorrectConstructorArg; + + public sealed class Foo : EntitySystem + { + public void Good() + { + // Subscribing to other events works + SubscribeLocalEvent(); + // First arg true allows subscribing + SubscribeLocalEvent(); + SubscribeLocalEvent(); + } + + public void Bad() + { + // Can't subscribe if AutoGenerateComponentState isn't even present + SubscribeLocalEvent(); + + // Can't subscribe if first arg is not specified/false + SubscribeLocalEvent(); + SubscribeLocalEvent(); + + // Can't subscribe with RaiseAfterAutoHandleState = true because that's + // secretly a no-op. + SubscribeLocalEvent(); + } + } + """; + + await Verifier(code, + // /0/Test0.cs(31,9): error RA0040: Tried to subscribe to AfterAutoHandleStateEvent for 'NotAutoGen' which doesn't have an AutoGenerateComponentState attribute + VerifyCS.Diagnostic(AfterAutoHandleStateAnalyzer.MissingAttribute).WithSpan(31, 9, 31, 69).WithArguments("NotAutoGen"), + // /0/Test0.cs(34,9): error RA0041: Tried to subscribe to AfterAutoHandleStateEvent for 'AutoGenNoArgs' which doesn't have RaiseAfterAutoHandleState set + VerifyCS.Diagnostic(AfterAutoHandleStateAnalyzer.MissingAttributeParam).WithSpan(34, 9, 34, 72).WithArguments("AutoGenNoArgs"), + // /0/Test0.cs(35,9): error RA0041: Tried to subscribe to AfterAutoHandleStateEvent for 'AutoGenFalse' which doesn't have RaiseAfterAutoHandleState set + VerifyCS.Diagnostic(AfterAutoHandleStateAnalyzer.MissingAttributeParam).WithSpan(35, 9, 35, 71).WithArguments("AutoGenFalse"), + // /0/Test0.cs(39,9): error RA0041: Tried to subscribe to AfterAutoHandleStateEvent for 'AutoGenIncorrectConstructorArg' which doesn't have RaiseAfterAutoHandleState set + VerifyCS.Diagnostic(AfterAutoHandleStateAnalyzer.MissingAttributeParam).WithSpan(39, 9, 39, 89).WithArguments("AutoGenIncorrectConstructorArg") ); + } +} diff --git a/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj b/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj index 8f0f1f8f9..a6d59100b 100644 --- a/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj +++ b/Robust.Analyzers.Tests/Robust.Analyzers.Tests.csproj @@ -10,6 +10,7 @@ + diff --git a/Robust.Analyzers/AfterAutoHandleStateAnalyzer.cs b/Robust.Analyzers/AfterAutoHandleStateAnalyzer.cs new file mode 100644 index 000000000..aca89af56 --- /dev/null +++ b/Robust.Analyzers/AfterAutoHandleStateAnalyzer.cs @@ -0,0 +1,85 @@ +#nullable enable +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Robust.Roslyn.Shared; + +namespace Robust.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class AfterAutoHandleStateAnalyzer : DiagnosticAnalyzer +{ + private const string AfterAutoHandleStateEventName = "AfterAutoHandleStateEvent"; + private const string AutoGenStateAttribute = "Robust.Shared.Analyzers.AutoGenerateComponentStateAttribute"; + private const string SubscribeLocalEventName = "SubscribeLocalEvent"; + + public static readonly DiagnosticDescriptor MissingAttribute = new( + Diagnostics.IdAutoGenStateAttributeMissing, + "Unreachable AfterAutoHandleState subscription", + "Tried to subscribe to AfterAutoHandleStateEvent for '{0}' which doesn't have an " + + "AutoGenerateComponentState attribute", + "Usage", + DiagnosticSeverity.Error, + true, + // Does this even show up anywhere in Rider? >:( + "You must mark your component with '[AutoGenerateComponentState(true)]' to subscribe to this event." + ); + + public static readonly DiagnosticDescriptor MissingAttributeParam = new( + Diagnostics.IdAutoGenStateParamMissing, + "Unreachable AfterAutoHandleState subscription", + "Tried to subscribe to AfterAutoHandleStateEvent for '{0}' which doesn't have " + + "raiseAfterAutoHandleState set", + "Usage", + DiagnosticSeverity.Error, + true, + "The AutoGenerateComponentState attribute must be passed 'true' in order to subscribe to this event." + ); + + public override ImmutableArray SupportedDiagnostics => + [MissingAttribute, MissingAttributeParam]; + + public override void Initialize(AnalysisContext context) + { + // This is more to stop user error rather than code generation error + // (Plus this shouldn't affect code gen anyway) + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(compilationContext => + { + var autoGenStateAttribute = compilationContext.Compilation.GetTypeByMetadataName(AutoGenStateAttribute); + // No attribute, no analyzer. + if (autoGenStateAttribute is null) + return; + + compilationContext.RegisterOperationAction( + analysisContext => CheckEventSubscription(analysisContext, autoGenStateAttribute), + OperationKind.Invocation); + }); + } + + private static void CheckEventSubscription(OperationAnalysisContext context, ITypeSymbol autoGenStateAttribute) + { + if (context.Operation is not IInvocationOperation operation) + return; + + // Check the method has the right name and has the right type args + if (operation.TargetMethod is not + { Name: SubscribeLocalEventName, TypeArguments: [var component, { Name: AfterAutoHandleStateEventName }] }) + return; + + // Search the component's attributes for something matching autoGenStateAttribute + AttributeHelper.HasAttribute(component, autoGenStateAttribute, out var autoGenAttribute); + + // First argument is raiseAfterAutoHandleState—note it shouldn't ever + // be null, since it has a default, but eh. + if (autoGenAttribute?.ConstructorArguments[0].Value is true) + return; + + context.ReportDiagnostic(Diagnostic.Create(autoGenAttribute is null ? MissingAttribute : MissingAttributeParam, + operation.Syntax.GetLocation(), + component.Name)); + } +} diff --git a/Robust.Roslyn.Shared/AttributeHelper.cs b/Robust.Roslyn.Shared/AttributeHelper.cs index d907a8025..3e5a49852 100644 --- a/Robust.Roslyn.Shared/AttributeHelper.cs +++ b/Robust.Roslyn.Shared/AttributeHelper.cs @@ -45,8 +45,8 @@ public static class AttributeHelper } public static bool HasAttribute( - INamedTypeSymbol symbol, - INamedTypeSymbol attribute, + ITypeSymbol symbol, + ITypeSymbol attribute, [NotNullWhen(true)] out AttributeData? matchedAttribute) { matchedAttribute = null; diff --git a/Robust.Roslyn.Shared/Diagnostics.cs b/Robust.Roslyn.Shared/Diagnostics.cs index 6525bbfd3..c19fa62af 100644 --- a/Robust.Roslyn.Shared/Diagnostics.cs +++ b/Robust.Roslyn.Shared/Diagnostics.cs @@ -43,6 +43,8 @@ public static class Diagnostics public const string IdPrototypeNetSerializable = "RA0037"; public const string IdPrototypeSerializable = "RA0038"; public const string IdPrototypeInstantiation = "RA0039"; + public const string IdAutoGenStateAttributeMissing = "RA0040"; + public const string IdAutoGenStateParamMissing = "RA0041"; public static SuppressionDescriptor MeansImplicitAssignment => new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");