Page 3 of 10               << Previous  1  2  3  4  5  6  7  8  9  10  Next >>

Custom FxCop Rule - Do not use GOTO Statements

This custom FxCop rule enforces the idea of not using VB Goto statements in your code - never a good programming practice.

To determine if we have GOTOs in the MSIL (intermediate language), we need to look at the OpCodes. This can be done with any dissasembler. The opcode for a GOTO is an unconditional branch. Looking at the IL you will see br.s instructions followed by an address. Many source code statements can yield unconditional branch IL instructions, but this doesn't mean the statement is a GOTO. We need to see if we have any unconditional branches to locations where those locations do nothing, e.g. contain the opcode of NOP.

C# code for the custom FxCop rule:

using System;
using System.Collections.Generic;
using System.Text;
using System.Collections;
using Microsoft.Cci;
using Microsoft.FxCop.Sdk;
using Microsoft.FxCop.Sdk.Introspection;

public class DoNotUseGotoStatements : BaseMigrationIntrospectionRule
{
    public DoNotUseGotoStatements() : base("DoNotUseGotoStatements")
    {
    }

    public override ProblemCollection Check(Member member)
    {
        // Method is a subtype of Member.
        Method method = member as Method;

        if (method == null)
            return base.Check(member);

        //
        // Number of instructions in the method Method's name.
        //
        InstructionList ops = method.Instructions;
        string methodName = method.Name.Name.ToUpper();

        //
        // If the method doesn't have any opcodes, no analysis is needed.
        //
        if (ops.Length.Equals(0))
            return base.Check(member);

        //
        // Dont bother with default constructors.
        // Default constructor does nothing except call the constructor 
        // in its base class. It is usually inserted into an assembly by 
        // the compiler.
        //
        if (methodName == ".CTOR" && IsDefaultCtor(method))
              return base.Check(member);
        int numberGotos = 0;

        //
        // Count the number of branches that branch to locations 
        // containing the NOP opcode.
        //
        if (isGotoUsed(method, out numberGotos))
        {
            base.Problems.Add(new Problem(GetResolution(
                method.FullName, numberGotos.ToString()), method));
        }
        return base.Problems;
    }
        
    //
    // See if the method is a default constructor - constructor that does nothing 
    // except call the constructor in its base class, usually inserted into your 
    // assemblies by the compiler.
    //
    private bool IsDefaultCtor(Method method)
    {
        InstructionList ops = method.Instructions;

        if (ops.Length.Equals(4))
        {
            //
            // Filter out default ctor generated by the compiler.
            //
            LocalList localList = ops[0].Value as LocalList;

            if (localList != null && localList.Length != 0) return false;
            if (ops[1].OpCode != OpCode.Ldarg_0) return false;
            if (ops[2].OpCode != OpCode.Call) return false;

            InstanceInitializer init = ops[2].Value as InstanceInitializer;

            if (init == null) return false;
            if (ops[3].OpCode != OpCode.Ret) return false;

            return true;
        }
        return false;
    }

    //
    // Checks if the method contains Goto statements.
    //
    private bool isGotoUsed(Method method, out int numberGotos)
    {
        InstructionList ops = method.Instructions;
        ArrayList targetAddresses = new ArrayList();
        ArrayList nopAddresses = new ArrayList();
        numberGotos = 0;

        //
        // Save a list of all unconditional Break opcodes and 
        // their target offsets.
        //
        for (int i = 0; i < ops.Length; i++)
        {
            switch (ops[i].OpCode)
            {
                case OpCode.Br_S:
                    targetAddresses.Add(ops[i].Value.ToString());
                    break;
                case OpCode.Nop:
                    nopAddresses.Add(ops[i].Offset.ToString());
                    break;
                default:
                    break;
            }
        }
        
        //
        // If any Break has a target that is a NOP, it's probably a Goto statement.
        //
        if (targetAddresses.Count > 0 && nopAddresses.Count > 0)
        {
            for (int i = 0; i < targetAddresses.Count; i++)
            {
                if (nopAddresses.Contains(targetAddresses[i]))
                {
                    numberGotos += 1;
                }
            }
        }
        return (numberGotos > 0);
    }
}
 

Rule definition in the XML rules file:

<Rule TypeName="DoNotUseGotoStatements" 
        Category="VBMigration" CheckId="AA1002">
    <Name>
        Do not use 'Goto' statements
    </Name>
    <Description>
        'Goto' statements should be avoided completely in .NET
    </Description>
    <Url>
        http://www.thescarms.com/
    </Url>
    <Resolution>
        Do not use 'Goto' statements in .NET. Break out functionality into 
        separate functions. {1} Goto(s) used in '{0}'
    </Resolution>
    <MessageLevel Certainty="85">
        Warning</MessageLevel>
    <FixCategories>
        NonBreaking
    </FixCategories>
    <Owner />
<Rule>

Page 3 of 10               << Previous  1  2  3  4  5  6  7  8  9  10  Next >>