CC0004 and exception filters

[dariusbuinovskij]

#New analyzer:

CC0004 analyzer unable to handle filters. I some cases I need to log exceptions (catch block never reached). I can disable warning by adding #pragma warning disable, but I would like to avoid that. Can I expect that to be fixed in coming releases?

Before:

		static void Main()
		{
			try
			{
				// Some code...
			}
			catch (Exception ex) when (Log(ex))
			{ }
		}

		static bool Log(Exception ex)
		{
			Console.WriteLine(ex.Message);
			return false;
		}

After:

			try
			{
				// Some code...
			}
#pragma warning disable CC0004 // Catch block cannot be empty
			catch (Exception ex) when (Log(ex))
			{ }
#pragma warning restore CC0004 // Catch block cannot be empty
		}

		static bool Log(Exception ex)
		{
			Console.WriteLine(ex.Message);
			return false;
		}

[jwooley]

At first glance I was going to recommend against this as a typical monadic comprehension (like filter/where) would not guard against side effects. However, checking the C# 6 FAQ, this behavior is actually recommended:

It is also a common and accepted form of “abuse” to use exception filters for side effects; e.g. logging. They can inspect an exception “flying by” without intercepting its course. In those cases, the filter will often be a call to a false-returning helper function which executes the side effects:

This may be a valid extension to make for this analyzer. We could go even further and have an analyzer that detects the following pattern and recommends using an expression filter to avoid the catch/throw performance penalty:

bad:

try
{
  // Do something
}
catch
{
  WriteLine(ex.Message);
  throw;
}

better:

try
{
  // Do Something
}
catch (Exception ex) when (Log(ex))
{
}

bool Log(Exception ex)
{
  WriteLine(ex.Message);
  // TODO: Add conditional evaluation logic here
  return false;
}

[dariusbuinovskij]

Yes, I know that logging is side effect of exception filters, but is better than
try { // Do something } catch { WriteLine(ex.Message); throw; }

Yes, analyzer you suggested sounds good. As I understand error about empty catch block will disappear as well when you will add such extra analyzer?