diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index c15430bbc..8cf8f2f75 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -44,6 +44,7 @@ END TEMPLATE--> * Control layout properties such as `Margin` can now be set via style sheets. * Distance between lines of a `RichTextLabel` can now be modified with `LineHeightScale`. * UI theme prototypes are now updated when reloaded. +* New `RA0025` analyzer diagnostic warns for manual assignment to `[Dependency]` fields. ### Bugfixes diff --git a/Robust.Analyzers.Tests/DependencyAssignAnalyzerTest.cs b/Robust.Analyzers.Tests/DependencyAssignAnalyzerTest.cs new file mode 100644 index 000000000..f8b696992 --- /dev/null +++ b/Robust.Analyzers.Tests/DependencyAssignAnalyzerTest.cs @@ -0,0 +1,65 @@ +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 DependencyAssignAnalyzerTest +{ + private const string BaseCode = """ + using System; + + namespace Robust.Shared.IoC; + + [AttributeUsage(AttributeTargets.Field)] + public sealed class DependencyAttribute : Attribute + { + } + """; + + private static Task Verifier(string code, params DiagnosticResult[] expected) + { + var test = new CSharpAnalyzerTest() + { + TestState = + { + AdditionalReferences = { typeof(DependencyAssignAnalyzer).Assembly }, + Sources = { code, BaseCode } + }, + }; + + // 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.IoC; + + public sealed class Foo + { + [Dependency] + private object? Field; + + public Foo() + { + Field = "A"; + } + } + """; + + await Verifier(code, + // /0/Test0.cs(10,9): warning RA0025: Tried to assign to [Dependency] field 'Field'. Remove [Dependency] or inject it via field injection instead. + VerifyCS.Diagnostic().WithSpan(10, 9, 10, 20).WithArguments("Field")); + } +} diff --git a/Robust.Analyzers/DependencyAssignAnalyzer.cs b/Robust.Analyzers/DependencyAssignAnalyzer.cs new file mode 100644 index 000000000..607a1e5d9 --- /dev/null +++ b/Robust.Analyzers/DependencyAssignAnalyzer.cs @@ -0,0 +1,61 @@ +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 DependencyAssignAnalyzer : DiagnosticAnalyzer +{ + private const string DependencyAttributeType = "Robust.Shared.IoC.DependencyAttribute"; + + private static readonly DiagnosticDescriptor Rule = new ( + Diagnostics.IdDependencyFieldAssigned, + "Assignment to dependency field", + "Tried to assign to [Dependency] field '{0}'. Remove [Dependency] or inject it via field injection instead.", + "Usage", + DiagnosticSeverity.Warning, + true); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterOperationAction(CheckAssignment, OperationKind.SimpleAssignment); + } + + private static void CheckAssignment(OperationAnalysisContext context) + { + if (context.Operation is not ISimpleAssignmentOperation assignment) + return; + + if (assignment.Target is not IFieldReferenceOperation fieldRef) + return; + + var field = fieldRef.Field; + var attributes = field.GetAttributes(); + if (attributes.Length == 0) + return; + + var depAttribute = context.Compilation.GetTypeByMetadataName(DependencyAttributeType); + if (!HasAttribute(attributes, depAttribute)) + return; + + context.ReportDiagnostic(Diagnostic.Create(Rule, assignment.Syntax.GetLocation(), field.Name)); + } + + private static bool HasAttribute(ImmutableArray attributes, ISymbol symbol) + { + foreach (var attribute in attributes) + { + if (SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, symbol)) + return true; + } + + return false; + } +} diff --git a/Robust.Roslyn.Shared/Diagnostics.cs b/Robust.Roslyn.Shared/Diagnostics.cs index b829a2f4c..94709a5f2 100644 --- a/Robust.Roslyn.Shared/Diagnostics.cs +++ b/Robust.Roslyn.Shared/Diagnostics.cs @@ -28,6 +28,7 @@ public static class Diagnostics public const string IdComponentPauseNoFields = "RA0022"; public const string IdComponentPauseNoParentAttribute = "RA0023"; public const string IdComponentPauseWrongTypeAttribute = "RA0024"; + public const string IdDependencyFieldAssigned = "RA0025"; public static SuppressionDescriptor MeansImplicitAssignment => new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");