microsoft / vs-threading

The Microsoft.VisualStudio.Threading is a xplat library that provides many threading and synchronization primitives used in Visual Studio and other applications.
Other
983 stars 146 forks source link

VSTHRD200 should be able to have method type configured #1327

Open jamiehankins opened 3 months ago

jamiehankins commented 3 months ago

Is your feature request related to a problem? Please describe.

This analyzer reports all instances of this condition. There are different schools of thought about requiring an Async suffix on awaitable methods.

Describe the solution you'd like

I'd like to see .editorconfig settings to modify the behavior of this analyzer. dotnet_diagnostic.VSTHRD200.allowed_exceptions = private, internal, protected, no_non_async

Whether it takes this exact form doesn't matter. I just want to shut this warning up for private methods.

Describe alternatives you've considered

I can silence all instances of this analyzer, silence it by file, live with warnings, or follow rules that we don't agree with.

Additional context

I use source generators for binding methods to a view, and the "fix" functionality renames methods but not the generated reference to them in XAML. So this:

private async Task AddStation() { }

which is generated as

public global::CommunityToolkit.Mvvm.Input.IRelayCommand DeleteUserCommand => deleteUserCommand ??= new global::CommunityToolkit.Mvvm.Input.RelayCommand(new global::System.Action(DeleteUser), () => CanDeleteUser);

would be DeleteUserAsyncCommand, and I would have to go through all of my XAML views and fix it.

AArnott commented 3 months ago

Thanks for this thoughtful write-up. I'd add "local functions" to your list of possible variants. At the moment, I think VSTHRD200 ignores them, and I'm content with that policy.

As for the rest of the variations you propose, it adds complexity, and that would need to be accompanied by additional automated testing. I don't see Visual Studio funding this in the forseeable future, but if you're interested in contributing this feature with tests, I could see us merging it. If you want to take this on, we can iterate on the .editorconfig syntax.

We should also consider #1315.