Fix CancellationToken parameter ordering and name collision in v3 OperationCompiler (#341)

* Initial plan

* fix: insert CT between required and optional params; generate unique CT name

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/b0c519de-0186-40ca-8174-42ed67a5316a

* fix: add explicit restore + --no-restore to BuildTests to fix NETSDK1005

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/565d6633-576d-4587-b924-a29b0ea53c2c

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

Author: Copilot

build.fsx

@@ -105,7 +105,12 @@ Target.createFinal "StopServer" (fun _ ->
 //Process.killAllByName "dotnet"
 )
 
-Target.create "BuildTests" (fun _ -> dotnet "build" "SwaggerProvider.TestsAndDocs.sln -c Release")
+Target.create "BuildTests" (fun _ ->
+    // Explicit restore ensures project.assets.json has all target frameworks before the build.
+    // Without this, the inner-build restores triggered by Paket.Restore.targets may overwrite
+    // the assets file with only one TFM, causing NETSDK1005 for the other TFM.
+    dotnet "restore" "SwaggerProvider.TestsAndDocs.sln"
+    dotnet "build" "SwaggerProvider.TestsAndDocs.sln -c Release --no-restore")
 
 // --------------------------------------------------------------------------------------
 // Run the unit tests using test runner

src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs

@@ -96,7 +96,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
         let (|NoMediaType|_|)(content: IDictionary<string, OpenApiMediaType>) =
             if isNull content || content.Count = 0 then Some() else None
 
-        let payloadMime, parameters =
+        let payloadMime, parameters, ctArgIndex =
             /// handles de-duplicating Swagger parameter names if the same parameter name
             /// appears in multiple locations in a given operation definition.
             let uniqueParamName usedNames (param: IOpenApiParameter) =
@@ -147,18 +147,15 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
 
             let payloadTy = bodyFormatAndParam |> Option.map fst |> Option.defaultValue NoData
 
-            let orderedParameters =
-                let required, optional =
-                    [ yield! openApiParameters
-                      if bodyFormatAndParam.IsSome then
-                          yield bodyFormatAndParam.Value |> snd ]
-                    |> List.distinctBy(fun op -> op.Name, op.In)
-                    |> List.partition(_.Required)
+            let requiredOpenApiParams, optionalOpenApiParams =
+                [ yield! openApiParameters
+                  if bodyFormatAndParam.IsSome then
+                      yield bodyFormatAndParam.Value |> snd ]
+                |> List.distinctBy(fun op -> op.Name, op.In)
+                |> List.partition(_.Required)
 
-                List.append required optional
-
-            let usedNames, providedParameters =
-                ((Set.empty, []), orderedParameters)
+            let buildProvidedParameters usedNames (paramList: IOpenApiParameter list) =
+                ((usedNames, []), paramList)
                 ||> List.fold(fun (names, parameters) current ->
                     let names, paramName = uniqueParamName names current
 
@@ -173,26 +170,37 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
                             ProvidedParameter(paramName, paramType, false, paramDefaultValue)
 
                     (names, providedParam :: parameters))
-                |> fun (names, ps) -> names, List.rev ps
+                |> fun (finalNames, ps) -> finalNames, List.rev ps
+
+            let namesAfterRequired, requiredProvidedParams =
+                buildProvidedParameters Set.empty requiredOpenApiParams
 
-            let parameters =
+            let _, optionalProvidedParams =
+                buildProvidedParameters namesAfterRequired optionalOpenApiParams
+
+            let ctArgIndex, parameters =
                 if includeCancellationToken then
-                    // Find a unique name for the CancellationToken parameter that doesn't
-                    // conflict with any OpenAPI parameter already in use.
-                    let ctParamName =
-                        Seq.initInfinite(fun i ->
-                            if i = 0 then
-                                "cancellationToken"
-                            else
-                                $"cancellationToken{i}")
-                        |> Seq.find(fun n -> not(Set.contains n usedNames))
+                    // Collect all used param names to generate a unique CT parameter name
+                    let usedNames =
+                        (requiredProvidedParams @ optionalProvidedParams)
+                        |> List.map(fun p -> p.Name)
+                        |> Set.ofList
+
+                    let rec findUniqueName candidate n =
+                        if Set.contains candidate usedNames then
+                            findUniqueName $"cancellationToken{n}" (n + 1)
+                        else
+                            candidate
 
-                    let ctParam = ProvidedParameter(ctParamName, typeof<Threading.CancellationToken>)
-                    providedParameters @ [ ctParam ]
+                    let ctName = findUniqueName "cancellationToken" 1
+                    let ctParam = ProvidedParameter(ctName, typeof<Threading.CancellationToken>)
+                    // CT is inserted after required params so it never follows optional params
+                    let ctArgIndex = List.length requiredProvidedParams
+                    ctArgIndex, requiredProvidedParams @ [ ctParam ] @ optionalProvidedParams
                 else
-                    providedParameters
+                    -1, requiredProvidedParams @ optionalProvidedParams
 
-            payloadTy.ToMediaType(), parameters
+            payloadTy.ToMediaType(), parameters, ctArgIndex
 
         // find the inner type value
         let retMimeAndTy =
@@ -278,16 +286,21 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
                         // Locates parameters matching the arguments
                         let mutable payloadExp = None
 
-                        // When the CancellationToken overload is generated, CancellationToken is always appended last.
-                        // Extract it by position to avoid name-collision issues and invalid Expr.Coerce
-                        // on a struct type (which generates an invalid castclass IL instruction).
+                        // When the CancellationToken overload is generated, CT is inserted at ctArgIndex
+                        // (after required params, before optional params). Extract it by that known index
+                        // to avoid name-collision issues and invalid Expr.Coerce on a struct type.
                         let apiArgs, ct =
                             let allArgs = List.tail args // skip `this`
 
                             if includeCancellationToken then
-                                match List.rev allArgs with
-                                | ctArg :: revApiArgs -> List.rev revApiArgs, Expr.Cast<Threading.CancellationToken>(ctArg)
-                                | [] -> failwith "Expected CancellationToken argument but argument list was empty"
+                                let ctArg = List.item ctArgIndex allArgs
+
+                                let apiArgs =
+                                    allArgs
+                                    |> List.indexed
+                                    |> List.choose(fun (i, a) -> if i = ctArgIndex then None else Some a)
+
+                                apiArgs, Expr.Cast<Threading.CancellationToken>(ctArg)
                             else
                                 allArgs, <@ Threading.CancellationToken.None @>