No Modulus Direct Equality

csharp.no_modulus_direct_equality

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Description

Reports an == or != comparison between a % expression and a non-zero integer literal, in either operand order and including a negated literal. Comparisons against 0 are not reported, and neither is a dividend that cannot be negative: an unsigned integral type, a cast to one, or a collection size.

Rationale

In C# the remainder operator keeps the sign of the dividend, so -3 % 2 is -1 and not 1. The familiar odd-number test x % 2 == 1 therefore answers "even" for every negative odd value, and x % 2 == -1 has the mirror-image blind spot for positive ones. Because most test data is positive, the defect survives review and unit tests and only shows up on real input. Comparing against zero is immune to the sign rule, which is why it is the dependable form.

A dividend that cannot be negative has a remainder that cannot be negative either, so the comparison is correct and is not reported. Three shapes qualify: a variable of an unsigned integral type (uint, ulong, ushort, byte), a cast to one of those, and a collection size — the Length, LongLength, Count and Capacity properties, and the Count() and LongCount() methods. Parity and padding tests on s.Length are ordinary code and matching those member names is deliberate. When the dividend is anything else (a method call, an element access, an unresolved field), the comparison is still reported rather than assumed safe.

public bool IsOddBroken(int x)
{
    return x % 2 == 1;                // FLAW — false for -3, -5, -7, ...
}

public bool IsOddUnsigned(uint x)
{
    return x % 2 == 1;                // OK — an unsigned dividend has no negative remainder
}

public bool IsOddLength(string s)
{
    return s.Length % 2 == 1;         // OK — a length is never negative
}

public bool IsOdd(int x)
{
    return x % 2 != 0;                // OK — zero is unaffected by the sign of the dividend
}

public bool EveryThird(int x)
{
    return Math.Abs(x % 3) == 1;      // OK — the remainder is normalised before comparing
}

Remediation

Rewrite the test against zero: x % 2 != 0 for "odd", x % n == 0 for "divisible by n". When a specific non-zero remainder really is the question, normalise first — Math.Abs(x % n) == r, or ((x % n) + n) % n == r to get the mathematical (always non-negative) remainder. If the reported dividend can in fact never be negative but its type could not be resolved here, making the type explicit (or asserting the range) both silences the finding and documents the assumption.

References

Remainder operator %