Skip to content

Fixes handling of methods Task.WaitAsync and SemaphoreSlim.WaitAsync#122

Merged
virzak merged 6 commits into
zompinc:masterfrom
michelebastione:waitasync-fixes
Jun 25, 2026
Merged

Fixes handling of methods Task.WaitAsync and SemaphoreSlim.WaitAsync#122
virzak merged 6 commits into
zompinc:masterfrom
michelebastione:waitasync-fixes

Conversation

@michelebastione

@michelebastione michelebastione commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Description

This PR addresses two closely related bugs that caused entire method bodies being inadvertently dropped at the invocation of Task.WaitAsync or SemaphoreSlim.WaitAsync.

Both issues originated from a syntax check for the string "WaitAsync":

  1. In AsyncToSyncRewriter.ReplaceAsync the pattern matching for case MemberAccessExpressionSyntax lacked a fallback for validation purposes (when called from EndWithAsync). Hence WaitAsync calls chained at the end of a method (e.g., FooAsync().WaitAsync()) made the entire expression chain always return null, leading to them ultimately being discarded.
  2. Due to the check being based solely on the ExpressionSyntax value, there was no differentiation between Task methods, which are supposed to be dropped, and methods such as SemaphoreSlim.WaitAsync, which should be converted to SemaphoreSlim.Wait instead. As a result, synchronization primitives were being deleted from the generated code.

Changes Introduced

  1. More clearly separated the concerns of methods ReplaceAsync and EndsWithAsync. The latter now implements a fallback to a non null value for the MemberAccessExpressionSyntax pattern matching case, thus preventing the entire chain call to be dropped.
  2. Added the TryReplaceIdentifier method that uses the code block's SemanticModel for making sure that a symbol called "WaitAsync" is removed only when its containing type corresponds to System.Threading.Tasks.Task or System.Threading.Tasks.TimeProviderTaskExtensions.

fixes #119
fixes #121

@michelebastione michelebastione changed the title Fix handling of methods Task.WaitAsync and SemaphoreSlim.WaitAsync Fixes handling of methods Task.WaitAsync and SemaphoreSlim.WaitAsync Jun 20, 2026
@michelebastione

Copy link
Copy Markdown
Contributor Author

I assume the failed job is simply due to the fact that there is no package ready to be published, as I noticed the same error in other PRs, but do let me know if there's anything I'm supposed to fix.

virzak and others added 3 commits June 25, 2026 06:55
download-artifact@v5 no longer populates a per-name packages/ subdirectory
from a nameless "download all" step, so the sign job's packages/ directory
was empty and the packages-signed artifact was never uploaded, failing publish.

Download the packages artifact explicitly into packages/.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fork PRs don't have access to the signing/publish secrets, so the sign
and publish jobs can't do anything useful and the publish job fails when
the (never-produced) packages-signed artifact can't be downloaded.

Gate both jobs to run only on non-fork events (push, workflow_dispatch,
and same-repo PRs), leaving build+test+pack to run for everyone.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@virzak

virzak commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Hi again, @michelebastione,

Thanks for the contribution.
I removed some of the boilerplate code from the test cases and fixed the GitHub actions.

@virzak virzak merged commit f1fb155 into zompinc:master Jun 25, 2026
4 checks passed
@virzak

virzak commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Your changes are in 2.0.15 on nuget

@michelebastione michelebastione deleted the waitasync-fixes branch June 25, 2026 18:07
@michelebastione

Copy link
Copy Markdown
Contributor Author

Thanks @virzak! We've been hard at work on MiniExcel, in the past year we fixed 100+ issues and added many new features, and your project definitely helped us navigate through it more swiftly. Now that we're almost ready to release the full fledged v2 I thought it could be a good moment to take a step back and give back a little, so here I am lol.
If you're still using MiniExcel please feel free to leave us more feedback on what you would like to see fixed or implemented before we ship the new version!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants