Skip to content

Commit fd9ef6e

Browse files
Add BuildPropertiesForBackCompatibility API, align docs with main
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/e014a173-0713-44e4-a0b5-4ce8f896d407 Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
1 parent d6d7c2a commit fd9ef6e

3 files changed

Lines changed: 84 additions & 35 deletions

File tree

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -536,15 +536,6 @@ protected internal override PropertyProvider[] BuildProperties()
536536
continue;
537537
}
538538

539-
// Backcompat fix for property types: if a property existed in the last contract
540-
// with a compatible but non-identical type, retain the previous type to avoid
541-
// breaking consumers of the library.
542-
if (TryGetLastContractPropertyTypeOverride(outputProperty, out var lastContractPropertyType))
543-
{
544-
outputProperty.Type = lastContractPropertyType.ApplyInputSpecProperty(property);
545-
CodeModelGenerator.Instance.Emitter.Info($"Changed property {Name}.{outputProperty.Name} type to {lastContractPropertyType} to match last contract.");
546-
}
547-
548539
if (!isDiscriminator)
549540
{
550541
var derivedProperty = InputDerivedProperties.FirstOrDefault(p => p.Value.ContainsKey(property.Name)).Value?[property.Name];
@@ -582,7 +573,7 @@ protected internal override PropertyProvider[] BuildProperties()
582573
properties.AddRange(AdditionalPropertyProperties);
583574
}
584575

585-
return [.. properties];
576+
return [.. BuildPropertiesForBackCompatibility(properties)];
586577
}
587578

588579
private IEnumerable<InputModelType> EnumerateBaseModels()
@@ -1267,8 +1258,9 @@ _ when type.Equals(_additionalPropsUnknownType, ignoreNullable: true) => type,
12671258
}
12681259

12691260
/// <summary>
1270-
/// Determines whether the type of a generated property should be replaced with the type
1271-
/// of the matching property in the last contract to preserve backward compatibility.
1261+
/// Rewrites property types so that, when a property exists in the last contract with a
1262+
/// compatible but non-identical type, the previous type is preserved. This avoids
1263+
/// source-breaking changes for consumers of the library.
12721264
/// </summary>
12731265
/// <remarks>
12741266
/// The override is applied when the generated and last-contract property types differ but
@@ -1286,22 +1278,31 @@ _ when type.Equals(_additionalPropsUnknownType, ignoreNullable: true) => type,
12861278
/// <description>
12871279
/// All other public properties whose top-level type name (including any generic
12881280
/// argument names) matches the last contract. This handles cases such as a property
1289-
/// previously generated as a nullable scalar being regenerated as non-nullable, or a
1290-
/// property whose type is sourced from a different assembly but represents the same
1291-
/// logical type.
1281+
/// previously generated as a nullable scalar being regenerated as non-nullable.
12921282
/// </description>
12931283
/// </item>
12941284
/// </list>
12951285
/// </remarks>
1296-
/// <param name="outputProperty">The property generated from the current input spec.</param>
1297-
/// <param name="lastContractPropertyType">
1298-
/// When the method returns <c>true</c>, the type from the last contract that should be
1299-
/// used to override the generated property type.
1300-
/// </param>
1301-
/// <returns>
1302-
/// <c>true</c> if the generated property type should be replaced with the last contract's
1303-
/// type; otherwise <c>false</c>.
1304-
/// </returns>
1286+
protected internal override IReadOnlyList<PropertyProvider> BuildPropertiesForBackCompatibility(IEnumerable<PropertyProvider> originalProperties)
1287+
{
1288+
var properties = originalProperties as IReadOnlyList<PropertyProvider> ?? [.. originalProperties];
1289+
if (LastContractPropertiesMap.Count == 0)
1290+
{
1291+
return properties;
1292+
}
1293+
1294+
foreach (var outputProperty in properties)
1295+
{
1296+
if (TryGetLastContractPropertyTypeOverride(outputProperty, out var lastContractPropertyType))
1297+
{
1298+
outputProperty.Type = lastContractPropertyType.ApplyInputSpecProperty(outputProperty.InputProperty);
1299+
CodeModelGenerator.Instance.Emitter.Info($"Changed property {Name}.{outputProperty.Name} type to {lastContractPropertyType} to match last contract.");
1300+
}
1301+
}
1302+
1303+
return properties;
1304+
}
1305+
13051306
private bool TryGetLastContractPropertyTypeOverride(
13061307
PropertyProvider outputProperty,
13071308
[NotNullWhen(true)] out CSharpType? lastContractPropertyType)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,16 @@ protected internal virtual IReadOnlyList<MethodProvider> BuildMethodsForBackComp
613613
protected internal virtual IReadOnlyList<ConstructorProvider> BuildConstructorsForBackCompatibility(IEnumerable<ConstructorProvider> originalConstructors)
614614
=> [.. originalConstructors];
615615

616+
/// <summary>
617+
/// Called from <see cref="BuildProperties"/> to apply backward-compatibility adjustments
618+
/// to the set of properties produced for this type. Overrides can replace, reorder, or
619+
/// otherwise rewrite properties based on the <see cref="LastContractView"/>.
620+
/// </summary>
621+
/// <param name="originalProperties">The properties as produced from the current input spec.</param>
622+
/// <returns>The possibly-adjusted list of properties.</returns>
623+
protected internal virtual IReadOnlyList<PropertyProvider> BuildPropertiesForBackCompatibility(IEnumerable<PropertyProvider> originalProperties)
624+
=> [.. originalProperties];
625+
616626
private IReadOnlyList<EnumTypeMember>? _enumValues;
617627

