Merge main: align with #10319 revert (drop AreNamesEqual on collection 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>

Author: Copilot

packages/http-client-csharp/eng/pipeline/publish.yml

@@ -106,6 +106,12 @@ extends:
         jobs:
           - job: CreatePR
             timeoutInMinutes: 90
+            variables:
+              # Redirect temp and cache directories to Agent.TempDirectory (a separate, larger partition)
+              # to avoid running out of disk space on the root partition during generation
+              TMPDIR: $(Agent.TempDirectory)
+              NUGET_PACKAGES: $(Agent.TempDirectory)/nuget
+              npm_config_cache: $(Agent.TempDirectory)/npm-cache
             steps:
               - checkout: self
               - pwsh: |
@@ -150,6 +156,7 @@ extends:
                   workingFile: $(Build.SourcesDirectory)/packages/http-client-csharp/.npmrc
 
               - download: current
+                artifact: build_artifacts_csharp
                 displayName: Download pipeline artifacts
 
               - pwsh: |

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs

@@ -1007,6 +1007,7 @@ internal static List<ParameterProvider> GetMethodParameters(
             int required = 100;
             int bodyRequired = 200;
             int bodyOptional = 300;
+            int contentType = 350;
             int optional = 400;
 
             var operation = serviceMethod.Operation;
@@ -1124,7 +1125,12 @@ internal static List<ParameterProvider> GetMethodParameters(
                         break;
                     case ParameterLocation.Query:
                     case ParameterLocation.Header:
-                        if (parameter.DefaultValue == null)
+                        if (inputParam is InputHeaderParameter { IsContentType: true }
+                            && !HasContentTypeBeforeBodyInLastContract(serviceMethod.Name, client.BackCompatProvider))
+                        {
+                            sortedParams.Add(contentType++, parameter);
+                        }
+                        else if (parameter.DefaultValue == null)
                         {
                             sortedParams.Add(required++, parameter);
                         }
@@ -1144,7 +1150,7 @@ internal static List<ParameterProvider> GetMethodParameters(
 
             if (operation.IsMultipartFormData)
             {
-                sortedParams.Add(bodyRequired++, ScmKnownParameters.ContentType);
+                sortedParams.Add(contentType++, ScmKnownParameters.ContentType);
             }
 
             if (methodType == ScmMethodKind.CreateRequest)
@@ -1159,6 +1165,62 @@ internal static List<ParameterProvider> GetMethodParameters(
             return [.. sortedParams.Values];
         }
 
+        /// <summary>
+        /// Checks if the last contract view contains a method matching the given name where
+        /// a "contentType" parameter appears before the body ("content") parameter.
+        /// If so, we should preserve that ordering for backward compatibility.
+        /// </summary>
+        private static bool HasContentTypeBeforeBodyInLastContract(string methodName, TypeProvider backCompatProvider)
+        {
+            const string contentTypeParamName = "contentType";
+            const string contentParamName = "content";
+
+            var lastContractMethods = backCompatProvider.LastContractView?.Methods;
+            if (lastContractMethods == null || lastContractMethods.Count == 0)
+            {
+                return false;
+            }
+
+            var syncMethodName = methodName;
+            var asyncMethodName = methodName + "Async";
+
+            foreach (var method in lastContractMethods)
+            {
+                if (!string.Equals(method.Signature.Name, syncMethodName, StringComparison.OrdinalIgnoreCase)
+                    && !string.Equals(method.Signature.Name, asyncMethodName, StringComparison.OrdinalIgnoreCase))
+                {
+                    continue;
+                }
+
+                int contentTypeIndex = -1;
+                int bodyIndex = -1;
+                for (int i = 0; i < method.Signature.Parameters.Count; i++)
+                {
+                    var param = method.Signature.Parameters[i];
+                    if (string.Equals(param.Name, contentTypeParamName, StringComparison.OrdinalIgnoreCase))
+                    {
+                        contentTypeIndex = i;
+                    }
+                    else if (string.Equals(param.Name, contentParamName, StringComparison.OrdinalIgnoreCase))
+                    {
+                        bodyIndex = i;
+                    }
+
+                    if (contentTypeIndex >= 0 && bodyIndex >= 0)
+                    {
+                        break;
+                    }
+                }
+
+                if (contentTypeIndex >= 0 && bodyIndex >= 0 && contentTypeIndex < bodyIndex)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
         internal static InputModelType GetSpreadParameterModel(InputParameter inputParam)
         {
             if (inputParam.Type is InputModelType model)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs

@@ -233,6 +233,135 @@ public void TestGetMethodParameters_ProperOrdering()
             Assert.AreEqual("b", orderedPathParams[2].Name);
         }
 
+        [TestCase(true, false)]
+        [TestCase(false, false)]
+        [TestCase(true, true)]
+        [TestCase(false, true)]
+        public void TestGetMethodParameters_ContentTypeAfterBody(bool isRequired, bool isExtensibleEnum)
+        {
+            InputType contentTypeType = isExtensibleEnum
+                ? InputFactory.StringEnum("ContentTypeEnum",
+                    [("application/json", "application/json"), ("application/xml", "application/xml")],
+                    isExtensible: true)
+                : InputPrimitiveType.String;
+            var contentTypeHeader = InputFactory.HeaderParameter(
+                "contentType",
+                contentTypeType,
+                isRequired: isRequired,
+                isContentType: true,
+                serializedName: "Content-Type",
+                defaultValue: isRequired ? null : InputFactory.Constant.String("application/json"));
+            var bodyParam = InputFactory.BodyParameter("body", InputPrimitiveType.String, isRequired: true);
+            var pathParam = InputFactory.PathParameter("skillId", InputPrimitiveType.String, isRequired: true);
+
+            var operation = InputFactory.Operation(
+                "UpdateSkillDefaultVersion",
+                parameters: [pathParam, contentTypeHeader, bodyParam]);
+
+            var serviceMethod = InputFactory.BasicServiceMethod(
+                "UpdateSkillDefaultVersion",
+                operation);
+
+            var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]);
+            var clientProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);
+            Assert.IsNotNull(clientProvider);
+
+            var methodParameters = RestClientProvider.GetMethodParameters(serviceMethod, ScmMethodKind.Protocol, clientProvider!);
+
+            Assert.AreEqual(3, methodParameters.Count);
+            Assert.AreEqual("skillId", methodParameters[0].Name);
+            Assert.AreEqual("content", methodParameters[1].Name);
+            Assert.AreEqual("contentType", methodParameters[2].Name);
+        }
+
+        [Test]
+        public async Task ContentTypeOrderPreservedFromLastContractView()
+        {
+            // Create an operation with a content-type header (union) and body
+            var contentTypeEnum = InputFactory.StringEnum("ContentTypeEnum",
+                [("application/json", "application/json"), ("application/xml", "application/xml")],
+                isExtensible: true);
+            var contentTypeHeader = InputFactory.HeaderParameter(
+                "contentType",
+                contentTypeEnum,
+                isRequired: true,
+                isContentType: true,
+                serializedName: "Content-Type");
+            var bodyParam = InputFactory.BodyParameter("body", InputPrimitiveType.String, isRequired: true);
+            var pathParam = InputFactory.PathParameter("skillId", InputPrimitiveType.String, isRequired: true);
+
+            var operation = InputFactory.Operation(
+                "UpdateSkillDefaultVersion",
+                parameters: [pathParam, contentTypeHeader, bodyParam]);
+
+            var serviceMethod = InputFactory.BasicServiceMethod(
+                "UpdateSkillDefaultVersion",
+                operation);
+
+            var client = InputFactory.Client("TestClient", methods: [serviceMethod]);
+
+            // Load with a last contract that has contentType before body
+            var generator = await MockHelpers.LoadMockGeneratorAsync(
+                clients: () => [client],
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType<ClientProvider>().FirstOrDefault();
+            Assert.IsNotNull(clientProvider);
+            Assert.IsNotNull(clientProvider!.LastContractView);
+
+            var methodParameters = RestClientProvider.GetMethodParameters(serviceMethod, ScmMethodKind.Protocol, clientProvider!);
+
+            // When the last contract had contentType before body, the ordering should be preserved
+            Assert.AreEqual(3, methodParameters.Count);
+            Assert.AreEqual("skillId", methodParameters[0].Name);
+            Assert.AreEqual("contentType", methodParameters[1].Name); // contentType stays before body for back-compat
+            Assert.AreEqual("content", methodParameters[2].Name);
+        }
+
+        [Test]
+        public async Task ContentTypeAfterBodyInLastContractView()
+        {
+            // Create an operation with a content-type header (union) and body
+            var contentTypeEnum = InputFactory.StringEnum("ContentTypeEnum",
+                [("application/json", "application/json"), ("application/xml", "application/xml")],
+                isExtensible: true);
+            var contentTypeHeader = InputFactory.HeaderParameter(
+                "contentType",
+                contentTypeEnum,
+                isRequired: true,
+                isContentType: true,
+                serializedName: "Content-Type");
+            var bodyParam = InputFactory.BodyParameter("body", InputPrimitiveType.String, isRequired: true);
+            var pathParam = InputFactory.PathParameter("skillId", InputPrimitiveType.String, isRequired: true);
+
+            var operation = InputFactory.Operation(
+                "UpdateSkillDefaultVersion",
+                parameters: [pathParam, contentTypeHeader, bodyParam]);
+
+            var serviceMethod = InputFactory.BasicServiceMethod(
+                "UpdateSkillDefaultVersion",
+                operation);
+
+            var client = InputFactory.Client("TestClient", methods: [serviceMethod]);
+
+            // Load with a last contract that already has contentType after body
+            var generator = await MockHelpers.LoadMockGeneratorAsync(
+                clients: () => [client],
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType<ClientProvider>().FirstOrDefault();
+            Assert.IsNotNull(clientProvider);
+            Assert.IsNotNull(clientProvider!.LastContractView);
+
+            var methodParameters = RestClientProvider.GetMethodParameters(serviceMethod, ScmMethodKind.Protocol, clientProvider!);
+
+            // When the last contract already had contentType after body, the new ordering is used
+            Assert.AreEqual(3, methodParameters.Count);
+            Assert.AreEqual("skillId", methodParameters[0].Name);
+            Assert.AreEqual("content", methodParameters[1].Name);
+            Assert.AreEqual("contentType", methodParameters[2].Name); // contentType after body
+        }
+
         [TestCase(true, true)]
         [TestCase(true, false)]
         [TestCase(false, true)]

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ContentTypeAfterBodyInLastContractView/TestClient.cs

@@ -0,0 +1,15 @@
+#nullable disable
+
+using System.ClientModel;
+using System.ClientModel.Primitives;
+using System.Threading.Tasks;
+
+namespace Sample
+{
+    public partial class TestClient
+    {
+        // This represents a previous contract where contentType already appears after body (content)
+        public virtual Task<ClientResult> UpdateSkillDefaultVersionAsync(string skillId, BinaryContent content, string contentType, RequestOptions options = null) { return null; }
+        public virtual ClientResult UpdateSkillDefaultVersion(string skillId, BinaryContent content, string contentType, RequestOptions options = null) { return null; }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ContentTypeOrderPreservedFromLastContractView/TestClient.cs

@@ -0,0 +1,15 @@
+#nullable disable
+
+using System.ClientModel;
+using System.ClientModel.Primitives;
+using System.Threading.Tasks;
+
+namespace Sample
+{
+    public partial class TestClient
+    {
+        // This represents a previous contract where contentType appears before body (content)
+        public virtual Task<ClientResult> UpdateSkillDefaultVersionAsync(string skillId, string contentType, BinaryContent content, RequestOptions options = null) { return null; }
+        public virtual ClientResult UpdateSkillDefaultVersion(string skillId, string contentType, BinaryContent content, RequestOptions options = null) { return null; }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs

@@ -286,7 +286,7 @@ private static bool TryBuildMethodArgumentsForOverload(
                 }
                 else
                 {
-                    arguments.Add(previousParameter);
+                    arguments.Add(parameter.PositionalReference(previousParameter));
                 }
             }
 

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs

@@ -1263,15 +1263,14 @@ _ when type.Equals(_additionalPropsUnknownType, ignoreNullable: true) => type,
         /// source-breaking changes for consumers of the library.
         /// </summary>
         /// <remarks>
-        /// The override is applied when the generated and last-contract property types differ but
-        /// are logically compatible. Two categories are supported:
+        /// The override is applied when the generated and last-contract property types differ.
+        /// Two categories are supported:
         /// <list type="bullet">
         /// <item>
         /// <description>
         /// Read-write collection properties (<see cref="CSharpType.IsReadWriteList"/> or
-        /// <see cref="CSharpType.IsReadWriteDictionary"/>) whose element/key/value type names
-        /// match the last contract. This handles cases such as <c>IReadOnlyList&lt;T&gt;</c>
-        /// being regenerated as <c>IList&lt;T&gt;</c>.
+        /// <see cref="CSharpType.IsReadWriteDictionary"/>). This handles cases such as
+        /// <c>IReadOnlyList&lt;T&gt;</c> being regenerated as <c>IList&lt;T&gt;</c>.
         /// </description>
         /// </item>
         /// <item>
@@ -1318,23 +1317,13 @@ private bool TryGetLastContractPropertyTypeOverride(
                 return false;
             }
 
-            // Read-write collections: allow the wrapper (e.g. IList<T> vs IReadOnlyList<T>) to
-            // change as long as the element/key/value type names still match. We compare
-            // Arguments by name (not just ElementType) so this covers both list element types
-            // and dictionary key/value types. AreNamesEqual is used rather than Equals because
-            // the argument types may come from different sources (TypeProvider vs compiled
-            // assembly) but represent the same logical type.
+            // Read-write collections: preserve the previous type whenever it differs (e.g.
+            // IReadOnlyList<T> regenerated as IList<T>). This matches the long-standing
+            // behavior in the inline back-compat block.
             if (outputProperty.Type.IsReadWriteList || outputProperty.Type.IsReadWriteDictionary)
             {
-                if (outputProperty.Type.Arguments.Count == candidate.Arguments.Count &&
-                    outputProperty.Type.Arguments.Zip(candidate.Arguments).All(
-                        pair => pair.First.AreNamesEqual(pair.Second)))
-                {
-                    lastContractPropertyType = candidate;
-                    return true;
-                }
-
-                return false;
+                lastContractPropertyType = candidate;
+                return true;
             }
 
             // Other properties: require the entire type name (top-level plus any generic

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs

@@ -193,7 +193,54 @@ public async Task BackCompatibility_NewModelPropertyAdded()
             Assert.IsNotNull(body);
             var result = body!.ToDisplayString();
             Assert.AreEqual(
-                "return PublicModel1(stringProp, modelProp, listProp, dictProp: default);\n",
+                "return PublicModel1(stringProp: stringProp, modelProp: modelProp, listProp: listProp, dictProp: default);\n",
+                result);
+        }
+
+        // This test validates that when a new property is added AND the previous contract had a different
+        // parameter ordering, the backward-compat overload uses named arguments to correctly call the current method.
+        [Test]
+        public async Task BackCompatibility_NewPropertyAddedWithDifferentParamOrder()
+        {
+            _instance = (await MockHelpers.LoadMockGeneratorAsync(
+                inputNamespaceName: "Sample.Namespace",
+                inputModelTypes: ModelList,
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object;
+
+            var modelFactory = _instance!.OutputLibrary.ModelFactory.Value;
+            Assert.AreEqual("SampleNamespaceModelFactory", modelFactory.Name);
+
+            modelFactory.ProcessTypeForBackCompatibility();
+
+            var methods = modelFactory.Methods;
+            // There should be an additional method for backward compatibility
+            Assert.AreEqual(ModelList.Length - ModelList.Where(m => m.Access == "internal").Count() + 1, methods.Count);
+
+            var currentOverloadMethod = methods
+                .FirstOrDefault(m => m.Signature.Name == "PublicModel1" && m.Signature.Parameters.Any(p => p.Name == "dictProp"));
+            var backwardCompatibilityMethod = methods
+                .FirstOrDefault(m => m.Signature.Name == "PublicModel1" && m.Signature.Parameters.All(p => p.Name != "dictProp"));
+            Assert.IsNotNull(currentOverloadMethod);
+            Assert.IsNotNull(backwardCompatibilityMethod);
+
+            // validate the signature of the backward compatibility method preserves the previous parameter order
+            var parameters = backwardCompatibilityMethod!.Signature.Parameters;
+            Assert.AreEqual(3, parameters.Count);
+            Assert.AreEqual("modelProp", parameters[0].Name);
+            Assert.AreEqual("stringProp", parameters[1].Name);
+            Assert.AreEqual("listProp", parameters[2].Name);
+            foreach (var param in parameters)
+            {
+                Assert.IsNull(param.DefaultValue);
+            }
+
+            // validate the previous method body uses named arguments to ensure correct mapping
+            // even though the parameter order differs between the previous and current methods
+            var body = backwardCompatibilityMethod!.BodyStatements;
+            Assert.IsNotNull(body);
+            var result = body!.ToDisplayString();
+            Assert.AreEqual(
+                "return PublicModel1(stringProp: stringProp, modelProp: modelProp, listProp: listProp, dictProp: default);\n",
                 result);
         }