Skip to content

Comments

fix: add missing lastParameter variable in TimeoutCancellationTokenAnalyzer#4956

Merged
thomhurst merged 1 commit intomainfrom
fix/timeout-analyzer-lastparam
Feb 19, 2026
Merged

fix: add missing lastParameter variable in TimeoutCancellationTokenAnalyzer#4956
thomhurst merged 1 commit intomainfrom
fix/timeout-analyzer-lastparam

Conversation

@thomhurst
Copy link
Owner

Summary

  • PR fix: point analyzer diagnostics to specific parameters instead of methods #4924 introduced a reference to lastParameter on line 73 of TimeoutCancellationTokenAnalyzer.cs without declaring the variable, causing CS0103 build errors on main
  • This breaks all PR CI checks (locale tests de-DE, fr-FR, pl-PL fail because the build fails)
  • Adds var lastParameter = parameters[parameters.Length - 1] declaration before the usage
  • Also points the CancellationTokenMustBeLastParameter diagnostic at the actual CancellationToken parameter location instead of the method

Test plan

  • dotnet build TUnit.Analyzers/TUnit.Analyzers.csproj passes with 0 errors
  • CI locale tests (de-DE, fr-FR, pl-PL) should pass after merge

…alyzer

PR #4924 introduced a reference to `lastParameter` without declaring
it, causing CS0103 build errors. Add the variable declaration and also
point the CancellationTokenMustBeLastParameter diagnostic at the actual
CancellationToken parameter location.
@thomhurst thomhurst merged commit 808e8ac into main Feb 19, 2026
11 of 14 checks passed
@thomhurst thomhurst deleted the fix/timeout-analyzer-lastparam branch February 19, 2026 08:03
@claude
Copy link
Contributor

claude bot commented Feb 19, 2026

Code Review

No high-signal issues found. Checked for bugs and CLAUDE.md compliance.

Summary

This is a correct and minimal fix for the regression introduced in #4924. The two changes are both good:

  1. var lastParameter = parameters[parameters.Length - 1]; — Properly declares the missing variable that was causing the CS0103 compiler error. The array access is safe because the parameters.IsDefaultOrEmpty guard earlier in the method guarantees at least one element by this point.

  2. Improved diagnostic location — Changing the CancellationTokenMustBeLastParameter diagnostic to point at parameters[cancellationTokenIndex] instead of context.Symbol.Locations is a meaningful improvement: it highlights the misplaced CancellationToken parameter directly in the IDE rather than the method declaration site, giving developers a more actionable error squiggle.

Minor Note (non-blocking)

CLAUDE.md asks to "prefer latest syntax and APIs." Since ImmutableArray<T> supports C# 8's index-from-end operator and the project targets LangVersion=latest, the new line could use parameters[^1] instead of parameters[parameters.Length - 1]. That said, the existing codebase uses both patterns in the analyzer project, so this is not a blocker — just something to consider for future consistency.

Overall this is a well-scoped, correct fix.

thomhurst added a commit that referenced this pull request Feb 19, 2026
PR #4956 changed the CancellationTokenMustBeLastParameter diagnostic
to point at the CancellationToken parameter instead of the method name.
Update 3 test expectations to match the new diagnostic location.
thomhurst added a commit that referenced this pull request Feb 19, 2026
* fix: update Core public API snapshots for all frameworks

The merged PRs #4929 (hardcoded constants) and #4949 (retry policies)
added new public API surface to TUnit.Core but the DotNet8_0, DotNet9_0,
and DotNet10_0 verified snapshot files were not updated, causing
Core_Library_Has_No_API_Changes test failures in all PR CI pipelines.

* fix: update analyzer tests for CancellationToken parameter location

PR #4956 changed the CancellationTokenMustBeLastParameter diagnostic
to point at the CancellationToken parameter instead of the method name.
Update 3 test expectations to match the new diagnostic location.
This was referenced Feb 22, 2026
This was referenced Feb 23, 2026
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.

1 participant