BRO1149: Merge an `if` into the enclosing `if`¶

An if that is the only statement of another if, where neither has an else, joins it: if (a) { if (b) { ... } } becomes if (a && b) { ... }. Not a StyleCop rule: Sonar's S1066 reports the same pattern, and StyleBro also fixes it.

Example¶

Before¶

public static class Orders
{
    public static void Ship(Order order, bool expressAllowed)
    {
        if (order.IsPaid)
        {
            if (expressAllowed || order.Total > 100)
            {
                order.Express = true;
            }
        }
    }
}

public sealed class Order
{
    public bool IsPaid { get; set; }

    public decimal Total { get; set; }

    public bool Express { get; set; }
}

After¶

public static class Orders
{
    public static void Ship(Order order, bool expressAllowed)
    {
        if (order.IsPaid && (expressAllowed || order.Total > 100))
        {
            order.Express = true;
        }
    }
}

public sealed class Order
{
    public bool IsPaid { get; set; }

    public decimal Total { get; set; }

    public bool Express { get; set; }
}

Why¶

Two nested conditions with nothing between them are one condition written twice as deep. Merged, the body loses a level of indentation and the reader sees at once that both must hold.

How the fix works¶

The conditions are joined with &&, in order; one that binds looser than && (||, ??, ?:, an assignment) gets parentheses. && evaluates left to right and stops early, like the nested ifs, and a pattern variable from the first condition is still definitely assigned in the second (if (o is string s && s.Length > 0)).
The inner body is kept with its braces (or without them) and moves left by as much as the inner if was indented deeper than the outer one; a condition spread over several lines moves the same way. The outer if's {, } and the inner if (...) go. Once the outer condition spans lines, the inner one starts a line of its own, aligned with the outer condition, && where dotnet_style_operator_placement_when_wrapping puts it (at the beginning by default) when BRO1520 enforces that key. When BRO1520 is off (always with CSharpier), it goes at the end of the outer condition's line if the conditions already wrap after an operator there (a && + b), and where the key says otherwise; its own later lines keep their indentation relative to it.
Deeper nests become one condition (if (a && b && c)), and nests inside the merged body are merged in the same fix, so one dotnet format run is enough.
When BRO1514-BRO1516 (braces) are on, the merged if gets the braces they want in the same edit: merging can move a statement into an if/else chain (else if (a) + if (b) X();), where another brace rule reports it.

Not reported¶

An else on either if.
An outer block with more than the inner if (a statement, a local, a comment).
A variable declared in an inner condition (out var x, is string s) whose name is used elsewhere in the block around the outer if: a variable declared in an if condition is in scope in the whole enclosing block, so moving it up would collide with a local of the same name in a sibling block (CS0136).
A comment or directive in the text that goes () {, if (, the inner ) and the closing }), or an #if, #elif, #else or #endif anywhere in the outer if or right after it (an else inside #if): each target framework's copy could see other code. A #region or #pragma line in the body or after the if doesn't keep it: BRO1112/BRO1113 remove regions, and the merge would otherwise come only on the next run.
A condition with a part of type dynamic (d == 1, d.IsReady) in any of the merged ifs, with stylebro_allow_nested_dynamic_ifs = true (see Configuration).
A body that would move left but holds a string spanning lines (moving its lines would change the string), or a line that isn't indented far enough to move.

Configuration¶

[*.cs]
# true: nested ifs with a condition that has a part of type 'dynamic' aren't merged, like Sonar's S1066 (default false)
stylebro_allow_nested_dynamic_ifs = false

Sonar's S1066 skips an inner condition with a dynamic part; stylebro-migrate writes true for a repository that enforces S1066. With it, any merged condition with a dynamic part (the inner or the outer one) keeps the nest as it is. It is an option because finding out needs the compiler's types for the whole method, which costs time in every build.

Compared with other analyzers¶

Sonar's S1066 reports the same pattern (on the inner if) and has no fix. The SDK has no such rule. With stylebro_allow_nested_dynamic_ifs = true, BRO1149 reports no if S1066 accepts, so stylebro-migrate turns it on with that setting for a repository that enforces S1066. S1066 reports a few more: an if inside nested blocks (if (a) { { if (b) ... } }), a plain dynamic variable as the condition (if (d); S1066 only looks at the parts of a condition), and the inner pair of a nest whose middle condition is dynamic.

Also fixes Sonar's S1066 when the SonarAnalyzer.CSharp package is in the project: the fix is registered for S1066 too (the inner if keyword, where Sonar reports it) and applies at the places this rule would fix, so dotnet format clears Sonar's warnings there even with this rule off. Where this rule skips the code (see Not reported), no fix is offered and Sonar's warning stays.

Compared with StyleCop¶

StyleCop has no such rule. stylebro-migrate leaves it off.

BRO1149: Merge an if into the enclosing if¶