Don't use Thread.Sleep() analyzers - initial implementation #50
Conversation
Codecov Report
@@ Coverage Diff @@
## master #50 +/- ##
==========================================
+ Coverage 81.75% 82.22% +0.47%
==========================================
Files 36 49 +13
Lines 2970 3741 +771
Branches 226 252 +26
==========================================
+ Hits 2428 3076 +648
- Misses 434 553 +119
- Partials 108 112 +4
Continue to review full report at Codecov.
|
…Yield()`. Don't use 0 as an Thread.Sleep's argument in other tests
update DontUseThreadSleepCodeUniversalCodeFixProvider CodeAction's title so that it mentions Task.Yield() as a possible fix
…rted to 'await Task.Yield()'
…es which are semanticaly equal to zero
…xpressionSyntaxExtensions
… in Thread.Sleep() method call this includes code changes, new extension method and changes in CodeFixProvider
… provider for VisualBasic
| return typeMetadata.Equals(propertySymbol.ContainingType) && (propertySymbol.Name == propertyName); | ||
| } | ||
|
|
||
| public static bool IsInsideAsyncCode(this ExpressionSyntax invocationExpression, ref SyntaxNode enclosingMethodOrFunctionDeclaration) |
There was a problem hiding this comment.
I could probably use IMethodSymbol.IsAsync in this method.
Current approach is purely syntactic, so it should be faster (I don't know how much it matters).
| { | ||
| context.RegisterCodeFix( | ||
| CodeAction.Create( | ||
| "Use `await Task.Delay(...)` or `await Task.Yield()`", |
There was a problem hiding this comment.
What is the reason for having the same message for both here? How does VS render the backticks?
There was a problem hiding this comment.
If there were two separate messages for code fixes, semantic model would had to be awaited when the code fix is registered which would degrade performance. Currently, semantic model is awaited only if a user applies code fix. I don't think that user experience will be much better if there are two separate message.
I can change the code to include two separate code fix messages if you do think that it's worth it.
Backticks are displayed literally in VS2015 Update 2.
I believe that either quotes or backticks should be used to quote code - IMHO it improves readability. I used backticks because I wanted to be consistent with the AvoidAsyncSuffixCodeFixProvider. On the other hand no quotation characters are used in UseConfigureAwaitCodeFixProvider.
There was a problem hiding this comment.
No idea if it is worth it, just looked a bit strange with a roulette type code fix :)
| var firstNodeWithCorrectSpan = root | ||
| .FindNode(diagnostic.Location.SourceSpan); | ||
| InvocationExpressionSyntax expression = firstNodeWithCorrectSpan | ||
| .DescendantNodesAndSelf() |
There was a problem hiding this comment.
I don't know Roslyn well enough to know if firstNodeWithCorrectSpan is always not null here.
There was a problem hiding this comment.
I don't know Roslyn very well either, but I think that I can justify that expression and firstNodeWithCorrectSpan are not null.
- In
DontUseThreadSleepAnalyzerBase, theSyntaxNodeActionis registered onSyntaxKind.InvocationExpressionand reports analysis onInvocationExpression.Syntax's location - I use diangnostic's location to assign value to
firstNodeWithCorrectSpan.firstNodeWithCorrectSpanis assigned usingSyntaxNode.FindNodemethod which (by default) returns outermost node encompassing the given span (source: http://www.coderesx.com/roslyn/html/6497A5DA.htm). - There must be a node of type
InvocationExpressionSyntaxamongfirstNodeWithCorrectSpan's descendant nodes because diagnostic was reported on usingInvocationExpressionSyntax's location, soexpressionis not null.
…eturn true if and only if non-null value is assigned to out parameter
|
It's been a while since I've created this PR. Do you have more feedback? I would like to make further contributions to this project, but I don't know if it's still maintained. |
…ternative to using Thread.Sleep()
|
I've moved |
I've created two analyzers:
Analyzers have a common code fix which fixes occurrences of Thread.Sleep() in async code. If Thread.Sleep is used in non async code, there's no obvious automatic fix for the code, so no codefix is proposed.
I've made a decision to treat "async void" code as asynchronous.
I think that the first analysis should be active by default.
I've updated version of Microsoft.CodeAnalysis in a separate commit - if I recall correctly, I needed some functionality from the newer version. I did not observe any regression.
I've also updated a version of Visual Studio in sln file in a separate commit. I guess that majority of people use the most recent version of VS2015 so I think that it's convenient to include that change in repo.
I don’t know whether contributors are listed somewhere. I added comments listing me as a contributor in my pull request, but feel free to remove them if you want.