diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 43d2559a48b..78b0d3d9e6d 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -733,7 +733,8 @@ protected internal override ConstructorProvider[] BuildConstructors() { if (_inputModel.IsUnknownDiscriminatorModel) { - return [FullConstructor]; + _initializationConstructor = FullConstructor; + return [_initializationConstructor]; } // Build the standard single initialization constructor @@ -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( @@ -756,11 +760,14 @@ protected internal override ConstructorProvider[] BuildConstructors() GetPropertyInitializers(true, parameters: constructorParameters) }, this); + _initializationConstructor = constructor; var constructors = new List { constructor }; // Add FullConstructor if parameters are different - if (!constructorParameters.SequenceEqual(FullConstructor.Signature.Parameters)) + if (!ConstructorBackCompatHelper.HaveSameParameterIdentity( + constructorParameters, + FullConstructor.Signature.Parameters)) { constructors.Add(FullConstructor); } @@ -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."); + } + } + /// /// Restores previously-published public constructors that the current generation would otherwise /// drop. The primary scenario is a previously required property becoming optional: the corresponding @@ -800,14 +818,8 @@ protected internal override IReadOnlyList 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; } @@ -834,8 +846,8 @@ protected internal override IReadOnlyList 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; } @@ -1174,7 +1186,10 @@ private IEnumerable GetAllBaseFieldsForConstructorInitialization( } private (IReadOnlyList 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(); var constructorParameters = new List(); @@ -1222,6 +1237,16 @@ private IEnumerable 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) @@ -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 @@ -1446,16 +1481,41 @@ private MethodBodyStatement GetPropertyInitializers( IReadOnlyList? parameters = null) { List methodBodyStatements = new(CanonicalView.Properties.Count + CanonicalView.Fields.Count + 1); - Dictionary parameterMap = parameters?.ToDictionary(p => p.Name) ?? []; + Dictionary propertyParameterMap = []; + Dictionary 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 @@ -1504,7 +1564,8 @@ private MethodBodyStatement GetPropertyInitializers( private void CreatePropertyAssignmentStatement( bool isPrimaryConstructor, List methodBodyStatements, - Dictionary parameterMap, + Dictionary propertyParameterMap, + Dictionary fieldParameterMap, PropertyProvider? property = default, FieldProvider? field = default) { @@ -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) { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs index 5dbcb7f0aa1..1409d988c7b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs @@ -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)) { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index b881b47e65c..091ed6fba69 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs @@ -1045,60 +1045,12 @@ private Dictionary BuildCurrentMethodSignatureM } /// - /// Returns this type's constructors with backward compatibility applied against - /// . The default implementation preserves a previously-published - /// public constructor on an abstract base type: when the current generation would emit a - /// private protected constructor whose parameters match a public constructor in - /// the last contract, the modifier is promoted back to public. Override and call - /// base to extend this behavior. + /// Returns this type's constructors with non-structural backward compatibility applied against + /// . Structural constructor compatibility is applied by the owning + /// provider before constructor bodies and callers are materialized. /// protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable 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 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? _enumValues; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs index 000c664e000..46e85d15279 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -50,35 +50,6 @@ public static bool IsMethodRemovalAcceptedInBaseline(TypeProvider enclosingType, return true; } - /// - /// 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 .ctor member of their declaring type. Emits an - /// informational log entry when a suppression is honored. - /// - 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; - } - /// /// Finds the current method that has the same parameter set as /// (matched by name and return type) but in a different order, or null when there is none. @@ -101,28 +72,6 @@ public static bool IsConstructorRemovalAcceptedInBaseline(TypeProvider enclosing return null; } - /// - /// 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. - /// - public static bool ParametersMatch(IReadOnlyList params1, IReadOnlyList 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; - } - /// /// Returns the previously-published name of a parameter whose original (spec) name is /// , looked up in . When diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ConstructorBackCompatHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ConstructorBackCompatHelper.cs new file mode 100644 index 00000000000..70a4509c38a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/ConstructorBackCompatHelper.cs @@ -0,0 +1,295 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.TypeSpec.Generator.EmitterRpc; +using Microsoft.TypeSpec.Generator.Primitives; +using Microsoft.TypeSpec.Generator.Providers; + +namespace Microsoft.TypeSpec.Generator.Utilities +{ + /// + /// Owns constructor-specific compatibility policy. Structural changes are planned before a + /// constructor is materialized; late processing is limited to accessibility preservation. + /// + internal static class ConstructorBackCompatHelper + { + public static bool TryRestoreInitializationParameters( + ModelProvider model, + MethodSignatureModifiers currentModifiers, + IReadOnlyList currentParameters, + out IReadOnlyList restoredParameters) + { + restoredParameters = currentParameters; + if (currentModifiers.HasFlag(MethodSignatureModifiers.Static) + || (!MethodSignatureHelper.IsPublicApi(currentModifiers) && !IsPrivateProtected(currentModifiers)) + || model.LastContractView?.Constructors is not { Count: > 0 } previousConstructors) + { + return false; + } + + IReadOnlyList? candidate = null; + foreach (var previousConstructor in previousConstructors) + { + if (!IsEligiblePreviousConstructor(model, previousConstructor) + || !TryCreateRestoredParameters(currentParameters, previousConstructor.Signature.Parameters, out var restored)) + { + continue; + } + + // Multiple viable previous overloads make the intended contract ambiguous. + if (candidate is not null) + { + return false; + } + candidate = restored; + candidate = restored; + } + + if (candidate is null) + { + return false; + } + + restoredParameters = candidate; + for (int i = 0; i < currentParameters.Count; i++) + { + if (!string.Equals(currentParameters[i].Name, restoredParameters[i].Name, StringComparison.Ordinal) + || !ReferenceEquals(currentParameters[i], restoredParameters[i])) + { + CodeModelGenerator.Instance.Emitter.Debug( + $"Restored parameter '{restoredParameters[i].Name}' at position {i} on constructor '{model.Name}' from last contract (current generated name was '{currentParameters[i].Name}').", + BackCompatibilityChangeCategory.ParameterNamePreserved); + } + } + + return true; + } + + public static IReadOnlyList ApplyLateCompatibility( + TypeProvider type, + IEnumerable originalConstructors) + { + List constructors = [.. originalConstructors]; + if (!type.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract) + || type.LastContractView?.Constructors is not { Count: > 0 } previousConstructors) + { + return constructors; + } + + foreach (var previousConstructor in previousConstructors) + { + if (!IsEligiblePreviousConstructor(type, previousConstructor)) + { + continue; + } + + var currentConstructor = constructors.FirstOrDefault(c => + IsPrivateProtected(c.Signature.Modifiers) + && HaveSameParameterIdentity(c.Signature.Parameters, previousConstructor.Signature.Parameters)); + if (currentConstructor is null) + { + continue; + } + + var restoredModifiers = previousConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + ? MethodSignatureModifiers.Public + : MethodSignatureModifiers.Protected; + currentConstructor.Signature.Update(modifiers: restoredModifiers); + CodeModelGenerator.Instance.Emitter.Debug( + $"Promoted constructor '{type.Name}({string.Join(", ", currentConstructor.Signature.Parameters.Select(p => p.Type.ToString()))})' from 'private protected' to '{restoredModifiers.ToString().ToLowerInvariant()}' to match last contract.", + BackCompatibilityChangeCategory.ConstructorModifierPreserved); + } + + return constructors; + } + + public static bool IsEligiblePreviousConstructor(TypeProvider type, ConstructorProvider previousConstructor) + { + if (!MethodSignatureHelper.IsPublicApi(previousConstructor.Signature.Modifiers) + || previousConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Static)) + { + return false; + } + + return !IsRemovalAcceptedInBaseline(type, previousConstructor.Signature); + } + + public static bool HaveSameParameterIdentity( + IReadOnlyList first, + IReadOnlyList second) + { + if (first.Count != second.Count) + { + return false; + } + + for (int i = 0; i < first.Count; i++) + { + if (!HaveSameParameterIdentity(first[i], second[i])) + { + return false; + } + } + + return true; + } + + internal static bool TryCreateRestoredParameters( + IReadOnlyList currentParameters, + IReadOnlyList previousParameters, + out IReadOnlyList restoredParameters) + { + restoredParameters = currentParameters; + if (currentParameters.Count != previousParameters.Count + || previousParameters.Select(p => p.Name).Distinct(StringComparer.Ordinal).Count() != previousParameters.Count) + { + return false; + } + + int matchingNameCount = previousParameters.Count(previous => + currentParameters.Any(current => string.Equals(current.Name, previous.Name, StringComparison.OrdinalIgnoreCase))); + if (matchingNameCount != 0 && matchingNameCount != previousParameters.Count) + { + return false; + } + + var orderedCurrentParameters = new ParameterProvider[currentParameters.Count]; + if (matchingNameCount == previousParameters.Count) + { + var used = new HashSet(ReferenceEqualityComparer.Instance); + for (int i = 0; i < previousParameters.Count; i++) + { + var matches = currentParameters + .Where(current => !used.Contains(current) + && string.Equals(current.Name, previousParameters[i].Name, StringComparison.OrdinalIgnoreCase)) + .Take(2) + .ToArray(); + if (matches.Length != 1 || !HaveSameParameterIdentity(matches[0], previousParameters[i])) + { + return false; + } + + orderedCurrentParameters[i] = matches[0]; + used.Add(matches[0]); + } + } + else + { + for (int i = 0; i < currentParameters.Count; i++) + { + if (!HaveSameParameterIdentity(currentParameters[i], previousParameters[i])) + { + return false; + } + + orderedCurrentParameters[i] = currentParameters[i]; + } + } + + bool changed = orderedCurrentParameters + .Where((parameter, i) => + !ReferenceEquals(parameter, currentParameters[i]) + || !string.Equals(parameter.Name, previousParameters[i].Name, StringComparison.Ordinal)) + .Any(); + if (!changed + || orderedCurrentParameters.Any(p => p.Attributes.Count > 0 || p.InitializationValue is not null)) + { + return false; + } + + var candidate = orderedCurrentParameters + .Select((parameter, i) => PartialMethodCustomization.CloneParameterWithName( + parameter, + previousParameters[i].Name, + removeDefault: false, + forceClone: true)) + .ToArray(); + if (!HaveSameParameterIdentity(candidate, previousParameters) || !HasLegalParameterOrder(candidate)) + { + return false; + } + + restoredParameters = candidate; + return true; + } + + private static bool HaveSameParameterIdentity(ParameterProvider first, ParameterProvider second) + => TypesMatchForOverload(first.Type, second.Type) + && first.IsRef == second.IsRef + && first.IsOut == second.IsOut + && first.IsIn == second.IsIn + && first.IsParams == second.IsParams; + + private static bool TypesMatchForOverload(CSharpType first, CSharpType second) + { + if (!first.AreNamesEqual(second) + || (first.IsValueType && first.IsNullable != second.IsNullable)) + { + return false; + } + + for (int i = 0; i < first.Arguments.Count; i++) + { + if (!TypesMatchForOverload(first.Arguments[i], second.Arguments[i])) + { + return false; + } + } + + return true; + } + + private static bool HasLegalParameterOrder(IReadOnlyList parameters) + { + bool sawOptional = false; + for (int i = 0; i < parameters.Count; i++) + { + var parameter = parameters[i]; + if (parameter.IsParams) + { + if (i != parameters.Count - 1 || parameter.DefaultValue is not null) + { + return false; + } + + continue; + } + + if (parameter.DefaultValue is not null) + { + sawOptional = true; + } + else if (sawOptional) + { + return false; + } + } + + return true; + } + + private static bool IsPrivateProtected(MethodSignatureModifiers modifiers) + => modifiers.HasFlag(MethodSignatureModifiers.Private) + && modifiers.HasFlag(MethodSignatureModifiers.Protected); + + private static bool IsRemovalAcceptedInBaseline(TypeProvider type, ConstructorSignature previousSignature) + { + var parameterTypes = previousSignature.Parameters.Select(p => p.Type).ToArray(); + if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMethodRemovalSuppressed( + type.Type.FullyQualifiedName, + ".ctor", + parameterTypes) != true) + { + return false; + } + + CodeModelGenerator.Instance.Emitter.Info( + $"Skipping back-compat for '{type.Type.FullyQualifiedName}..ctor'; removal is accepted in the ApiCompat baseline.", + BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped); + return true; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/TypeSymbolExtensions.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/TypeSymbolExtensions.cs index 9e8c4a6c493..533936bb21a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/TypeSymbolExtensions.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/TypeSymbolExtensions.cs @@ -121,7 +121,10 @@ public static string GetFullyQualifiedName(this ITypeSymbol typeSymbol) // Handle array types if (typeSymbol is IArrayTypeSymbol arrayTypeSymbol) { - return GetFullyQualifiedName(arrayTypeSymbol.ElementType) + "[]"; + return GetFullyQualifiedName(arrayTypeSymbol.ElementType) + + "[" + + new string(',', arrayTypeSymbol.Rank - 1) + + "]"; } // Handle tuples & generic types if (typeSymbol is INamedTypeSymbol namedTypeSymbol) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index 15a9ffec1c0..b68d3aa804a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -2299,6 +2299,122 @@ protected override TypeProvider[] BuildSerializationProviders() } } + [Test] + public async Task BackCompat_ConstructorParameterSwapIsAppliedBeforeBody() + { + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("maximumCount", InputPrimitiveType.Int32, isRequired: true), + InputFactory.Property("minimumCount", InputPrimitiveType.Int32, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var model = (ModelProvider)CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .Single(t => t.Name == "MockInputModel"); + var constructor = model.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + + Assert.Multiple(() => + { + Assert.That( + constructor.Signature.Parameters.Select(p => p.Name), + Is.EqualTo(new[] { "minimumCount", "maximumCount" })); + Assert.That( + constructor.Signature.Parameters.Select(p => p.Property?.Name), + Is.EqualTo(new[] { "MinimumCount", "MaximumCount" })); + Assert.AreEqual( + "MaximumCount = maximumCount;\nMinimumCount = minimumCount;\n", + constructor.BodyStatements!.ToDisplayString()); + Assert.AreEqual("maximumCount", model.Properties.Single(p => p.Name == "MaximumCount").AsParameter.Name); + Assert.AreEqual("minimumCount", model.Properties.Single(p => p.Name == "MinimumCount").AsParameter.Name); + Assert.AreEqual( + 1, + model.Constructors.Count(c => ConstructorBackCompatHelper.HaveSameParameterIdentity( + c.Signature.Parameters, + constructor.Signature.Parameters))); + }); + } + + [Test] + public async Task BackCompat_ConstructorParameterRenameIsAppliedBeforeBody() + { + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("newName", InputPrimitiveType.String, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var model = (ModelProvider)CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .Single(t => t.Name == "MockInputModel"); + var constructor = model.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + + Assert.Multiple(() => + { + Assert.AreEqual("oldName", constructor.Signature.Parameters.Single().Name); + Assert.That(constructor.BodyStatements!.ToDisplayString(), Does.Contain("NewName = oldName;")); + Assert.AreEqual("newName", model.Properties.Single().AsParameter.Name); + Assert.AreEqual( + 1, + model.Constructors.Count(c => ConstructorBackCompatHelper.HaveSameParameterIdentity( + c.Signature.Parameters, + constructor.Signature.Parameters))); + }); + } + + [Test] + public async Task BackCompat_DerivedInitializerUsesRestoredBaseParameterOrder() + { + var baseModel = InputFactory.Model( + "BaseModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("maximumCount", InputPrimitiveType.Int32, isRequired: true), + InputFactory.Property("minimumCount", InputPrimitiveType.Int32, isRequired: true), + ]); + var derivedModel = InputFactory.Model( + "DerivedModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("label", InputPrimitiveType.String, isRequired: true), + ], + baseModel: baseModel); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [baseModel, derivedModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var derived = (ModelProvider)CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .Single(t => t.Name == "DerivedModel"); + var constructor = derived.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + + Assert.Multiple(() => + { + Assert.That( + constructor.Signature.Parameters.Select(p => p.Name), + Is.EqualTo(new[] { "minimumCount", "maximumCount", "label" })); + Assert.IsTrue(constructor.Signature.Initializer!.IsBase); + Assert.That( + constructor.Signature.Initializer.Arguments.Select(a => a.ToDisplayString()), + Is.EqualTo(new[] { "minimumCount", "maximumCount" })); + }); + } + [Test] public async Task BackCompat_AbstractTypeConstructorAccessibility() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs index 4dc7415bd15..c674bd737a2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs @@ -7,12 +7,13 @@ namespace Sample.Models { public partial class MockInputModel { - // The user supplies their own (name, resources) constructor, replacing the one the + // The user supplies their own (name, resourceList) constructor with the same overload + // identity as the previous (name, resources) constructor, replacing the one the // generator would otherwise restore for back compat. Restoration must be skipped so the // generated overload does not collide with this custom code. - public MockInputModel(string name, string resources) : this(name) + public MockInputModel(string name, string resourceList) : this(name) { - Resources = resources; + Resources = resourceList; } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameIsAppliedBeforeBody/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameIsAppliedBeforeBody/MockInputModel.cs new file mode 100644 index 00000000000..078b9a972e8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameIsAppliedBeforeBody/MockInputModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + public MockInputModel(string oldName) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapIsAppliedBeforeBody/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapIsAppliedBeforeBody/MockInputModel.cs new file mode 100644 index 00000000000..05ca872cda9 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapIsAppliedBeforeBody/MockInputModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + public MockInputModel(int minimumCount, int maximumCount) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_DerivedInitializerUsesRestoredBaseParameterOrder/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_DerivedInitializerUsesRestoredBaseParameterOrder/Models.cs new file mode 100644 index 00000000000..5cefc297e3b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_DerivedInitializerUsesRestoredBaseParameterOrder/Models.cs @@ -0,0 +1,17 @@ +namespace Sample.Models +{ + public partial class BaseModel + { + public BaseModel(int minimumCount, int maximumCount) + { + } + } + + public partial class DerivedModel : BaseModel + { + public DerivedModel(int minimumCount, int maximumCount, string label) + : base(minimumCount, maximumCount) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityDoesNotRenameParametersWhenTypesDiffer/ConstructorParameterTypeMismatchType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityDoesNotRenameParametersWhenTypesDiffer/ConstructorParameterTypeMismatchType.cs new file mode 100644 index 00000000000..4325a63730a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityDoesNotRenameParametersWhenTypesDiffer/ConstructorParameterTypeMismatchType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class ConstructorParameterTypeMismatchType + { + public ConstructorParameterTypeMismatchType(int oldName) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityMatchesNestedValueTypeNullability/ConstructorNestedNullableOverloadType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityMatchesNestedValueTypeNullability/ConstructorNestedNullableOverloadType.cs new file mode 100644 index 00000000000..622de4aa4f0 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityMatchesNestedValueTypeNullability/ConstructorNestedNullableOverloadType.cs @@ -0,0 +1,11 @@ +using System.Collections.Generic; + +namespace Test +{ + public class ConstructorNestedNullableOverloadType + { + public ConstructorNestedNullableOverloadType(List previousNullableItems) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityMatchesValueTypeNullability/ConstructorNullableOverloadType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityMatchesValueTypeNullability/ConstructorNullableOverloadType.cs new file mode 100644 index 00000000000..a8c394ae117 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityMatchesValueTypeNullability/ConstructorNullableOverloadType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class ConstructorNullableOverloadType + { + public ConstructorNullableOverloadType(int? previousNullable) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRejectsMixedNullabilityPermutation/ConstructorMixedNullablePermutationType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRejectsMixedNullabilityPermutation/ConstructorMixedNullablePermutationType.cs new file mode 100644 index 00000000000..fe575d52459 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRejectsMixedNullabilityPermutation/ConstructorMixedNullablePermutationType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class ConstructorMixedNullablePermutationType + { + public ConstructorMixedNullablePermutationType(int nonNullable, int? nullable) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresParameterNameCasing/ConstructorParameterCasingType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresParameterNameCasing/ConstructorParameterCasingType.cs new file mode 100644 index 00000000000..844a54371fd --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresParameterNameCasing/ConstructorParameterCasingType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class ConstructorParameterCasingType + { + public ConstructorParameterCasingType(string vmwareSiteId) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresRotatedParameterNames/ConstructorParameterRotationType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresRotatedParameterNames/ConstructorParameterRotationType.cs new file mode 100644 index 00000000000..c33972822c3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresRotatedParameterNames/ConstructorParameterRotationType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class ConstructorParameterRotationType + { + public ConstructorParameterRotationType(string first, string second, string third) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresSimpleParameterRename/SimpleConstructorParameterRenameType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresSimpleParameterRename/SimpleConstructorParameterRenameType.cs new file mode 100644 index 00000000000..fd2bcfc2922 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresSimpleParameterRename/SimpleConstructorParameterRenameType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class SimpleConstructorParameterRenameType + { + public SimpleConstructorParameterRenameType(string oldName) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresSwappedParameterNames/ConstructorParameterRenameType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresSwappedParameterNames/ConstructorParameterRenameType.cs new file mode 100644 index 00000000000..6d70c7d40a0 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityRestoresSwappedParameterNames/ConstructorParameterRenameType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class ConstructorParameterRenameType + { + public ConstructorParameterRenameType(int minimumCount, int maximumCount) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 0d702ffcc2f..fa22c37312a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs @@ -232,6 +232,429 @@ public async Task BuildConstructorsForBackCompatibilityKeepsModifierOnNonAbstrac Assert.IsFalse(constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); } + [Test] + public async Task BuildConstructorsForBackCompatibilityRestoresSwappedParameterNames() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var maximumCount = new ParameterProvider("maximumCount", $"The maximum count.", new CSharpType(typeof(int))); + var minimumCount = new ParameterProvider("minimumCount", $"The minimum count.", new CSharpType(typeof(int))); + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Public, + [maximumCount, minimumCount]), + new MethodBodyStatement[] + { + Snippet.This.Property("MaximumCount").Assign(maximumCount).Terminate(), + Snippet.This.Property("MinimumCount").Assign(minimumCount).Terminate(), + }, + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "ConstructorParameterRenameType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + var previousParameters = typeProvider.LastContractView!.Constructors.Single().Signature.Parameters; + Assert.IsTrue(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentConstructor.Signature.Parameters, + previousParameters, + out var restoredParameters)); + + Assert.Multiple(() => + { + Assert.That(restoredParameters.Select(p => p.Name), Is.EqualTo(new[] { "minimumCount", "maximumCount" })); + Assert.AreEqual("The minimum count.", restoredParameters[0].Description.Format); + Assert.AreEqual("The maximum count.", restoredParameters[1].Description.Format); + Assert.AreNotSame(minimumCount, restoredParameters[0]); + Assert.That(currentConstructor.Signature.Parameters, Is.EqualTo(new[] { maximumCount, minimumCount })); + }); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityRestoresSimpleParameterRename() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var parameter = new ParameterProvider("newName", $"The renamed value.", new CSharpType(typeof(string))); + _ = parameter.AsVariable(); + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Public, + [parameter]), + Snippet.This.Property("Value").Assign(parameter).Terminate(), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "SimpleConstructorParameterRenameType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + var previousParameters = typeProvider.LastContractView!.Constructors.Single().Signature.Parameters; + Assert.IsTrue(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentConstructor.Signature.Parameters, + previousParameters, + out var restoredParameters)); + + Assert.Multiple(() => + { + Assert.AreEqual("oldName", restoredParameters.Single().Name); + Assert.AreEqual("newName", parameter.Name); + Assert.AreNotSame(parameter, restoredParameters.Single()); + }); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityRestoresParameterNameCasing() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Public, + [new ParameterProvider("vMwareSiteId", $"", new CSharpType(typeof(string)))]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "ConstructorParameterCasingType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + var previousParameters = typeProvider.LastContractView!.Constructors.Single().Signature.Parameters; + Assert.IsTrue(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentConstructor.Signature.Parameters, + previousParameters, + out var restoredParameters)); + + Assert.AreEqual("vmwareSiteId", restoredParameters.Single().Name); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityRestoresRotatedParameterNames() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Public, + [ + new ParameterProvider("third", $"", new CSharpType(typeof(string))), + new ParameterProvider("first", $"", new CSharpType(typeof(string))), + new ParameterProvider("second", $"", new CSharpType(typeof(string))), + ]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "ConstructorParameterRotationType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + var previousParameters = typeProvider.LastContractView!.Constructors.Single().Signature.Parameters; + Assert.IsTrue(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentConstructor.Signature.Parameters, + previousParameters, + out var restoredParameters)); + + Assert.That( + restoredParameters.Select(p => p.Name), + Is.EqualTo(new[] { "first", "second", "third" })); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityDoesNotRenameParametersWhenTypesDiffer() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Public, + [new ParameterProvider("newName", $"", new CSharpType(typeof(string)))]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "ConstructorParameterTypeMismatchType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + var previousParameters = typeProvider.LastContractView!.Constructors.Single().Signature.Parameters; + + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentConstructor.Signature.Parameters, + previousParameters, + out _)); + Assert.AreEqual("newName", currentConstructor.Signature.Parameters.Single().Name); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityMatchesValueTypeNullability() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var nonNullableParameter = new ParameterProvider("nonNullableCurrent", $"", new CSharpType(typeof(int))); + var nullableParameter = new ParameterProvider("nullableCurrent", $"", new CSharpType(typeof(int?))); + var typeProvider = new TestTypeProvider( + name: "ConstructorNullableOverloadType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: + [ + new ConstructorProvider( + new ConstructorSignature(typeof(object), $"", MethodSignatureModifiers.Public, [nonNullableParameter]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()), + new ConstructorProvider( + new ConstructorSignature(typeof(object), $"", MethodSignatureModifiers.Public, [nullableParameter]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()), + ]); + + var previousNullableParameters = typeProvider.LastContractView!.Constructors + .Single(c => c.Signature.Parameters.Single().Type.IsNullable) + .Signature.Parameters; + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + [nonNullableParameter], + previousNullableParameters, + out _)); + Assert.IsTrue(ConstructorBackCompatHelper.TryCreateRestoredParameters( + [nullableParameter], + previousNullableParameters, + out var restoredParameters)); + + Assert.Multiple(() => + { + Assert.AreEqual("nonNullableCurrent", nonNullableParameter.Name); + Assert.AreEqual("previousNullable", restoredParameters.Single().Name); + Assert.AreEqual("nullableCurrent", nullableParameter.Name); + }); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityMatchesNestedValueTypeNullability() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var nonNullableParameter = new ParameterProvider("nonNullableCurrent", $"", new CSharpType(typeof(List))); + var nullableParameter = new ParameterProvider("nullableCurrent", $"", new CSharpType(typeof(List))); + var typeProvider = new TestTypeProvider( + name: "ConstructorNestedNullableOverloadType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: + [ + new ConstructorProvider( + new ConstructorSignature(typeof(object), $"", MethodSignatureModifiers.Public, [nonNullableParameter]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()), + new ConstructorProvider( + new ConstructorSignature(typeof(object), $"", MethodSignatureModifiers.Public, [nullableParameter]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()), + ]); + + var previousNullableParameters = typeProvider.LastContractView!.Constructors + .Single(c => c.Signature.Parameters.Single().Type.Arguments.Single().IsNullable) + .Signature.Parameters; + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + [nonNullableParameter], + previousNullableParameters, + out _)); + Assert.IsTrue(ConstructorBackCompatHelper.TryCreateRestoredParameters( + [nullableParameter], + previousNullableParameters, + out var restoredParameters)); + + Assert.Multiple(() => + { + Assert.AreEqual("nonNullableCurrent", nonNullableParameter.Name); + Assert.AreEqual("previousNullableItems", restoredParameters.Single().Name); + Assert.AreEqual("nullableCurrent", nullableParameter.Name); + }); + } + + [Test] + public async Task BuildConstructorsForBackCompatibilityRejectsMixedNullabilityPermutation() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var nullable = new ParameterProvider("nullable", $"", new CSharpType(typeof(int))); + var nonNullable = new ParameterProvider("nonNullable", $"", new CSharpType(typeof(int?))); + var currentConstructor = new ConstructorProvider( + new ConstructorSignature(typeof(object), $"", MethodSignatureModifiers.Public, [nullable, nonNullable]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()); + var typeProvider = new TestTypeProvider( + name: "ConstructorMixedNullablePermutationType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + typeProvider.ProcessTypeForBackCompatibility(); + + Assert.Multiple(() => + { + Assert.That(currentConstructor.Signature.Parameters, Is.EqualTo(new[] { nullable, nonNullable })); + Assert.That( + currentConstructor.Signature.Parameters.Select(p => p.Name), + Is.EqualTo(new[] { "nullable", "nonNullable" })); + Assert.IsFalse(currentConstructor.Signature.Parameters[0].Type.IsNullable); + Assert.IsTrue(currentConstructor.Signature.Parameters[1].Type.IsNullable); + }); + } + + [Test] + public void ConstructorParameterRestorationRejectsDuplicateFinalNames() + { + ParameterProvider[] currentParameters = + [ + new("first", $"", new CSharpType(typeof(string))), + new("second", $"", new CSharpType(typeof(string))), + ]; + ParameterProvider[] previousParameters = + [ + new("duplicate", $"", new CSharpType(typeof(string))), + new("duplicate", $"", new CSharpType(typeof(string))), + ]; + Assert.Multiple(() => + { + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentParameters, + previousParameters, + out _)); + Assert.That(currentParameters.Select(p => p.Name), Is.EqualTo(new[] { "first", "second" })); + }); + } + + [Test] + public void ConstructorParameterRestorationRejectsPartialNameOverlap() + { + ParameterProvider[] currentParameters = + [ + new("second", $"", new CSharpType(typeof(string))), + new("replacement", $"", new CSharpType(typeof(string))), + new("third", $"", new CSharpType(typeof(string))), + ]; + ParameterProvider[] previousParameters = + [ + new("first", $"", new CSharpType(typeof(string))), + new("second", $"", new CSharpType(typeof(string))), + new("third", $"", new CSharpType(typeof(string))), + ]; + + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + currentParameters, + previousParameters, + out _)); + } + + [Test] + public void ConstructorParameterIdentityIncludesRefKindsAndParams() + { + var value = new ParameterProvider("value", $"", new CSharpType(typeof(int))); + var byRef = new ParameterProvider("value", $"", new CSharpType(typeof(int)), isRef: true); + var byIn = new ParameterProvider("value", $"", new CSharpType(typeof(int)), isIn: true); + var byOut = new ParameterProvider("value", $"", new CSharpType(typeof(int)), isOut: true); + var values = new ParameterProvider("values", $"", new CSharpType(typeof(int[])), isParams: true); + var array = new ParameterProvider("values", $"", new CSharpType(typeof(int[]))); + + Assert.Multiple(() => + { + Assert.IsFalse(ConstructorBackCompatHelper.HaveSameParameterIdentity([value], [byRef])); + Assert.IsFalse(ConstructorBackCompatHelper.HaveSameParameterIdentity([value], [byIn])); + Assert.IsFalse(ConstructorBackCompatHelper.HaveSameParameterIdentity([value], [byOut])); + Assert.IsFalse(ConstructorBackCompatHelper.HaveSameParameterIdentity([values], [array])); + }); + } + + [Test] + public void ConstructorParameterIdentityIncludesArrayRankAndNestedNullability() + { + var vector = new ParameterProvider("value", $"", new CSharpType(typeof(int[]))); + var matrix = new ParameterProvider("value", $"", new CSharpType(typeof(int[,]))); + var nestedNonNullable = new ParameterProvider("value", $"", new CSharpType(typeof(List))); + var nestedNullable = new ParameterProvider("value", $"", new CSharpType(typeof(List))); + + Assert.Multiple(() => + { + Assert.IsFalse(ConstructorBackCompatHelper.HaveSameParameterIdentity([vector], [matrix])); + Assert.IsFalse(ConstructorBackCompatHelper.HaveSameParameterIdentity([nestedNonNullable], [nestedNullable])); + }); + } + + [Test] + public void ConstructorParameterRestorationRejectsIllegalOptionalOrderAndAttributes() + { + var required = new ParameterProvider("required", $"", new CSharpType(typeof(string))); + var optional = new ParameterProvider("optional", $"", new CSharpType(typeof(string)), defaultValue: Snippet.Default); + ParameterProvider[] previousParameters = + [ + new("optional", $"", new CSharpType(typeof(string))), + new("required", $"", new CSharpType(typeof(string)), defaultValue: Snippet.Default), + ]; + + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + [required, optional], + previousParameters, + out _)); + + var attributed = new ParameterProvider( + "newName", + $"", + new CSharpType(typeof(string)), + attributes: [new AttributeStatement(typeof(ObsoleteAttribute))]); + Assert.IsFalse(ConstructorBackCompatHelper.TryCreateRestoredParameters( + [attributed], + [new ParameterProvider("oldName", $"", new CSharpType(typeof(string)))], + out _)); + } + + [Test] + public async Task ConstructorCompatibilityExcludesStaticAndPrivateProtectedContracts() + { + await MockHelpers.LoadMockGeneratorAsync(); + var typeProvider = new TestTypeProvider(); + var parameter = new ParameterProvider("value", $"", new CSharpType(typeof(string))); + var staticConstructor = new ConstructorProvider( + new ConstructorSignature( + typeof(object), + $"", + MethodSignatureModifiers.Public | MethodSignatureModifiers.Static, + [parameter]), + Snippet.ThrowExpression(Snippet.Null), + typeProvider); + var privateProtectedConstructor = new ConstructorProvider( + new ConstructorSignature( + typeof(object), + $"", + MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected, + [parameter]), + Snippet.ThrowExpression(Snippet.Null), + typeProvider); + + Assert.Multiple(() => + { + Assert.IsFalse(ConstructorBackCompatHelper.IsEligiblePreviousConstructor(typeProvider, staticConstructor)); + Assert.IsFalse(ConstructorBackCompatHelper.IsEligiblePreviousConstructor(typeProvider, privateProtectedConstructor)); + }); + } + // Validates that the base TypeProvider generalizes the new-optional-parameter back-compat to any // TypeProvider: a public method that gained an optional non-body parameter relative to the last // contract gets a hidden overload matching the previous signature that delegates to the current one. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TypeSymbolExtensionsTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TypeSymbolExtensionsTests.cs index b031193c398..91b45cd7179 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TypeSymbolExtensionsTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TypeSymbolExtensionsTests.cs @@ -151,6 +151,29 @@ public class Container Assert.AreEqual("System.Collections.Generic.IReadOnlyList`1", name); } + [Test] + public void MultidimensionalArrayFullyQualifiedNamePreservesRank() + { + var compilation = CSharpCompilation.Create( + "TestAssembly", + [CSharpSyntaxTree.ParseText(""" + namespace Sample + { + public class Container + { + public int[,] Matrix { get; } + } + } + """)], + [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)]); + var property = compilation.GetTypeByMetadataName("Sample.Container")! + .GetMembers("Matrix") + .OfType() + .Single(); + + Assert.AreEqual("System.Int32[,]", property.Type.GetFullyQualifiedName()); + } + private static IPropertySymbol GetPropertySymbol(Compilation compilation, string containerName, string propertyName) { var typeSymbol = compilation.GetTypeByMetadataName($"Sample.{containerName}");