Skip to content

Fix LastContractPropertiesMap overriding collection property type when element type changes#10319

Merged
live1206 merged 4 commits intomainfrom
copilot/fix-lastcontractpropertiesmap-collection-type
Apr 15, 2026
Merged

Fix LastContractPropertiesMap overriding collection property type when element type changes#10319
live1206 merged 4 commits intomainfrom
copilot/fix-lastcontractpropertiesmap-collection-type

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 9, 2026

The backcompat logic in ModelProvider.BuildProperties() unconditionally overrides a collection property's type to match the last contract when types differ. This was intended to preserve IReadOnlyList<T>IList<T> wrapper changes, but it also overrides the type when the element type itself has legitimately changed (e.g., via @typeChangedFrom), discarding the intentional type change.

Fix

  • ModelProvider.cs: Added an AreNamesEqual-based argument comparison guard so the override only fires when the element/value types match between old and new — i.e., only the collection wrapper changed. We use AreNamesEqual rather than SequenceEqual/Equals because the argument types may come from different sources (TypeProvider vs compiled assembly) but represent the same logical type. We compare all Arguments (not just ElementType) to cover both list element types and dictionary key/value types.
if (LastContractPropertiesMap.TryGetValue(outputProperty.Name,
        out CSharpType? lastContractPropertyType) &&
    !outputProperty.Type.Equals(lastContractPropertyType) &&
    outputProperty.Type.Arguments.Count == lastContractPropertyType.Arguments.Count &&
    outputProperty.Type.Arguments.Zip(lastContractPropertyType.Arguments).All(
        pair => pair.First.AreNamesEqual(pair.Second)))
  • ModelProviderTests.cs: Added BackCompat_CollectionPropertyTypeNotOverriddenWhenElementTypeChanges — last contract has IList<IDictionary<string, BinaryData>>, new model produces IList<NewElementModel>, asserts the new element type is preserved.
  • Test data: MockInputModel.cs simulating last contract with the old element types.

…perty type when element type changes

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/6f54fa17-1102-4fb6-8ce6-812c730e37c3

Co-authored-by: live1206 <5196139+live1206@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Apr 9, 2026
Copilot AI changed the title [WIP] Fix lastContractPropertiesMap collection property type override Fix LastContractPropertiesMap overriding collection property type when element type changes Apr 9, 2026
Copilot AI requested a review from live1206 April 9, 2026 02:12
@live1206
Copy link
Copy Markdown
Contributor

live1206 commented Apr 9, 2026

@copilot could you help to check the CI failures?

… vs compiled assembly types

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/fa5a9394-f6b4-4432-aa3f-c3ffd8551f7c

Co-authored-by: live1206 <5196139+live1206@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 9, 2026

@copilot could you help to check the CI failures?

The CI workflows show action_required (pending approval) with no jobs executed yet. However, I found an actual test failure locally: BackCompat_ReadOnlyCollectionEnumPropertiesAreRetained was failing because Arguments.SequenceEqual uses CSharpType.Equals which does full structural comparison — the ElementEnum from the current TypeProvider and from the compiled last contract assembly have different internal _type fields, so they don't match structurally even though they represent the same logical type.

Fixed in e5061a8 by replacing SequenceEqual with AreNamesEqual-based argument comparison, which correctly matches types across different sources by their fully-qualified names. All 29 BackCompat tests pass now.

@live1206 live1206 marked this pull request as ready for review April 14, 2026 05:12
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 14, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10319

commit: e5061a8

@live1206 live1206 added this pull request to the merge queue Apr 15, 2026
Merged via the queue into main with commit cff89bb Apr 15, 2026
31 checks passed
@live1206 live1206 deleted the copilot/fix-lastcontractpropertiesmap-collection-type branch April 15, 2026 01:36
github-merge-queue bot pushed a commit that referenced this pull request Apr 17, 2026
…type when element type changes" (#10411)

Reverts #10319

This change is leading to breaks in the Search library. The original
behavior probably makes more sense. GAed property types cannot be
changed without introducing new property names via custom code.
Copilot AI added a commit that referenced this pull request Apr 18, 2026
…n back-compat)

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/1cb571b4-94fe-450a-bda0-73a4c98e8f9f

Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C# Generator: LastContractPropertiesMap overrides collection property type even when element type changes

4 participants