Add analyzers to detect some prototype misuse (#6048)

* Add analyzers to detect some prototype misuse

Detects people marking prototype as NetSerializable.

Detects people creating new prototype instances themselves.

* Update Robust.Analyzers/PrototypeNetSerializableAnalyzer.cs

Co-authored-by: Tayrtahn <tayrtahn@gmail.com>

---------

Co-authored-by: Tayrtahn <tayrtahn@gmail.com>
This commit is contained in:
Pieter-Jan Briers
2025-06-26 22:24:23 +02:00
committed by GitHub
parent 6436ff8040
commit c73b54862e
12 changed files with 320 additions and 8 deletions

View File

@@ -0,0 +1,64 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using NUnit.Framework;
using VerifyCS =
Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<Robust.Analyzers.PrototypeInstantiationAnalyzer, Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
namespace Robust.Analyzers.Tests;
[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)]
[TestFixture]
[TestOf(typeof(PrototypeInstantiationAnalyzer))]
public sealed class PrototypeInstantiationAnalyzerTest
{
private static Task Verifier(string code, params DiagnosticResult[] expected)
{
var test = new RTAnalyzerTest<PrototypeInstantiationAnalyzer>()
{
TestState =
{
Sources = { code }
},
};
TestHelper.AddEmbeddedSources(
test.TestState,
"Robust.Shared.Prototypes.Attributes.cs",
"Robust.Shared.Prototypes.IPrototype.cs",
"Robust.Shared.Serialization.Manager.Attributes.DataFieldAttribute.cs"
);
// 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.Serialization;
using Robust.Shared.Prototypes;
[Prototype]
public sealed class FooPrototype : IPrototype
{
[IdDataField]
public string ID { get; private set; } = default!;
}
public static class Bad
{
public static FooPrototype Real()
{
return new FooPrototype();
}
}
""";
await Verifier(code,
// /0/Test0.cs(15,16): warning RA0039: Do not instantiate prototypes directly. Prototypes should always be instantiated by the prototype manager.
VerifyCS.Diagnostic().WithSpan(15, 16, 15, 34));
}
}

View File

@@ -0,0 +1,61 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using NUnit.Framework;
using VerifyCS =
Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<Robust.Analyzers.PrototypeNetSerializableAnalyzer, Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
namespace Robust.Analyzers.Tests;
[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)]
[TestFixture]
[TestOf(typeof(PrototypeNetSerializableAnalyzer))]
public sealed class PrototypeNetSerializableAnalyzerTest
{
private static Task Verifier(string code, params DiagnosticResult[] expected)
{
var test = new RTAnalyzerTest<PrototypeNetSerializableAnalyzer>()
{
TestState =
{
Sources = { code }
},
};
TestHelper.AddEmbeddedSources(
test.TestState,
"Robust.Shared.Serialization.NetSerializableAttribute.cs",
"Robust.Shared.Prototypes.Attributes.cs",
"Robust.Shared.Prototypes.IPrototype.cs",
"Robust.Shared.Serialization.Manager.Attributes.DataFieldAttribute.cs"
);
// 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 System;
using Robust.Shared.Serialization;
using Robust.Shared.Prototypes;
[Prototype]
[Serializable, NetSerializable]
public sealed class FooPrototype : IPrototype
{
[IdDataField]
public string ID { get; private set; } = default!;
}
""";
await Verifier(code,
// /0/Test0.cs(7,21): warning RA0037: Type FooPrototype is a prototype and marked as [NetSerializable]. Prototypes should not be directly sent over the network, send their IDs instead.
VerifyCS.Diagnostic(PrototypeNetSerializableAnalyzer.RuleNetSerializable).WithSpan(7, 21, 7, 33).WithArguments("FooPrototype"),
// /0/Test0.cs(7,21): warning RA0038: Type FooPrototype is a prototype and marked as [Serializable]. Prototypes should not be directly sent over the network, send their IDs instead.
VerifyCS.Diagnostic(PrototypeNetSerializableAnalyzer.RuleSerializable).WithSpan(7, 21, 7, 33).WithArguments("FooPrototype"));
}
}

View File

@@ -0,0 +1,17 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Testing;
namespace Robust.Analyzers.Tests;
public sealed class RTAnalyzerTest<TAnalyzer> : CSharpAnalyzerTest<TAnalyzer, DefaultVerifier>
where TAnalyzer : DiagnosticAnalyzer, new()
{
protected override ParseOptions CreateParseOptions()
{
var baseOptions = (CSharpParseOptions) base.CreateParseOptions();
return baseOptions.WithPreprocessorSymbols("ROBUST_ANALYZERS_TEST");
}
}

View File

