Skip to content

Commit

Permalink
Analyzer to check that notnullableflag is being properly used (space-…
Browse files Browse the repository at this point in the history
  • Loading branch information
PaulRitter authored Dec 20, 2022
1 parent a0b067a commit bafbdb6
Show file tree
Hide file tree
Showing 39 changed files with 338 additions and 132 deletions.
2 changes: 1 addition & 1 deletion RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ END TEMPLATE-->

### New features

*None yet*
* Serv4's notNullableOverride parameter is now enforced by analyzer. For more info, see [the docs](https://docs.spacestation14.io/en/engine/serialization).

### Bugfixes

Expand Down
5 changes: 5 additions & 0 deletions Robust.Analyzers/Diagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ public static class Diagnostics
public const string IdUseGenericVariant = "RA0005";
public const string IdUseGenericVariantInvalidUsage = "RA0006";
public const string IdUseGenericVariantAttributeValueError = "RA0007";
public const string IdNotNullableFlagNotSet = "RA0008";
public const string IdInvalidNotNullableFlagValue = "RA0009";
public const string IdInvalidNotNullableFlagImplementation = "RA0010";
public const string IdInvalidNotNullableFlagType = "RA0011";
public const string IdNotNullableFlagValueType = "RA0012";

public static SuppressionDescriptor MeansImplicitAssignment =>
new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");
Expand Down
171 changes: 171 additions & 0 deletions Robust.Analyzers/NotNullableFlagAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Robust.Analyzers;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class NotNullableFlagAnalyzer : DiagnosticAnalyzer
{
private const string Attribute = "Robust.Shared.Analyzers.NotNullableFlagAttribute";

private static readonly DiagnosticDescriptor NotNullableNotSetRule = new (
Diagnostics.IdNotNullableFlagNotSet,
"Not Nullable Flag not set",
"Class type parameter {0} is not annotated as nullable and notNullableOverride is not set to true",
"Usage",
DiagnosticSeverity.Error,
true,
"Assign true to notNullableOverride or specify the type parameter as nullable.");

private static readonly DiagnosticDescriptor InvalidNotNullableValueRule = new (
Diagnostics.IdInvalidNotNullableFlagValue,
"Not Nullable Flag wrongfully set",
"Class type parameter {0} is annotated as nullable but notNullableOverride is set to true",
"Usage",
DiagnosticSeverity.Error,
true,
"Remove the true assignment to notNullableOverride or remove the nullable specifier of the type parameter.");

private static readonly DiagnosticDescriptor InvalidNotNullableImplementationRule = new (
Diagnostics.IdInvalidNotNullableFlagImplementation,
"Invalid NotNullable flag implementation.",
"NotNullable flag is either not typed as bool, or does not have a default value equaling false",
"Usage",
DiagnosticSeverity.Error,
true,
"Ensure that the notNullable flag is typed bool and has false set as a default value.");

private static readonly DiagnosticDescriptor InvalidNotNullableTypeRule = new (
Diagnostics.IdInvalidNotNullableFlagType,
"Failed to resolve type parameter",
"Failed to resolve type parameter \"{0}\".",
"Usage",
DiagnosticSeverity.Error,
true,
"Use nameof to avoid typos.");

private static readonly DiagnosticDescriptor NotNullableFlagValueTypeRule = new (
Diagnostics.IdNotNullableFlagValueType,
"NotNullable flag not supported for value types.",
"Value types as generic arguments are not supported for NotNullable flags",
"Usage",
DiagnosticSeverity.Error,
true,
"Nullable value types are distinct at runtime when inspected with reflection. Therefore they are not supported for NotNullable flags.");

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create(
NotNullableNotSetRule,
InvalidNotNullableValueRule,
InvalidNotNullableImplementationRule,
InvalidNotNullableTypeRule,
NotNullableFlagValueTypeRule);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();
context.RegisterOperationAction(CheckNotNullableFlag, OperationKind.Invocation);
}

private bool TryGetTypeArgument(IMethodSymbol methodSymbol, string typeParamName, out ITypeSymbol typeArgument)
{
for (var index = 0; index < methodSymbol.TypeParameters.Length; index++)
{
if (methodSymbol.TypeParameters[index].Name != typeParamName)
continue;

typeArgument = methodSymbol.TypeArguments[index];
return true;
}

typeArgument = null;
return false;
}

private void CheckNotNullableFlag(OperationAnalysisContext context)
{
if (context.Operation is not IInvocationOperation invocationOperation || !invocationOperation.TargetMethod.IsGenericMethod)
return;

var attribute = context.Compilation.GetTypeByMetadataName(Attribute);
var @bool = context.Compilation.GetSpecialType(SpecialType.System_Boolean);

foreach (var argument in invocationOperation.Arguments)
{
if(argument.Parameter == null) continue;

foreach (var attributeData in argument.Parameter.GetAttributes())
{
if (!SymbolEqualityComparer.Default.Equals(attributeData.AttributeClass, attribute))
continue;

if (!SymbolEqualityComparer.Default.Equals(argument.Parameter.Type, @bool) ||
!argument.Parameter.HasExplicitDefaultValue ||
argument.Parameter.ExplicitDefaultValue as bool? != false)
{
context.ReportDiagnostic(Diagnostic.Create(
InvalidNotNullableImplementationRule,
argument.Parameter.Locations[0]));
break;
}

if (!TryGetTypeArgument(invocationOperation.TargetMethod,
attributeData.ConstructorArguments[0].Value as string, out var typeArgument))
{
context.ReportDiagnostic(Diagnostic.Create(
InvalidNotNullableTypeRule,
argument.Parameter.Locations[0],
attributeData.ConstructorArguments[0].Value as string));
break;
}

//until i find a way to implement it sanely, generic calls are exempt from this attribute
if(typeArgument is ITypeParameterSymbol) break;

//dont ask me why, argument.ConstantValue just straight up doesnt work.
//i still kept it in here as a fallback, incase it ever starts working again lol -<paul
var constantValue = (argument.Value as ILiteralOperation)?.ConstantValue ?? argument.ConstantValue;

if (typeArgument.IsValueType)
{
if (argument.ArgumentKind != ArgumentKind.DefaultValue)
{
//todo diagnostic only use for struct types
context.ReportDiagnostic(Diagnostic.Create(
NotNullableFlagValueTypeRule,
argument.Syntax.GetLocation()));
}
break;
}

if (typeArgument.NullableAnnotation == NullableAnnotation.None ||
(argument.ArgumentKind != ArgumentKind.DefaultValue && !constantValue.HasValue))
break;

var flagValue = argument.ArgumentKind != ArgumentKind.DefaultValue ||
constantValue.Value as bool? == true;

var nullable = typeArgument.NullableAnnotation == NullableAnnotation.Annotated;

if (nullable && flagValue)
{
context.ReportDiagnostic(Diagnostic.Create(InvalidNotNullableValueRule,
argument.Syntax.GetLocation(),
typeArgument));
}
else if (!nullable && !flagValue)
{
context.ReportDiagnostic(Diagnostic.Create(NotNullableNotSetRule,
argument.Syntax.GetLocation(),
typeArgument));
}

break;
}
}
}
}
5 changes: 5 additions & 0 deletions Robust.Analyzers/Robust.Analyzers.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" Version="4.0.1" />
</ItemGroup>

<ItemGroup>
<!-- Needed for NotNullableFlagAnalyzer. -->
<Compile Include="..\Robust.Shared\Analyzers\NotNullableFlagAttribute.cs" />
</ItemGroup>

<ItemGroup>
<!-- Needed for FriendAnalyzer. -->
<Compile Include="..\Robust.Shared\Analyzers\AccessAttribute.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public SerializationCopyBenchmark()

var seedMapping = yamlStream.Documents[0].RootNode.ToDataNodeCast<SequenceDataNode>().Cast<MappingDataNode>(0);

Seed = SerializationManager.Read<SeedDataDefinition>(seedMapping);
Seed = SerializationManager.Read<SeedDataDefinition>(seedMapping, notNullableOverride: true);
}

private const string String = "ABC";
Expand All @@ -45,7 +45,7 @@ public SerializationCopyBenchmark()
[Benchmark]
public string? CreateCopyString()
{
return SerializationManager.CreateCopy(String);
return SerializationManager.CreateCopy(String, notNullableOverride: true);
}

[Benchmark]
Expand All @@ -57,13 +57,13 @@ public SerializationCopyBenchmark()
[Benchmark]
public DataDefinitionWithString? CreateCopyDataDefinitionWithString()
{
return SerializationManager.CreateCopy(DataDefinitionWithString);
return SerializationManager.CreateCopy(DataDefinitionWithString, notNullableOverride: true);
}

[Benchmark]
public SeedDataDefinition? CreateCopySeedDataDefinition()
{
return SerializationManager.CreateCopy(Seed);
return SerializationManager.CreateCopy(Seed, notNullableOverride: true);
}

[Benchmark]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public SerializationReadBenchmark()
[Benchmark]
public string ReadString()
{
return SerializationManager.Read<string>(StringNode);
return SerializationManager.Read<string>(StringNode, notNullableOverride: true);
}

