Click here to Skip to main content
14,766,145 members
Please Sign up or sign in to vote.
4.53/5 (7 votes)
See more:
I have written a rule for catching empty catch block using fxcop.But it catches block which have throw statement in it.I want to write a rule which throws a warning message for empty catch block.
Any help would be apprecited.

public override ProblemCollection Check(Member member)
        {
            Method method = member as Method;
            bool isCatchExists = false;
           bool isThrowExists = false;
            Instruction objInstr = null;
            if (method != null)
            {
                InstructionList iList = method.Instructions;

                for (int i = 0; i < iList.Length; i++)
                {
                    if (iList[i].OpCode == OpCode._Catch)
                    {
                        isCatchExists = true;
                    }

                    if (isCatchExists)
                    {
                       
                    }


                    if (isCatchExists)
                    {
                        //If the call is not made to HandleError method. 
                        if (!isThrowExists)
                        {
                            Resolution resolu = GetResolution(new string[] { method.ToString() });
                            Problems.Add(new Problem(resolu));
                        }
                    }

                   
                }
            }
            return Problems;

        }
    }
Posted
Comments
   
Good idea. Empty catch blocks are needed is some rare cases though, usually for recovery in some poorly designed or implemented 3rd-party APIs. Nevertheless, my 5 for the question.
Have you implemented other useful rules?
--SA
bsb25 9-Nov-11 4:11am
   
I have completed FXcop rules for empty catch block and empty finally.

I found following example:
Custom FxCop rule for detecting empty catch blocks[^].
Note following (there is no throw check in your code):
public override Problem[] Check (Method method )
{

bool isCatchExists = false;
bool isThrowExists = false;

//Get all the instrections of the method.
InstructionList iList = method.Instructions ;

for(int i=0;i<ilist.length;i++)>
{
if(iListIdea.OpCode == OpCode._Catch )
{
isCatchExists = true;
}

//Checking for the existance of the catch block
if(isCatchExists)
{
      // Checking for throw 
       if(iListIdea.OpCode == OpCode.Throw)
      {
            isThrowExists = true;
      }
}
}

if(isCatchExists )
{
//If the call is not made to HandleError method.
if(!isThrowExists)
{
Problems.Add(GetResolution(method.FullName));
}
}
return ( Problems.AsArray() );

} 
   
Comments
Uday P.Singh 30-Aug-11 10:40am
   
my 5!
Elina Blank 30-Aug-11 11:00am
   
thanks
The core of this question is how you define 'empty'. Your code will raise as an issue any catch block, and that posted by Elina, any catch block that does not contain a throw*. Catch blocks that do nothing except call a method (e.g. a log function) or even just set a field or return a value (error conditions in asynchronous handlers, perhaps) are acceptable and shouldn't throw a style violation (unless you have a policy against them).

You need to actually inspect the contents of the catch block (I don't remember if that is before or after the catch opcode, I haven't played with this stuff), and see if it hits your definition of 'empty'. I recommend that 'empty' means no return statements, assignments (checking if a local variable is later used is too hard to be worth it) or method calls (instance or static).

(*: actually, any function which has at least one catch and no throw after the catch. For example this would pass:
void ContrivedExample(){
 int a = 42;
 try { a = 5 / 0; } catch {}

 throw new ArgumentException();
}

)
   

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900