-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Description
The generated ToPartitionKeyString(int throughPartIndex, bool includeTrailingDelimiter = true) method has incomplete switch coverage when the partition key has a single component.
The switch only handles (0, false). Since the default value of includeTrailingDelimiter is true, calling ToPartitionKeyString(0) hits the _ default case and throws InvalidOperationException.
The generator should emit a (0, true) case that appends the PrimaryKeySeparator to the partition value, similar to how it handles multi-component partition keys in other composite keys.
Reproduction
Visible in the snapshot baseline:
SourceGeneratorSnapshotTests.RepeatingPropertyCompositeKey_SourceOutput#UnitTests.UserTagKey.g.verified.cs (lines ~54-61)
Expected behaviour
ToPartitionKeyString(0) should return the first partition component with the trailing delimiter appended, consistent with multi-component partition key behaviour.
The throwing behaviour for (lastIndex, true) is intentional — there is no following delimiter to include, so the combination is invalid. However:
-
The documentation on
IPrimaryKey<TSelf>.ToPartitionKeyStringandICompositePrimaryKey<TSelf>.ToSortKeyStringdoes not communicate this. The param doc says "Whether to include the following delimiter character in the formatted string" but does not explain what happens when there is no following delimiter (i.e. whenthroughPartIndexrefers to the last component). -
The default value of
includeTrailingDelimiter = truemakes the most natural call pattern (ToPartitionKeyString(0)) a guaranteed throw for single-component keys, andToPartitionKeyString(lastIndex)a throw for all keys. The default should be changed tofalsefor safety, so that callers who omit the parameter get the value through the specified index without an unexpected exception.
Both changes constitute a breaking change to the shipped public API (PublicAPI.Shipped.txt).
Discovered via
Snapshot testing added in #48 — this is a pre-existing generator issue, not a regression.