Fix S6966 FP: EntityFrameworks IDbContextFactory CreateDbContext method is preferred over its Async counterpart

[manuel-rw]

Description

Rule ID: S6966

As documented in dotnet/efcore#26630, the CreateDbContextAsync() is a method for special edge cases only.
The synchronous CreateDbContext() is preferred for most cases. This has been reflected in updated rules in the Roslyn analyzers: dotnet/roslyn-analyzers#7036

However, Sonar currently emits S6966 for any usage of the CreateDbContext() method.

Repro steps

public class DependencyInjectedService
{
  private readonly IDbContextFactory<AppDbContext> _factory;

  public DependencyInjectedService(IDbContextFactory<AppDbContext> factory)
  {
    _factory = factory;
  }
  
  public async Task DoSomeWork()
  {
    using AppDbContext dbContext = _factory.CreateDbContext(); // Awaitable method should be used csharpsquid:S6966
  }
}

public class AppDbContext : DbContext
{
}

Expected behavior

Since CreateDbContext is preferred Sonar should not emit a diagnostic warning.

Actual behavior

Sonar emits the S6966 warning and proposes the CreateDbContextAsync method instead.

Known workarounds

1. Disable the rule -> also disables the check for all other methods
2. Create a custom extension method which wraps any calls to CreateDbContext() and ignores the warning on this specific line. Obvious disadvantage being that the efcore API is being hidden and additional complexity is being introduced.

Related information

• C#/VB.NET Plugins version: C# 9.27 (build 93347), VB 9.27 (build 93347)
• Visual Studio version: Using Rider, but irrelevant for this ticket -> build and Sonar is executed within pipeline
• MSBuild / dotnet version: msbuild 7.1.1 / .NET 8.0.303
• SonarScanner for .NET version:
• Operating System: Windows 11, 23H2 (64bit)

[sebastien-marichal]

Hello @manuel-rw,

I am not able to reproduce the issue with your code snippet.
S6966 should not be emitted in your context as you use the synchronous version in a synchronous method.
It would raise if DoSomeWork would be marked with the async keyword, in which case I believe it would be a true positive.

Am I missing something?

[manuel-rw]

Hi @sebastien-marichal ,
thanks for the fast reply.
Seems like I forgot copy over the asynchronous method declaration from the project.
You are right, the warning is emitted in a method that is asynchronous and returns a Task.
I updated my comment above. Sorry for the confusion.

As far as I understood the linked issues, this should not emit S6966 because the async method is intended for special usecases only and causes additional overhead. Hence Sonar should not suggest the async method.

[sebastien-marichal]

That's not my understanding. The only overhead I see is people relying on the covariance, which is not a problem in your case (I might miss some context).

Why do you think it is only intended for special use cases?

For me, the rule makes sense in this context.

[manuel-rw]

Hi,
as mentioned in my above comment, .NET 9 for Microsoft.CodeAnalysis.NetAnalyzers version 9 will suppress this warning for the call to CreateDbContextAsync() which is also documented in dotnet/roslyn-analyzers#7058.

Referencing dotnet/efcore#26630 again:

In retrospect, it's possible we should not have added this

We discussed, and while maybe we should not have added this API (time will tell), we don't think making a breaking change to remove it would be the best thing to do now.

Source: https://github.com/dotnet/efcore/blob/1e32b1da8fd01a79b88c66d2c05e69994c238c9e/src/EFCore/IDbContextFactory.cs#L27-L37

And as mentioned by https://stackoverflow.com/a/71186090, the method does not perform any I/O or actions that would require asynchronous handling.

[sebastien-marichal]

Thank you for bringing up the Roslyn PR; I was missing it.

I agree now that CreateDbContextAsync should be excluded.