How do I detect and correct usless try catch blocks?

Question

I've started using the .Net Complier Platform (Roslyn) to assist with enforcing coding standards.

One issue I'm struggling with is discovering and catching useless try...catch blocks.

For example:

// Would like to have this detected and offer to remove the try...catch
try
{
    // Do some work
}
catch(Exception ex)
{
    throw ex;
}

It would be good to also detect the fact that the code is using throw ex; rather than just throw; such as:

try
{
    // So some work
}
catch(Exception ex)
{
    // Log the error or anything to manage the exception
    throw ex;  // <-- how to detect and offer a fix for this
}

Any static code analyzer can help with that http://stackoverflow.com/questions/38635/what-static-analysis-tools-are-available-for-c — Jack Malkovich, Jun 17 '14 at 06:58
Jack, thanks for the comment and link however I want to use Roslyn for this as it will detect these issues directly, i.e. I don't want to have to run a build or any other tools to find these issues. I currently use Code Analysis and Gendarme, which work fine, but are not providing the instant feedback I'm looking for. — Steve Wood, Jun 17 '14 at 07:07
I can't figure out how to detect the code sections I mentioned in my question and then correct them. So, for example, how do I find a `try catch` that does nothing more than throw the caught exception and then, apply the fix, i.e. keep whatever `//So some work` is but remove the suronding block. — Steve Wood, Jun 18 '14 at 07:17
For those of you that have put this as 'on hold as unclear' maybe it would be very helpful to SO newbies like me to explain why you have done so? I've read my question back a few times and I can't see how I can make it any more explicit. Even given example of what I'm after. Please help me to provide a better question. — Steve Wood, Jun 20 '14 at 07:14

score 2 · Accepted Answer · answered Jun 18 '14 at 16:18

It sort of depends on what you consider a "useless try-catch". I've assumed you mean catch statements that do no other work except throwing the exception.

Given a C# syntax tree with the code you've provided, you might want to find all the syntax nodes of type CatchClauseSyntax.

You could then look within each for StatementSyntax that is not of type ThrowStatementSyntax. If there are any statements that are not throw, we assume real work is being done here.

For example:

var tree = CSharpSyntaxTree.ParseText(@"
public class MyClass {
public void Method()
{
    try { }
    catch(Exception e)
    {
        //useless
        throw e;
    }
    try {  }
    catch(Exception e)
    {
        //Some work
        int aVariable = 4;
        throw e;
    }
}
}
");

//Finds all catch clauses
var catchClauses = tree.GetRoot().DescendantNodesAndSelf().OfType<CatchClauseSyntax>();
//Look at the catch blocks
var catchBlocks = catchClauses.Select(n => n.DescendantNodes().OfType<BlockSyntax>().First());
//Filter out the clauses where statements all are only throw statements
var uselessClauses = catchBlocks.Where(n => n.Statements.All(m => m is ThrowStatementSyntax));

How do I detect and correct usless try catch blocks?

1 Answers1

Linked