Skip to content

Fix C# constructor parameter name back compatibility #11606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Wei Hu (live1206) wants to merge 7 commits into from
+1,066 −129
Closed

Fix C# constructor parameter name back compatibility #11606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
34d99ec
fix(http-client-csharp): restore constructor parameter names
live1206 Aug 10, 2026
29a6927
chore: remove C# emitter changelog entry
live1206 Aug 10, 2026
4926a2f
fix(http-client-csharp): retain parameter matcher
live1206 Aug 10, 2026
511601b
fix(http-client-csharp): preserve constructor parameter semantics
live1206 Aug 10, 2026
872695c
fix(http-client-csharp): match nullable constructor overloads
live1206 Aug 10, 2026
17036ec
fix(http-client-csharp): match nested constructor types
live1206 Aug 10, 2026
8a5f57c
fix(http-client-csharp): reconcile constructor contracts early
live1206 Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 84 additions & 20 deletions .../http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,8 @@ protected internal override ConstructorProvider[] BuildConstructors()
{
if (_inputModel.IsUnknownDiscriminatorModel)
{
return [FullConstructor];
_initializationConstructor = FullConstructor;
return [_initializationConstructor];
}

// Build the standard single initialization constructor
Expand All @@ -742,7 +743,10 @@ protected internal override ConstructorProvider[] BuildConstructors()
: _inputModel.Usage.HasFlag(InputModelTypeUsage.Input)
? MethodSignatureModifiers.Public
: MethodSignatureModifiers.Internal;
var (constructorParameters, constructorInitializer) = BuildConstructorParameters(true);
var (constructorParameters, constructorInitializer) = BuildConstructorParameters(
true,
accessibility,
reconcileWithLastContract: true);

var constructor = new ConstructorProvider(
signature: new ConstructorSignature(
Expand All @@ -756,11 +760,14 @@ protected internal override ConstructorProvider[] BuildConstructors()
GetPropertyInitializers(true, parameters: constructorParameters)
},
this);
_initializationConstructor = constructor;

var constructors = new List<ConstructorProvider> { constructor };

// Add FullConstructor if parameters are different
if (!constructorParameters.SequenceEqual(FullConstructor.Signature.Parameters))
if (!ConstructorBackCompatHelper.HaveSameParameterIdentity(
constructorParameters,
FullConstructor.Signature.Parameters))
{
constructors.Add(FullConstructor);
}
Expand All @@ -775,6 +782,17 @@ protected internal override ConstructorProvider[] BuildConstructors()
return [.. constructors];
}

private ConstructorProvider? _initializationConstructor;
internal ConstructorProvider InitializationConstructor
{
get
{
_ = Constructors;
return _initializationConstructor
?? throw new InvalidOperationException($"Initialization constructor for '{Name}' was not built.");
}
}

/// <summary>
/// Restores previously-published public constructors that the current generation would otherwise
/// drop. The primary scenario is a previously required property becoming optional: the corresponding
Expand All @@ -800,14 +818,8 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors

foreach (var previousConstructor in previousConstructors)
{
if (!MethodSignatureHelper.IsPublicApi(previousConstructor.Signature.Modifiers))
{
continue;
}

var previousParameters = previousConstructor.Signature.Parameters;

if (BackCompatHelper.IsConstructorRemovalAcceptedInBaseline(this, previousConstructor.Signature))
if (!ConstructorBackCompatHelper.IsEligiblePreviousConstructor(this, previousConstructor))
{
continue;
}
Expand All @@ -834,8 +846,8 @@ protected internal override IReadOnlyList<ConstructorProvider> BuildConstructors

// If a constructor with the same parameters already exists - either still generated or
// supplied by custom code (which lives in the canonical view) - there is nothing to restore.
if (constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters))
|| CanonicalView.Constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters)))
if (constructors.Any(c => ConstructorBackCompatHelper.HaveSameParameterIdentity(c.Signature.Parameters, previousParameters))
|| CanonicalView.Constructors.Any(c => ConstructorBackCompatHelper.HaveSameParameterIdentity(c.Signature.Parameters, previousParameters)))
{
continue;
}
Expand Down Expand Up @@ -1174,7 +1186,10 @@ private IEnumerable<FieldProvider> GetAllBaseFieldsForConstructorInitialization(
}

private (IReadOnlyList<ParameterProvider> Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters(
bool isInitializationConstructor, bool includeDiscriminatorParameter = false)
bool isInitializationConstructor,
MethodSignatureModifiers modifiers = MethodSignatureModifiers.None,
bool includeDiscriminatorParameter = false,
bool reconcileWithLastContract = false)
{
var baseParameters = new List<ParameterProvider>();
var constructorParameters = new List<ParameterProvider>();
Expand Down Expand Up @@ -1222,6 +1237,16 @@ private IEnumerable<FieldProvider> GetAllBaseFieldsForConstructorInitialization(
p.Property is null
|| (!overriddenProperties.Contains(p.Property!) && (!p.Property.IsDiscriminator || !isInitializationConstructor || (includeDiscriminatorParameter && IsMultiLevelDiscriminator)))));

