Use Short Circuit Logic

ID

csharp.use_short_circuit_logic

Severity

critical

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

code_smell, control-flow, suspicious-operator

Description

Reports the non-short-circuit boolean operators & and | applied to boolean operands, where the short-circuit forms && and || were intended.

Rationale

& and | evaluate both operands, always. && and || stop as soon as the result is decided. That difference is the whole point of a guard clause: if (s != null & s.Length > 0) still reads s.Length when s is null and throws the exact exception the null test was written to avoid. The same applies to any right-hand side with side effects — a call that mutates state, a lazily initialised property, an expensive lookup — all of which run even when the left-hand side has already settled the answer.

What the rule looks at is the operands, not the position. A boolean & is just as wrong in an assignment, in a return, or nested inside a larger && chain as it is in an if condition, so the same expression is reported wherever it appears.

To avoid flagging deliberate bit manipulation, every operand must be established as boolean. Syntax settles the common shapes — a comparison, a type test, a negation, a boolean literal, a variable declared bool — and the remaining operands are resolved by type, which is what brings in boolean-returning methods and properties. Because the test is on the resolved type, integer work such as int i = 1 | 2 or the bit test (flags & Mask) != 0 is left alone, as is any expression whose operand types cannot be resolved.

public class Validator
{
    public bool HasContent(string s)
    {
        if (s != null & s.Length > 0)   // FLAW — s.Length runs even when s is null
        {
            return true;
        }
        return false;
    }

    public bool Ready(bool loaded, bool failed)
    {
        while (loaded & !failed)        // FLAW — both operands always evaluated
        {
            Step();
        }
        return true;
    }

    public bool Complete(string s)
    {
        bool valid = s != null & s.Length > 0;   // FLAW — not a condition, same defect
        return valid;
    }

    public bool Available()
    {
        return IsReady() | IsLoaded();  // FLAW — both calls always run
    }

    public bool Safe(string s)
    {
        return s != null && s.Length > 0;   // OK — short-circuits on null
    }

    public bool BitTest(int flags, int mask)
    {
        return (flags & mask) != 0;     // OK — bit manipulation on integers
    }

    private bool IsReady() { return true; }
    private bool IsLoaded() { return true; }
    private static void Step() { }
}

Remediation

Replace & with && and | with ||. Then re-read the right-hand operand: if it was relied on for a side effect, move that side effect out of the condition onto its own statement so it no longer depends on evaluation order.

References