S4456 — Parameter validation in yielding methods should be wrapped

Why is this an issue?

Because of the way yield methods are rewritten by the compiler (they become lazily evaluated state machines) any exceptions thrown during the parameters check will happen only when the collection is iterated over. That could happen far away from the source of the buggy code.

Therefore it is recommended to split the method into two: an outer method handling the validation (no longer lazy) and an inner (lazy) method to handle the iteration.

This rule raises an issue when a method throws any exception derived from ArgumentException and contains the yield keyword.

Noncompliant code example

public static IEnumerable<TSource> TakeWhile<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate) // Noncompliant
{
    if (source == null) { throw new ArgumentNullException(nameof(source)); }
    if (predicate == null) { throw new ArgumentNullException(nameof(predicate)); }

    foreach (var element in source)
    {
        if (!predicate(element)) { break; }
        yield return element;
    }
}

public static async IAsyncEnumerable<TSource> TakeWhileAsync(this IEnumerable<TSource> source, Func<TSource, bool> predicate) // Noncompliant
{
    if (source == null) { throw new ArgumentNullException(nameof(source)); }
    if (predicate == null) { throw new ArgumentNullException(nameof(predicate)); }

    foreach (var element in source)
    {
        await Task.Delay(10);
        if (!predicate(element)) { break; }
        yield return element;
    }
}

Compliant solution

public static IEnumerable<TSource> TakeWhile<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    if (source == null) { throw new ArgumentNullException(nameof(source)); }
    if (predicate == null) { throw new ArgumentNullException(nameof(predicate)); }
    return TakeWhileIterator<TSource>(source, predicate);
}

private static IEnumerable<TSource> TakeWhileIterator<TSource>(IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    foreach (TSource element in source)
    {
        if (!predicate(element)) break;
        yield return element;
    }
}

public static IAsyncEnumerable<TSource> TakeWhileAsync(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    if (source == null) { throw new ArgumentNullException(nameof(source)); }
    if (predicate == null) { throw new ArgumentNullException(nameof(predicate)); }

    return TakeWhileAsyncIterator<TSource>(source, predicate);
}

private static async IAsyncEnumerable<TSource> TakeWhileAsyncIterator<TSource>(IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    foreach (TSource element in source)
    {
        await Task.Delay(10);
        if (!predicate(element)) break;
        yield return element;
    }
}

Resources

Related rules

• {rule:csharpsquid:S4457} - Parameter validation in async/await methods should be wrapped