fix: Redundant [NotNullWhen(true)] on non-nullable string parameters in composite TryParse

[DrBarnabus]

Description

The generated composite key TryParse(string partitionKey, string sortKey, ..., out T? result) overload annotates the non-nullable string parameters with [NotNullWhen(true)], which has no effect on non-nullable reference types.

The single-key TryParse(string? s, ..., out T? result) overload correctly uses string? with [NotNullWhen(true)].

Either the composite overload should accept string? parameters (matching the single-key pattern), or the [NotNullWhen(true)] attribute should be removed from the non-nullable parameters.

Reproduction

Visible in the snapshot baseline:
SourceGeneratorSnapshotTests.ClashingKeyNames_SourceOutput#AnotherNamespace.BasicPrimaryKey.g.verified.cs (lines ~154-163)

Expected behaviour

Consistent nullability annotations: either use string? with [NotNullWhen(true)] (like the single-key overload) or use string without the attribute.

Discovered via

Snapshot testing added in #48 — this is a pre-existing generator issue, not a regression.

[DrBarnabus]

Investigation

Confirmed: Valid Issue

The issue is real and the reported behavior is accurate. The composite TryParse overload applies [NotNullWhen(true)] to a non-nullable string parameter, which has no semantic effect.

Root Cause

The issue is in a single location — SourceGenerator.Emitter.cs:266 inside WriteCompositeTryParseMethodImplementation():

public static bool TryParse([NotNullWhen(true)] string partitionKey, string sortKey, [MaybeNullWhen(false)] out T? result)

There are actually two problems on this line:

1. partitionKey is string (non-nullable) but annotated with [NotNullWhen(true)] — the attribute has no effect on a non-nullable type
2. sortKey is string (non-nullable) with no annotation at all — inconsistent with partitionKey

Meanwhile the method body still performs a null guard (if (partitionKey is null || sortKey is null)), which suggests the intent was always to accept nullable inputs.

For comparison, the single-key TryParse at line 97 is correct:

public static bool TryParse([NotNullWhen(true)] string? primaryKey, [MaybeNullWhen(false)] out T? result)

All other [NotNullWhen(true)] usages in the emitter are correct:

• Line 97: single-key TryParse — string? ✓
• Line 327: IParsable<T>.TryParse — string? ✓

Impact

• 5 snapshot baselines exhibit this pattern (all composite key types)
• No runtime correctness issue — the generated code works fine; the null guard handles null inputs regardless
• Static analysis impact — callers with nullable analysis enabled won't get the benefit of [NotNullWhen(true)] narrowing after a successful TryParse call on the composite overload

Categorization

Bug — incorrect nullability annotations in generated code. Low severity (cosmetic/analyzer-quality, no runtime impact).

Reproduction

Visible in any composite key snapshot baseline, e.g.:

SourceGeneratorSnapshotTests.BasicCompositePrimaryKey_SourceOutput#UnitTests.BasicPrimaryKey.g.verified.cs (line 202):

public static bool TryParse([NotNullWhen(true)] string partitionKey, string sortKey, [MaybeNullWhen(false)] out BasicPrimaryKey? result)
//                                               ^^^^^^                ^^^^^^
//                           should be string? with [NotNullWhen]    should be string? with [NotNullWhen]

Compared to the single-key overload in ClashingKeyNames (line 88):

public static bool TryParse([NotNullWhen(true)] string? primaryKey, [MaybeNullWhen(false)] out BasicPrimaryKey? result)
//                                               ^^^^^^^
//                                               correctly nullable

Proposed Fix

Make the composite overload consistent with the single-key pattern — change both parameters to string? and apply [NotNullWhen(true)] to both.

In SourceGenerator.Emitter.cs, line 266, change the generated signature from:

public static bool TryParse([NotNullWhen(true)] string partitionKey, string sortKey, [MaybeNullWhen(false)] out T? result)

to:

public static bool TryParse([NotNullWhen(true)] string? partitionKey, [NotNullWhen(true)] string? sortKey, [MaybeNullWhen(false)] out T? result)

This is preferred over simply removing the annotation because:

1. It matches the single-key TryParse pattern (accepts nullable, narrows on success)
2. The method body already handles null inputs gracefully
3. It gives callers proper nullability narrowing after a successful call

The fix is a one-line change in the emitter template, plus updating the 5 affected snapshot baselines.