SonarSource/sonar-dotnet

Fix S6966 FP: MongoDB Find can not be replaced by FindAsync

Closed this issue · 2 comments

Description

S6966 is being triggered on methods that do not match in method signature. For example, in the MongoDB driver, Find and FindAsync are not related. Find returns IFindFluent for building chained mongo requests, while FindAsync returns IAsyncCursor. The following should not be triggering S6966.

UnrelatedThing Find(string s, int i = 0) or UnrelatedThing Find(string s)
Task<string> FindAsync(string s)

Only string Find(string s) should trigger this rule.

CleanShot 2024-05-09 at 15 17 41@2x

Repro steps

_ = Find("123"); // INVALID
_ = await FindAsync("123"); // VALID
_ = FindSync("123"); // VALID
private sealed record UnrelatedThing(string Name, int Number);

private UnrelatedThing Find(string s, int i = 0)
{
    return new UnrelatedThing(s, i);
}

private async Task<string> FindAsync(string s)
{
    return await Task.FromResult(s);
}

private string FindSync(string s)
{
    return s;
}

Expected behavior

_ = Find("123"); should not trigger S6966.

Actual behavior

_ = Find("123"); triggers S6966.

Known workarounds

Please provide a description of any known workarounds.

Related information

  • C#/VB.NET Plugins version
  • Visual Studio version
  • MSBuild / dotnet version
  • SonarScanner for .NET version (if used)
  • Operating System

Thank you @ahusseini

this is a known problem for Mongo DB's API design and has already been documented for the rule:

var sort = Builders<Person>.Sort.Descending(nameof(Person.Name));
// FP for
// * Find https://mongodb.github.io/mongo-csharp-driver/2.8/apidocs/html/M_MongoDB_Driver_IMongoCollectionExtensions_Find__1_3.htm
// * FindAsync https://mongodb.github.io/mongo-csharp-driver/2.8/apidocs/html/M_MongoDB_Driver_IMongoCollectionExtensions_FindAsync__1_3.htm
// Speculative binding finds "FindAsync" but the return type IAsyncCursor<> of FindAsync is not compatible with return type IFindFluent<,> of "Find"
// Speculative binding does overload resolution according to the C# rules, which ignore return types.
// It seems to ignore the compiler binding error for the following "Sort" which is only defined on IFindFluent, but not in IAsyncCursor.
var snapshot = await personCollection.Find(s => s.Id > 10) // Noncompliant FP
.Sort(sort) // Not defined on IAsyncCursor (return type of FindAsync)
.FirstOrDefaultAsync().ConfigureAwait(false);

In Mongo DB FindSync is the synchronous version of FindAsync while Find is meant for something else. Because FindAsync and Find only differ in their return type, overload resolution is successful.

However, because the return types are not compatible, the overload should be rejected by our logic.

I marked this as a false positive and put it in our backlog.

We have two options on how to solve this

  1. We need to ensure that the (unwrapped) return type of the async method is covariant-compatible with the return type of the original method. This may produce FNs in a couple of cases like this
    • The return value is ignored on the call side
    • There is an explicit conversion of the return value to the type returned by the async version
    • Nullable<struct> vs struct
    • There might be all kinds of complicated cases with generics (e.g. IAdditionOperators vs int)
  2. Exclude MongoDB's Find method.

Option 2) can be done easily once #9318 is merged.