diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs index 660f0cdd3cf..c06c97a44b8 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs @@ -46,6 +46,7 @@ public class ScmModelProvider : ModelProvider internal const string ScmEvaluationTypeDiagnosticId = "SCME0001"; internal const string FileBinaryContentDiagnosticId = "SCME0004"; + private const string IncompatibleBackcompatBaseTypeDiagnostic = "incompatible-backcompat-base-type"; internal const string ScmEvaluationTypeSuppressionJustification = "Type is for evaluation purposes only and is subject to change or removal in future updates."; @@ -73,6 +74,22 @@ public ScmModelProvider(InputModelType inputModel) : base(inputModel) BaseJsonPatchProperty = new(GetBaseJsonPatchProperty()); } + protected override CSharpType? BuildBaseTypeForBackCompatibility(CSharpType? currentBase) + { + var previousBase = LastContractView?.BaseType; + if (_inputModel.DiscriminatorValue is not null && + previousBase is not null && + !IsInBaseTypeHierarchy(currentBase, previousBase)) + { + CodeModelGenerator.Instance.Emitter.ReportDiagnostic( + IncompatibleBackcompatBaseTypeDiagnostic, + $"Could not preserve base type '{previousBase.FullyQualifiedName}' on model '{BuildNamespace()}.{BuildName()}' because the model participates in the current discriminator hierarchy."); + return currentBase; + } + + return base.BuildBaseTypeForBackCompatibility(currentBase); + } + protected override FieldProvider[] BuildFields() { if (JsonPatchField is null) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs index d5b58f5a7ca..6266a428561 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs @@ -1,11 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text.Json.Serialization; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Primitives; @@ -262,6 +265,74 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual(Helpers.GetExpectedFromFile("Serialization"), serializationContent); } + [Test] + public async Task BackCompat_DifferentLastContractBaseIsNotRestoredAcrossDiscriminatorHierarchy() + { + var previousBase = InputFactory.Model("previousBase", properties: []); + var derivedModel = InputFactory.Model( + "derivedModel", + discriminatedKind: "derived", + usage: InputModelTypeUsage.Json, + properties: []); + var currentBase = InputFactory.Model( + "currentBase", + usage: InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true) + ], + discriminatedModels: new Dictionary { ["derived"] = derivedModel }); + + await MockHelpers.LoadMockGeneratorAsync( + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + inputModels: () => [previousBase, currentBase, derivedModel]); + + var models = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .ToArray(); + foreach (var model in models) + { + model.ProcessTypeForBackCompatibility(); + } + + var derivedProvider = models.Single(t => t.Name == "DerivedModel"); + Assert.Multiple(() => + { + Assert.AreEqual("PreviousBase", derivedProvider.LastContractView?.BaseType?.Name, + "The regression requires a different last-contract base"); + Assert.AreEqual("CurrentBase", derivedProvider.BaseType?.Name, + "The current discriminator hierarchy must remain assignable"); + Assert.That(models, Has.Some.Matches(m => m.IsUnknownDiscriminatorModel), + "The current discriminator hierarchy should include its unknown subtype"); + }); + + string[] supportingProviderNames = ["ModelSerializationExtensions", "ChangeTrackingDictionary", "SampleContext", "TypeFormatters", "SerializationFormat"]; + var generatedProviders = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders + .Where(provider => provider is ScmModel || supportingProviderNames.Contains(provider.Name)) + .Concat(models.SelectMany(model => model.SerializationProviders)) + .Distinct() + .ToArray(); + var syntaxTrees = generatedProviders.Select(provider => + CSharpSyntaxTree.ParseText( + new TypeProviderWriter(provider).Write().Content, + path: $"{provider.Name}.cs")) + .Append(CSharpSyntaxTree.ParseText( + "namespace Sample { public partial class SampleContext { public static SampleContext Default => null; } }", + path: "SampleContext.Default.cs")); + var references = AppDomain.CurrentDomain.GetAssemblies() + .Where(a => !a.IsDynamic && !string.IsNullOrEmpty(a.Location)) + .Select(a => MetadataReference.CreateFromFile(a.Location)); + var compilation = CSharpCompilation.Create( + "DiscriminatorModels", + syntaxTrees, + references, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + Assert.That( + compilation.GetDiagnostics().Where(d => d.Severity is DiagnosticSeverity.Warning or DiagnosticSeverity.Error), + Is.Empty, + "The discriminator deserializer should compile after incompatible base restoration is skipped"); + } + [Test] public async Task BackCompat_AccessibleParameterlessSerializationConstructorIsPreserved() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_DifferentLastContractBaseIsNotRestoredAcrossDiscriminatorHierarchy/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_DifferentLastContractBaseIsNotRestoredAcrossDiscriminatorHierarchy/Models.cs new file mode 100644 index 00000000000..d84c8dea3a1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_DifferentLastContractBaseIsNotRestoredAcrossDiscriminatorHierarchy/Models.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public partial class PreviousBase + { + } + + public partial class DerivedModel : PreviousBase + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs index 6a2aad4f357..ec737b2f3fe 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs @@ -22,6 +22,9 @@ public enum BackCompatibilityChangeCategory /// A property type was preserved from the last contract. PropertyTypePreserved, + /// A model base type was preserved from the last contract. + ModelBaseTypePreserved, + /// A constructor modifier (e.g. private protected -> public) was preserved from the last contract. ConstructorModifierPreserved, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs index 7c073c93c99..1552b25c58e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs @@ -174,6 +174,7 @@ public void WriteBufferedMessages() BackCompatibilityChangeCategory.ParameterNamePreserved => "Parameter Name Preserved", BackCompatibilityChangeCategory.AdditionalPropertiesShapePreserved => "AdditionalProperties Shape Preserved", BackCompatibilityChangeCategory.PropertyTypePreserved => "Property Type Preserved", + BackCompatibilityChangeCategory.ModelBaseTypePreserved => "Model Base Type Preserved", BackCompatibilityChangeCategory.ConstructorModifierPreserved => "Constructor Modifier Preserved", BackCompatibilityChangeCategory.EnumMemberReordering => "Enum Member Reordering", BackCompatibilityChangeCategory.ApiVersionEnumMemberAdded => "Api Version Enum Member Added From Last Contract", 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 c48a3611d7d..c1ea18757cd 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 @@ -134,43 +134,43 @@ private IReadOnlyList BuildDerivedModels() private TypeProvider? BuildBaseTypeProvider() { - // First check if there's a generated base model + // First check if there's a generated base model. if (BaseModelProvider != null) { return BaseModelProvider; } - // If there's a custom base type that's not a generated model, create a provider for it - if (CustomCodeView?.BaseType != null && !string.IsNullOrEmpty(CustomCodeView.BaseType.Namespace)) + var baseType = BaseType; + if (baseType is null || string.IsNullOrEmpty(baseType.Namespace)) { - var baseType = CustomCodeView.BaseType; + return null; + } - // Try to find it in the CSharpTypeMap first - if (CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(baseType, out var existingProvider)) - { - return existingProvider; - } + // A base preserved from the last contract can be a framework or external type, just like + // a custom base. Resolve it from the current compilation rather than retaining a symbol + // that exists only in the baseline assembly. + if (CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(baseType, out var existingProvider)) + { + return existingProvider; + } - // Try to find the type in the customization compilation. Referenced assemblies are - // included so custom bases from framework or external packages are represented by - // normal symbol-backed providers. - var baseTypeProvider = CodeModelGenerator.Instance.SourceInputModel.FindForTypeInCurrentCompilation( - baseType.Namespace, - baseType.Name, - baseType.DeclaringType?.Name, - includeReferencedAssemblies: true); + var baseTypeProvider = CodeModelGenerator.Instance.SourceInputModel.FindForTypeInCurrentCompilation( + GetMetadataNamespace(baseType), + GetMetadataSimpleName(baseType), + baseType.DeclaringType?.ClrMetadataName, + includeReferencedAssemblies: true); - if (baseTypeProvider != null) - { - // Cache it in CSharpTypeMap for future lookups - CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap[baseType] = baseTypeProvider; - return baseTypeProvider; - } + if (baseTypeProvider != null) + { + CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap[baseType] = baseTypeProvider; + return baseTypeProvider; + } - // If we couldn't find the type symbol, create a SystemObjectTypeProvider that - // represents the external type without member metadata. + // Preserve the existing fallback for unresolved custom base types. Last-contract bases + // are selected only after they have been resolved against the current build. + if (CustomCodeView?.BaseType != null) + { var systemObjectTypeProvider = new SystemObjectTypeProvider(baseType); - // Cache it in CSharpTypeMap for future lookups CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap[baseType] = systemObjectTypeProvider; return systemObjectTypeProvider; } @@ -246,6 +246,74 @@ protected override string BuildNamespace() => string.IsNullOrEmpty(_inputModel.N CodeModelGenerator.Instance.TypeFactory.GetCleanNameSpace(_inputModel.Namespace); protected override CSharpType? BuildBaseType() + { + var currentBase = BuildCurrentBaseType(); + return BuildBaseTypeForBackCompatibility(currentBase); + } + + /// + /// Returns the model base type after applying backward compatibility against . + /// The default implementation conservatively restores a resolvable previously-published base type. + /// Override and call base to extend this behavior, or override without calling base to replace it. + /// This hook runs while the base type is being built, before model members and serialization are materialized. + /// + /// The base type selected from custom code or the current input model. + protected virtual CSharpType? BuildBaseTypeForBackCompatibility(CSharpType? currentBase) + { + var previousBase = LastContractView?.BaseType; + if (previousBase is null || IsInBaseTypeHierarchy(currentBase, previousBase)) + { + return currentBase; + } + + // A generated partial cannot replace a different base declared by custom code: all partial + // declarations must specify the same base class. Keep the custom base authoritative and + // report that the previous inheritance relationship could not be restored. + if (CustomCodeView?.BaseType is not null) + { + ReportIncompatibleBackcompatBaseType( + previousBase, + $"custom code declares base type '{currentBase?.FullyQualifiedName}'"); + return currentBase; + } + + if (!TryResolveTypeInCurrentBuild(previousBase, out var resolvedPreviousBaseProvider)) + { + CodeModelGenerator.Instance.Emitter.ReportDiagnostic( + DiagnosticCodes.UnavailableBackcompatType, + $"Could not preserve base type '{previousBase.FullyQualifiedName}' on model '{BuildNamespace()}.{BuildName()}' because the previous base is unavailable or does not expose an accessible parameterless constructor in the current build."); + return currentBase; + } + + if (HasDirectPropertyNameCollision(resolvedPreviousBaseProvider)) + { + ReportIncompatibleBackcompatBaseType( + previousBase, + "the current model directly declares a property from the previous base hierarchy"); + return currentBase; + } + + var resolvedPreviousBase = GetResolvedBaseType(previousBase, resolvedPreviousBaseProvider); + CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap[resolvedPreviousBase] = resolvedPreviousBaseProvider; + CodeModelGenerator.Instance.Emitter.Info( + $"Changed base type of model '{BuildName()}' from '{currentBase?.FullyQualifiedName ?? "object"}' to '{resolvedPreviousBase.FullyQualifiedName}' to match the last contract.", + BackCompatibilityChangeCategory.ModelBaseTypePreserved); + return resolvedPreviousBase; + } + + /// + /// Reports that a last-contract base type cannot be restored without making the current model invalid. + /// + /// The base type from the last contract. + /// The reason the base type cannot be restored. + private void ReportIncompatibleBackcompatBaseType(CSharpType previousBase, string reason) + { + CodeModelGenerator.Instance.Emitter.ReportDiagnostic( + DiagnosticCodes.IncompatibleBackcompatBaseType, + $"Could not preserve base type '{previousBase.FullyQualifiedName}' on model '{BuildNamespace()}.{BuildName()}' because {reason}."); + } + + private CSharpType? BuildCurrentBaseType() { if (CustomCodeView?.BaseType != null) { @@ -288,12 +356,191 @@ protected override string BuildNamespace() => string.IsNullOrEmpty(_inputModel.N return customBase; } - if (_inputModel.BaseModel == null) + return _inputModel.BaseModel is null + ? null + : CodeModelGenerator.Instance.TypeFactory.CreateModel(_inputModel.BaseModel)?.Type; + } + + protected static bool IsInBaseTypeHierarchy(CSharpType? currentBase, CSharpType previousBase) + { + var visited = new HashSet(StringComparer.Ordinal); + for (var type = currentBase; type is not null && visited.Add(GetMetadataTypeIdentity(type)); type = type.BaseType) { - return null; + if (AreMetadataTypesEqual(type, previousBase)) + { + return true; + } + } + + return false; + } + + private bool HasDirectPropertyNameCollision(TypeProvider previousBase) + { + var enclosingTypeName = BuildName(); + var directPropertyNames = _inputModel.Properties + .Select(property => GetGeneratedPropertyName(property, enclosingTypeName)) + .ToHashSet(StringComparer.Ordinal); + if (directPropertyNames.Count == 0) + { + return false; } - return CodeModelGenerator.Instance.TypeFactory.CreateModel(_inputModel.BaseModel)?.Type; + var visited = new HashSet(); + for (TypeProvider? provider = previousBase; provider is not null && visited.Add(provider); provider = provider.BaseTypeProvider) + { + if (provider.Properties.Any(property => + IsInheritedProperty(property) && directPropertyNames.Contains(property.Name))) + { + return true; + } + } + + return false; + } + + private static bool IsInheritedProperty(PropertyProvider property) + => !property.Modifiers.HasFlag(MethodSignatureModifiers.Private) || + property.Modifiers.HasFlag(MethodSignatureModifiers.Protected); + + private string GetGeneratedPropertyName(InputModelProperty property, string enclosingTypeName) + { + var propertyType = CodeModelGenerator.Instance.TypeFactory.CreateCSharpType(property.Type); + return propertyType is null + ? PropertyProvider.AvoidPropertyNameCollision( + property.IsExactName + ? property.Name + : property.Name.ToIdentifierName().NormalizeCSharpAcronyms(property.Type.IsDateTimeInputType()), + enclosingTypeName) + : PropertyProvider.GetPropertyName(property, propertyType, this, enclosingTypeName); + } + + private bool TryResolveTypeInCurrentBuild(CSharpType type, [NotNullWhen(true)] out TypeProvider? resolvedProvider) + { + foreach (var provider in CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.Values) + { + if (provider is not null && + AreMetadataTypesEqual(provider.Type, type) && + TryUseProviderAsBase(provider, out resolvedProvider)) + { + return true; + } + } + + // The previous base may occur later in input order. Force-create all input models before + // deciding that no generated provider is available. + foreach (var model in CodeModelGenerator.Instance.InputLibrary.InputNamespace.Models) + { + CodeModelGenerator.Instance.TypeFactory.CreateModel(model); + } + + foreach (var provider in CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.Values) + { + if (provider is not null && + AreMetadataTypesEqual(provider.Type, type) && + TryUseProviderAsBase(provider, out resolvedProvider)) + { + return true; + } + } + + var currentProvider = CodeModelGenerator.Instance.SourceInputModel.FindForTypeInCurrentCompilation( + GetMetadataNamespace(type), + GetMetadataSimpleName(type), + type.DeclaringType?.ClrMetadataName, + includeReferencedAssemblies: true); + if (currentProvider is null) + { + resolvedProvider = null; + return false; + } + + CodeModelGenerator.Instance.TypeFactory.CSharpTypeMap[currentProvider.Type] = currentProvider; + return TryUseProviderAsBase(currentProvider, out resolvedProvider); + } + + private static string GetMetadataSimpleName(CSharpType type) + { + var metadataName = type.ClrMetadataName; + var separatorIndex = metadataName.LastIndexOf('+'); + return separatorIndex < 0 ? metadataName : metadataName[(separatorIndex + 1)..]; + } + + private static CSharpType GetResolvedBaseType(CSharpType requestedType, TypeProvider resolvedProvider) + { + var resolvedType = resolvedProvider.Type; + return resolvedProvider is NamedTypeSymbolProvider + ? ApplyTypeConstruction(resolvedType, requestedType) + : resolvedType; + } + + private static CSharpType ApplyTypeConstruction(CSharpType resolvedType, CSharpType requestedType) + { + var declaringType = resolvedType.DeclaringType; + if (declaringType is not null && requestedType.DeclaringType is not null) + { + declaringType = ApplyTypeConstruction(declaringType, requestedType.DeclaringType); + } + + var arguments = resolvedType.Arguments.Count == requestedType.Arguments.Count + ? requestedType.Arguments + : resolvedType.Arguments; + return new CSharpType( + resolvedType.Name, + resolvedType.Namespace, + resolvedType.IsValueType, + resolvedType.IsNullable, + declaringType, + arguments, + resolvedType.IsPublic, + resolvedType.IsStruct, + resolvedType.BaseType); + } + + private static bool AreMetadataTypesEqual(CSharpType left, CSharpType right) + => string.Equals( + GetMetadataTypeIdentity(left), + GetMetadataTypeIdentity(right), + StringComparison.Ordinal); + + private static string GetMetadataTypeIdentity(CSharpType type) + { + var typeArguments = type.Arguments.Count == 0 + ? string.Empty + : $"[{string.Join(",", type.Arguments.Select(GetMetadataTypeIdentity))}]"; + var declaringType = type.DeclaringType is null + ? string.Empty + : $"@{GetMetadataTypeIdentity(type.DeclaringType)}"; + return $"{GetMetadataNamespace(type)}.{type.ClrMetadataName}{typeArguments}{declaringType}"; + } + + private static string GetMetadataNamespace(CSharpType type) + { + while (type.DeclaringType is not null) + { + type = type.DeclaringType; + } + + return type.Namespace; + } + + private static bool TryUseProviderAsBase(TypeProvider provider, [NotNullWhen(true)] out TypeProvider? resolvedProvider) + { + // Generated model bases already participate in ModelProvider's constructor chaining. A + // symbol-backed base does not, so generated constructors can only rely on an accessible + // parameterless constructor (explicit or implicit). + if (provider is ModelProvider || + provider is NamedTypeSymbolProvider { HasAccessibleParameterlessConstructor: true } || + provider is not NamedTypeSymbolProvider && provider.Constructors.Any(c => + c.Signature.Parameters.Count == 0 && + MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))) + { + resolvedProvider = provider; + return true; + } + + resolvedProvider = null; + return false; } protected override TypeProvider[] BuildSerializationProviders() diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs index 1a822a16db8..4227d3c07c3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs @@ -50,6 +50,18 @@ internal string MetadataName internal string MetadataSimpleName => _metadataSimpleName ??= _namedTypeSymbol.Name; + internal bool HasAccessibleParameterlessConstructor => _namedTypeSymbol.InstanceConstructors.Any(constructor => + constructor.Parameters.Length == 0 && IsConstructorAccessibleFromGeneratedType(constructor)); + + private bool IsConstructorAccessibleFromGeneratedType(IMethodSymbol constructor) + => constructor.DeclaredAccessibility switch + { + Accessibility.Public or Accessibility.Protected or Accessibility.ProtectedOrInternal => true, + Accessibility.Internal or Accessibility.ProtectedAndInternal => + SymbolEqualityComparer.Default.Equals(constructor.ContainingAssembly, _compilation.Assembly), + _ => false, + }; + private protected sealed override NamedTypeSymbolProvider? BuildCustomCodeView(string? generatedTypeName = default, string? generatedTypeNamespace = default) => null; private protected sealed override TypeProvider? BuildLastContractView(string? generatedTypeName = default, string? generatedTypeNamespace = default) => null; @@ -72,6 +84,10 @@ private static string GetMetadataName(INamedTypeSymbol symbol) protected override string BuildNamespace() => _namedTypeSymbol.ContainingNamespace.GetFullyQualifiedNameFromDisplayString(); + protected override TypeProvider? BuildDeclaringTypeProvider() => _namedTypeSymbol.ContainingType is null + ? null + : new NamedTypeSymbolProvider(_namedTypeSymbol.ContainingType, _compilation); + protected override IReadOnlyList BuildAttributes() => [.._namedTypeSymbol.GetAttributes().Select(a => new AttributeStatement(a))]; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PropertyProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PropertyProvider.cs index 758c7dbe09f..d642ab2b081 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PropertyProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PropertyProvider.cs @@ -102,44 +102,7 @@ private PropertyProvider(InputProperty inputProperty, CSharpType propertyType, T IsDiscriminator = IsDiscriminatorProperty(inputProperty); var hasOutputUsage = inputProperty.EnclosingType?.Usage.HasFlag(InputModelTypeUsage.Output) ?? false; Modifiers = IsDiscriminator || (!hasOutputUsage && _isRequiredNonNullableConstant) ? MethodSignatureModifiers.Internal : MethodSignatureModifiers.Public; - var identifierName = inputProperty.IsExactName ? inputProperty.Name : inputProperty.Name.ToIdentifierName(); - if (!inputProperty.IsExactName) - { - var isDateTime = inputProperty.Type.IsDateTimeInputType(); - var canonicalName = identifierName.NormalizeCSharpAcronyms(isDateTime); - var enclosingTypeName = enclosingType.Name; - var lastContractProperties = enclosingType.LastContractView?.Properties - .Where(p => MethodSignatureHelper.IsPublicApi(p.Modifiers)) - .ToList(); - - // An exact input-identifier match is authoritative: it is the name this property would have had - // before normalization, so it does not need the disambiguation required by normalized candidates. - // The shipped member may carry the enclosing-type collision suffix, so candidates are compared - // against the collision-adjusted form of each name we are looking for. - var previousProperty = - lastContractProperties?.FirstOrDefault(p => p.Name == AvoidPropertyNameCollision(identifierName, enclosingTypeName)) - ?? lastContractProperties?.FirstOrDefault(p => - p.Name == AvoidPropertyNameCollision(canonicalName, enclosingTypeName) && - !IsClaimedBySiblingProperty(p.Name, inputProperty, enclosingType)); - - if (previousProperty is null && - isDateTime && - !identifierName.EndsWith("On", StringComparison.Ordinal)) - { - // Both the current and previous conventions render date-time names as On. Requiring - // that suffix on the contract name prevents a removed property such as StartDate from being - // mistaken for the historical name of StartTime even though both normalize to StartsOn. - var specStem = identifierName.NormalizeCSharpAcronyms().GetDateTimeStem(); - previousProperty = specStem is null - ? null - : lastContractProperties?.FirstOrDefault(p => - HasDateTimeStem(p.Name, specStem, enclosingTypeName) && - p.Type.WithNullable(false).Equals(Type.WithNullable(false)) && - !IsClaimedBySiblingProperty(p.Name, inputProperty, enclosingType)); - } - identifierName = previousProperty?.Name ?? canonicalName; - } - Name = AvoidPropertyNameCollision(identifierName, enclosingType.Name); + Name = GetPropertyName(inputProperty, Type, enclosingType, enclosingType.Name); Body = new AutoPropertyBody(propHasSetter, setterModifier, GetPropertyInitializationValue(propertyType, inputProperty)); WireInfo = new PropertyWireInformation(inputProperty); @@ -203,7 +166,52 @@ private void BuildDocs() } } - private static string AvoidPropertyNameCollision(string propertyName, string enclosingTypeName) => + internal static string GetPropertyName( + InputProperty inputProperty, + CSharpType propertyType, + TypeProvider enclosingType, + string enclosingTypeName) + { + var identifierName = inputProperty.IsExactName ? inputProperty.Name : inputProperty.Name.ToIdentifierName(); + if (!inputProperty.IsExactName) + { + var isDateTime = inputProperty.Type.IsDateTimeInputType(); + var canonicalName = identifierName.NormalizeCSharpAcronyms(isDateTime); + var lastContractProperties = enclosingType.LastContractView?.Properties + .Where(p => MethodSignatureHelper.IsPublicApi(p.Modifiers)) + .ToList(); + + // An exact input-identifier match is authoritative: it is the name this property would have had + // before normalization, so it does not need the disambiguation required by normalized candidates. + // The shipped member may carry the enclosing-type collision suffix, so candidates are compared + // against the collision-adjusted form of each name we are looking for. + var previousProperty = + lastContractProperties?.FirstOrDefault(p => p.Name == AvoidPropertyNameCollision(identifierName, enclosingTypeName)) + ?? lastContractProperties?.FirstOrDefault(p => + p.Name == AvoidPropertyNameCollision(canonicalName, enclosingTypeName) && + !IsClaimedBySiblingProperty(p.Name, inputProperty, enclosingType, enclosingTypeName)); + + if (previousProperty is null && + isDateTime && + !identifierName.EndsWith("On", StringComparison.Ordinal)) + { + // Both the current and previous conventions render date-time names as On. Requiring + // that suffix on the contract name prevents a removed property such as StartDate from being + // mistaken for the historical name of StartTime even though both normalize to StartsOn. + var specStem = identifierName.NormalizeCSharpAcronyms().GetDateTimeStem(); + previousProperty = specStem is null + ? null + : lastContractProperties?.FirstOrDefault(p => + HasDateTimeStem(p.Name, specStem, enclosingTypeName) && + p.Type.WithNullable(false).Equals(propertyType.WithNullable(false)) && + !IsClaimedBySiblingProperty(p.Name, inputProperty, enclosingType, enclosingTypeName)); + } + identifierName = previousProperty?.Name ?? canonicalName; + } + return AvoidPropertyNameCollision(identifierName, enclosingTypeName); + } + + internal static string AvoidPropertyNameCollision(string propertyName, string enclosingTypeName) => propertyName == enclosingTypeName ? $"{propertyName}Property" : propertyName; private static bool HasDateTimeStem(string contractName, string specStem, string enclosingTypeName) @@ -220,10 +228,9 @@ private static bool HasDateTimeStem(string contractName, string specStem, string private static bool IsClaimedBySiblingProperty( string contractName, InputProperty inputProperty, - TypeProvider enclosingType) + TypeProvider enclosingType, + string enclosingTypeName) { - var enclosingTypeName = enclosingType.Name; - foreach (var sibling in inputProperty.EnclosingType?.Properties ?? []) { if (ReferenceEquals(sibling, inputProperty)) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/DiagnosticCodes.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/DiagnosticCodes.cs index 35a67676649..ec18200d1f9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/DiagnosticCodes.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/DiagnosticCodes.cs @@ -9,5 +9,6 @@ internal static class DiagnosticCodes public const string InvalidAccessModifier = "invalid-access-modifier"; public const string PluginBuildFailed = "plugin-build-failed"; public const string UnavailableBackcompatType = "unavailable-backcompat-type"; + public const string IncompatibleBackcompatBaseType = "incompatible-backcompat-base-type"; } } 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 f0c040b1432..ecbe7ff7e16 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 @@ -8,6 +8,8 @@ using System.IO; using System.Linq; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; @@ -502,6 +504,431 @@ public void BuildBaseType() Assert.AreEqual(baseModel!.Type, derivedModel!.Type.BaseType); } + [Test] + public async Task BackCompat_BaseTypeChangePreservesLastContractBaseType() + { + var previousBase = InputFactory.Model("PreviousBase", properties: []); + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [previousBase, currentBase, derivedModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "DerivedModel"); + + Assert.AreEqual("PreviousBase", modelProvider.LastContractView?.BaseType?.Name, + "The regression requires the previously shipped base type to be available from the last contract"); + + modelProvider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(previousBase.Name, modelProvider.BaseType?.Name, + "A model must remain assignable to its previously shipped CLR base type"); + } + + [Test] + public async Task BackCompat_BaseTypeHookCanKeepCurrentBaseType() + { + var previousBase = InputFactory.Model("PreviousBase", properties: []); + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + createModelCore: input => input == derivedModel + ? new BaseTypeBackCompatibilityOverridingModelProvider(input) + : new ModelProvider(input), + inputModelTypes: [previousBase, currentBase, derivedModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync( + method: nameof(BackCompat_BaseTypeChangePreservesLastContractBaseType))); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(); + + Assert.Multiple(() => + { + Assert.AreEqual(previousBase.Name, modelProvider.LastContractView?.BaseType?.Name, + "The default back-compat behavior would restore the previous base"); + Assert.AreEqual(currentBase.Name, modelProvider.CapturedCurrentBase?.Name, + "The hook should receive the base selected from the current input model"); + Assert.AreEqual(currentBase.Name, modelProvider.BaseType?.Name, + "A downstream provider can override the hook to retain the current base"); + }); + } + + [TestCase("sharedProperty", "sharedProperty", false, false)] + [TestCase("sharedProperty", "sharedProperty", true, false)] + [TestCase("shared-property", "shared_property", false, false)] + [TestCase("previousBase", "previousBaseProperty", false, false)] + [TestCase("IpAddress", "IpAddress", false, true)] + public async Task BackCompat_GeneratedLastContractBaseWithPropertyCollisionIsNotRestored( + string previousPropertyName, + string currentPropertyName, + bool hasMismatchedType, + bool previousPropertyIsExact) + { + var previousBase = InputFactory.Model( + "PreviousBase", + properties: [InputFactory.Property(previousPropertyName, InputPrimitiveType.String, isExactName: previousPropertyIsExact)]); + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model( + "DerivedModel", + properties: + [ + InputFactory.Property( + currentPropertyName, + hasMismatchedType ? InputPrimitiveType.Int32 : InputPrimitiveType.String) + ], + baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [previousBase, currentBase, derivedModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProviders = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .ToArray(); + var derivedProvider = modelProviders.Single(t => t.Name == "DerivedModel"); + + derivedProvider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(currentBase.Name, derivedProvider.BaseType?.Name, + "The previous base must not be restored when it would collide with a directly declared current property"); + + var syntaxTrees = modelProviders.Select(provider => + CSharpSyntaxTree.ParseText(new TypeProviderWriter(provider).Write().Content)); + var references = AppDomain.CurrentDomain.GetAssemblies() + .Where(a => !a.IsDynamic && !string.IsNullOrEmpty(a.Location)) + .Select(a => MetadataReference.CreateFromFile(a.Location)); + var compilation = CSharpCompilation.Create( + "PropertyCollisionModels", + syntaxTrees, + references, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + Assert.That( + compilation.GetDiagnostics().Where(d => d.Severity is DiagnosticSeverity.Warning or DiagnosticSeverity.Error), + Is.Empty, + "The generated model hierarchy should compile after the incompatible base restoration is skipped"); + } + + [Test] + public async Task BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored() + { + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model( + "DerivedModel", + properties: [InputFactory.Property("sharedProperty", InputPrimitiveType.String)], + baseModel: currentBase); + var privatePropertyDerivedModel = InputFactory.Model( + "PrivatePropertyDerivedModel", + properties: [InputFactory.Property("privateProperty", InputPrimitiveType.String)], + baseModel: currentBase); + + var mockGenerator = await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [currentBase, derivedModel, privatePropertyDerivedModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync("Current"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync("LastContract")); + + var modelProviders = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .ToArray(); + var modelProvider = modelProviders.Single(t => t.Name == "DerivedModel"); + var privatePropertyModelProvider = modelProviders.Single(t => t.Name == "PrivatePropertyDerivedModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + privatePropertyModelProvider.ProcessTypeForBackCompatibility(); + + Assert.Multiple(() => + { + Assert.AreEqual(currentBase.Name, modelProvider.BaseType?.Name, + "The previous symbol-backed base must not be restored when one of its properties collides with a directly declared current property"); + Assert.AreEqual("ExternalBase", privatePropertyModelProvider.BaseType?.Name, + "A private base property is not inherited and must not block restoration"); + }); + + var syntaxTrees = modelProviders + .Select(provider => CSharpSyntaxTree.ParseText(new TypeProviderWriter(provider).Write().Content)) + .Concat(mockGenerator.Object.SourceInputModel.Customization!.SyntaxTrees.Where(tree => + Path.GetFileName(tree.FilePath) == "ExternalBase.cs")); + var references = AppDomain.CurrentDomain.GetAssemblies() + .Where(a => !a.IsDynamic && !string.IsNullOrEmpty(a.Location)) + .Select(a => MetadataReference.CreateFromFile(a.Location)); + var compilation = CSharpCompilation.Create( + "SymbolBackedPropertyCollisionModels", + syntaxTrees, + references, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + Assert.That( + compilation.GetDiagnostics().Where(d => d.Severity is DiagnosticSeverity.Warning or DiagnosticSeverity.Error), + Is.Empty, + "The generated model hierarchy should compile after the incompatible symbol-backed base restoration is skipped"); + } + + [Test] + public async Task BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase() + { + var previousBase = InputFactory.Model("PreviousBase", properties: []); + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [previousBase, currentBase, derivedModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync("Current"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync("LastContract")); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "DerivedModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual("CustomBase", modelProvider.BaseType?.Name, + "An explicit custom base must remain authoritative when the previous base cannot be restored without conflicting partial declarations"); + } + + [Test] + public async Task BackCompat_CurrentBaseDerivedFromLastContractBaseIsPreserved() + { + var previousBase = InputFactory.Model("PreviousBase", properties: []); + var currentBase = InputFactory.Model("CurrentBase", properties: [], baseModel: previousBase); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [previousBase, currentBase, derivedModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync( + method: nameof(BackCompat_BaseTypeChangePreservesLastContractBaseType))); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "DerivedModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(currentBase.Name, modelProvider.BaseType?.Name, + "The current base should remain when it already derives from the previously shipped base"); + } + + [Test] + public async Task BackCompat_BaseTypeChangePreservesNonGeneratedLastContractBaseType() + { + const string nestedBaseSource = """ + namespace Sample.Models + { + public class Outer + { + public class Middle + { + public class NestedBase + { + } + } + } + + public class OtherOuter + { + public class Middle + { + public class NestedBase + { + } + } + } + + public class GenericBase + { + } + } + """; + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + var nestedDerivedModel = InputFactory.Model("NestedDerivedModel", properties: [], baseModel: currentBase); + var genericDerivedModel = InputFactory.Model("GenericDerivedModel", properties: [], baseModel: currentBase); + + var mockGenerator = await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [currentBase, derivedModel, nestedDerivedModel, genericDerivedModel], + compilation: async () => await Helpers.GetCompilationFromSourceFilesAsync( + [("NestedBase.cs", nestedBaseSource)]), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var placeholderType = new CSharpType(typeof(Exception)); + mockGenerator.Object.TypeFactory.CSharpTypeMap[placeholderType] = new SystemObjectTypeProvider(placeholderType); + var unrelatedNestedBase = mockGenerator.Object.SourceInputModel.FindForTypeInCurrentCompilation( + "Sample.Models", + "NestedBase", + "OtherOuter+Middle"); + Assert.IsNotNull(unrelatedNestedBase); + mockGenerator.Object.TypeFactory.CSharpTypeMap[unrelatedNestedBase!.Type] = unrelatedNestedBase; + + var modelProviders = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .ToArray(); + var modelProvider = modelProviders.Single(t => t.Name == "DerivedModel"); + var nestedModelProvider = modelProviders.Single(t => t.Name == "NestedDerivedModel"); + var genericModelProvider = modelProviders.Single(t => t.Name == "GenericDerivedModel"); + + Assert.Multiple(() => + { + Assert.AreEqual(nameof(Exception), modelProvider.LastContractView?.BaseType?.Name); + Assert.AreEqual(nameof(System), modelProvider.LastContractView?.BaseType?.Namespace, + "The regression requires the previously shipped framework base type to be available from the last contract"); + Assert.AreEqual("NestedBase", nestedModelProvider.LastContractView?.BaseType?.Name, + "The regression requires a multi-level nested base type"); + Assert.AreEqual("GenericBase", genericModelProvider.LastContractView?.BaseType?.Name, + "The regression requires a constructed generic base type"); + }); + + modelProvider.ProcessTypeForBackCompatibility(); + nestedModelProvider.ProcessTypeForBackCompatibility(); + genericModelProvider.ProcessTypeForBackCompatibility(); + + Assert.Multiple(() => + { + Assert.AreEqual(nameof(Exception), modelProvider.BaseType?.Name, + "A model must remain assignable to its previously shipped non-generated CLR base type"); + Assert.AreEqual(nameof(System), modelProvider.BaseType?.Namespace); + Assert.IsInstanceOf(modelProvider.BaseTypeProvider, + "The preserved base should resolve from the current referenced assemblies without a generated model provider"); + Assert.AreEqual("NestedBase", nestedModelProvider.BaseType?.Name, + "A nested base should resolve using its complete CLR declaring-type metadata name"); + Assert.AreEqual("Outer", nestedModelProvider.BaseType?.DeclaringType?.DeclaringType?.Name); + Assert.AreEqual("GenericBase", genericModelProvider.BaseType?.Name, + "A generic base should resolve using its metadata arity"); + Assert.That(genericModelProvider.BaseType?.Arguments, Has.Count.EqualTo(1)); + Assert.AreEqual("global::Sample.Models.GenericBase", genericModelProvider.BaseType?.ToString(), + "The restored base must retain its last-contract generic construction"); + }); + } + + [Test] + public async Task BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored() + { + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [currentBase, derivedModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync("Current"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync("LastContract")); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "DerivedModel"); + + Assert.AreEqual("ExternalBase", modelProvider.LastContractView?.BaseType?.Name, + "The regression requires a resolvable previous base without a parameterless constructor"); + + modelProvider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(currentBase.Name, modelProvider.BaseType?.Name, + "The previous base must not be restored when generated constructors cannot chain to it"); + } + + [Test] + public async Task BackCompat_ReferencedLastContractBaseWithInternalParameterlessConstructorIsNotRestored() + { + const string externalBaseSource = """ + namespace Sample.Models + { + public class ExternalBase + { + internal ExternalBase() { } + public ExternalBase(string value) { } + } + } + """; + var externalCompilation = CSharpCompilation.Create( + "ExternalAssembly", + [CSharpSyntaxTree.ParseText(externalBaseSource)], + [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)], + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + using var externalAssembly = new MemoryStream(); + var emitResult = externalCompilation.Emit(externalAssembly); + Assert.That(emitResult.Success, Is.True, string.Join(Environment.NewLine, emitResult.Diagnostics)); + var externalReference = MetadataReference.CreateFromImage(externalAssembly.ToArray()); + + var currentBase = InputFactory.Model("CurrentBase", properties: []); + var derivedModel = InputFactory.Model("DerivedModel", properties: [], baseModel: currentBase); + + var mockGenerator = await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [currentBase, derivedModel], + additionalMetadataReferences: [externalReference], + compilation: async () => + { + var compilation = await Helpers.GetCompilationFromSourceFilesAsync([]); + return compilation.WithOptions( + ((CSharpCompilationOptions)compilation.Options).WithMetadataImportOptions(MetadataImportOptions.All)); + }, + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync("LastContract")); + + var referencedBase = mockGenerator.Object.SourceInputModel.FindForTypeInCurrentCompilation( + "Sample.Models", "ExternalBase", includeReferencedAssemblies: true); + Assert.IsNotNull(referencedBase, "The previous base must resolve from the referenced assembly"); + Assert.That(referencedBase!.Constructors, Has.Some.Matches(c => + c.Signature.Parameters.Count == 0 && c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal))); + Assert.That(referencedBase.Constructors, Has.Some.Matches(c => + c.Signature.Parameters.Count == 1 && c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public))); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "DerivedModel"); + + Assert.AreEqual("ExternalBase", modelProvider.LastContractView?.BaseType?.Name, + "The regression requires a previous base that resolves from a referenced assembly"); + + modelProvider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(currentBase.Name, modelProvider.BaseType?.Name, + "An internal constructor from a referenced assembly is not accessible to the generated derived model"); + } + + [Test] + public async Task BackCompat_LastContractBaseRestoresInheritedProperties() + { + var previousBase = InputFactory.Model( + "PreviousBase", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String), + InputFactory.Property("location", InputPrimitiveType.String), + InputFactory.Property("tags", InputFactory.Dictionary(InputPrimitiveType.String)), + ]); + var currentBase = InputFactory.Model( + "CurrentBase", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String), + InputFactory.Property("location", InputPrimitiveType.String), + InputFactory.Property("tags", InputFactory.Dictionary(InputPrimitiveType.String)), + ]); + var derivedModel = InputFactory.Model( + "DerivedModel", + properties: [InputFactory.Property("childProp", InputPrimitiveType.String)], + baseModel: currentBase); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [previousBase, currentBase, derivedModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "DerivedModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + Assert.Multiple(() => + { + Assert.AreEqual(previousBase.Name, modelProvider.BaseType?.Name, + "The previously shipped base type must be preserved"); + Assert.That(modelProvider.BaseTypeProvider?.Properties.Select(p => p.Name), + Is.EquivalentTo(new[] { "Id", "Location", "Tags" }), + "Properties shipped on the previous GA base must remain inherited"); + Assert.That(modelProvider.Properties.Select(p => p.Name), Is.EqualTo(new[] { "ChildProp" }), + "Properties supplied by the preserved base must not be duplicated on the derived model"); + }); + } + [Test] public void OverridingBuildBaseType_AutoResolvesBaseModelProviderForGeneratedModel() { @@ -603,6 +1030,21 @@ public BuildBaseTypeOverridingModelProvider(InputModelType inputModel, CSharpTyp protected override CSharpType? BuildBaseType() => _redirectedBaseType; } + private sealed class BaseTypeBackCompatibilityOverridingModelProvider : ModelProvider + { + public BaseTypeBackCompatibilityOverridingModelProvider(InputModelType inputModel) : base(inputModel) + { + } + + public CSharpType? CapturedCurrentBase { get; private set; } + + protected override CSharpType? BuildBaseTypeForBackCompatibility(CSharpType? currentBase) + { + CapturedCurrentBase = currentBase; + return currentBase; + } + } + // Regression: custom code (such as an inheritable system base model) can produce a base // ModelProvider chain that cycles back on itself. Base-model traversal during constructor, // field, and raw-data discovery must terminate instead of recursing infinitely. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesLastContractBaseType/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesLastContractBaseType/Models.cs new file mode 100644 index 00000000000..d84c8dea3a1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesLastContractBaseType/Models.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public partial class PreviousBase + { + } + + public partial class DerivedModel : PreviousBase + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesNonGeneratedLastContractBaseType/DerivedModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesNonGeneratedLastContractBaseType/DerivedModel.cs new file mode 100644 index 00000000000..bdb693b1275 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_BaseTypeChangePreservesNonGeneratedLastContractBaseType/DerivedModel.cs @@ -0,0 +1,28 @@ +namespace Sample.Models +{ + public partial class DerivedModel : System.Exception + { + } + + public class Outer + { + public class Middle + { + public class NestedBase + { + } + } + } + + public partial class NestedDerivedModel : Outer.Middle.NestedBase + { + } + + public class GenericBase + { + } + + public partial class GenericDerivedModel : GenericBase + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(Current)/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(Current)/Models.cs new file mode 100644 index 00000000000..7211d5622b1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(Current)/Models.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public class CustomBase + { + } + + public partial class DerivedModel : CustomBase + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(LastContract)/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(LastContract)/Models.cs new file mode 100644 index 00000000000..c1fc50ee8a4 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_CustomBaseTakesPrecedenceOverDifferentLastContractBase(LastContract)/Models.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public class PreviousBase + { + } + + public class DerivedModel : PreviousBase + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_GeneratedLastContractBaseWithPropertyCollisionIsNotRestored/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_GeneratedLastContractBaseWithPropertyCollisionIsNotRestored/Models.cs new file mode 100644 index 00000000000..5b89f09f7bb --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_GeneratedLastContractBaseWithPropertyCollisionIsNotRestored/Models.cs @@ -0,0 +1,12 @@ +namespace Sample.Models +{ + public partial class PreviousBase + { + public string SharedProperty { get; set; } + } + + public partial class DerivedModel : PreviousBase + { + public string IpAddress { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_LastContractBaseRestoresInheritedProperties/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_LastContractBaseRestoresInheritedProperties/Models.cs new file mode 100644 index 00000000000..b2508ddfa0d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_LastContractBaseRestoresInheritedProperties/Models.cs @@ -0,0 +1,16 @@ +using System.Collections.Generic; + +namespace Sample.Models +{ + public partial class PreviousBase + { + public string Id { get; set; } + public string Location { get; set; } + public IDictionary Tags { get; set; } + } + + public partial class DerivedModel : PreviousBase + { + public string ChildProp { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(Current)/ExternalBase.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(Current)/ExternalBase.cs new file mode 100644 index 00000000000..f851a660c28 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(Current)/ExternalBase.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public class ExternalBase + { + public ExternalBase(string value) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(LastContract)/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(LastContract)/Models.cs new file mode 100644 index 00000000000..50a4ead7471 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_NonGeneratedLastContractBaseWithoutParameterlessConstructorIsNotRestored(LastContract)/Models.cs @@ -0,0 +1,16 @@ +namespace Sample.Models +{ + public class ExternalBase + { + public ExternalBase(string value) + { + } + } + + public class DerivedModel : ExternalBase + { + public DerivedModel(string value) : base(value) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ReferencedLastContractBaseWithInternalParameterlessConstructorIsNotRestored(LastContract)/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ReferencedLastContractBaseWithInternalParameterlessConstructorIsNotRestored(LastContract)/Models.cs new file mode 100644 index 00000000000..02b8353795d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ReferencedLastContractBaseWithInternalParameterlessConstructorIsNotRestored(LastContract)/Models.cs @@ -0,0 +1,20 @@ +namespace Sample.Models +{ + public class ExternalBase + { + internal ExternalBase() + { + } + + public ExternalBase(string value) + { + } + } + + public class DerivedModel : ExternalBase + { + public DerivedModel() + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored(Current)/ExternalBase.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored(Current)/ExternalBase.cs new file mode 100644 index 00000000000..3fcbfd73534 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored(Current)/ExternalBase.cs @@ -0,0 +1,8 @@ +namespace Sample.Models +{ + public class ExternalBase + { + public string SharedProperty { get; set; } + private string PrivateProperty { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored(LastContract)/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored(LastContract)/Models.cs new file mode 100644 index 00000000000..23bab7f15fb --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_SymbolBackedLastContractBaseWithPropertyCollisionIsNotRestored(LastContract)/Models.cs @@ -0,0 +1,16 @@ +namespace Sample.Models +{ + public class ExternalBase + { + public string SharedProperty { get; set; } + private string PrivateProperty { get; set; } + } + + public class DerivedModel : ExternalBase + { + } + + public class PrivatePropertyDerivedModel : ExternalBase + { + } +}