Skip to content

Commit

Permalink
Fix S6966 FP: EntityFrameworks IDbContextFactory CreateDbContext meth…
Browse files Browse the repository at this point in the history
…od is preferred over its Async counterpart
  • Loading branch information
sebastien-marichal committed Aug 6, 2024
1 parent b0e8a3f commit 270d700
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 8 deletions.
17 changes: 9 additions & 8 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/UseAwaitableMethod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ private static ImmutableArray<Func<IMethodSymbol, bool>> BuildExclusions(Compila
{
exclusions.Add(x => x.IsAny(KnownType.Microsoft_EntityFrameworkCore_DbSet_TEntity, ExcludedMethodNames)); // https://github.com/SonarSource/sonar-dotnet/issues/9269
exclusions.Add(x => x.IsAny(KnownType.Microsoft_EntityFrameworkCore_DbContext, ExcludedMethodNames)); // https://github.com/SonarSource/sonar-dotnet/issues/9269
exclusions.Add(x => x.IsImplementingInterfaceMember(KnownType.Microsoft_EntityFrameworkCore_IDbContextFactory_TContext, "CreateDbContext"));
}
if (compilation.GetTypeByMetadataName(KnownType.FluentValidation_IValidator) is not null)
{
Expand All @@ -109,25 +110,25 @@ private static ImmutableArray<Func<IMethodSymbol, bool>> BuildExclusions(Compila
}

private static ImmutableArray<ISymbol> FindAwaitableAlternatives(WellKnownExtensionMethodContainer wellKnownExtensionMethodContainer, ImmutableArray<Func<IMethodSymbol, bool>> exclusions,
InvocationExpressionSyntax invocationExpression, SemanticModel semanticModel, ISymbol containingSymbol, CancellationToken cancel)
InvocationExpressionSyntax invocationExpression, SemanticModel model, ISymbol containingSymbol, CancellationToken cancel)
{
var awaitableRoot = GetAwaitableRootOfInvocation(invocationExpression);
if (awaitableRoot is not { Parent: AwaitExpressionSyntax } // Invocation result is already awaited.
&& invocationExpression.EnclosingScope() is { } scope
&& IsAsyncCodeBlock(scope)
&& semanticModel.GetSymbolInfo(invocationExpression, cancel).Symbol is IMethodSymbol { MethodKind: not MethodKind.DelegateInvoke } methodSymbol
&& model.GetSymbolInfo(invocationExpression, cancel).Symbol is IMethodSymbol { MethodKind: not MethodKind.DelegateInvoke } methodSymbol
&& !(methodSymbol.IsAwaitableNonDynamic() // The invoked method returns something awaitable (but it isn't awaited).
|| methodSymbol.ContainingType.DerivesFromAny(ExcludedTypes))
&& !exclusions.Any(x => x(methodSymbol)))
{
// 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 }
var invokedType = invocationExpression.Expression.GetLeftOfDot() is { } expression && model.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());
// Get the method alternatives and exclude candidates that would resolve to the containing method (endless loop)
var awaitableAlternatives = SpeculativeBindCandidates(semanticModel, awaitableRoot, invocationExpression, awaitableCandidates)
var awaitableAlternatives = SpeculativeBindCandidates(model, awaitableRoot, invocationExpression, awaitableCandidates)
.Where(x => !containingSymbol.Equals(x))
.ToImmutableArray();
return awaitableAlternatives;
Expand All @@ -148,15 +149,15 @@ private static IEnumerable<INamedTypeSymbol> WellKnownExtensionMethodContainer(W
? extensionMethodContainer
: [];

private static IEnumerable<ISymbol> SpeculativeBindCandidates(SemanticModel semanticModel, SyntaxNode awaitableRoot,
private static IEnumerable<ISymbol> SpeculativeBindCandidates(SemanticModel model, SyntaxNode awaitableRoot,
InvocationExpressionSyntax invocationExpression, IEnumerable<IMethodSymbol> awaitableCandidates) =>
awaitableCandidates
.Select(x => x.Name)
.Distinct()
.Select(x => SpeculativeBindCandidate(semanticModel, x, awaitableRoot, invocationExpression))
.Select(x => SpeculativeBindCandidate(model, x, awaitableRoot, invocationExpression))
.WhereNotNull();

private static IMethodSymbol SpeculativeBindCandidate(SemanticModel semanticModel, string candidateName, SyntaxNode awaitableRoot,
private static IMethodSymbol SpeculativeBindCandidate(SemanticModel model, string candidateName, SyntaxNode awaitableRoot,
InvocationExpressionSyntax invocationExpression)
{
var invocationIdentifierName = invocationExpression.GetMethodCallIdentifier()?.Parent;
Expand All @@ -165,7 +166,7 @@ private static IMethodSymbol SpeculativeBindCandidate(SemanticModel semanticMode
return null;
}
var invocationReplaced = ReplaceInvocation(awaitableRoot, invocationExpression, invocationIdentifierName, candidateName);
var speculativeSymbolInfo = semanticModel.GetSpeculativeSymbolInfo(invocationReplaced.SpanStart, invocationReplaced, SpeculativeBindingOption.BindAsExpression);
var speculativeSymbolInfo = model.GetSpeculativeSymbolInfo(invocationReplaced.SpanStart, invocationReplaced, SpeculativeBindingOption.BindAsExpression);
var speculativeSymbol = speculativeSymbolInfo.Symbol as IMethodSymbol;
return speculativeSymbol;
}
Expand Down
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public sealed partial class KnownType
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_IDbContextFactory_TContext = new("Microsoft.EntityFrameworkCore.IDbContextFactory", "TContext");
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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,32 @@ public async Task DatabaseFacade(DatabaseFacade databaseFacade)
databaseFacade.UseTransaction(null); // Noncompliant
}
}

// https://github.com/SonarSource/sonar-dotnet/issues/9590
public class Repro_9590
{
public async Task DoSomeWork(IDbContextFactory<AppDbContext> factory, MyDbContextFactory factory2, NotADbContextFactory factory3)
{
using AppDbContext dbContext = factory.CreateDbContext(); // Compliant - CreateDbContextAsync is excluded
using AppDbContext dbContext2 = factory2.CreateDbContext(); // Compliant - CreateDbContextAsync is excluded
using AppDbContext dbContext3 = factory3.CreateDbContext(); // Noncompliant
}

public class MyDbContextFactory : IDbContextFactory<AppDbContext>
{
public AppDbContext CreateDbContext() => throw new NotImplementedException();

public Task<AppDbContext> CreateDbContextAsync() => throw new NotImplementedException();
}

public class NotADbContextFactory
{
public AppDbContext CreateDbContext() => throw new NotImplementedException();

public Task<AppDbContext> CreateDbContextAsync() => throw new NotImplementedException();
}

public class AppDbContext : DbContext
{
}
}

0 comments on commit 270d700

Please sign in to comment.