False postives on S3267: These loops can not be simplified using LINQ

Rules and Languages Report False-positive / False-negative...
1

If we look to the two following examples:

static class Compliant
{
    public static bool IsMatch(IEnumable<string> labels, (ReadOnlySpan<char> value)
    {
        foreach (var label in labels) // FP: by ref structs can not appear in lambda expressions, anonymous functions etc..
        {
            if (label.AsSpan().Equals(value)) return true;
        }
        return false;
    }

   public static T? FirstOrNone<T>(this IEnumerable<T> enumerable, Predicate<T> predicate) where T : struct
   {
       foreach (var element in enumerable) // FP
       {
           if (predicate(element))
           {
               return element;
           }
       }
       return null;
   }
}

Reported by SonarAnalyzer.CSharp v10.7.0.110445

The latter might be rewritten as:

static class Compliant
{
   public static T? FirstOrNone<T>(this IEnumerable<T> enumerable, Predicate<T> predicate) where T : struct
   {
       var enumerator = enumerable.GetEnumerator();
       while (enumerator.MoveNext())
       {
           if (predicate(enumerator.Current))
           {
               return enumerator.Current;
           }
       }
       return null;
   }
}

But I would argue that the code is harder to read than before the refactoring.

4

Hi @Corniel

the first case (ref struct) is a duplicate of Fix S3267 FP: `ref struct` types cannot leave the stack · Issue #8430 · SonarSource/sonar-dotnet · GitHub

For the second one (struct constraint) you are right. You can actually rewrite the method using LINQ, but this isn’t very intuitive:

   public static T? FirstOrNone<T>(this IEnumerable<T> enumerable, Predicate<T> predicate) where T : struct =>
       enumerable.Cast<T?>().FirstOrDefault(x => predicate(x!.Value));

sharplab

I will create an issue and re-producer for this case.

1 Like
7

Not only is the rewrite not easy to understand, it is also not good from a performance point of view. :wink: