feat: support C# 11 required modifier during validation (Closes #76)

[Moha-sami]

Summary

Fixes#76. Adds support for C# 11's required modifier (RequiredMemberAttribute) in TypeDetailsCache.cs so properties marked with required are automatically treated as required during MiniValidator.TryValidate.

Includes new unit tests covering required property validation across .NET 8, 9, and 10.

[DamianEdwards]

The cache-layer approach makes sense, but the current implementation changes validation semantics beyond the behavior requested in #76. In particular, nullable required members become invalid, explicit TypeDescriptor validation can be overridden, and unrelated same-named attributes are treated as compiler metadata.

[DamianEdwards]

Could this compare the fully qualified name to System.Runtime.CompilerServices.RequiredMemberAttribute instead? Matching only Type.Name means an unrelated user-defined attribute named RequiredMemberAttribute in any namespace will silently add required validation.

[DamianEdwards]

Please defer this decision until after all attribute sources have been processed. TypeDescriptor attributes are concatenated after the property attributes, so a RequiredAttribute supplied there (for example, with AllowEmptyStrings = true) arrives too late to prevent this default attribute from being added. Both attributes then run and the explicit validation behavior cannot prevail.

[DamianEdwards]

This should not synthesize [Required] for nullable members. The C# required modifier requires initialization, not a non-null value, so required string? Name and required int? Value can validly be initialized to null. Issue #76 explicitly calls out that nullable required members should be ignored, while the new test currently locks in the opposite behavior. Please gate this using nullability metadata and cover both nullable reference and value types.

[Moha-sami]

Hi @DamianEdwards, thanks a lot for the thorough review and great feedback!

I've updated the PR implementation based on your comments:

1. Full Type Name Matching: Switched to checking attr.GetType().FullName == "System.Runtime.CompilerServices.RequiredMemberAttribute" to prevent false positives on same-named attributes in custom namespaces.
2. Deferred Decision: Deferred adding the synthesized [Required] attribute until after all attribute sources (including TypeDescriptor attributes) have been processed.
3. Nullability Gating: Used NullabilityInfoContext to ensure [Required] is only synthesized for non-nullable required members. Nullable required members (required string? and required int?) are now preserved as valid when null.

Added unit tests covering all these edge cases across .NET 8, 9, and 10. All 276 unit tests are passing cleanly!

[DamianEdwards]

The previous netstandard2.0 compile failure and the original attribute identity/ordering concerns are addressed. Three correctness issues remain in the updated nullability implementation: non-nullable value types affect RequiresValidation, the shared nullability context is unsafe under concurrent cache initialization, and the netstandard2.0 fallback misses ambient nullable context metadata. Coverage for explicit RequiredAttribute options/subclasses retaining precedence would also be useful, but that is not the blocker here.

[DamianEdwards]

Please exclude non-nullable value types here. required int cannot be null, so adding [Required] does not change instance validation, but it does make the public RequiresValidation API return true for a type that otherwise has nothing to validate. Issue #76 explicitly says required value types should be ignored; a regression test for required int would catch this.

[DamianEdwards]

A shared NullabilityInfoContext is not safe here because this cache can initialize details for different types concurrently, while NullabilityInfoContext mutates internal caches without synchronization. Concurrent first use can throw (for example, duplicate-key ArgumentException). Please create a context per lookup or synchronize access to this instance.

[DamianEdwards]

This fallback misses nullable annotations represented by NullableContextAttribute on the declaring type (or an enclosing type/module) rather than a property-level NullableAttribute. Roslyn can encode required string? that way, so the netstandard2.0 target will classify it as non-nullable and reject null. Please resolve the ambient nullable context as well as an explicit property flag, and add a test that exercises this path.

[Moha-sami]

Hi @DamianEdwards, thanks again for the sharp review!

I've pushed an update addressing all three points:

1. Value Types Excluded: required value types (required int, required int?) are now explicitly excluded from synthesizing [Required] so RequiresValidation returns false for types containing only value types.
2. Thread Safety: NullabilityInfoContext is now instantiated per lookup inside IsReferenceTypeNullable to eliminate concurrency issues during parallel cache initialization.
3. Ambient Nullability on netstandard2.0: The netstandard2.0 fallback now inspects NullableContextAttribute on declaring and enclosing types when property-level annotations are omitted.

All 279 unit tests across .NET 8, 9, and 10 are passing cleanly!

[DamianEdwards]

All findings from the previous rounds are addressed. One cross-target correctness issue remains: the netstandard2.0 nullable-metadata fallback does not honor nullable flow annotations, so it can apply stricter validation than the .NET 8 asset. The inline comment includes a suggested implementation and coverage approach.

[DamianEdwards]

The netstandard2.0 path still differs from NullabilityInfoContext for nullable flow annotations. For example, [AllowNull] public required string Value has nullable write state, and [MaybeNull] has nullable read state; this fallback currently sees neither and synthesizes [Required], so the same model validates differently depending on which target asset is loaded.

Please mirror the modern path before falling back to NullableAttribute/NullableContextAttribute: detect System.Diagnostics.CodeAnalysis.AllowNullAttribute for the write side and System.Diagnostics.CodeAnalysis.MaybeNullAttribute for the read side, inspecting the property and the setter value/getter return metadata that NullabilityInfoContext considers. Comparing full type names keeps this compatible where those attribute types are not directly referenceable. If either side permits null, return true.

Please also cover both annotations through the fallback path. Since the current test project targets only .NET 8+, it always selects the NullabilityInfoContext branch; either exercise the netstandard2.0 asset from a consumer test or extract the metadata parser into target-independent code that can be tested directly.

[Moha-sami]

Hi @DamianEdwards, thanks for catching that!

I've pushed an update (ca6b644) addressing the flow annotations and fallback testing:

1. Flow Attribute Detection: Added HasNullableFlowAttribute in IsReferenceTypeNullableFallback to detect [AllowNull] and [MaybeNull] annotations on properties, getter return parameters, and setter value parameters by full name (System.Diagnostics.CodeAnalysis.AllowNullAttribute and MaybeNullAttribute).
2. Target-Independent Testing: Extracted IsReferenceTypeNullableFallback so the netstandard2.0 fallback path is now directly unit tested on .NET 8+ to guarantee full parity with NullabilityInfoContext.

Added tests covering [AllowNull] and [MaybeNull] required properties. All 285 unit tests across .NET 8, 9, and 10 build and pass cleanly!