diff --git a/Robust.Analyzers.Tests/EntitySystemSubscriptionConversionAnalyzerTest.cs b/Robust.Analyzers.Tests/EntitySystemSubscriptionConversionAnalyzerTest.cs new file mode 100644 index 00000000000..3c5fdb4c339 --- /dev/null +++ b/Robust.Analyzers.Tests/EntitySystemSubscriptionConversionAnalyzerTest.cs @@ -0,0 +1,274 @@ +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; + +[TestOf(typeof(EntitySystemSubscriptionConversionAnalyzer))] +public sealed class EntitySystemSubscriptionConversionAnalyzerTest +{ + private static Task Verifier(string code, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest() + { + TestState = + { + Sources = { code } + }, + }; + + test.TestState.Sources.Add(("TestTypeDefs.cs", TestTypeDefs)); + + // ExpectedDiagnostics cannot be set, so we need to AddRange here... + test.TestState.ExpectedDiagnostics.AddRange(expected); + + return test.RunAsync(); + } + + private const string TestTypeDefs = """ + using Robust.Shared.GameObjects; + using System; + + namespace Robust.Shared.GameObjects + { + public interface IComponent; + public abstract class Component : IComponent; + + public readonly struct EntityUid; + + public delegate void ComponentEventRefHandler(EntityUid uid, TComp component, ref TEvent args) + where TComp : IComponent + where TEvent : notnull; + + public delegate void ComponentEventHandler(EntityUid uid, TComp component, TEvent args) + where TComp : IComponent + where TEvent : notnull; + + public interface IEntitySystem; + public abstract class EntitySystem : IEntitySystem + { + public virtual void Initialize() { } + public void SubscribeLocalEvent( + ComponentEventRefHandler handler, + Type[]? before = null, + Type[]? after = null) + where TComp : IComponent + where TEvent : notnull + { } + + public void SubscribeLocalEvent( + ComponentEventHandler handler, + Type[]? before = null, + Type[]? after = null) + where TComp : IComponent + where TEvent : notnull + { } + } + } + + namespace Robust.Shared.Analyzers + { + public sealed class SubscribeLocalEventAttribute : Attribute; + } + + public readonly struct TestEvent; + public readonly struct TestEvent2; + public readonly struct TestEvent3; + public sealed partial class TestComponent : IComponent; + """; + + [Test] + [Description("Tests that a SubscribeLocalEvent invocation in an EntitySystem Intialize method is flagged as elligible for conversion.")] + public async Task FlagSubscribeLocalEvent() + { + const string code = """ + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, + // /0/Test0.cs(9,9): info RA0058: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(9, 9, 9, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a subscription using an anonymous delegate is not flagged as elligible for conversion.")] + public async Task IgnoreAnonymousDelegate() + { + const string code = """ + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent((u, c, ref _) => OnTest(u, c)); + } + + private void OnTest(EntityUid uid, TestComponent comp) { } + } + """; + + await Verifier(code, []); + } + + [Test] + [Description("Tests that a subscription in a method containing preprocessor directives is not flagged as elligible for conversion.")] + public async Task IgnoreWithPreprocessorDirectives() + { + const string code = """ + + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + #if DEBUG + SubscribeLocalEvent(OnTest); + #else + SubscribeLocalEvent(OnTest2); + #endif + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) { } + private void OnTest2(EntityUid uid, TestComponent comp, ref TestEvent args) { } + } + """; + + await Verifier(code, []); + } + + [Test] + [Description("Tests that a subscription using a generic type parameter is not flagged as elligible for conversion.")] + public async Task IgnoreWithGenericComponent() + { + const string code = """ + + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + where TComp : Component + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); + } + + private void OnTest(EntityUid uid, TComp comp, ref TestEvent args) { } + } + """; + + await Verifier(code, []); + } + + [Test] + [Description("Tests that subscriptions using generic methods as event handlers are not flagged as elligible for conversion.")] + public async Task IgnoreWithGenericHandler() + { + const string code = """ + + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); + SubscribeLocalEvent(OnTest); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref T args) where T : TestEventArgs { } + } + + public class TestEventArgs; + public sealed class TestEventClassA : TestEventArgs; + public sealed class TestEventClassB : TestEventArgs; + """; + + await Verifier(code, []); + } + + [Test] + [Description("Tests that subscriptions using event handlers with abstract event types are not flagged as elligible for conversion.")] + public async Task IgnoreWithAbstractHandler() + { + const string code = """ + + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); + SubscribeLocalEvent(OnTest); + } + + private void OnTest(EntityUid uid, TestComponent comp, TestEventArgs args) { } + } + + public abstract class TestEventArgs; + public sealed class TestEventClassA : TestEventArgs; + public sealed class TestEventClassB : TestEventArgs; + """; + + await Verifier(code, []); + } + + [Test] + [Description("Tests that subscriptions within if statement blocks are not flagged as elligible for conversion.")] + public async Task IgnoreWithIfStatement() + { + const string code = """ + + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + if (true) + SubscribeLocalEvent(OnTest); + else + SubscribeLocalEvent(OnTest2); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) { } + private void OnTest2(EntityUid uid, TestComponent comp, ref TestEvent args) { } + } + """; + + await Verifier(code, []); + } +} diff --git a/Robust.Analyzers.Tests/EntitySystemSubscriptionConversionFixerTest.cs b/Robust.Analyzers.Tests/EntitySystemSubscriptionConversionFixerTest.cs new file mode 100644 index 00000000000..2b1c237d17f --- /dev/null +++ b/Robust.Analyzers.Tests/EntitySystemSubscriptionConversionFixerTest.cs @@ -0,0 +1,743 @@ +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; + +public sealed class EntitySystemSubscriptionConversionFixerTest +{ + private static Task Verifier(string code, string fixedCode, params DiagnosticResult[] expected) + { + var test = new CSharpCodeFixTest() + { + TestState = + { + Sources = { code }, + }, + FixedState = + { + Sources = { fixedCode }, + } + }; + + test.TestState.Sources.Add(("TestTypeDefs.cs", TestTypeDefs)); + test.FixedState.Sources.Add(("TestTypeDefs.cs", TestTypeDefs)); + + test.TestState.ExpectedDiagnostics.AddRange(expected); + + return test.RunAsync(); + } + + private static Task Verifier(string[] code, string[] fixedCode, params DiagnosticResult[] expected) + { + var test = new CSharpCodeFixTest(); + + foreach (var file in code) + { + test.TestState.Sources.Add(file); + } + foreach (var file in fixedCode) + { + test.FixedState.Sources.Add(file); + } + + test.TestState.Sources.Add(("TestTypeDefs.cs", TestTypeDefs)); + test.FixedState.Sources.Add(("TestTypeDefs.cs", TestTypeDefs)); + + test.TestState.ExpectedDiagnostics.AddRange(expected); + + return test.RunAsync(); + } + + private const string TestTypeDefs = """ + using Robust.Shared.GameObjects; + using System; + + namespace Robust.Shared.GameObjects + { + public interface IComponent; + public abstract class Component : IComponent; + + public readonly struct EntityUid; + + public delegate void ComponentEventRefHandler(EntityUid uid, TComp component, ref TEvent args) + where TComp : IComponent + where TEvent : notnull; + public delegate void EntityEventHandler(T ev); + public delegate void EntitySessionEventHandler(T msg, string foo); + + public interface IEntitySystem; + public abstract class EntitySystem : IEntitySystem + { + public virtual void Initialize() { } + public void SubscribeLocalEvent( + ComponentEventRefHandler handler, + Type[]? before = null, Type[]? after = null) + where TComp : IComponent + where TEvent : notnull + { } + protected void SubscribeNetworkEvent( + EntityEventHandler handler, + Type[]? before = null, Type[]? after = null) + where T : notnull + { } + protected void SubscribeAllEvent( + EntitySessionEventHandler handler, + Type[]? before = null, Type[]? after = null) + where T : notnull + { } + } + } + + namespace Robust.Shared.Analyzers + { + public sealed class SubscribeLocalEventAttribute(Type[]? before = null, Type[]? after = null) : Attribute; + public sealed class SubscribeNetworkEventAttribute(Type[]? before = null, Type[]? after = null) : Attribute; + public sealed class EventSubscriptionAttribute(Type[]? before = null, Type[]? after = null) : Attribute; + } + + public readonly struct TestEvent; + public readonly struct TestEvent2; + public sealed partial class TestComponent : IComponent; + public sealed class TestNetworkEvent; + + public sealed class SomeOtherSystemA : EntitySystem; + public sealed class SomeOtherSystemB : EntitySystem; + """; + + [Test] + [Description("Tests that a SubscribeLocalEvent invocation is correctly converted to an attribute.")] + public async Task ConvertLocalEvent() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0058: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that multiple SubscribeLocalEvent invocations are correctly converted to attributes.")] + public async Task ConvertLocalEvent_Multiple() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + SubscribeLocalEvent(OnTest2); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + + private void OnTest2(EntityUid uid, TestComponent comp, ref TestEvent2 args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + + [SubscribeLocalEvent] + private void OnTest2(EntityUid uid, TestComponent comp, ref TestEvent2 args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute"), + // /0/Test0.cs(11,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(11, 9, 11, 64).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that multiple SubscribeLocalEvent invocations are correctly converted to attributes when there is a gap between them.")] + public async Task ConvertLocalEvent_MultipleWithGap() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + + SubscribeLocalEvent(OnTest2); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + + private void OnTest2(EntityUid uid, TestComponent comp, ref TestEvent2 args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + + [SubscribeLocalEvent] + private void OnTest2(EntityUid uid, TestComponent comp, ref TestEvent2 args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute"), + // /0/Test0.cs(12,9): info RA0057:Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(12, 9, 12, 64).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a SubscribeLocalEvent invocation with before and after parameters is correctly converted to an attribute.")] + public async Task ConvertLocalEvent_WithBeforeAfter() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest, before: [typeof(SomeOtherSystemA)], after: new[] { typeof(SomeOtherSystemB) }); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent(before: [typeof(SomeOtherSystemA)], after: [typeof(SomeOtherSystemB)])] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0058: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 141).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a SubscribeLocalEvent invocation a before parameter is correctly converted to an attribute.")] + public async Task ConvertLocalEvent_WithBefore() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest, before: new[] { typeof(SomeOtherSystemA) }); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent(before: [typeof(SomeOtherSystemA)])] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0058: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 106).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a SubscribeLocalEvent invocation an after parameter is correctly converted to an attribute.")] + public async Task ConvertLocalEvent_WithAfter() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest, after: [typeof(SomeOtherSystemA), typeof(SomeOtherSystemB)]); + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent(after: [typeof(SomeOtherSystemA), typeof(SomeOtherSystemB)])] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0058: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 123).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a class that isn't marked partial is given the partial modifier when converted.")] + public async Task ConvertLocalEvent_AddPartial() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a class is given the using directive for the SubscribeLocalEventAttribute namespace when converted.")] + public async Task ConvertLocalEvent_AddUsingDirective() + { + const string code = """ + using Robust.Shared.GameObjects; + + public sealed class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(9,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(9, 9, 9, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that the conversion isn't confused by other methods with the same name as the event handler.")] + public async Task ConvertLocalEvent_HandlerOverload() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + } + + private void OnTest(string foo) { } + + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + private void OnTest(string foo) { } + + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } + + [Test] + [Description("Tests that a SubscribeNetworkEvent invocation is correctly converted to an attribute.")] + public async Task ConvertNetworkEvent() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeNetworkEvent(OnTest); // Comment here + } + + private void OnTest(TestNetworkEvent args) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [SubscribeNetworkEvent] + private void OnTest(TestNetworkEvent args) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0058: Event subscription using SubscribeNetworkEvent can be converted to use SubscribeNetworkEventAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 56).WithArguments("SubscribeNetworkEvent", "SubscribeNetworkEventAttribute") + ); + } + + [Test] + [Description("Tests that a SubscribeAllEvent invocation is correctly converted to an attribute.")] + public async Task ConvertAllEvent() + { + const string code = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeAllEvent(OnTest); // Comment here + } + + private void OnTest(TestNetworkEvent args, string foo) + { + // Do something + } + } + """; + + const string fixedCode = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + + [EventSubscription] + private void OnTest(TestNetworkEvent args, string foo) + { + // Do something + } + } + """; + + await Verifier(code, fixedCode, + // /0/Test0.cs(10,9): info RA0058: Event subscription using SubscribeAllEvent can be converted to use EventSubscriptionAttribute + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 52).WithArguments("SubscribeAllEvent", "EventSubscriptionAttribute") + ); + } + + [Test] + [Description("Tests that the conversion works correctly when the Initialize and event handler methods are declared in separate files (partial classes).")] + public async Task ConvertLocalEvent_WithPartials() + { + const string code1 = """ + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + + SubscribeLocalEvent(OnTest); // Comment here + } + } + """; + + const string code2 = """ + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + const string fixed1 = """ + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + public override void Initialize() + { + base.Initialize(); + } + } + """; + + const string fixed2 = """ + using Robust.Shared.Analyzers; + using Robust.Shared.GameObjects; + + public sealed partial class InitalizeBasedSystem : EntitySystem + { + [SubscribeLocalEvent] + private void OnTest(EntityUid uid, TestComponent comp, ref TestEvent args) + { + // Do something + } + } + """; + + await Verifier([code1, code2], [fixed1, fixed2], + // /0/Test0.cs(9,9): info RA0057: Event subscription using SubscribeLocalEvent can be converted to use SubscribeLocalEventAttribute + VerifyCS.Diagnostic().WithSpan(9, 9, 9, 62).WithArguments("SubscribeLocalEvent", "SubscribeLocalEventAttribute") + ); + } +} diff --git a/Robust.Analyzers/EntitySystemSubscriptionConversionAnalyzer.cs b/Robust.Analyzers/EntitySystemSubscriptionConversionAnalyzer.cs new file mode 100644 index 00000000000..358caf99d15 --- /dev/null +++ b/Robust.Analyzers/EntitySystemSubscriptionConversionAnalyzer.cs @@ -0,0 +1,167 @@ +#nullable enable +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; +using Robust.Roslyn.Shared; + +namespace Robust.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class EntitySystemSubscriptionConversionAnalyzer : DiagnosticAnalyzer +{ + private const string EntitySystemTypeName = "Robust.Shared.GameObjects.IEntitySystem"; + private const string SubscribeLocalEventAttributeTypeName = "Robust.Shared.Analyzers.SubscribeLocalEventAttribute"; + private const string SubscribeLocalEventMethodName = "SubscribeLocalEvent"; + private const string SubscribeNetworkEventMethodName = "SubscribeNetworkEvent"; + private const string SubscribeAllEventMethodName = "SubscribeAllEvent"; + private const string SubscribeAllEventAttributeName = "EventSubscription"; + private static readonly string[] SubscribeMethods = + [ + SubscribeLocalEventMethodName, + SubscribeNetworkEventMethodName, + SubscribeAllEventMethodName, + ]; + + /// + /// The key used to access the needed replacement attribute name in the diagnostic's Properties dictionary. + /// + public const string AttributeNameKey = "attribute"; + + public static readonly DiagnosticDescriptor EntitySystemSubscriptionConversionPossible = new( + Diagnostics.IdEntitySystemSubscriptionConversionPossible, + "Convert to attribute-based subscription", + "Event subscription using {0} can be converted to use {1}", + "Usage", + DiagnosticSeverity.Info, + true + ); + + public override ImmutableArray SupportedDiagnostics => + [ + EntitySystemSubscriptionConversionPossible, + ]; + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(ctx => + { + // If the subscription attributes aren't available in this compilation, we can't do anything. + if (ctx.Compilation.GetTypeByMetadataName(SubscribeLocalEventAttributeTypeName) is null) + return; + + if (ctx.Compilation.GetTypeByMetadataName(EntitySystemTypeName) is not { } entitySystemType) + return; + + ctx.RegisterSymbolStartAction(symbolContext => + { + // We only care about classes + if (symbolContext.Symbol is not INamedTypeSymbol typeSymbol || typeSymbol.TypeKind != TypeKind.Class) + return; + + // Filter out anything that isn't an EntitySystem + if (!typeSymbol.AllInterfaces.Contains(entitySystemType)) + return; + + // Check each method definition in the class + symbolContext.RegisterOperationAction(AnalyzeMethod, OperationKind.MethodBody); + }, SymbolKind.NamedType); + }); + } + + private static void AnalyzeMethod(OperationAnalysisContext context) + { + if (context.Operation is not IMethodBodyOperation method) + return; + + if (method.BlockBody is null) + return; + + // If the class contains any sort of conditional directives, + // we consider it too complicated for automatic conversion. + var classSyntax = method.Syntax.Ancestors().OfType().First(); + if (classSyntax.ContainsDirective(SyntaxKind.IfDirectiveTrivia | SyntaxKind.ElseDirectiveTrivia | SyntaxKind.ElifDirectiveTrivia | SyntaxKind.EndIfDirectiveTrivia)) + return; + + // Examine each operation within the Initialize method body + foreach (var initOperation in method.BlockBody.ChildOperations) + { + // We only care about method invocations + if (initOperation is not IExpressionStatementOperation expression + || expression.Operation is not IInvocationOperation invocation) + continue; + + // Check if the invoked method is one of the SubscribeWhateverEvent methods + if (SubscribeMethods.Contains(invocation.TargetMethod.Name)) + { + // If any of the type arguments of the invocation is a type parameter (rather than a distinct Type), + // the attribute can't handle it, so we skip it. + // For example, RaiseLocalEvent(), where TTreeComp is a type arg to the containing class. + if (invocation.TargetMethod.TypeArguments.OfType().Any()) + continue; + + // Ignore anything that isn't a direct method reference, i.e. an anonymous delegate. + if (invocation.Arguments.SingleOrDefault(arg => arg.Parameter?.Name == "handler") is not { } handlerArg + || handlerArg.Value.Syntax is not IdentifierNameSyntax) + continue; + + // Get the symbol for the event handler method. + // We use OriginalDefinition to get the generic form if it's a generic method. + // So we get MyEventHandler instead of MyEventHandler. + if (((handlerArg.Value as IDelegateCreationOperation)?.Target as IMethodReferenceOperation)?.Method.OriginalDefinition is not { } handlerMethod) + continue; + + // If the target method is generic, we can't subscribe using the attribute. + if (handlerMethod.IsGenericMethod) + continue; + + // If the target method's event type is abstract, we can't subscribe using the attribute, + // since the subscription needs the exact type. + // This assumes that the event is the last parameter in the handler's signature, + // which seems like a reasonable assumption at the time of writing. + var handlerEventType = handlerMethod.Parameters.Last().Type; + if (handlerEventType.IsAbstract) + continue; + + // If the handler is a virtual or abstract method, we can't use the attribute + // since we would have to add it to the base class. + if (handlerMethod.IsVirtual || handlerMethod.IsAbstract) + continue; + + // Find the name of the attribute we need to use to replace the invocation and + // add it to the diagnostic so the code fixer can easily get it. + var attributeName = ToAttributeName(invocation.TargetMethod.Name); + var props = new Dictionary + { + { AttributeNameKey, attributeName } + }; + + // Flag this subscription as elligible for conversion + context.ReportDiagnostic(Diagnostic.Create( + EntitySystemSubscriptionConversionPossible, + invocation.Syntax.GetLocation(), + props.ToImmutableDictionary(), + invocation.TargetMethod.Name, + $"{attributeName}Attribute" + )); + } + } + } + + /// + /// Returns the name of the appropriate attribute to replace the given subscription method. + /// + public static string ToAttributeName(string methodName) + { + return methodName switch + { + SubscribeAllEventMethodName => SubscribeAllEventAttributeName, + _ => methodName + }; + } +} diff --git a/Robust.Analyzers/EntitySystemSubscriptionConversionFixer.cs b/Robust.Analyzers/EntitySystemSubscriptionConversionFixer.cs new file mode 100644 index 00000000000..28b9f02e0b6 --- /dev/null +++ b/Robust.Analyzers/EntitySystemSubscriptionConversionFixer.cs @@ -0,0 +1,237 @@ +#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 Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Simplification; +using static Robust.Roslyn.Shared.Diagnostics; + +namespace Robust.Analyzers; + +[ExportCodeFixProvider(LanguageNames.CSharp)] +public sealed class EntitySystemSubscriptionConversionFixer : CodeFixProvider +{ + private const string AttributeNamespace = "Robust.Shared.Analyzers"; + + public override ImmutableArray FixableDiagnosticIds => + [ + IdEntitySystemSubscriptionConversionPossible + ]; + + public override FixAllProvider GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + foreach (var diagnostic in context.Diagnostics) + { + switch (diagnostic.Id) + { + case IdEntitySystemSubscriptionConversionPossible: + return RegisterSubscriptionConversion(context, diagnostic); + } + } + + return Task.CompletedTask; + } + + private static async Task RegisterSubscriptionConversion(CodeFixContext context, Diagnostic diagnostic) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken); + + var span = diagnostic.Location.SourceSpan; + var invocationSyntax = root?.FindToken(span.Start).Parent?.AncestorsAndSelf().OfType().First(); + var classSyntax = invocationSyntax?.AncestorsAndSelf().OfType().First(); + + if (invocationSyntax is null || classSyntax is null) + return; + + // Get the name of the Attribute we need to add to the event handler method. + if (diagnostic.Properties[EntitySystemSubscriptionConversionAnalyzer.AttributeNameKey] is not string attributeName) + return; + + context.RegisterCodeFix(CodeAction.Create( + "Convert subscription to attribute", + c => ConvertSubscription(context.Document, invocationSyntax, classSyntax, attributeName, c), + "Convert subscription to attribute" + ), diagnostic); + } + + private static async Task ConvertSubscription( + Document document, + InvocationExpressionSyntax invocationSyntax, + ClassDeclarationSyntax classSyntax, + string attributeName, + CancellationToken c) + { + // Get the identifier of the event handler method. + if (invocationSyntax.ArgumentList.Arguments[0].Expression is not IdentifierNameSyntax handlerMethodIdentifer) + throw new InvalidOperationException($"Exception determining event handler method identifier for {invocationSyntax}"); + + var model = await document.GetSemanticModelAsync(c); + if (model.GetSymbolInfo(handlerMethodIdentifer, c).Symbol is not IMethodSymbol handlerMethodSymbol) + throw new InvalidOperationException($"Failed to find event handler method {handlerMethodIdentifer}"); + + if (model.GetDeclaredSymbol(classSyntax) is not { } classSymbol) + throw new InvalidOperationException($"Failed to find symbol for class {classSyntax.Identifier}"); + + if (model?.GetOperation(invocationSyntax) is not IInvocationOperation invocationOperation) + throw new InvalidOperationException($"Failed to find invocation operation"); + + var beforeTypes = GetTypesList(invocationOperation, "before"); + var afterTypes = GetTypesList(invocationOperation, "after"); + + // Create a SolutionEditor to edit multiple documents without worrying about immutability. + // The Initialize method might be in a different document than the handler, thanks to partial classes. + var editor = new SolutionEditor(document.Project.Solution); + + // Get an editor for the document containing the Initialize method. + var initializeEditor = await editor.GetDocumentEditorAsync(document.Id, c); + // Make our changes to the document containing the Initialize method. + ModifyInitialize(initializeEditor, invocationSyntax); + + // Find the ID for the document containing the event handler method. + var handlerDocId = editor.OriginalSolution.GetDocumentId(handlerMethodSymbol.DeclaringSyntaxReferences.First().SyntaxTree); + + // Get an editor for the document containing the event handler method. + // If the event handler is in the same document as the Initialize method, just reuse the same editor. + var handlerEditor = (handlerDocId == document.Id) ? initializeEditor : await editor.GetDocumentEditorAsync(handlerDocId, c); + // Make our changes to the document containing the event handler method. + ModifyHandler(handlerEditor, handlerMethodSymbol, attributeName, beforeTypes, afterTypes); + + // Make sure the class is marked as partial. + EnsureClassPartial(initializeEditor, classSymbol, classSyntax); + + // Return the modified solution. + return editor.GetChangedSolution(); + } + + /// + /// Edits the document containing the Intialize method. + /// Removes the SubscribeWhateverEvent method invocation. + /// + /// An editor for the document containing the Initialize method. + /// The SyntaxNode for the invocation of the Initialize method. + private static void ModifyInitialize( + DocumentEditor editor, + InvocationExpressionSyntax invocationSyntax) + { + // Remove the SubscribeWhateverEvent invocation from the Initialize method. + editor.RemoveNode(invocationSyntax.Parent!, SyntaxRemoveOptions.KeepUnbalancedDirectives); + } + + /// + /// Edits the document containing the event handler method. + /// Adds the SubscribeWhateverEventAttribute to the method. + /// + /// An editor for the document containing the event handler method. + /// The symbol for the event handler method. + /// The name of the Attribute to be added. + private static void ModifyHandler( + DocumentEditor editor, + IMethodSymbol handlerMethodSymbol, + string attributeName, + IEnumerable? beforeTypes, + IEnumerable? afterTypes + ) + { + // Get the syntax node for the event handler method. + var handlerMethodSyntax = handlerMethodSymbol.DeclaringSyntaxReferences.First().GetSyntax() as MethodDeclarationSyntax; + + // Generate an annotation containing the full name of the attribute we're adding. + // The magic string "SymbolId" makes this a SymbolAnnotation for Simplifier.AddImportsAnnotation to use. + var symbolAnnotation = new SyntaxAnnotation("SymbolId", $"{AttributeNamespace}.{attributeName}Attribute"); + + // Create the identifier for the attribute, annotating it with the full class name and AddImportsAnnotation. + // When Roslyn applies this code fix, AddImportsAnnotation tells it to add any missing using directives, + // but it needs the full name of the class to be able to do so. + var identifier = editor.Generator.IdentifierName(attributeName).WithAdditionalAnnotations(symbolAnnotation, Simplifier.AddImportsAnnotation); + + // Generate the SubscribeWhateverEvent attribute. + var attr = editor.Generator.Attribute(identifier); + + // Generate attribute argument syntax nodes for the before and after arguments. + var before = GenerateTypesArgument(beforeTypes, "before"); + var after = GenerateTypesArgument(afterTypes, "after"); + + // Remove either or both if they are null (meaning they weren't in the original invocation). + var args = new[]{before, after}.Where(arg => arg is not null); + + // If either or both are non-null, add them as arguments to the attribute. + // If both are null, we don't add anything otherwise we get empty parentheses on the attribute. + if (args.Any()) + attr = editor.Generator.AddAttributeArguments(attr, args!); + + // Add the attribute to the event handler method. + editor.AddAttribute(handlerMethodSyntax!, attr); + } + + /// + /// Marks the class as partial if it's not already. + /// + private static void EnsureClassPartial( + DocumentEditor editor, + INamedTypeSymbol classSymbol, + ClassDeclarationSyntax classSyntax) + { + // Use the current modifiers as a base. + var oldModifiers = DeclarationModifiers.From(classSymbol); + // Add the partial modifier if it's not already there. + editor.SetModifiers(classSyntax, oldModifiers.WithPartial(true)); + } + + /// + /// Extracts an enumerable containing the nodes passed to the named + /// parameter of an invocation. + /// + /// The method invocation the argument is being passed to. + /// The name of the parameter ("before" or "after") + /// + /// Thrown if the passed value is not a valid type of expression. + /// The passed value must be either a collection expression or an array literal. + /// + private static IEnumerable? GetTypesList(IInvocationOperation invocationOperation, string parameter) + { + // Get the operation representing the argument we're looking for. + var arg = invocationOperation.Arguments.Where(arg => arg.Parameter?.Name == parameter).SingleOrDefault(); + // If the argument is omitted, the operation will be a DefaultValueOperation. + if (arg.Value is IDefaultValueOperation or null) + return null; + // The way of getting the set of elements varies depending on the syntax that was used. + var expression = (arg.Syntax as ArgumentSyntax)?.Expression; + return expression switch + { + // SubscribeLocalEvent(MyMethod, before: [typeof(MyOtherSystem)]) + CollectionExpressionSyntax collection => collection.Elements.OfType().Select(e => e.Expression), + // SubscribeLocalEvent(MyMethod, before: new Type[] { typeof(MyOtherSystem) }) + ArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer?.Expressions ?? [], + // SubscribeLocalEvent(MyMethod, before: new[] { typeof(MyOtherSystem) }) + ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.Initializer.Expressions, + _ => throw new InvalidOperationException("Invalid types list") + }; + } + + /// + /// Returns a syntax node representing an attribute argument passing a collection expression of typeof expressions. + /// + /// The typeof expressions to populate the collection. + /// The name of the method parameter this argument is being passed to ("before" or "after"). + private static AttributeArgumentSyntax? GenerateTypesArgument(IEnumerable? types, string name) + { + if (types is null) + return null; + // Explicitly naming the parameters is much nicer for readability, especially with optional parameters. + var nameColon = SyntaxFactory.NameColon(name); + // Throw our list of typeof expressions into a collection expression. + var syntaxList = SyntaxFactory.SeparatedList(types.Select(SyntaxFactory.ExpressionElement)); + var collection = SyntaxFactory.CollectionExpression(syntaxList); + // Return the complete argument to be added to the attribute. + return SyntaxFactory.AttributeArgument(null, nameColon, collection); + } +} diff --git a/Robust.Roslyn.Shared/Diagnostics.cs b/Robust.Roslyn.Shared/Diagnostics.cs index 03b44dab2a5..711cee00725 100644 --- a/Robust.Roslyn.Shared/Diagnostics.cs +++ b/Robust.Roslyn.Shared/Diagnostics.cs @@ -61,6 +61,7 @@ public static class Diagnostics public const string IdInvalidContainingTypeForGeneratedSubscription = "RA0055"; public const string IdNonPartialContainingTypeForGeneratedSubscription = "RA0056"; public const string IdDataFieldOutsideDefinition = "RA0057"; + public const string IdEntitySystemSubscriptionConversionPossible = "RA0058"; public static SuppressionDescriptor MeansImplicitAssignment => new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");