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

.editorconfig Async methods should have "Async" suffix when interface in play #40050

Closed
vsfeedback opened this issue Nov 28, 2019 · 4 comments · Fixed by MicrosoftDocs/visualstudio-docs#4435

Comments

@vsfeedback
Copy link

This issue has been moved from a ticket on Developer Community.


I use the following .editoconfig settings to require async methods in our codebase to have their name ending in "Async".

[*.{cs,vb}]

# Async methods should have "Async" suffix
dotnet_naming_rule.async_methods_end_in_async.symbols = any_async_methods
dotnet_naming_rule.async_methods_end_in_async.style = end_in_async
dotnet_naming_rule.async_methods_end_in_async.severity = warning

dotnet_naming_symbols.any_async_methods.applicable_kinds = method
dotnet_naming_symbols.any_async_methods.applicable_accessibilities = *
dotnet_naming_symbols.any_async_methods.required_modifiers = async

dotnet_naming_style.end_in_async.required_suffix = Async
dotnet_naming_style.end_in_async.capitalization = pascal_case

While it works great, it doesn't work when interfaces come into play. Take below as an example. Get should be prompted as a warning that it should end in "Async". It makes sense that the interface member isn't, because:

  1. It doesn't have the "required_modifiers" async that the config requires
  2. I'm not 100% sure it's even considered a "method" but I couldn't find another "applicable_kinds" value that seemed appropriate. Is there one?

The fact that the interface member isn't highlighted then seems to flow through to the concretion, I guess because there's some logic that says editorconfig prefs should be ignored if it's simply following the interface? Comment out Get member of IMyContract and Get in Something gets highlighted.

public interface IMyContract
{
    Task<string> Get();
}

public class Something : IMyContract
{
    public async Task<string> Get() => await Task.FromResult("");
}

So it feels as if:

  1. I need a way to target the interface member and enforce the suffix
    (e.g. interface_member that returns Task? Just wonder if there is ever a case for returning Task when an implementation isn't async though?), or
  2. The concretion should still get the rule violation in this specific case

Cheers,
Ben


Original Comments

Visual Studio Feedback System on 4/3/2019, 09:44 PM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.

Visual Studio Feedback System on 4/4/2019, 04:58 AM:

Thank you for sharing your feedback! Our teams prioritize action on product issues with broad customer impact. See details at: https://docs.microsoft.com/en-us/visualstudio/ide/report-a-problem?view=vs-2017#faq . In case you need answers to common questions or need assisted support, be sure to use https://visualstudio.microsoft.com/vs/support/ . We'll keep you posted on any updates to this feedback.

ben.mccallum on 4/6/2019, 09:44 AM:

One thing that is tricky with a solution here is that if you rely on some third-party packages and are implementing their interfaces, you don't really want to get warnings about the naming convention if you can't really control their interface and it's naming convention.

For instance, our solution uses MediatR extensively so we're inheriting from IRequestHandler which has a member called "Handle". Ideally that'd be HandleAsync, but we don't want to get warnings given "it is what it is". So any solution here would need to understand that third-party deps should not create warnings.

Cheers again.

Visual Studio Feedback System on 8/23/2019, 03:00 AM:

I'm closing this report, because in the last 90 days there was no internal developer activity and very small number of new votes or comments. Sorry, we would like to look at this further, but we just don’t have the time right now. Our goal in closing older issues such as this one is to keep customers informed on which issues are currently being pursued by Visual Studio teams. If you still have trouble with our latest version, please report it as a new problem.

andrew.grady on 11/26/2019, 06:45 AM:

I am also running into this issue and I spent hours trying to figure out why it worked in some cases but not others. The existing behavior is confusing. In my case, all public async methods are tied to an interface. I would assume that would be the case for most others as coding to an interface is a best practice.

So my editorconfig rule (the same as the example above) only picks up private async methods which is a small fraction of the async methods implemented.

Can you please reconsider fixing this or at the very least documenting it for others who will probably run into this? Your documentation linked below makes no mention of this limitation.

https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-naming-conventions?view=vs-2019


Original Solutions

(no solutions)

@sharwell
Copy link
Member

The naming conventions feature for .editorconfig is not able to apply the full rules for the async method suffix. However, the Microsoft.VisualStudio.Threading.Analyzers package implements this analyzer as VSTHRD200.

@gradyal
Copy link

gradyal commented Dec 2, 2019

@sharwell I'd like to request that it be fully implemented. If it's not possible, can you please update the documentation below to avoid confusion?

https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-naming-conventions?view=vs-2019

@CenturySparkle
Copy link

CenturySparkle commented Oct 20, 2023

I've been trying to find the source culprit of this erroneous message for a couple years now and I finally found it. I had been blaming Resharper, but turns out it is Visual Studio itself! I have turn off the rule method_should_be_pascal_case because it was throwing warnings on all of our interfaces. Please fix this!

The worst part is that in interfaces, the asyncmethod and method naming rules conflict with each other. Async or no Async you get a warning.

@sharwell
Copy link
Member

@CenturySparkle Can you file a new issue for the problem you encountered? It would be best to include a copy of some source code where a warning was reported, and the .editorconfig file.

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

Successfully merging a pull request may close this issue.

6 participants