fix(dpg): path/url parameters not ordered by optional value

src/AutoRest.CSharp/LowLevel/Output/OperationMethodChainBuilder.cs

@@ -80,7 +80,7 @@ public void BuildNextPageMethod(IReadOnlyDictionary<InputOperation, OperationMet
         public LowLevelClientMethod BuildOperationMethodChain()
         {
             var returnTypeChain = BuildReturnTypes();
-            var protocolMethodParameters = _orderedParameters.Select(p => p.Protocol).WhereNotNull().OrderBy(p => p.DefaultValue != null).ToArray();
+            var protocolMethodParameters = _orderedParameters.Select(p => p.Protocol).WhereNotNull().ToArray();
             var protocolMethodSignature = new MethodSignature(_restClientMethod.Name, _restClientMethod.Summary, _restClientMethod.Description, _restClientMethod.Accessibility | Virtual, returnTypeChain.Protocol, null, protocolMethodParameters);
             var convenienceMethod = ShouldConvenienceMethodGenerated(returnTypeChain) ? BuildConvenienceMethod(returnTypeChain) : null;
 
@@ -176,10 +176,9 @@ private ConvenienceMethod BuildConvenienceMethod(ReturnTypeChain returnTypeChain
                 name = _restClientMethod.Name.IsLastWordSingular() ? $"{_restClientMethod.Name}Value" : $"{_restClientMethod.Name.LastWordToSingular()}Values";
             }
 
-            var parameters = _orderedParameters.Select(p => p.Convenience).WhereNotNull().OrderBy(p => p.DefaultValue != null).ToArray();
+            var parameters = _orderedParameters.Select(p => p.Convenience).WhereNotNull().ToArray();
             var protocolToConvenience = _orderedParameters
                 .Where(p => p.Protocol != null)
-                .OrderBy(p => p.Protocol!.DefaultValue != null)
                 .Select(p => (p.Protocol!, p.Convenience))
                 .ToArray();
 
@@ -192,7 +191,8 @@ private void BuildParameters()
         {
             var operationParameters = Operation.Parameters.Where(rp => !RestClientBuilder.IsIgnoredHeaderParameter(rp));
 
-            var pathParameters = new Dictionary<string, InputParameter>();
+            var requiredPathParameters = new Dictionary<string, InputParameter>();
+            var optionalPathParameters = new Dictionary<string, InputParameter>();
             var requiredRequestParameters = new List<InputParameter>();
             var optionalRequestParameters = new List<InputParameter>();
 
@@ -225,8 +225,11 @@ private void BuildParameters()
                             : requestConditionSerializationFormat;
 
                         break;
-                    case { Location: RequestLocation.Uri or RequestLocation.Path }:
-                        pathParameters.Add(operationParameter.NameInRequest, operationParameter);
+                    case { Location: RequestLocation.Uri or RequestLocation.Path, DefaultValue: null }:
+                        requiredPathParameters.Add(operationParameter.NameInRequest, operationParameter);
+                        break;
+                    case { Location: RequestLocation.Uri or RequestLocation.Path, DefaultValue: not null }:
+                        optionalPathParameters.Add(operationParameter.NameInRequest, operationParameter);
                         break;
                     case { IsRequired: true, DefaultValue: null }:
                         requiredRequestParameters.Add(operationParameter);
@@ -238,10 +241,12 @@ private void BuildParameters()
             }
 
             AddWaitForCompletion();
-            AddUriOrPathParameters(Operation.Uri, pathParameters);
-            AddUriOrPathParameters(Operation.Path, pathParameters);
+            AddUriOrPathParameters(Operation.Uri, requiredPathParameters);
+            AddUriOrPathParameters(Operation.Path, requiredPathParameters);
             AddQueryOrHeaderParameters(requiredRequestParameters);
             AddBody(bodyParameter, contentTypeRequestParameter);
+            AddUriOrPathParameters(Operation.Uri, optionalPathParameters);
+            AddUriOrPathParameters(Operation.Path, optionalPathParameters);
             AddQueryOrHeaderParameters(optionalRequestParameters);
             AddRequestConditionHeaders(requestConditionHeaders, requestConditionRequestParameter, requestConditionSerializationFormat);
             AddRequestContext();
@@ -269,10 +274,6 @@ private void AddUriOrPathParameters(string uriPart, IReadOnlyDictionary<string,
                 {
                     AddParameter(text, requestParameter);
                 }
-                else
-                {
-                    ErrorHelpers.ThrowError($"\n\nError while processing request '{uriPart}'\n\n  '{text}' in URI is missing a matching definition in the path parameters collection{ErrorHelpers.UpdateSwaggerOrFile}");
-                }
             }
         }
 

src/AutoRest.CSharp/Properties/launchSettings.json

@@ -476,6 +476,14 @@
       "commandName": "Project",
       "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestServerProjectsLowLevel\\paging\\Generated"
     },
