diff --git a/analyzers/its/expected/BlazorSample/S6966-BlazorSample-net7.0.json b/analyzers/its/expected/BlazorSample/S6966-BlazorSample-net7.0.json new file mode 100644 index 00000000000..adc4239634e --- /dev/null +++ b/analyzers/its/expected/BlazorSample/S6966-BlazorSample-net7.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await RunAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/BlazorSample/BlazorSample/Program.cs#L29", + "Location": "Line 29 Position 1-10" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/CSharpLatest/S6966-NetCore31.MVC.ConfigurableRules-netcoreapp3.1.Views.json b/analyzers/its/expected/CSharpLatest/S6966-NetCore31.MVC.ConfigurableRules-netcoreapp3.1.Views.json new file mode 100644 index 00000000000..a78adad83c1 --- /dev/null +++ b/analyzers/its/expected/CSharpLatest/S6966-NetCore31.MVC.ConfigurableRules-netcoreapp3.1.Views.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await RenderSectionAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/CSharpLatest/NetCore31WithConfigurableRules/Views/Shared/_Layout.cshtml#L46", + "Location": "Line 46 Position 7-48" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore6-net6.0.json b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore6-net6.0.json new file mode 100644 index 00000000000..b10b3a5fa62 --- /dev/null +++ b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore6-net6.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await RunAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/AspNetCore6/Program.cs#L27", + "Location": "Line 27 Position 1-10" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore7-net7.0.json b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore7-net7.0.json new file mode 100644 index 00000000000..6b27d03caf0 --- /dev/null +++ b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore7-net7.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await RunAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/AspNetCore7/Program.cs#L27", + "Location": "Line 27 Position 1-10" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore8-net8.0.json b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore8-net8.0.json new file mode 100644 index 00000000000..3f5538e5205 --- /dev/null +++ b/analyzers/its/expected/ManuallyAddedNoncompliantIssues.CS/S6966-AspNetCore8-net8.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await RunAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/AspNetCore8/Program.cs#L27", + "Location": "Line 27 Position 1-10" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/RazorSample/S6966-RazorSample-net7.0.json b/analyzers/its/expected/RazorSample/S6966-RazorSample-net7.0.json new file mode 100644 index 00000000000..d19cf47db78 --- /dev/null +++ b/analyzers/its/expected/RazorSample/S6966-RazorSample-net7.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await RunAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/RazorSample/RazorSample/Program.cs#L25", + "Location": "Line 25 Position 1-10" + } + ] +} \ No newline at end of file diff --git a/analyzers/its/expected/akka.net/S6966-Akka.Cluster.Sharding-netstandard2.0.json b/analyzers/its/expected/akka.net/S6966-Akka.Cluster.Sharding-netstandard2.0.json new file mode 100644 index 00000000000..4364c712392 --- /dev/null +++ b/analyzers/its/expected/akka.net/S6966-Akka.Cluster.Sharding-netstandard2.0.json @@ -0,0 +1,10 @@ +{ + "Issues": [ + { + "Id": "S6966", + "Message": "Await StartProxyAsync instead.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/contrib/cluster/Akka.Cluster.Sharding/ClusterSharding.cs#L742", + "Location": "Line 742 Position 24-92" + } + ] +} \ No newline at end of file diff --git a/analyzers/rspec/cs/S6966.html b/analyzers/rspec/cs/S6966.html new file mode 100644 index 00000000000..99eaf88ba54 --- /dev/null +++ b/analyzers/rspec/cs/S6966.html @@ -0,0 +1,70 @@ +

In an async method, any blocking operations should be avoided.

+

Why is this an issue?

+

Using a synchronous method instead of its asynchronous +counterpart in an async method blocks the execution and is considered bad practice for several reasons:

+
+
+ Resource Utilization +
+
+

Each thread consumes system resources, such as memory. When a thread is blocked, it’s not doing any useful work, but it’s still consuming these + resources. This can lead to inefficient use of system resources.

+
+
+ Scalability +
+
+

Blocking threads can limit the scalability of your application. In a high-load scenario where many operations are happening concurrently, each + blocked thread represents a missed opportunity to do useful work. This can prevent your application from effectively handling increased load.

+
+
+ Performance +
+
+

Blocking threads can degrade the performance of your application. If all threads in the thread pool become blocked, new tasks can’t start + executing until an existing task completes and frees up a thread. This can lead to delays and poor responsiveness.

+
+
+

Instead of blocking, it’s recommended to use the async operator with async methods. This +allows the system to release the current thread back to the thread pool until the awaited task is complete, improving scalability and +responsiveness.

+

How to fix it

+

Code examples

+

Noncompliant code example

+
+public async Task Examples(Stream stream, DbSet<Person> dbSet)
+{
+    stream.Read(array, 0, 1024);            // Noncompliant
+    File.ReadAllLines("path");              // Noncompliant
+    dbSet.ToList();                         // Noncompliant in Entity Framework Core queries
+    dbSet.FirstOrDefault(x => x.Age >= 18); // Noncompliant in Entity Framework Core queries
+}
+
+

Compliant solution

+
+public async Task Examples(Stream stream, DbSet<Person> dbSet)
+{
+    await stream.ReadAsync(array, 0, 1024);
+    await File.ReadAllLinesAsync("path");
+    await dbSet.ToListAsync();
+    await dbSet.FirstOrDefaultAsync(x => x.Age >= 18);
+}
+
+

Resources

+

Documentation

+ +

Articles & blog posts

