-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Context
This is the largest single refactoring step in the model layer and must be delivered as one atomic change — model changes, parser changes, and emitter updates are all interdependent.
Current model structure
The key part model currently has two separate sealed record types that share near-identical fields:
// PropertyKeyPart.cs
public sealed record PropertyKeyPart(
PropertySpec Property,
string? Format,
ParseType ParseType,
FormatType FormatType
) : ValueKeyPart;
// RepeatingPropertyKeyPart.cs
public sealed record RepeatingPropertyKeyPart(
PropertySpec Property,
char Separator,
string? Format,
ParseType InnerParseType,
FormatType InnerFormatType,
TypeRef InnerType
) : ValueKeyPart;Both inherit from ValueKeyPart : KeyPart, which carries LengthRequired (int) and ExactLengthRequirement (bool). PropertyKeyPart uses computed values from PropertyValidation.GetFormattedLength(), while RepeatingPropertyKeyPart always hardcodes LengthRequired = 1 and ExactLengthRequirement = false — this is set explicitly in the Parser (lines 419-420). In the unified model, these values will be determined by whether CollectionSemantics is present.
ParseType and FormatType
ParseType and FormatType are separate enums whose variants currently mirror each other:
ParseType |
FormatType |
|---|---|
Guid |
Guid |
String |
String |
Enum |
Enum |
SpanParsable |
SpanFormattable |
Today, the Parser always assigns these in lockstep (e.g., if ParseType = Guid then FormatType = Guid). However, this is an artifact of the current supported type set — not a fundamental constraint. A user type could implement ISpanParsable<T> without implementing ISpanFormattable (currently rejected as an error), and future work to support IParsable<T> or IFormattable independently would break the 1:1 pairing (e.g., ParseType.SpanParsable with a regular FormatType.Formattable).
For this reason, the new PropertyTypeDescriptor must keep ParseType and FormatType as separate fields — they describe independent capabilities that happen to be paired today but may diverge.
Parser duplication
The Parser has two local functions — ToPropertyKeyPart (lines 232-326, ~95 lines) and ToRepeatingPropertyKeyPart (lines 328-422, ~95 lines) — that are approximately ~60% duplicated (~55-60 shared lines per method). The shared logic includes: property validation, interface detection, PropertyTypeInfo construction, format validation, type compatibility validation, and the ParseType/FormatType determination switch. The unique parts that differ are:
- Collection type validation:
ToPropertyKeyPartrejects collection types (CollectionType != None);ToRepeatingPropertyKeyPartrequires them (CollectionType == Noneis an error) - Type symbol extraction:
ToPropertyKeyPartuses the property'stypeSymboldirectly;ToRepeatingPropertyKeyPartcasts toINamedTypeSymboland extractsTypeArguments[0]as the inner type - Length calculation:
ToPropertyKeyPartcallsPropertyValidation.GetFormattedLength()and uses the result;ToRepeatingPropertyKeyParthardcodesLengthRequired = 1/ExactLengthRequirement = false - Return type construction: Different record types with different field names (
ParseTypevsInnerParseType, etc.)
ISpanParsable/ISpanFormattable detection is duplicated 4 times
The interface detection logic for ISpanParsable and ISpanFormattable appears in four places:
- Parser
ToPropertyKeyPart(lines 257-259) — usesSymbolDisplayFormat.FullyQualifiedFormat, producing"global::System.ISpanParsable<...>", matched viaStartsWith - Parser
ToRepeatingPropertyKeyPart(lines 355-357) — same format as above - PropertyAnalyzer
CreateInnerTypeInfo(lines 291-293) — uses defaultToDisplayString(), producing"System.ISpanParsable<...>", matched viaStartsWith - PropertyAnalyzer
CreatePropertyTypeInfo(lines 318-320) — same format as the analyzer method above
The Parser and Analyzer use different display formats for the same logical check. The Parser uses SymbolDisplayFormat.FullyQualifiedFormat which produces the global:: prefix ("global::System.ISpanParsable"), while the Analyzer uses the default display format producing "System.ISpanParsable". Both work because they use StartsWith (to handle the generic arity), but they are inconsistent. A shared helper method created during this unification should normalize the approach.
Equality implications
ImmutableEquatableArray<KeyPart> uses SequenceEqual for comparison, which requires each KeyPart element to implement IEquatable<T>. C# record types satisfy this automatically via compiler-generated structural equality. Since CollectionSemantics will also be a record, its nullable presence on the unified PropertyKeyPart will participate correctly in structural equality. Similarly, PropertyTypeDescriptor as a record will get correct equality for free. The existing SourceGeneratorIncrementalTests verify that incremental caching still works after model changes.
Blocked by
Tasks
Unify PropertyKeyPart and RepeatingPropertyKeyPart
- Create a new
CollectionSemanticsrecord:record CollectionSemantics(char Separator, TypeRef InnerType) - Replace
PropertyKeyPartandRepeatingPropertyKeyPartwith a single unifiedPropertyKeyPartcarrying aCollectionSemantics?field (null = single property, non-null = repeating) - For repeating properties,
LengthRequiredis always 1 andExactLengthRequirementis always false — set by parser based onCollectionSemanticspresence - Verify record equality works correctly with nullable
CollectionSemantics(structural comparison) — theImmutableEquatableArray<KeyPart>collection depends on this - Update all pattern matches/switch expressions throughout the Emitter that currently distinguish
PropertyKeyPartvsRepeatingPropertyKeyPart(there are 20+ match sites inSourceGenerator.Emitter.cs)
Merge ParseType and FormatType into PropertyTypeDescriptor
- Create
record PropertyTypeDescriptor(ParseType ParseType, FormatType FormatType)— these are kept as separate fields to support future independent parse/format capability (see Context) - Replace the separate
ParseType/FormatTypefields on oldPropertyKeyPartand theInnerParseType/InnerFormatTypeon oldRepeatingPropertyKeyPartwith a singleTypeDescriptorproperty - Update all Emitter switch sites that match on
ParseTypeorFormatTypeto useTypeDescriptor.ParseType/TypeDescriptor.FormatType
Unify Parser type resolution
- Merge
ToPropertyKeyPart()(lines 232-326) andToRepeatingPropertyKeyPart()(lines 328-422) into a single method — these are ~60% duplicated (~55-60 shared lines per 95-line method). The differences are:- Collection type validation (single checks no collection; repeating checks collection present)
- Type symbol extraction (single uses
typeSymboldirectly; repeating extracts inner type from genericTypeArguments[0]) - Length calculation (single calls
GetFormattedLength(); repeating hardcodesLengthRequired = 1/ExactLengthRequirement = false) PropertyTypeInfoconstruction (single uses property type name; repeating uses inner type name)
- Consolidate the duplicated
PropertyTypeInfoconstruction and ISpanParsable/ISpanFormattable interface detection (currently implemented 4 times: Parser lines 257-259, Parser lines 355-357, PropertyAnalyzer lines 291-293, PropertyAnalyzer lines 318-320). Extract a shared helper method that normalizes the display format difference between Parser (FullyQualifiedFormatproducing"global::System.ISpanParsable") and Analyzer (default display producing"System.ISpanParsable")
Verification
- All snapshot tests pass (generated output identical)
- All
SourceGeneratorIncrementalTestspass (incremental caching via structural equality still works) - All functional tests pass (parse/format round-trips)
- Run benchmarks to check for regressions