if (reconcileWithLastContract
&& ConstructorBackCompatHelper.TryRestoreInitializationParameters(
this,
modifiers,
constructorParameters,
out var restoredParameters))
{
constructorParameters = [.. restoredParameters];
}

// construct the initializer using the parameters from base signature
ConstructorInitializer? constructorInitializer = null;
if (BaseModelProvider != null)
Expand All @@ -1245,8 +1270,18 @@ p.Property is null
}
else
{
// Standard base constructor call
constructorInitializer = new ConstructorInitializer(true, [.. baseParameters.Select(p => GetExpressionForCtor(p, overriddenProperties, isInitializationConstructor, constructorParameters))]);
// Build the call from the reconciled base signature so a restored parameter
// order is reflected in every derived constructor call.
var baseSignatureParameters = isInitializationConstructor && !HasBaseModelProviderCycle()
? BaseModelProvider.InitializationConstructor.Signature.Parameters
: baseParameters;
constructorInitializer = new ConstructorInitializer(
true,
[.. baseSignatureParameters.Select(p => GetExpressionForCtor(
p,
overriddenProperties,
isInitializationConstructor,
constructorParameters))]);
}
}
else
Expand Down Expand Up @@ -1446,16 +1481,41 @@ private MethodBodyStatement GetPropertyInitializers(
IReadOnlyList<ParameterProvider>? parameters = null)
{
List<MethodBodyStatement> methodBodyStatements = new(CanonicalView.Properties.Count + CanonicalView.Fields.Count + 1);
Dictionary<string, ParameterProvider> parameterMap = parameters?.ToDictionary(p => p.Name) ?? [];
Dictionary<PropertyProvider, ParameterProvider> propertyParameterMap = [];
Dictionary<FieldProvider, ParameterProvider> fieldParameterMap = [];
if (parameters is not null)
{
foreach (var parameter in parameters)
{
if (parameter.Property is not null)
{
propertyParameterMap.TryAdd(parameter.Property, parameter);
}
else if (parameter.Field is not null)
{
fieldParameterMap.TryAdd(parameter.Field, parameter);
}
}
}

foreach (var property in CanonicalView.Properties)
{
CreatePropertyAssignmentStatement(isPrimaryConstructor, methodBodyStatements, parameterMap, property);
CreatePropertyAssignmentStatement(
isPrimaryConstructor,
methodBodyStatements,
propertyParameterMap,
fieldParameterMap,
property);
}

foreach (var field in CanonicalView.Fields)
{
CreatePropertyAssignmentStatement(isPrimaryConstructor, methodBodyStatements, parameterMap, field: field);
CreatePropertyAssignmentStatement(
isPrimaryConstructor,
methodBodyStatements,
propertyParameterMap,
fieldParameterMap,
field: field);
}

// If discriminator is defined as optional in the base model, but we have an expression for it, assign it in the
Expand Down Expand Up @@ -1504,7 +1564,8 @@ private MethodBodyStatement GetPropertyInitializers(
private void CreatePropertyAssignmentStatement(
bool isPrimaryConstructor,
List<MethodBodyStatement> methodBodyStatements,
Dictionary<string, ParameterProvider> parameterMap,
Dictionary<PropertyProvider, ParameterProvider> propertyParameterMap,
Dictionary<FieldProvider, ParameterProvider> fieldParameterMap,
PropertyProvider? property = default,
FieldProvider? field = default)
{
Expand Down Expand Up @@ -1537,7 +1598,10 @@ private void CreatePropertyAssignmentStatement(

var type = property?.Type ?? field!.Type;

if (parameterMap.TryGetValue(property?.AsParameter.Name ?? field!.AsParameter.Name, out var parameter) || Type.IsStruct)
var hasParameter = property is not null
? propertyParameterMap.TryGetValue(property, out var parameter)
: fieldParameterMap.TryGetValue(field!, out parameter);
if (hasParameter || Type.IsStruct)
{
if (parameter != null)
{
Expand Down
6 changes: 4 additions & 2 deletions ...csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,11 @@ internal static ParameterProvider CloneParameterWithName(
ParameterProvider source,
string newName,
bool removeDefault,
ParameterValidationType? validation = null)
ParameterValidationType? validation = null,
bool forceClone = false)
{
if (source.Name == newName
if (!forceClone
&& source.Name == newName
&& !(removeDefault && source.DefaultValue != null)
&& (validation == null || validation == source.Validation))
{
Expand Down
56 changes: 4 additions & 52 deletions ...s/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1045,60 +1045,12 @@ private Dictionary<MethodSignature, MethodProvider> BuildCurrentMethodSignatureM
}

/// <summary>
/// Returns this type's constructors with backward compatibility applied against
/// <see cref="LastContractView"/>. The default implementation preserves a previously-published
/// public constructor on an abstract base type: when the current generation would emit a
/// <c>private protected</c> constructor whose parameters match a <c>public</c> constructor in
/// the last contract, the modifier is promoted back to <c>public</c>. Override and call
/// <c>base</c> to extend this behavior.
/// Returns this type's constructors with non-structural backward compatibility applied against
/// <see cref="LastContractView"/>. Structural constructor compatibility is applied by the owning
/// provider before constructor bodies and callers are materialized.
/// </summary>
protected internal virtual IReadOnlyList<ConstructorProvider> BuildConstructorsForBackCompatibility(IEnumerable<ConstructorProvider> originalConstructors)
{
// Only handle the case of changing modifiers on abstract base types.
if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract))
{
return [.. originalConstructors];
}

if (LastContractView?.Constructors == null || LastContractView.Constructors.Count == 0)
{
return [.. originalConstructors];
}

List<ConstructorProvider> constructors = [.. originalConstructors];

// Check if the last contract had a public constructor with matching parameters
foreach (var previousConstructor in LastContractView.Constructors)
{
if (!previousConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public))
{
continue;
}

// Find a matching constructor in the current version by parameter signature
for (int i = 0; i < constructors.Count; i++)
{
var currentConstructor = constructors[i];
if (!currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private) ||
!currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected))
{
continue;
}

// Check if parameters match (same count and types)
if (BackCompatHelper.ParametersMatch(currentConstructor.Signature.Parameters, previousConstructor.Signature.Parameters))
{
// Change the modifier from private protected to public
currentConstructor.Signature.Update(modifiers: MethodSignatureModifiers.Public);
CodeModelGenerator.Instance.Emitter.Debug(
$"Promoted constructor '{Name}({string.Join(", ", currentConstructor.Signature.Parameters.Select(p => p.Type.ToString()))})' from 'private protected' to 'public' to match last contract.",
BackCompatibilityChangeCategory.ConstructorModifierPreserved);
}
}
}

return [.. constructors];
}
=> ConstructorBackCompatHelper.ApplyLateCompatibility(this, originalConstructors);

private IReadOnlyList<EnumTypeMember>? _enumValues;

Expand Down
51 changes: 0 additions & 51 deletions ...tp-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,35 +50,6 @@ public static bool IsMethodRemovalAcceptedInBaseline(TypeProvider enclosingType,
return true;
}

/// <summary>
/// Returns true when the removal of a previously-published constructor — identified by the
/// enclosing type's fully-qualified name and the exact parameter types — has been accepted in the
/// ApiCompat baseline, in which case back compatibility must not restore it. Constructors are
/// recorded in the baseline as the <c>.ctor</c> member of their declaring type. Emits an
/// informational log entry when a suppression is honored.
/// </summary>
public static bool IsConstructorRemovalAcceptedInBaseline(TypeProvider enclosingType, ConstructorSignature previousSignature)
{
var parameterTypes = new CSharpType[previousSignature.Parameters.Count];
for (int i = 0; i < parameterTypes.Length; i++)
{
parameterTypes[i] = previousSignature.Parameters[i].Type;
}

if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMethodRemovalSuppressed(
enclosingType.Type.FullyQualifiedName,
".ctor",
parameterTypes) != true)
{
return false;
}

CodeModelGenerator.Instance.Emitter.Info(
$"Skipping back-compat for '{enclosingType.Type.FullyQualifiedName}..ctor'; removal is accepted in the ApiCompat baseline.",
BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped);
return true;
}

/// <summary>
/// Finds the current method that has the same parameter set as <paramref name="previousSignature"/>
/// (matched by name and return type) but in a different order, or null when there is none.
Expand All @@ -101,28 +72,6 @@ public static bool IsConstructorRemovalAcceptedInBaseline(TypeProvider enclosing
return null;
}

/// <summary>
/// Returns true when two parameter lists match positionally by type name and parameter name
/// (and have the same count). Used to align a current member with its last-contract counterpart.
/// </summary>
public static bool ParametersMatch(IReadOnlyList<ParameterProvider> params1, IReadOnlyList<ParameterProvider> params2)
{
if (params1.Count != params2.Count)
{
return false;
}

for (int i = 0; i < params1.Count; i++)
{
if (!params1[i].Type.AreNamesEqual(params2[i].Type) || params1[i].Name != params2[i].Name)
{
return false;
}
}

return true;
}

/// <summary>
/// Returns the previously-published name of a parameter whose original (spec) name is
/// <paramref name="originalName"/>, looked up in <paramref name="lastContractView"/>. When
Expand Down
Loading
Loading