Compiler warning CA1839 on DbSet.Add

[jamesgurung]

Since updating to .NET 8 Preview 7, all calls to DbSet<TEntity>.Add(TEntity entity) and DbSet<TEntity>.AddRange(params TEntity[] entities) result in a CA1839 compiler warning.

For example, calling context.Users.Add(user); results in:

Warning CA1849 'DbSet.Add(User)' synchronously blocks. Await 'DbSet.AddAsync(User, CancellationToken)' instead.

However, the summary for AddAsync reads:

This method is async only to allow special value generators, such as the one used by 'Microsoft.EntityFrameworkCore.Metadata.SqlServerValueGenerationStrategy.SequenceHiLo', to access the database asynchronously. For all other cases the non async method should be used.

Should this warning be suppressed by default?

[Spacefish]

Did a PR to fix this: https://github.com/dotnet/roslyn-analyzers/pull/6858

[roji]

This is a bit tricky: if the user happens to be using a value generator which does I/O (such as HiLo), then this warning makes sense just like it does for any other .NET API where an async counterpart exists. The problem is that usage of HiLo is rare, and for those cases using Add is fine and the warning is noise. On the other hand, there's no great disadvantage in calling AddAsync instead of Add.

Note #30957 which is the long-term solution here, i.e. obsolete AddAsync altogether.

[roji]

Note: https://github.com/dotnet/roslyn-analyzers/pull/6858 suppresses the diagnostic in the analyzer. We should consider doing this in the EF repo instead via a diagnostics suppressor, in order to eventually remove the Roslyn-side special-casing (though that's useful for now as it also takes care of older EF versions where our suppressor wouldn't be available, thanks @Spacefish).

Unless, of course, we go with #30957 which will obsolete AddAsync altogether.

[virzak]

So the consensus for most projects is to keep Add since AddAsync might go away. Is that correct?

[ajcvickers]

@virzak AddAsync isn't going to go away anytime soon. You should use it if your value generator needs to access the database outside of SaveChanges. This is uncommon.