NDepend v2.9 comes with a set of default CQL rules concerning usage guidelines of the .NET
Framework. Some of them are inspired from some FxCop
rules while some others
are inspired from our development experience. There are significant advantages in the usage of CQL to write such rules:
The set of rules is
straightforward to customize with your own rules or modified rules,
more adapted to your code base.
This shows how to use CQL to write rules about
usage of any
framework, not only the .NET framework.
Using CQL rule naming + comments is a good way to
store the convention itself and its name/description at the same place.
Finally, once your application analyzed, NDepend can run dozens of CQL
rules per second, even on multi-millions Lines of Code code base (from
our tests 7 seconds to run 325 rules on the entire .NET Fx 3.5 code
base, WPF/WCF assemblies included, this represent around 1.5M lines of
C# code).
We would
like to hear from you about any .NET Framework usage rules you learnt the hard
way. (support at ndepend dot com)
Althought
CQL rules can be read seamlessly, I take a chance to precise the meaning of the
prefix OPTIONAL: found before code elements’ names. When the CQL compiler parses
a code element’ name, the default behavior is to emit an error if the code
element is not found. Thus to write generic CQL rules applicable to any code
base, no matter which .NET framework code element it references, we use the
OPTIONAL: prefix to prevent such error on missing code elements.
System
// <Name>Mark ISerializable types with serializable</Name>
SELECT TYPES WHERE
IsPublic AND !IsDelegate AND
Implement "OPTIONAL:System.Runtime.Serialization.ISerializable" AND
!HasAttribute "OPTIONAL:System.SerializableAttribute"
// To be recognized by the CLR as serializable, types must be marked with the
// SerializableAttribute attribute even if the type uses a custom serialization routine
// through implementation of the ISerializable interface.
// <Name>Mark assemblies with assembly version</Name>
WARN IF Count > 0 IN SELECT ASSEMBLIES WHERE
!HasAttribute "OPTIONAL:System.Reflection.AssemblyVersionAttribute" AND
!IsFrameworkAssembly
// The identity of an assembly is composed of the following information:
// - Assembly name
// - Version number
// - Culture
// - Public key (for strong-named assemblies).
// The .NET Framework uses the version number to uniquely identify an assembly,
// and to bind to types in strong-named assemblies. The version number is used
// together with version and publisher policy. By default, applications run only
// with the assembly version with which they were built.
// <Name>Mark assemblies with CLSCompliant</Name>
WARN IF Count > 0 IN SELECT ASSEMBLIES WHERE
!HasAttribute "OPTIONAL:System.CLSCompliantAttribute" AND
!IsFrameworkAssembly
// The Common Language Specification (CLS) defines naming restrictions, data types,
// and rules to which assemblies must conform if they are to be used across programming
// languages. Good design dictates that all assemblies explicitly indicate
// CLS compliance with CLSCompliantAttribute. If the attribute is not present on an
// assembly, the assembly is not compliant.
// <Name>Mark assemblies with ComVisible</Name>
WARN IF Count > 0 IN SELECT ASSEMBLIES WHERE
!HasAttribute "OPTIONAL:System.Runtime.InteropServices.ComVisibleAttribute" AND
!IsFrameworkAssembly
// The ComVisibleAttribute attribute determines how COM clients access managed code.
// Good design dictates that assemblies explicitly indicate COM visibility.
// COM visibility can be set for an entire assembly and then overridden for individual
// types and type members. If the attribute is not present, the contents of the assembly
// are visible to COM clients.
// <Name>Mark attributes with AttributeUsageAttribute</Name>
WARN IF Count > 0 IN SELECT TYPES WHERE
DeriveFrom "OPTIONAL:System.Attribute" AND
!HasAttribute "OPTIONAL:System.AttributeUsageAttribute" AND
!IsInFrameworkAssembly
// When defining a custom attribute, mark it using AttributeUsageAttribute to
// indicate where in the source code the custom attribute can be applied.
// An attribute's meaning and intended usage will determine its valid locations
// in code. For example, if you are defining an attribute that identifies the
// person responsible for maintaining and enhancing each type in a library,
// and responsibility is always assigned at the type level, compilers should
// allow the attribute on classes, enumerations, and interfaces, but should
// not allow it on methods, events, or properties. Organizational policies and
// procedures would dictate whether the attribute should be allowed on assemblies.
// <Name>Remove calls to GC.Collect()</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
IsDirectlyUsing "OPTIONAL:System.GC.Collect()" OR
IsDirectlyUsing "OPTIONAL:System.GC.Collect(Int32)" OR
IsDirectlyUsing "OPTIONAL:System.GC.Collect(Int32,GCCollectionMode)"
// It is preferrable to avoid calling GC.Collect() explicitely
// in order to avoid some performance pitfall.
// More in information on this here:
// http://blogs.msdn.com/ricom/archive/2004/11/29/271829.aspx
// <Name>Don't call GC.Collect() without calling GC.WaitForPendingFinalizers()</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
(IsDirectlyUsing "OPTIONAL:System.GC.Collect()" OR
IsDirectlyUsing "OPTIONAL:System.GC.Collect(Int32)" OR
IsDirectlyUsing "OPTIONAL:System.GC.Collect(Int32,GCCollectionMode)")
AND
!IsDirectlyUsing "OPTIONAL:System.GC.WaitForPendingFinalizers()"
// It is preferrable to avoid calling GC.Collect() explicitely
// in order to avoid some performance pitfall.
// But if you wish to call GC.Collect(), you must do it this way:
// GC.Collect();
// GC.WaitForPendingFinalizers();
// GC.Collect();
// To make sure that finalizer got executed, and object with finalizer got cleaned properly.
// <Name>Enum Storage should be Int32</Name>
WARN IF Count > 0 IN SELECT FIELDS WHERE
NameIs "value__" AND
!IsOfType "OPTIONAL:System.Int32" AND
!IsInFrameworkAssembly
// An enumeration is a value type that defines a set of related named constants.
// By default, the System.Int32 data type is used to store the constant value. Even
// though you can change this underlying type, it is not necessary or recommended
// for most scenarios. Note that there is no significant performance gain in using
// a data type smaller than Int32. If you cannot use the default data type, you should
// use one of the CLS-compliant integral types, Byte, Int16, Int32, or Int64, to
// ensure that all of the enumeration's values are representable in CLS-compliant
// programming languages.
// <Name>Do not raise too general exception types</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
// The following exception types are too general to provide sufficient information
// to the user:
( ( DepthOfCreateA "OPTIONAL:System.Exception" == 1 OR
DepthOfCreateA "OPTIONAL:System.ApplicationException" == 1 OR
DepthOfCreateA "OPTIONAL:System.SystemException" == 1 )
// Test for non-constructor, else this constraint would warn
// on ctor of classes that derive from these exception types.
AND !IsConstructor )
// <Name>Do not raise reserved exception types</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
// The following exception types are reserved and should be thrown only by the
// common language runtime:
( DepthOfCreateA "OPTIONAL:System.ExecutionEngineException" == 1 OR
DepthOfCreateA "OPTIONAL:System.IndexOutOfRangeException" == 1 OR
DepthOfCreateA "OPTIONAL:System.NullReferenceException" == 1 OR
DepthOfCreateA "OPTIONAL:System.OutOfMemoryException" == 1 )
// <Name>Use integral or string argument for indexers</Name>
SELECT METHODS WHERE
IsIndexerGetter AND
!( NameIs "get_Item(String)" OR
NameLike "get_Item\(Int" OR
NameLike "get_Item\(Byte" OR
NameLike "get_Item\(SByte" )
// Indexers, that is, indexed properties, should use integer or string types for the
// index. These types are typically used for indexing data structures and increase
// the usability of the library. Use of the Object type should be restricted to those
// cases where the specific integer or string type cannot be specified at design time.
// If the design requires other types for the index, reconsider whether the type
// represents a logical data store. If it does not represent a logical data store,
// use a method.
// <Name>Uri fields should be of type System.Uri</Name>
WARN IF Count > 0 IN SELECT FIELDS WHERE
(NameLike "Uri$" OR NameLike "Url$") AND !IsOfType "OPTIONAL:System.Uri"
// A field which name end with 'Uri' is deemed as representing a uri.
// Such field should be of type System.Uri.
// <Name>Types should not extend System.ApplicationException</Name>
WARN IF Count > 0 IN SELECT TYPES WHERE
DepthOfDeriveFrom "OPTIONAL:System.ApplicationException" == 1
// For .NET Framework version 1, it was recommended to derive new exceptions from
// ApplicationException. The recommendation has changed and new exceptions should
// derive from System.Exception or one of its subclasses in the System namespace.
System.Collections
// <Name>Don't use .NET 1.x HashTable and ArrayList</Name>
WARN IF Count > 0 IN SELECT TOP 10 METHODS WHERE
// Prefer using the class System.Collections.Generic.Dictionary<K,V> over
// System.Collections.HashTable.
CreateA "OPTIONAL:System.Collections.HashTable" OR
// Prefer using the class System.Collections.Generic.List<T> over
// System.Collections.ArrayList.
CreateA "OPTIONAL:System.Collections.ArrayList"
// You can be forced to use HashTable or ArrayList
// because if you are using tier code that requires working with these classes
// or because you are coding with .NET 1.x.
// <Name>Caution with List.Contains()</Name>
SELECT METHODS WHERE
IsDirectlyUsing "OPTIONAL:System.Collections.Generic.List<T>.Contains(T)" OR
IsDirectlyUsing "OPTIONAL:System.Collections.Generic.IList<T>.Contains(T)" OR
IsDirectlyUsing "OPTIONAL:System.Collections.ArrayList.Contains(Object)"
// The cost of checking if a list contains an object is proportional to the size
// of the list (O(N) operation). For large lists and/or frequent calls to Contains(),
// prefer using the System.Collections.Generic.HashSet<T> class where calls to
// Contains() takes a constant time (O(0) operation).
// <Name>Prefer return collection abstraction instead of implementation</Name>
SELECT METHODS WHERE
ReturnTypeIs "OPTIONAL:System.Collections.Generic.List<T>" OR
ReturnTypeIs "OPTIONAL:System.Collections.Generic.HashSet<T>" OR
ReturnTypeIs "OPTIONAL:System.Collections.Generic.Dictionary<TKey,TValue>"
// Most of the time, clients of a method doesn't need to know the exact implementation
// of the collection returned. It is preferrable to return a collection interface such as IList<T>,
// ICollection<T> or IEnumerable<T>.
System.Runtime.InteropServices
// <Name>P/Invokes should be static and not be visible</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
!IsInFrameworkAssembly AND
(HasAttribute "OPTIONAL:System.Runtime.InteropServices.DllImportAttribute") AND
( IsPublic OR
!IsStatic)
// Methods marked with the DllImportAttribute attribute (or methods defined using the
// Declare keyword in Visual Basic) use Platform Invocation Services to access unmanaged
// code. Such methods should not be exposed. Keeping these methods private or internal
// ensures that your library cannot be used to breach security by allowing callers access
// to unmanaged APIs they could not call otherwise.
// <Name>Move P/Invokes to NativeMethods class</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
!IsInFrameworkAssembly AND
HasAttribute "OPTIONAL:System.Runtime.InteropServices.DllImportAttribute" AND
!FullNameLike "NativeMethods."
// Platform Invocation methods, such as those marked with the
// System.Runtime.InteropServices.DllImportAttribute attribute, or methods
// defined by using the Declare keyword in Visual Basic, access unmanaged code.
// These methods should be in one of the following classes:
//
// - NativeMethods - This class does not suppress stack walks for unmanaged
// code permission. (System.Security.SuppressUnmanagedCodeSecurityAttribute
// must not be applied to this class.) This class is for methods that can
// be used anywhere because a stack walk will be performed.
//
// - SafeNativeMethods - This class suppresses stack walks for unmanaged code
// permission. (System.Security.SuppressUnmanagedCodeSecurityAttribute is
// applied to this class.) This class is for methods that are safe for anyone
// to call. Callers of these methods are not required to do a full security
// review to ensure that the usage is secure because the methods are harmless
// for any caller.
//
// - UnsafeNativeMethods - This class suppresses stack walks for unmanaged
// code permission. (System.Security.SuppressUnmanagedCodeSecurityAttribute
// is applied to this class). This class is for methods that are potentially
// dangerous. Any caller of these methods must do a full security review
// to ensure that the usage is secure because no stack walk will be performed.
// <Name>NativeMethods class should be static and internal</Name>
WARN IF Count > 0 IN SELECT TYPES WHERE
!IsInFrameworkAssembly AND
( NameIs "NativeMethods" OR
NameIs "SafeNativeMethods" OR
NameIs "UnsafeNativeMethods") AND
IsPublic OR
!IsStatic
// Native Methods' classes are declared as internal (Friend, in Visual Basic) and static.
System.Threading
// <Name>Method non-synchronized that read mutable states</Name>
SELECT METHODS WHERE
(ReadsMutableObjectState OR ReadsMutableTypeState) AND
!IsDirectlyUsing "OPTIONAL:System.Threading.Monitor" AND
!IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock"
// Mutable object states are instance fields that can be modifed throught
// the lifetime of the object. Mutable type states are static fields that
// can be modifed throught the lifetime of the program. This query lists
// methods that read mutable state without synchronizing access. In the
// case of multi-threaded program, doing so can lead to state corruption.
// <Name>Don't create threads explicitely</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
CreateA "OPTIONAL:System.Threading.Thread"
// Prefer using the thread pool instead of creating manually your own thread.
// Threads are costly objects.
// They take approximately 200,000 cycles to create and about 100,000 cycles to destroy.
// By default they reserve 1 megabyte of virtual memory for its stack and use
// 2,000-8,000 cycles for each context switch.
// As a consequence, it is preferrable to let the thread pool recycle threads.
// Creating custom thread can also be the sign of flawed design, where tasks and threads
// have affinity. It is preferrable to code tasks that can be ran on any thread.
// <Name>Don't use Thread.Sleep()</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
IsDirectlyUsing "OPTIONAL:System.Threading.Thread.Sleep(Int32)"
// Usage of Thread.Sleep() is a sign of flawed design.
// More information on this here:
// http://msmvps.com/blogs/peterritchie/archive/2007/04/26/thread-sleep-is-a-sign-of-a-poorly-designed-program.aspx
// <Name>Don't use Thread.Abort()</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
IsDirectlyUsing "OPTIONAL:System.Threading.Thread.Abort()" OR
IsDirectlyUsing "OPTIONAL:System.Threading.Thread.Abort(Object)"
// Usage of Thread.Abort() is dangerous.
// More information on this here:
// http://www.interact-sw.co.uk/iangblog/2004/11/12/cancellation
// <Name>Monitor Enter/Exit must be both called within the same method</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.Enter(Object)" AND
!IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.Exit(Object)"
// <Name>Monitor TryEnter/Exit must be both called within the same method</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
( IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.Enter(Object)" OR
IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.TryEnter(Object,Int32)" OR
IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.TryEnter(Object,TimeSpan)") AND
!IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.Exit(Object)"
// <Name>ReaderWriterLock AcquireReaderLock/ReleaseLock must be both called within the same method</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
( IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.AcquireReaderLock(Int32)" OR
IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.AcquireReaderLock(TimeSpan)" OR
IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.TryEnter(Object,TimeSpan)")
AND
!( IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.ReleaseReaderLock()" OR
IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.ReleaseLock()")
// <Name>ReaderWriterLock AcquireWriterLock/ReleaseLock must be both called within the same method</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
( IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.AcquireWriterLock(Int32)" OR
IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.AcquireWriterLock(TimeSpan)" OR
IsDirectlyUsing "OPTIONAL:System.Threading.Monitor.TryEnter(Object,TimeSpan)")
AND
!( IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.ReleaseWriterLock()" OR
IsDirectlyUsing "OPTIONAL:System.Threading.ReaderWriterLock.ReleaseLock()")
System.Xml
// <Name>Method should not return concrete XmlNode</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
( ReturnTypeIs "OPTIONAL:System.Xml.XmlDocument" OR
ReturnTypeIs "OPTIONAL:System.Xml.XmlAttribute" OR
ReturnTypeIs "OPTIONAL:System.Xml.XmlDocumentFragment" OR
ReturnTypeIs "OPTIONAL:System.Xml.XmlEntity" OR
ReturnTypeIs "OPTIONAL:System.Xml.XmlLinkedNode" OR
ReturnTypeIs "OPTIONAL:System.Xml.XmlNotation" OR
ReturnTypeIs "OPTIONAL:System.Xml.XmlNode" )
// The class System.Xml.XmlNode implements the interface
// System.Xml.Xpath.IXPathNavigable. It is preferrable to return
// this interface instead of a concrete class.
// <Name>Types should not extend System.Xml.XmlDocument</Name>
WARN IF Count > 0 IN SELECT TYPES WHERE
DepthOfDeriveFrom "OPTIONAL:System.Xml.XmlDocument" == 1
// Do not create a subclass of XmlDocument if you want to create
// an XML view of an underlying object model or data source.