-
Notifications
You must be signed in to change notification settings - Fork 22
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
Rm "async" anywhere from IAsyncEnumerable & methods returning it #905
Conversation
tests/D2L.CodeStyle.Analyzers.Test/Async/Generator/AsyncToSyncMethodTransformerTests.cs
Show resolved
Hide resolved
src/D2L.CodeStyle.Analyzers/Async/Generator/AsyncToSyncMethodTransformer.cs
Outdated
Show resolved
Hide resolved
src/D2L.CodeStyle.Analyzers/Async/Generator/AsyncToSyncMethodTransformer.cs
Outdated
Show resolved
Hide resolved
src/D2L.CodeStyle.Analyzers/Async/Generator/AsyncToSyncMethodTransformer.cs
Outdated
Show resolved
Hide resolved
tests/D2L.CodeStyle.Analyzers.Test/Async/Generator/AsyncToSyncMethodTransformerTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jeff Ashton <[email protected]>
…Brightspace/D2L.CodeStyle into anacoda/handle-iasyncenumerable
src/D2L.CodeStyle.Analyzers/Async/Generator/AsyncToSyncMethodTransformer.cs
Outdated
Show resolved
Hide resolved
@@ -73,6 +75,13 @@ AttributeSyntax attribute | |||
); | |||
|
|||
private SyntaxToken RemoveAsyncSuffix( SyntaxToken ident, bool optional = false ) { | |||
if( m_removeAsyncAnywhere && ident.ValueText.Contains( "Async" ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this check optional
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, because there are cases where a function may return IAsyncEnumerable and not end in Async and we want the sync generator to transform it without a diagnostic
TA177961
Motivation is that the sync generator currently can't convert IAsyncEnumerable and most related functions because the Async keyword isn't at the end, for example here:
https://github.com/Brightspace/lms/blob/c4f2338b4d7f6b582b8c56ac959fd953f0846ec3/lp/framework/core/D2L.LP/LP/Security/OAuth2/AccessTokens/Keys/MsSqlKeyDataProvider.cs#L63
In general, there is no clear naming convention for methods that return IAsyncEnumerable except for adding the keyword async somewhere: https://stackoverflow.com/questions/59439281/is-there-a-definitive-naming-convention-for-methods-returning-iasyncenumerable/59452285#59452285
So for converting these we will change all variables/return types defined IAsyncEnumerable to IEnumerable, as well as removing "async" from any method that returns an IAsyncEnumerable.
I also had to make new test methods for this because we needed another method in the class that returns an IAsyncEnumerable