src/NLog/Internal/ConfigurationManager.cs (1)

51-53: ⚠️ Potential issue

Handle potential null return
ConfigurationManager.ConnectionStrings[name] can return null, conflicting with the non-nullable return type. Either update the return signature to ConnectionStringSettings? or add a null check with a descriptive exception.

src/NLog/Internal/CallSiteInformation.cs (2)

108-112: 🛠️ Refactor suggestion

Parameter nullability mismatch & redundant null-check

stackTrace is declared as a non-nullable parameter, yet the body immediately guards against it being null.
Either the method intends to accept null, or the check is dead code:

-public void SetStackTrace(StackTrace stackTrace, int? userStackFrame = null, Type? loggerType = null)
+public void SetStackTrace(StackTrace? stackTrace, int? userStackFrame = null, Type? loggerType = null)

Adjust the caller sites accordingly or drop the stackTrace != null check.
Having the signature and implementation aligned will avoid confusing warnings and needless branching.

---

113-118: ⚠️ Potential issue

Possible NullReference when GetFrames() returns null

StackTrace.GetFrames() may legitimately return null.
If that happens, FindCallingMethodOnStackTrace receives a null array and later dereferences .Length, leading to an exception.

-var stackFrames = stackTrace.GetFrames();
+var stackFrames = stackTrace?.GetFrames() ?? Array.Empty<StackFrame>();

Applying this guard keeps the downstream logic intact and eliminates the crash scenario.

src/NLog/Internal/ObjectReflectionCache.cs (1)

106-114: 🛠️ Refactor suggestion

objectPath should be nullable-annotated if you expect null

objectPath is declared non-nullable but the very first statement checks if (objectPath is null).
Either drop the test or make the intent explicit:

-public bool TryGetObjectProperty(object value, string[] objectPath, out object? foundValue)
+public bool TryGetObjectProperty(object value, string[]? objectPath, out object? foundValue)

Misaligned annotations defeat the purpose of nullable-reference-type adoption and may hide real defects.

src/NLog/Internal/DynamicallyAccessedMemberTypes.cs (4)

203-203: Mark Scope as nullable
Changing public string Scope { get; set; } to public string? Scope { get; set; } correctly reflects that this property is optional. Consider updating its XML <summary> to note that it may be null.

---

214-214: Mark Target as nullable
Updating public string Target { get; set; } to public string? Target { get; set; } aligns with nullable reference guidelines. Also update its documentation to indicate it can be null when not set.

---

226-226: Mark MessageId as nullable
The change from string to string? for MessageId is appropriate given its optional nature. Please adjust the XML <summary> to mention its nullability.

---

231-231: Mark Justification as nullable
Annotating Justification as string? correctly signals it may be omitted. Consider adding a note to the XML docs that it can be null.

src/NLog/Internal/SortHelpers.cs (2)

272-280: Avoid heap-allocating an empty Dictionary for the empty enumerator case

GetEnumerator() currently does:

return new Enumerator(new Dictionary<TKey, TValue>());

That allocates both a dictionary and its enumerator every time an empty instance is enumerated.
You can shave the allocation completely:

- else
-     return new Enumerator(new Dictionary<TKey, TValue>());
+ else
+     return default;   // struct enumerator with default(state==empty)

You already handle a default-constructed enumerator correctly (_multiBuckets == null & _singleBucketFirstRead == false).
A small but free performance win.

---

217-224: Enumerator re-wraps an already correct KeyValuePair – redundant copy

Inside Current for the multi-bucket path you create a new KeyValuePair from the existing one:

return new KeyValuePair<TKey, TValue>(_multiBuckets.Current.Key, _multiBuckets.Current.Value);

_multiBuckets.Current is already the desired KeyValuePair; the extra copy is unnecessary:

- return new KeyValuePair<TKey, TValue>(_multiBuckets.Current.Key, _multiBuckets.Current.Value);
+ return _multiBuckets.Current;

src/NLog/Internal/ISupportsInitialize.cs (1)

38-38: Standardize using directive placement
Moving using NLog.Config; inside the namespace matches the pattern adopted in other nullable-enabled files. Ensure this style is applied uniformly or consider consolidating using directives outside namespaces for consistency.