[Benchmark]
Expand All @@ -55,13 +55,13 @@ public int ReadInteger()
[Benchmark]
public DataDefinitionWithString ReadDataDefinitionWithString()
{
return SerializationManager.Read<DataDefinitionWithString>(StringDataDefNode);
return SerializationManager.Read<DataDefinitionWithString>(StringDataDefNode, notNullableOverride: true);
}

[Benchmark]
public SeedDataDefinition ReadSeedDataDefinition()
{
return SerializationManager.Read<SeedDataDefinition>(SeedNode);
return SerializationManager.Read<SeedDataDefinition>(SeedNode, notNullableOverride: true);
}

[Benchmark]
Expand Down
24 changes: 12 additions & 12 deletions Robust.Benchmarks/Serialization/SerializationArrayBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,84 +46,84 @@ public SerializationArrayBenchmark()
[BenchmarkCategory("read")]
public string[]? ReadEmptyString()
{
return SerializationManager.Read<string[]>(EmptyNode);
return SerializationManager.Read<string[]>(EmptyNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public string[]? ReadOneString()
{
return SerializationManager.Read<string[]>(OneIntNode);
return SerializationManager.Read<string[]>(OneIntNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public string[]? ReadTenStrings()
{
return SerializationManager.Read<string[]>(TenIntsNode);
return SerializationManager.Read<string[]>(TenIntsNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public int[]? ReadEmptyInt()
{
return SerializationManager.Read<int[]>(EmptyNode);
return SerializationManager.Read<int[]>(EmptyNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public int[]? ReadOneInt()
{
return SerializationManager.Read<int[]>(OneIntNode);
return SerializationManager.Read<int[]>(OneIntNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public int[]? ReadTenInts()
{
return SerializationManager.Read<int[]>(TenIntsNode);
return SerializationManager.Read<int[]>(TenIntsNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public DataDefinitionWithString[]? ReadEmptyStringDataDef()
{
return SerializationManager.Read<DataDefinitionWithString[]>(EmptyNode);
return SerializationManager.Read<DataDefinitionWithString[]>(EmptyNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public DataDefinitionWithString[]? ReadOneStringDataDef()
{
return SerializationManager.Read<DataDefinitionWithString[]>(OneStringDefNode);
return SerializationManager.Read<DataDefinitionWithString[]>(OneStringDefNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public DataDefinitionWithString[]? ReadTenStringDataDefs()
{
return SerializationManager.Read<DataDefinitionWithString[]>(TenStringDefsNode);
return SerializationManager.Read<DataDefinitionWithString[]>(TenStringDefsNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public SealedDataDefinitionWithString[]? ReadEmptySealedStringDataDef()
{
return SerializationManager.Read<SealedDataDefinitionWithString[]>(EmptyNode);
return SerializationManager.Read<SealedDataDefinitionWithString[]>(EmptyNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public SealedDataDefinitionWithString[]? ReadOneSealedStringDataDef()
{
return SerializationManager.Read<SealedDataDefinitionWithString[]>(OneStringDefNode);
return SerializationManager.Read<SealedDataDefinitionWithString[]>(OneStringDefNode, notNullableOverride: true);
}

[Benchmark]
[BenchmarkCategory("read")]
public SealedDataDefinitionWithString[]? ReadTenSealedStringDataDefs()
{
return SerializationManager.Read<SealedDataDefinitionWithString[]>(TenStringDefsNode);
return SerializationManager.Read<SealedDataDefinitionWithString[]>(TenStringDefsNode, notNullableOverride: true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public SerializationWriteBenchmark()

var seedMapping = yamlStream.Documents[0].RootNode.ToDataNodeCast<SequenceDataNode>().Cast<MappingDataNode>(0);

Seed = SerializationManager.Read<SeedDataDefinition>(seedMapping);
Seed = SerializationManager.Read<SeedDataDefinition>(seedMapping, notNullableOverride: true);
}

private const string String = "ABC";
Expand All @@ -45,7 +45,7 @@ public SerializationWriteBenchmark()
[Benchmark]
public DataNode WriteString()
{
return SerializationManager.WriteValue(String);
return SerializationManager.WriteValue(String, notNullableOverride: true);
}

[Benchmark]
Expand All @@ -57,13 +57,13 @@ public DataNode WriteInteger()
[Benchmark]
public DataNode WriteDataDefinitionWithString()
{
return SerializationManager.WriteValue(DataDefinitionWithString);
return SerializationManager.WriteValue(DataDefinitionWithString, notNullableOverride: true);
}

[Benchmark]
public DataNode WriteSeedDataDefinition()
{
return SerializationManager.WriteValue(Seed);
return SerializationManager.WriteValue(Seed, notNullableOverride: true);
}

[Benchmark]
Expand Down
Loading

0 comments on commit bafbdb6

Please sign in to comment.