FXCop Custom Rule to detect if catch has logging (Custom Code Analysis)

231 Views Asked by At

I have implemented several "stock" Microsoft Code Analysis rules. However they were lacking in one area that they didn't have detection in a catch to see if logging was implemented.

So my test project has these two methods. I would expect to see one of them raise my custom error while the other passes.

public void CatchTestNoLogging()
{
   try
   { string A = "adsf"; }
   catch (Exception ex)
   { throw; }
 }

 public void CatchTestLogging()
 {
    try
    { string A = "adsf"; }
    catch (Exception ex)
    { 
       log.Error("Test Error");
       throw;
     }
  }

My custom rule is able to detect if there is a catch but I can't see how I detect if the Logging is used?

This is a SNIP of the custom rule:

if (iList[i].OpCode == OpCode._Catch)
{
   isCatchExists = true; //this gets hit as I want
   //so the question is what can I do to detect if logging is implemented in the catch?
}

Just a quick pointer on how I access would be great. Thank You

1

There are 1 best solutions below

0
On

Well,

It's not perfect but here is what I got. I knew / had a framework from Google of how to look for a catch block...Well that's half what I needed so I started there and that code looked like this:

if (item.OpCode == OpCode._Catch)
{
   isCatchExists = true;
}

So I stepped through in the debugger to see what a catch with logging (log4net) looked like compared to no logging and see if something tangible jumps out. Well I did find this:

Side by Side

Exploring the log4net object I see this:

enter image description here

So I didn't see a way to determine that the log.error was in the catch but I can determine if there is one present. SO I wrote this:

public override ProblemCollection Check(Member member)
{
   Method method = member as Method;

   bool isCatchExists = false;
   bool isLogError = false;

   if(method != null)
   {
      //Get all the instrections of the method.
      InstructionCollection iList = method.Instructions;

      foreach (Instruction item in iList)
      {
         #region Check For Catch

         if (item.OpCode == OpCode._Catch)
         {
            isCatchExists = true;
         }

         #endregion

         #region Check for Error Logging (log4net)

         if (item.Value != null)
         {

             if (item.OpCode == OpCode.Callvirt && item.Value.ToString() == "log4net.ILog.Error")
             {
               isLogError = true;
             }
         }

         #endregion

      }


      if (isCatchExists && !isLogError)
      {
        Problems.Add(new Problem(this.GetNamedResolution("AddLogging", member.FullName)));
      }
    }
        return this.Problems;
 }

This works but with some caveats.

  1. This is hardcoded to log4net. a. I don't like the hard code but I would likely add a different rule for other logging methodologies anyway.

  2. I can't determine if the log.error is IN the catch block. a. So a person could have a log.error in the try block and my rule would pass it.