src/NLog/Internal/ReusableAsyncLogEventList.cs (1)

38-39: Using directives inside namespace
Relocating using System.Collections.Generic; and using NLog.Common; into the NLog.Internal namespace follows the pattern in other internal files. Verify that this style is consistently applied across the codebase.

src/NLog/Internal/SingleItemOptimizedHashSet.cs (1)

173-194: Duplicate null-checks sprinkled across methods

Contains and Remove each replicate the logic
Count == 1 && _singleItem is not null && Comparer.Equals(_singleItem, item).

Consider extracting this into a small helper (MatchesSingleItem(item)) to keep the code DRY and reduce the chance of future divergence.

src/NLog/Internal/CallSiteInformation.cs (2)

171-187: Minor: redundant null-coalescing & micro-optimisation

After assigning callerClassName = CallerClassName ?? string.Empty; the variable is never null.
Consequently, the later Substring branch can return callerClassName directly without the else, and the IsNullOrEmpty check can be replaced by Length != 0 for a tiny perf gain.
Purely optional, but it tightens the intent.

---

199-204: Redundant null-coalescing in method name retrieval

Here you already validated CallerMethodName is non-empty; the extra ?? string.Empty is impossible to reach.

-if (!string.IsNullOrEmpty(CallerMethodName))
-    return CallerMethodName ?? string.Empty;
+if (!string.IsNullOrEmpty(CallerMethodName))
+    return CallerMethodName;      // value cannot be null here

src/NLog/Internal/ObjectReflectionCache.cs (1)

701-708: Null-safe conversion when reflecting over generic KeyValuePair

keyString.ToString() again assumes non-null; use Convert.ToString(keyString, CultureInfo.InvariantCulture) or the null-conditional above to avoid surprises.

src/NLog/Internal/SortHelpers.cs (2)

112-118: ⚠️ Potential issue

default(KeyValuePair<…>) produces an invalid “phantom” bucket

When inputs.Count == 0 you construct the return value like this:

return new ReadOnlySingleBucketGroupBy<TKey, IList<TValue>>(default(KeyValuePair<TKey, IList<TValue>>), keyComparer);

default(KeyValuePair<TKey, IList<TValue>>) is not null; the nullable wrapper therefore gets a value with HasValue == true whose .Key is default(TKey) ( null for reference-type keys ).
Iteration over the result will thus yield a single element whose key violates the TKey : notnull contract and will surprise callers that expect an empty collection.

- return new ReadOnlySingleBucketGroupBy<TKey, IList<TValue>>(default(KeyValuePair<TKey, IList<TValue>>), keyComparer);
+ return new ReadOnlySingleBucketGroupBy<TKey, IList<TValue>>(singleBucket: null, keyComparer);

(You could also expose an explicit Empty singleton on the struct.)

---

173-181: ⚠️ Potential issue

Struct lost IReadOnlyDictionary semantics but the rest of the codebase still relies on them

ReadOnlySingleBucketGroupBy now implements only IEnumerable<KeyValuePair<…>>.
However, external code (see SortHelpersTests.cs in this PR and many callers in the repo) still uses:

• Keys, Values
• ContainsKey, TryGetValue
• indexer []
• CopyTo, Count (property), …

Because these members are no longer provided (and no extension methods cover them) the solution will not compile.

Two options:

1. Re-add IReadOnlyDictionary<TKey,TValue> (preferred – minimal churn for callers)

   struct ReadOnlySingleBucketGroupBy<TKey,TValue> :
       IReadOnlyDictionary<TKey,TValue>, IEnumerable<KeyValuePair<TKey,TValue>> where TKey : notnull

   and implement the missing members using the existing single/multi-bucket backing fields.

2. If the simplified surface is intentional, update all call-sites and unit tests in this PR accordingly (large scope).

Failing to address this will break the build.

src/NLog/Internal/SingleItemOptimizedHashSet.cs (1)

96-97: 🛠️ Refactor suggestion

Count misreports when default(T) is a legitimate element

Using EqualityComparer<T?>.Default.Equals(_singleItem, default(T)) treats the default value of T (e.g., 0, false, empty struct) as absence of an item.
Consequences:

