My Visual Studio 2008 Code Analysis Rules

Although a couple of suggestion for changes to existing rules seem to have made it into Visual Studio 2008 Beta 2, unfortunately, none of my suggestions for new Code Analysis rules made it into Orcas Beta 2 (and thus likely not in RTM).

I was holding off on writing my own rules waiting for a stable SDK because I didn’t really want to write several different versions of the same rule.  But, unfortunately that also wasn’t in the cards for Orcas, so I’ve begun writing a few rules for Visual Studio 2008 that I feel are important.

Some of the rules I’ve suggested over the years that I’ve implemented in this Code Analysis add-in are:
DoNotUseCurrentUICultureAsIFormatProviderArgument
I’ve blogged about issues using CurrentUICulture for formatting and parsing methods (i.e. as an IFormatProvider argument) in the past.  The issue is that CurrentUICulture can be assigned a neutral or a specific culture but formatting and parsing requires a specific culture.  A neutral culture is one that only specifies language and not region or country.  Region and country define the formatting specifications.  Using a neutral culture, although syntactically correct, as an IFormatProvider argument will result in an exception.

DoNotLockOnOPubliclyVisibleObjects
This guideline is from from the MSDN documentation (in various places, see http://msdn2.microsoft.com/en-us/library/c5kehkcz(vs.80).aspx for one).  Code Analysis already checks for locks on typeof but it doesn’t check for locks on this or other public members.  DoNotLockOnOPubliclyVisibleObjects checks for lock on “this” (or SyncLock on “Me”) as well as direct use of Monitor.Enter/Exit.

I’ve created some rules for checking some of the issues I’ve blogged about in the past:
DoNotUseThreadAbort
DoNotUseThreadSleep
DoNotUsesleepWithZeroArgument
…I won’t flog that horse more than it’s already been flogged…


I created one of many (I hope) C#3/VB9 rules:
AvoidExtensionMethodsThatDuplicateInstanceMethods
In Visual C# Orcas you can write an extension method that has the same signature as an instance method.  With the method resolution rules in C# this extension method will never get called; but no warning is issued to that effect.  For example:

using System;

using ArrayExtensions;

 

namespace Application

{

    class Program

    {

        static void Main ( string[] args )

        {

            int[] arr = new int[10];

            arr.SetValue(10, 0);

        }

    }

}

 

namespace ArrayExtensions

{

    internal static class Extensions

    {

        public static void SetValue ( this Array array, object value, int index )

        {

            array.SetValue(value, index);

        }

    }

}

No warning is given that Extensions.SetValue can never be called as an extension method. 

On much the same lines as AvoidExtensionMethodsThatDuplicateInstanceMethods I’ve also added:
DoNotRecursivelyReferenceProperty
Rather than requesting either of these be added to Code Analysis I’d recommend these checks actually be performed by the compiler (as I have with recursively calling a property).  In the meantime, these rules can be flagged as errors in Visual Studio.  Syntactically in C# there’s nothing wrong with calling a property from itself.  This, of course, will result in a StackOverflowException and your application’s termination because of the infinite recursion–without warning or error.  With DoNotRecursivelyReferenceProperty, there isn’t enough metadata to efficiently distinguish between directly calling an object’s property from itself or calling another object’s property from the same property, so this rule will be raised on code like this:

    class Entity

    {

        private Entity otherEntity;

        private string text;

        String Text

        {

            get

            {

                if (otherEntity != null)

                {

                    return otherEntity.Text;

                }

                else

                {

                    return this.text;

                }

            }

        }

        //…

    }


I don’t think this is a bad thing.  For one thing it’s an unlikely scenario; and another, there’s no way to tell if the code might get hit by an infinite recursion (otherEntity could be assigned “this”).  In the unlikely event this scenario is legit (i.e. you’re checking to make sure the object is not “this”) then you can simply add a suppression for this rule where appropriate.  For example:

        [System.Diagnostics.CodeAnalysis.SuppressMessage("PRI.Design", "PRICA1001:DoNotRecursivelyReferenceProperty")]

        String Text

        {

            get

            {

                if (otherEntity != null)

                {

                    return otherEntity.Text;

                }

                else

                {

                    return this.text;

                }

            }

        }


Download


The add-ins may be downloaded from here: http://www.peterritchie.com/Hamlet/Downloads/105.aspx


Deployment
After exiting all instances of Visual Studio 2008 Beta 2, simply unzip the contents to “C:\Program Files\Microsoft Visual Studio 9.0\Team Tools\Static Analysis Tools\FxCop\Rules” and re-load Visual Studio.

4 thoughts on “My Visual Studio 2008 Code Analysis Rules”

  1. Nice work!

    Have a look at RuleUtilities.GetInstancePointer to solve the DoNotRecursivelyReferenceProperty false positives. DoNotCallOverrideableInConstructors does a similar thing.

    Regards

    David

  2. Loading this in the a non-Beta 2008 (9.0.21022.8 RTM) gives me an error:

    Error 208 CA0053 : Unable to load rule assembly ‘C:\Program Files\Microsoft Visual Studio 9.0\Team Tools\Static Analysis Tools\FxCop\Rules\PRI.CodeAnalysis.Rules.Design.dll': Unable to load one or more of the requested types. Retrieve the LoaderExceptions property for more information.

    Is this something in my configuration, or did something change from the Beta that broke these rules?

    I’m chasing a debugger crash issue that I think may be related to a recursive property, so this rule of yours would be extremely helpful!

    For completeness, here’s what I have loaded:

    Microsoft Visual Studio 2008
    Version 9.0.21022.8 RTM
    Microsoft .NET Framework
    Version 3.5

    Installed Edition: Enterprise

    Microsoft Visual Basic 2008 91899-153-0000007-60707
    Microsoft Visual Basic 2008

    Microsoft Visual C# 2008 91899-153-0000007-60707
    Microsoft Visual C# 2008

    Microsoft Visual C++ 2008 91899-153-0000007-60707
    Microsoft Visual C++ 2008

    Microsoft Visual Studio 2008 Team Explorer 91899-153-0000007-60707
    Microsoft Visual Studio 2008 Team Explorer
    Version 9.0.21022.8

    Microsoft Visual Studio 2008 Tools for Office 91899-153-0000007-60707
    Microsoft Visual Studio 2008 Tools for Office

    Microsoft Visual Studio Team System 2008 Architecture Edition 91899-153-0000007-60707
    Microsoft Visual Studio Team System 2008 Architecture Edition

    Microsoft Visual Studio Team System 2008 Database Edition 91899-153-0000007-60707
    Microsoft Visual Studio Team System 2008 Database Edition

    Microsoft Visual Studio Team System 2008 Development Edition 91899-153-0000007-60707
    Microsoft Visual Studio Team System 2008 Development Edition

    Portions of International CorrectSpell™ spelling correction system © 1993 by Lernout & Hauspie Speech Products N.V. All rights reserved.

    The American Heritage® Dictionary of the English Language, Third Edition Copyright © 1992 Houghton Mifflin Company. Electronic version licensed from Lernout & Hauspie Speech Products N.V. All rights reserved.

    Microsoft Visual Studio Team System 2008 Test Edition 91899-153-0000007-60707
    Microsoft Visual Studio Team System 2008 Test Edition

    Microsoft Visual Web Developer 2008 91899-153-0000007-60707
    Microsoft Visual Web Developer 2008

    Crystal Reports AAJ60-G0MSA4K-68000CF
    Crystal Reports Basic for Visual Studio 2008

    Thanks!

  3. @Paulo I’ve run the rules without problem in both the RTM version of 2008 (9.0.21022.8 RTM) and the SP1 version of 2008. (9.0.30729.1 SP).

    If you’ve installed a Beta, I recommend uninstalling it before upgrading to a non-beta version. Sometimes this means completely uninstalling all Visual Studio versions to get it to work proberly.

    Visual Studio betas are not kind to your machine.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>