Skip to content

chore: Enable nullable annotation for validators #2902

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.

Sign up for GitHub

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

Merged
timcassell merged 3 commits into from
Dec 8, 2025
Merged

chore: Enable nullable annotation for validators #2902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f4bfa3
chore: enable nullable for validators
filzrev Dec 7, 2025
c24094c
chore: fix code based on review comment
filzrev Dec 8, 2025
d5020ae
chore: fix code based on review comment
filzrev Dec 8, 2025
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
13 changes: 8 additions & 5 deletions src/BenchmarkDotNet/Validators/BaselineValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
using System.Collections.Generic;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Order;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using BenchmarkDotNet.Jobs;

#nullable enable

namespace BenchmarkDotNet.Validators
{
Expand All @@ -16,14 +19,14 @@ private BaselineValidator() { }
public IEnumerable<ValidationError> Validate(ValidationParameters input)
{
var allBenchmarks = input.Benchmarks.ToImmutableArray();
var orderProvider = input.Config.Orderer;
var orderProvider = input.Config.Orderer ?? DefaultOrderer.Instance;

var benchmarkLogicalGroups = allBenchmarks
.Select(benchmark => orderProvider.GetLogicalGroupKey(allBenchmarks, benchmark))
.ToArray();

var logicalGroups = benchmarkLogicalGroups.Distinct().ToArray();
foreach (string logicalGroup in logicalGroups)
foreach (string? logicalGroup in logicalGroups)
{
var benchmarks = allBenchmarks.Where((benchmark, index) => benchmarkLogicalGroups[index] == logicalGroup).ToArray();
int methodBaselineCount = benchmarks.Select(b => b.Descriptor).Distinct().Count(it => it.Baseline);
Expand All @@ -38,7 +41,7 @@ public IEnumerable<ValidationError> Validate(ValidationParameters input)
}
}

private ValidationError CreateError(string subject, string property, string groupName, string className, string actual) =>
private ValidationError CreateError(string subject, string property, string? groupName, string className, string actual) =>
new ValidationError(
TreatsWarningsAsErrors,
$"Only 1 {subject} in a group can have \"{property}\" applied to it, group {groupName} in class {className} has {actual}");
Expand Down
4 changes: 3 additions & 1 deletion src/BenchmarkDotNet/Validators/CompilationValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using Microsoft.CodeAnalysis.CSharp;
using BenchmarkDotNet.Attributes;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public class CompilationValidator : IValidator
Expand Down Expand Up @@ -121,7 +123,7 @@ private class BenchmarkMethodEqualityComparer : IEqualityComparer<BenchmarkCase>
{
internal static readonly IEqualityComparer<BenchmarkCase> Instance = new BenchmarkMethodEqualityComparer();

public bool Equals(BenchmarkCase x, BenchmarkCase y)
public bool Equals(BenchmarkCase? x, BenchmarkCase? y)
{
if (x == null && y == null) return true;
if (x == null || y == null) return false;
Expand Down
4 changes: 3 additions & 1 deletion src/BenchmarkDotNet/Validators/ConfigValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using BenchmarkDotNet.Extensions;
using BenchmarkDotNet.Loggers;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public class ConfigValidator : IValidator
Expand Down Expand Up @@ -38,7 +40,7 @@ public IEnumerable<ValidationError> Validate(ValidationParameters validationPara
yield return pathValidation;
}

private static ValidationError ValidateArtifactsPath(string artifactsPath)
private static ValidationError? ValidateArtifactsPath(string artifactsPath)
{
if (artifactsPath == null) // null is OK, default path will be used
return null;
Expand Down
4 changes: 3 additions & 1 deletion src/BenchmarkDotNet/Validators/DeferredExecutionValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Threading.Tasks;
using BenchmarkDotNet.Extensions;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public class DeferredExecutionValidator : IValidator
Expand All @@ -29,7 +31,7 @@ public IEnumerable<ValidationError> Validate(ValidationParameters validationPara
private bool IsDeferredExecution(Type returnType)
{
if (returnType.IsByRef && !returnType.IsGenericType)
return IsDeferredExecution(returnType.GetElementType());
return IsDeferredExecution(returnType.GetElementType()!);

if (returnType.IsGenericType && (returnType.GetGenericTypeDefinition() == typeof(Task<>) || returnType.GetGenericTypeDefinition() == typeof(ValueTask<>)))
return IsDeferredExecution(returnType.GetGenericArguments().Single());
Expand Down
17 changes: 10 additions & 7 deletions src/BenchmarkDotNet/Validators/DotNetSdkValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;

#nullable enable

namespace BenchmarkDotNet.Validators
{
internal static class DotNetSdkValidator
Expand All @@ -26,15 +29,15 @@ public static IEnumerable<ValidationError> ValidateCoreSdks(string? customDotNet
var requiredSdkVersion = benchmark.GetRuntime().RuntimeMoniker.GetRuntimeVersion();
if (!GetInstalledDotNetSdks(customDotNetCliPath).Any(sdk => sdk >= requiredSdkVersion))
{
yield return new ValidationError(true, $"The required .NET Core SDK version {requiredSdkVersion} or higher for runtime moniker {benchmark.Job.Environment.Runtime.RuntimeMoniker} is not installed.", benchmark);
yield return new ValidationError(true, $"The required .NET Core SDK version {requiredSdkVersion} or higher for runtime moniker {benchmark.Job.Environment.Runtime!.RuntimeMoniker} is not installed.", benchmark);
}
}

public static IEnumerable<ValidationError> ValidateFrameworkSdks(BenchmarkCase benchmark)
{
var targetRuntime = benchmark.Job.Environment.HasValue(EnvironmentMode.RuntimeCharacteristic)
? benchmark.Job.Environment.Runtime
: ClrRuntime.GetTargetOrCurrentVersion(benchmark.Descriptor.WorkloadMethod.DeclaringType.Assembly);
? benchmark.Job.Environment.Runtime!
: ClrRuntime.GetTargetOrCurrentVersion(benchmark.Descriptor.WorkloadMethod.DeclaringType!.Assembly);
var requiredSdkVersion = targetRuntime.RuntimeMoniker.GetRuntimeVersion();

var installedVersionString = cachedFrameworkSdks.Value.FirstOrDefault();
Expand All @@ -44,7 +47,7 @@ public static IEnumerable<ValidationError> ValidateFrameworkSdks(BenchmarkCase b
}
}

public static bool IsCliPathInvalid(string customDotNetCliPath, BenchmarkCase benchmarkCase, out ValidationError? validationError)
public static bool IsCliPathInvalid(string? customDotNetCliPath, BenchmarkCase benchmarkCase, [NotNullWhen(true)] out ValidationError? validationError)
{
validationError = null;

Expand All @@ -71,7 +74,7 @@ public static bool IsCliPathInvalid(string customDotNetCliPath, BenchmarkCase be

private static IEnumerable<Version> GetInstalledDotNetSdks(string? customDotNetCliPath)
{
string dotnetExecutable = string.IsNullOrEmpty(customDotNetCliPath) ? "dotnet" : customDotNetCliPath;
string dotnetExecutable = string.IsNullOrEmpty(customDotNetCliPath) ? "dotnet" : customDotNetCliPath!;
var startInfo = new ProcessStartInfo(dotnetExecutable, "--list-sdks")
{
RedirectStandardOutput = true,
Expand Down Expand Up @@ -146,13 +149,13 @@ private static void Get45PlusFromRegistry(List<string> versions)

if (ndpKey.GetValue("Version") != null)
{
versions.Add(ndpKey.GetValue("Version").ToString());
versions.Add(ndpKey.GetValue("Version")!.ToString()!);
}
else
{
if (ndpKey.GetValue("Release") != null)
{
versions.Add(CheckFor45PlusVersion((int)ndpKey.GetValue("Release")));
versions.Add(CheckFor45PlusVersion((int)ndpKey.GetValue("Release")!));
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
Expand All @@ -8,6 +9,8 @@
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Running;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public abstract class ExecutionValidatorBase : IValidator
Expand Down Expand Up @@ -53,11 +56,11 @@ public IEnumerable<ValidationError> Validate(ValidationParameters validationPara
return errors;
}

private bool TryCreateBenchmarkTypeInstance(Type type, List<ValidationError> errors, out object? instance)
private bool TryCreateBenchmarkTypeInstance(Type type, List<ValidationError> errors, [NotNullWhen(true)] out object? instance)
{
try
{
instance = Activator.CreateInstance(type);
instance = Activator.CreateInstance(type)!;

return true;
}
Expand Down Expand Up @@ -124,7 +127,7 @@ private bool TryToCallGlobalMethod<T>(object benchmarkTypeInstance, List<Validat

private string GetAttributeName(Type type) => type.Name.Replace("Attribute", string.Empty);

private void TryToGetTaskResult(object result)
private void TryToGetTaskResult(object? result)
{
if (result == null)
{
Expand Down Expand Up @@ -241,7 +244,7 @@ private bool TryToSetParamsProperties(object benchmarkTypeInstance, List<Validat
protected static string GetDisplayExceptionMessage(Exception ex)
{
if (ex is TargetInvocationException targetInvocationException)
ex = targetInvocationException.InnerException;
ex = targetInvocationException.InnerException!;

return ex?.Message ?? "Unknown error";
}
Expand Down
11 changes: 7 additions & 4 deletions src/BenchmarkDotNet/Validators/ParamsAllValuesValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Extensions;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public class ParamsAllValuesValidator : IValidator
Expand All @@ -24,7 +27,7 @@ public IEnumerable<ValidationError> Validate(ValidationParameters input) =>
.SelectMany(type => type.GetTypeMembersWithGivenAttribute<ParamsAllValuesAttribute>(ReflectionFlags))
.Distinct()
.Select(member => GetErrorOrDefault(member.ParameterType))
.Where(error => error != null);
.WhereNotNull();

private bool IsBool(Type paramType) => paramType == typeof(bool);
private bool IsEnum(Type paramType) => paramType.GetTypeInfo().IsEnum;
Expand All @@ -33,17 +36,17 @@ private bool IsFlagsEnum(Type paramType)
var typeInfo = paramType.GetTypeInfo();
return typeInfo.IsEnum && typeInfo.IsDefined(typeof(FlagsAttribute));
}
private bool IsNullable(Type paramType, out Type underlyingType)
private bool IsNullable(Type paramType, [NotNullWhen(true)] out Type? underlyingType)
{
underlyingType = Nullable.GetUnderlyingType(paramType);
return underlyingType != null;
}

private ValidationError GetErrorOrDefault(Type parameterType)
private ValidationError? GetErrorOrDefault(Type parameterType)
{
switch (parameterType)
{
case Type t when IsNullable(t, out Type underType):
case Type t when IsNullable(t, out var underType):
return GetErrorOrDefault(underType);

case Type t when IsFlagsEnum(t):
Expand Down
10 changes: 6 additions & 4 deletions src/BenchmarkDotNet/Validators/ReturnValueValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
using BenchmarkDotNet.Running;
using BenchmarkDotNet.Toolchains.InProcess.NoEmit;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public class ReturnValueValidator : ExecutionValidatorBase
Expand All @@ -32,7 +34,7 @@ protected override void ExecuteBenchmarks(object benchmarkTypeInstance, IEnumera
try
{
InProcessNoEmitRunner.FillMembers(benchmarkTypeInstance, benchmark);
var result = benchmark.Descriptor.WorkloadMethod.Invoke(benchmarkTypeInstance, null);
var result = benchmark.Descriptor.WorkloadMethod.Invoke(benchmarkTypeInstance, null)!;

if (benchmark.Descriptor.WorkloadMethod.ReturnType != typeof(void))
results.Add((benchmark, result));
Expand Down Expand Up @@ -64,7 +66,7 @@ private class ParameterInstancesEqualityComparer : IEqualityComparer<ParameterIn
{
public static ParameterInstancesEqualityComparer Instance { get; } = new ParameterInstancesEqualityComparer();

public bool Equals(ParameterInstances x, ParameterInstances y)
public bool Equals(ParameterInstances? x, ParameterInstances? y)
{
if (ReferenceEquals(x, y))
return true;
Expand Down Expand Up @@ -98,7 +100,7 @@ private class InDepthEqualityComparer : IEqualityComparer
public static InDepthEqualityComparer Instance { get; } = new InDepthEqualityComparer();

[SuppressMessage("ReSharper", "MemberHidesStaticFromOuterClass")]
public new bool Equals(object x, object y)
public new bool Equals(object? x, object? y)
{
if (ReferenceEquals(x, y) || object.Equals(x, y))
return true;
Expand Down Expand Up @@ -137,7 +139,7 @@ private bool CompareStructural(object x, object y)

return false;

Array ToStructuralEquatable(object obj)
Array? ToStructuralEquatable(object obj)
{
switch (obj)
{
Expand Down
4 changes: 3 additions & 1 deletion src/BenchmarkDotNet/Validators/ValidationError.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using BenchmarkDotNet.Running;
using JetBrains.Annotations;

#nullable enable

namespace BenchmarkDotNet.Validators
{
public class ValidationError : IEquatable<ValidationError>
Expand Down Expand Up @@ -43,6 +45,6 @@ public override bool Equals(object? obj)

public static bool operator ==(ValidationError left, ValidationError right) => Equals(left, right);

public static bool operator !=(ValidationError left, ValidationError right) => !Equals(left, right);
public static bool operator !=(ValidationError? left, ValidationError? right) => !Equals(left, right);
}
}
10 changes: 5 additions & 5 deletions src/BenchmarkDotNet/Validators/ValidationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ public class ValidationParameters
{
public IReadOnlyList<BenchmarkCase> Benchmarks { get; }

public ImmutableConfig? Config { get; }
public ImmutableConfig Config { get; }
Copy link
Collaborator

@timcassell timcassell Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand why this is changed from nullable when null is explicitly passed in the implicit operators. We should just remove those operators if they are no longer useful.

Copy link
Contributor Author

@filzrev filzrev Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just remove those operators if they are no longer useful.

I think that's the correct way to fix it.

Though when removing these implicit operators. It cause about 39 build errors on test projects. and without implicit operators.
It need to add a lot of additional code. (e.g. Create ToValidationParameters extension method and call this method on every location that using implicit operators)

Copy link
Collaborator

@timcassell timcassell Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about changing null to DefaultConfig.Instance.CreateImmutableConfig() for now?

Copy link
Contributor Author

@filzrev filzrev Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified code to use DefaultConfig.Instance.CreateImmutableConfig() on implicit operators that accept BenchmarkCase[] benchmarksCase as parameter.
And use actual config when accept (BenchmarkRunInfo benchmarkRunInfo) as parameter.

In anyway, this config is not be referenced by existing unit tests/integration tests.


public ValidationParameters(IReadOnlyList<BenchmarkCase> benchmarks, ImmutableConfig? config)
public ValidationParameters(IReadOnlyList<BenchmarkCase> benchmarks, ImmutableConfig config)
{
Benchmarks = benchmarks;
Config = config;
}

// to have backward compatibility for people who implemented IValidator(Benchmark[] benchmarks)
public static implicit operator ValidationParameters(BenchmarkCase[] benchmarksCase) => new ValidationParameters(benchmarksCase, null);
public static implicit operator ValidationParameters(BenchmarkRunInfo benchmarkRunInfo) => new ValidationParameters(benchmarkRunInfo.BenchmarksCases, null);
// Note: Following implicit operators are expected to be used for test projects.
Copy link
Contributor Author

@filzrev filzrev Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove backward compatibility comments.
Because when null is passed to config parameter.
It'll throw NullReferenceException when accessing config property.

These implicit operators seems to be used for test projects.

public static implicit operator ValidationParameters(BenchmarkCase[] benchmarksCase) => new ValidationParameters(benchmarksCase, DefaultConfig.Instance.CreateImmutableConfig());
public static implicit operator ValidationParameters(BenchmarkRunInfo benchmarkRunInfo) => new ValidationParameters(benchmarkRunInfo.BenchmarksCases, benchmarkRunInfo.Config);
}
}