From caddb16b3be3ab932b66a8bbda36296455a00338 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Mon, 27 Jul 2026 17:35:31 -0500 Subject: [PATCH 1/4] chore: clean up TypeProvider attributes --- .../ModelReaderWriterContextDefinition.cs | 74 +++++++++++++------ ...ModelReaderWriterContextDefinitionTests.cs | 5 +- .../src/Providers/TypeProvider.cs | 68 ++++++++++++++--- .../ProviderReferenceMapAnalyzer.Helpers.cs | 11 --- 4 files changed, 113 insertions(+), 45 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs index 14e4ddef553..57b1044fff0 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs @@ -26,6 +26,10 @@ public class ModelReaderWriterContextDefinition : TypeProvider internal static readonly string s_name = $"{RemovePeriods(ScmCodeModelGenerator.Instance.TypeFactory.PrimaryNamespace)}Context"; + // Customized buildable types come from custom code, which is stable, so compute them once. + private HashSet? _customizedBuildableTypes; + private HashSet CustomizedBuildableTypes => _customizedBuildableTypes ??= BuildCustomizedBuildableTypes(); + protected override string BuildName() => s_name; protected override string BuildRelativeFilePath() => Path.Combine("src", "Generated", "Models", $"{Name}.cs"); @@ -39,16 +43,10 @@ protected override TypeSignatureModifiers BuildDeclarationModifiers() // They are rebuilt at write time while non-buildable attributes, including visitor updates, are preserved. protected override bool ShouldAnalyzeAttributesInReferenceMap => false; - protected override IReadOnlyList BuildAttributesForWrite() - { - var visitorAttributes = base.BuildAttributesForWrite().Where(static attribute => !IsBuildableAttribute(attribute)); - return [.. BuildAttributes(), .. visitorAttributes]; - } - protected override IReadOnlyList BuildAttributes() { var attributes = new Dictionary(); - var customizedBuildableTypes = GetCustomizedBuildableTypes(); + var customizedBuildableTypes = CustomizedBuildableTypes; // Add ModelReaderWriterBuildableAttribute for all IPersistableModel types (HashSet buildableTypes, HashSet buildableProviders) = CollectBuildableTypes(); @@ -88,12 +86,56 @@ protected override IReadOnlyList BuildAttributes() provider.Type.FullyQualifiedName); } - AddLastContractBuildableAttributes(attributes, customizedBuildableTypes); - // Sort by the simple type name (last part after the last dot) instead of the fully qualified name return attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value).ToList(); } + protected override IReadOnlyList BuildAttributesForBackCompatibility(IReadOnlyList originalAttributes) + { + if (LastContractView?.Attributes is not { Count: > 0 }) + { + return originalAttributes; + } + + // Re-key the generated buildable attributes so last-contract entries can be deduplicated against + // them and the combined set can be re-sorted; any non-buildable attributes are preserved as-is. + var attributes = new Dictionary(); + var others = new List(); + foreach (var attribute in originalAttributes) + { + var identity = GetBuildableAttributeIdentity(attribute); + if (identity != null) + { + attributes[identity] = attribute; + } + else + { + others.Add(attribute); + } + } + + AddLastContractBuildableAttributes(attributes, CustomizedBuildableTypes); + + return [.. attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value), .. others]; + } + + private static string? GetBuildableAttributeIdentity(MethodBodyStatement attribute) + { + var attributeStatement = attribute switch + { + AttributeStatement direct => direct, + SuppressionStatement suppression => suppression.AsStatement(), + _ => null + }; + + var targetType = attributeStatement is null + ? null + : GetBuildableAttributeTargetType(attributeStatement); + return targetType is null + ? null + : GetTypeIdentity(targetType); + } + private void AddLastContractBuildableAttributes( Dictionary attributes, HashSet customizedBuildableTypes) @@ -178,19 +220,7 @@ private void AddLastContractBuildableAttributes( return null; } - private static bool IsBuildableAttribute(MethodBodyStatement statement) - { - var attribute = statement switch - { - AttributeStatement directAttribute => directAttribute, - SuppressionStatement suppression => suppression.AsStatement(), - _ => null - }; - - return attribute?.Type.Equals(s_buildableAttributeType) == true; - } - - private HashSet GetCustomizedBuildableTypes() + private HashSet BuildCustomizedBuildableTypes() { var customizedTypes = new HashSet(StringComparer.Ordinal); foreach (var attribute in CustomCodeView?.Attributes ?? []) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs index 4685d2e0489..1604cf2e06f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs @@ -2172,12 +2172,15 @@ await MockHelpers.LoadMockGeneratorAsync( // Buildable attributes restored from the last contract are symbol-based (IsFrameworkType == false), so // match by fully qualified name to cover both generated and restored entries. private static List GetBuildableAttributes(ModelReaderWriterContextDefinition contextDefinition) - => contextDefinition.Attributes + { + contextDefinition.ProcessTypeForBackCompatibility(); + return contextDefinition.Attributes .Where(a => string.Equals( a.Type.FullyQualifiedName, typeof(ModelReaderWriterBuildableAttribute).FullName, StringComparison.Ordinal)) .ToList(); + } [Test] public async Task CustomProjectionPropertiesDoNotAddBuildableTypes() 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 31bc0d3fd63..bfaa08d8042 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 @@ -343,6 +343,9 @@ private IReadOnlyList ApplyCustomizationFilter(IEnumerable? _attributes; + // Snapshot of the attributes as they were before the first Update call + private HashSet? _originalAttributes; + public IReadOnlyList Attributes { get @@ -360,21 +363,41 @@ public IReadOnlyList Attributes } } - internal IReadOnlyList GetAttributes() => _attributes ??= BuildAttributes(); - - internal IReadOnlyList GetAttributesForWrite() => BuildAttributesForWrite(); - /// - /// Builds the attributes emitted by the writer. Providers whose generated attributes depend on final - /// generation decisions can override this without replacing attributes updated by visitors. - /// - protected internal virtual IReadOnlyList BuildAttributesForWrite() => GetAttributes(); - - /// - /// Indicates whether this provider's attributes should contribute to reference-map analysis. + /// Indicates whether this provider's attributes are stable enough to be cached and analyzed by the + /// reference map. Providers whose generated attributes depend on final generation decisions return + /// false so their attributes are rebuilt at write time. /// protected internal virtual bool ShouldAnalyzeAttributesInReferenceMap => true; + internal IReadOnlyList GetAttributesForWrite() + { + if (ShouldAnalyzeAttributesInReferenceMap) + { + return _attributes ??= BuildAttributes(); + } + + return RebuildAttributes(); + } + + // Rebuilds the generated attributes (including any back-compatibility additions) from the finalized + // generation state and re-attaches the attributes a visitor contributed on top of the previously + // generated set. Providers whose attributes depend on final generation decisions may have cached a + // value during reference-map analysis, so the generated portion is always recomputed here. + private IReadOnlyList RebuildAttributes() + { + var visitorAdditions = _attributes is null || _originalAttributes is null + ? [] + : _attributes.Where(a => !_originalAttributes.Contains(a)).ToList(); + + var result = BuildAttributesForBackCompatibility([.. BuildAttributes(), .. visitorAdditions]); + + // Exclude the visitor additions from the snapshot so they stay identifiable when the generated set is rebuilt. + var visitorSet = new HashSet(visitorAdditions); + _originalAttributes = [.. result.Where(a => !visitorSet.Contains(a))]; + return _attributes = result; + } + /// /// Determines whether a provider remains in the generated output after reference-map analysis. /// @@ -636,6 +659,7 @@ public virtual void Reset() _enumValues = null; _enumUnderlyingType = null; _attributes = null; + _originalAttributes = null; _deprecated = null; _description = null; _type = null; @@ -722,6 +746,12 @@ public void Update( } if (attributes != null) { + // For providers whose generated attributes are rebuilt at write time, remember the attributes + // as they were before the first update so GetAttributesForWrite can preserve the additions. + if (!ShouldAnalyzeAttributesInReferenceMap) + { + _originalAttributes ??= [.. Attributes]; + } _attributes = [.. attributes]; } @@ -854,6 +884,14 @@ internal void ProcessTypeForBackCompatibility() Update(fields: newFields, methods: newMethods, constructors: newConstructors); } + + // Providers whose attributes depend on final generation decisions build their attributes at write + // time; materialize them here (applying attribute back-compatibility) so reads before the write + // reflect the result. The generated portion is refreshed again at write against the final state. + if (!ShouldAnalyzeAttributesInReferenceMap) + { + RebuildAttributes(); + } } // Runs newly-added back-compatibility members through every registered visitor while leaving @@ -904,6 +942,14 @@ private static IReadOnlyList VisitNewMembers( protected internal virtual IReadOnlyList? BuildEnumValuesForBackCompatibility(IReadOnlyList originalEnumValues) => null; + /// + /// Returns this type's attributes with backward compatibility applied against + /// . The default implementation applies no back-compatibility and + /// returns the attributes unchanged. Override to restore attributes that were present in the last contract. + /// + protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IReadOnlyList originalAttributes) + => originalAttributes; + /// /// Returns this type's methods with backward compatibility applied against /// . The default implementation restores the previous diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.Helpers.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.Helpers.cs index 14fac6b079b..2443088696f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.Helpers.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/ReferenceMap/ProviderReferenceMapAnalyzer.Helpers.cs @@ -311,17 +311,6 @@ private static void AddUnambiguousMatchingName(HashSet target, string na } } - private static void AddMatchingNamesWithSimpleNameSuffix(HashSet target, string suffix, HashSet nodes) - { - foreach (var node in nodes) - { - if (GetSimpleName(node).EndsWith(suffix, StringComparison.Ordinal)) - { - target.Add(node); - } - } - } - private static Dictionary BuildSimpleNameLookup(HashSet nodes) => BuildSimpleNameLookup(nodes, ignoreGenericArity: true); From 8b867980e0ca3be4427cf1eb10be3c21ff0bf6d5 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Wed, 29 Jul 2026 13:51:35 -0500 Subject: [PATCH 2/4] fix: only key buildable attributes by ModelReaderWriterBuildableAttribute type Verify the attribute type is ModelReaderWriterBuildableAttribute before deriving a buildable identity so visitor-added type-valued attributes are preserved instead of overwriting the generated buildable attribute. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0922f61d-c8c9-4b4b-bfe6-e53ce3af4653 --- .../ModelReaderWriterContextDefinition.cs | 12 +++-- ...ModelReaderWriterContextDefinitionTests.cs | 48 +++++++++++++++++++ .../SampleContext.cs | 16 +++++++ 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute/SampleContext.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs index 57b1044fff0..5940c4a6c0e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs @@ -128,9 +128,15 @@ protected override IReadOnlyList BuildAttributesForBackComp _ => null }; - var targetType = attributeStatement is null - ? null - : GetBuildableAttributeTargetType(attributeStatement); + if (attributeStatement is null || !string.Equals( + attributeStatement.Type.FullyQualifiedName, + s_buildableAttributeType.FullyQualifiedName, + StringComparison.Ordinal)) + { + return null; + } + + var targetType = GetBuildableAttributeTargetType(attributeStatement); return targetType is null ? null : GetTypeIdentity(targetType); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs index 1604cf2e06f..45b534953e3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.ClientModel.Providers; +using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; @@ -1911,6 +1912,53 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + [Test] + public async Task VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute() + { + // A visitor appends a non-buildable attribute that carries a typeof(...) argument for the same model + // that already has a generated ModelReaderWriterBuildableAttribute. Back-compat re-keying must key by + // the buildable attribute's target type only, so the visitor attribute cannot displace the generated + // buildable entry and is instead preserved alongside it. + var regularModel = InputFactory.Model("RegularModel", properties: + [ + InputFactory.Property("Property1", InputPrimitiveType.String) + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModels: () => [regularModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var contextDefinition = new ModelReaderWriterContextDefinition(); + + // Resolve the CSharpType used by the generated buildable attribute so the visitor attribute keys to + // the exact same type identity. + var buildableTargetType = contextDefinition.Attributes + .Where(a => string.Equals( + a.Type.FullyQualifiedName, + typeof(ModelReaderWriterBuildableAttribute).FullName, + StringComparison.Ordinal)) + .Select(a => a.Arguments.OfType().First().Type) + .Single(t => t.Name == "RegularModel"); + + // Simulate a visitor appending [TypeConverter(typeof(RegularModel))] after the generated attributes. + var visitorAttributeType = new CSharpType(typeof(System.ComponentModel.TypeConverterAttribute)); + var visitorAttribute = new AttributeStatement(visitorAttributeType, Snippet.TypeOf(buildableTargetType)); + contextDefinition.Update(attributes: [.. contextDefinition.Attributes, visitorAttribute]); + + var buildableAttributes = GetBuildableAttributes(contextDefinition); + + Assert.AreEqual(1, buildableAttributes + .Count(a => a.Arguments.First().ToDisplayString().Contains("RegularModel")), + "The generated buildable attribute for RegularModel must survive the visitor's type-valued attribute"); + + Assert.IsTrue(contextDefinition.Attributes.Any(a => string.Equals( + a.Type.FullyQualifiedName, + visitorAttributeType.FullyQualifiedName, + StringComparison.Ordinal) + && a.Arguments.First().ToDisplayString().Contains("RegularModel")), + "The visitor's non-buildable type-valued attribute must be preserved"); + } + [Test] public async Task BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContractBuildableAttributes() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute/SampleContext.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute/SampleContext.cs new file mode 100644 index 00000000000..62e69f01085 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute/SampleContext.cs @@ -0,0 +1,16 @@ +using System.ClientModel.Primitives; + +namespace Sample +{ + [ModelReaderWriterBuildable(typeof(Sample.Models.RegularModel))] + public partial class SampleContext + { + } +} + +namespace Sample.Models +{ + public partial class RegularModel + { + } +} From d72f3806cb953a6337d748ecdba3b8cf727f8d8c Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Wed, 29 Jul 2026 14:11:01 -0500 Subject: [PATCH 3/4] test: verify visitor type-valued attribute preserves buildable attribute via TestData Rework the coverage into an end-to-end back-compat scenario: the last contract declares a buildable attribute for the model, a visitor appends [TypeConverter(typeof(Model))], and the generated context is compared against a TestData expected file that shows both attributes preserved. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0922f61d-c8c9-4b4b-bfe6-e53ce3af4653 --- ...ModelReaderWriterContextDefinitionTests.cs | 45 +++++++++---------- ...ibuteDoesNotOverwriteBuildableAttribute.cs | 16 +++++++ 2 files changed, 36 insertions(+), 25 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs index 45b534953e3..b7db17f446e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs @@ -1915,10 +1915,11 @@ await MockHelpers.LoadMockGeneratorAsync( [Test] public async Task VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute() { - // A visitor appends a non-buildable attribute that carries a typeof(...) argument for the same model - // that already has a generated ModelReaderWriterBuildableAttribute. Back-compat re-keying must key by - // the buildable attribute's target type only, so the visitor attribute cannot displace the generated - // buildable entry and is instead preserved alongside it. + // The last contract declares a buildable attribute for RegularModel (so back-compat re-keying runs), + // RegularModel is emitted by the current generation, and a visitor appends a non-buildable attribute + // that carries a typeof(RegularModel) argument. Back-compat re-keying must key by the buildable + // attribute's target type only, so the visitor attribute cannot displace the generated buildable entry + // and is instead preserved alongside it in the final generated context. var regularModel = InputFactory.Model("RegularModel", properties: [ InputFactory.Property("Property1", InputPrimitiveType.String) @@ -1930,33 +1931,27 @@ await MockHelpers.LoadMockGeneratorAsync( var contextDefinition = new ModelReaderWriterContextDefinition(); - // Resolve the CSharpType used by the generated buildable attribute so the visitor attribute keys to - // the exact same type identity. - var buildableTargetType = contextDefinition.Attributes + // Reuse the exact CSharpType from the generated buildable attribute so the visitor attribute shares the + // same type identity as the buildable attribute it must not overwrite. + var modelType = contextDefinition.Attributes .Where(a => string.Equals( a.Type.FullyQualifiedName, typeof(ModelReaderWriterBuildableAttribute).FullName, StringComparison.Ordinal)) - .Select(a => a.Arguments.OfType().First().Type) - .Single(t => t.Name == "RegularModel"); - - // Simulate a visitor appending [TypeConverter(typeof(RegularModel))] after the generated attributes. - var visitorAttributeType = new CSharpType(typeof(System.ComponentModel.TypeConverterAttribute)); - var visitorAttribute = new AttributeStatement(visitorAttributeType, Snippet.TypeOf(buildableTargetType)); + .SelectMany(a => a.Arguments.OfType()) + .Select(argument => argument.Type) + .First(t => t.Name == "RegularModel"); + + // Simulate a library visitor appending [TypeConverter(typeof(RegularModel))] after the generated + // attributes, mirroring how visitor additions are attached on top of the generated set. + var visitorAttribute = new AttributeStatement( + new CSharpType(typeof(System.ComponentModel.TypeConverterAttribute)), + Snippet.TypeOf(modelType)); contextDefinition.Update(attributes: [.. contextDefinition.Attributes, visitorAttribute]); - var buildableAttributes = GetBuildableAttributes(contextDefinition); - - Assert.AreEqual(1, buildableAttributes - .Count(a => a.Arguments.First().ToDisplayString().Contains("RegularModel")), - "The generated buildable attribute for RegularModel must survive the visitor's type-valued attribute"); - - Assert.IsTrue(contextDefinition.Attributes.Any(a => string.Equals( - a.Type.FullyQualifiedName, - visitorAttributeType.FullyQualifiedName, - StringComparison.Ordinal) - && a.Arguments.First().ToDisplayString().Contains("RegularModel")), - "The visitor's non-buildable type-valued attribute must be preserved"); + var writer = new TypeProviderWriter(contextDefinition); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute.cs new file mode 100644 index 00000000000..2e5438c7c14 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute.cs @@ -0,0 +1,16 @@ +// + +#nullable disable + +using System.ClientModel.Primitives; +using System.ComponentModel; +using Sample.Models; + +namespace Sample +{ + [global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.Models.RegularModel))] + [global::System.ComponentModel.TypeConverterAttribute(typeof(global::Sample.Models.RegularModel))] + public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext + { + } +} From 23c2f3fe274e35d3add2dc0a6b80eddebb098730 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Wed, 29 Jul 2026 14:32:27 -0500 Subject: [PATCH 4/4] fix: recompute customized buildable types instead of caching them Remove the _customizedBuildableTypes cache so BuildAttributes recomputes the customized buildable types from the current customization view on every rebuild. Reset() recreates CustomCodeView, and the stale cache could otherwise keep suppressing types from a previously resolved view. Add a reset regression test that validates the generated context via TestData before and after Reset. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0922f61d-c8c9-4b4b-bfe6-e53ce3af4653 --- .../ModelReaderWriterContextDefinition.cs | 8 ++---- ...ModelReaderWriterContextDefinitionTests.cs | 28 +++++++++++++++++++ ...lculatesCustomizedBuildableTypes(Empty).cs | 14 ++++++++++ .../SampleContext.cs | 6 ++++ ...tesCustomizedBuildableTypes(Suppressed).cs | 12 ++++++++ .../SampleContext.cs | 9 ++++++ 6 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty).cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty)/SampleContext.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed).cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed)/SampleContext.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs index 5940c4a6c0e..74f5feb0693 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs @@ -26,10 +26,6 @@ public class ModelReaderWriterContextDefinition : TypeProvider internal static readonly string s_name = $"{RemovePeriods(ScmCodeModelGenerator.Instance.TypeFactory.PrimaryNamespace)}Context"; - // Customized buildable types come from custom code, which is stable, so compute them once. - private HashSet? _customizedBuildableTypes; - private HashSet CustomizedBuildableTypes => _customizedBuildableTypes ??= BuildCustomizedBuildableTypes(); - protected override string BuildName() => s_name; protected override string BuildRelativeFilePath() => Path.Combine("src", "Generated", "Models", $"{Name}.cs"); @@ -46,7 +42,7 @@ protected override TypeSignatureModifiers BuildDeclarationModifiers() protected override IReadOnlyList BuildAttributes() { var attributes = new Dictionary(); - var customizedBuildableTypes = CustomizedBuildableTypes; + var customizedBuildableTypes = BuildCustomizedBuildableTypes(); // Add ModelReaderWriterBuildableAttribute for all IPersistableModel types (HashSet buildableTypes, HashSet buildableProviders) = CollectBuildableTypes(); @@ -114,7 +110,7 @@ protected override IReadOnlyList BuildAttributesForBackComp } } - AddLastContractBuildableAttributes(attributes, CustomizedBuildableTypes); + AddLastContractBuildableAttributes(attributes, BuildCustomizedBuildableTypes()); return [.. attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value), .. others]; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs index b7db17f446e..3f6fc3ba736 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs @@ -1872,6 +1872,34 @@ public async Task CustomizedBuildableAttributesAreNotRegenerated() "Buildable attributes supplied by a customized context should not be regenerated"); } + [Test] + public async Task ResetRecalculatesCustomizedBuildableTypes() + { + var clientProvider = new TestClientProviderWithResponseErrorReturnType(); + var outputLibrary = new TestOutputLibrary([clientProvider]); + var mockGenerator = MockHelpers.LoadMockGenerator(createOutputLibrary: () => outputLibrary); + + // The initial customization declares a buildable attribute for Azure.ResponseError, so the generated + // buildable attribute for that type is suppressed. + var suppressedCompilation = await Helpers.GetCompilationFromDirectoryAsync("Suppressed"); + mockGenerator.SetupProperty(p => p.SourceInputModel, new SourceInputModel(suppressedCompilation, null)); + + var contextDefinition = new ModelReaderWriterContextDefinition(); + var suppressedContent = new TypeProviderWriter(contextDefinition).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile("Suppressed"), suppressedContent); + + // Change the customization view so it no longer suppresses the buildable attribute, then reset the + // provider. Reset must clear the customized buildable types so they are recalculated from the current + // customization view instead of reusing the stale suppression. + var emptyCompilation = await Helpers.GetCompilationFromDirectoryAsync("Empty"); + mockGenerator.Object.SourceInputModel = new SourceInputModel(emptyCompilation, null); + + contextDefinition.Reset(); + + var recalculatedContent = new TypeProviderWriter(contextDefinition).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile("Empty"), recalculatedContent); + } + [Test] public async Task LastContractBuildableAttributesAreRestoredWhenMissing() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty).cs new file mode 100644 index 00000000000..9787206c70d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty).cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +using System.ClientModel.Primitives; +using Azure; + +namespace Sample +{ + [global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Azure.ResponseError))] + public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty)/SampleContext.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty)/SampleContext.cs new file mode 100644 index 00000000000..052c5d576a4 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty)/SampleContext.cs @@ -0,0 +1,6 @@ +namespace Sample +{ + public partial class SampleContext + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed).cs new file mode 100644 index 00000000000..a564d121130 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed).cs @@ -0,0 +1,12 @@ +// + +#nullable disable + +using System.ClientModel.Primitives; + +namespace Sample +{ + public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed)/SampleContext.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed)/SampleContext.cs new file mode 100644 index 00000000000..dd85bacb583 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed)/SampleContext.cs @@ -0,0 +1,9 @@ +using System.ClientModel.Primitives; + +namespace Sample +{ + [ModelReaderWriterBuildable(typeof(Azure.ResponseError))] + public partial class SampleContext + { + } +}