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

VSTHRD104 should not fire for extension methods wrapping async methods #1215

Open
markusschaber opened this issue Jul 6, 2023 · 1 comment

Comments

@markusschaber
Copy link

Bug description

VSTHRD104 claims: "Expose an async version of this method that does not synchronously block. Then simplify this method to call that async method within a JoinableTaskFactory.Run delegate."

I did exactly this, exposing the async methods on an interface, and the synchroneous method added via extension methods (so not all implementations need to provide their own implementation).

However, now VSTHRD104 fires on just those extension methods.

Repro steps

public interface IMessagePoolRemoteRpcService
{
    Task ClearStatusPoolAsync(string category);
}

public static class SyncRemoteMessagePoolExtensions
{
    private static readonly JoinableTaskFactory Jtf = new(new JoinableTaskContext());

// VSTHRD104 fires here...
    public static void ClearStatusPool(this IMessagePoolRemoteRpcService service, string category)
        => Jtf.Run(() => service.ClearStatusPoolAsync(category));
}

Expected behavior

I don't expect VSTHRD104 to trigger on a method which just implements the the pattern suggested by the same analyzer.

Actual behavior

I get a warning of VSTHRD104 on the static extension method I created to implement the suggestion of just the same rule.

  • Version used: Microsoft.VisualStudio.Threading.Analyzers 17.6.40, indirectly referenced via Microsoft.VisualStudio.Threading
  • Application (if applicable):

Additional context

Add any other context about the problem here.

@AArnott
Copy link
Member

AArnott commented Jul 18, 2023

Your scenario here seems valid. I'm not sure when we'll have time to fix this. If you'd care to submit a PR with a regression test, I'd be happy to review and merge it.

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

No branches or pull requests

2 participants