No Unchecked Enumerable Sum

ID

csharp.no_unchecked_enumerable_sum

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

arithmetic-overflow, linq, misleading-code, reliability

Description

Reports a Sum() call over a sequence of int or long that sits inside an unchecked block or unchecked(…​) expression. The aggregate adds with overflow checking of its own, so the unchecked wrapper changes nothing about how it behaves.

Rationale

unchecked is a statement of intent. It says the arithmetic inside may overflow, that wrapping is the accepted outcome, and that no exception should be raised. Around ordinary + that is exactly what happens.

The LINQ Sum aggregate does not participate. Its implementation adds inside its own checked context, so it raises an overflow exception when the running total leaves the range of the element type — whether or not the call site is unchecked. The wrapper is not merely redundant: it is a comment that contradicts the code it encloses.

The cost is paid twice. A reader looking for the code paths that can throw skips this one, because it is marked as arithmetic that wraps. And when the exception does arrive, it comes from inside a framework method, so the stack trace names the aggregate rather than the unchecked that was supposed to be governing it — and the enclosing block is the last place anyone looks.

double, float and decimal sequences are not reported. Floating-point arithmetic has no checked form at all, and the decimal aggregate throws on overflow in either context, so unchecked was never promising anything there.

A Sum inside a try that has a catch is also not reported: catching the overflow is a decision about what the code should do when the total does not fit, which makes the surrounding unchecked redundant rather than misleading.

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

public class Totals
{
    public int Wrapped(int[] values)
    {
        unchecked
        {
            return values.Sum();            // FLAW — throws on overflow regardless
        }
    }

    public double Average(List<double> samples)
    {
        unchecked
        {
            return samples.Sum();           // OK — floating point has no checked form
        }
    }

    public int Guarded(int[] values)
    {
        unchecked
        {
            try
            {
                return values.Sum();        // OK — the overflow is handled deliberately
            }
            catch (OverflowException)
            {
                return int.MaxValue;
            }
        }
    }
}

Remediation

Decide what should happen when the total does not fit, and write that.

To let the total wrap, do the addition yourself inside the unchecked block with a fold or a loop, so the context you declared is the one doing the arithmetic:

unchecked
{
    int total = 0;
    foreach (int value in values) total += value;
    return total;
}

To keep the exception, drop the unchecked wrapper — it was never having an effect — and handle OverflowException where the failure can be dealt with.

To avoid the overflow instead of reacting to it, widen the accumulator: sum into a long with values.Sum(v ⇒ (long)v), or into a decimal when even long could be exceeded.

References