@@ -17,6 +17,10 @@
<EmbeddedResource Include="..\Robust.Shared\Analyzers\ObsoleteInheritanceAttribute.cs" LogicalName="Robust.Shared.Analyzers.ObsoleteInheritanceAttribute.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" />
<EmbeddedResource Include="..\Robust.Shared\Serialization\NetSerializableAttribute.cs" LogicalName="Robust.Shared.Serialization.NetSerializableAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Prototypes\Attributes.cs" LogicalName="Robust.Shared.Prototypes.Attributes.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Prototypes\IPrototype.cs" LogicalName="Robust.Shared.Prototypes.IPrototype.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Serialization\Manager\Attributes\DataFieldAttribute.cs" LogicalName="Robust.Shared.Serialization.Manager.Attributes.DataFieldAttribute.cs" LinkBase="Implementations" />
</ItemGroup>
<PropertyGroup>

View File

@@ -0,0 +1,48 @@
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 PrototypeInstantiationAnalyzer : DiagnosticAnalyzer
{
private const string PrototypeInterfaceType = "Robust.Shared.Prototypes.IPrototype";
public static readonly DiagnosticDescriptor Rule = new(
Diagnostics.IdPrototypeInstantiation,
"Do not instantiate prototypes directly",
"Do not instantiate prototypes directly. Prototypes should always be instantiated by the prototype manager.",
"Usage",
DiagnosticSeverity.Warning,
true);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule];
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(static ctx =>
{
var prototypeInterface = ctx.Compilation.GetTypeByMetadataName(PrototypeInterfaceType);
if (prototypeInterface == null)
return;
ctx.RegisterOperationAction(symContext => Check(prototypeInterface, symContext), OperationKind.ObjectCreation);
});
}
private static void Check(INamedTypeSymbol prototypeInterface, OperationAnalysisContext ctx)
{
if (ctx.Operation is not IObjectCreationOperation { Type: { } resultType } creationOp)
return;
if (!TypeSymbolHelper.ImplementsInterface(resultType, prototypeInterface))
return;
ctx.ReportDiagnostic(Diagnostic.Create(Rule, creationOp.Syntax.GetLocation()));
}
}

View File

@@ -0,0 +1,76 @@
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Robust.Roslyn.Shared;
namespace Robust.Analyzers;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class PrototypeNetSerializableAnalyzer : DiagnosticAnalyzer
{
private const string PrototypeInterfaceType = "Robust.Shared.Prototypes.IPrototype";
private const string NetSerializableAttributeType = "Robust.Shared.Serialization.NetSerializableAttribute";
public static readonly DiagnosticDescriptor RuleNetSerializable = new(
Diagnostics.IdPrototypeNetSerializable,
"Prototypes should not be [NetSerializable]",
"Type {0} is a prototype and marked as [NetSerializable]. Prototypes should not be directly sent over the network, send their IDs instead.",
"Usage",
DiagnosticSeverity.Warning,
true);
public static readonly DiagnosticDescriptor RuleSerializable = new(
Diagnostics.IdPrototypeSerializable,
"Prototypes should not be [Serializable]",
"Type {0} is a prototype and marked as [Serializable]. Prototypes should not be directly sent over the network, send their IDs instead.",
"Usage",
DiagnosticSeverity.Warning,
true);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [
RuleNetSerializable,
RuleSerializable
];
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(static ctx =>
{
var prototypeInterface = ctx.Compilation.GetTypeByMetadataName(PrototypeInterfaceType);
var netSerializableAttribute = ctx.Compilation.GetTypeByMetadataName(NetSerializableAttributeType);
if (prototypeInterface == null || netSerializableAttribute == null)
return;
ctx.RegisterSymbolAction(symbolContext => CheckClass(prototypeInterface, netSerializableAttribute, symbolContext), SymbolKind.NamedType);
});
}
private static void CheckClass(
INamedTypeSymbol prototypeInterface,
INamedTypeSymbol netSerializableAttribute,
SymbolAnalysisContext symbolContext)
{
if (symbolContext.Symbol is not INamedTypeSymbol symbol)
return;
if (!TypeSymbolHelper.ImplementsInterface(symbol, prototypeInterface))
return;
if (AttributeHelper.HasAttribute(symbol, netSerializableAttribute, out _))
{
symbolContext.ReportDiagnostic(
Diagnostic.Create(RuleNetSerializable, symbol.Locations[0], symbol.ToDisplayString()));
}
if (symbol.IsSerializable)
{
symbolContext.ReportDiagnostic(
Diagnostic.Create(RuleSerializable, symbol.Locations[0], symbol.ToDisplayString()));
}
}
}

View File