+    "Parameters-Cadl": {
+      "commandName": "Project",
+      "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestProjects\\Parameters-Cadl\\Generated"
+    },
+    "Parameters-LowLevel": {
+      "commandName": "Project",
+      "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestProjects\\Parameters-LowLevel\\Generated"
+    },
     "ParameterSequence-LowLevel": {
       "commandName": "Project",
       "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestProjects\\ParameterSequence-LowLevel\\Generated"
@@ -496,10 +504,6 @@
       "commandName": "Project",
       "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestProjects\\ReferenceTypes\\Generated"
     },
-    "RequestContextAllOptional-LowLevel": {
-      "commandName": "Project",
-      "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestProjects\\RequestContextAllOptional-LowLevel\\Generated"
-    },
     "required-optional": {
       "commandName": "Project",
       "commandLineArgs": "--standalone $(SolutionDir)\\test\\TestServerProjects\\required-optional\\Generated"

test/AutoRest.TestServer.Tests/Mgmt/OutputLibrary/MgmtParamOrderingTests.cs

@@ -53,6 +53,7 @@ public void ValidateOperationParameterList(string operationGroupName, string met
         [TestCase(typeof(VirtualMachineScaleSetResource), "Deallocate", true, new[]{ "vmInstanceIDs", "expand" }, new[] { false, false })]
         [TestCase(typeof(VirtualMachineScaleSetResource), "DeleteInstances", true, new[]{ "vmInstanceIDs", "forceDeletion"}, new[] { true, false })]
         [TestCase(typeof(VirtualMachineScaleSetResource), "GetInstanceView", false, new[]{ "filter", "expand" }, new[] { true, false })]
+        [TestCase(typeof(AvailabilitySetsRestOperations), "Update", false, new[]{ "subscriptionId", "resourceGroupName", "patch", "availabilitySetName" }, new[] { true, true, true, false })]
         public void ValidateOperationMethodParameterList(Type type, string methodName, bool isLro, string[] parameterNames, bool[] isRequiredParameters)
         {
             var parameters = type.GetMethod(methodName).GetParameters();

test/AutoRest.TestServerLowLevel.Tests/LowLevel/ParameterSequenceTests.cs

@@ -0,0 +1,28 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Linq;
+using Parameters_LowLevel;
+using ParametersCadl;
+using NUnit.Framework;
+
+namespace AutoRest.LowLevel.Tests
+{
+    public class ParameterSequenceTests
+    {
+        [TestCase(typeof(ParametersLowlevelClient), "OptionalPathParameters", new string[] { "id", "skip", "name", "context" }, new bool[] { false, false, true, true})]
+        [TestCase(typeof(ParametersLowlevelClient),"OptionalPathParametersWithMixedSequence", new string[] { "id", "name", "skip", "context" }, new bool[] { false, true, true, true})]
+        [TestCase(typeof(ParametersLowlevelClient),"OptionalPathBodyParametersWithMixedSequence", new string[] { "name", "skip", "content", "id", "top", "max", "context" }, new bool[] { false, false, false, true, true, true, true})]
+        [TestCase(typeof(ParameterOrdersClient),"OperationValue", new string[] { "start", "end", "cancellationToken" }, new bool[] { false, true, true})]
+        [TestCase(typeof(ParameterOrdersClient),"Operation", new string[] { "start", "end", "context" }, new bool[] { false, true, true})]
+        [TestCase(typeof(ParameterOrdersClient),"Operation2Value", new string[] { "end", "start", "cancellationToken" }, new bool[] { false, true, true})]
+        [TestCase(typeof(ParameterOrdersClient),"Operation2", new string[] { "end", "start", "context" }, new bool[] { false, true, true})]
+        public void OptionalParameterAtEnd(Type type, string methodName, string[] parameterNames, bool[] isOptional)
+        {
+            var method = type.GetMethod(methodName);
+            Assert.AreEqual(parameterNames, method.GetParameters().Select(p => p.Name).ToArray());
+            Assert.AreEqual(isOptional, method.GetParameters().Select(p => p.HasDefaultValue).ToArray());
+        }
+    }
+}

test/AutoRest.TestServerLowLevel.Tests/RequestContextAllOptionalTests.cs

@@ -4,7 +4,7 @@
 using Azure;
 using Azure.Core;
 using NUnit.Framework;
-using RequestContextAllOptional_LowLevel;
+using Parameters_LowLevel;
 
 namespace AutoRest.TestServer.Tests
 {
@@ -14,7 +14,7 @@ public class RequestContextAllOptionalTests
         public void NoRequestBodyResponseBody([Values("NoRequestBodyResponseBody", "NoRequestBodyResponseBodyAsync")] string methodName)
         {
             TypeAsserts.HasPublicInstanceMethod(
-                typeof(RequestContextAllOptionalClient),
+                typeof(ParametersLowlevelClient),
                 methodName,
                 new TypeAsserts.Parameter[] {
                     new("id", typeof(int)),
@@ -28,7 +28,7 @@ public void NoRequestBodyResponseBody([Values("NoRequestBodyResponseBody", "NoRe
         [Test]
         public void OptionalRequestContextDeleteNoRequestBodyResponseBody()
         {
-            var client = typeof(RequestContextAllOptionalClient);
+            var client = typeof(ParametersLowlevelClient);
             var method = client.GetMethod("DeleteNoRequestBodyResponseBody");
             var parameters = method.GetParameters();
 
@@ -42,7 +42,7 @@ public void OptionalRequestContextDeleteNoRequestBodyResponseBody()
         [Test]
         public void RequestBodyResponseBody()
         {
-            var client = typeof(RequestContextAllOptionalClient);
+            var client = typeof(ParametersLowlevelClient);
             var method = client.GetMethod("RequestBodyResponseBody");
             var parameters = method.GetParameters();
 
@@ -56,7 +56,7 @@ public void RequestBodyResponseBody()
         [Test]
         public void NoRequestBodyNoResponseBody()
         {
-            var client = typeof(RequestContextAllOptionalClient);
+            var client = typeof(ParametersLowlevelClient);
             var method = client.GetMethod("NoRequestBodyNoResponseBody");
             var parameters = method.GetParameters();
 
@@ -68,7 +68,7 @@ public void NoRequestBodyNoResponseBody()
         [Test]
         public void RequestBodyNoResponseBody()
         {
-            var client = typeof(RequestContextAllOptionalClient);
+            var client = typeof(ParametersLowlevelClient);
             var method = client.GetMethod("RequestBodyNoResponseBody");
             var parameters = method.GetParameters();
 

test/TestProjects/MgmtParamOrdering/Generated/RestOperations/AvailabilitySetsRestOperations.cs

@@ -123,7 +123,7 @@ public Response<AvailabilitySetData> CreateOrUpdate(string subscriptionId, strin
             }
         }
 
-        internal HttpMessage CreateUpdateRequest(string subscriptionId, string resourceGroupName, string availabilitySetName, AvailabilitySetPatch patch)
+        internal HttpMessage CreateUpdateRequest(string subscriptionId, string resourceGroupName, AvailabilitySetPatch patch, string availabilitySetName)
         {
             var message = _pipeline.CreateMessage();
             var request = message.Request;
@@ -150,19 +150,19 @@ internal HttpMessage CreateUpdateRequest(string subscriptionId, string resourceG
         /// <summary> Update an availability set. </summary>
         /// <param name="subscriptionId"> Subscription credentials which uniquely identify Microsoft Azure subscription. The subscription ID forms part of the URI for every service call. </param>
         /// <param name="resourceGroupName"> The name of the resource group. </param>
-        /// <param name="availabilitySetName"> The name of the availability set. </param>
         /// <param name="patch"> Parameters supplied to the Update Availability Set operation. </param>
+        /// <param name="availabilitySetName"> The name of the availability set. </param>
         /// <param name="cancellationToken"> The cancellation token to use. </param>
-        /// <exception cref="ArgumentNullException"> <paramref name="subscriptionId"/>, <paramref name="resourceGroupName"/>, <paramref name="availabilitySetName"/> or <paramref name="patch"/> is null. </exception>
+        /// <exception cref="ArgumentNullException"> <paramref name="subscriptionId"/>, <paramref name="resourceGroupName"/>, <paramref name="patch"/> or <paramref name="availabilitySetName"/> is null. </exception>
         /// <exception cref="ArgumentException"> <paramref name="subscriptionId"/>, <paramref name="resourceGroupName"/> or <paramref name="availabilitySetName"/> is an empty string, and was expected to be non-empty. </exception>
-        public async Task<Response<AvailabilitySetData>> UpdateAsync(string subscriptionId, string resourceGroupName, string availabilitySetName, AvailabilitySetPatch patch, CancellationToken cancellationToken = default)
+        public async Task<Response<AvailabilitySetData>> UpdateAsync(string subscriptionId, string resourceGroupName, AvailabilitySetPatch patch, string availabilitySetName = "test", CancellationToken cancellationToken = default)
         {
             Argument.AssertNotNullOrEmpty(subscriptionId, nameof(subscriptionId));
             Argument.AssertNotNullOrEmpty(resourceGroupName, nameof(resourceGroupName));
-            Argument.AssertNotNullOrEmpty(availabilitySetName, nameof(availabilitySetName));
             Argument.AssertNotNull(patch, nameof(patch));
+            Argument.AssertNotNullOrEmpty(availabilitySetName, nameof(availabilitySetName));
 
-            using var message = CreateUpdateRequest(subscriptionId, resourceGroupName, availabilitySetName, patch);
+            using var message = CreateUpdateRequest(subscriptionId, resourceGroupName, patch, availabilitySetName);
             await _pipeline.SendAsync(message, cancellationToken).ConfigureAwait(false);
             switch (message.Response.Status)
             {
@@ -181,19 +181,19 @@ public async Task<Response<AvailabilitySetData>> UpdateAsync(string subscription
         /// <summary> Update an availability set. </summary>
         /// <param name="subscriptionId"> Subscription credentials which uniquely identify Microsoft Azure subscription. The subscription ID forms part of the URI for every service call. </param>
         /// <param name="resourceGroupName"> The name of the resource group. </param>
-        /// <param name="availabilitySetName"> The name of the availability set. </param>
         /// <param name="patch"> Parameters supplied to the Update Availability Set operation. </param>
+        /// <param name="availabilitySetName"> The name of the availability set. </param>
         /// <param name="cancellationToken"> The cancellation token to use. </param>
-        /// <exception cref="ArgumentNullException"> <paramref name="subscriptionId"/>, <paramref name="resourceGroupName"/>, <paramref name="availabilitySetName"/> or <paramref name="patch"/> is null. </exception>
+        /// <exception cref="ArgumentNullException"> <paramref name="subscriptionId"/>, <paramref name="resourceGroupName"/>, <paramref name="patch"/> or <paramref name="availabilitySetName"/> is null. </exception>
         /// <exception cref="ArgumentException"> <paramref name="subscriptionId"/>, <paramref name="resourceGroupName"/> or <paramref name="availabilitySetName"/> is an empty string, and was expected to be non-empty. </exception>
-        public Response<AvailabilitySetData> Update(string subscriptionId, string resourceGroupName, string availabilitySetName, AvailabilitySetPatch patch, CancellationToken cancellationToken = default)
+        public Response<AvailabilitySetData> Update(string subscriptionId, string resourceGroupName, AvailabilitySetPatch patch, string availabilitySetName = "test", CancellationToken cancellationToken = default)
         {
             Argument.AssertNotNullOrEmpty(subscriptionId, nameof(subscriptionId));
             Argument.AssertNotNullOrEmpty(resourceGroupName, nameof(resourceGroupName));
-            Argument.AssertNotNullOrEmpty(availabilitySetName, nameof(availabilitySetName));
             Argument.AssertNotNull(patch, nameof(patch));
+            Argument.AssertNotNullOrEmpty(availabilitySetName, nameof(availabilitySetName));
 
-            using var message = CreateUpdateRequest(subscriptionId, resourceGroupName, availabilitySetName, patch);
+            using var message = CreateUpdateRequest(subscriptionId, resourceGroupName, patch, availabilitySetName);
             _pipeline.Send(message, cancellationToken);
             switch (message.Response.Status)
             {

test/TestProjects/MgmtParamOrdering/MgmtParamOrdering.json

@@ -100,7 +100,8 @@
                         "in": "path",
                         "required": true,
                         "type": "string",
-                        "description": "The name of the availability set."
+                        "description": "The name of the availability set.",
+                        "x-ms-client-default": "test"
                     },
                     {
                         "name": "parameters",