Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add analyzer to check for ICloneable or parameterless constructor #471

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project>
<!-- Set common properties regarding assembly information and nuget packages -->
<PropertyGroup>
<TimeWarpStateVersion>11.0.0-beta.85+8.0.303</TimeWarpStateVersion>
<TimeWarpStateVersion>11.0.0-beta.86+8.0.400</TimeWarpStateVersion>
<Authors>Steven T. Cramer</Authors>
<Product>TimeWarp State</Product>
<PackageVersion>$(TimeWarpStateVersion)</PackageVersion>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ StateReadOnlyPublicPropertiesRule | Design | Error | StateReadOnlyPublicProperti
StateSealedClassRule | Design | Warning | StateInheritanceAnalyzer
TW0001 | TimeWarp.State | Error | TimeWarpStateActionAnalyzer
TWD001 | Debug | Info | TimeWarpStateActionAnalyzer
TWS001 | Design | Error | StateImplementationAnalyzer
56 changes: 56 additions & 0 deletions Source/TimeWarp.State.Analyzer/StateImplementationAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
namespace TimeWarp.State.Analyzer;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class StateImplementationAnalyzer : DiagnosticAnalyzer
{
public const string DiagnosticId = "TWS001";

private static readonly LocalizableString Title = "State implementation must implement ICloneable or have a parameterless constructor";
private static readonly LocalizableString MessageFormat = "The state implementation '{0}' must implement ICloneable or have a parameterless constructor";
private static readonly LocalizableString Description = "State implementations should either implement ICloneable for proper cloning or have a parameterless constructor for serialization purposes.";
private const string Category = "Design";

private static readonly DiagnosticDescriptor Rule = new(DiagnosticId, Title, MessageFormat, Category, DiagnosticSeverity.Error, isEnabledByDefault: true, description: Description);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(Rule); } }

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();

context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType);
}

private static void AnalyzeSymbol(SymbolAnalysisContext context)
{
var namedTypeSymbol = (INamedTypeSymbol)context.Symbol;

if (IsStateImplementation(namedTypeSymbol))
{
if (!ImplementsICloneable(namedTypeSymbol) && !HasParameterlessConstructor(namedTypeSymbol))
{
var diagnostic = Diagnostic.Create(Rule, namedTypeSymbol.Locations[0], namedTypeSymbol.Name);
context.ReportDiagnostic(diagnostic);
}
}
}

private static bool IsStateImplementation(INamedTypeSymbol symbol)
{
if (symbol.BaseType == null)
return false;

return symbol.BaseType.Name == "State" && symbol.BaseType.TypeArguments.Length == 1;
}

private static bool ImplementsICloneable(INamedTypeSymbol symbol)
{
return symbol.AllInterfaces.Any(i => i.Name == "ICloneable");
}

private static bool HasParameterlessConstructor(INamedTypeSymbol symbol)
{
return symbol.Constructors.Any(c => c.Parameters.Length == 0);
}
}
1 change: 1 addition & 0 deletions Source/TimeWarp.State.Plus/EventIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ internal class EventIds

// Feature - StateInitializedNotificationHandler
public static readonly EventId StateInitializedNotificationHandler_Handling = new(1200, nameof(StateInitializedNotificationHandler_Handling));
public static readonly EventId StateInitializedNotificationHandler_LoadActionSetNotFound = new(1201, nameof(StateInitializedNotificationHandler_LoadActionSetNotFound));

// Feature - PersistenceService
public static readonly EventId PersistenceService_LoadState = new(1300, nameof(PersistenceService_LoadState));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ public sealed class FeatureFlagState : State<FeatureFlagState>
{
public FeatureFlagState(ISender sender) : base(sender) {}

[JsonConstructor]
public FeatureFlagState() {}

public override void Initialize() => throw new NotImplementedException();
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ ILogger<PersistenceService> logger
{
string typeName =
stateType.Name ??
throw new InvalidOperationException("The type provided has a null full name, which is not supported for persistence operations.");
throw new InvalidOperationException(message: "The type provided has a null full name, which is not supported for persistence operations.");

Logger.LogInformation(EventIds.PersistenceService_LoadState, "Loading State for {stateType}", stateType);
Logger.LogInformation(EventIds.PersistenceService_LoadState, message: "Loading State for {stateType}", stateType);

string? serializedState = persistentStateMethod switch
{
Expand All @@ -40,7 +40,7 @@ ILogger<PersistenceService> logger
Logger.LogTrace
(
EventIds.PersistenceService_LoadState_SerializedState,
"Serialized State: {serializedState}",
message: "Serialized State: {serializedState}",
serializedState
);

Expand All @@ -51,9 +51,15 @@ ILogger<PersistenceService> logger
{
result = JsonSerializer.Deserialize(serializedState, stateType, JsonSerializerOptions);
}
catch (JsonException ex)
catch (JsonException jsonException)
{
Logger.LogError(EventIds.PersistenceService_LoadState_DeserializationError, ex, "Error deserializing state for {stateType}", stateType);
Logger.LogError
(
EventIds.PersistenceService_LoadState_DeserializationError,
jsonException,
message: "Error deserializing state for {stateType}",
stateType
);
throw;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,29 @@ public async Task Handle(StateInitializedNotification stateInitializedNotificati
string assemblyQualifiedName = stateInitializedNotification.StateType.AssemblyQualifiedName ?? throw new InvalidOperationException();

string typeName = assemblyQualifiedName.Replace(fullName, $"{fullName}+LoadActionSet+Action");
Logger.LogDebug(EventIds.StateInitializedNotificationHandler_Handling, "StateInitializedNotificationHandler: {StateTypeName}", stateInitializedNotification.StateType.Name);

Logger.LogDebug
(
EventIds.StateInitializedNotificationHandler_Handling,
message: "StateInitializedNotificationHandler: {StateTypeName}",
stateInitializedNotification.StateType.Name
);

var actionType = Type.GetType(typeName);

if (actionType != null)
{
object action = Activator.CreateInstance(actionType) ?? throw new InvalidOperationException();
await Sender.Send(action, cancellationToken);
}
else
{
Logger.LogDebug
(
EventIds.StateInitializedNotificationHandler_LoadActionSetNotFound,
message: "StateInitializedNotificationHandler: {LoadActionSetTypeName} not found.",
typeName
);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
namespace TimeWarp.Features.Routing;

using System.Text.Json.Serialization;

/// <summary>
/// Maintain the Route in TimeWarp.State
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ public sealed partial class ThemeState : State<ThemeState>
public Theme CurrentTheme { get; private set; }

public ThemeState(ISender sender) : base(sender) {}

[JsonConstructor]
public ThemeState() {}

public override void Initialize() => CurrentTheme = Theme.System;

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
namespace TimeWarp.State.Plus.Features.Timers;

using Microsoft.Extensions.Options;
using System.Timers;

public sealed partial class TimerState : State<TimerState>
public sealed partial class TimerState : State<TimerState>, ICloneable
{
private readonly ILogger<TimerState> Logger;
private readonly IPublisher Publisher;
private readonly MultiTimerOptions MultiTimerOptions;
private Dictionary<string, (Timer Timer, TimerConfig TimerConfig)> Timers = new();

public TimerState
(
IOptions<MultiTimerOptions> multiTimerOptionsAccessor,
Expand All @@ -19,6 +21,31 @@ IPublisher publisher
Publisher = publisher;
MultiTimerOptions = multiTimerOptionsAccessor.Value;
}

/// <summary>
/// Creates a new instance of TimerState with the same configuration and Timer instances.
/// </summary>
/// <remarks>
/// This method performs a shallow clone of the state.
/// It reuses the existing Timer instances and configuration.
/// If an error occurs in an action, it will not rollback to the previous state.
/// This approach is intentional to maintain consistency of timer states across clones.
/// Actions are expected to be well-tested and reliable, minimizing the risk of failures.
/// </remarks>
/// <returns>A new TimerState instance with the same configuration and Timer instances.</returns>
public object Clone()
{
return new TimerState
(
new OptionsWrapper<MultiTimerOptions>(MultiTimerOptions),
Logger,
Publisher
)
{
Timers = this.Timers
};
}

public override void Initialize()
{
Timers.Clear();
Expand Down
1 change: 1 addition & 0 deletions Source/TimeWarp.State.Plus/GlobalUsings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@
global using Microsoft.JSInterop;
global using System.Reflection;
global using System.Text.Json;
global using System.Text.Json.Serialization;
global using TimeWarp.State.Plus;
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ IPublisher publisher
Logger.LogDebug
(
EventIds.StateTransactionBehavior_Constructing,
"constructing {ClassName}<{RequestType},{ResponseType}>",
message: "constructing {ClassName}<{RequestType},{ResponseType}>",
className,
typeof(TRequest).Name,
typeof(TResponse).Name
Expand All @@ -53,14 +53,14 @@ CancellationToken cancellationToken
{
// Analyzer will ensure the following. If IAction it has to be nested in a IState implementation.
Type enclosingStateType = typeof(TRequest).GetEnclosingStateType();
var originalState = (IState)Store.GetState(enclosingStateType)!;// Not null because of Analyzer
var originalState = (IState)Store.GetState(enclosingStateType);
IState newState = (originalState is ICloneable cloneable) ?
(IState)cloneable.Clone() :
originalState.Clone
(
(ex, path, _, _) =>
{
Logger.LogWarning("Cloning error: {path} {Message}", path, ex.Message);
Logger.LogWarning(message: "Cloning error: {path} {Message}", path, ex.Message);
}
);

Expand All @@ -75,7 +75,7 @@ CancellationToken cancellationToken
Logger.LogDebug
(
EventIds.StateTransactionBehavior_Cloning,
"Cloned State of type {declaringType} originalState.Guid:{originalState_Guid} newState.Guid:{newState_Guid}",
message: "Cloned State of type {declaringType} originalState.Guid:{originalState_Guid} newState.Guid:{newState_Guid}",
enclosingStateType,
originalState.Guid,
newState.Guid
Expand All @@ -94,15 +94,15 @@ CancellationToken cancellationToken
(
EventIds.StateTransactionBehavior_Exception,
exception,
"Error cloning State. Type:{enclosingStateType}",
message: "Error cloning State. Type:{enclosingStateType}",
enclosingStateType
);

// If something fails we restore system to previous state.
Logger.LogInformation
(
EventIds.StateTransactionBehavior_Restoring,
"Attempting to restore State of type: {enclosingStateType}",
message: "Attempting to restore State of type: {enclosingStateType}",
enclosingStateType
);

Expand Down
Loading