Add else if support to RCS1211

[jamesHargreaves12]

Adds support for the following:

if(p){
    return 1;
}
else if (q){
    return 2;
}

to

if(p){
    return 1;
}
if(q){
    return 2;
}

[jamesHargreaves12]

The diffs from L64 onwards do not change behaviour. They are just reducing nesting / Moving the checks that require a semantic model later.

[josefpihrt]

I'm not sure that this proposal fits into this analyzer.

The purpose of this analyzer is simply to remove (or let's say unwrap) last else (in if-else sequence). To be honest, I don't think that this analyzer is that much useful. It should serve more like a refactoring when you want to remove last else (this is why default severity is set to hidden/silent).

This proposal does something else: it essentially split if-else sequence into sequence of separate 'if' statements.

There is actually a refactoring RR0190 which does very similar or same thing.

Regarding the current implementation I see two problems:

1. If there is a sequence of more if-else block when the last if-else is reported and fixed then the previous one will be reported and so on which is not desirable.

2. Current implementation will possibly split "if-else-if-else" into "if if-else" which is weird.

    public async Task Test_UnnecessaryElseIf()
    {
        await VerifyDiagnosticAndFixAsync(@"
class C
{
    int M(bool flag, bool flag2)
    {
        if (flag)
        {
            return 1;
        }
        [|else|] if (flag2)
        {
            M(flag, flag2);
        }
        else
        {
            return 2;
        }

        return 3;
    }
}
", @"
class C
{
    int M(bool flag, bool flag2)
    {
        if (flag)
        {
            return 1;
        }

        if (flag2)
        {
            M(flag, flag2);
        }
        else
        {
            return 2;
        }

        return 3;
    }
}
");
    }

My recommendation would be to keep the analyzer as it is and leave the "split" functionality to the refactoring RR0190.

[jamesHargreaves12]

I agree that the current behaviour of applying it causing the diagnostic to show up in a different place is not ideal and is something that should be fixed... I was going to implement this but then I decided that would be more work and I would do this change piece meal. This sounded like it was a good idea given your comment.

The Rider analogue of this analyzer has exactly the behaviour that you think is weird and personally, I like it. But if you don't want to merge it then feel free to close this PR.

[josefpihrt]

The Rider analogue of this analyzer has exactly the behaviour

@jamesHargreaves12 Could you send me a link to their docs? I would like to take a closer look at that analyzer.

[josefpihrt]

@jamesHargreaves12 ping

[marcinsmialek]

From Rider docs:

Inspections:
https://www.jetbrains.com/help/rider/Reference__Code_Inspections_CSHARP.html#CodeSmell
Adjusted by resharper_redundant_if_else_block_highlighting, by default set to Hint level, which is pretty useful.

Details:
https://www.jetbrains.com/help/rider/RedundantIfElseBlock.html

These ReSharper / Rider EditorConfig values may make it easier to find:
https://www.jetbrains.com/help/rider/EditorConfig_Index.html
resharper_redundant_else_block_highlighting
resharper_redundant_if_else_block_highlighting

There's a fix for it: Remove redundant 'else' keyword