The other day my co-worker Jeffrey Richter and I were discussing my latest infatuation, Roslyn analyzers. As we bounced around a few ideas one came to the forefront; every catch block should throw. This is not a hard and fast rule, but eating an exception, especially accidentally, has caused more bugs in .NET than we all care to count. My mind started spinning thinking that I want to see if I can automate checking for eaten exceptions in a Roslyn analyzer.
At first glance, the idea seems pretty easy. Look for the catch blocks and look to see if there are any throws, returns, or, goto statements. (Realizing that you can use a goto in a catch block made me involuntarily shudder.) Anyway, sketching through a quick algorithm of going about analyzing the syntax nodes made me realize there was going to be nothing quick about it. This was going to be a lot of work, especially when you really start thinking about all the ramifications. While I welcomed the challenge, I thought I better look deeper at the Roslyn API to see what it offers. Fortunately for everyone, there’s a super nice control flow analysis engine built right into Roslyn.
To see how it worked, I created a Roslyn console application and dove in. The main processing method is shown below that processes a C# program string. I created every possible way to execute in a catch block and ran the program to see what the results would be. You can find the whole gist here: https://gist.github.com/JohnWintellect/78e065b7e60fcf6dffa8
After throwing every possible catch block processing through the above method, it was very easy to tell that a catch block had a rethrow or throw as the only way out if the following conditions were met:
- The flow analysis succeeded
- The return statement count was zero
- The endpoint reached flag was true
- The exit point count was zero
As you can see the Analyze Flow Control API is simple to use and provides a ton of useful information. You will have to be careful using the API because the more code you analyze, the slower it performs. This is especially important if you are using the code flow API inside analyzers and code fixes as those run as part of the compile as well as when editing in the IDE.
In my case, catch blocks generally have smaller amounts of code so I think the trade off of the notification that you have an eaten exception is worth it. In the final analyzer that I wrote, I also looked to see if there were any diagnostics (errors or warnings) inside the catch block I was currently analyzing. If there were, I skip the analysis. That ensures I’m only calling the control flow API on good code and avoiding potential slow analysis because of errors.
Another decision I made with my rule was that it should only be an information notification. Because there are a few (very few!) instances where it’s reasonable to eat exceptions I didn’t want to force people to insert suppressions in their code. That also means you only see those informational notifications when you have that source file open. I’m right on the fence of making CatchBlocksShouldRethrow analyzer at least warning level. I reserve the right to change my mind on the level in the future.
As with all Wintellect analyzers, you can find the code here: https://github.com/Wintellect/Wintellect.Analyzers. To look at the complete code for CatchBlocksShouldRethrow, go here””: https://github.com/Wintellect/Wintellect.Analyzers/blob/master/Source/Wintellect.Analyzers/Wintellect.Analyzers/Design/CatchBlocksShouldRethrowAnalyzer.cs. Please do fork and contribute or request analyzers you think the world should have. I’m happy to become your personal analyzer and code fix developer.