Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 3, 2025

The FlowTestContextCancellationTokenFixer was not correctly handling methods with named arguments when adding CancellationToken parameters. This resulted in invalid C# syntax being generated.

Problem

When applying the code fixer to methods with optional parameters, the fixer would generate invalid code:

public void DoSomething(string x = "", int i = 1, CancellationToken ct = default) {}

[TestMethod]
public void A()
{
    DoSomething(); // Fixed as DoSomething(TestContext.CancellationToken); ❌ Invalid - skips optional params
    DoSomething(i: 15); // Fixed as DoSomething(i: 15, TestContext.CancellationToken); ❌ Invalid - mixes named/positional
}

Solution

Enhanced the AddCancellationTokenArgument method to:

  1. Analyze method signatures using the semantic model to find CancellationToken parameters and their names
  2. Determine appropriate argument style based on existing arguments and parameter positions
  3. Use named arguments when necessary to maintain valid C# syntax

The fixer now generates correct code:

DoSomething(); // → DoSomething(ct: TestContext.CancellationToken); ✅
DoSomething(i: 15); // → DoSomething(i: 15, ct: TestContext.CancellationToken); ✅
DoSomething("test"); // → DoSomething("test", ct: TestContext.CancellationToken); ✅
DoSomething("test", 42); // → DoSomething("test", 42, TestContext.CancellationToken); ✅

Key Changes

  • Smart argument detection: Uses ShouldUseNamedArgument helper to determine when named arguments are required
  • Parameter name preservation: Correctly uses the actual parameter name (e.g., ct, cancellationToken)
  • Backward compatibility: Maintains existing behavior for Task.Delay and other common scenarios
  • Comprehensive test coverage: Added test cases covering various argument combinations

The fix ensures the code fixer generates syntactically correct and consistent C# code that follows language conventions.

Fixes #6500.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • cflvsblobprodcus383.vsblob.vsassets.io
    • Triggering command: /home/REDACTED/work/testfx/testfx/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/testfx/testfx/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/testfx/testfx/artifacts/toolset/11.0.0-beta.25452.3.txt (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Code fixer for TestContext.CancellationToken doesn't handle named arg Fix CancellationToken code fixer to handle named arguments properly Sep 3, 2025
@Copilot Copilot AI requested a review from Evangelink September 3, 2025 09:00
Copilot finished work on behalf of Evangelink September 3, 2025 09:00
@Evangelink Evangelink marked this pull request as ready for review September 3, 2025 09:14
@Evangelink Evangelink enabled auto-merge (squash) September 3, 2025 09:14
// Create the TestContext.CancellationToken expression
MemberAccessExpressionSyntax testContextExpression = SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.IdentifierName(testContextMemberName),
SyntaxFactory.IdentifierName("CancellationToken"));

editor.ReplaceNode(invocationExpression, (node, _) =>
editor.ReplaceNode(invocationExpression, (node, generator) =>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
editor.ReplaceNode(invocationExpression, (node, generator) =>
editor.ReplaceNode(invocationExpression, (node, _) =>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code fixer for TestContext.CancellationToken doesn't handle named arg
3 participants