Updated analyzers to handle generic replacements in redactions, and n…

…ot enforce nullability
dolittle · Oct 16, 2024 · 16d0c45 · 16d0c45
1 parent 699a1c9
commit 16d0c45
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 14 deletions.
diff --git 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
@@ -45,6 +45,7 @@ public static class DiagnosticIds
     public const string ExceptionInMutation = "SDK0010";
 
     public const string NonNullableRedactableField = "SDK0011";
+    public const string RedactableFieldIncorrectType = "SDK0012";
 
     /// <summary>
     /// Aggregate missing the required Attribute.

diff --git 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;
+
 
     /// <inheritdoc />
-    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [_rule];
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
+        [_nonNullableRule, _incorrectTypeRule];
 
     /// <inheritdoc />
     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
@@ -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<string>(""<removed>"")]
+    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<string>(""<removed>"")]
+    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<int>(-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<int>(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
@@ -48,9 +48,9 @@ public void WhenTypeHasRedactableProperties()
         RedactedType<RedactedEvent>.RedactedProperties
             .Should().BeEquivalentTo(new Dictionary<string, object?>
             {
-                { "SomeVal", -1 },
+                { "SomeVal", null },
                 { "SomeImportantPii", "<fjernet pga gdpr-forespørsel>" },
-                { "BirthDate", null },
+                { "BirthDate", -1 },
             });
     }