forked from microsoft/vs-threading
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add VSTHRD110: Observe result of async calls
Closes microsoft#233
- Loading branch information
Showing
20 changed files
with
448 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
# VSTHRD110 Observe result of async calls | ||
|
||
Tasks returned from async methods should be awaited, or assigned to a variable for observation later. | ||
Methods that return `Task`s often complete and report their work via the `Task` they return, and simply | ||
invoking the method does not guarantee that its work is complete nor successful. Using the `await` keyword | ||
just before the method call causes execution of the calling method to effectively suspend until the called | ||
method has completed and rethrows any exception thrown by the method. | ||
|
||
When a `Task` or `Task<T>` is returned and is not awaited or redirected in some other way, | ||
within the context of a synchronous method, a warning is reported. | ||
|
||
This rule does *not* apply to calls made within async methods, since [CS4014][CS4014] already reports these. | ||
|
||
## Examples of patterns that are flagged by this analyzer | ||
|
||
```csharp | ||
void Foo() { | ||
DoStuffAsync(); | ||
} | ||
|
||
async Task DoStuffAsync() { /* ... */ } | ||
``` | ||
|
||
## Solution | ||
|
||
Convert the method to be async and await the expression: | ||
|
||
```csharp | ||
async Task FooAsync() { | ||
await DoStuffAsync(); | ||
} | ||
|
||
async Task DoStuffAsync() { /* ... */ } | ||
``` | ||
|
||
When the calling method's signature cannot be changed, wrap the method body in a `JoinableTaskFactory.Run` delegate instead: | ||
|
||
```csharp | ||
void Foo() { | ||
jtf.Run(async delegate { | ||
await DoStuffAsync(); | ||
}); | ||
} | ||
|
||
async Task DoStuffAsync() { /* ... */ } | ||
``` | ||
|
||
One other option is to assign the result of the method call to a field or local variable, presumably to track it later: | ||
|
||
```csharp | ||
void Foo() { | ||
Task watchThis = DoStuffAsync(); | ||
} | ||
|
||
async Task DoStuffAsync() { /* ... */ } | ||
``` | ||
|
||
When tracking the `Task` with a field, remember that to await it later without risk of deadlocking, | ||
wrap it in a `JoinableTask` using `JoinableTaskFactory.RunAsync`, per [the 3rd rule](../threading_rules.md#Rule3). | ||
|
||
```csharp | ||
JoinableTask watchThis; | ||
|
||
void Foo() { | ||
this.watchThis = jtf.RunAsync(() => DoStuffAsync()); | ||
} | ||
|
||
async Task WaitForFooToFinishAsync() { | ||
await this.watchThis; | ||
} | ||
|
||
async Task DoStuffAsync() { /* ... */ } | ||
``` | ||
|
||
[CS4014]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs4014 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
179 changes: 179 additions & 0 deletions
179
...VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,179 @@ | ||
namespace Microsoft.VisualStudio.Threading.Analyzers.Tests | ||
{ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using CodeAnalysis.Diagnostics; | ||
using Microsoft.CodeAnalysis; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
||
public class VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests : DiagnosticVerifier | ||
{ | ||
private DiagnosticResult expect = new DiagnosticResult | ||
{ | ||
Id = VSTHRD110ObserveResultOfAsyncCallsAnalyzer.Id, | ||
Severity = DiagnosticSeverity.Warning, | ||
}; | ||
|
||
public VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests(ITestOutputHelper logger) | ||
: base(logger) | ||
{ | ||
} | ||
|
||
protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new VSTHRD110ObserveResultOfAsyncCallsAnalyzer(); | ||
|
||
[Fact] | ||
public void SyncMethod_ProducesDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
class Test { | ||
void Foo() | ||
{ | ||
BarAsync(); | ||
} | ||
Task BarAsync() => null; | ||
} | ||
"; | ||
|
||
this.expect = this.CreateDiagnostic(7, 9, 8); | ||
this.VerifyCSharpDiagnostic(test, this.expect); | ||
} | ||
|
||
[Fact] | ||
public void SyncDelegateWithinAsyncMethod_ProducesDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
class Test { | ||
async Task Foo() | ||
{ | ||
await Task.Run(delegate { | ||
BarAsync(); | ||
}); | ||
} | ||
Task BarAsync() => null; | ||
} | ||
"; | ||
|
||
this.expect = this.CreateDiagnostic(8, 13, 8); | ||
this.VerifyCSharpDiagnostic(test, this.expect); | ||
} | ||
|
||
[Fact] | ||
public void AssignToLocal_ProducesNoDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
class Test { | ||
void Foo() | ||
{ | ||
Task t = BarAsync(); | ||
} | ||
Task BarAsync() => null; | ||
} | ||
"; | ||
|
||
this.VerifyCSharpDiagnostic(test); | ||
} | ||
|
||
[Fact] | ||
public void ForgetExtension_ProducesNoDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
using Microsoft.VisualStudio.Threading; | ||
class Test { | ||
void Foo() | ||
{ | ||
BarAsync().Forget(); | ||
} | ||
Task BarAsync() => null; | ||
} | ||
"; | ||
|
||
this.VerifyCSharpDiagnostic(test); | ||
} | ||
|
||
[Fact] | ||
public void AssignToField_ProducesNoDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
class Test { | ||
Task t; | ||
void Foo() | ||
{ | ||
this.t = BarAsync(); | ||
} | ||
Task BarAsync() => null; | ||
} | ||
"; | ||
|
||
this.VerifyCSharpDiagnostic(test); | ||
} | ||
|
||
[Fact] | ||
public void PassToOtherMethod_ProducesNoDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
class Test { | ||
void Foo() | ||
{ | ||
OtherMethod(BarAsync()); | ||
} | ||
Task BarAsync() => null; | ||
void OtherMethod(Task t) { } | ||
} | ||
"; | ||
|
||
this.VerifyCSharpDiagnostic(test); | ||
} | ||
|
||
[Fact] | ||
public void AsyncMethod_ProducesNoDiagnostic() | ||
{ | ||
var test = @" | ||
using System.Threading.Tasks; | ||
class Test { | ||
async Task FooAsync() | ||
{ | ||
BarAsync(); | ||
} | ||
Task BarAsync() => null; | ||
} | ||
"; | ||
|
||
this.VerifyCSharpDiagnostic(test); // CS4014 should already take care of this case. | ||
} | ||
|
||
private DiagnosticResult CreateDiagnostic(int line, int column, int length, string messagePattern = null) => | ||
new DiagnosticResult | ||
{ | ||
Id = this.expect.Id, | ||
MessagePattern = messagePattern ?? this.expect.MessagePattern, | ||
Severity = this.expect.Severity, | ||
Locations = new[] { new DiagnosticResultLocation("Test0.cs", line, column, line, column + length) }, | ||
}; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.