Problem with C# S2327 - "try" statements with identical "catch" and/or "finally" blocks should be merged

[bjmyers]

There is an issue with C# rule S2327: “try” statements with identical “catch” and/or “finally” blocks should be merged.

The sample code from the official documents shows this code that violates the rule:

try
{
    DoTheFirstThing(a, b);
}
catch (InvalidOperationException ex)
{
    HandleException(ex);
}

DoSomeOtherStuff();

try  // Noncompliant; catch is identical to previous
{
    DoTheSecondThing();
}
catch (InvalidOperationException ex)
{
    HandleException(ex);
}

And suggests it be changed to this:

try
{
    DoTheFirstThing(a, b);
    DoSomeOtherStuff();
    DoTheSecondThing();
}
catch (InvalidOperationException ex)
{
    HandleException(ex);
}

The problem is that these snippets aren’t the same. If DoTheFirstThing throws an exception, the first snippet will still execute DoSomeOtherStuff and DoTheSecondThing. The second block of code won’t execute either of those methods.

The following C# program demonstrates the difference in behavior. SonarLint for Visual Studio 2019, version 4.25.0.20544, flags the code as noncompliant where indicated.
Program.txt (2.2 KB)

I think this rule is fundamentally flawed. Due to the short-circuiting nature of exception handling, combining multiple try/catch statements into one will always exhibit different behavior than when they were separate. Refactoring such code to use multiple methods, etc. for readability might be appropriate, but merging the try/catch statements is incorrect.

[Pavel_Mikula]

Hi @bjmyers,

We had almost the same question recently asked in this issue

While the rule itself is questionable in this case, it behaves as specified. You can deactivate the rule if it doesn’t match your projects’ needs.

Or you can use it to improve your code anyway by refactoring this method into something like this to improve readability and maintainability of the code:

    TryDoTheFirstThing(a, b);
    DoSomeOtherStuff();
    TryDoTheSecondThing();