-
Notifications
You must be signed in to change notification settings - Fork 466
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
Cleanup DoNotUseWhenAllOrWaitAllWithSingleArgument #5060
Cleanup DoNotUseWhenAllOrWaitAllWithSingleArgument #5060
Conversation
@mavasani I'm unable to debug the failure.
But the behavior of IOperation API looks very strange: Is that a bug in the operation tree? |
Codecov Report
@@ Coverage Diff @@
## release/6.0.1xx-preview4 #5060 +/- ##
=========================================================
Coverage 95.55% 95.56%
=========================================================
Files 1205 1203 -2
Lines 274343 274337 -6
Branches 16632 16632
=========================================================
+ Hits 262151 262157 +6
Misses 9988 9988
+ Partials 2204 2192 -12 |
TestCodeFixToEnableAnalyzerReleaseTracking failed 😕 , closing and re-opening |
...alyzers/Core/Microsoft.NetCore.Analyzers/Tasks/DoNotUseWhenAllOrWaitAllWithSingleArgument.cs
Show resolved
Hide resolved
@@ -12,7 +13,8 @@ | |||
|
|||
namespace Microsoft.NetCore.Analyzers.Tasks | |||
{ | |||
public abstract class DoNotUseWhenAllOrWaitAllWithSingleArgumentFixer : CodeFixProvider | |||
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared] |
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.
oops 😅 I really thought I had this change in already. Thanks for fixing!
@@ -112,7 +112,8 @@ private static bool IsSingleTaskArgument(IInvocationOperation invocation, INamed | |||
return false; | |||
} | |||
|
|||
return namedTypeSymbol.Equals(taskType) || namedTypeSymbol.ConstructedFrom.Equals(genericTaskType); | |||
return namedTypeSymbol.Equals(taskType, SymbolEqualityComparer.Default) || |
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 thought symbol.Equals(otherSymbol)
implicitly used the SymbolEqualityComparer.Default
if available?
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.
@ryzngard It does. But there is an analyzer (RS1024) to enforce using SymbolEqualityComparer explicitly to make the intent clear.
Currently the analyzer is disabled due to few bugs and will be re-enabled once they are fixed.
It looks like you figured this out, but the Value being an implicitly generated IArrayCreationOperation changes the behavior slightly. |
Thanks for doing this! |
Quick follow-up to #4841
cc: @ryzngard