@@ -43,4 +43,22 @@ public static class AttributeHelper
return defaultValue;
}
public static bool HasAttribute(
INamedTypeSymbol symbol,
INamedTypeSymbol attribute,
[NotNullWhen(true)] out AttributeData? matchedAttribute)
{
matchedAttribute = null;
foreach (var typeAttribute in symbol.GetAttributes())
{
if (SymbolEqualityComparer.Default.Equals(typeAttribute.AttributeClass, attribute))
{
matchedAttribute = typeAttribute;
return true;
}
}
return false;
}
}

View File

@@ -40,6 +40,9 @@ public static class Diagnostics
public const string IdObsoleteInheritance = "RA0034";
public const string IdObsoleteInheritanceWithMessage = "RA0035";
public const string IdDataFieldYamlSerializable = "RA0036";
public const string IdPrototypeNetSerializable = "RA0037";
public const string IdPrototypeSerializable = "RA0038";
public const string IdPrototypeInstantiation = "RA0039";
public static SuppressionDescriptor MeansImplicitAssignment =>
new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");

View File

@@ -6,7 +6,7 @@ namespace Robust.Roslyn.Shared;
public static class TypeSymbolHelper
{
public static bool ShittyTypeMatch(INamedTypeSymbol type, string attributeMetadataName)
public static bool ShittyTypeMatch(ITypeSymbol type, string attributeMetadataName)
{
// Doing it like this only allocates when the type actually matches, which is good enough for me right now.
if (!attributeMetadataName.EndsWith(type.Name))
@@ -15,7 +15,7 @@ public static class TypeSymbolHelper
return type.ToDisplayString() == attributeMetadataName;
}
public static bool ImplementsInterface(INamedTypeSymbol type, string interfaceTypeName)
public static bool ImplementsInterface(ITypeSymbol type, string interfaceTypeName)
{
foreach (var interfaceType in type.AllInterfaces)
{
@@ -25,4 +25,15 @@ public static class TypeSymbolHelper
return false;
}
public static bool ImplementsInterface(ITypeSymbol type, INamedTypeSymbol interfaceType)
{
foreach (var @interface in type.AllInterfaces)
{
if (SymbolEqualityComparer.Default.Equals(@interface, interfaceType))
return true;
}
return false;
}
}

View File

@@ -1,6 +1,8 @@
using System;
#if !ROBUST_ANALYZERS_TEST
using JetBrains.Annotations;
using Robust.Shared.Serialization.Manager.Attributes;
#endif
namespace Robust.Shared.Prototypes;
@@ -9,10 +11,12 @@ namespace Robust.Shared.Prototypes;
/// To prevent needing to instantiate it because interfaces can't declare statics.
/// </summary>
[AttributeUsage(AttributeTargets.Class, Inherited = false)]
#if !ROBUST_ANALYZERS_TEST
[BaseTypeRequired(typeof(IPrototype))]
[MeansImplicitUse]
[MeansDataDefinition]
[Virtual]
#endif
public class PrototypeAttribute : Attribute
{
/// <summary>
@@ -35,10 +39,12 @@ public class PrototypeAttribute : Attribute
}
[AttributeUsage(AttributeTargets.Class, Inherited = false)]
#if !ROBUST_ANALYZERS_TEST
[BaseTypeRequired(typeof(IPrototype))]
[MeansImplicitUse]
[MeansDataDefinition]
[MeansDataRecord]
#endif
public sealed class PrototypeRecordAttribute : PrototypeAttribute
{
public PrototypeRecordAttribute(string type, int loadPriority = 1) : base(type, loadPriority)

View File

@@ -1,11 +1,8 @@
using System;
using JetBrains.Annotations;
using Robust.Shared.Serialization.Manager.Attributes;
using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototype;
using Robust.Shared.Serialization.TypeSerializers.Interfaces;
#if !ROBUST_ANALYZERS_TEST
using Robust.Shared.ViewVariables;
using YamlDotNet.Core.Tokens;
using YamlDotNet.RepresentationModel;
#endif
namespace Robust.Shared.Prototypes
{
@@ -22,7 +19,10 @@ namespace Robust.Shared.Prototypes
/// An ID for this prototype instance.
/// If this is a duplicate, an error will be thrown.
/// </summary>
[ViewVariables(VVAccess.ReadOnly)] string ID { get; }
#if !ROBUST_ANALYZERS_TEST
[ViewVariables(VVAccess.ReadOnly)]
#endif
string ID { get; }
}
public interface IInheritingPrototype

View File

@@ -1,12 +1,16 @@
using System;
#if !ROBUST_ANALYZERS_TEST
using JetBrains.Annotations;
#endif
namespace Robust.Shared.Serialization.Manager.Attributes
{
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
#if !ROBUST_ANALYZERS_TEST
[MeansImplicitAssignment]
[MeansImplicitUse(ImplicitUseKindFlags.Assign)]
[Virtual]
#endif
public class DataFieldAttribute : DataFieldBaseAttribute
{
/// <summary>