[Wei Hu (live1206)]

Summary

• preserve exact public GA property names when their date-time stems match the current MTG-normalized property
• keep canonical MTG naming for new properties and when a GA name is incompatible or already claimed
• keep customization suppression and canonical wire information associated with the correct spec property across preserved names, aliases, exact names, and refiltering
• cover historical, canonical, projected-provider, collision, customization, and sibling-ownership scenarios
• normalize ExpireTime and ExpireOn to ExpiresOn for new APIs, making ExpireOn and ExpirationTime share the semantic Expires stem

Fixes #11772

Validation

• dotnet test packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Microsoft.TypeSpec.Generator.Tests.csproj --no-restore --filter 'FullyQualifiedName~PropertyProviderTests|FullyQualifiedName~ModelCustomizationTests|FullyQualifiedName~CSharpNameExtensionsTests' -- NUnit.NumberOfTestWorkers=1

[pkg-pr-new]

Open in StackBlitz

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

commit: 5b0a731

[github-actions]

No changes needing a change description found.

[Copilot]

Pull request overview

This PR updates the C# HTTP client generator’s backward-compatibility naming logic so that date-time property names previously shipped in a GA baseline are preserved (e.g., StartOn, EndOn, ExpireOn), while new properties still follow the canonical date-time normalization rules.

Changes:

• Extend date-time suffix normalization to treat Expire* consistently (e.g., ExpireOn → ExpiresOn) in CSharpNameExtensions.
• Update PropertyProvider to preserve the exact property name from the last contract when it normalizes to the current canonical name.
• Add unit tests + last-contract test data covering previously shipped historical names and already-canonical baselines.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PropertyProvider.cs	Preserve historical GA date-time property names by matching last-contract members against canonical normalized names.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/CSharpNameExtensions.cs	Add Expire -> Expires mapping for date-time verb normalization.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/PropertyProviderTests.cs	Add regression tests for preserving historical GA property names and for already-canonical contracts.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/PropertyProviderTests/TestPropertyNamePreservesPreviouslyGeneratedDateTimeSuffixFromLastContract/TestModel.cs	Last-contract baseline containing StartOn for preservation testing.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/PropertyProviderTests/TestPropertyNamePreservesHistoricalDateTimeNamesFromLastContract/TestModels.cs	Last-contract baseline containing historical names (EndOn, ExpireOn, etc.) and canonical StartsOn.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/CSharpNameExtensionsTests.cs	Add normalization tests for ExpireOn/expireOn to produce ExpiresOn/expiresOn.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

[Wei Hu (live1206)]

Full Azure-branded DPG regeneration completed successfully with the latest changes: all 47 libraries regenerated and there are no resulting SDK file diffs.

Draft regeneration PR: Azure/azure-sdk-for-net#62499

- by copilot

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

[Wei Hu (live1206)]

Thanks for the re-review and mutation testing. Addressed the two suggested follow-ups in 788eaab74:

• activeProperties now uses ReferenceEqualityComparer.Instance, making provider-instance identity explicit even if a derived PropertyProvider later overrides equality.
• added the directly named TestDateTimeNameAlreadyEndingOnDoesNotUseHistoricalStemMatch and an explanatory comment for the spec-side On guard.

I also renamed _nounToVerbMap to _nounToVerbRules. The exact and compound passes remain separate so exact matches always win before any potentially overlapping suffix rule.

For the declaration-order report from Copilot, I reversed the GA fixture to declare StartsOn before StartOn. TestPropertyNameDoesNotReuseCanonicalNameClaimedBySibling still passes because the current non-deferred lookup evaluates the ownership guard inside the historical predicate and continues to the available StartOn; the reported candidate-caching behavior belonged to the reverted reconciliation prototype.

The focused property/customization/name suite passes (254 tests).

- by copilot

[JoshLove-msft]

Addendum — my re-review above landed ten seconds after 83d94f97f was pushed, so it was written against a3572f466 and the two follow-ups in it are still open. Here is the review of b7b952eb8 + the revert.

b7b952eb8 — first exact spec mapping

Full suite at 83d94f97f: Failed: 0, Passed: 2105, no build warnings.

Mutation-tested both new guards, reverting one at a time and re-running the whole suite:

Guard removed	Suite result
the else (rebuild the map even when knownSpecProperties was supplied)	fails DuplicateExactIdentifierCustomizationSuppressesOnlyFirstSpecMatch
exactNames.Add(name) in AddExactName	fails DuplicateExactIdentifierCustomizationSuppressesOnlyFirstSpecMatch

Both covered, and the test is a good one — asserting the surviving property and that both wire names remain on the canonical view catches the over-suppression directly rather than by proxy.

I traced the else specifically, since "stop populating this map on one path" is the kind of change that tends to lose something quietly. The only caller supplying knownSpecProperties is CanonicalTypeProvider.BuildProperties, which passes the generated provider's complete map; the entries the removed rebuild would have added come from generatedProperties, whose spec names are a subset of that map because it was built pre-filter. So nothing is dropped and the comment is accurate. It also composes correctly with the activeProperties guard from a3572f466: the map's values are the same PropertyProvider instances as in propertiesToFilter, so filtered-out properties remain in the map but fail the reference check, which is the intended outcome.

Making AddExactName's per-pass first-wins mirror _exactSpecPropertyNames in CanonicalTypeProvider is nice — the two places that resolve exact-vs-alias precedence now read the same way.

The revert

Reverting 3b5aaa62a was the right call, and #11783 is the right home for it. A change that reaches into property-build, constructor, serialization, and visitor lifecycle does not belong in a back-compat naming fix, and validating it against the real management-plane flattening path is exactly the prerequisite.

I verified the revert is clean rather than taking it on trust: PropertyProvider.cs and PropertyProviderTests.cs are byte-identical between a3572f466 and 83d94f97f, and neither prototype test-data directory survives in the tree. Since b7b952eb8 landed between the prototype and its revert and touched TypeProvider.cs, that was worth confirming.

Still open from the re-review above

Neither is a blocker; restating so they don't get lost to the comment-ordering collision:

1. new HashSet<PropertyProvider>(propertiesToFilter) depends on reference equality, which holds today but is not enforced — PropertyProvider is not sealed, so a derived provider overriding Equals/GetHashCode would silently turn the stale-alias filter into a value match with no test failing. ReferenceEqualityComparer.Instance would pin it.
2. The spec-side !identifierName.EndsWith("On", StringComparison.Ordinal) guard is covered only by ExactSpecNameTakesPrecedenceOverCanonicalAliasForCustomization, a test named for a different concern, and the inline comment explains only the contract-side On requirement.

Plus the two cosmetic notes (_nounToVerbMap is now an array, and _ = Properties; re-entrancy).

State of the PR from my side: the core change is sound, every behavioral guard I could identify is individually covered by a test that fails without it, and the full 2105-test suite is green at 83d94f97f.

--generated by Copilot

[Copilot]

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.

[Wei Hu (live1206)]

Addressed the remaining cosmetic notes in 95cb3cc6b:

• removed the defensive _ = Properties from GeneratedPropertiesBySpecName, avoiding the newly exposed re-entrancy path;
• made CanonicalTypeProvider.BuildProperties explicitly build Properties first, capture the populated map immediately afterward, and reuse that snapshot;
• retained the two-pass noun/verb lookup but documented why: all exact matches must win before any potentially overlapping compound suffix rule.

The earlier 788eaab74 commit also pinned active-property membership to ReferenceEqualityComparer.Instance, added direct coverage for the spec-side On guard, and renamed the array to _nounToVerbRules.

The focused suite remains green at 254 tests.

- by copilot

[Copilot]

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PropertyProvider.cs:280

