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

MA0157 phrasing is unclear #782

Closed
martindisch opened this issue Jan 10, 2025 · 6 comments · Fixed by #783
Closed

MA0157 phrasing is unclear #782

martindisch opened this issue Jan 10, 2025 · 6 comments · Fixed by #783

Comments

@martindisch
Copy link
Contributor

martindisch commented Jan 10, 2025

Version of the Meziantou.Analyzer NuGet package

2.0.185

Rule Identifier

MA0157

Target Framework

net9.0

C# Language version

C# 13

Description

Updated the issue because at first I thought it was a bug, now I think I'm just confused :)

Based on the examples for MA0156

// compliant
IAsyncEnumerable<string> FooAsync() => throw null;

// non-compliant
IAsyncEnumerable<string> Foo() => throw null;

and MA0157

// compliant
IAsyncEnumerable<string> Foo() => throw null;

// non-compliant
IAsyncEnumerable<string> FooAsync() => throw null;

it looks like they're mutually exclusive. The first is for when you want an Async suffix for methods that return an IAsyncEnumerable<T> and the second is for when you don't want that. So we can choose which style we prefer by enabling either one of them.

If my understanding is correct, then the naming "MA0157 - Do not use 'Async' suffix when a method does not return IAsyncEnumerable" seems confusing and doesn't represent what's going on. Because it's still about cases that do return an IAsyncEnumerable<T>. Am I getting this right?

@martindisch martindisch changed the title MA0157 thinks method doesn't return IAsyncEnumerable<T> MA0157 phrasing is unclear Jan 10, 2025
@meziantou
Copy link
Owner

meziantou commented Jan 10, 2025

If you want to enforce the naming convention:

  • You have to choose between MA0137 and MA0138 for methods returning awaitable types such as Task
  • You have to choose between MA0156 and MA0157 for methods returning IAsyncEnumerable type

@martindisch
Copy link
Contributor Author

Thanks for pointing this out, it makes sense when you say it like that. However I'm still a bit confused because

  • Based on the examples given, MA0137 and MA0138 don't seem to be mutually exclusive. One says use an async suffix when there is an awaitable type returned (Task FooAsync()) and the other says don't use an async suffix when there's no awaitable type returned (void Foo()). It's different cases they're dealing with.
  • For MA0156 and MA0157 the situation is slightly different. Both deal with the case where an IAsyncEnumerable<T> is returned, but one wants the suffix (IAsyncEnumerable<string> FooAsync()) and the other doesn't (IAsyncEnumerable<string> Foo()). Here we really have to choose.

That's why I'm so confused with the error message of MA0157, because the code example is for a case where an async enumerable is returned, but the error says "Do not use 'Async' suffix when a method does not return IAsyncEnumerable".

@meziantou
Copy link
Owner

You are right for MA0137 and MA0138. For IAsyncEnumerable, there is no consensus about the Async suffix. Is the method async or is the enumeration async? Even in the .NET BCL, you can see both. That's why you have to choose which rule you want.

@martindisch
Copy link
Contributor Author

Ah I'm beginning to understand! Then the only thing that throws me off is the message for MA0157, wouldn't it be more accurate to put it like in #783?

@meziantou
Copy link
Owner

Thanks for the PR! The message will be much clearer :)

@martindisch
Copy link
Contributor Author

Thanks for your help 👍

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

Successfully merging a pull request may close this issue.

2 participants