Add ForbidLiteralAttribute and analyzer (#5808)

* Add ForbidLiteral attribute, analyzer, and test

* Removed unused code

* Switch order of methods. It's better this way.
This commit is contained in:
Tayrtahn
2025-04-08 10:39:38 -04:00
committed by GitHub
parent 4a24539629
commit 45bb8740a0
5 changed files with 303 additions and 0 deletions

View File

@@ -0,0 +1,189 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Testing;
using NUnit.Framework;
using VerifyCS =
Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<Robust.Analyzers.ForbidLiteralAnalyzer, Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
namespace Robust.Analyzers.Tests;
[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)]
[TestFixture]
public sealed class ForbidLiteralAnalyzerTest
{
private static Task Verifier(string code, params DiagnosticResult[] expected)
{
var test = new CSharpAnalyzerTest<ForbidLiteralAnalyzer, DefaultVerifier>()
{
TestState =
{
Sources = { code },
},
};
TestHelper.AddEmbeddedSources(
test.TestState,
"Robust.Shared.Analyzers.ForbidLiteralAttribute.cs"
);
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 System.Collections.Generic;
using Robust.Shared.Analyzers;
public sealed class TestClass
{
public static void OneParameterForbidden([ForbidLiteral] string value) { }
public static void TwoParametersFirstForbidden([ForbidLiteral] string first, string second) { }
public static void TwoParametersBothForbidden([ForbidLiteral] string first, [ForbidLiteral] string second) { }
public static void ListParameterForbidden([ForbidLiteral] List<string> values) { }
public static void ParamsListParameterForbidden([ForbidLiteral] params List<string> values) { }
}
public record struct StringWrapper(string value)
{
private readonly string _value = value;
public static implicit operator string(StringWrapper wrapper)
{
return wrapper._value;
}
}
""";
[Test]
public async Task TestOneParameter()
{
const string code = """
public sealed class Tester
{
private const string _constValue = "foo";
private static readonly string StaticValue = "bar";
private static readonly StringWrapper WrappedValue = new("biz");
public void Test()
{
TestClass.OneParameterForbidden(_constValue);
TestClass.OneParameterForbidden(StaticValue);
TestClass.OneParameterForbidden(WrappedValue);
TestClass.OneParameterForbidden("baz");
}
}
""";
await Verifier(code,
// /0/Test0.cs(12,41): error RA0033: The "value" parameter of OneParameterForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(12, 41, 12, 46).WithArguments("value", "OneParameterForbidden")
);
}
[Test]
public async Task TestTwoParametersFirstForbidden()
{
const string code = """
public sealed class Tester
{
private const string _constValue = "foo";
public void Test()
{
TestClass.TwoParametersFirstForbidden(_constValue, "whatever");
TestClass.TwoParametersFirstForbidden(_constValue, _constValue);
TestClass.TwoParametersFirstForbidden("foo", "whatever");
}
}
""";
await Verifier(code,
// /0/Test0.cs(9,47): error RA0033: The "first" parameter of TwoParametersFirstForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(9, 47, 9, 52).WithArguments("first", "TwoParametersFirstForbidden")
);
}
[Test]
public async Task TestTwoParametersBothForbidden()
{
const string code = """
public sealed class Tester
{
private const string _constValue = "foo";
private static readonly string StaticValue = "bar";
public void Test()
{
TestClass.TwoParametersBothForbidden(_constValue, _constValue);
TestClass.TwoParametersBothForbidden(_constValue, StaticValue);
TestClass.TwoParametersBothForbidden(_constValue, "whatever");
TestClass.TwoParametersBothForbidden("whatever", _constValue);
}
}
""";
await Verifier(code,
// /0/Test0.cs(10,59): error RA0033: The "second" parameter of TwoParametersBothForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(10, 59, 10, 69).WithArguments("second", "TwoParametersBothForbidden"),
// /0/Test0.cs(11,46): error RA0033: The "first" parameter of TwoParametersBothForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(11, 46, 11, 56).WithArguments("first", "TwoParametersBothForbidden")
);
}
[Test]
public async Task TestListParameter()
{
const string code = """
public sealed class Tester
{
private const string _constValue = "foo";
private static readonly string StaticValue = "bar";
private static readonly StringWrapper WrappedValue = new("biz");
public void Test()
{
TestClass.ListParameterForbidden([_constValue, StaticValue, WrappedValue]);
TestClass.ListParameterForbidden(["foo", _constValue, "bar"]);
}
}
""";
await Verifier(code,
// /0/Test0.cs(10,43): warning RA0033: The "values" parameter of ListParameterForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(10, 43, 10, 48).WithArguments("values", "ListParameterForbidden"),
// /0/Test0.cs(10,63): warning RA0033: The "values" parameter of ListParameterForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(10, 63, 10, 68).WithArguments("values", "ListParameterForbidden")
);
}
[Test]
public async Task TestParamsListParameter()
{
const string code = """
public sealed class Tester
{
private const string _constValue = "foo";
private static readonly string StaticValue = "bar";
private static readonly StringWrapper WrappedValue = new("biz");
public void Test()
{
TestClass.ParamsListParameterForbidden(_constValue, StaticValue, WrappedValue);
TestClass.ParamsListParameterForbidden("foo", _constValue, "bar");
}
}
""";
await Verifier(code,
// /0/Test0.cs(10,48): warning RA0033: The "values" parameter of ParamsListParameterForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(10, 48, 10, 53).WithArguments("values", "ParamsListParameterForbidden"),
// /0/Test0.cs(10,68): warning RA0033: The "values" parameter of ParamsListParameterForbidden forbids literal values
VerifyCS.Diagnostic().WithSpan(10, 68, 10, 73).WithArguments("values", "ParamsListParameterForbidden")
);
}
}

View File

@@ -13,6 +13,7 @@
<EmbeddedResource Include="..\Robust.Shared\Analyzers\MustCallBaseAttribute.cs" LogicalName="Robust.Shared.IoC.MustCallBaseAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Analyzers\PreferNonGenericVariantForAttribute.cs" LogicalName="Robust.Shared.Analyzers.PreferNonGenericVariantForAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Analyzers\PreferOtherTypeAttribute.cs" LogicalName="Robust.Shared.Analyzers.PreferOtherTypeAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Analyzers\ForbidLiteralAttribute.cs" LogicalName="Robust.Shared.Analyzers.ForbidLiteralAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\IoC\DependencyAttribute.cs" LogicalName="Robust.Shared.IoC.DependencyAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\GameObjects\EventBusAttributes.cs" LogicalName="Robust.Shared.GameObjects.EventBusAttributes.cs" LinkBase="Implementations" />
</ItemGroup>

View File

@@ -0,0 +1,101 @@
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 ForbidLiteralAnalyzer : DiagnosticAnalyzer
{
private const string ForbidLiteralType = "Robust.Shared.Analyzers.ForbidLiteralAttribute";
public static DiagnosticDescriptor ForbidLiteralRule = new(
Diagnostics.IdForbidLiteral,
"Parameter forbids literal values",
"The {0} parameter of {1} forbids literal values",
"Usage",
DiagnosticSeverity.Warning,
true,
"Pass in a validated wrapper type like ProtoId, or a const or static value."
);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [ForbidLiteralRule];
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();
context.RegisterOperationAction(AnalyzeOperation, OperationKind.Invocation);
}
private void AnalyzeOperation(OperationAnalysisContext context)
{
if (context.Operation is not IInvocationOperation invocationOperation)
return;
// Check each parameter of the method invocation
foreach (var argumentOperation in invocationOperation.Arguments)
{
// Check for our attribute on the parameter
if (!AttributeHelper.HasAttribute(argumentOperation.Parameter, ForbidLiteralType, out _))
continue;
// Handle parameters using the params keyword
if (argumentOperation.Syntax is InvocationExpressionSyntax subExpressionSyntax)
{
// Check each param value
foreach (var subArgument in subExpressionSyntax.ArgumentList.Arguments)
{
CheckArgumentSyntax(context, argumentOperation, subArgument);
}
continue;
}
// Not params, so just check the single parameter
if (argumentOperation.Syntax is not ArgumentSyntax argumentSyntax)
continue;
CheckArgumentSyntax(context, argumentOperation, argumentSyntax);
}
}
private void CheckArgumentSyntax(OperationAnalysisContext context, IArgumentOperation operation, ArgumentSyntax argumentSyntax)
{
// Handle collection types
if (argumentSyntax.Expression is CollectionExpressionSyntax collectionExpressionSyntax)
{
// Check each value of the collection
foreach (var elementSyntax in collectionExpressionSyntax.Elements)
{
if (elementSyntax is not ExpressionElementSyntax expressionSyntax)
continue;
// Check if a literal was passed in
if (expressionSyntax.Expression is not LiteralExpressionSyntax)
continue;
context.ReportDiagnostic(Diagnostic.Create(ForbidLiteralRule,
expressionSyntax.GetLocation(),
operation.Parameter.Name,
(context.Operation as IInvocationOperation).TargetMethod.Name
));
}
return;
}
// Not a collection, just a single value to check
// Check if it's a literal
if (argumentSyntax.Expression is not LiteralExpressionSyntax)
return;
context.ReportDiagnostic(Diagnostic.Create(ForbidLiteralRule,
argumentSyntax.GetLocation(),
operation.Parameter.Name,
(context.Operation as IInvocationOperation).TargetMethod.Name
));
}
}

View File

@@ -36,6 +36,7 @@ public static class Diagnostics
public const string IdUseNonGenericVariant = "RA0030";
public const string IdPreferOtherType = "RA0031";
public const string IdDuplicateDependency = "RA0032";
public const string IdForbidLiteral = "RA0033";
public static SuppressionDescriptor MeansImplicitAssignment =>
new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");

View File

@@ -0,0 +1,11 @@
using System;
namespace Robust.Shared.Analyzers;
/// <summary>
/// Marks that values used for this parameter should not be literal values.
/// This helps prevent magic numbers/strings/etc, by indicating that values
/// should either be wrapped (for validation) or defined as constants or readonly statics.
/// </summary>
[AttributeUsage(AttributeTargets.Parameter)]
public sealed class ForbidLiteralAttribute : Attribute;