No Explicit Conversion In Foreach

ID

csharp.no_explicit_conversion_in_foreach

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

reliability, suspicious-cast, type-conversion

Description

Reports a foreach whose declared loop-variable type cannot be reached from the collection’s element type by an implicit conversion. The language silently inserts a cast of every element, so the loop throws InvalidCastException on the first element that does not fit.

Rationale

foreach is specified to convert each element to the declared loop-variable type. When that conversion is implicit — iterating a List<Dog> as Animal or as an interface Dog implements — nothing can fail. When it is not, the compiler still accepts the loop and emits a cast, and the failure is deferred to run time.

Two things make that failure expensive. It is partial: elements before the offending one have already been processed, so a loop that writes to a database or a file leaves the work half done and the exception gives no hint about how far it got. And it is invisible at the call site: there is no cast operator in the source, so a reader sees a plain loop, not a downcast. Compare with the explicit alternative, foreach (var a in animals) { var d = (Dog) a; …​ }, where the cast is visible and can be replaced with a pattern match.

A loop that intends to process only some elements should say so, with OfType<T>() or a type pattern, rather than relying on every element happening to be of the derived type.

The rule reports only conversions it can decide from the file being analysed. The collection must be a variable declared with an explicit array or single-type-argument collection type, and the element type together with its whole supertype chain must be declared in the same compilation unit. When the hierarchy leaves the file, an unseen base class or interface could make the conversion implicit, so the rule stays silent rather than guessing. Loops over var collections, method results and LINQ chains are likewise not analysed.

using System.Collections.Generic;
using System.Linq;

public class Animal { }
public class Dog : Animal { }
public class Cat : Animal { }

public class Kennel
{
    public void Walk(List<Animal> animals)
    {
        foreach (Dog dog in animals)          // FLAW — casts every element, throws on the first Cat
        {
            Feed(dog);
        }
    }

    public void WalkSafely(List<Animal> animals)
    {
        foreach (Animal animal in animals)    // OK — implicit upcast, cannot fail
        {
            Feed(animal);
        }

        foreach (Dog dog in animals.OfType<Dog>())  // OK — non-dogs are filtered out, not cast
        {
            Feed(dog);
        }
    }

    private void Feed(Animal animal) { }
}

Remediation

Pick the option that matches the intent:

  • All elements really are of the derived type — fix the collection’s declared element type so the loop needs no conversion at all.

  • Only some elements are of interest — filter with OfType<T>(), or iterate with var and use a type pattern (if (animal is Dog dog)) so non-matching elements are skipped instead of throwing.

  • A mismatch is a genuine error — iterate with var, cast explicitly, and let the cast document the assumption at the point where it is made.

References