diff --git a/docfx/analyzers/VSTHRD107.md b/docfx/analyzers/VSTHRD107.md index 8694cf2ea..3c4198d97 100644 --- a/docfx/analyzers/VSTHRD107.md +++ b/docfx/analyzers/VSTHRD107.md @@ -1,6 +1,6 @@ # VSTHRD107 Await Task within using expression -The C# `using` statement requires that the used expression implement `IDisposable`. +The C# `using` statement and `using` declaration require that the used expression or variable implement `IDisposable`. Because `Task` implements `IDisposable`, one may accidentally omit an `await` operator and `Dispose` of the `Task` instead of the `T` result itself when `T` derives from `IDisposable`. @@ -12,11 +12,18 @@ using (lck.EnterAsync()) { // ... } + +using (var releaser = lck.EnterAsync()) +{ + // ... +} + +using var releaser = lck.EnterAsync(); ``` ## Solution -Add the `await` operator within the `using` expression. +Add the `await` operator within the `using` expression or variable initializer. ```csharp AsyncSemaphore lck; @@ -24,4 +31,11 @@ using (await lck.EnterAsync()) { // ... } + +using (var releaser = await lck.EnterAsync()) +{ + // ... +} + +using var releaser = await lck.EnterAsync(); ``` diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzer.cs index e214b5057..5230f420b 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzer.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; @@ -47,29 +47,73 @@ public class VSTHRD107AwaitTaskWithinUsingExpressionAnalyzer : DiagnosticAnalyze /// public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Descriptor); + /// public override void Initialize(AnalysisContext context) { context.EnableConcurrentExecution(); context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.RegisterSyntaxNodeAction( - Utils.DebuggableWrapper(this.AnalyzeNode), + Utils.DebuggableWrapper(this.AnalyzeUsingStatement), SyntaxKind.UsingStatement); + context.RegisterSyntaxNodeAction( + Utils.DebuggableWrapper(this.AnalyzeLocalDeclaration), + SyntaxKind.LocalDeclarationStatement); } - private void AnalyzeNode(SyntaxNodeAnalysisContext context) + private void AnalyzeUsingStatement(SyntaxNodeAnalysisContext context) { var usingStatement = (UsingStatementSyntax)context.Node; - if (usingStatement.Expression is object) + + // In C# 8+, an asynchronous using statement (`await using (...)`) disposes the target + // asynchronously via IAsyncDisposable. This analyzer only flags unawaited tasks in synchronous + // using statements that dispose via IDisposable. + if (usingStatement.AwaitKeyword.IsKind(SyntaxKind.AwaitKeyword)) + { + return; + } + + if (usingStatement.Expression is not null) + { + this.CheckAndReport(context, usingStatement.Expression); + } + else if (usingStatement.Declaration is not null) + { + this.CheckAndReport(context, usingStatement.Declaration); + } + } + + private void AnalyzeLocalDeclaration(SyntaxNodeAnalysisContext context) + { + var localDeclaration = (LocalDeclarationStatementSyntax)context.Node; + + // Check synchronous `using var x = ...;` declarations. + // Skip `await using var x = ...;` because it targets IAsyncDisposable rather than IDisposable. + if (localDeclaration.UsingKeyword.IsKind(SyntaxKind.UsingKeyword) && + !localDeclaration.AwaitKeyword.IsKind(SyntaxKind.AwaitKeyword)) + { + this.CheckAndReport(context, localDeclaration.Declaration); + } + } + + private void CheckAndReport(SyntaxNodeAnalysisContext context, VariableDeclarationSyntax variableDeclaration) + { + foreach (VariableDeclaratorSyntax variable in variableDeclaration.Variables) { - TypeInfo expressionTypeInfo = context.SemanticModel.GetTypeInfo(usingStatement.Expression, context.CancellationToken); - ITypeSymbol? expressionType = expressionTypeInfo.Type; - if (expressionType?.Name == nameof(Task) && - expressionType.BelongsToNamespace(Namespaces.SystemThreadingTasks)) + if (variable.Initializer?.Value is { } initValue) { - context.ReportDiagnostic( - Diagnostic.Create(Descriptor, usingStatement.Expression.GetLocation())); + this.CheckAndReport(context, initValue); } } } + + private void CheckAndReport(SyntaxNodeAnalysisContext context, ExpressionSyntax expression) + { + TypeInfo expressionTypeInfo = context.SemanticModel.GetTypeInfo(expression, context.CancellationToken); + if (Utils.IsTask(expressionTypeInfo.Type)) + { + context.ReportDiagnostic( + Diagnostic.Create(Descriptor, expression.GetLocation())); + } + } } diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD107AwaitTaskWithinUsingExpressionCodeFix.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD107AwaitTaskWithinUsingExpressionCodeFix.cs index a399d0ec4..68ed0dd74 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD107AwaitTaskWithinUsingExpressionCodeFix.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD107AwaitTaskWithinUsingExpressionCodeFix.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; @@ -56,25 +56,32 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) { Document? document = context.Document; SyntaxNode? root = await document.GetSyntaxRootOrThrowAsync(ct).ConfigureAwait(false); - MethodDeclarationSyntax method = root.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf() ?? throw new InvalidOperationException("Unable to find MethodDeclaration."); + MethodDeclarationSyntax? method = root.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf(); - (document, method, _) = await FixUtils.UpdateDocumentAsync( - document, - method, - m => - { - root = m.SyntaxTree.GetRoot(ct); - UsingStatementSyntax? usingStatement = root.FindNode(diagnostic.Location.SourceSpan).FirstAncestorOrSelf() ?? throw new InvalidOperationException("Unable to find using statement."); - AwaitExpressionSyntax awaitExpression = SyntaxFactory.AwaitExpression( - SyntaxFactory.ParenthesizedExpression(usingStatement.Expression!)); - UsingStatementSyntax modifiedUsingStatement = usingStatement.WithExpression(awaitExpression) - .WithAdditionalAnnotations(Simplifier.Annotation); - return m.ReplaceNode(usingStatement, modifiedUsingStatement); - }, - ct).ConfigureAwait(false); - (document, method) = await method.MakeMethodAsync(document, ct).ConfigureAwait(false); + if (method is not null) + { + (document, method, _) = await FixUtils.UpdateDocumentAsync( + document, + method, + m => + { + root = m.SyntaxTree.GetRoot(ct); + ExpressionSyntax expression = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true).FirstAncestorOrSelf() + ?? throw new InvalidOperationException("Unable to find expression."); + return (MethodDeclarationSyntax)ReplaceExpressionWithAwait(m, expression); + }, + ct).ConfigureAwait(false); + (document, method) = await method.MakeMethodAsync(document, ct).ConfigureAwait(false); - return document.Project.Solution; + return document.Project.Solution; + } + else + { + ExpressionSyntax expression = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true).FirstAncestorOrSelf() + ?? throw new InvalidOperationException("Unable to find expression."); + SyntaxNode newRoot = ReplaceExpressionWithAwait(root, expression); + return document.WithSyntaxRoot(newRoot).Project.Solution; + } }, "only action"), diagnostic); @@ -84,4 +91,30 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) /// public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + private static SyntaxNode ReplaceExpressionWithAwait(SyntaxNode syntaxRoot, ExpressionSyntax expression) + { + // We're moving trivia from the original expression to the overall await expression. + ExpressionSyntax expressionToAwait = expression.WithoutTrivia(); + + // Complex expressions should be parenthesized to ensure the `await` is applied to the whole expression. + if (expressionToAwait is not (InvocationExpressionSyntax or IdentifierNameSyntax or MemberAccessExpressionSyntax or ElementAccessExpressionSyntax)) + { + expressionToAwait = SyntaxFactory.ParenthesizedExpression(expressionToAwait); + } + + AwaitExpressionSyntax awaitExpression = SyntaxFactory.AwaitExpression(expressionToAwait) + .WithTriviaFrom(expression) + .WithAdditionalAnnotations(Simplifier.Annotation); + + StatementSyntax? statement = expression.FirstAncestorOrSelf(); + if (statement is not null) + { + StatementSyntax newStatement = statement.ReplaceNode(expression, awaitExpression) + .WithAdditionalAnnotations(Simplifier.Annotation); + return syntaxRoot.ReplaceNode(statement, newStatement); + } + + return syntaxRoot.ReplaceNode(expression, awaitExpression); + } } diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzerTests.cs index 2656c456b..0da7c71fa 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD107AwaitTaskWithinUsingExpressionAnalyzerTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using CSVerify = Microsoft.VisualStudio.Threading.Analyzers.Tests.CSharpCodeFixVerifier; @@ -246,4 +246,238 @@ void F() { DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(9, 16, 9, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } + + [Fact] + public async Task UsingStatementWithVarDeclarationInSyncMethod_GeneratesError() + { + var test = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + void F() { + AsyncSemaphore lck = null; + using (var releaser = lck.EnterAsync()) + { + } + } +} +"; + var withFix = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task FAsync() { + AsyncSemaphore lck = null; + using (var releaser = await lck.EnterAsync()) + { + } + } +} +"; + + DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(8, 31, 8, 47); + await CSVerify.VerifyCodeFixAsync(test, expected, withFix); + } + + [Fact] + public async Task UsingStatementWithVarDeclarationInAsyncMethod_GeneratesError() + { + var test = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using (var releaser = lck.EnterAsync()) + { + } + } +} +"; + var withFix = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using (var releaser = await lck.EnterAsync()) + { + } + } +} +"; + + DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(8, 31, 8, 47); + await CSVerify.VerifyCodeFixAsync(test, expected, withFix); + } + + [Fact] + public async Task UsingDeclarationInSyncMethod_GeneratesError() + { + var test = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + void F() { + AsyncSemaphore lck = null; + using var releaser = lck.EnterAsync(); + } +} +"; + var withFix = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task FAsync() { + AsyncSemaphore lck = null; + using var releaser = await lck.EnterAsync(); + } +} +"; + + DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(8, 30, 8, 46); + await CSVerify.VerifyCodeFixAsync(test, expected, withFix); + } + + [Fact] + public async Task UsingDeclarationInAsyncMethod_GeneratesError() + { + var test = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using var releaser = lck.EnterAsync(); + } +} +"; + var withFix = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using var releaser = await lck.EnterAsync(); + } +} +"; + + DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(8, 30, 8, 46); + await CSVerify.VerifyCodeFixAsync(test, expected, withFix); + } + + [Fact] + public async Task UsingDeclarationWithExplicitInterfaceType_GeneratesError() + { + var test = @" +using System; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using IDisposable releaser = lck.EnterAsync(); + } +} +"; + var withFix = @" +using System; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using IDisposable releaser = await lck.EnterAsync(); + } +} +"; + + DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(9, 38, 9, 54); + await CSVerify.VerifyCodeFixAsync(test, expected, withFix); + } + + [Fact] + public async Task UsingStatementWithExplicitInterfaceType_GeneratesError() + { + var test = @" +using System; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using (IDisposable releaser = lck.EnterAsync()) + { + } + } +} +"; + var withFix = @" +using System; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using (IDisposable releaser = await lck.EnterAsync()) + { + } + } +} +"; + + DiagnosticResult expected = CSVerify.Diagnostic().WithSpan(9, 39, 9, 55); + await CSVerify.VerifyCodeFixAsync(test, expected, withFix); + } + + [Fact] + public async Task UsingAwaitDeclaration_GeneratesNoError() + { + var test = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using var releaser = await lck.EnterAsync(); + } +} +"; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task UsingStatementWithAwaitDeclaration_GeneratesNoError() + { + var test = @" +using System.Threading.Tasks; +using Microsoft.VisualStudio.Threading; + +class Test { + async Task F() { + AsyncSemaphore lck = null; + using (var releaser = await lck.EnterAsync()) + { + } + } +} +"; + + await CSVerify.VerifyAnalyzerAsync(test); + } }