Skip to content
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.

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

Implement analyzer to warn for use of Assert methods in async void #4640

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

Youssef1313
Copy link
Member

Fixes #4613

@Youssef1313 Youssef1313 force-pushed the async-void-assert-analyzer branch 2 times, most recently from 11d6a73 to c09eca4 Compare January 14, 2025 14:42
@Youssef1313 Youssef1313 force-pushed the async-void-assert-analyzer branch from c09eca4 to 8646dec Compare January 14, 2025 14:42
Evangelink
Evangelink previously approved these changes Jan 14, 2025
Co-authored-by: Amaury Levé <amauryleve@microsoft.com>

Verified

This commit was signed with the committer’s verified signature.
danielroe Daniel Roe
@Evangelink Evangelink enabled auto-merge (squash) January 15, 2025 13:13
@Evangelink Evangelink merged commit 0640599 into microsoft:main Jan 15, 2025
9 checks passed
@Youssef1313 Youssef1313 deleted the async-void-assert-analyzer branch January 15, 2025 13:15
{
if (containingSymbol is IMethodSymbol { IsAsync: true, ReturnsVoid: true })
{
return true;
Copy link

@UliKu-philips UliKu-philips Feb 19, 2025

Choose a reason for hiding this comment

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

ReturnsVoid seems to be true also for async Task methods...
(getting the new warning now in .net framework 4.8 project for async Task TestMethod())

Copy link
Member Author

Choose a reason for hiding this comment

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

@UliKu-philips Could you please open an issue for that, with a repro? I'm not able to reproduce the behavior you describe.

Copy link

@UliKu-philips UliKu-philips Feb 21, 2025

Choose a reason for hiding this comment

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

actually I think I am wrong... in the example here, triggering MSTEST0040 is caused by the async lambda. The Subscribe method takes an Action<T> which is void:

        [TestMethod]
        public async Task TestMethodAsync()
        {
            using (var waitEvent = new ManualResetEventSlim())
            {
                using (Observable.Timer(dueTime: TimeSpan.FromSeconds(0), period: TimeSpan.FromSeconds(1)).Subscribe(
                    async t =>
                    {
                        await Task.Delay(1);
                        Assert.IsTrue(t > 0);
                        waitEvent.Set();
                    }))
                {
                    waitEvent.Wait(TimeSpan.FromSeconds(2));
                    await Task.Delay(1);
                }
            }
        }

https://github.com/UliKu-philips/UnitTestAsyncTaskWarning/blob/main/UnitTestProject1/UnitTest1.cs

Thanks for the great work!

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

Successfully merging this pull request may close these issues.

Don't assert on async delegate
3 participants