Methods should be declared static if possible

Rule ND1208 Object Oriented Design

An instance method that can be safely declared static should be. The narrower a method's scope, the fewer bugs: static methods can't access instance members, the compiler enforces it, and the signature tells clients the call can't alter object state. It is also a micro performance optimization (no this reference passed).

Beware: declaring a designer-bound event handler static might break the designer generated code if it uses this.Method() syntax.

How to fix

Declare matched methods static; consider moving them to a static utility class since they use no instance member of their type or base-types.

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Methods should be declared static if possible</Name>
// <Id>ND1208:MethodsShouldBeDeclaredStaticIfPossible</Id>
warnif count > 0

from t in JustMyCode.Types.Where(t =>
   !t.IsStatic && !t.IsInterface &&
   !t.IsEnumeration && !t.IsDelegate &&
   !t.IsGeneratedByCompiler &&

   // Don't force static call on base class
   !t.IsAbstract &&

   // Don't advise to declare Global ASP.NET or ApiController methods as static
   !t.SimpleName.EndsWith("Controller") &&
   !t.DeriveFrom("System.Web.HttpApplication".AllowNoMatch()) &&
   !t.DeriveFrom("System.Web.Http.ApiController".AllowNoMatch()) &&
   !t.DeriveFrom("Microsoft.AspNetCore.Mvc.Controller".AllowNoMatch()) &&
   !t.DeriveFrom("Microsoft.AspNetCore.Mvc.ControllerBase".AllowNoMatch()) &&
   !t.HasAttribute("Microsoft.AspNetCore.Mvc.ControllerAttribute".AllowNoMatch()) &&
   
   // Don't set as static methods of JSON serialized type
   !t.TypesUsed.Any(t1 => t1.IsAttributeClass && 
      (t1.ParentNamespace.Name == "Newtonsoft.Json" || t1.ParentNamespace.Name == "System.Text.Json")
    ) &&

   // Don't set as static methods of XML serialized type
   !t.HasAttribute("System.Xml.Serialization.XmlRootAttribute".AllowNoMatch()) &&
   !t.IsUsing("System.Xml.Serialization.XmlElementAttribute".AllowNoMatch()) &&
   !t.IsUsing("System.Xml.Serialization.XmlAttributeAttribute".AllowNoMatch()) &&
   !t.HasAttribute("System.SerializableAttribute".AllowNoMatch()) &&

   // Methods of classes that derive from Unity classes are used implicitly by the Unity engine
   !t.BaseClasses.Any(bc => bc.ParentNamespace.Name.StartsWithAny("UnityEngine","UnityEditor"))
)

let methodsThatCanBeMadeStatic = 
   from m in t.InstanceMethods

   // An instance method can be turned to static if it is not virtual, 
   // not using the this reference and also, not using
   // any of its class or base classes instance fields or instance methods.
   where !m.IsAbstract && !m.IsVirtual &&
         !m.AccessThis && !m.IsExplicitInterfaceImpl &&
         !m.IsProtected && // Protected method access doesn't match well with static methods

         // Don't warn about not yet implemented methods.
         !m.CreateA("System.NotImplementedException".AllowNoMatch()) &&

         // These Razor methods must not be static
         !m.SimpleNameEqualsAny("OnGet","OnPost", "OnGetAsync", "OnPostAsync") &&

          // Optimization: Using FirstOrDefault() avoid to check all members, 
          //               as soon as one member is found
          //               we know the method m cannot be made static.
          m.MembersUsed.FirstOrDefault(
              mUsed => !mUsed.IsStatic && 
                       (mUsed.ParentType == t || 
                        t.DeriveFrom(mUsed.ParentType))
          ) == null 
          
   select m

from m in methodsThatCanBeMadeStatic
let staticFieldsUsed = m.ParentType.StaticFields.UsedBy(m).Where(f => !f.IsGeneratedByCompiler)
let methodsCallingMe = m.MethodsCallingMe

// All callers of the method must be in JustMyCode (or in test code),
// else having a method declared as static would break the call from the code generated
// like when a WPF Connect() method is binding a method to an event.
where methodsCallingMe.All(m1 => JustMyCode.Contains(m1) || m1.IsTestCode)

select new { 
   m, 
   staticFieldsUsed,
   methodsCallingMe,
   Debt = (1 + methodsCallingMe.Count())*30.ToSeconds().ToDebt(),
   Severity = Severity.Medium
}

//<Expl>The *{0}* can be declared as static because it doesn't use any instance member through the *this* reference.</Expl>

//<Desc>
// An instance method that can be *safely* declared static should be. The
// narrower a method's scope, the fewer bugs: static methods can't access
// instance members, the compiler enforces it, and the signature tells
// clients the call can't alter object state. It is also a micro performance
// optimization (no *this* reference passed).
//
// Beware: declaring a designer-bound event handler static might break the
// designer generated code if it uses *this.Method()* syntax.
//</Desc>

//<HowToFix>
// Declare matched methods static; consider moving them to a static utility
// class since they use no instance member of their type or base-types.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Object Oriented Design: all 14 items