From 16d0c45318d068d0ad1e50ec483ecbc1289f0377 Mon Sep 17 00:00:00 2001 From: Magne Helleborg Date: Wed, 16 Oct 2024 14:16:10 +0200 Subject: [PATCH] Updated analyzers to handle generic replacements in redactions, and not enforce nullability --- Source/Analyzers/DescriptorRules.cs | 10 +++ Source/Analyzers/DiagnosticIds.cs | 1 + .../Analyzers/RedactablePropertyAnalyzer.cs | 43 ++++++++--- .../RedactablePropertyAnalyzerTests.cs | 73 +++++++++++++++++++ Tests/Events/Redaction/RedactionTests.cs | 4 +- 5 files changed, 117 insertions(+), 14 deletions(-) diff --git a/Source/Analyzers/DescriptorRules.cs b/Source/Analyzers/DescriptorRules.cs index 73ed984d..474d748d 100644 --- a/Source/Analyzers/DescriptorRules.cs +++ b/Source/Analyzers/DescriptorRules.cs @@ -97,6 +97,16 @@ static class DescriptorRules DiagnosticSeverity.Warning, isEnabledByDefault: true, description: "The field should be nullable since the value can be deleted."); + + internal static readonly DiagnosticDescriptor IncorrectRedactableFieldType = + new( + DiagnosticIds.RedactableFieldIncorrectType, + title: "Generic Type must match the property type", + messageFormat: "The generic type for RedactablePersonalDataAttribute was {0}, must match the property type {1}", + DiagnosticCategories.Sdk, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "The generic type must match the property type."); internal static class Events diff --git a/Source/Analyzers/DiagnosticIds.cs b/Source/Analyzers/DiagnosticIds.cs index 79c412a7..302de6e1 100644 --- a/Source/Analyzers/DiagnosticIds.cs +++ b/Source/Analyzers/DiagnosticIds.cs @@ -45,6 +45,7 @@ public static class DiagnosticIds public const string ExceptionInMutation = "SDK0010"; public const string NonNullableRedactableField = "SDK0011"; + public const string RedactableFieldIncorrectType = "SDK0012"; /// /// Aggregate missing the required Attribute. diff --git a/Source/Analyzers/RedactablePropertyAnalyzer.cs b/Source/Analyzers/RedactablePropertyAnalyzer.cs index 8000109f..85988cf1 100644 --- a/Source/Analyzers/RedactablePropertyAnalyzer.cs +++ b/Source/Analyzers/RedactablePropertyAnalyzer.cs @@ -16,10 +16,13 @@ namespace Dolittle.SDK.Analyzers; [DiagnosticAnalyzer(LanguageNames.CSharp)] public class RedactablePropertyAnalyzer : DiagnosticAnalyzer { - static readonly DiagnosticDescriptor _rule = DescriptorRules.NonNullableRedactableField; + static readonly DiagnosticDescriptor _nonNullableRule = DescriptorRules.NonNullableRedactableField; + static readonly DiagnosticDescriptor _incorrectTypeRule = DescriptorRules.IncorrectRedactableFieldType; + /// - public override ImmutableArray SupportedDiagnostics { get; } = [_rule]; + public override ImmutableArray SupportedDiagnostics { get; } = + [_nonNullableRule, _incorrectTypeRule]; /// public override void Initialize(AnalysisContext context) @@ -38,21 +41,37 @@ private static void AnalyzeProperty(SyntaxNodeAnalysisContext context) if (propertySymbol == null) return; - var hasAttribute = propertySymbol.GetAttributes() - .Any(attr => attr.AttributeClass?.Name == "RedactablePersonalDataAttribute"); + var personalDataAttribute = propertySymbol.GetAttributes() + .FirstOrDefault(attr => attr.AttributeClass?.Name == "RedactablePersonalDataAttribute"); + + if (personalDataAttribute == null) return; - if (!hasAttribute) return; + if (personalDataAttribute.AttributeClass is { IsGenericType: true } namedTypeSymbol) + { + // If the RedactablePersonalDataAttribute is generic, the type parameter must match the property type + var typeArgument = namedTypeSymbol.TypeArguments.First(); + if (!typeArgument.Equals(propertySymbol.Type, SymbolEqualityComparer.Default)) + { + context.ReportDiagnostic(Diagnostic.Create(_incorrectTypeRule, personalDataAttribute.ApplicationSyntaxReference?.GetSyntax().GetLocation() ?? propertyDeclaration.GetLocation(), + typeArgument.Name, propertySymbol.Type.Name)); + } - if (propertySymbol.Type.NullableAnnotation != NullableAnnotation.Annotated) + } + else { - context.ReportDiagnostic(Diagnostic.Create(_rule, propertyDeclaration.GetLocation(), propertySymbol.Name)); + // If it the non + if (propertySymbol.Type.NullableAnnotation != NullableAnnotation.Annotated) + { + context.ReportDiagnostic(Diagnostic.Create(_nonNullableRule, propertyDeclaration.GetLocation(), + propertySymbol.Name)); + } } } - + private static void AnalyzeRecordDeclaration(SyntaxNodeAnalysisContext context) { var recordDeclaration = (RecordDeclarationSyntax)context.Node; - + // Check if it's a record with a primary constructor if (recordDeclaration.ParameterList == null) return; @@ -65,13 +84,13 @@ private static void AnalyzeRecordDeclaration(SyntaxNodeAnalysisContext context) { continue; } - + var parameterSymbol = context.SemanticModel.GetDeclaredSymbol(parameter); if (!IsNullable(parameterSymbol)) { - context.ReportDiagnostic(Diagnostic.Create(_rule, parameter.GetLocation(), parameterSymbol.Name)); + context.ReportDiagnostic(Diagnostic.Create(_nonNullableRule, parameter.GetLocation(), + parameterSymbol.Name)); } - } } diff --git a/Tests/Analyzers/Diagnostics/RedactablePropertyAnalyzerTests.cs b/Tests/Analyzers/Diagnostics/RedactablePropertyAnalyzerTests.cs index 4a0a9953..18f887ab 100644 --- a/Tests/Analyzers/Diagnostics/RedactablePropertyAnalyzerTests.cs +++ b/Tests/Analyzers/Diagnostics/RedactablePropertyAnalyzerTests.cs @@ -41,6 +41,60 @@ class SomeEvent await VerifyAnalyzerFindsNothingAsync(code); } + [Fact] + public async Task OnPropertyWithReplacedRedactableStringAttribute() + { + var code = @" +using Dolittle.SDK.Events; +using Dolittle.SDK.Events.Redaction; + +[EventType(""44a755a7-e755-4076-bad4-fbc6ec3c0ea5"")] +class SomeEvent +{ + [RedactablePersonalData("""")] + public string SomeProperty { get; set; } +} +"; + + await VerifyAnalyzerFindsNothingAsync(code); + } + + [Fact] + public async Task OnPropertyWithReplacedRedactableStringAttributeWithDifferentNullability() + { + var code = @" +using Dolittle.SDK.Events; +using Dolittle.SDK.Events.Redaction; + +[EventType(""44a755a7-e755-4076-bad4-fbc6ec3c0ea5"")] +class SomeEvent +{ + [RedactablePersonalData("""")] + public string? SomeProperty { get; set; } +} +"; + + await VerifyAnalyzerFindsNothingAsync(code); + } + + [Fact] + public async Task OnPropertyWithReplacedRedactableIntAttribute() + { + var code = @" +using Dolittle.SDK.Events; +using Dolittle.SDK.Events.Redaction; + +[EventType(""44a755a7-e755-4076-bad4-fbc6ec3c0ea5"")] +class SomeEvent +{ + [RedactablePersonalData(-1)] + public int SomeProperty { get; set; } +} +"; + + await VerifyAnalyzerFindsNothingAsync(code); + } + [Fact] public async Task OnPropertyWithNonNullableRedactableAttribute() { @@ -60,6 +114,25 @@ await VerifyAnalyzerAsync(code, .WithSpan(8, 5, 9, 45).WithArguments("SomeProperty")); } + [Fact] + public async Task OnPropertyWithIntAttributeOnString() + { + var code = @" +using Dolittle.SDK.Events; +using Dolittle.SDK.Events.Redaction; + +[EventType(""44a755a7-e755-4076-bad4-fbc6ec3c0ea5"")] +class SomeEvent +{ + [RedactablePersonalData(123)] + public string SomeProperty { get; set; } +} +"; + await VerifyAnalyzerAsync(code, + Diagnostic(DescriptorRules.IncorrectRedactableFieldType) + .WithSpan(8, 6, 8, 38).WithMessage("The generic type for RedactablePersonalDataAttribute was Int32, must match the property type String")); + } + [Fact] public async Task OnRecordPropertyWithNonNullableRedactableAttribute() { diff --git a/Tests/Events/Redaction/RedactionTests.cs b/Tests/Events/Redaction/RedactionTests.cs index 408c93aa..deb3a68e 100644 --- a/Tests/Events/Redaction/RedactionTests.cs +++ b/Tests/Events/Redaction/RedactionTests.cs @@ -48,9 +48,9 @@ public void WhenTypeHasRedactableProperties() RedactedType.RedactedProperties .Should().BeEquivalentTo(new Dictionary { - { "SomeVal", -1 }, + { "SomeVal", null }, { "SomeImportantPii", "" }, - { "BirthDate", null }, + { "BirthDate", -1 }, }); }