Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,68 @@ private ConstructorProvider BuildFullConstructor()
this);
}

protected internal override 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];

// Check if parameters match (same count and types)
if (ParametersMatch(currentConstructor.Signature.Parameters, previousConstructor.Signature.Parameters))
{
// Change the modifier from private protected to public
if (currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private) &&
currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected))
{
currentConstructor.Signature.Update(modifiers: MethodSignatureModifiers.Public);
}
}
}
}

return [.. constructors];
}

private 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;
}

private IEnumerable<PropertyProvider> GetAllBasePropertiesForConstructorInitialization(bool includeAllHierarchyDiscriminator = false)
{
var properties = new Stack<List<PropertyProvider>>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,17 +565,26 @@ internal void EnsureBuilt()

internal void ProcessTypeForBackCompatibility()
{
if (LastContractView?.Methods == null || LastContractView?.Methods.Count == 0)
var hasMethods = LastContractView?.Methods != null && LastContractView.Methods.Count > 0;
var hasConstructors = LastContractView?.Constructors != null && LastContractView.Constructors.Count > 0;

if (!hasMethods && !hasConstructors)
{
return;
}

Update(methods: BuildMethodsForBackCompatibility(Methods));
var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null;
var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null;

Update(methods: newMethods, constructors: newConstructors);
}

protected internal virtual IReadOnlyList<MethodProvider> BuildMethodsForBackCompatibility(IEnumerable<MethodProvider> originalMethods)
=> [.. originalMethods];

protected internal virtual IReadOnlyList<ConstructorProvider> BuildConstructorsForBackCompatibility(IEnumerable<ConstructorProvider> originalConstructors)
=> [.. originalConstructors];

private IReadOnlyList<EnumTypeMember>? _enumValues;

private bool ShouldGenerate(ConstructorProvider constructor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1760,5 +1760,52 @@ protected override TypeProvider[] BuildSerializationProviders()
return [new Mock<TypeProvider>() { CallBase = true }.Object];
}
}

[Test]
public async Task BackCompat_AbstractTypeConstructorAccessibility()
{
var discriminatorEnum = InputFactory.StringEnum("kindEnum", [("One", "one"), ("Two", "two")]);
var derivedInputModel = InputFactory.Model(
"DerivedModel",
discriminatedKind: "one",
properties:
[
InputFactory.Property("kind", InputFactory.EnumMember.String("One", "one", discriminatorEnum), isRequired: true, isDiscriminator: true),
InputFactory.Property("derivedProp", InputPrimitiveType.Int32, isRequired: true)
]);
var inputModel = InputFactory.Model(
"BaseModel",
properties:
[
InputFactory.Property("kind", discriminatorEnum, isRequired: false, isDiscriminator: true),
InputFactory.Property("baseProp", InputPrimitiveType.String, isRequired: true)
],
discriminatedModels: new Dictionary<string, InputModelType>() { { "one", derivedInputModel }});

await MockHelpers.LoadMockGeneratorAsync(
inputModelTypes: [inputModel],
lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());

var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "BaseModel") as ModelProvider;

Assert.IsNotNull(modelProvider);

// Without ProcessTypeForBackCompatibility, constructor should be private protected
var privateProtectedConstructor = modelProvider!.Constructors
.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private)
&& c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)
&& c.Signature.Parameters.Count == 1);
Assert.IsNotNull(privateProtectedConstructor, "Expected a private protected constructor before back compat processing");

// Call ProcessTypeForBackCompatibility to apply backward compatibility logic
modelProvider.ProcessTypeForBackCompatibility();

// After ProcessTypeForBackCompatibility, constructor should be public to match last contract
var publicConstructor = modelProvider.Constructors
.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)
&& c.Signature.Parameters.Count == 1);
Assert.IsNotNull(publicConstructor, "Constructor modifier should be changed to public for backward compatibility");
Assert.AreEqual("baseProp", publicConstructor!.Signature.Parameters[0].Name);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System.Collections.Generic;

namespace Sample.Models
{
public abstract partial class BaseModel
{
/// <summary> Initializes a new instance of BaseModel. </summary>
public BaseModel(string baseProp)
{
BaseProp = baseProp;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [Model Properties](#model-properties)
- [API Version Enum](#api-version-enum)
- [Non-abstract Base Models](#non-abstract-base-models)
- [Model Constructors](#model-constructors)

## Overview

Expand Down Expand Up @@ -212,3 +213,67 @@ public class BaseModel
public string CommonProperty { get; set; }
}
```

### Model Constructors

The generator maintains backward compatibility for model constructors on abstract base types to prevent breaking changes when constructor accessibility changes.

#### Scenario: Public Constructor on Abstract Base Type

**Description:** When an abstract base type had a public constructor in the previous version, but the current TypeSpec generation would create a `private protected` constructor, the generator automatically changes the modifier to `public` to maintain backward compatibility.

This commonly occurs when:

- Migrating from autorest-generated code to TypeSpec-generated code
- Abstract base types with discriminators had public parameterless constructors in previous versions

**Example:**

Previous version had a public parameterless constructor:

```csharp
public abstract partial class SearchIndexerDataIdentity
{
/// <summary> Initializes a new instance of SearchIndexerDataIdentity. </summary>
public SearchIndexerDataIdentity()
{
}
}
```

Current TypeSpec would generate a private protected constructor:

```csharp
public abstract partial class SearchIndexerDataIdentity
{
/// <summary> Initializes a new instance of SearchIndexerDataIdentity. </summary>
/// <param name="odataType"> A URI fragment specifying the type of identity. </param>
private protected SearchIndexerDataIdentity(string odataType)
{
OdataType = odataType;
}
}
```

**Generated Compatibility Result:**

When a matching public constructor exists in the last contract, the modifier is changed from `private protected` to `public`:

```csharp
public abstract partial class SearchIndexerDataIdentity
{
/// <summary> Initializes a new instance of SearchIndexerDataIdentity. </summary>
/// <param name="odataType"> A URI fragment specifying the type of identity. </param>
public SearchIndexerDataIdentity(string odataType)
{
OdataType = odataType;
}
}
```

**Key Points:**

- Only applies to abstract base types
- The constructor must have matching parameters (same count, types, and names)
- The modifier is changed from `private protected` to `public`
- No additional constructors are generated; only the accessibility is adjusted