fix: Emitter generates redundant length checks in Parse/TryParse methods

[DrBarnabus]

Description

The source generator emits redundant length validation in Parse and TryParse methods when a partition/sort key consists of a single value part (no split required). Two independent code generation paths — the global WriteLengthCheck guard and the per-property checks in WriteParsePropertiesImplementation — both emit length validation without coordinating, resulting in dead code.

Scenario 1: Duplicate exact-length check before Guid parsing

In the RepeatingPropertyCompositeKey scenario (UserTagKey), Parse validates partitionKey.Length != 36 at the top-level guard, then emits the identical partitionKey.Length != 36 check again before calling Guid.TryParseExact. The second check is always false at that point. Same pattern in TryParse.

Scenario 2: Minimum-length guard followed by dead zero-length check

In the InvariantCultureDisabled scenario (BasicPrimaryKey), Parse emits an early sortKey.Length < 1 guard that throws FormatException, then later checks sortKey.Length == 0 — which can never be true since the first guard already threw. The same pattern appears in TryParse.

Not actually redundant (corrected from original report)

The original report also listed the per-part Guid length check in BasicPrimaryKey for the partition key (partitionKey.Length < 40 followed by partitionKey[partitionKeyPartRanges[0]].Length != 38). This is not redundant — after splitting a multi-part key, an individual range can be shorter than expected even though the total input met the minimum length. The per-part check is valid in this case.

Reproduction

Visible in snapshot baselines:

• SourceGeneratorSnapshotTests.RepeatingPropertyCompositeKey_SourceOutput#UnitTests.UserTagKey.g.verified.cs
  • Parse: line 167 (partitionKey.Length != 36) duplicated at line 178
  • TryParse: line 218 (partitionKey.Length != 36) duplicated at line 229
• SourceGeneratorSnapshotTests.InvariantCultureDisabled_SourceOutput#UnitTests.BasicPrimaryKey.g.verified.cs
  • Parse: line 112 (sortKey.Length < 1) makes line 126 (sortKey.Length == 0) dead
  • TryParse: line 152 (sortKey.Length < 1) makes line 166 (sortKey.Length == 0) dead

Expected behaviour

The emitter should avoid emitting duplicate/redundant length checks. Each validation should appear exactly once in the generated code path. Per-property length checks should be suppressed when the input is unsplit and the global guard already covers the same condition.

Discovered via

Snapshot testing added in #48 — these are pre-existing generator issues, not regressions.

[DrBarnabus]

Investigation

Category

Code quality / optimization — the generated code is functionally correct but contains unnecessary redundancy.

Confirmed Scenarios

I've confirmed three distinct redundancy patterns in the emitter output. The issue description accurately identifies two of them, and there's a third related pattern:

---

1. Duplicate exact-length check on Guid partition key (RepeatingPropertyCompositeKey / UserTagKey)

Template: {UserId}|TAG_{Tags...#} where UserId is Guid (format "d" = 36 chars exact)

The partition key has a single property with an exact length requirement, so WriteLengthCheck emits partitionKey.Length != 36. Then WriteParsePropertiesImplementation hits the ParseType.Guid case which calls ToStrictLengthCheck and emits the identical check again:

// Parse (lines 167-178) — same pattern in TryParse (lines 218-229)
if (partitionKey.Length != 36)                                              // WriteLengthCheck
    throw new FormatException("Unrecognized format.");
// ... split logic ...
if (partitionKey.Length != 36 || !Guid.TryParseExact(partitionKey, "d", out var userId))  // ToStrictLengthCheck
    throw new FormatException("Unrecognized format.");

The second partitionKey.Length != 36 is always false at that point — it's dead code.

---

2. Redundant per-part Guid length check after minimum-length guard (InvariantCultureDisabled / BasicPrimaryKey)

Template: {FirstPart:B}#{SecondPart}|{ThirdPart} where FirstPart is Guid (format "b" = 38 chars exact), SecondPart is double (variable length)

Since SecondPart has variable length, WriteLengthCheck emits partitionKey.Length < 40 (minimum guard). Then the Guid case emits a per-part exact check on the split range:

// Parse (lines 109-120) — same pattern in TryParse (lines 149-160)
if (partitionKey.Length < 40)                                               // WriteLengthCheck (minimum)
    throw new FormatException("Unrecognized format.");
// ... split into partitionKeyPartRanges ...
if (partitionKey[partitionKeyPartRanges[0]].Length != 38 || !Guid.TryParseExact(...))  // ToStrictLengthCheck
    throw new FormatException("Unrecognized format.");

This one is not strictly dead code — after the split, the first range could theoretically be shorter than 38 chars even though the whole input is >= 40 (e.g. "X#<37-char-string>"). So in this specific case, the per-part check is actually useful and provides value. This scenario from the issue is not actually redundant.

---

3. Dead zero-length string check after minimum-length guard (InvariantCultureDisabled / BasicPrimaryKey)

Template: Same as above — sort key is {ThirdPart} where ThirdPart is string

The sort key has a single string property (min length 1), so WriteLengthCheck emits sortKey.Length < 1. Then ParseType.String emits sortKey.Length == 0:

// Parse (lines 112-126) — same pattern in TryParse (lines 152-166)
if (sortKey.Length < 1)                    // WriteLengthCheck
    throw new FormatException("Unrecognized format.");
// ...
if (sortKey.Length == 0)                   // ParseType.String case (line 479)
    throw new FormatException("Unrecognized format.");

sortKey.Length < 1 and sortKey.Length == 0 are semantically identical. The second check is dead code.

---

Root Cause

The emitter has two independent code generation paths that both validate input length, and they don't coordinate:

1. WriteLengthCheck (line 337) — emits a global guard at the top of Parse/TryParse based on the aggregate length requirements of all parts
2. Per-property checks in WriteParsePropertiesImplementation (line 438):
   • ToStrictLengthCheck (line 527) — unconditionally prepends input.Length != N || for any KeyPart with ExactLengthRequirement = true (Guid properties)
   • ParseType.String case (line 477) — unconditionally emits input.Length == 0

Neither path is aware of what the other has already emitted.

Impact

• Correctness: None — the redundant checks are logically harmless; they don't change behaviour
• Performance: Negligible — redundant comparisons on hot paths, but the branch predictor will handle them trivially
• Code quality: Low — generated code is slightly larger and harder to read than necessary
• Snapshot noise: Low — extra lines in verified baselines make diffs slightly noisier

Truly redundant scenarios (confirmed)

#	Scenario	Redundancy	Dead code?
1	Single exact-length Guid partition/sort key	Global != N then per-property != N on same input	Yes — identical check
3	Single string partition/sort key	Global < 1 then per-property == 0	Yes — equivalent check

Not actually redundant (correcting the issue)

#	Scenario	Why it's valid
2	Guid part in multi-part key with variable-length sibling	Global check is on total input length; per-part check is on the split range which could be shorter

Suggested Fix

Suppress the per-property length check when it's provably covered by the global guard. Two targeted changes in WriteParsePropertiesImplementation:

1. ToStrictLengthCheck for Guid: Skip the length prefix when the input variable is the unsplit key (i.e. parts.Count == 1) and the global check used exact length (parts.All(p => p.ExactLengthRequirement)). When the input is a split range (partitionKey[partRanges[i]]), always keep the per-part check since it validates the individual segment.

2. ParseType.String zero-length check: Skip it when the input variable is the unsplit key and parts.Count == 1, since the global WriteLengthCheck already guarantees Length >= 1.

Both conditions boil down to: if there's only one value part and no split, the global guard already covers it. This can be passed as a bool skipRedundantLengthCheck parameter derived from parts.Count == 1 (or more precisely, parts.OfType<ValueKeyPart>().Count() == 1 with no delimiters triggering a split).

This keeps the per-part checks for multi-part keys (where they validate individual segments after splitting) while eliminating them for single-part keys where the global guard is sufficient.