Skip to content

Commit

Permalink
chore!: Enable nullable reference types (#253)
Browse files Browse the repository at this point in the history
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## Enable nullable reference types
<!-- add the description of the PR here -->

- This PR enables the nullable validation and treats warnings as errors.

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

Fixes #208 

### Notes
<!-- any additional notes for this PR -->
This PR turns on the nullable checks for the dotnet checks. This gives
us a better and safer codebase since it checks for potential null
exceptions.

### Breaking changes
While this PR won't require changes to the exposed API, it might show
some errors to the clients consuming it.

---------

Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
  • Loading branch information
askpt committed Apr 5, 2024
1 parent 9b9c3fd commit 5a5312c
Show file tree
Hide file tree
Showing 45 changed files with 486 additions and 272 deletions.
2 changes: 2 additions & 0 deletions build/Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<!-- Workaround for IDE0005 (Remove unnecessary usings/imports); see https://github.com/dotnet/roslyn/issues/41640 -->
<NoWarn>EnableGenerateDocumentationFile</NoWarn>
<Nullable>enable</Nullable>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)'=='Debug'">
Expand Down
12 changes: 8 additions & 4 deletions src/OpenFeature/Api.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void SetProvider(FeatureProvider featureProvider)
/// </summary>
/// <remarks>The provider cannot be set to null. Attempting to set the provider to null has no effect.</remarks>
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
public async Task SetProviderAsync(FeatureProvider featureProvider)
public async Task SetProviderAsync(FeatureProvider? featureProvider)
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
await this._repository.SetProvider(featureProvider, this.GetContext()).ConfigureAwait(false);
Expand All @@ -80,6 +80,10 @@ public void SetProvider(string clientName, FeatureProvider featureProvider)
/// <param name="featureProvider">Implementation of <see cref="FeatureProvider"/></param>
public async Task SetProviderAsync(string clientName, FeatureProvider featureProvider)
{
if (string.IsNullOrWhiteSpace(clientName))
{
throw new ArgumentNullException(nameof(clientName));
}
this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider);
await this._repository.SetProvider(clientName, featureProvider, this.GetContext()).ConfigureAwait(false);
}
Expand Down Expand Up @@ -138,8 +142,8 @@ public FeatureProvider GetProvider(string clientName)
/// <param name="logger">Logger instance used by client</param>
/// <param name="context">Context given to this client</param>
/// <returns><see cref="FeatureClient"/></returns>
public FeatureClient GetClient(string name = null, string version = null, ILogger logger = null,
EvaluationContext context = null) =>
public FeatureClient GetClient(string? name = null, string? version = null, ILogger? logger = null,
EvaluationContext? context = null) =>
new FeatureClient(name, version, logger, context);

/// <summary>
Expand Down Expand Up @@ -200,7 +204,7 @@ public void AddHooks(IEnumerable<Hook> hooks)
/// Sets the global <see cref="EvaluationContext"/>
/// </summary>
/// <param name="context">The <see cref="EvaluationContext"/> to set</param>
public void SetContext(EvaluationContext context)
public void SetContext(EvaluationContext? context)
{
this._evaluationContextLock.EnterWriteLock();
try
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/FeatureProviderException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class FeatureProviderException : Exception
/// <param name="errorType">Common error types <see cref="ErrorType"/></param>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public FeatureProviderException(ErrorType errorType, string message = null, Exception innerException = null)
public FeatureProviderException(ErrorType errorType, string? message = null, Exception? innerException = null)
: base(message, innerException)
{
this.ErrorType = errorType;
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/FlagNotFoundException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class FlagNotFoundException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public FlagNotFoundException(string message = null, Exception innerException = null)
public FlagNotFoundException(string? message = null, Exception? innerException = null)
: base(ErrorType.FlagNotFound, message, innerException)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/GeneralException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class GeneralException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public GeneralException(string message = null, Exception innerException = null)
public GeneralException(string? message = null, Exception? innerException = null)
: base(ErrorType.General, message, innerException)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/InvalidContextException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class InvalidContextException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public InvalidContextException(string message = null, Exception innerException = null)
public InvalidContextException(string? message = null, Exception? innerException = null)
: base(ErrorType.InvalidContext, message, innerException)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/ParseErrorException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class ParseErrorException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public ParseErrorException(string message = null, Exception innerException = null)
public ParseErrorException(string? message = null, Exception? innerException = null)
: base(ErrorType.ParseError, message, innerException)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/ProviderNotReadyException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class ProviderNotReadyException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public ProviderNotReadyException(string message = null, Exception innerException = null)
public ProviderNotReadyException(string? message = null, Exception? innerException = null)
: base(ErrorType.ProviderNotReady, message, innerException)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/TargetingKeyMissingException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class TargetingKeyMissingException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public TargetingKeyMissingException(string message = null, Exception innerException = null)
public TargetingKeyMissingException(string? message = null, Exception? innerException = null)
: base(ErrorType.TargetingKeyMissing, message, innerException)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/OpenFeature/Error/TypeMismatchException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class TypeMismatchException : FeatureProviderException
/// </summary>
/// <param name="message">Exception message</param>
/// <param name="innerException">Optional inner exception</param>
public TypeMismatchException(string message = null, Exception innerException = null)
public TypeMismatchException(string? message = null, Exception? innerException = null)
: base(ErrorType.TypeMismatch, message, innerException)
{
}
Expand Down
36 changes: 18 additions & 18 deletions src/OpenFeature/EventExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal class EventExecutor : IAsyncDisposable
{
private readonly object _lockObj = new object();
public readonly Channel<object> EventChannel = Channel.CreateBounded<object>(1);
private FeatureProvider _defaultProvider;
private FeatureProvider? _defaultProvider;
private readonly Dictionary<string, FeatureProvider> _namedProviderReferences = new Dictionary<string, FeatureProvider>();
private readonly List<FeatureProvider> _activeSubscriptions = new List<FeatureProvider>();

Expand Down Expand Up @@ -99,7 +99,7 @@ internal void RemoveClientHandler(string client, ProviderEventTypes type, EventH
}
}

internal void RegisterDefaultFeatureProvider(FeatureProvider provider)
internal void RegisterDefaultFeatureProvider(FeatureProvider? provider)
{
if (provider == null)
{
Expand All @@ -115,7 +115,7 @@ internal void RegisterDefaultFeatureProvider(FeatureProvider provider)
}
}

internal void RegisterClientFeatureProvider(string client, FeatureProvider provider)
internal void RegisterClientFeatureProvider(string client, FeatureProvider? provider)
{
if (provider == null)
{
Expand All @@ -124,7 +124,7 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider provi
lock (this._lockObj)
{
var newProvider = provider;
FeatureProvider oldProvider = null;
FeatureProvider? oldProvider = null;
if (this._namedProviderReferences.TryGetValue(client, out var foundOldProvider))
{
oldProvider = foundOldProvider;
Expand All @@ -136,7 +136,7 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider provi
}
}

private void StartListeningAndShutdownOld(FeatureProvider newProvider, FeatureProvider oldProvider)
private void StartListeningAndShutdownOld(FeatureProvider newProvider, FeatureProvider? oldProvider)
{
// check if the provider is already active - if not, we need to start listening for its emitted events
if (!this.IsProviderActive(newProvider))
Expand Down Expand Up @@ -174,7 +174,7 @@ private bool IsProviderActive(FeatureProvider providerRef)
return this._activeSubscriptions.Contains(providerRef);
}

private void EmitOnRegistration(FeatureProvider provider, ProviderEventTypes eventType, EventHandlerDelegate handler)
private void EmitOnRegistration(FeatureProvider? provider, ProviderEventTypes eventType, EventHandlerDelegate handler)
{
if (provider == null)
{
Expand Down Expand Up @@ -202,22 +202,22 @@ private void EmitOnRegistration(FeatureProvider provider, ProviderEventTypes eve
{
handler.Invoke(new ProviderEventPayload
{
ProviderName = provider.GetMetadata()?.Name,
ProviderName = provider.GetMetadata().Name,
Type = eventType,
Message = message
});
}
catch (Exception exc)
{
this.Logger?.LogError("Error running handler: " + exc);
this.Logger.LogError(exc, "Error running handler");
}
}
}

private async void ProcessFeatureProviderEventsAsync(object providerRef)
private async void ProcessFeatureProviderEventsAsync(object? providerRef)
{
var typedProviderRef = (FeatureProvider)providerRef;
if (typedProviderRef.GetEventChannel() is not { Reader: { } reader })
var typedProviderRef = (FeatureProvider?)providerRef;
if (typedProviderRef?.GetEventChannel() is not { Reader: { } reader })
{
return;
}
Expand Down Expand Up @@ -249,7 +249,7 @@ private async void ProcessEventAsync()
case Event e:
lock (this._lockObj)
{
if (this._apiHandlers.TryGetValue(e.EventPayload.Type, out var eventHandlers))
if (e.EventPayload?.Type != null && this._apiHandlers.TryGetValue(e.EventPayload.Type, out var eventHandlers))
{
foreach (var eventHandler in eventHandlers)
{
Expand All @@ -260,11 +260,11 @@ private async void ProcessEventAsync()
// look for client handlers and call invoke method there
foreach (var keyAndValue in this._namedProviderReferences)
{
if (keyAndValue.Value == e.Provider)
if (keyAndValue.Value == e.Provider && keyAndValue.Key != null)
{
if (this._clientHandlers.TryGetValue(keyAndValue.Key, out var clientRegistry))
{
if (clientRegistry.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
if (e.EventPayload?.Type != null && clientRegistry.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
{
foreach (var eventHandler in clientEventHandlers)
{
Expand All @@ -288,7 +288,7 @@ private async void ProcessEventAsync()
// if there is an association for the client to a specific feature provider, then continue
continue;
}
if (keyAndValues.Value.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
if (e.EventPayload?.Type != null && keyAndValues.Value.TryGetValue(e.EventPayload.Type, out var clientEventHandlers))
{
foreach (var eventHandler in clientEventHandlers)
{
Expand All @@ -311,7 +311,7 @@ private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e)
}
catch (Exception exc)
{
this.Logger?.LogError("Error running handler: " + exc);
this.Logger.LogError(exc, "Error running handler");
}
}

Expand All @@ -325,7 +325,7 @@ public async Task Shutdown()

internal class Event
{
internal FeatureProvider Provider { get; set; }
internal ProviderEventPayload EventPayload { get; set; }
internal FeatureProvider? Provider { get; set; }
internal ProviderEventPayload? EventPayload { get; set; }
}
}
2 changes: 1 addition & 1 deletion src/OpenFeature/Extension/EnumExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ internal static class EnumExtensions
public static string GetDescription(this Enum value)
{
var field = value.GetType().GetField(value.ToString());
var attribute = field.GetCustomAttributes(typeof(DescriptionAttribute), false).FirstOrDefault() as DescriptionAttribute;
var attribute = field?.GetCustomAttributes(typeof(DescriptionAttribute), false).FirstOrDefault() as DescriptionAttribute;
return attribute?.Description ?? value.ToString();
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/OpenFeature/FeatureProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public abstract class FeatureProvider
/// <param name="context"><see cref="EvaluationContext"/></param>
/// <returns><see cref="ResolutionDetails{T}"/></returns>
public abstract Task<ResolutionDetails<bool>> ResolveBooleanValue(string flagKey, bool defaultValue,
EvaluationContext context = null);
EvaluationContext? context = null);

/// <summary>
/// Resolves a string feature flag
Expand All @@ -55,7 +55,7 @@ public abstract Task<ResolutionDetails<bool>> ResolveBooleanValue(string flagKey
/// <param name="context"><see cref="EvaluationContext"/></param>
/// <returns><see cref="ResolutionDetails{T}"/></returns>
public abstract Task<ResolutionDetails<string>> ResolveStringValue(string flagKey, string defaultValue,
EvaluationContext context = null);
EvaluationContext? context = null);

/// <summary>
/// Resolves a integer feature flag
Expand All @@ -65,7 +65,7 @@ public abstract Task<ResolutionDetails<string>> ResolveStringValue(string flagKe
/// <param name="context"><see cref="EvaluationContext"/></param>
/// <returns><see cref="ResolutionDetails{T}"/></returns>
public abstract Task<ResolutionDetails<int>> ResolveIntegerValue(string flagKey, int defaultValue,
EvaluationContext context = null);
EvaluationContext? context = null);

/// <summary>
/// Resolves a double feature flag
Expand All @@ -75,7 +75,7 @@ public abstract Task<ResolutionDetails<int>> ResolveIntegerValue(string flagKey,
/// <param name="context"><see cref="EvaluationContext"/></param>
/// <returns><see cref="ResolutionDetails{T}"/></returns>
public abstract Task<ResolutionDetails<double>> ResolveDoubleValue(string flagKey, double defaultValue,
EvaluationContext context = null);
EvaluationContext? context = null);

/// <summary>
/// Resolves a structured feature flag
Expand All @@ -85,7 +85,7 @@ public abstract Task<ResolutionDetails<double>> ResolveDoubleValue(string flagKe
/// <param name="context"><see cref="EvaluationContext"/></param>
/// <returns><see cref="ResolutionDetails{T}"/></returns>
public abstract Task<ResolutionDetails<Value>> ResolveStructureValue(string flagKey, Value defaultValue,
EvaluationContext context = null);
EvaluationContext? context = null);

/// <summary>
/// Get the status of the provider.
Expand Down
8 changes: 4 additions & 4 deletions src/OpenFeature/Hook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public abstract class Hook
/// <typeparam name="T">Flag value type (bool|number|string|object)</typeparam>
/// <returns>Modified EvaluationContext that is used for the flag evaluation</returns>
public virtual Task<EvaluationContext> Before<T>(HookContext<T> context,
IReadOnlyDictionary<string, object> hints = null)
IReadOnlyDictionary<string, object>? hints = null)
{
return Task.FromResult(EvaluationContext.Empty);
}
Expand All @@ -42,7 +42,7 @@ public virtual Task<EvaluationContext> Before<T>(HookContext<T> context,
/// <param name="hints">Caller provided data</param>
/// <typeparam name="T">Flag value type (bool|number|string|object)</typeparam>
public virtual Task After<T>(HookContext<T> context, FlagEvaluationDetails<T> details,
IReadOnlyDictionary<string, object> hints = null)
IReadOnlyDictionary<string, object>? hints = null)
{
return Task.CompletedTask;
}
Expand All @@ -55,7 +55,7 @@ public virtual Task After<T>(HookContext<T> context, FlagEvaluationDetails<T> de
/// <param name="hints">Caller provided data</param>
/// <typeparam name="T">Flag value type (bool|number|string|object)</typeparam>
public virtual Task Error<T>(HookContext<T> context, Exception error,
IReadOnlyDictionary<string, object> hints = null)
IReadOnlyDictionary<string, object>? hints = null)
{
return Task.CompletedTask;
}
Expand All @@ -66,7 +66,7 @@ public virtual Task Error<T>(HookContext<T> context, Exception error,
/// <param name="context">Provides context of innovation</param>
/// <param name="hints">Caller provided data</param>
/// <typeparam name="T">Flag value type (bool|number|string|object)</typeparam>
public virtual Task Finally<T>(HookContext<T> context, IReadOnlyDictionary<string, object> hints = null)
public virtual Task Finally<T>(HookContext<T> context, IReadOnlyDictionary<string, object>? hints = null)
{
return Task.CompletedTask;
}
Expand Down
Loading

0 comments on commit 5a5312c

Please sign in to comment.