618628
private bool ShouldGenerate(ConstructorProvider constructor)

packages/http-client-csharp/generator/docs/backward-compatibility.md

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
- [Parameter Naming](#parameter-naming)
1515
- [Page Size Parameter Casing Correction](#scenario-page-size-parameter-casing-correction)
1616
- [Top Parameter Conversion to MaxCount](#scenario-top-parameter-conversion-to-maxcount)
17+
- [Content-Type Parameter Ordering](#content-type-parameter-ordering)
18+
- [Content-Type Before Body Preserved from Last Contract](#scenario-content-type-before-body-preserved-from-last-contract)
1719

1820
## Overview
1921

@@ -110,15 +112,7 @@ public static PublicModel1 PublicModel1(
110112

111113
### Model Properties
112114

113-
The generator attempts to maintain backward compatibility for all public model property types by preserving the previous property type when it differs from the type that would be generated from the current spec but is still logically compatible.
114-
115-
The following scenarios are supported for any public model property:
116-
117-
- **Collection wrapper change:** A property previously generated as a read-only collection (e.g. `IReadOnlyList<T>` / `IReadOnlyDictionary<TKey, TValue>`) that is now produced as a read-write collection (e.g. `IList<T>` / `IDictionary<TKey, TValue>`), or vice versa. The collection element/key/value type names must still match – the wrapper is preserved but genuine element-type changes are honoured.
118-
- **Nullability change:** A scalar, enum, or model property that was previously generated as nullable (e.g. `int?`, `StatusEnum?`) and is now produced as non-nullable (or vice versa). The last contract's nullability is preserved when the top-level type name (and any generic argument names) still matches.
119-
- **Same-name types from different sources:** Properties whose generated type is logically the same as the last contract's type, but sourced from different assemblies (e.g. a `TypeProvider`-produced type vs. a compiled-assembly type). Equality is evaluated by name rather than identity so these are treated as the same type.
120-
121-
The override is **not** applied when the top-level property type names differ (for example a property that changed from `string` to `int`) – such changes are passed through so that the new spec is honoured.
115+
The generator attempts to maintain backward compatibility for model property types by preserving the previous property type whenever it differs from the type produced by the current spec but is still logically compatible. This applies to all public model properties (scalars, enums, models, and collections).
122116

123117
#### Scenario: Collection Property Type Changed
124118

@@ -171,8 +165,8 @@ public int? Count { get; set; }
171165
**Implementation Details:**
172166

173167
- The generator compares property types against the `LastContractView`.
174-
- For read-write lists and dictionaries, if the previous type was different but the element/key/value type names match, the previous type is retained.
175-
- For all other properties, if the previous type's top-level name (including any generic argument names) matches the new type's top-level name, the previous type is retained – this covers nullability changes and same-name types sourced from different assemblies.
168+
- If the previous type is logically compatible with the new type (same top-level name, with matching generic argument names), the previous type is retained. This covers read-write collection wrapper changes (e.g. `IList<T>``IReadOnlyList<T>`) as well as nullability changes on scalars, enums, and models.
169+
- If the top-level property type names differ entirely (for example `string``int`), the new spec is honoured and no override is applied.
176170
- A diagnostic message is logged: `"Changed property {ModelName}.{PropertyName} type to {LastContractType} to match last contract."`
177171

178172
### AdditionalProperties Type Preservation
@@ -623,3 +617,47 @@ public virtual AsyncPageable<Item> GetItemsAsync(int? maxCount = null, Cancellat
623617
- This conversion is specific to paging operations only
624618
- Existing client code with `top` continues to compile without changes
625619
- New code benefits from the standardized `maxCount` naming convention
620+
621+
### Content-Type Parameter Ordering
622+
623+
The generator places the `contentType` parameter after the body (`content`) parameter in method signatures. However, backward compatibility is maintained when the last contract had a different ordering.
624+
625+
#### Scenario: Content-Type Before Body Preserved from Last Contract
626+
627+
**Description:** The generator places `contentType` after the `content` (body) parameter. However, if the last contract had `contentType` before `content`, the generator preserves that ordering to avoid breaking existing code.
628+
629+
This commonly occurs when a library was previously generated with contentType before body and has already been released (GA'd).
630+
631+
**Example:**
632+
633+
**contentType before body exists in LastContractView - preserved for backward compatibility**
634+
635+
Previous version had `contentType` before `content`:
636+
637+
```csharp
638+
public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string contentType, BinaryContent content, RequestOptions options = null)
639+
{
640+
// ...
641+
}
642+
```
643+
644+
Current TypeSpec defines a content type:
645+
646+
```typespec
647+
op UpdateSkillDefaultVersion(
648+
@path skill_id: string,
649+
@header contentType: string,
650+
@body body: SetDefaultSkillVersionBody,
651+
): SkillResource;
652+
```
653+
654+
**Generated Compatibility Result:**
655+
656+
The generator detects that the previous contract had `contentType` before `content` and preserves that ordering:
657+
658+
```csharp
659+
public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string contentType, BinaryContent content, RequestOptions options = null)
660+
{
661+
// contentType stays before content for backward compatibility
662+
}
663+
```

0 commit comments

Comments
 (0)