+ + diff --git a/analyzers/rspec/cs/S6966.json b/analyzers/rspec/cs/S6966.json new file mode 100644 index 00000000000..02f3331eb45 --- /dev/null +++ b/analyzers/rspec/cs/S6966.json @@ -0,0 +1,23 @@ +{ + "title": "Awaitable method should be used", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "async-await" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-6966", + "sqKey": "S6966", + "scope": "All", + "quickfix": "targeted", + "code": { + "impacts": { + "RELIABILITY": "MEDIUM" + }, + "attribute": "COMPLETE" + } +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index 5006acc1f55..c2b1e24c7a2 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -317,6 +317,6 @@ "S6930", "S6931", "S6934", - "S6961" + "S6966" ] } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/ExpressionSyntaxExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/ExpressionSyntaxExtensions.cs index 2f75c0e0210..52fed73eedb 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/ExpressionSyntaxExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/ExpressionSyntaxExtensions.cs @@ -97,5 +97,20 @@ public static bool TryGetExpressionComparedToNull(this ExpressionSyntax expressi return false; } + + /// + /// Returns the expression, representing the left side of the dot. This is useful for finding the expression of an invoked expression.
+ /// For the expression of the invocation M() in the expression this.A.B.M() the member access this.A.B is returned and
+ /// for this.A?.B?.M() the member binding .B is returned. + /// + /// The expression to start the search of. Should be an MemberAccess or a MemberBinding. + /// The expression left of the dot or question mark dot. + public static ExpressionSyntax GetLeftOfDot(this ExpressionSyntax expression) => + expression switch + { + MemberAccessExpressionSyntax memberAccessExpression => memberAccessExpression.Expression, + MemberBindingExpressionSyntax memberBindingExpression => memberBindingExpression.GetParentConditionalAccessExpression()?.Expression, + _ => null, + }; } } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs index 1bbff8b46c3..f262ca1d14e 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs @@ -31,18 +31,21 @@ public static partial class SyntaxNodeExtensions SyntaxKind.AddAccessorDeclaration, SyntaxKind.AnonymousMethodExpression, SyntaxKind.BaseConstructorInitializer, + SyntaxKind.CompilationUnit, SyntaxKind.ConstructorDeclaration, SyntaxKind.ConversionOperatorDeclaration, SyntaxKind.DestructorDeclaration, - SyntaxKind.EqualsValueClause, + SyntaxKind.EnumMemberDeclaration, + SyntaxKind.FieldDeclaration, SyntaxKind.GetAccessorDeclaration, - SyntaxKind.GlobalStatement, SyntaxKindEx.InitAccessorDeclaration, SyntaxKindEx.LocalFunctionStatement, SyntaxKind.MethodDeclaration, SyntaxKind.OperatorDeclaration, + SyntaxKind.Parameter, SyntaxKind.ParenthesizedLambdaExpression, SyntaxKindEx.PrimaryConstructorBaseType, + SyntaxKind.PropertyDeclaration, SyntaxKind.RemoveAccessorDeclaration, SyntaxKind.SetAccessorDeclaration, SyntaxKind.SimpleLambdaExpression, diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs new file mode 100644 index 00000000000..4a8c48dd9cb --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs @@ -0,0 +1,180 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2024 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Shared.Extensions; +using WellKnownExtensionMethodContainer = SonarAnalyzer.Common.MultiValueDictionary; +namespace SonarAnalyzer.Rules.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class UseAwaitableMethod : SonarDiagnosticAnalyzer +{ + private const string DiagnosticId = "S6966"; + private const string MessageFormat = "Await {0} instead."; + + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterCompilationStartAction(compilationStart => + { + // Not every async method is defined in the same class/interface as its non-async counterpart. + // For example the EntityFrameworkQueryableExtensions.AnyAsync() method provides an async version of the Enumerable.Any() method for IQueryable types. + // WellKnownExtensionMethodContainer stores where to look for the async versions of certain methods from a type, e.g. async versions of methods from + // System.Linq.Enumerable can be found in Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions. + var wellKnownExtensionMethodContainer = BuildWellKnownExtensionMethodContainers(compilationStart.Compilation); + context.RegisterCodeBlockStartAction(CSharpGeneratedCodeRecognizer.Instance, codeBlockStart => + { + if (IsAsyncCodeBlock(codeBlockStart.CodeBlock)) + { + codeBlockStart.RegisterNodeAction(nodeContext => + { + var invocationExpression = (InvocationExpressionSyntax)nodeContext.Node; + + var awaitableAlternatives = FindAwaitableAlternatives(wellKnownExtensionMethodContainer, codeBlockStart.CodeBlock, invocationExpression, + nodeContext.SemanticModel, nodeContext.ContainingSymbol, nodeContext.Cancel); + if (awaitableAlternatives.FirstOrDefault() is { Name: { } alternative }) + { + nodeContext.ReportIssue(Rule, invocationExpression, alternative); + } + }, SyntaxKind.InvocationExpression); + } + }); + }); + + private static WellKnownExtensionMethodContainer BuildWellKnownExtensionMethodContainers(Compilation compilation) + { + var wellKnownExtensionMethodContainer = new WellKnownExtensionMethodContainer(); + var queryable = compilation.GetTypeByMetadataName(KnownType.System_Linq_Queryable); + var enumerable = compilation.GetTypeByMetadataName(KnownType.System_Linq_Enumerable); + if (queryable is not null && enumerable is not null) + { + if (compilation.GetTypeByMetadataName(KnownType.Microsoft_EntityFrameworkCore_EntityFrameworkQueryableExtensions) is { } entityFrameworkQueryableExtensions) + { + wellKnownExtensionMethodContainer.Add(queryable, entityFrameworkQueryableExtensions); + wellKnownExtensionMethodContainer.Add(enumerable, entityFrameworkQueryableExtensions); + } + if (compilation.GetTypeByMetadataName(KnownType.Microsoft_EntityFrameworkCore_RelationalQueryableExtensions) is { } relationalQueryableExtensions) + { + wellKnownExtensionMethodContainer.Add(queryable, relationalQueryableExtensions); + wellKnownExtensionMethodContainer.Add(enumerable, relationalQueryableExtensions); + } + } + if (compilation.GetTypeByMetadataName(KnownType.System_Net_Sockets_Socket) is { } socket + && compilation.GetTypeByMetadataName(KnownType.System_Net_Sockets_SocketTaskExtensions) is { } socketTaskExtensions) + { + wellKnownExtensionMethodContainer.Add(socket, socketTaskExtensions); + } + return wellKnownExtensionMethodContainer; + } + + private static ImmutableArray FindAwaitableAlternatives(WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer, SyntaxNode codeBlock, + InvocationExpressionSyntax invocationExpression, SemanticModel semanticModel, ISymbol containingSymbol, CancellationToken cancel) + { + var awaitableRoot = GetAwaitableRootOfInvocation(invocationExpression); + if (awaitableRoot is { Parent: AwaitExpressionSyntax }) + { + return ImmutableArray.Empty; // Invocation result is already awaited. + } + if (invocationExpression.EnclosingScope() is { } scope && !IsAsyncCodeBlock(scope)) + { + return ImmutableArray.Empty; // Not in an async scope + } + if (semanticModel.GetSymbolInfo(invocationExpression, cancel).Symbol is IMethodSymbol methodSymbol + && !methodSymbol.IsAwaitableNonDynamic(semanticModel, invocationExpression.SpanStart)) // The invoked method returns something awaitable (but it isn't awaited). + { + // Perf: Before doing (expensive) speculative re-binding in SpeculativeBindCandidates, we check if there is an "..Async()" alternative in scope. + var invokedType = invocationExpression.Expression.GetLeftOfDot() is { } expression && semanticModel.GetTypeInfo(expression) is { Type: { } type } + ? type // A dotted expression: Lookup the type, left of the dot (this may be different from methodSymbol.ContainingType) + : containingSymbol.ContainingType; // If not dotted, than the scope is the current type. Local function support is missing here. + var members = GetMethodSymbolsInScope($"{methodSymbol.Name}Async", wellKnownExtensionMethodContainer, invokedType, methodSymbol.ContainingType); + var awaitableCandidates = members.Where(x => x.IsAwaitableNonDynamic(semanticModel, invocationExpression.SpanStart)); + var awaitableAlternatives = SpeculativeBindCandidates(semanticModel, codeBlock, awaitableRoot, invocationExpression, awaitableCandidates).ToImmutableArray(); + return awaitableAlternatives; + } + return ImmutableArray.Empty; + } + + private static IEnumerable GetMethodSymbolsInScope(string methodName, WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer, + ITypeSymbol invokedType, ITypeSymbol methodContainer) => + ((ITypeSymbol[])[.. invokedType.GetSelfAndBaseTypes(), .. WellKnownExtensionMethodContainer(wellKnownExtensionMethodContainer, methodContainer), methodContainer]) + .Distinct() + .SelectMany(x => x.GetMembers(methodName)) + .OfType(); + + private static IEnumerable WellKnownExtensionMethodContainer(WellKnownExtensionMethodContainer lookup, ITypeSymbol invokedType) => + lookup.TryGetValue(invokedType, out var extensionMethodContainer) + ? extensionMethodContainer + : []; + + private static IEnumerable SpeculativeBindCandidates(SemanticModel semanticModel, SyntaxNode codeBlock, SyntaxNode awaitableRoot, + InvocationExpressionSyntax invocationExpression, IEnumerable awaitableCandidates) => + awaitableCandidates.Where(x => SpeculativeBindCandidate(semanticModel, x, codeBlock, awaitableRoot, invocationExpression)); + + private static bool SpeculativeBindCandidate(SemanticModel semanticModel, IMethodSymbol candidate, SyntaxNode codeBlock, SyntaxNode awaitableRoot, InvocationExpressionSyntax invocationExpression) + { + var root = codeBlock.SyntaxTree.GetRoot(); + var invocationIdentifierName = invocationExpression.GetMethodCallIdentifier()?.Parent as IdentifierNameSyntax; + + var invocationAnnotation = new SyntaxAnnotation(); + var replace = root.ReplaceNodes([awaitableRoot, invocationIdentifierName, invocationExpression], (original, newNode) => + { + var result = newNode; + if (original == invocationIdentifierName) + { + result = SyntaxFactory.IdentifierName(candidate.Name).WithTriviaFrom(invocationIdentifierName); + } + if (original == invocationExpression) + { + result = result.WithAdditionalAnnotations(invocationAnnotation); + } + if (original == awaitableRoot && result is ExpressionSyntax resultExpression) + { + result = SyntaxFactory.AwaitExpression(resultExpression); + } + return result; + }); + var invocationReplaced = replace.GetAnnotatedNodes(invocationAnnotation).First(); + var speculativeSymbolInfo = semanticModel.GetSpeculativeSymbolInfo(invocationReplaced.SpanStart, invocationReplaced, SpeculativeBindingOption.BindAsExpression); + var speculativeSymbol = speculativeSymbolInfo.Symbol as IMethodSymbol; + return candidate.Equals(speculativeSymbol) || candidate.Equals(speculativeSymbol?.ReducedFrom); + } + + private static ExpressionSyntax GetAwaitableRootOfInvocation(ExpressionSyntax expression) => + expression switch + { + { Parent: ConditionalAccessExpressionSyntax conditional } => conditional.GetRootConditionalAccessExpression(), + { Parent: MemberAccessExpressionSyntax memberAccess } => memberAccess.GetRootConditionalAccessExpression() ?? GetAwaitableRootOfInvocation(memberAccess), + { Parent: PostfixUnaryExpressionSyntax { RawKind: (int)SyntaxKindEx.SuppressNullableWarningExpression } parent } => GetAwaitableRootOfInvocation(parent), + { Parent: ParenthesizedExpressionSyntax parent } => GetAwaitableRootOfInvocation(parent), + { } self => self, + }; + + private static bool IsAsyncCodeBlock(SyntaxNode codeBlock) => + codeBlock switch + { + CompilationUnitSyntax => true, + BaseMethodDeclarationSyntax { Modifiers: { } modifiers } => modifiers.AnyOfKind(SyntaxKind.AsyncKeyword), + AnonymousFunctionExpressionSyntax { AsyncKeyword: { } asyncKeyword } => asyncKeyword.IsKind(SyntaxKind.AsyncKeyword), + var localFunction when LocalFunctionStatementSyntaxWrapper.IsInstance(localFunction) => ((LocalFunctionStatementSyntaxWrapper)localFunction).Modifiers.AnyOfKind(SyntaxKind.AsyncKeyword), + _ => false, + }; +} diff --git a/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.Roslyn.cs b/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.Roslyn.cs new file mode 100644 index 00000000000..b49f9001cd8 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.Common/Extensions/ISymbolExtensions.Roslyn.cs @@ -0,0 +1,83 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#nullable enable + +using System.Diagnostics.CodeAnalysis; + +namespace Microsoft.CodeAnalysis.Shared.Extensions; + +// Copied from https://github.com/dotnet/roslyn/blob/ca66296efa86bd8078508fe7b38b91b415364f78/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs +[ExcludeFromCodeCoverage] +public static class ISymbolExtensions +{ + /// + /// If the is a method symbol, returns if the method's return type is "awaitable", but not if it's . + /// If the is a type symbol, returns if that type is "awaitable". + /// An "awaitable" is any type that exposes a GetAwaiter method which returns a valid "awaiter". This GetAwaiter method may be an instance method or an extension method. + /// + /// + /// Copied from . + /// + public static bool IsAwaitableNonDynamic(this ISymbol? symbol, SemanticModel semanticModel, int position) + { + var methodSymbol = symbol as IMethodSymbol; + ITypeSymbol? typeSymbol = null; + + if (methodSymbol == null) + { + typeSymbol = symbol as ITypeSymbol; + if (typeSymbol == null) + { + return false; + } + } + else + { + if (methodSymbol.ReturnType == null) + { + return false; + } + } + + // otherwise: needs valid GetAwaiter + var potentialGetAwaiters = semanticModel.LookupSymbols(position, + container: typeSymbol ?? methodSymbol!.ReturnType.OriginalDefinition, + name: WellKnownMemberNames.GetAwaiter, + includeReducedExtensionMethods: true); + var getAwaiters = potentialGetAwaiters.OfType().Where(x => !x.Parameters.Any()); + return getAwaiters.Any(VerifyGetAwaiter); + } + + // Copied from https://github.com/dotnet/roslyn/blob/ca66296efa86bd8078508fe7b38b91b415364f78/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs#L611 + private static bool VerifyGetAwaiter(IMethodSymbol getAwaiter) + { + var returnType = getAwaiter.ReturnType; + if (returnType == null) + { + return false; + } + + // bool IsCompleted { get } + if (!returnType.GetMembers().OfType().Any(p => p.Name == WellKnownMemberNames.IsCompleted && p.Type.SpecialType == SpecialType.System_Boolean && p.GetMethod != null)) + { + return false; + } + + var methods = returnType.GetMembers().OfType(); + + // NOTE: (vladres) The current version of C# Spec, §7.7.7.3 'Runtime evaluation of await expressions', requires that + // NOTE: the interface method INotifyCompletion.OnCompleted or ICriticalNotifyCompletion.UnsafeOnCompleted is invoked + // NOTE: (rather than any OnCompleted method conforming to a certain pattern). + // NOTE: Should this code be updated to match the spec? + + // void OnCompleted(Action) + // Actions are delegates, so we'll just check for delegates. + if (!methods.Any(x => x.Name == WellKnownMemberNames.OnCompleted && x.ReturnsVoid && x.Parameters is { Length: 1 } parameter && parameter[0] is { Type.TypeKind: TypeKind.Delegate })) + return false; + + // void GetResult() || T GetResult() + return methods.Any(m => m.Name == WellKnownMemberNames.GetResult && !m.Parameters.Any()); + } +} diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs index 80ee19193ec..6ec98cf264e 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs @@ -106,6 +106,7 @@ public sealed partial class KnownType public static readonly KnownType Microsoft_EntityFrameworkCore_DbContext = new("Microsoft.EntityFrameworkCore.DbContext"); public static readonly KnownType Microsoft_EntityFrameworkCore_DbContextOptionsBuilder = new("Microsoft.EntityFrameworkCore.DbContextOptionsBuilder"); public static readonly KnownType Microsoft_EntityFrameworkCore_DbSet_TEntity = new("Microsoft.EntityFrameworkCore.DbSet", "TEntity"); + public static readonly KnownType Microsoft_EntityFrameworkCore_EntityFrameworkQueryableExtensions = new("Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions"); public static readonly KnownType Microsoft_EntityFrameworkCore_Migrations_Migration = new("Microsoft.EntityFrameworkCore.Migrations.Migration"); public static readonly KnownType Microsoft_EntityFrameworkCore_MySQLDbContextOptionsExtensions = new("Microsoft.EntityFrameworkCore.MySQLDbContextOptionsExtensions"); public static readonly KnownType Microsoft_EntityFrameworkCore_NpgsqlDbContextOptionsExtensions = new("Microsoft.EntityFrameworkCore.NpgsqlDbContextOptionsExtensions"); @@ -391,6 +392,7 @@ public sealed partial class KnownType public static readonly KnownType System_Net_Security_SslPolicyErrors = new("System.Net.Security.SslPolicyErrors"); public static readonly KnownType System_Net_SecurityProtocolType = new("System.Net.SecurityProtocolType"); public static readonly KnownType System_Net_Sockets_Socket = new("System.Net.Sockets.Socket"); + public static readonly KnownType System_Net_Sockets_SocketTaskExtensions = new("System.Net.Sockets.SocketTaskExtensions"); public static readonly KnownType System_Net_Sockets_TcpClient = new("System.Net.Sockets.TcpClient"); public static readonly KnownType System_Net_Sockets_UdpClient = new("System.Net.Sockets.UdpClient"); public static readonly KnownType System_Net_WebClient = new("System.Net.WebClient"); diff --git a/analyzers/tests/SonarAnalyzer.Test/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.Test/Extensions/SyntaxNodeExtensionsTest.cs index 27f024af618..6e11c9860d8 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Extensions/SyntaxNodeExtensionsTest.cs @@ -690,7 +690,7 @@ public X M() } } """; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); var parentConditional = ExtensionsCS.GetParentConditionalAccessExpression(node); parentConditional.ToString().Should().Be(parent); parentConditional.Expression.ToString().Should().Be(parentExpression); @@ -726,7 +726,7 @@ Return Nothing End Function End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic); var parentConditional = ExtensionsVB.GetParentConditionalAccessExpression(node); parentConditional.ToString().Should().Be(parent); parentConditional.Expression.ToString().Should().Be(parentExpression); @@ -758,7 +758,7 @@ public X M() }} }} "; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); var parentConditional = ExtensionsCS.GetRootConditionalAccessExpression(node); parentConditional.ToString().Should().Be(expression.Replace("$$", string.Empty)); } @@ -789,7 +789,7 @@ public void M(int p) { } } """; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); var argumentList = ExtensionsCS.ArgumentList(node).Arguments; var argument = argumentList.Should().ContainSingle().Which; (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); @@ -810,7 +810,7 @@ public C(int p): base(p) { } } """; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); var argumentList = ExtensionsCS.ArgumentList(node).Arguments; var argument = argumentList.Should().ContainSingle().Which; (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); @@ -823,7 +823,7 @@ public void ArgumentList_CS_PrimaryConstructorBaseType() public class Base(int p); public class Derived(int p): $$Base(1)$$; """; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); var argumentList = ExtensionsCS.ArgumentList(node).Arguments; var argument = argumentList.Should().ContainSingle().Which; (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); @@ -840,7 +840,7 @@ public void M() { } } """; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); ExtensionsCS.ArgumentList(node).Should().BeNull(); } @@ -860,7 +860,7 @@ public void M() { } } """; - var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.CSharp); var sut = () => ExtensionsCS.ArgumentList(node); sut.Should().Throw(); } @@ -887,7 +887,7 @@ Public Sub M(p As Integer) End Sub End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); var argument = argumentList.Arguments.Should().ContainSingle().Which; (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); @@ -904,7 +904,7 @@ Public Sub M() End Sub End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); argumentList.Arguments.Should().SatisfyRespectively( a => (a.GetExpression() is SyntaxVB.IdentifierNameSyntax { Identifier.ValueText: "s" }).Should().BeTrue(), @@ -919,7 +919,7 @@ public void ArgumentList_VB_Attribute() Public Class C End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); var argument = argumentList.Arguments.Should().ContainSingle().Which; (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); @@ -939,7 +939,7 @@ Dim arr(0) As Integer End Sub End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); var argument = argumentList.Arguments.Should().ContainSingle().Which; (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); @@ -955,7 +955,7 @@ Public Sub M() End Sub End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: false); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic, getInnermostNodeForTie: false); ExtensionsVB.ArgumentList(node).Should().BeNull(); } @@ -974,7 +974,7 @@ Public Sub M() End Sub End Class """; - var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var node = NodeBetweenMarkers(code, AnalyzerLanguage.VisualBasic, getInnermostNodeForTie: true); var sut = () => ExtensionsVB.ArgumentList(node); sut.Should().Throw(); } @@ -992,7 +992,7 @@ public class C { $${{declarations}}$$ } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.ParameterList(node); actual.Should().NotBeNull(); var entry = actual.Parameters.Should().ContainSingle().Which; @@ -1016,7 +1016,7 @@ public void M() {{declarations}} } } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.ParameterList(node); actual.Should().NotBeNull(); var entry = actual.Parameters.Should().ContainSingle().Which; @@ -1031,7 +1031,7 @@ public class C { $$~C() { }$$ } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.ParameterList(node); actual.Should().NotBeNull(); actual.Parameters.Should().BeEmpty(); @@ -1054,7 +1054,7 @@ public void ParameterList_PrimaryConstructors(string type) { }$$ - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.ParameterList(node); actual.Should().NotBeNull(); var entry = actual.Parameters.Should().ContainSingle().Which; @@ -1072,7 +1072,7 @@ public class C { {{declaration}} } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.ParameterList(node); actual.Should().BeNull(); } @@ -1141,7 +1141,7 @@ unsafe class Test public Test(int i) { } {{member}} } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.GetIdentifier(node); if (expected is null) { @@ -1163,7 +1163,7 @@ public void GetIdentifier_CompilationUnit(string member, string expected) { var node = NodeBetweenMarkers($$""" {{member}} - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.GetIdentifier(node); if (expected is null) { @@ -1187,7 +1187,7 @@ public class Base(int i) { } public class Derived(int i) {{baseType}} { } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.GetIdentifier(node); if (expected is null) { @@ -1207,8 +1207,8 @@ public class Derived(int i) {{baseType}} { } [DataRow("""Derived() { $$var x = 42;$$ }""", SyntaxKind.ConstructorDeclaration)] [DataRow("""public static implicit operator int(Derived d) => $$42$$;""", SyntaxKind.ConversionOperatorDeclaration)] [DataRow("""~Derived() { $$var x = 42;$$ }""", SyntaxKind.DestructorDeclaration)] - [DataRow("""int field = $$int.Parse("42")$$;""", SyntaxKind.EqualsValueClause)] - [DataRow("""int Property { get; set; } = $$int.Parse("42")$$;""", SyntaxKind.EqualsValueClause)] + [DataRow("""int field = $$int.Parse("42")$$;""", SyntaxKind.FieldDeclaration)] + [DataRow("""int Property { get; set; } = $$int.Parse("42")$$;""", SyntaxKind.PropertyDeclaration)] [DataRow("""int Property { set { $$_ = value;$$ } }""", SyntaxKind.SetAccessorDeclaration)] [DataRow("""int Property { set { $$_ = value;$$ } }""", SyntaxKind.SetAccessorDeclaration)] [DataRow("""int Method() { return LocalFunction(); int LocalFunction() { $$return 42;$$ } }""", SyntaxKindEx.LocalFunctionStatement)] @@ -1220,6 +1220,8 @@ public class Derived(int i) {{baseType}} { } [DataRow("""int Method() { Func lambda = x => $$x + 1$$; return lambda(42); }""", SyntaxKind.SimpleLambdaExpression)] [DataRow("""event EventHandler SomeEvent { add { int x = 42; } remove { $$int x = 42;$$ } }""", SyntaxKind.RemoveAccessorDeclaration)] [DataRow("""Derived(int arg) : this($$arg.ToString()$$) { }""", SyntaxKind.ThisConstructorInitializer)] + [DataRow("""enum E { A = $$1$$ }""", SyntaxKind.EnumMemberDeclaration)] + [DataRow("""void M(int i = $$1$$) { }""", SyntaxKind.Parameter)] #if NET [DataRow("""int Property { init { $$_ = value;$$ } }""", SyntaxKindEx.InitAccessorDeclaration)] [DataRow("""record BaseRec(int I); record DerivedRec(int I): BaseRec($$I++$$);""", SyntaxKindEx.PrimaryConstructorBaseType)] @@ -1240,22 +1242,33 @@ public class Derived: Base Derived(string arg) { } {{member}} } - """, LanguageNames.CSharp); + """, AnalyzerLanguage.CSharp); var actual = ExtensionsCS.EnclosingScope(node)?.Kind() ?? SyntaxKind.None; actual.Should().Be(expectedSyntaxKind); } - private static SyntaxNode NodeBetweenMarkers(string code, string language, bool getInnermostNodeForTie = false) + [TestMethod] + public void EnclosingScope_TopLevelStatements() + { + var node = NodeBetweenMarkers($$""" + using System; + + $$Console.WriteLine("")$$; + + """, AnalyzerLanguage.CSharp, outputKind: OutputKind.ConsoleApplication); + var actual = ExtensionsCS.EnclosingScope(node)?.Kind() ?? SyntaxKind.None; + actual.Should().Be(SyntaxKind.CompilationUnit); + } + + private static SyntaxNode NodeBetweenMarkers(string code, AnalyzerLanguage language, bool getInnermostNodeForTie = false, OutputKind outputKind = OutputKind.DynamicallyLinkedLibrary) { var position = code.IndexOf("$$"); var lastPosition = code.LastIndexOf("$$"); var length = lastPosition == position ? 0 : lastPosition - position - "$$".Length; code = code.Replace("$$", string.Empty); - var (tree, _) = IsCSharp() ? TestHelper.CompileCS(code) : TestHelper.CompileVB(code); + var (tree, _) = TestHelper.Compile(code, ignoreErrors: false, language, outputKind: outputKind); var node = tree.GetRoot().FindNode(new TextSpan(position, length), getInnermostNodeForTie: getInnermostNodeForTie); return node; - - bool IsCSharp() => language == LanguageNames.CSharp; } private static SyntaxToken GetFirstTokenOfKind(SyntaxTree syntaxTree, SyntaxKind kind) => diff --git a/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs index d70ecf1e58e..f2615a021ff 100644 --- a/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs +++ b/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs @@ -6890,7 +6890,7 @@ internal static class RuleTypeMappingCS // ["S6963"], // ["S6964"], // ["S6965"], - // ["S6966"], + ["S6966"] = "CODE_SMELL", // ["S6967"], // ["S6968"], // ["S6969"], diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs new file mode 100644 index 00000000000..e041152b9ba --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/UseAwaitableMethodTest.cs @@ -0,0 +1,98 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2024 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using SonarAnalyzer.Rules.CSharp; + +namespace SonarAnalyzer.Test.Rules; + +[TestClass] +public class UseAwaitableMethodTest +{ + private const string EntityFrameworkVersion = "7.0.18"; + + private readonly VerifierBuilder builder = new VerifierBuilder(); + + [TestMethod] + public void UseAwaitableMethod_CS() => + builder.AddPaths("UseAwaitableMethod.cs").Verify(); + + [TestMethod] + public void UseAwaitableMethod_CS_Test() => + builder + .WithOptions(ParseOptionsHelper.FromCSharp11) + .AddReferences(MetadataReferenceFacade.SystemNetPrimitives) + .AddReferences(MetadataReferenceFacade.SystemNetSockets) + .AddSnippet(""" + using System; + using System.Text; + using System.IO; + using System.Net; + using System.Net.Sockets; + using System.Threading.Tasks; + using System.Collections.Generic; + + public class Sockets + { + async Task CreateActionAsync(StreamReader reader) + { + Action action = () => + { + reader.ReadLine(); // Compliant + }; + return action; + } + } + """).Verify(); + + [TestMethod] + public void UseAwaitableMethod_Sockets() => + builder + .AddReferences(MetadataReferenceFacade.SystemNetPrimitives) + .AddReferences(MetadataReferenceFacade.SystemNetSockets) + .AddPaths("UseAwaitableMethod_Sockets.cs") + .Verify(); + +#if NET + [TestMethod] + public void UseAwaitableMethod_CSharp9() => + builder + .WithTopLevelStatements() + .AddPaths("UseAwaitableMethod_CSharp9.cs") + .Verify(); + + [TestMethod] + public void UseAwaitableMethod_CSharp8() => + builder + .WithOptions(ParseOptionsHelper.FromCSharp8) + .AddPaths("UseAwaitableMethod_CSharp8.cs") + .Verify(); + + [TestMethod] + public void UseAwaitableMethod_EF() => + builder + .WithOptions(ParseOptionsHelper.FromCSharp11) + .AddReferences([CoreMetadataReference.SystemComponentModelTypeConverter]) + .AddReferences(NuGetMetadataReference.MicrosoftEntityFrameworkCore(EntityFrameworkVersion)) + .AddReferences(NuGetMetadataReference.MicrosoftEntityFrameworkCoreRelational(EntityFrameworkVersion)) + .AddReferences(NuGetMetadataReference.MicrosoftEntityFrameworkCoreSqlServer(EntityFrameworkVersion)) + .AddPaths("UseAwaitableMethod_EF.cs") + .Verify(); +#endif +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs new file mode 100644 index 00000000000..33209cc0b19 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod.cs @@ -0,0 +1,193 @@ +using System.IO; +using System.Threading.Tasks; +using System; +using System.Threading; + +public class C +{ + public C Child { get; } + void VoidMethod() { } + Task VoidMethodAsync() => Task.CompletedTask; + + C ReturnMethod() => null; + Task ReturnMethodAsync() => Task.FromResult(null); + + bool BoolMethod() => true; + Task BoolMethodAsync() => Task.FromResult(true); + + C this[int i] => null; + public static C operator +(C c) => default(C); + public static C operator +(C c1, C c2) => default(C); + public static C operator -(C c) => default(C); + public static C operator -(C c1, C c2) => default(C); + public static C operator !(C c) => default(C); + public static C operator ~(C c) => default(C); + public static implicit operator int(C c) => default(C); + + async Task MethodInvocations() + { + VoidMethod(); // Noncompliant {{Await VoidMethodAsync instead.}} + await VoidMethodAsync(); // Compliant + VoidMethodAsync(); // Compliant: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs4014 + this.VoidMethod(); // Noncompliant + this.Child?.VoidMethod(); // Noncompliant + this.Child.Child?.VoidMethod(); // Noncompliant + + ReturnMethod(); // Noncompliant + _ = ReturnMethod(); // Noncompliant + this.ReturnMethod().ReturnMethod().ReturnMethod(); +// ^^^^^^^^^^^^^^^^^^^ +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @-1 +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @-2 + _ = true ? ReturnMethod() : ReturnMethod(); + // ^^^^^^^^^^^^^^ + // ^^^^^^^^^^^^^^ @-1 + await ReturnMethod().ReturnMethodAsync(); // Noncompliant + // ^^^^^^^^^^^^^^ + } + + public void NonAsyncMethod_VoidReturning() + { + VoidMethod(); // Compliant + } + + public Task NonAsyncMethod_TaskReturning() + { + VoidMethod(); // Compliant: Enclosing Method must be async + return Task.CompletedTask; + } + + async Task OperatorPrecedence() // https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/#operator-precedence + { + _ = new C().ReturnMethod(); // Noncompliant + this[0].VoidMethod(); // Noncompliant + Child[0].VoidMethod(); // Noncompliant + Child.Child[0]?.VoidMethod(); // Noncompliant + Child?.Child[0].VoidMethod(); // Noncompliant + Child?.Child?[0].VoidMethod(); // Noncompliant + Child.Child?[0].VoidMethod(); // Noncompliant + Child?.Child?[0]?.VoidMethod(); // Noncompliant + _ = Child?.Child?[0]?.ReturnMethod()?.Child[0]; // Noncompliant + _ = (ReturnMethod()); // Noncompliant + _ = nameof(VoidMethod); // Compliant + _ = !BoolMethod(); // Noncompliant + _ = BoolMethod() ? ReturnMethod() : default(C); + // ^^^^^^^^^^^^ + // ^^^^^^^^^^^^^^ @-1 + _ = +ReturnMethod(); // Noncompliant + _ = -ReturnMethod(); // Noncompliant + _ = !ReturnMethod(); // Noncompliant + _ = ~ReturnMethod(); // Noncompliant + _ = ReturnMethod() + default(C); // Noncompliant + _ = ReturnMethod() - default(C); // Noncompliant + _ = ReturnMethod() - !ReturnMethod(); + // ^^^^^^^^^^^^^^ + // ^^^^^^^^^^^^^^ @-1 + } + + async Task ExtensionMethods() + { + this.ExtVoidMethod(); // Noncompliant + } +} + +public static class Extensions +{ + public static void ExtVoidMethod(this C c) { } + public static Task ExtVoidMethodAsync(this C c) => Task.CompletedTask; +} + +public class Overloads +{ + public long ImplicitConversionsMethod(int i, IComparable j) => 0; + public Task ImplicitConversionsMethodAsync(long otherName1, int otherName2) => Task.FromResult(0); + public Task ImplicitConversionsMethodAsync(byte otherName1, byte otherName2) => Task.FromResult((byte)0); + + public void TypeParameter(C c) { } + public Task TypeParameter(T t) where T : C => Task.CompletedTask; + + public async Task Test(int i, byte j) + { + long l1 = ImplicitConversionsMethod(i, j); // Noncompliant Can be resolved to first overload + long l2 = ImplicitConversionsMethod(i, (IComparable)j); // Compliant No fitting overload + var l3 = ImplicitConversionsMethod((byte)i, j); // Noncompliant Can be resolved to second overload + int l4 = (int)ImplicitConversionsMethod((byte)i, j); // Noncompliant Can be resolved to second overload + + TypeParameter(new C()); // Compliant: Adding "await" does never resolve to another overload + } +} + +public class Inheritance +{ + class Child : Inheritance + { + public void OuterVoidMethod() { } + public void InnerVoidMethod() { } + } + + class GrandChild : Child + { + public GrandChild Chain { get; } + + public async Task Test() + { + OuterVoidMethod(); // Noncompliant + this.OuterVoidMethod(); // Noncompliant + (this as Child).OuterVoidMethod(); // Noncompliant + this.Chain?.OuterVoidMethod(); // Noncompliant + this.Chain?.Chain?.OuterVoidMethod(); // Noncompliant + InnerVoidMethod(); // Noncompliant + this.InnerVoidMethod(); // Noncompliant + (this as Child).InnerVoidMethod(); // Compliant: InnerVoidMethodAsync is defined on GrandChild + this.Chain?.InnerVoidMethod(); // Noncompliant + this.Chain?.Chain?.InnerVoidMethod(); // Noncompliant + } + + public Task InnerVoidMethodAsync() => Task.CompletedTask; + } + + public Task OuterVoidMethodAsync() => Task.CompletedTask; + + async Task Test() + { + var grandChild = new GrandChild(); + grandChild.OuterVoidMethod(); // Noncompliant + grandChild?.OuterVoidMethod(); // Noncompliant + grandChild?.Chain?.OuterVoidMethod(); // Noncompliant + grandChild.InnerVoidMethod(); // Noncompliant + grandChild?.InnerVoidMethod(); // Noncompliant + grandChild?.Chain?.InnerVoidMethod(); // Noncompliant + } +} + +class AsynchronousLambdas +{ + async Task CallAsyncLambda(StreamReader reader) + { + await Task.Run(async () => { + await Foo(); + reader.ReadLine(); // Noncompliant + }); + Func a = async () => + { + await Foo(); + reader.ReadLine(); // Noncompliant + }; + Func b = async delegate () + { + await Foo(); + reader.ReadLine(); // Noncompliant + }; + } + + async Task CreateActionAsync(StreamReader reader) + { + Action action = () => + { + reader.ReadLine(); // Compliant + }; + return action; + } + + Task Foo() => null; +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_CSharp8.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_CSharp8.cs new file mode 100644 index 00000000000..7ffb9194861 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_CSharp8.cs @@ -0,0 +1,43 @@ +using System.IO; +using System.Threading.Tasks; +using System; + +public class C +{ + public C Child { get; } + C this[int i] => null; + public static implicit operator int(C c) => default(C); + + C ReturnMethod() => null; + Task ReturnMethodAsync() => null; + + async Task OperatorPrecedence() // https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/#operator-precedence + { + _ = ^ReturnMethod(); // Noncompliant + _ = Child?.ReturnMethod()!.Child; // Noncompliant + _ = (Child!?.Child?[0]!)?.ReturnMethod()!?.Child[0]!; // Noncompliant + _ = (ReturnMethod()!); // Noncompliant + _ = (ReturnMethod())!; // Noncompliant + _ = ((ReturnMethod())!); // Noncompliant + } + + async Task LocalFunctions() + { + VoidMethod(); // FN + + void VoidMethod() { } + Task VoidMethodAsync() => null; + } + + async Task InLocalFunction() + { + async Task AsyncLocalFunction(C c) + { + c.ReturnMethod(); // Noncompliant + } + void LocalFunction(C c) + { + c.ReturnMethod(); // Compliant + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_CSharp9.cs new file mode 100644 index 00000000000..c3134a9189c --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_CSharp9.cs @@ -0,0 +1,6 @@ +using System.IO; +using System.Threading.Tasks; +using System; + +var ms = new MemoryStream(); +ms.Dispose(); // Noncompliant {{Await DisposeAsync instead.}} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_EF.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_EF.cs new file mode 100644 index 00000000000..934fcf50039 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_EF.cs @@ -0,0 +1,40 @@ +using Microsoft.EntityFrameworkCore; +using System.IO; +using System.Threading.Tasks; +using System; +using System.Linq; + +public class EnitityFramework +{ + public async Task Query() + { + // Note to implementers: Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions and RelationalQueryableExtensions might be needed to be added to some sort of whitelist for IQueryables + DbSet dbSet = default; + dbSet.Add(null); // Noncompliant + dbSet.AddRange(null); // Noncompliant + dbSet.All(x => true); // Noncompliant + dbSet.Any(x => true); // Noncompliant + dbSet.Average(x => 1); // Noncompliant + dbSet.Contains(null); // Noncompliant + dbSet.Count(); // Noncompliant + dbSet.ElementAt(1); // Compliant: EF 8.0 only + dbSet.ElementAtOrDefault(1); // Compliant: EF 8.0 only + dbSet.ExecuteDelete(); // Noncompliant + dbSet.ExecuteUpdate(x => x.SetProperty(x => x.ToString(), x => string.Empty)); // Noncompliant + dbSet.Find(null); // Noncompliant + dbSet.First(); // Noncompliant + dbSet.FirstOrDefault(); // Noncompliant + dbSet.Last(); // Noncompliant + dbSet.LastOrDefault(); // Noncompliant + dbSet.Load(); // Noncompliant + dbSet.LongCount(); // Noncompliant + dbSet.Max(); // Noncompliant + dbSet.Min(); // Noncompliant + dbSet.Single(); // Noncompliant + dbSet.SingleOrDefault(); // Noncompliant + dbSet.Sum(x => 0); // Noncompliant + dbSet.ToArray(); // Noncompliant + dbSet.ToDictionary(x => 0); // Noncompliant + dbSet.ToList(); // Noncompliant + } +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_Sockets.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_Sockets.cs new file mode 100644 index 00000000000..242d727c490 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UseAwaitableMethod_Sockets.cs @@ -0,0 +1,21 @@ +using System; +using System.Text; +using System.IO; +using System.Net; +using System.Net.Sockets; +using System.Threading.Tasks; +using System.Collections.Generic; + +public class Sockets +{ + public async Task Connect() + { + var hostEndPoint = new IPEndPoint(new IPAddress(new byte[] { 127, 0, 0, 1 }), 80); + var buffer = new List>(); + var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + socket.Connect(hostEndPoint); // Noncompliant + socket.Accept(); // Noncompliant + socket.Receive(buffer, SocketFlags.Broadcast); // Noncompliant + socket.Send(buffer, SocketFlags.Broadcast); // Noncompliant + } +} diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/CoreMetadataReference.cs b/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/CoreMetadataReference.cs index 0bf4a722355..d9e6a8bac33 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/CoreMetadataReference.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/CoreMetadataReference.cs @@ -40,6 +40,7 @@ public static class CoreMetadataReference public static MetadataReference SystemCollectionsNonGeneric { get; } = CreateReference("System.Collections.NonGeneric.dll"); public static MetadataReference SystemConsole { get; } = CreateReference("System.Console.dll"); public static MetadataReference SystemComponentModelPrimitives { get; } = CreateReference("System.ComponentModel.Primitives.dll"); + public static MetadataReference SystemComponentModelTypeConverter { get; } = CreateReference("System.ComponentModel.TypeConverter.dll"); public static MetadataReference SystemDataCommon { get; } = CreateReference("System.Data.Common.dll"); public static MetadataReference SystemDiagnosticsTraceSource { get; } = CreateReference("System.Diagnostics.TraceSource.dll"); public static MetadataReference SystemDiagnosticsProcess { get; } = CreateReference("System.Diagnostics.Process.dll");