• Count returns 0 after inserting 0 into a SingleItemOptimizedHashSet<int>.
• Contains, Remove, enumeration, etc., become inconsistent.

A safer pattern is to track presence explicitly (e.g., introduce a private bool _hasSingleItem) instead of over-loading the default value as a sentinel:

-private readonly T? _singleItem;
+private readonly T? _singleItem;
+private readonly bool _hasSingleItem;
...
-public int Count => _hashset?.Count ?? (_hasSingleItem ? 1 : 0);

That eliminates ambiguity without affecting the hash-set optimisation.

Committable suggestion skipped: line range outside the PR's diff.

src/NLog/Internal/ReflectionHelpers.cs (1)

68-69: ⚠️ Potential issue

LateBoundMethod should allow null for static invocations

The XML docs explicitly say target may be null for static methods, yet the delegate signature is object target (non-nullable under NRT).
Callers will now receive CS8600/CS8625 warnings every time they legitimately pass null.

-public delegate object? LateBoundMethod(object target, object[] arguments);
+public delegate object? LateBoundMethod(object? target, object[] arguments);

This preserves compile-time null-safety while honouring the documented contract.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        public delegate object? LateBoundMethod(object? target, object[] arguments);

src/NLog/Internal/ObjectGraphScanner.cs (1)

71-75: ⚠️ Potential issue

Possible NullReferenceException when rootObjects contains null

FindReachableObjects accesses rootObject.GetType() without a null-check, but later methods now handle null safely.
Either filter nulls here or adjust the loop:

-foreach (var rootObject in rootObjects)
+foreach (var rootObject in rootObjects.Where(o => o is not null))

or inside the loop:

if (rootObject is null)
    continue;

Prevents an exception path that unit tests might not cover.

src/NLog/Internal/ObjectReflectionCache.cs (1)

665-668: ⚠️ Potential issue

ToString() on potentially-null key

item.Key (or keyString) can be null. Calling .ToString() will crash.

-yield return new KeyValuePair<string, object?>(item.Key.ToString(), item.Value);
+yield return new KeyValuePair<string, object?>(item.Key?.ToString() ?? string.Empty, item.Value);

Same applies to the generic overload further down (lines 731-735 & 739-742).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            {
                foreach (var item in new DictionaryEntryEnumerable(dictionary))
                    yield return new KeyValuePair<string, object?>(item.Key?.ToString() ?? string.Empty, item.Value);
            }

src/NLog.Targets.ConcurrentFile/Internal/SortHelpers.cs (3)

83-85: Minor: avoid boxing a null into Nullable<> to save an allocation

Returning new ReadOnlySingleBucketGroupBy<TKey, IList<TValue>>(singleBucket: null, …) forces the compiler to allocate a boxed Nullable<KeyValuePair<…>> representing null. A default-constructed ReadOnlySingleBucketGroupBy already represents “no buckets”:

-return new ReadOnlySingleBucketGroupBy<TKey, IList<TValue>>(singleBucket: null, keyComparer);
+return default;   // zero-alloc shortcut – struct fields default to null

This is a micro-optimisation, but bucket-sort helpers may sit on hot paths.

---

241-249: Avoid allocating an empty Dictionary each time GetEnumerator() is called on an empty group

new Dictionary<TKey, TValue>() creates both a managed object and an internal array simply to provide an enumerator that immediately returns false. A lightweight alternative keeps allocations at zero:

-else
-    return new Enumerator(new Dictionary<TKey, TValue>());
+else
+    return default;   // Enumerator’s default struct value → MoveNext() == false

You can keep behaviour identical by adding a default constructor path in Enumerator that yields false on MoveNext().

---

142-152: Consider exposing IReadOnlyDictionary<TKey,TValue> for discoverability

After the refactor the struct implements only IEnumerable<KeyValuePair<…>>, yet it advertises a Count property and internally represents bucket mappings. Implementing IReadOnlyDictionary<TKey,TValue> (which is non-mutating) would:

1. Preserve the “dictionary-like” intent of the previous type.
2. Allow easy key look-ups (ContainsKey, TryGetValue) without re-enumeration.
3. Require minimal additional code because most members already exist internally.

If keeping the surface area minimal is intentional, feel free to ignore, but calling this out now avoids future “missing API” churn.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro