Skip to content

Remove obsolete backcompat handling - #12573

Open
JoshLove01 wants to merge 8 commits into
mainfrom
joshlove01-remove-obsolete-backcompat
Open

JoshLove01 wants to merge 8 commits into
mainfrom
joshlove01-remove-obsolete-backcompat

Conversation

@JoshLove01

@JoshLove01 JoshLove01 commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • remove back-compat-based filtering of generated Obsolete attributes from types, properties, existing methods, and restored methods
  • apply the current API deprecation state regardless of whether the previous contract was obsolete
  • preserve custom and last-visitor Obsolete precedence while preventing duplicate attributes

Testing

  • TypeProviderTests: 191 passed
  • Microsoft.TypeSpec.Generator.ClientModel.Tests: 2,780 passed

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:49
@microsoft-github-policy-service microsoft-github-policy-service Bot added emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp labels Nov 12, 2026
@pkg-pr-new

pkg-pr-new Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

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

commit: bd50690

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Obsolete-attribute reconciliation can leave duplicate attributes that break SDK compilation, and existing ClientModel tests still assert the removed behavior.

2 open issues
What changed in this PR

This PR updates the C# client generator so backward-compatible methods retain current deprecation metadata rather than inheriting the previous contract’s obsolete status.

Changes:

  • Apply current Obsolete attributes to restored overloads and reordered methods.
  • Prefer visitor-supplied deprecation attributes over generated attributes.
  • Update provider tests to expect current deprecation messages.
File Description
packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator/​test/​Providers/​TypeProviderTests.cs Updates deprecation expectations for restored and reordered methods.
packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator/​src/​Utilities/​BackCompatHelper.cs Copies current deprecation metadata to restored overloads.
packages/​http-client-csharp/​generator/​Microsoft.TypeSpec.Generator/​src/​Providers/​TypeProvider.cs Replaces prior-contract deprecation filtering with visitor-attribute precedence.

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Visitor Obsolete reconciliation is skipped for clients without a prior contract, leaving duplicate attributes that cause compiler error CS0579.

2 open issues

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:58
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Repeated visitor-supplied Obsolete attributes remain duplicated on restored overloads, causing CS0579 compilation failures, and the regression test omits the main visitor pass that exposes the issue.

1 open issue
2 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 13, 2026 00:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Restored overloads can retain duplicate visitor-added attributes under the actual generation order, causing CS0579 compilation errors that the current regression does not cover.

1 open issue

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 13, 2026 00:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The restored overload can expose a different deprecation message or DiagnosticId because it selects the first visitor-added Obsolete attribute while the current method keeps the last, and the regression test cannot detect this mismatch.

1 open issue
1 resolved since last review

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 13, 2026 00:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The regression test’s filter excludes restored overloads from visitor attribute updates, leaving duplicate-attribute reconciliation on those overloads untested.

1 open issue
1 resolved since last review

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 13, 2026 01:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the C# generator changes aligning deprecation metadata with the current API contract.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 13, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the deprecation-handling changes.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

No deployments
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.

2 participants