From bfe1124ec4411607d7f804d54aad66ed235bbb8f Mon Sep 17 00:00:00 2001 From: jolov Date: Wed, 16 Sep 2026 10:37:47 -0700 Subject: [PATCH 1/2] fix(http-client-csharp): use current docs for compatibility overloads Build all shared operation compatibility documentation from the current method, mapping parameter docs and exception references to the preserved signature without mutating primary docs. Cover structured metadata, missing docs, nullable shims, client post-processing, and released DLL/XML baseline round trips. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ClientProviders/ClientProviderTests.cs | 64 ++++ .../Statements/XmlDocExceptionStatement.cs | 3 + .../src/Statements/XmlDocParamStatement.cs | 9 +- .../src/Utilities/BackCompatHelper.cs | 74 ++-- .../test/Utilities/BackCompatHelperTests.cs | 349 +++++++++++++++++- .../OperationClient.cs | 31 ++ .../generator/docs/backward-compatibility.md | 4 + 7 files changed, 510 insertions(+), 24 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/BackCompatHelperTests/CompatibilityDocumentationIsStableAcrossPublishedBaselines/OperationClient.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs index fef94f5ff15..26a8e45fd8e 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs @@ -10,6 +10,7 @@ using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using System.Threading.Tasks; +using System.Xml.Linq; using Microsoft.TypeSpec.Generator.ClientModel.Providers; using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Input; @@ -3315,6 +3316,69 @@ public async Task BackCompatibility_NewOptionalNonBodyParameterAdded() Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + [Test] + public async Task BackCompatibility_CurrentDocumentationSurvivesClientProcessing() + { + var operation = InputFactory.Operation( + "GetData", + parameters: + [ + InputFactory.QueryParameter("param1", InputPrimitiveType.Int32, isRequired: true), + InputFactory.BodyParameter("param2", InputPrimitiveType.String, isRequired: true), + InputFactory.HeaderParameter("param3", InputPrimitiveType.Boolean) + ], + responses: [InputFactory.OperationResponse([200], bodytype: InputPrimitiveType.String)]); + var method = InputFactory.BasicServiceMethod("GetData", operation, parameters: + [ + InputFactory.MethodParameter("param1", InputPrimitiveType.Int32, location: InputRequestLocation.Query, isRequired: true), + InputFactory.MethodParameter("param2", InputPrimitiveType.String, location: InputRequestLocation.Body, isRequired: true), + InputFactory.MethodParameter("param3", InputPrimitiveType.Boolean, location: InputRequestLocation.Header) + ]); + var client = InputFactory.Client(TestClientName, methods: [method]); + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: () => Helpers.GetCompilationFromDirectoryAsync(method: nameof(BackCompatibility_NewOptionalNonBodyParameterAdded)), + configuration: """{"disable-xml-docs": false}"""); + var provider = generator.Object.OutputLibrary.TypeProviders.OfType().Single(); + var originalMethods = provider.Methods.ToArray(); + foreach (var original in originalMethods.Where(m => m.Signature.Name is "GetData" or "GetDataAsync")) + { + original.XmlDocs.Update(summary: new XmlDocSummaryStatement([$"Current operation."], + new XmlDocStatement("list", [], new XmlDocStatement("item", [$"Current metadata."])))); + } + var originalDocs = originalMethods.ToDictionary(m => m, RenderDocs); + + provider.ProcessTypeForBackCompatibility(); + + var shims = provider.Methods.Except(originalMethods).ToArray(); + Assert.AreEqual(2, shims.Length); + foreach (var shim in shims) + { + string rendered = RenderDocs(shim); + var docs = XElement.Parse("" + string.Join("\n", rendered.Split('\n') + .Where(line => line.StartsWith("///")).Select(line => line[3..])) + ""); + Assert.IsNotNull(docs.Element("summary")?.Element("list")); + StringAssert.Contains("Current operation.", rendered); + Assert.AreEqual(new[] { "param1", "param2", "cancellationToken" }, + docs.Elements("param").Select(p => (string?)p.Attribute("name"))); + Assert.IsTrue(docs.Descendants("paramref").All(p => (string?)p.Attribute("name") != "param3")); + Assert.AreEqual(1, shim.Suppressions.Count); + } + foreach (var original in originalMethods) + { + Assert.AreEqual(originalDocs[original], RenderDocs(original)); + } + + static string RenderDocs(MethodProvider method) + { + using var writer = new CodeWriter(); + using (writer.WriteXmlDocs(method.XmlDocs)) + { + return writer.ToString(false); + } + } + } + // The current TypeSpec adds two new optional non-body parameters relative to the last contract. // Expected: a single back-compat overload matching the previous signature is added that // delegates to the new method, passing default for both new parameters. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocExceptionStatement.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocExceptionStatement.cs index 0904790bea0..4cec153d0cc 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocExceptionStatement.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocExceptionStatement.cs @@ -30,6 +30,9 @@ public XmlDocExceptionStatement(Type exceptionType, string reason, IReadOnlyList _reason = reason; } + internal XmlDocExceptionStatement WithParameters(IReadOnlyList parameters) + => new XmlDocExceptionStatement(ExceptionType, _reason, parameters); + private static string GetText(Type exceptionType) => exceptionType switch { { } when exceptionType == typeof(ArgumentNullException) => "is null.", diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocParamStatement.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocParamStatement.cs index 03caafb7631..f3c49a5eac6 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocParamStatement.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Statements/XmlDocParamStatement.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; +using System.Collections.Generic; using Microsoft.TypeSpec.Generator.Providers; using Microsoft.TypeSpec.Generator.Snippets; @@ -9,7 +11,12 @@ namespace Microsoft.TypeSpec.Generator.Statements public sealed class XmlDocParamStatement : XmlDocStatement { public XmlDocParamStatement(ParameterProvider parameter) - : base($"", $"", [parameter.Description]) + : this(parameter, [parameter.Description]) + { + } + + internal XmlDocParamStatement(ParameterProvider parameter, IEnumerable lines, params XmlDocStatement[] innerStatements) + : base($"", $"", lines, innerStatements) { Parameter = parameter; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs index f941827f34f..5123a920e7b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -644,7 +644,7 @@ private static MethodProvider BuildNewOptionalParameterOverload( signature, body, enclosingType, - previousMethod.XmlDocs); + BuildOverloadXmlDocs(currentMethod, signature, enclosingType)); } // Forwards a previous parameter to the current method. When the parameter's value-type nullability @@ -859,7 +859,7 @@ private static MethodProvider BuildChangedParameterNullabilityOverload( signature, body, enclosingType, - previousMethod.XmlDocs); + BuildOverloadXmlDocs(currentMethod, signature, enclosingType)); } // Given two signatures already known to have equal parameter count and types (including nullability), @@ -956,45 +956,75 @@ private static MethodProvider BuildOptionalityRestorationOverload( var body = BuildDelegatingBody(enclosingType, currentSignature, arguments); var signature = BuildHiddenOverloadSignature(previousSignature, shimParameters); - var xmlDocs = BuildXmlDocsWithoutParameter(previousMethod.XmlDocs, droppedParameter.Name.ToVariableName()); return new MethodProvider( signature, body, enclosingType, - xmlDocs); + BuildOverloadXmlDocs(currentMethod, signature, enclosingType)); } - // Rebuilds an XML doc provider without any reference to the dropped parameter: its <param> - // entry is removed and every <exception> that referenced it is rebuilt without that - // <paramref> (exceptions that referenced only the dropped parameter are removed entirely). - // This avoids stale-doc compile errors (CS1572/CS1734) on the reduced-arity overload. - private static XmlDocProvider BuildXmlDocsWithoutParameter(XmlDocProvider docs, string droppedVariableName) + // Released XML has already been flattened by NamedTypeSymbolProvider. Use the current + // documentation, binding parameter tags/references to the shim without mutating either method. + private static XmlDocProvider BuildOverloadXmlDocs( + MethodProvider currentMethod, + MethodSignature signature, + TypeProvider enclosingType) { - var filteredParameters = docs.Parameters - .Where(p => p.Parameter.Name.ToVariableName() != droppedVariableName) - .ToList(); + var docs = currentMethod.XmlDocs; + var parametersByName = signature.Parameters.ToDictionary(p => p.Name.ToVariableName()); + var parameterDocsByName = docs.Parameters.ToDictionary(p => p.Parameter.Name.ToVariableName()); + var parameters = new List(); + foreach (var parameter in signature.Parameters) + { + if (parameterDocsByName.TryGetValue(parameter.Name.ToVariableName(), out var parameterDoc)) + { + parameters.Add(new XmlDocParamStatement(parameter, parameterDoc.Lines, [.. parameterDoc.InnerStatements])); + } + } - var filteredExceptions = new List(docs.Exceptions.Count); + var exceptions = new List(); foreach (var exceptionDoc in docs.Exceptions) { var remaining = exceptionDoc.Parameters - .Where(p => p.Name.ToVariableName() != droppedVariableName) - .ToList(); + .Select(p => parametersByName.GetValueOrDefault(p.Name.ToVariableName())) + .OfType() + .ToArray(); - // Drop an exception that referenced only the removed parameter; keep an unrelated one as-is; - // otherwise rebuild it without the removed paramref. - if (exceptionDoc.Parameters.Count > 0 && remaining.Count == 0) + if (exceptionDoc.Parameters.Count > 0 && remaining.Length == 0) { continue; } - filteredExceptions.Add(remaining.Count == exceptionDoc.Parameters.Count - ? exceptionDoc - : new XmlDocExceptionStatement(exceptionDoc.ExceptionType, remaining)); + exceptions.Add(exceptionDoc.WithParameters(remaining)); + } + + // The shim can validate more than its target, notably before unwrapping T? to T. + foreach (var exceptionDoc in MethodProviderHelpers.BuildXmlDocs(signature, enclosingType).Exceptions) + { + AddValidationException(exceptionDoc); + } + var currentParametersByName = currentMethod.Signature.Parameters.ToDictionary(p => p.Name.ToVariableName()); + var unwrappedParameters = signature.Parameters.Where(p => + currentParametersByName.TryGetValue(p.Name.ToVariableName(), out var currentParameter) + && IsNullabilityRelaxedValueType(p.Type, currentParameter.Type)).ToArray(); + if (unwrappedParameters.Length > 0) + { + AddValidationException(new XmlDocExceptionStatement(typeof(ArgumentNullException), unwrappedParameters)); } - return new XmlDocProvider(docs.Summary, filteredParameters, filteredExceptions, docs.Returns, docs.Inherit); + return new XmlDocProvider(docs.Summary, parameters, exceptions, docs.Returns, docs.Inherit); + + void AddValidationException(XmlDocExceptionStatement exceptionDoc) + { + var documentedParameters = exceptions.Where(e => e.ExceptionType == exceptionDoc.ExceptionType) + .SelectMany(e => e.Parameters).ToHashSet(); + var missingParameters = exceptionDoc.Parameters.Where(p => !documentedParameters.Contains(p)).ToArray(); + if (missingParameters.Length > 0) + { + exceptions.Add(exceptionDoc.WithParameters(missingParameters)); + } + } } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/BackCompatHelperTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/BackCompatHelperTests.cs index f06a12083be..257c11d6108 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/BackCompatHelperTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/BackCompatHelperTests.cs @@ -1,9 +1,23 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using System.Xml.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Snippets; +using Microsoft.TypeSpec.Generator.Statements; using Microsoft.TypeSpec.Generator.Tests.Common; using Microsoft.TypeSpec.Generator.Utilities; using NUnit.Framework; @@ -15,7 +29,7 @@ public class BackCompatHelperTests [SetUp] public void Setup() { - MockHelpers.LoadMockGenerator(); + MockHelpers.LoadMockGenerator(includeXmlDocs: true); } // Detection: a nullable value-type parameter that became non-nullable qualifies. @@ -109,5 +123,338 @@ public void IsSingleNullableParameterOptionalToRequiredRejectsOtherChanges() Sig(Opt("a", new InputNullableType(InputPrimitiveType.Int32)), Opt("b", new InputNullableType(InputPrimitiveType.Int32))), Sig(Req("a", new InputNullableType(InputPrimitiveType.Int32)), Opt("b", new InputNullableType(InputPrimitiveType.Int32))))); } + + [Test] + public void AddedOptionalParameterUsesCurrentDocumentation( + [Values(false, true)] bool async, + [Values(0, 12, 24)] int indentation) + { + var current = CreateOperation(async); + var previous = CreatePreviousOperation(current, indentation); + string originalDocs = RenderDocs(current.XmlDocs); + var shim = CreateShim(current, previous); + var docs = ReadDocs(shim.XmlDocs); + + AssertCurrentDocumentation(docs); + Assert.AreEqual(new[] { "id", "cancellationToken" }, docs.Elements("param").Select(p => (string?)p.Attribute("name"))); + Assert.AreEqual("Current identifier.", docs.Elements("param").First().Value.Trim()); + AssertValidParameterReferences(shim); + Assert.AreEqual(originalDocs, RenderDocs(current.XmlDocs)); + Assert.AreEqual(new[] { "id", "expand", "cancellationToken" }, current.Signature.Parameters.Select(p => p.Name)); + Assert.AreEqual(new[] { "id", "cancellationToken" }, shim.Signature.Parameters.Select(p => p.Name)); + Assert.IsTrue(shim.Signature.Parameters.All(p => p.DefaultValue is null)); + Assert.IsFalse(shim.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Async)); + + using var writer = new CodeWriter(); + writer.WriteMethod(shim); + var syntax = SyntaxFactory.ParseMemberDeclaration(writer.ToString(false))!; + var invocation = syntax.DescendantNodes().OfType().Single(); + Assert.AreEqual(current.Signature.Name, ((MemberAccessExpressionSyntax)invocation.Expression).Name.Identifier.ValueText); + Assert.AreEqual(new[] { "id", "expand", "cancellationToken" }, + invocation.ArgumentList.Arguments.Select(a => a.NameColon?.Name.Identifier.ValueText)); + Assert.IsTrue(invocation.ArgumentList.Arguments[1].Expression.IsKind(SyntaxKind.DefaultLiteralExpression)); + Assert.AreEqual(1, syntax.DescendantNodes().OfType().Count()); + } + + [TestCase("Id", "id")] + [TestCase("some_id", "someId")] + [TestCase("class", "class")] + public void CompatibilityDocumentationMapsParametersAndPreservesExceptionReasons(string previousName, string currentName) + { + var current = CreateOperation(false, currentName); + var previous = CreatePreviousOperation(current, 24, previousName); + var id = current.Signature.Parameters[0]; + var expand = current.Signature.Parameters[1]; + current.XmlDocs.Update(exceptions: + [ + new XmlDocExceptionStatement(typeof(ArgumentException), "has an unsupported value.", [id, expand]), + new XmlDocExceptionStatement(typeof(InvalidOperationException), "cannot be expanded.", [expand]), + new XmlDocExceptionStatement(typeof(NotSupportedException), "The service does not support this operation.", []) + ]); + string originalDocs = RenderDocs(current.XmlDocs); + + var shim = CreateShim(current, previous); + var docs = ReadDocs(shim.XmlDocs); + AssertCurrentDocumentation(docs); + var parameterName = shim.Signature.Parameters[0].AsVariable().Declaration.RequestedName; + Assert.AreEqual(parameterName, (string?)docs.Elements("param").First().Attribute("name")); + Assert.AreEqual("Current identifier.", docs.Elements("param").First().Value.Trim()); + Assert.AreEqual(previousName, shim.Signature.Parameters[0].Name); + Assert.IsTrue(docs.Elements("exception").Any(e => e.Value.Contains("has an unsupported value."))); + Assert.IsTrue(docs.Elements("exception").Any(e => e.Value.Contains("The service does not support this operation."))); + Assert.IsFalse(docs.Elements("exception").Any(e => e.Value.Contains("cannot be expanded."))); + AssertValidParameterReferences(shim); + Assert.AreEqual(originalDocs, RenderDocs(current.XmlDocs)); + Assert.AreEqual(currentName, id.Name); + } + + [Test] + public void CompatibilityDocumentationPreservesParameterContentAndLaterReordering() + { + var current = CreateOperation(false); + var previous = CreatePreviousOperation(current, 12); + var id = current.Signature.Parameters[0]; + current.XmlDocs.Update(parameters: + [ + new XmlDocParamStatement(id, [$"Updated documentation with {typeof(string):C}."], + new XmlDocStatement("c", [$"identifier"])), + .. current.XmlDocs.Parameters.Skip(1) + ]); + string originalDocs = RenderDocs(current.XmlDocs); + var shim = CreateShim(current, previous); + var parameterDoc = ReadDocs(shim.XmlDocs).Elements("param").First(); + Assert.IsTrue(parameterDoc.Value.Contains("Updated documentation")); + Assert.IsNotNull(parameterDoc.Element("c")); + Assert.IsNotNull(parameterDoc.Element("see")); + Assert.AreEqual("Current identifier.", id.Description.ToString()); + + Assert.IsTrue(BackCompatHelper.TryRestorePreviousParameterOrder(shim, + WithParameters(shim.Signature, shim.Signature.Parameters.Reverse().ToArray()))); + shim.Update(suppressions: []); + AssertCurrentDocumentation(ReadDocs(shim.XmlDocs)); + Assert.AreEqual(new[] { "cancellationToken", "id" }, + ReadDocs(shim.XmlDocs).Elements("param").Select(p => (string?)p.Attribute("name"))); + AssertValidParameterReferences(shim); + Assert.AreEqual(originalDocs, RenderDocs(current.XmlDocs)); + } + + [Test] + public void NullableCompatibilityDocumentationIncludesShimGuard( + [Values(false, true)] bool async, + [Values(false, true)] bool addedOptionalParameter) + { + var current = CreateOperation(async); + var value = new ParameterProvider("value", $"Current value.", new CSharpType(typeof(int))); + var signature = WithParameters(current.Signature, + addedOptionalParameter ? [value, .. current.Signature.Parameters.Skip(1)] : [value, current.Signature.Parameters[2]]); + var docsBeforeUpdate = current.XmlDocs; + docsBeforeUpdate.Update(parameters: signature.Parameters.Select(p => new XmlDocParamStatement(p)).ToArray(), exceptions: []); + current.Update(signature: signature, xmlDocProvider: docsBeforeUpdate); + var previousValue = new ParameterProvider("value", $"Stale value.", new CSharpType(typeof(int), isNullable: true), Snippet.Default); + var previous = new MethodProvider( + WithParameters(current.Signature, [previousValue, current.Signature.Parameters.Last()]), + MethodBodyStatement.Empty, new TestTypeProvider(), + new XmlDocProvider(new XmlDocSummaryStatement([$"Stale summary."]))); + string originalDocs = RenderDocs(current.XmlDocs); + + var shim = CreateShim(current, previous); + var docs = ReadDocs(shim.XmlDocs); + AssertCurrentDocumentation(docs); + Assert.AreEqual("Current value.", docs.Elements("param").First().Value.Trim()); + Assert.IsFalse(docs.Value.Contains("Stale")); + var nullException = docs.Elements("exception").Single(e => ((string?)e.Attribute("cref"))!.Contains("ArgumentNullException")); + Assert.AreEqual("value", (string?)nullException.Element("paramref")?.Attribute("name")); + Assert.IsTrue(shim.Signature.Parameters[0].Type.IsNullable); + Assert.AreEqual(originalDocs, RenderDocs(current.XmlDocs)); + AssertValidParameterReferences(shim); + } + + [Test] + public void ReducedArityCompatibilityDocumentationFiltersRemovedParameter([Values(false, true)] bool async) + { + var current = CreateOperation(async); + current.Signature.Parameters[1].DefaultValue = null; + var previousParameters = new[] + { + new ParameterProvider("id", $"Stale identifier.", new CSharpType(typeof(string))), + new ParameterProvider("expand", $"Stale expansion.", new CSharpType(typeof(int), isNullable: true), Snippet.Default), + new ParameterProvider("cancellationToken", $"Stale cancellation.", new CSharpType(typeof(CancellationToken)), Snippet.Default) + }; + var previous = new MethodProvider(WithParameters(current.Signature, previousParameters), + MethodBodyStatement.Empty, new TestTypeProvider(), + new XmlDocProvider(new XmlDocSummaryStatement([$"Stale summary."]))); + string originalDocs = RenderDocs(current.XmlDocs); + + var shim = CreateShim(current, previous); + AssertCurrentDocumentation(ReadDocs(shim.XmlDocs)); + Assert.AreEqual(new[] { "id", "cancellationToken" }, shim.Signature.Parameters.Select(p => p.Name)); + Assert.IsNotNull(shim.Signature.Parameters[1].DefaultValue); + AssertValidParameterReferences(shim); + Assert.AreEqual(originalDocs, RenderDocs(current.XmlDocs)); + } + + [TestCase("absent")] + [TestCase("empty")] + [TestCase("inherit")] + public void CompatibilityDocumentationNeverFallsBackToStaleProse(string documentation) + { + var current = CreateOperation(false); + var previous = CreatePreviousOperation(current, 24); + var docs = documentation switch + { + "absent" => XmlDocProvider.Empty, + "empty" => new XmlDocProvider(new XmlDocSummaryStatement([$""]), returns: new XmlDocReturnsStatement($"")), + "inherit" => XmlDocProvider.InheritDocs, + _ => throw new ArgumentOutOfRangeException(nameof(documentation)) + }; + current.Update(xmlDocProvider: docs); + string originalDocs = RenderDocs(current.XmlDocs); + + var shim = CreateShim(current, previous); + Assert.AreEqual(originalDocs, RenderDocs(shim.XmlDocs)); + Assert.AreEqual(originalDocs, RenderDocs(current.XmlDocs)); + } + + [Test] + public void CompatibilityDocumentationUsesUndocumentedCurrentSignatureWithoutBaselineFallback() + { + var current = CreateOperation(false); + var previous = CreatePreviousOperation(current, 24); + current = new MethodProvider( + new MethodSignature(current.Signature.Name, null, current.Signature.Modifiers, current.Signature.ReturnType, + null, current.Signature.Parameters), + Snippet.Null, current.EnclosingType); + var shim = CreateShim(current, previous); + var docs = ReadDocs(shim.XmlDocs); + Assert.IsNull(docs.Element("summary")); + Assert.IsNull(docs.Element("returns")); + Assert.AreEqual("Current identifier.", docs.Elements("param").First().Value.Trim()); + Assert.IsFalse(docs.Value.Contains("Stale")); + AssertValidParameterReferences(shim); + } + + [Test] + public async Task CompatibilityDocumentationIsStableAcrossPublishedBaselines() + { + string source = File.ReadAllText(Path.Combine(Helpers.GetAssetFileOrDirectoryPath(false), "OperationClient.cs")); + var compilation = CSharpCompilation.Create("OriginalRelease", + [CSharpSyntaxTree.ParseText(source, new CSharpParseOptions(documentationMode: DocumentationMode.Diagnose))], + [ + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + MetadataReference.CreateFromFile(typeof(EditorBrowsableAttribute).Assembly.Location), + MetadataReference.CreateFromFile(Assembly.Load("System.Runtime").Location) + ], + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + string? expectedDocs = null; + for (int release = 0; release < 3; release++) + { + using var dll = new MemoryStream(); + using var xml = new MemoryStream(); + var emitted = compilation.Emit(dll, xmlDocumentationStream: xml); + Assert.IsTrue(emitted.Success, string.Join(Environment.NewLine, emitted.Diagnostics)); + Assert.IsFalse(emitted.Diagnostics.Any(d => d.Id is "CS1572" or "CS1734")); + var baseline = CSharpCompilation.Create("Baseline", references: + [ + .. compilation.References, + MetadataReference.CreateFromImage(dll.ToArray(), documentation: XmlDocumentationProvider.CreateFromBytes(xml.ToArray())) + ]); + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: () => Task.FromResult(baseline), includeXmlDocs: true); + var current = CreateOperation(false); + var currentAsync = CreateOperation(true); + var type = new TestTypeProvider(name: "OperationClient", methods: [current, currentAsync]); + if (release == 0) + { + var previousDocs = RenderDocs(type.LastContractView!.Methods.First(m => m.Signature.Name == "GetAllAsync").XmlDocs); + StringAssert.Contains("Stale summary.", previousDocs); + StringAssert.Contains("2026-03-01", previousDocs); + StringAssert.DoesNotContain(" m.Signature.Parameters.Count == 2).ToArray(); + Assert.AreEqual(2, shims.Length); + foreach (var shim in shims) + { + AssertCurrentDocumentation(ReadDocs(shim.XmlDocs)); + AssertValidParameterReferences(shim); + expectedDocs ??= RenderDocs(shim.XmlDocs); + Assert.AreEqual(expectedDocs, RenderDocs(shim.XmlDocs), $"Release {release}"); + } + string output = new TypeProviderWriter(type).Write().Content; + compilation = CSharpCompilation.Create($"Release{release}", + [CSharpSyntaxTree.ParseText(output, new CSharpParseOptions(documentationMode: DocumentationMode.Diagnose))], + compilation.References, new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + } + } + + private static MethodProvider CreateOperation(bool async, string idName = "id") + { + var id = new ParameterProvider(idName, $"Current identifier.", new CSharpType(typeof(string))); + var expand = new ParameterProvider("expand", $"Current expansion.", new CSharpType(typeof(int), isNullable: true), + Snippet.Default, location: ParameterLocation.Query); + var cancellation = new ParameterProvider("cancellationToken", $"Current cancellation.", new CSharpType(typeof(CancellationToken)), + Snippet.Default, location: ParameterLocation.Query); + var docs = new XmlDocProvider( + new XmlDocSummaryStatement([$"Current operation returning {typeof(string):C}."], + new XmlDocStatement($"", $"", [], + new XmlDocStatement("item", [], + new XmlDocStatement("term", [$"Default Api Version"]), + new XmlDocStatement("description", [$"2026-04-01"])))), + [new XmlDocParamStatement(id), new XmlDocParamStatement(expand), new XmlDocParamStatement(cancellation)], + [ + new XmlDocExceptionStatement(typeof(ArgumentNullException), [id, expand]), + new XmlDocExceptionStatement(typeof(ArgumentException), [expand]), + new XmlDocExceptionStatement(typeof(InvalidOperationException), "The operation failed.", []) + ], + new XmlDocReturnsStatement($"Current result as {typeof(string):C}.")); + return new MethodProvider( + new MethodSignature(async ? "GetAllAsync" : "GetAll", $"Signature description.", MethodSignatureModifiers.Public, + new CSharpType(async ? typeof(Task) : typeof(string)), $"Signature return.", [id, expand, cancellation]), + Snippet.Null, new TestTypeProvider(), docs); + } + + private static MethodProvider CreatePreviousOperation(MethodProvider current, int indentation, string idName = "id") + { + var continuation = new string(' ', indentation) + "Default Api Version.2026-03-01."; + return new MethodProvider( + new MethodSignature(current.Signature.Name, $"Stale summary.\n{continuation}", MethodSignatureModifiers.Public, + current.Signature.ReturnType, $"Stale return.", + [ + new ParameterProvider(idName, $"Stale identifier.", new CSharpType(typeof(string))), + new ParameterProvider("cancellationToken", $"Stale cancellation.", new CSharpType(typeof(CancellationToken)), Snippet.Default) + ]), + MethodBodyStatement.Empty, new TestTypeProvider()); + } + + private static MethodProvider CreateShim(MethodProvider current, MethodProvider previous) + { + var type = new CompatibilityTestType(current, previous); + var methods = new List { current }; + BackCompatHelper.AddBackCompatOverloads(type, methods); + Assert.AreEqual(2, methods.Count); + return methods[1]; + } + + private static MethodSignature WithParameters(MethodSignature signature, IReadOnlyList parameters) + => new MethodSignature(signature.Name, signature.Description, signature.Modifiers, signature.ReturnType, signature.ReturnDescription, parameters); + + private static string RenderDocs(XmlDocProvider docs) + { + using var writer = new CodeWriter(); + writer.WriteXmlDocsNoScope(docs); + return writer.ToString(false); + } + + private static XElement ReadDocs(XmlDocProvider docs) + => XElement.Parse("" + string.Join("\n", RenderDocs(docs).Split('\n') + .Where(line => line.StartsWith("///")).Select(line => line[3..])) + ""); + + private static void AssertCurrentDocumentation(XElement docs) + { + Assert.IsTrue(docs.Element("summary")!.Value.Contains("Current operation")); + Assert.IsNotNull(docs.Element("summary")!.Element("list")?.Element("item")?.Element("term")); + Assert.IsTrue(docs.Element("summary")!.Value.Contains("2026-04-01")); + Assert.IsTrue(docs.Element("summary")!.Descendants("see").Any()); + Assert.IsTrue(docs.Element("returns")!.Value.Contains("Current result")); + Assert.IsTrue(docs.Element("returns")!.Descendants("see").Any()); + Assert.IsFalse(docs.Value.Contains("Stale")); + Assert.IsFalse(docs.Value.Contains("2026-03-01")); + } + + private static void AssertValidParameterReferences(MethodProvider method) + { + var names = method.Signature.Parameters.Select(p => p.AsVariable().Declaration.RequestedName).ToHashSet(); + var docs = ReadDocs(method.XmlDocs); + foreach (var element in docs.Descendants().Where(e => e.Name.LocalName is "param" or "paramref")) + { + Assert.Contains((string?)element.Attribute("name"), names.ToArray()); + } + } + + private sealed class CompatibilityTestType(MethodProvider current, MethodProvider previous) + : TestTypeProvider(methods: [current]) + { + private protected override TypeProvider? BuildLastContractView(string? generatedTypeName = null, string? generatedTypeNamespace = null) + => new TestTypeProvider(methods: [previous]); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/BackCompatHelperTests/CompatibilityDocumentationIsStableAcrossPublishedBaselines/OperationClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/BackCompatHelperTests/CompatibilityDocumentationIsStableAcrossPublishedBaselines/OperationClient.cs new file mode 100644 index 00000000000..8f9707a56a0 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Utilities/TestData/BackCompatHelperTests/CompatibilityDocumentationIsStableAcrossPublishedBaselines/OperationClient.cs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Threading; +using System.Threading.Tasks; + +namespace Test +{ + public class OperationClient + { + /// + /// Stale summary. + /// Request Path./old/path.Operation Id.Old_List.Default Api Version.2026-03-01. + /// + /// Stale identifier. + /// Stale cancellation. + /// Stale result. + public string GetAll(string id, CancellationToken cancellationToken = default) => null; + + /// + /// Stale summary. + /// + /// Default Api Version2026-03-01 + /// + /// + /// Stale identifier. + /// Stale cancellation. + /// Stale result. + public Task GetAllAsync(string id, CancellationToken cancellationToken = default) => null; + } +} diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index b339b05c226..b1f42cef6d2 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -1092,6 +1092,10 @@ public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string con ### Client Methods +Compatibility overloads use the **current method's XML documentation**, not prose reconstructed from the released contract. This preserves structured summaries, current operation metadata, type references, and return documentation. Parameter documentation and exception parameter references are mapped to the compatibility signature; entries that refer only to omitted parameters are excluded. Additional validation performed by a shim, such as a null check before unwrapping a nullable value, is documented without changing the current method's documentation. + +The previous contract's DLL and XML documentation are selected by `ApiCompatVersion`. Using current documentation avoids retaining stale metadata or accumulating indentation as successive releases become the baseline. Repeated generation against the same unchanged baseline does not itself advance that baseline. Missing or empty current documentation does not fall back to released prose. + #### Scenario: New Optional Non-Body Parameter Added to a Service Method **Description:** When the current TypeSpec adds one or more new optional non-body parameters (e.g. query, header, path) to an existing service method, the generator emits a hidden back-compat overload that matches the previous contract's signature and delegates to the new method, passing `default` for the new parameter(s). The behavior is **intentionally restricted to non-body parameters** because adding a body parameter typically reflects a schema change and is handled differently. From 497e54cf16e4c99a0d2dbbf176529e5f34564ffa Mon Sep 17 00:00:00 2001 From: jolov Date: Wed, 16 Sep 2026 10:44:28 -0700 Subject: [PATCH 2/2] docs(http-client-csharp): remove compatibility guide additions Remove the four guide lines added by the operation compatibility documentation fix, as requested. Preserve the implementation and regression tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../generator/docs/backward-compatibility.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index b1f42cef6d2..b339b05c226 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -1092,10 +1092,6 @@ public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string con ### Client Methods -Compatibility overloads use the **current method's XML documentation**, not prose reconstructed from the released contract. This preserves structured summaries, current operation metadata, type references, and return documentation. Parameter documentation and exception parameter references are mapped to the compatibility signature; entries that refer only to omitted parameters are excluded. Additional validation performed by a shim, such as a null check before unwrapping a nullable value, is documented without changing the current method's documentation. - -The previous contract's DLL and XML documentation are selected by `ApiCompatVersion`. Using current documentation avoids retaining stale metadata or accumulating indentation as successive releases become the baseline. Repeated generation against the same unchanged baseline does not itself advance that baseline. Missing or empty current documentation does not fall back to released prose. - #### Scenario: New Optional Non-Body Parameter Added to a Service Method **Description:** When the current TypeSpec adds one or more new optional non-body parameters (e.g. query, header, path) to an existing service method, the generator emits a hidden back-compat overload that matches the previous contract's signature and delegates to the new method, passing `default` for the new parameter(s). The behavior is **intentionally restricted to non-body parameters** because adding a body parameter typically reflects a schema change and is handled differently.