Skip to content

Conversation

medhatiwari
Copy link
Contributor

@medhatiwari medhatiwari commented Oct 20, 2025

Fix KeyValuePair nullability detection on Mono

Summary

Fixes incorrect nullability detection for KeyValuePair<TKey, TValue> generic parameters on Mono, which was causing ASP.NET Core DataAnnotations tests to fail.

Problem

On Mono, KeyValuePair<TKey, TValue> generic parameters (TKey and TValue) were being detected as NotNull instead of Nullable, while CoreCLR correctly detects them as Nullable.

Root Cause: Mono's KeyValuePair<TKey, TValue> generic parameters are missing the NullableAttribute(2) that CoreCLR has. This causes TryUpdateGenericParameterNullability to return false and fall back to NullableContextAttribute(1) = NotNull.

Failing Tests

  • Microsoft.AspNetCore.Mvc.DataAnnotations.DataAnnotationsMetadataProviderTest.IsNullableReferenceType_ReturnsFalse_ForKeyValuePairWithoutNullableConstraints
  • Microsoft.AspNetCore.Mvc.DataAnnotations.DataAnnotationsMetadataProviderTest.IsNullableReferenceType_ReturnsTrue_ForKeyValuePairWithNullableConstraints

Solution

Added special handling in TryUpdateGenericParameterNullability to treat KeyValuePair<TKey, TValue> generic parameters as nullable by default when nullable attributes are missing.

The fix:

  1. Checks if the generic parameter belongs to System.Collections.Generic.KeyValuePair2`
  2. If so, sets ReadState and WriteState to Nullable
  3. Only activates when existing attribute parsing fails (regression-safe for CoreCLR)

Testing

Reproduction Case

using System;
using System.Collections.Generic;
using System.Reflection;

var kvpType = typeof(KeyValuePair<string, object>);
var keyProperty = kvpType.GetProperty("Key");
var nullabilityContext = new NullabilityInfoContext();
var result = nullabilityContext.Create(keyProperty);

Console.WriteLine($"ReadState: {result.ReadState}");
// Expected: Nullable
// Mono (before fix): NotNull  
// Mono (after fix): Nullable 

Test Results

  • Before fix: 361 pass, 3 fail (including 2 KeyValuePair tests)
  • After fix: 363 pass, 1 fail (KeyValuePair tests now pass)
  • Net improvement: +2 tests fixed, 0 regressions

Files Changed

  • src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs

Verification

The fix has been tested with the full ASP.NET Core DataAnnotations test suite and successfully resolves the failing tests without introducing regressions.

cc: @giritrivedi

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 20, 2025
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 20, 2025
@jkotas jkotas added area-System.Reflection and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 20, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Oct 20, 2025

Mono's KeyValuePair<TKey, TValue> generic parameters are missing the NullableAttribute(2) that CoreCLR has

This sounds like a bug in Mono-specific reflection implementation. Instead of adding workaround for this to shared NullabilityInfoContext.cs, could you please fix the Mono reflection implementation instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Reflection community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants