Skip to content

chore: Enable nullable for job/characteristics relating code #2900

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 for job/characteristics relating code #2900

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
587ebac
chore: enable nullable for job/characteristics relating code
filzrev Dec 6, 2025
74e204d
chore: fix nullable annotation of AssertHasValue
filzrev Dec 7, 2025
d7ebe52
chore: add additional nullable annotations
filzrev Dec 7, 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
10 changes: 6 additions & 4 deletions src/BenchmarkDotNet/Characteristics/Characteristic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Diagnostics.CodeAnalysis;
using static BenchmarkDotNet.Characteristics.CharacteristicHelper;

#nullable enable

namespace BenchmarkDotNet.Characteristics
{
public abstract class Characteristic
Expand All @@ -24,7 +26,7 @@ public abstract class Characteristic
null, fallbackValue,
false);

public static Characteristic<T> Create<TOwner, [DynamicallyAccessedMembers(CharacteristicObject.CharacteristicMemberTypes)] T>(string memberName, Func<CharacteristicObject, T, T> resolver, T fallbackValue, bool ignoreOnApply)
public static Characteristic<T> Create<TOwner, [DynamicallyAccessedMembers(CharacteristicObject.CharacteristicMemberTypes)] T>(string memberName, Func<CharacteristicObject, T?, T> resolver, T fallbackValue, bool ignoreOnApply)
Copy link
Contributor Author

@filzrev filzrev Dec 6, 2025

Choose a reason for hiding this comment

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

It need to allow null because some characteristics expected to return null(e.g. RuntimeCharacteristic/EnvironmentVariablesCharacteristic)

where TOwner : CharacteristicObject
=> new Characteristic<T>(
memberName,
Expand Down Expand Up @@ -52,7 +54,7 @@ protected Characteristic(
string id,
[DynamicallyAccessedMembers(CharacteristicObject.CharacteristicMemberTypes)] Type characteristicType,
Type declaringType,
object fallbackValue,
object? fallbackValue,
bool ignoreOnApply,
bool dontShowInSummary = false)
{
Expand Down Expand Up @@ -84,7 +86,7 @@ protected Characteristic(

public Type DeclaringType { get; }

private object FallbackValue { get; }
private object? FallbackValue { get; }

public object? this[CharacteristicObject obj]
{
Expand All @@ -94,7 +96,7 @@ public object? this[CharacteristicObject obj]

public bool HasChildCharacteristics => IsCharacteristicObjectSubclass(CharacteristicType);

internal virtual object ResolveValueCore(CharacteristicObject obj, object currentValue) =>
internal virtual object? ResolveValueCore(CharacteristicObject obj, object currentValue) =>
ReferenceEquals(currentValue, EmptyValue) ? FallbackValue : currentValue;

public override string ToString() => Id;
Expand Down
8 changes: 5 additions & 3 deletions src/BenchmarkDotNet/Characteristics/CharacteristicHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System.Reflection;
using JetBrains.Annotations;

#nullable enable

namespace BenchmarkDotNet.Characteristics
{
public static class CharacteristicHelper
Expand All @@ -17,7 +19,7 @@ internal static bool IsCharacteristicObjectSubclass(Type type) =>
private static bool IsCharacteristicSubclass(Type type) =>
type.GetTypeInfo().IsSubclassOf(typeof(Characteristic));

private static Characteristic AssertHasValue(MemberInfo member, Characteristic value)
private static Characteristic AssertHasValue(MemberInfo member, Characteristic? value)
{
if (member?.DeclaringType == null)
throw new NullReferenceException($"{nameof(member.DeclaringType)}");
Expand Down Expand Up @@ -54,12 +56,12 @@ private static IReadOnlyList<Characteristic> GetThisTypeCharacteristicsCore(
var fieldValues = characteristicObjectType.GetTypeInfo()
.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.FlattenHierarchy | BindingFlags.Static)
.Where(f => IsCharacteristicSubclass(f.FieldType))
.Select(f => AssertHasValue(f, (Characteristic)f.GetValue(null)));
.Select(f => AssertHasValue(f, (Characteristic?)f.GetValue(null)));

var propertyValues = characteristicObjectType.GetTypeInfo()
.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.FlattenHierarchy | BindingFlags.Static)
.Where(p => p.GetMethod != null && IsCharacteristicSubclass(p.PropertyType))
.Select(p => AssertHasValue(p, (Characteristic)p.GetValue(null)));
.Select(p => AssertHasValue(p, (Characteristic?)p.GetValue(null)));

// DONTTOUCH: DO NOT change the order of characteristic as it may break logic of some operations.
return fieldValues
Expand Down
56 changes: 29 additions & 27 deletions src/BenchmarkDotNet/Characteristics/CharacteristicObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Reflection;
using JetBrains.Annotations;

#nullable enable

namespace BenchmarkDotNet.Characteristics
{
// TODO: better naming.
Expand All @@ -17,15 +19,15 @@ public abstract class CharacteristicObject
DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.NonPublicProperties
| DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.NonPublicFields;

protected static string ResolveId(CharacteristicObject obj, string actual)
protected static string ResolveId(CharacteristicObject obj, string? actual)
{
if (!string.IsNullOrEmpty(actual) && actual != IdCharacteristic.FallbackValue)
return actual;
return actual!;

string result = CharacteristicSetPresenter.Display.ToPresentation(obj);

if (result.Length == 0)
result = IdCharacteristic.FallbackValue;
result = IdCharacteristic.FallbackValue!;
Copy link
Contributor Author

@filzrev filzrev Dec 6, 2025

Choose a reason for hiding this comment

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

IdCharacteristic specify Default as fallback value. so it must not be null.


return result;
}
Expand All @@ -44,7 +46,7 @@ protected CharacteristicObject()
sharedValues = new Dictionary<Characteristic, object>();
}

protected CharacteristicObject(string? id) : this()
protected CharacteristicObject(string id) : this()
{
if (!string.IsNullOrEmpty(id))
{
Expand Down Expand Up @@ -79,7 +81,7 @@ private void AssertIsNonFrozenRoot()
AssertIsRoot();
}

private static void AssertIsAssignable(Characteristic characteristic, object value)
private static void AssertIsAssignable(Characteristic characteristic, object? value)
{
if (ReferenceEquals(value, Characteristic.EmptyValue) || ReferenceEquals(value, null))
{
Expand Down Expand Up @@ -140,53 +142,53 @@ public bool HasValue(Characteristic characteristic)
return false;
}

internal T GetValue<[DynamicallyAccessedMembers(CharacteristicMemberTypes)] T>(Characteristic<T> characteristic)
internal T? GetValue<[DynamicallyAccessedMembers(CharacteristicMemberTypes)] T>(Characteristic<T> characteristic)
{
return (T)GetValue((Characteristic)characteristic);
return (T?)GetValue((Characteristic)characteristic);
}

internal object GetValue(Characteristic characteristic)
internal object? GetValue(Characteristic characteristic)
{
if (!sharedValues.TryGetValue(characteristic, out var result))
result = Characteristic.EmptyValue;

return ResolveCore(characteristic, result);
}

private object ResolveCore(Characteristic characteristic, object result)
private object? ResolveCore(Characteristic characteristic, object result)
{
return characteristic.ResolveValueCore(this, result);
}
#endregion

#region Resolve
public T ResolveValue<[DynamicallyAccessedMembers(CharacteristicMemberTypes)] T>(Characteristic<T> characteristic, IResolver resolver)
public T? ResolveValue<[DynamicallyAccessedMembers(CharacteristicMemberTypes)] T>(Characteristic<T> characteristic, IResolver resolver)
{
return resolver.Resolve(this, characteristic);
}

public T ResolveValue<[DynamicallyAccessedMembers(CharacteristicMemberTypes)] T>(Characteristic<T> characteristic, IResolver resolver, T defaultValue)
public T? ResolveValue<[DynamicallyAccessedMembers(CharacteristicMemberTypes)] T>(Characteristic<T> characteristic, IResolver resolver, T defaultValue)
{
return resolver.Resolve(this, characteristic, defaultValue);
}

public object ResolveValue(Characteristic characteristic, IResolver resolver)
public object? ResolveValue(Characteristic characteristic, IResolver resolver)
{
return resolver.Resolve(this, characteristic);
}

public object ResolveValue(Characteristic characteristic, IResolver resolver, object defaultValue)
public object? ResolveValue(Characteristic characteristic, IResolver resolver, object defaultValue)
{
return resolver.Resolve(this, characteristic, defaultValue);
}

public T ResolveValue<[DynamicallyAccessedMembers(CharacteristicObject.CharacteristicMemberTypes)] T>(Characteristic<T> characteristic, T defaultValue)
public T? ResolveValue<[DynamicallyAccessedMembers(CharacteristicObject.CharacteristicMemberTypes)] T>(Characteristic<T> characteristic, T defaultValue)
{
return HasValue(characteristic) ? GetValue(characteristic) : (T)characteristic.ResolveValueCore(this, defaultValue);
return HasValue(characteristic) ? GetValue(characteristic) : (T?)characteristic.ResolveValueCore(this, defaultValue!);
}

[PublicAPI]
public object ResolveValue(Characteristic characteristic, object defaultValue)
public object? ResolveValue(Characteristic characteristic, object defaultValue)
{
return HasValue(characteristic) ? GetValue(characteristic) : characteristic.ResolveValueCore(this, defaultValue);
}
Expand All @@ -203,16 +205,16 @@ public object ResolveValue(Characteristic characteristic, object defaultValue)
SetValue((Characteristic)characteristic, value);
}

internal void SetValue(Characteristic characteristic, object value)
internal void SetValue(Characteristic characteristic, object? value)
{
AssertNotFrozen();

if (characteristic.HasChildCharacteristics)
{
AssertIsAssignable(characteristic, value);

var oldObjectValue = (CharacteristicObject)GetValue(characteristic);
var newObjectValue = (CharacteristicObject)ResolveCore(characteristic, value);
var oldObjectValue = (CharacteristicObject?)GetValue(characteristic);
var newObjectValue = (CharacteristicObject?)ResolveCore(characteristic, value!);

if (!ReferenceEquals(oldObjectValue, newObjectValue))
{
Expand All @@ -226,7 +228,7 @@ internal void SetValue(Characteristic characteristic, object value)
}
}

private void SetValueCore(Characteristic characteristic, object value)
private void SetValueCore(Characteristic characteristic, object? value)
{
AssertIsAssignable(characteristic, value);

Expand All @@ -243,7 +245,7 @@ private void SetValueCore(Characteristic characteristic, object value)
$"The current node {this} has value for {characteristic} already.",
nameof(characteristic));

var characteristicObject = (CharacteristicObject)ResolveCore(characteristic, value);
var characteristicObject = (CharacteristicObject)ResolveCore(characteristic, value!)!;
characteristicObject.SetOwnerCore(OwnerOrSelf);

sharedValues[characteristic] = characteristicObject;
Expand Down Expand Up @@ -321,7 +323,7 @@ private void SetValueOnAttach(Characteristic characteristic, object value)
if (characteristic.HasChildCharacteristics)
{
// DONTTOUCH: workaround on case there were no parent characteristic.
var characteristicObject = (CharacteristicObject)GetValue(characteristic);
var characteristicObject = (CharacteristicObject?)GetValue(characteristic);
characteristicObject?.DetachFromOwner(characteristic);
}

Expand Down Expand Up @@ -358,13 +360,13 @@ private CharacteristicObject ApplyCore(
{
if (!HasValue(characteristic))
{
var characteristicObject = (CharacteristicObject)ResolveCore(characteristic, value);
var characteristicObject = (CharacteristicObject?)ResolveCore(characteristic, value);
if (characteristicObject != null)
{
value = Activator.CreateInstance(characteristicObject.GetType());
}

SetValueCore(characteristic, value);
SetValueCore(characteristic, value!);
}
}
else
Expand Down Expand Up @@ -399,20 +401,20 @@ protected CharacteristicObject UnfreezeCopyCore()
{
AssertIsRoot();

var newRoot = (CharacteristicObject)Activator.CreateInstance(GetType());
var newRoot = (CharacteristicObject)Activator.CreateInstance(GetType())!;
newRoot.ApplyCore(this);

// Preserve the IdCharacteristic of the original object
if (this.HasValue(IdCharacteristic))
{
newRoot.SetValue(IdCharacteristic, this.GetValue(IdCharacteristic));
newRoot.SetValue(IdCharacteristic, this.GetValue(IdCharacteristic)!);
}

return newRoot;
}
#endregion

public string Id => IdCharacteristic[this];
public string Id => IdCharacteristic[this]!;

public override string ToString() => Id;
}
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Characteristics/CharacteristicObject`1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public abstract class CharacteristicObject<T> : CharacteristicObject
{
protected CharacteristicObject() { }

protected CharacteristicObject(string? id) : base(id) { }
protected CharacteristicObject(string id) : base(id) { }

public new T Apply(CharacteristicObject other) => (T)ApplyCore(other);

Expand Down
16 changes: 9 additions & 7 deletions src/BenchmarkDotNet/Characteristics/CharacteristicPresenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Jobs;

#nullable enable

namespace BenchmarkDotNet.Characteristics
{
public abstract class CharacteristicPresenter
Expand All @@ -16,7 +18,7 @@ public abstract class CharacteristicPresenter

public abstract string ToPresentation(CharacteristicObject obj, Characteristic characteristic);

public abstract string ToPresentation(object characteristicValue, Characteristic characteristic);
public abstract string ToPresentation(object? characteristicValue, Characteristic characteristic);

private class DefaultCharacteristicPresenter : CharacteristicPresenter
{
Expand All @@ -26,11 +28,11 @@ public override string ToPresentation(CharacteristicObject obj, Characteristic c
return job.ResolvedId;

return obj.HasValue(characteristic)
? ToPresentation(characteristic[obj], characteristic)
? ToPresentation(characteristic[obj]!, characteristic)
: "Default";
}

public override string ToPresentation(object value, Characteristic characteristic)
public override string ToPresentation(object? value, Characteristic characteristic)
{
if (!(value is string) && value is IEnumerable collection)
return ToPresentation(collection);
Expand Down Expand Up @@ -64,7 +66,7 @@ private static string ToPresentation(IEnumerable collection)
return buffer.ToString();
}

private static string ToPresentation(object value)
private static string ToPresentation(object? value)
=> (value as IFormattable)?.ToString(null, DefaultCultureInfo.Instance)
?? value?.ToString()
?? "";
Expand All @@ -77,11 +79,11 @@ private class SourceCodeCharacteristicPresenter : CharacteristicPresenter
public override string ToPresentation(CharacteristicObject obj, Characteristic characteristic)
=> ToPresentation(characteristic[obj], characteristic);

public override string ToPresentation(object characteristicValue, Characteristic characteristic)
public override string ToPresentation(object? characteristicValue, Characteristic characteristic)
{
// TODO: DO NOT hardcode Characteristic suffix
string id = characteristic.Id;
string type = characteristic.DeclaringType.FullName;
string type = characteristic.DeclaringType.FullName!;
string value = SourceCodeHelper.ToSourceCode(characteristicValue);
return $"{type}.{id}Characteristic[job] = {value}";
}
Expand All @@ -94,7 +96,7 @@ public override string ToPresentation(CharacteristicObject obj, Characteristic c
? ToPresentation(characteristic[obj], characteristic)
: "Default";

public override string ToPresentation(object characteristicValue, Characteristic characteristic)
public override string ToPresentation(object? characteristicValue, Characteristic characteristic)
=> FolderNameHelper.ToFolderName(characteristicValue);
}
}
Expand Down
18 changes: 10 additions & 8 deletions src/BenchmarkDotNet/Characteristics/Characteristic`1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
using System;
using System.Diagnostics.CodeAnalysis;

#nullable enable

namespace BenchmarkDotNet.Characteristics
{
public class Characteristic<[DynamicallyAccessedMembers(CharacteristicObject.CharacteristicMemberTypes)] T> : Characteristic
{
internal Characteristic(
string id,
Type declaringType,
Func<CharacteristicObject, T, T>? resolver,
T fallbackValue,
Func<CharacteristicObject, T?, T>? resolver,
T? fallbackValue,
bool ignoreOnApply,
bool dontShowInSummary = false)
: base(id, typeof(T), declaringType, fallbackValue, ignoreOnApply, dontShowInSummary)
Expand All @@ -18,22 +20,22 @@ internal Characteristic(
FallbackValue = fallbackValue;
}

private Func<CharacteristicObject, T, T>? Resolver { get; }
private Func<CharacteristicObject, T?, T>? Resolver { get; }

public T FallbackValue { get; }
public T? FallbackValue { get; }

public new T this[CharacteristicObject obj]
public new T? this[CharacteristicObject obj]
{
get { return obj.GetValue(this); }
set { obj.SetValue(this, value); }
}

internal override object ResolveValueCore(CharacteristicObject obj, object currentValue)
internal override object? ResolveValueCore(CharacteristicObject obj, object currentValue)
{
if (Resolver == null)
return (T)base.ResolveValueCore(obj, currentValue);
return (T?)base.ResolveValueCore(obj, currentValue);

return Resolver(obj, (T)base.ResolveValueCore(obj, currentValue));
return Resolver(obj, (T?)base.ResolveValueCore(obj, currentValue));
}
}
}
Loading