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 9e726fb171f..6396b784520 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 @@ -67,6 +67,7 @@ protected override FormattableString BuildDescription() private List? _additionalPropertyProperties; private ModelProvider? _baseModelProvider; private ConstructorProvider? _fullConstructor; + private (string Name, string Namespace)? _fullConstructorIdentity; internal PropertyProvider? DiscriminatorProperty { get; private set; } private readonly bool _isDiscriminatedBaseType; @@ -189,6 +190,7 @@ public override void Reset() _additionalPropertyFields = null; _additionalPropertyProperties = null; _fullConstructor = null; + _fullConstructorIdentity = null; _isMultiLevelDiscriminator = null; } @@ -230,7 +232,29 @@ protected FieldProvider? RawDataField protected internal bool SupportsBinaryDataAdditionalProperties => AdditionalPropertyProperties.Any(p => p.Type.ElementType.Equals(_additionalPropsUnknownType) || (p.Type.ElementType.IsFrameworkType && p.Type.ElementType.FrameworkType == typeof(object))); - public ConstructorProvider FullConstructor => _fullConstructor ??= BuildFullConstructor(); + /// + /// The constructor that takes every serializable property. + /// + /// + /// This instance is also returned as part of , and callers are free to + /// mutate the constructors they receive. An identity change invalidates the constructor list, so the cached + /// instance is rebuilt alongside it; otherwise a rebuild would reuse the same instance and re-apply any + /// mutation, producing duplicated members. + /// + public ConstructorProvider FullConstructor + { + get + { + var identity = (Type.Name, Type.Namespace); + if (_fullConstructor is null || _fullConstructorIdentity != identity) + { + _fullConstructor = BuildFullConstructor(); + _fullConstructorIdentity = identity; + } + + return _fullConstructor; + } + } protected override string BuildNamespace() => string.IsNullOrEmpty(_inputModel.Namespace) ? // TODO remove null check once https://github.com/Azure/typespec-azure/issues/2209 is fixed. 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 66ce8b8954e..4933eddc500 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 @@ -11,6 +11,7 @@ using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Snippets; using Microsoft.TypeSpec.Generator.Statements; using Microsoft.TypeSpec.Generator.Tests.Common; using Microsoft.TypeSpec.Generator.Utilities; @@ -2357,6 +2358,76 @@ public void TestUpdate_ResetsSerializationProviders() Assert.AreEqual(1, newerSerializationProviders.Count); } + // BuildConstructors returns the cached FullConstructor instance, so invalidating the constructor + // list on an identity change without invalidating FullConstructor left the stale instance in the + // rebuilt list. Derived generators mutate the constructors they get back from BuildConstructors, + // and reusing the same instance caused those mutations to be applied more than once. + [Test] + public void TestUpdate_ResetsFullConstructor() + { + MockHelpers.LoadMockGenerator(); + var inputModel = InputFactory.Model("TestModel", properties: [InputFactory.Property("prop1", InputPrimitiveType.String)]); + var modelProvider = new ModelProvider(inputModel); + + var fullConstructor = modelProvider.FullConstructor; + Assert.IsTrue(modelProvider.Constructors.Contains(fullConstructor)); + + // Change name + modelProvider.Update(name: "NewName"); + var newFullConstructor = modelProvider.FullConstructor; + Assert.AreNotSame(fullConstructor, newFullConstructor); + // The rebuilt constructor list must contain the rebuilt full constructor, not the stale one + Assert.IsTrue(modelProvider.Constructors.Contains(newFullConstructor)); + Assert.IsFalse(modelProvider.Constructors.Contains(fullConstructor)); + + // Change namespace + modelProvider.Update(@namespace: "NewNamespace"); + var newerFullConstructor = modelProvider.FullConstructor; + Assert.AreNotSame(newFullConstructor, newerFullConstructor); + Assert.IsTrue(modelProvider.Constructors.Contains(newerFullConstructor)); + } + + // Regression coverage for the duplication that the stale FullConstructor caused: a generator that + // adds a suppression to the full constructor every time it builds the constructor list must not see + // its additions accumulate when an identity change triggers a rebuild. + [Test] + public void TestUpdate_DoesNotReapplyConstructorMutationsAfterIdentityChange() + { + MockHelpers.LoadMockGenerator(); + var inputModel = InputFactory.Model("TestModel", properties: [InputFactory.Property("prop1", InputPrimitiveType.String)]); + var modelProvider = new MutatingModelProvider(inputModel); + + _ = modelProvider.Constructors; + Assert.AreEqual(1, modelProvider.FullConstructor.Suppressions.Count); + + modelProvider.Update(name: "NewName"); + _ = modelProvider.Constructors; + + Assert.AreEqual(1, modelProvider.FullConstructor.Suppressions.Count); + } + + // Mimics how ScmModelProvider post-processes the constructors returned from the base implementation. + private class MutatingModelProvider : ModelProvider + { + public MutatingModelProvider(InputModelType inputModel) : base(inputModel) + { + } + + protected internal override ConstructorProvider[] BuildConstructors() + { + var constructors = base.BuildConstructors(); + foreach (var constructor in constructors) + { + if (ReferenceEquals(constructor, FullConstructor)) + { + var suppression = new SuppressionStatement(null, Snippet.Literal("TEST0001"), "Test suppression."); + constructor.Update(suppressions: [suppression, .. constructor.Suppressions]); + } + } + return constructors; + } + } + private class TestModelProvider : ModelProvider { private readonly string? _name;