Skip to content

Commit

Permalink
EventHandler improvements (#232)
Browse files Browse the repository at this point in the history
* StartFrom / StopAt feature for event handlers

* Mutation analyzer for public aggregate methods. Checks for incorrect mutations outside of using events

* EventHandler analyzers. Includes checks for visibility, date parsing, date logic & EventContext fix for Handlers
  • Loading branch information
mhelleborg authored Sep 18, 2023
1 parent c0865a2 commit c1b2d40
Show file tree
Hide file tree
Showing 21 changed files with 1,059 additions and 41 deletions.
70 changes: 64 additions & 6 deletions Source/Analyzers/AggregateAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public class AggregateAnalyzer : DiagnosticAnalyzer
DescriptorRules.Aggregate.MutationShouldBePrivate,
DescriptorRules.Aggregate.MutationHasIncorrectNumberOfParameters,
DescriptorRules.Aggregate.MutationsCannotProduceEvents,
DescriptorRules.Events.MissingAttribute
DescriptorRules.Events.MissingAttribute,
DescriptorRules.Aggregate.PublicMethodsCannotMutateAggregateState
);

/// <inheritdoc />
Expand All @@ -53,7 +54,8 @@ static void AnalyzeAggregates(SyntaxNodeAnalysisContext context)

var handledEvents = CheckOnMethods(context, aggregateSymbol);
CheckApplyInvocations(context, aggregateSyntax, handledEvents);
CheckApplyInvocationsInOnMethods(context, aggregateSyntax, aggregateSymbol);
CheckApplyInvocationsInOnMethods(context, aggregateSymbol);
CheckMutationsInPublicMethods(context, aggregateSymbol);
}


Expand Down Expand Up @@ -144,10 +146,8 @@ static void CheckApplyInvocations(SyntaxNodeAnalysisContext context, ClassDeclar
}
}

static void CheckApplyInvocationsInOnMethods(SyntaxNodeAnalysisContext context, ClassDeclarationSyntax aggregateClassSyntax, INamedTypeSymbol aggregateType)
static void CheckApplyInvocationsInOnMethods(SyntaxNodeAnalysisContext context, INamedTypeSymbol aggregateType)
{
var semanticModel = context.SemanticModel;

var onMethods = aggregateType
.GetMembers()
.Where(member => member.Name.Equals("On"))
Expand Down Expand Up @@ -194,4 +194,62 @@ static void CheckApplyInvocationsInOnMethods(SyntaxNodeAnalysisContext context,
}
}
}
}

static void CheckMutationsInPublicMethods(SyntaxNodeAnalysisContext context, INamedTypeSymbol aggregateType)
{
var publicMethods = aggregateType
.GetMembers()
.Where(member => !member.Name.Equals("On"))
.OfType<IMethodSymbol>()
.Where(method => method.DeclaredAccessibility.HasFlag(Accessibility.Public))
.ToArray();
if (publicMethods.Length == 0)
{
return;
}
var walker = new MutationWalker(context, aggregateType);

foreach (var onMethod in publicMethods)
{
if (onMethod.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is not MethodDeclarationSyntax syntax)
{
continue;
}
walker.Visit(syntax);
}
}

class MutationWalker : CSharpSyntaxWalker
{
readonly SyntaxNodeAnalysisContext _context;
readonly INamedTypeSymbol _aggregateType;

public MutationWalker(SyntaxNodeAnalysisContext context, INamedTypeSymbol aggregateType)
{
_context = context;
_aggregateType = aggregateType;
}

public override void VisitAssignmentExpression(AssignmentExpressionSyntax node)
{
var leftExpression = node.Left;

if (leftExpression is IdentifierNameSyntax || leftExpression is MemberAccessExpressionSyntax)
{
var symbolInfo = _context.SemanticModel.GetSymbolInfo(leftExpression);
if (symbolInfo.Symbol is IFieldSymbol || symbolInfo.Symbol is IPropertySymbol)
{
var containingType = symbolInfo.Symbol.ContainingType;
if (containingType != null && SymbolEqualityComparer.Default.Equals(_aggregateType, containingType))
{
var diagnostic = Diagnostic.Create(DescriptorRules.Aggregate.PublicMethodsCannotMutateAggregateState, leftExpression.GetLocation());
_context.ReportDiagnostic(diagnostic);
}
}
}

base.VisitAssignmentExpression(node);
}

// You can also add other types of mutations like increments, decrements, method calls etc.
}}
1 change: 1 addition & 0 deletions Source/Analyzers/AnalysisExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,5 @@ static bool MatchesName(this AttributeArgumentSyntax attributeArgument, string p
{
return attributeArgument.NameColon?.Name.Identifier.Text == parameterName;
}

}
119 changes: 119 additions & 0 deletions Source/Analyzers/CodeFixes/EventHandlerEventContextCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Copyright (c) Dolittle. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Dolittle.SDK.Analyzers.CodeFixes;

[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AttributeMissingCodeFixProvider)), Shared]
public class EventHandlerEventContextCodeFixProvider : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(
DiagnosticIds.EventHandlerMissingEventContext
);

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
var document = context.Document;

foreach (var diagnostic in context.Diagnostics)
{
switch (diagnostic.Id)
{
case DiagnosticIds.EventHandlerMissingEventContext:
context.RegisterCodeFix(
CodeAction.Create(
"Add EventContext parameter",
ct => AddEventContextParameter(context, diagnostic, document, ct),
nameof(EventHandlerEventContextCodeFixProvider) + ".AddEventContext"),
diagnostic);
break;
}
}


return Task.CompletedTask;
}

async Task<Document> AddEventContextParameter(CodeFixContext context, Diagnostic diagnostic, Document document, CancellationToken ct)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root is null) return document;

// Find the method declaration identified by the diagnostic.
var methodDeclaration = GetMethodDeclaration(root, diagnostic);
if (methodDeclaration is null)
{
return document;
}


var updatedRoot = root.ReplaceNode(methodDeclaration, WithEventContextParameter(methodDeclaration));
var newRoot = EnsureNamespaceImported((CompilationUnitSyntax)updatedRoot, "Dolittle.SDK.Events");
return document.WithSyntaxRoot(newRoot);
}

/// <summary>
/// Adds EventContext parameter to the method declaration
/// </summary>
/// <param name="methodDeclaration"></param>
/// <returns></returns>
MethodDeclarationSyntax WithEventContextParameter(MethodDeclarationSyntax methodDeclaration)
{
var existingParameters = methodDeclaration.ParameterList.Parameters;
// Get the first parameter that is not the EventContext parameter
var eventParameter = existingParameters.FirstOrDefault(parameter => parameter.Type?.ToString() != "EventContext");
if (eventParameter is null)
{
return methodDeclaration;
}


var eventContextParameter = SyntaxFactory.Parameter(SyntaxFactory.Identifier("ctx")).WithType(SyntaxFactory.ParseTypeName("EventContext"));

var originalParameterList = methodDeclaration.ParameterList;
var newParameterList = SyntaxFactory.ParameterList(
SyntaxFactory.SeparatedList(
new[]
{
eventParameter,
eventContextParameter
}
)
).WithLeadingTrivia(originalParameterList.GetLeadingTrivia())
.WithTrailingTrivia(originalParameterList.GetTrailingTrivia());
return methodDeclaration.WithParameterList(newParameterList);
}

MethodDeclarationSyntax? GetMethodDeclaration(SyntaxNode root, Diagnostic diagnostic)
{
var diagnosticSpan = diagnostic.Location.SourceSpan;
var methodDeclaration = root.FindToken(diagnosticSpan.Start).Parent?.AncestorsAndSelf().OfType<MethodDeclarationSyntax>().First();
return methodDeclaration;
}

public static CompilationUnitSyntax EnsureNamespaceImported(CompilationUnitSyntax root, string namespaceToInclude)
{
var usingDirective = SyntaxFactory.UsingDirective(SyntaxFactory.ParseName(namespaceToInclude));
var existingUsings = root.Usings;

if (existingUsings.Any(u => u.Name?.ToFullString() == namespaceToInclude))
{
// Namespace is already imported.
return root;
}
var lineEndingTrivia = root.DescendantTrivia().First(_ => _.IsKind(SyntaxKind.EndOfLineTrivia));
usingDirective = usingDirective.WithTrailingTrivia(lineEndingTrivia);

return root.WithUsings(existingUsings.Add(usingDirective));
}
}
51 changes: 50 additions & 1 deletion Source/Analyzers/DescriptorRules.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,26 @@ namespace Dolittle.SDK.Analyzers;

static class DescriptorRules
{
internal static readonly DiagnosticDescriptor InvalidTimestamp =
new(
DiagnosticIds.InvalidTimestampParameter,
title: "Invalid DateTimeOffset format",
messageFormat: "Value '{0}' should be a valid DateTimeOffset",
DiagnosticCategories.Sdk,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "The value should be a valid DateTimeOffset.");

internal static readonly DiagnosticDescriptor InvalidStartStopTimestamp =
new(
DiagnosticIds.InvalidStartStopTime,
title: "Start is not before stop",
messageFormat: "'{0}' should be before '{1}'",
DiagnosticCategories.Sdk,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "Start timestamp should be before stop timestamp.");

internal static readonly DiagnosticDescriptor InvalidIdentity =
new(
DiagnosticIds.AttributeInvalidIdentityRuleId,
Expand All @@ -26,7 +46,17 @@ static class DescriptorRules
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "Assign a unique identity in the attribute");


internal static readonly DiagnosticDescriptor InvalidAccessibility =
new(
DiagnosticIds.InvalidAccessibility,
title: "Invalid accessibility level",
messageFormat: "{0} needs to be '{1}'",
DiagnosticCategories.Sdk,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: "Change the accessibility level to '{1}'.");

internal static class Events
{
internal static readonly DiagnosticDescriptor MissingAttribute =
Expand All @@ -38,6 +68,16 @@ internal static class Events
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "Mark the event with an EventTypeAttribute and assign an identifier to it");

internal static readonly DiagnosticDescriptor MissingEventContext =
new(
DiagnosticIds.EventHandlerMissingEventContext,
title: "Handle method does not take EventContext as the second parameter",
messageFormat: "{0} is missing EventContext argument",
DiagnosticCategories.Sdk,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: "Add the EventContext as the second parameter to the Handle method");
}

internal static class Aggregate
Expand Down Expand Up @@ -90,5 +130,14 @@ internal static class Aggregate
DiagnosticSeverity.Error,
isEnabledByDefault: true
);

internal static readonly DiagnosticDescriptor PublicMethodsCannotMutateAggregateState = new(
DiagnosticIds.PublicMethodsCannotMutateAggregateState,
"Aggregates should only be mutated with events",
"Public methods can not mutate the state of an aggregate. All mutations needs to be done via events.",
DiagnosticCategories.Sdk,
DiagnosticSeverity.Warning,
isEnabledByDefault: true
);
}
}
22 changes: 22 additions & 0 deletions Source/Analyzers/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@ public static class DiagnosticIds
/// </summary>
public const string IdentityIsNotUniqueRuleId = "SDK0003";

/// <summary>
/// Invalid timestamp.
/// </summary>
public const string InvalidTimestampParameter = "SDK0004";

/// <summary>
/// Invalid timestamp.
/// </summary>
public const string InvalidStartStopTime = "SDK0005";

public const string InvalidAccessibility = "SDK0006";

public const string EventHandlerMissingEventContext = "SDK0007";

/// <summary>
/// Aggregate missing the required Attribute.
/// </summary>
Expand All @@ -44,4 +58,12 @@ public static class DiagnosticIds
/// Apply can not be used in an On-method.
/// </summary>
public const string AggregateMutationsCannotProduceEvents = "AGG0005";

/// <summary>
/// Public methods can not mutate the state of an aggregate.
/// All mutations need to be done in On-methods.
/// </summary>
public const string PublicMethodsCannotMutateAggregateState = "AGG0006";


}
2 changes: 2 additions & 0 deletions Source/Analyzers/DolittleTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ static class DolittleTypes

public const string ICommitEventsInterface = "Dolittle.SDK.Events.Store.ICommitEvents";

public const string EventContext = "Dolittle.SDK.Events.EventContext";


}
Loading

0 comments on commit c1b2d40

Please sign in to comment.