From 5526d4cfec077a3f2134abd0f325297652d72c9f Mon Sep 17 00:00:00 2001 From: jolov Date: Sat, 22 Aug 2026 11:19:34 -0700 Subject: [PATCH] Invalidate full constructor with constructor cache Ensure ModelProvider clears its cached FullConstructor whenever TypeProvider invalidates the constructor list, including same-value identity updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Providers/ModelProvider.cs | 30 +++++-------------- .../src/Providers/TypeProvider.cs | 9 ++++-- .../ModelProviders/ModelProviderTests.cs | 12 ++++++++ 3 files changed, 26 insertions(+), 25 deletions(-) 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 6396b784520..540d6e02176 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,7 +67,6 @@ 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,11 +188,15 @@ public override void Reset() _rawDataField = null; _additionalPropertyFields = null; _additionalPropertyProperties = null; - _fullConstructor = null; - _fullConstructorIdentity = null; _isMultiLevelDiscriminator = null; } + private protected override void ResetConstructors() + { + base.ResetConstructors(); + _fullConstructor = null; + } + protected FieldProvider? RawDataField { get @@ -235,26 +238,7 @@ protected FieldProvider? RawDataField /// /// 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; - } - } + public ConstructorProvider FullConstructor => _fullConstructor ??= BuildFullConstructor(); 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/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index 4687727365e..ae89bad165a 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 @@ -690,7 +690,7 @@ public virtual void Reset() _methods = null; _properties = null; _fields = null; - _constructors = null; + ResetConstructors(); _implements = null; _serializationProviders = null; _nestedTypes = null; @@ -712,6 +712,11 @@ public virtual void Reset() _arguments = null; } + private protected virtual void ResetConstructors() + { + _constructors = null; + } + /// /// Updates the type provider with new values for its properties, methods, constructors, etc. /// @@ -830,7 +835,7 @@ private void ResetMembersBasedOnIdentityChange(string? name = null, string? @nam // recalculate declaration modifiers and constructors _declarationModifiers = null; // constructors might change based on declaration modifier changes - _constructors = null; + ResetConstructors(); // serialization providers need to reflect the new type name/namespace _serializationProviders = null; Type.Update(name: name, @namespace: @namespace); 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 72ab4b878c7..da225c14ffd 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 @@ -2385,6 +2385,13 @@ public void TestUpdate_ResetsFullConstructor() var newerFullConstructor = modelProvider.FullConstructor; Assert.AreNotSame(newFullConstructor, newerFullConstructor); Assert.IsTrue(modelProvider.Constructors.Contains(newerFullConstructor)); + + // Re-applying the current identity also invalidates the constructor list because + // customization metadata and declaration modifiers may have changed. + modelProvider.Update(name: modelProvider.Name); + var sameIdentityFullConstructor = modelProvider.FullConstructor; + Assert.AreNotSame(newerFullConstructor, sameIdentityFullConstructor); + Assert.IsTrue(modelProvider.Constructors.Contains(sameIdentityFullConstructor)); } // Regression coverage for the duplication that the stale FullConstructor caused: a generator that @@ -2404,6 +2411,11 @@ public void TestUpdate_DoesNotReapplyConstructorMutationsAfterIdentityChange() _ = modelProvider.Constructors; Assert.AreEqual(1, modelProvider.FullConstructor.Suppressions.Count); + + modelProvider.Update(name: modelProvider.Name); + _ = modelProvider.Constructors; + + Assert.AreEqual(1, modelProvider.FullConstructor.Suppressions.Count); } // Mimics how ScmModelProvider post-processes the constructors returned from the base implementation.