Skip to content

Fix: CT parameter name uniqueness in CancellationToken overload generation #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sergey-tihon merged 2 commits into from
Mar 25, 2026
Merged

Fix: CT parameter name uniqueness in CancellationToken overload generation #339

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c71e4cc
Initial plan
Copilot Mar 23, 2026
27888f8
fix: generate unique CT parameter name to avoid collision with OpenAP…
Copilot Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,

List.append required optional

let providedParameters =
let usedNames, providedParameters =
((Set.empty, []), orderedParameters)
||> List.fold(fun (names, parameters) current ->
let names, paramName = uniqueParamName names current
Expand All @@ -173,16 +173,21 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
ProvidedParameter(paramName, paramType, false, paramDefaultValue)

(names, providedParam :: parameters))
|> snd
// because we built up our list in reverse order with the fold,
// reverse it again so that all required properties come first
|> List.rev
|> fun (names, ps) -> names, List.rev ps

let parameters =
if includeCancellationToken then
let ctParam =
ProvidedParameter("cancellationToken", typeof<Threading.CancellationToken>)
// 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))

Comment on lines +181 to 189
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a second ad-hoc unique-name generator (Seq.initInfinite + Set.contains) even though the codebase already has UniqueNameGenerator with the same suffixing convention (and case-insensitive uniqueness). Consider factoring this into a shared helper (or reusing UniqueNameGenerator by seeding it with usedNames) to avoid future divergence in how “unique names” are defined.

Suggested change
// 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))
// conflict with any OpenAPI parameter already in use, using the shared
// UniqueNameGenerator to keep suffixing and case sensitivity consistent.
let nameGen = UniqueNameGenerator()
usedNames |> Set.iter (fun n -> nameGen.Claim n |> ignore)
let ctParamName = nameGen.Claim "cancellationToken"

Copilot uses AI. Check for mistakes.
let ctParam = ProvidedParameter(ctParamName, typeof<Threading.CancellationToken>)
providedParameters @ [ ctParam ]
Comment on lines +180 to 191
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a regression test for the name-collision scenario this change fixes (an OpenAPI parameter named cancellationToken). Without a test that loads/generates a client from such a spec and asserts the generated overload compiles (and/or that the injected CT param gets a suffixed name), this could regress unnoticed.

Copilot uses AI. Check for mistakes.
else
providedParameters
Expand Down
Loading