• This ownership check only enumerates custom members declared on the current type, while FilterCustomizedProperties also treats custom members inherited from BaseTypeProvider as claims (TypeProvider.cs:98-149). Consequently, if a base customization explicitly renames an unrelated member to StartOn, a derived startTime property can preserve the GA StartOn here and then be removed by filtering instead of falling back to StartsOn, losing the derived spec property. Apply the same base-type traversal used by customization filtering when collecting explicitly renamed claims.

        private static IEnumerable<(string Name, string OriginalName)> GetExplicitlyRenamedCustomNames(TypeProvider enclosingType)
        {
            foreach (var customProperty in enclosingType.CustomCodeView?.Properties ?? [])
            {
                if (customProperty.OriginalName is { } propertyOriginalName)

[Copilot]

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated no new comments.

[Wei Hu (live1206)]

regen: Azure/azure-sdk-for-net#62530

[JoshLove-msft]

Re-reviewed at 5b0a7314. As before I verified everything in a local worktree rather than reading the diff, and mutation-tested each new guard by removing it one at a time and re-running the suite.

Verification

• Full suite at 5b0a7314: 2106/2106, no warnings.
• Repeated 5x to check stability: 5/5 green. Merge base 806e1ab8 also 5/5 green at 2079/2079.
• The second revert is exactly clean: git diff 95cb3cc6b9 5b0a7314 is empty, so no prototype code or orphaned test data was left behind.

Both prior follow-ups addressed

1. ReferenceEqualityComparer.Instance on the activeProperties set ✓
2. A directly-named test for the spec-side On guard (TestDateTimeNameAlreadyEndingOnDoesNotUseHistoricalStemMatch) ✓

I also noticed Reset() now clears _generatedPropertiesBySpecName. That addresses the stale-map problem more fundamentally than the filtering-time guard alone.

Mutation results

Guard removed	Result
spec-side !identifierName.EndsWith("On")	❌ fails ExactSpecNameTakesPrecedenceOverCanonicalAliasForCustomization and TestDateTimeNameAlreadyEndingOnDoesNotUseHistoricalStemMatch
!IsClaimedBySiblingProperty(...) in the stem lookup	❌ fails 4 tests, including both sibling tests
else on knownSpecProperties	❌ fails DuplicateExactIdentifierCustomizationSuppressesOnlyFirstSpecMatch
exactNames.Add(name) in AddExactName	❌ fails the same test
ReferenceEqualityComparer.Instance	✅ passes — see below

The fixture reversal is a genuine strengthening

Reversing TestPropertyNameDoesNotReuseCanonicalNameClaimedBySibling/TestModel.cs to declare StartsOn before StartOn is not cosmetic. I ran the differential with IsClaimedBySiblingProperty removed from the stem lookup:

• original order (StartOn first) → test still passes
• reversed order (StartsOn first) → test fails

Because the lookup uses FirstOrDefault in declaration order, the old order let the correct answer be found before the guard was ever consulted. Reversed, StartsOn has to be actively rejected first. The guard is load-bearing in that test now, and it wasn't before.

ReferenceEqualityComparer — uncovered, and currently a no-op

Removing it fails nothing. PropertyProvider overrides neither Equals nor GetHashCode, so EqualityComparer<PropertyProvider>.Default is already reference equality. I still think it's worth keeping as executable documentation of the intent, but flagging honestly that it constrains nothing today: if someone later gives PropertyProvider value equality, this line is what preserves correctness, and no test would catch its removal.

The _ = Properties; round-trip — my error

I owe you a correction. My re-entrancy note on _ = Properties; was explicitly hypothetical and labelled "not a regression" — no caller reads the map mid-build. It nonetheless cost a real safeguard, and the code is now back to the unenforced ordering dependency I originally raised as item 4.

The call site in CanonicalTypeProvider.BuildProperties does the right thing explicitly and the XML doc states the contract, so this is defensible as-is. If you want enforcement at zero re-entrancy risk:

internal IReadOnlyDictionary<string, PropertyProvider> GeneratedPropertiesBySpecName
{
    get
    {
        Debug.Assert(_properties is not null, "Properties must be built before reading this map.");
        return _generatedPropertiesBySpecName;
    }
}

Entirely your call — a documented contract with a single compliant caller is a reasonable place to stop.

The IPStartOn acronym scenario is handled

On the suppressed bot comment about acronym normalization, I traced it by hand: spec ipStartTime gives stem IPStarts; candidate IPStartOn stems to IPStarts and matches; but sibling ipStartOn emits IPStartOn, so comparing the emitted form rejects it. No duplicate member. TestPropertyNameDoesNotReuseHistoricalNameClaimedByNormalizedSibling covers it.

Nit

The explanation of why the spec-side On guard exists lives in the test file. A one-line comment beside the guard in PropertyProvider.cs would help, since that's where someone would be tempted to remove it.

Not a finding

During mutation runs I saw two one-off failures (CanRemoveMethodWithGenericTypeConstraintParameter, CanChangePropertyAccessibility). Neither reproduced — the first passed on two repeat mutated runs and three isolated runs, and the second occurred under a mutation that is provably a no-op. Both base and head were 5/5 green. I'm recording it only so it isn't mistaken for signal; I attribute it to local machine contention rather than this PR.

LGTM.

--generated by Copilot