> ## Documentation Index > Fetch the complete documentation index at: https://docs.codeant.ai/llms.txt > Use this file to discover all available pages before exploring further. # C #

Jump statements, such as return, yield break, goto, and continue let you change the default flow of program execution, but jump statements that direct the control flow to the original direction are just a waste of keystrokes.

```csharp Bad theme={null} void Foo() { goto A; // Noncompliant A: while (condition1) { if (condition2) { continue; // Noncompliant } else { DoTheThing(); } } return; // Noncompliant; this is a void method } ``` ```csharp Fix theme={null} void Foo() { while (condition1) { if (!condition2) { DoTheThing(); } } } ```

A cast is an explicit conversion, which is a way to tell the compiler the intent to convert from one type to another.

```csharp Bad theme={null} void Method(object value) { int i; i = (int)value; // Casting (explicit conversion) from float to int } ``` ```csharp Fix theme={null} public interface IMyInterface { /* ... */ } public class Implementer : IMyInterface { /* ... */ } public class AnotherClass { /* ... */ } public static class Program { public static void Main() { var another = new AnotherClass(); var x = (IMyInterface)another; // Noncompliant: InvalidCastException is being thrown } } ```

When a class implements the IEquatable\ interface, it enters a contract that, in effect, states "I know how to compare two instances of type T or any type derived from T for equality.". However if that class is derived, it is very unlikely that the base class will know how to make a meaningful comparison. Therefore that implicit contract is now broken.

Alternatively IEqualityComparer\ provides a safer interface and is used by collections or Equals could be made virtual.

This rule raises an issue when an unsealed, public or protected class implements IEquatable\ and the Equals is neither virtual nor abstract.

```csharp Bad theme={null} using System; namespace MyLibrary { public class Base : IEquatable // Noncompliant { public bool Equals(Base other) { if (other == null) { return false; } // do comparison of base properties return true; } public override bool Equals(object other) => Equals(other as Base); } class A : Base { public bool Equals(A other) { if (other == null) { return false; } // do comparison of A properties return base.Equals(other); } public override bool Equals(object other) => Equals(other as A); } class B : Base { public bool Equals(B other) { if (other == null) { return false; } // do comparison of B properties return base.Equals(other); } public override bool Equals(object other) => Equals(other as B); } internal class Program { static void Main(string[] args) { A a = new A(); B b = new B(); Console.WriteLine(a.Equals(b)); // This calls the WRONG equals. This causes Base.Equals(Base) // to be called which only compares the properties in Base and ignores the fact that // a and b are different types. In the working example A.Equals(Object) would have been // called and Equals would return false because it correctly recognizes that a and b are // different types. If a and b have the same base properties they will be returned as equal. } } } ``` ```csharp Fix theme={null} using System; namespace MyLibrary { public sealed class Foo : IEquatable { public bool Equals(Foo other) { // Your code here } } } ```

Caller information attributes provide a way to get information about the caller of a method through optional parameters. But they only work right if their values aren’t provided explicitly. So if you define a method with caller info attributes in the middle of the parameter list, the caller is forced to use named arguments if they want to use the method properly.

This rule raises an issue when the following attributes are used on parameters before the end of the parameter list:

```csharp Bad theme={null} void TraceMessage([CallerMemberName] string memberName = "", [CallerFilePath] string filePath = "", [CallerLineNumber] int lineNumber = 0, string message = null) // Noncompliant: decorated parameters appear before "message" parameter { /* ... */ } ``` ```csharp Fix theme={null} void TraceMessage(string message = null, [CallerMemberName] string memberName = "", [CallerFilePath] string filePath = "", [CallerLineNumber] int lineNumber = 0) { /* ... */ } ```

Passing a parameter by reference, which is what happens when you use the \`out or ref parameter modifiers, means that the method will receive a pointer to the argument, rather than the argument itself. If the argument was a value type, the method will be able to change the argument’s values. If it was a reference type, then the method receives a pointer to a pointer, which is usually not what was intended. Even when it is what was intended, this is the sort of thing that’s difficult to get right, and should be used with caution.

This rule raises an issue when out or ref is used on a non-Optional parameter in a public method. Optional\` parameters are covered by S3447.

```csharp Bad theme={null} public void GetReply( ref MyClass input, // Noncompliant out string reply) // Noncompliant { ... } ``` ```csharp Fix theme={null} public string GetReply(MyClass input) { ... } public bool TryGetReply(MyClass input, out string reply) { ... } public ReplyData GetReply(MyClass input) { ... } internal void GetReply(ref MyClass input, out string reply) { ... } ```

It is important to be careful when searching for characters within a substring. Let’s consider the following example:

```csharp Bad theme={null} if (str.SubString(startIndex).IndexOf(char1) == -1) // Noncompliant: a new string is going to be created by "Substring" { // ... } ``` ```csharp Fix theme={null} if (str.IndexOf(char1, startIndex) == -1) // Compliant: no new instance of string is created { // ... } ```

There’s no reason for a \`Main method to throw anything. After all, what’s going to catch it?

Instead, the method should itself gracefully handle any exceptions that may bubble up to it, attach as much contextual information as possible, and perform whatever logging or user communication is necessary, and Exit\` with a non-zero (i.e. non-success) exit code if necessary.

```csharp Bad theme={null} public static void Main(string[] args) { // Noncompliant if (args.Length == 0) { throw new ArgumentException(); } // do stuff } ``` ```csharp Fix theme={null} private const int ERROR_INVALID_COMMAND_LINE = 0x667; public static void Main(string[] args) { // Noncompliant if (args.Length == 0) { Console.WriteLine("Should provide at least one argument"); Environment.ExitCode = ERROR_INVALID_COMMAND_LINE; } // do stuff } ```

When a method in a derived class has:

  • the same name as a method in the base class

  • but types of parameters that are ancestors (for example string in the base class and object in the derived class)

the result is that the base method becomes hidden.

As shown in the following code snippet, when an instance of the derived class is used, invoking the method with an argument that matches the less derived parameter type will invoke the derived class method instead of the base class method:

```csharp Bad theme={null} class BaseClass { internal void MyMethod(string str) => Console.WriteLine("BaseClass: Method(string)"); } class DerivedClass : BaseClass { internal void MyMethod(object str) => Console.WriteLine("DerivedClass: Method(object)"); // Noncompliant } // ... BaseClass baseObj = new BaseClass(); baseObj.MyMethod("Hello"); // Output: BaseClass: Method(string) DerivedClass derivedObj = new DerivedClass(); derivedObj.MyMethod("Hello"); // Output: DerivedClass: Method(object) - DerivedClass method is hiding the BaseClass method BaseClass derivedAsBase = new DerivedClass(); derivedAsBase.MyMethod("Hello"); // Output: BaseClass: Method(string) ``` ```csharp Fix theme={null} class BaseClass { internal void MyMethod(string str) => Console.WriteLine("BaseClass: Method(string)"); } class DerivedClass : BaseClass { internal void MyOtherMethod(object str) => Console.WriteLine("DerivedClass: Method(object)"); // Compliant } // ... BaseClass baseObj = new BaseClass(); baseObj.MyMethod("Hello"); // Output: BaseClass: Method(string) DerivedClass derivedObj = new DerivedClass(); derivedObj.MyMethod("Hello"); // Output: BaseClass: Method(string) BaseClass derivedAsBase = new DerivedClass(); derivedAsBase.MyMethod("Hello"); // Output: BaseClass: Method(string) ```

double.NaN and float.NaN are not equal to anything, not even themselves.

When anything is compared with NaN using one of the comparison operators >, >=, \<, ⇐ or the equality operator ==, the result will always be false. In contrast, when anything is compared with NaN using the inequality operator !=, the result will always be true.

Instead, the best way to see whether a variable is equal to NaN is to use the float.IsNaN and double.IsNaN methods, which work as expected.

```csharp Bad theme={null} var a = double.NaN; if (a == double.NaN) // Noncompliant: always false { Console.WriteLine("a is not a number"); } if (a != double.NaN) // Noncompliant: always true { Console.WriteLine("a is not NaN"); } ``` ```csharp Fix theme={null} var a = double.NaN; if (double.IsNaN(a)) { Console.WriteLine("a is not a number"); } if (!double.IsNaN(a)) { Console.WriteLine("a is not NaN"); } ```

When an object has a field annotated with ThreadStatic, that field is shared within a given thread, but unique across threads. Since a class' static initializer is only invoked for the first thread created, it also means that only the first thread will have the expected initial values.

Instead, allow such fields to be initialized to their default values or make the initialization lazy.

```csharp Bad theme={null} public class Foo { [ThreadStatic] public static object PerThreadObject = new object(); // Noncompliant. Will be null in all the threads except the first one. } ``` ```csharp Fix theme={null} public class Foo { [ThreadStatic] public static object _perThreadObject; public static object PerThreadObject { get { if (_perThreadObject == null) { _perThreadObject = new object(); } return _perThreadObject; } } } ```

The rules for method resolution are complex and perhaps not properly understood by all coders. The \`params keyword can make method declarations overlap in non-obvious ways, so that slight changes in the argument types of an invocation can resolve to different methods.

This rule raises an issue when an invocation resolves to a method declaration with params, but could also resolve to another non-params\` method too.

```csharp Bad theme={null} public class MyClass { private void Format(string a, params object[] b) { } private void Format(object a, object b, object c) { } } // ... MyClass myClass = new MyClass(); myClass.Format("", null, null); // Noncompliant, resolves to the first Format with params, but was that intended? ``` ```csharp Fix theme={null} ```

There’s no point in creating an array solely for the purpose of passing it to a params parameter. Simply pass the elements directly. They will be consolidated into an array automatically.

```csharp Bad theme={null} public void Base() { Method(new string[] { "s1", "s2" }); // Noncompliant: unnecessary Method(new string[] { }); // Noncompliant Method(new string[12]); // Compliant } public void Method(params string[] args) { // ... } ``` ```csharp Fix theme={null} public void Base() { Method("s1", "s2"); Method(); Method(new string[12]); } public void Method(params string[] args) { // ... } ```

When only a single public parameterless constructor is defined in a class, then that constructor can be removed because the compiler would generate it automatically. Similarly, empty static constructors and empty destructors are also wasted keystrokes.

```csharp Bad theme={null} class Sample { public Sample() { } // Noncompliant static Sample() { } // Noncompliant ~Sample() { } // Noncompliant ... } ``` ```csharp Fix theme={null} class Sample { ... } ```

\`System.Collections.Generic.List\ is a generic collection that is designed for performance and not inheritance. For example, it does not contain virtual members that make it easier to change the behavior of an inherited class. That means that future attempts to expand the behavior will be spoiled because the extension points simply aren’t there. Instead, one of the following generic collections should be used:

  • System.Collections.Generic.IEnumerable\

  • System.Collections.Generic.IReadOnlyCollection\

  • System.Collections.Generic.ICollection\

  • System.Collections.Generic.IReadOnlyList\

  • System.Collections.Generic.IList\

  • System.Collections.ObjectModel.Collection\

  • System.Collections.ObjectModel.ReadOnlyCollection\

  • System.Collections.ObjectModel.KeyedCollection\

This rule raises an issue every time a System.Collections.Generic.List\\` is exposed:

  • As an externally visible member.

  • As the return type of an externally visible method.

  • As a parameter type of an an externally visible method.

```csharp Bad theme={null} namespace Foo { public class Bar { public List Method1(T arg) // Noncompliant { //... } } } ``` ```csharp Fix theme={null} namespace Foo { public class Bar { public Collection Method1(T arg) { //... } } } ```

Short-circuit evaluation is an evaluation strategy for Boolean operators, that doesn’t evaluates the second argument of the operator if it is not needed to determine the result of the operation.

C# provides logical operators that implement short-circuit evaluation: && and ||, as well as non-short-circuit versions: & and |. Unlike short-circuit operators, non-short-circuit ones evaluate both operands and afterwards perform the logical operation.

For example false && FunctionCall() always results in false, even when FunctionCall invocation would raise an exception. Instead, false & FunctionCall() also evaluates FunctionCall(), and results in an exception if FunctionCall() invocation raises an exception.

Similarly, true || FunctionCall() always results in true, no matter what the return value of FunctionCall() would be.

The use of non-short-circuit logic in a boolean context is likely a mistake - one that could cause serious program errors as conditions are evaluated under the wrong circumstances.

```csharp Bad theme={null} if (GetTrue() | GetFalse()) // Noncompliant: both sides evaluated { } ``` ```csharp Fix theme={null} if (GetTrue() || GetFalse()) // Compliant: short-circuit logic used { } ```

When concatenating strings, it is very easy to forget a whitespace.

In some scenarios this might cause runtime errors, one of which is while creating an SQL query via concatenation:

```csharp Bad theme={null} string select = "SELECT p.FirstName, p.LastName, p.PhoneNumber" + "FROM Person as p" + // Noncompliant: concatenation results in "p.PhoneNumberFROM" "WHERE p.Id = @Id"; // Noncompliant: concatenation results in "pWHERE" ``` ```csharp Fix theme={null} string select = "SELECT p.FirstName, p.LastName, p.PhoneNumber" + " FROM Person as p" + " WHERE p.Id = @Id"; ```

In order to produce a formatted string, both string.Create and either FormattableString.Invariant or FormattableString.CurrentCulture can be used. However, string.Create rents array buffers from ArrayPool\ making it more performant, as well as preventing unnecessary allocations and future stress on the Garbage Collector.

This applies to .NET versions after .NET 6, when these string.Create overloads were introduced.

```csharp Bad theme={null} string Interpolate(string value) => FormattableString.Invariant($"Value: {value}"); ``` ```csharp Fix theme={null} string Interpolate(string value) => FormattableString.CurrentCulture($"Value: {value}"); ```

Suppose you override Object.Equals in a type, you must also override Object.GetHashCode. If two objects are equal according to the Equals method, then calling GetHashCode on each of them must yield the same integer. If this is not the case, many collections, such as a Hashtable or a Dictionary won’t handle class instances correctly.

In order to not have unpredictable behavior, Equals and GetHashCode should be either both inherited, or both overridden.

```csharp Bad theme={null} class MyClass // Noncompliant: should also override GetHashCode { public override bool Equals(object obj) { // ... } } ``` ```csharp Fix theme={null} class MyClass { public override bool Equals(object obj) { // ... } public override int GetHashCode() { // ... } } ```

In the interests of readability, code that can be simplified should be simplified. To that end, there are several ways IEnumerable language integrated queries (LINQ) can be simplified. This not only improves readabilty but can also lead to improved performance.

```csharp Bad theme={null} public void Foo(IEnumerable seq, List list) { var result1 = seq.Select(x => x as Car).Any(x => x != null); // Noncompliant; use OfType var result2 = seq.Select(x => x as Car).Any(x => x != null && x.HasOwner); // Noncompliant; use OfType before calling Any var result3 = seq.Where(x => x is Car).Select(x => x as Car); // Noncompliant; use OfType var result4 = seq.Where(x => x is Car).Select(x => (Car)x); // Noncompliant; use OfType var result5 = seq.Where(x => x.HasOwner).Any(); // Noncompliant; use Any([predicate]) var num = list.Count(); // Noncompliant; use the Count property var arr = seq.ToList().ToArray(); // Noncompliant; ToList is not needed var count = seq.ToList().Count(x => x.HasOwner); // Noncompliant; ToList is not needed } ``` ```csharp Fix theme={null} public void Foo(IEnumerable seq, List list) { var result1 = seq.OfType().Any(); var result2 = seq.OfType().Any(x => x.HasOwner); var result3 = seq.OfType(); var result4 = seq.OfType(); var result5 = seq.Any(x => x.HasOwner); var num = list.Count; var arr = seq.ToArray(); var count = seq.Count(x => x.HasOwner); } ```

A method using the \`VarArgs calling convention is not Common Language Specification (CLS) compliant and might not be accessible across programming languages, while the params keyword works the same way and is CLS compliant.

This rule raises an issue when a public or protected type contains a public or protected method that uses the VarArgs\` calling convention.

```csharp Bad theme={null} using System; namespace MyLibrary { public class Foo { public void Bar(__arglist) // Noncompliant { ArgIterator argumentIterator = new ArgIterator(__arglist); for(int i = 0; i < argumentIterator.GetRemainingCount(); i++) { Console.WriteLine( __refvalue(argumentIterator.GetNextArg(), string)); } } } } ``` ```csharp Fix theme={null} using System; [assembly: CLSCompliant(true)] namespace MyLibrary { public class Foo { public void Bar(params string[] wordList) { for(int i = 0; i < wordList.Length; i++) { Console.WriteLine(wordList[i]); } } } } ```

The NET Framework 2.0 introduced the generic interface \`System.Collections.Generic.IEnumerable\ and it should be preferred over the older, non generic, interfaces.

This rule raises an issue when a public type implements System.Collections.IEnumerable\`.

```csharp Bad theme={null} using System; using System.Collections; public class MyData { public MyData() { } } public class MyList : CollectionBase // Noncompliant { public void Add(MyData data) { InnerList.Add(data); } // ... } ``` ```csharp Fix theme={null} using System; using System.Collections.ObjectModel; public class MyData { public MyData() { } } public class MyList : Collection { // Implementation... } ```

When division is performed on ints, the result will always be an int. You can assign that result to a double, float or decimal with automatic type conversion, but having started as an int, the result will likely not be what you expect. If the result of int division is assigned to a floating-point variable, precision will have been lost before the assignment. Instead, at least one operand should be cast or promoted to the final type before the operation takes place.

```csharp Bad theme={null} static void Main() { decimal dec = 3/2; // Noncompliant Method(3/2); // Noncompliant } static void Method(float f) { } ``` ```csharp Fix theme={null} static void Main() { decimal dec = (decimal)3/2; Method(3.0F/2); } static void Method(float f) { } ```

The rules for method resolution can be complex and may not be fully understood by all developers. The situation becomes even more challenging when dealing with method overloads that have optional parameter values.

This rule raises an issue when an overload with default parameter values is hidden by another overload that does not have the optional parameters.

```csharp Bad theme={null} MyClass.Print(1); // which overload of Print will be called? public static class MyClass { public static void Print(int number) { } public static void Print(int number, string delimiter = "\n") { } // Noncompliant, default parameter value is hidden by overload } ``` ```csharp Fix theme={null} MyClass.Print(1); // which overload of Print will be called? public static class MyClass { public static void Print(int number) { } public static void Print(int number, string delimiter = "\n") { } // Noncompliant: default parameter value is hidden by overload } ```

Locking on a class field synchronizes not on the field itself, but on the object assigned to it. Thus, there are some good practices to follow to avoid problems related to thread synchronization.

  • Locking on a non-readonly field makes it possible for the field’s value to change while a thread is in the code block, locked on the old value. This allows another thread to lock on the new value and access the same block concurrently.

```csharp Bad theme={null} private Color color = new Color("red"); private void DoSomething() { // Synchronizing access via "color" lock (color) // Noncompliant: lock is actually on object instance "red" referred to by the "color" field { //... color = new Color("green"); // other threads now allowed into this block // ... } } ``` ```csharp Fix theme={null} private void DoSomething() { lock (new object()) // Noncompliant: every thread locks on a different new instance { // ... } } ```

Invoking a method designed to return a string representation of an object which is already a string is a waste of keystrokes. Similarly, explicitly invoking \`ToString() when the compiler would do it implicitly is also needless code-bloat.

This rule raises an issue when ToString() is invoked:

  • on a string

  • on a non-string operand to concatenation

  • on an argument to string.Format\`

```csharp Bad theme={null} var s = "foo"; var t = "fee fie foe " + s.ToString(); // Noncompliant var someObject = new object(); var u = "" + someObject.ToString(); // Noncompliant var v = string.Format("{0}", someObject.ToString()); // Noncompliant ``` ```csharp Fix theme={null} var s = "foo"; var t = "fee fie foe " + s; var someObject = new object(); var u = "" + someObject; var v = string.Format("{0}", someObject); ```

When a loop is filtering, selecting or aggregating, those functions can be handled with a clearer, more concise LINQ expression instead.

```csharp Bad theme={null} var result = new List(); foreach (var element in collection) // Noncompliant { if (condition(element)) { result.Add(element); } } foreach (var element in collection2) // Noncompliant { var someValue = element.Property; if (someValue != null) { result.Add(someValue); } } ``` ```csharp Fix theme={null} var result = new List(); foreach (var element in collection.Where(x => condition(x))) { result.Add(element); } foreach (var someValue in collection2.Select(x => x.Property).Where(y => y != null)) { result.Add(someValue); } ```

Catching NullReferenceException is generally considered a bad practice because it can hide bugs in your code. Instead of catching this exception, you should aim to prevent it. This makes your code more robust and easier to understand. In addition, constantly catching and handling NullReferenceException can lead to performance issues. Exceptions are expensive in terms of system resources, so they should be used cautiously and only for exceptional conditions, not for regular control flow.

```csharp Bad theme={null} public int GetLengthPlusTwo(string str) { try { return str.Length + 2; } catch (NullReferenceException e) { return 2; } } ``` ```csharp Fix theme={null} public int GetLengthPlusTwo(string str) { if (str is null) { return 2; } return str.Length + 2; } ```

The difference between private and protected visibility is that child classes can see and use protected members, but they cannot see private ones. Since a sealed class cannot have children, marking its members protected is confusingly pointless.

```csharp Bad theme={null} public sealed class MySealedClass { protected string name = "Fred"; // Noncompliant protected void SetName(string name) // Noncompliant { // ... } } ``` ```csharp Fix theme={null} public sealed class MySealedClass { private string name = "Fred"; public void SetName(string name) { // ... } } ```

public static mutable fields of classes which are accessed directly should be protected to the degree possible. This can be done by reducing the accessibility of the field or by changing the return type to an immutable type.

This rule raises issues for public static fields with a type inheriting/implementing System.Array or System.Collections.Generic.ICollection\.

```csharp Bad theme={null} public class A { public static string[] strings1 = {"first","second"}; // Noncompliant public static List strings3 = new List(); // Noncompliant } ``` ```csharp Fix theme={null} public class A { protected static string[] strings1 = {"first","second"}; protected static List strings3 = new List(); } ```

In general, async void test methods are not executed by test frameworks, therefore it’s better to avoid them altogether.

```csharp Bad theme={null} [TestMethod] public async void MyIgnoredTestMethod() // Noncompliant { /* ... */ } ``` ```csharp Fix theme={null} ```

In ASP.NET Core MVC, the routing middleware utilizes a series of rules and conventions to identify the appropriate controller and action method to handle a specific HTTP request. This process, known as conventional routing, is generally established using the MapControllerRoute method. This method is typically configured in one central location for all controllers during the application setup.

Conversely, attribute routing allows routes to be defined at the controller or action method level. It is possible to mix both mechanisms. Although it’s permissible to employ diverse routing strategies across multiple controllers, combining both mechanisms within one controller can result in confusion and increased complexity, as illustrated below.

```csharp Bad theme={null} // Conventional mapping definition app.MapControllerRoute( name: "default", pattern: "{controller=Home}/{action=Index}/{id?}"); public class PersonController { // Conventional routing: // Matches e.g. /Person/Index/123 public IActionResult Index(int? id) => View(); // Attribute routing: // Matches e.g. /Age/Ascending (and model binds "Age" to sortBy and "Ascending" to direction) // but does not match /Person/List/Age/Ascending [HttpGet(template: "{sortBy}/{direction}")] public IActionResult List(string sortBy, SortOrder direction) => View(); } ``` ```csharp Fix theme={null} public class PersonController : Controller { // Matches /Person/Index/123 public IActionResult Index(int? id) => View(); // Matches /Age/Ascending [HttpGet(template: "{sortBy}/{direction}")] // Noncompliant: The "Index" and the "List" actions are // reachable via different routing mechanisms and routes public IActionResult List(string sortBy, SortOrder direction) => View(); } ```

Using string.Equals to determine if a string is empty is significantly slower than using string.IsNullOrEmpty() or checking for ++string.Length

```csharp Bad theme={null} "".Equals(name); // Noncompliant !name.Equals(""); // Noncompliant name.Equals(string.Empty); // Noncompliant ``` ```csharp Fix theme={null} name != null && name.Length > 0 // Compliant but more error prone !string.IsNullOrEmpty(name) string.IsNullOrEmpty(name) ```

In Blazor, the \[JSInvokable] attribute is used to annotate a method, enabling it to be invoked from JavaScript code. The prerequisite for this functionality is that the method must be declared as public. Otherwise, a runtime error will be triggered when an attempt is made to call the method from JavaScript.

```csharp Bad theme={null} @code { [JSInvokable] private static void MyStaticMethod() { } // Noncompliant [JSInvokable] internal void MyMethod() { } // Noncompliant } ``` ```csharp Fix theme={null} @code { [JSInvokable] public static void MyStaticMethod() { } // Compliant [JSInvokable] public void MyMethod() { } // Compliant } ```

A loop statement with at most one iteration is equivalent to an if statement; the following block is executed only once.

If the initial intention was to conditionally execute the block only once, an if statement should be used instead. If that was not the initial intention, the block of the loop should be fixed so the block is executed multiple times.

A loop statement with at most one iteration can happen when a statement unconditionally transfers control, such as a jump statement or a throw statement, is misplaced inside the loop block.

This rule raises when the following statements are misplaced:

```csharp Bad theme={null} public object Method(IEnumerable items) { for (int i = 0; i < 10; i++) { Console.WriteLine(i); break; // Noncompliant: loop only executes once } foreach (object item in items) { return item; // Noncompliant: loop only executes once } return null; } ``` ```csharp Fix theme={null} public object Method(IEnumerable items) { for (int i = 0; i < 10; i++) { Console.WriteLine(i); } var item = items.FirstOrDefault(); if (item != null) { return item; } return null; } ```

Needing to cast from an interface to a concrete type indicates that something is wrong with the abstractions in use, likely that something is missing from the interface. Instead of casting to a discrete type, the missing functionality should be added to the interface. Otherwise there is a risk of runtime exceptions.

```csharp Bad theme={null} public interface IMyInterface { void DoStuff(); } public class MyClass1 : IMyInterface { public int Data { get { return new Random().Next(); } } public void DoStuff() { // TODO... } } public static class DowncastExampleProgram { static void EntryPoint(IMyInterface interfaceRef) { MyClass1 class1 = (MyClass1)interfaceRef; // Noncompliant int privateData = class1.Data; class1 = interfaceRef as MyClass1; // Noncompliant if (class1 != null) { // ... } } } ``` ```csharp Fix theme={null} static void EntryPoint(IMyInterface interfaceRef) { var o = (object)interfaceRef; ... } ```

The purpose of an abstract class is to provide some heritable behaviors while also defining methods which must be implemented by sub-classes.

A \`class with no abstract methods that was made abstract purely to prevent instantiation should be converted to a concrete class (i.e. remove the abstract keyword) with a protected constructor.

A class with only abstract methods and no inheritable behavior should be converted to an interface\`.

```csharp Bad theme={null} public abstract class Animal //Noncompliant; should be an interface { abstract void Move(); abstract void Feed(); } public abstract class Color //Noncompliant; should be concrete with a protected constructor { private int red = 0; private int green = 0; private int blue = 0; public int GetRed() { return red; } } ``` ```csharp Fix theme={null} public interface Animal { void Move(); void Feed(); } public class Color { private int red = 0; private int green = 0; private int blue = 0; protected Color() {} public int GetRed() { return red; } } public abstract class Lamp { private bool switchLamp = false; public abstract void Glow(); public void FlipSwitch() { switchLamp = !switchLamp; if (switchLamp) { Glow(); } } } ```

The SupplyParameterFromQuery attribute can be used to specify that a component parameter, of a routable component, comes from the query string.

Component parameters supplied from the query string support the following types:

  • bool, DateTime, decimal, double, float, Guid, int, long, string.

  • Nullable variants of the preceding types.

  • Arrays of the preceding types, whether they’re nullable or not nullable.

Query parameters should have one of the supported types. Otherwise, an unhandled exception will be raised at runtime.

```csharp Bad theme={null} Unhandled exception rendering component: Querystring values cannot be parsed as type ''. System.NotSupportedException: Querystring values cannot be parsed as type '' ... ``` ```csharp Fix theme={null} @page "/print"

Parameter value is: @Value

@code { [Parameter] [SupplyParameterFromQuery()] public TimeSpan Value { get; set; } // Noncompliant } ```

When an assembly uses Windows Forms (classes and interfaces from the \`System.Windows.Forms namespace) its entry point should be marked with the STAThreadAttribute to indicate that the threading model should be "Single-Threaded Apartment" (STA) which is the only one supported by Windows Forms.

This rule raises an issue when the entry point (static void Main\` method) of an assembly using Windows Forms is not marked as STA.

```csharp Bad theme={null} using System; using System.Windows.Forms; namespace MyLibrary { public class MyForm: Form { public MyForm() { this.Text = "Hello World!"; } public static void Main() // Noncompliant { var form = new MyForm(); Application.Run(form); } } } ``` ```csharp Fix theme={null} using System; using System.Windows.Forms; namespace MyLibrary { public class MyForm: Form { public MyForm() { this.Text = "Hello World!"; } [STAThread] public static void Main() { var form = new MyForm(); Application.Run(form); } } } ```

In the interests of keeping code clean, the simplest possible conditional syntax should be used. That means

  • using the \`??= operator for a self-assign-if-not-null operation,

  • using the ?? operator for an assign-if-not-null operation, and

  • using the ternary operator ?:\` for assignment to a single variable.

```csharp Bad theme={null} object a = null, b = null, x; if (a != null) // Noncompliant; needlessly verbose { x = a; } else { x = b; } x = a != null ? a : b; // Noncompliant; better but could still be simplified x = (a == null) ? new object() : a; // Noncompliant if (condition) // Noncompliant { x = a; } else { x = b; } if (a == null) // Noncompliant a = new object(); var y = null ?? new object(); // Noncompliant a = a ?? new object(); // Noncompliant for C# 8 ``` ```csharp Fix theme={null} object x; x = a ?? b; x = a ?? b; x = a ?? new object(); x = condition ? a : b; a ??= new object(); var y = new object(); a ??= new object(); ```

Because composite format strings are interpreted at runtime, rather than validated by the compiler, they can contain errors that lead to unexpected behaviors or runtime errors. This rule statically validates the good behavior of composite formats when calling the methods of

  • \`String.Format

  • StringBuilder.AppendFormat

  • Console.Write

  • Console.WriteLine

  • TextWriter.Write

  • TextWriter.WriteLine

  • Debug.WriteLine(String, Object\[])

  • Trace.TraceError(String, Object\[])

  • Trace.TraceInformation(String, Object\[])

  • Trace.TraceWarning(String, Object\[])

  • TraceSource.TraceInformation(String, Object\[])\`.

```csharp Bad theme={null} s = string.Format("{0}", arg0, arg1); // Noncompliant, arg1 is declared but not used. s = string.Format("{0} {2}", arg0, arg1, arg2); // Noncompliant, the format item with index 1 is missing so arg1 will not be used. s = string.Format("value is " + value); // Noncompliant; use an argument instead of concatenaion s = string.Format("no argument here"); ``` ```csharp Fix theme={null} s = string.Format("{0}", arg0); s = string.Format("{0} {1}", arg0, arg2); s = string.Format("value is {0}", value); s = no argument here"; ```

Classes that declare an implementation of \`Serializable should provide a serializable constructor. Without such a constructor, you’ll be unable to deserialize the class.

Serialization constructors should be private for sealed types and protected\` otherwise.

```csharp Bad theme={null} [Serializable] public class Person : ISerializable { // Noncompliant; missing serializable constructor public void GetObjectData (SerializationInfo info, StreamingContext context) { // ... } } ``` ```csharp Fix theme={null} [Serializable] public class Person : ISerializable { // Noncompliant; missing serializable constructor protected Person (SerializationInfo info, StreamingContext context) { // ... } public void GetObjectData (SerializationInfo info, StreamingContext context) { // ... } } ```

Many string operations, the \`Compare and Equals methods in particular, provide an overload that accepts a StringComparison enumeration value as a parameter. Calling these overloads and explicitly providing this parameter makes your code clearer and easier to maintain.

This rule raises an issue when a string comparison operation doesn’t use the overload that takes a StringComparison\` parameter.

```csharp Bad theme={null} using System; namespace MyLibrary { public class Foo { public bool HaveSameNames(string name1, string name2) { return string.Compare(name1, name2) == 0; // Noncompliant } } } ``` ```csharp Fix theme={null} using System; namespace MyLibrary { public class Foo { public bool HaveSameNames(string name1, string name2) { return string.Compare(name1, name2, StringComparison.OrdinalIgnoreCase) == 0; } } } ```

Because \`IEnumerables are lazy-evaluated, each iteration causes a re-retrieval of the values, which could involve considerable overhead. For instance, when the IEnumerable is backed by a database, each iteration requires an additional round of database interactions. For that reason, any time the set represented by an IEnumerable must be iterated multiple times, it should first be converted to a List, which will retrieve the values and store them in memory. From that point they can be iterated as often as needed without an additional performance hit.

This rule raises an issue for each iteration of an IEnumerable\` after the first one.

```csharp Bad theme={null} IEnumerable numbers = GetNumbers(); var count = numbers.Count(); // causes an iteration var last = numbers.Last(); // Noncompliant; causes an iteration foreach(var x in numbers) // Noncompliant { // ... } ``` ```csharp Fix theme={null} List numbers = GetNumbers().ToList(); // iterable to your heart's content var count = numbers.Count(); var last = numbers.[count -1]; foreach(var x in numbers) { // ... } ```

Since the compiler will automatically invoke the base type’s no-argument constructor, there’s no need to specify its invocation explicitly. Also, when only a single public parameterless constructor is defined in a class, then that constructor can be removed because the compiler would generate it automatically. Similarly, empty static constructors and empty destructors are also wasted keystrokes.

```csharp Bad theme={null} class X { public X() { } // Noncompliant static X() { } // Noncompliant ~X() { } // Noncompliant ... } class Y : X { public Y(int parameter) : base() // Noncompliant { /* does something with the parameter */ } } ``` ```csharp Fix theme={null} class X { ... } class Y : X { public Y(int parameter) { /* does something with the parameter */ } } ```

The DebuggerDisplayAttribute is used to determine how an object is displayed in the debugger window.

The DebuggerDisplayAttribute constructor takes a single mandatory argument: the string to be displayed in the value column for instances of the type. Any text within curly braces is evaluated as the name of a field or property, or any complex expression containing method calls and operators.

Naming a non-existent member between curly braces will result in a CS0103 error in the debug window when debugging objects. Although there is no impact on the production code, providing a wrong value can lead to difficulties when debugging the application.

This rule raises an issue when text specified between curly braces refers to members that don’t exist in the current context.

```csharp Bad theme={null} [DebuggerDisplay("Name: {Name}")] // Noncompliant - Name doesn't exist in this context public class Person { public string FullName { get; private set; } } ``` ```csharp Fix theme={null} [DebuggerDisplay("Name: {FullName}")] public class Person { public string FullName { get; private set; } } ```

Usually IntPtr and UIntPtr fields are used to store pointers to unmanaged resources and the finalizer will free the unmanaged resource pointed to by the pointer fields. If the garbage collector finalises the object while the managed resources are still in use it could lead to serious, hard to diagnose bug. To prevent this from happening the object should be kept alive by calling GC.KeepAlive(this) in methods calling unmanaged code on this pointers.

```csharp Bad theme={null} using System; namespace MyLibrary { class Foo { private IntPtr res; Foo() { GetRes (res); } ~Foo() { FreeRes (res); } void Bar() // Noncompliant { UseRes(res); } // Methods that would typically make calls to unmanaged code. void GetRes(IntPtr p) { // Allocate the resource ... } void FreeRes(IntPtr p) { // Free the resource and set the pointer to null ... } void UseRes(IntPtr p) { // Use the resource in unmanaged code ... } } } ``` ```csharp Fix theme={null} using System; namespace MyLibrary { class Foo { private IntPtr res; Foo() { GetRes (res); } ~Foo() { FreeRes (res); } void Bar() { UseRes(res); GC.KeepAlive(this); } // Methods that would typically make calls to unmanaged code. void GetRes(IntPtr p) { // Allocate the resource ... } void FreeRes(IntPtr p) { // Free the resource and set the pointer to null ... } void UseRes(IntPtr p) { // Use the resource in unmanaged code ... } } } ```

The ISerializable interface gives you control over how your class is serialized, but does not itself make the class serializable. Such classes must also have the \[Serializable] attribute.

```csharp Bad theme={null} public class Person : ISerializable { // Noncompliant; [Serializable] attribute missing // ... } ``` ```csharp Fix theme={null} [Serializable] public class Person : ISerializable { // ... } ```

Using the equality == and inequality != operators to compare two objects generally works. The operators can be overloaded, and therefore the comparison can resolve to the appropriate method. However, when the operators are used on interface instances, then == resolves to reference equality, which may result in unexpected behavior if implementing classes override Equals. Similarly, when a class overrides Equals, but instances are compared with non-overloaded ==, there is a high chance that value comparison was meant instead of the reference one.

```csharp Bad theme={null} public interface IMyInterface { } public class MyClass : IMyInterface { public override bool Equals(object obj) { //... } } public class Program { public static void Method(IMyInterface instance1, IMyInterface instance2) { if (instance1 == instance2) // Noncompliant, will do reference equality check, but was that intended? MyClass overrides Equals. { Console.WriteLine("Equal"); } } } ``` ```csharp Fix theme={null} public interface IMyInterface { } public class MyClass : IMyInterface { public override bool Equals(object obj) { //... } } public class Program { public static void Method(IMyInterface instance1, IMyInterface instance2) { if (object.Equals(instance1, instance2)) // object.Equals checks for null and then calls the instance based Equals, so MyClass.Equals { Console.WriteLine("Equal"); } } } ```

A static field in a generic type is not shared among instances of different closed constructed types, thus LengthLimitedSingletonCollection\.instances and LengthLimitedSingletonCollection\.instances will point to different objects, even though instances is seemingly shared among all LengthLimitedSingletonCollection\<> generic classes.

If you need to have a static field shared among instances with different generic arguments, define a non-generic base class to store your static members, then set your generic type to inherit from the base class.

```csharp Bad theme={null} public class LengthLimitedSingletonCollection where T : new() { protected const int MaxAllowedLength = 5; protected static Dictionary instances = new Dictionary(); // Noncompliant public static T GetInstance() { object instance; if (!instances.TryGetValue(typeof(T), out instance)) { if (instances.Count >= MaxAllowedLength) { throw new Exception(); } instance = new T(); instances.Add(typeof(T), instance); } return (T)instance; } } ``` ```csharp Fix theme={null} public class SingletonCollectionBase { protected static Dictionary instances = new Dictionary(); } public class LengthLimitedSingletonCollection : SingletonCollectionBase where T : new() { protected const int MaxAllowedLength = 5; public static T GetInstance() { object instance; if (!instances.TryGetValue(typeof(T), out instance)) { if (instances.Count >= MaxAllowedLength) { throw new Exception(); } instance = new T(); instances.Add(typeof(T), instance); } return (T)instance; } } ```

Calling GetType() on a nullable value type object returns the underlying value type. Therefore, comparing the returned Type object to typeof(Nullable\) will either throw an NullReferenceException or the result will always be true or false and can be known at compile time.

```csharp Bad theme={null} void DoChecks(Nullable value) where T : struct { bool areEqual = value.GetType() == typeof(Nullable); // Noncompliant: always false bool areNotEqual = value.GetType() != typeof(Nullable); // Noncompliant: always true Nullable nullable = null; bool nullComparison = nullable.GetType() != typeof(Nullable); // Noncompliant: throws NullReferenceException } ``` ```csharp Fix theme={null} void DoChecks(Nullable value) where T : struct { bool areEqual = value.GetType() == typeof(int); // Compliant: can be true or false bool areNotEqual = value.GetType() != typeof(int); // Compliant: can be true or false Nullable nullable = null; bool nullComparison = nullable is not null && nullable.GetType() == typeof(int); // Compliant: does not throw NullReferenceException } ```

Enumerable.Sum() always executes addition in a checked context, so an OverflowException will be thrown if the value exceeds MaxValue, even if an unchecked context was specified. Therefore, using this method inside an unchecked context will only make the code more confusing, since the behavior will still be checked.

This rule raises an issue when an unchecked context is specified for a Sum on integer types.

```csharp Bad theme={null} void Add(List list) { unchecked { try { int total = list.Sum(); } catch (System.OverflowException e) { // Exception handling } } } ``` ```csharp Fix theme={null} void Add(List list) { int total1 = unchecked(list.Sum()); // Noncompliant unchecked { int total2 = list.Sum(); // Noncompliant } } ```

Calling GetType on a Type variable will always return the System.Type representation, which is equivalent to typeof(System.Type). This also applies to passing a Type argument to IsInstanceOfType which always returns false.

In both cases, the results are entirely predictable and should be avoided.

```csharp Bad theme={null} typeof(Type).GetType(); // Can be used by convention to get an instance of 'System.RuntimeType' ``` ```csharp Fix theme={null} void ExamineSystemType(string str) { Type stringType = str.GetType(); Type runtimeType = stringType.GetType(); // Noncompliant if (stringType.IsInstanceOfType(typeof(string))) // Noncompliant; will always return false { /* ... */ } } ```

When exceptions occur, it is usually a bad idea to simply ignore them. Instead, it is better to handle them properly, or at least to log them.

This rule only reports on empty catch clauses that catch generic `Exception`s.

```csharp Bad theme={null} string text = ""; try { text = File.ReadAllText(fileName); } catch (Exception exc) // Noncompliant { } ``` ```csharp Fix theme={null} string text = ""; try { text = File.ReadAllText(fileName); } catch (Exception exc) { logger.Log(exc); } ```

Strings and integral types are typically used as indexers. When some other type is required, it typically indicates design problems, and potentially a situation where a method should be used instead.

```csharp Bad theme={null} public int this[MyCustomClass index] // Noncompliant { // get and set accessors } ``` ```csharp Fix theme={null} ```

Because parameter names could be changed during refactoring, they should not be spelled out literally in strings. Instead, use \`nameof(), and the string that’s output will always be correct.

This rule raises an issue when a string in the throw\` statement contains the name of one of the method parameters.

```csharp Bad theme={null} void DoSomething(int someParameter, string anotherParam) { if (someParameter < 0) { throw new ArgumentException("Bad argument", "someParameter"); // Noncompliant } if (anotherParam == null) { throw new Exception("anotherParam should not be null"); // Noncompliant } } ``` ```csharp Fix theme={null} void DoSomething(int someParameter) { if (someParameter < 0) { throw new ArgumentException("Bad argument", nameof(someParameter)); } if (anotherParam == null) { throw new Exception($"{nameof(anotherParam)} should not be null"); } } ```

Catching System.Exception seems like an efficient way to handle multiple possible exceptions. Unfortunately, it traps all exception types, including the ones that were not intended to be caught. To prevent any misunderstandings, the exception filters should be used. Alternatively each exception type should be in a separate catch block.

```csharp Bad theme={null} try { // do something that might throw a FileNotFoundException or IOException } catch (Exception e) // Noncompliant { // log exception ... } ``` ```csharp Fix theme={null} try { // do something } catch (Exception e) when (e is FileNotFoundException || e is IOException) { // do something } ```

ASP.Net has a feature to validate HTTP requests to prevent potentially dangerous content to perform a cross-site scripting (XSS) attack. There is no reason to disable this mechanism even if other checks to prevent XXS attacks are in place.

This rule raises an issue if a method with parameters is marked with System.Web.Mvc.HttpPostAttribute and not System.Web.Mvc.ValidateInputAttribute(true).

```csharp Bad theme={null} public class FooBarController : Controller { [HttpPost] // Noncompliant [ValidateInput(false)] public ActionResult Purchase(string input) { return Foo(input); } [HttpPost] // Noncompliant public ActionResult PurchaseSomethingElse(string input) { return Foo(input); } } ``` ```csharp Fix theme={null} public class FooBarController : Controller { [HttpPost] [ValidateInput(true)] // Compliant public ActionResult Purchase(string input) { return Foo(input); } } ```

Using Thread.Sleep in a test might introduce unpredictable and inconsistent results depending on the environment. Furthermore, it will block the thread, which means the system resources are not being fully used.

```csharp Bad theme={null} [TestMethod] public void SomeTest() { Thread.Sleep(500); // Noncompliant // assertions... } ``` ```csharp Fix theme={null} [TestMethod] public async Task SomeTest() { await Task.Delay(500); // assertions... } ```

The Monitor.Pulse call releases the object on which it was called and wakes up the first thread waiting for the lock on that object. Significantly, it only releases one lock, and if multiple locks are held when it is called deadlocks could result.

```csharp Bad theme={null} public void doSomething(Object obj) { lock (this) //first lock { lock (obj) { // second lock // ... Monitor.Pulse(obj); // Noncompliant; only the second lock is released } } } ``` ```csharp Fix theme={null} ```

Creating a new Exception without actually throwing does not achieve the intended purpose.

```csharp Bad theme={null} if (x < 0) { new ArgumentException("x must be nonnegative"); } ``` ```csharp Fix theme={null} if (x < 0) { throw new ArgumentException("x must be nonnegative"); } ```

In the interests of making code as usable as possible, interfaces and delegates with generic parameters should use the \`out and in modifiers when possible to make the interfaces and delegates covariant and contravariant, respectively.

The out keyword can be used when the type parameter is used only as a return type in the interface or delegate. Doing so makes the parameter covariant, and allows interface and delegate instances created with a sub-type to be used as instances created with a base type. The most notable example of this is IEnumerable\, which allows the assignment of an IEnumerable\ instance to an IEnumerable\ variable, for instance.

The in keyword can be used when the type parameter is used only as a method parameter in the interface or a parameter in the delegate. Doing so makes the parameter contravariant, and allows interface and delegate instances created with a base type to be used as instances created with a sub-type. I.e. this is the inversion of covariance. The most notable example of this is the Action\ delegate, which allows the assignment of an Action\ instance to a Action\\` variable, for instance.

```csharp Bad theme={null} interface IConsumer // Noncompliant { bool Eat(T fruit); } ``` ```csharp Fix theme={null} interface IConsumer { bool Eat(T fruit); } ```

If you’re using a struct, it is likely because you’re interested in performance. But by failing to implement IEquatable\ you’re loosing performance when comparisons are made because without IEquatable\, boxing and reflection are used to make comparisons.

```csharp Bad theme={null} struct MyStruct // Noncompliant { public int Value { get; set; } } ``` ```csharp Fix theme={null} struct MyStruct : IEquatable { public int Value { get; set; } public bool Equals(MyStruct other) { // ... } } ```

Adding params to a method override has no effect. The compiler accepts it, but the callers won’t be able to benefit from the added modifier.

```csharp Bad theme={null} class Base { public virtual void Method(int[] numbers) { ... } } class Derived : Base { public override void Method(params int[] numbers) // Noncompliant, method can't be called with params syntax. { ... } } ``` ```csharp Fix theme={null} class Base { public virtual void Method(int[] numbers) { ... } } class Derived : Base { public override void Method(int[] numbers) { ... } } ```

An assertion is a piece of code that’s used during development when the compilation debug mode is activated. It allows a program to check itself as it runs. When an assertion is true, that means everything is operating as expected.

In non-debug mode, all Debug.Assert calls are automatically left out (via the Conditional("DEBUG") mechanism). So, by contract, the boolean expressions that are evaluated by those assertions must not contain any side effects. Otherwise, when leaving the debug mode, the functional behavior of the application is not the same anymore.

The rule will raise if the method name starts with any of the following remove, delete, add, pop, update, retain, insert, push, append, clear, dequeue, enqueue, dispose, put, or set, although SetEquals will be ignored.

```csharp Bad theme={null} Debug.Assert(list.Remove("dog")); ``` ```csharp Fix theme={null} bool result = list.Remove("dog"); Debug.Assert(result); ```

Unfortunately, it is possible to make constructor calls recursive. When this happens, you get a class that cannot be instantiated.

As a general rule, no constructor should make a call to another constructor in the same class that requires fewer arguments than the calling constructor received. I.e. the constructor that accepts the most arguments is the one that has the fullest picture of how the class should look. It should perform class initialization.

```csharp Bad theme={null} class Foo { int start; Foo() : this(0) { } Foo(int v) : this() { } // Noncompliant } ``` ```csharp Fix theme={null} class Foo { int start; Foo() : this(0) { } Foo(int v) { this.count = v; } } ```

Unnecessary keywords simply clutter the code and should be removed. Specifically:

  • \`partial on type declarations that are completely defined in one place

  • sealed on members of sealed classes

  • unsafe method or block inside construct already marked with unsafe, or when there are no unsafe constructs in the block

  • checked and unchecked\` blocks with no integral-type arithmetic operations

```csharp Bad theme={null} public partial class MyClass // Noncompliant { public virtual void Method() { } } public sealed class MyOtherClass : MyClass { public sealed override void Method() // Noncompliant { } } ``` ```csharp Fix theme={null} public class MyClass { public virtual void Method() { } } public sealed class MyOtherClass : MyClass { public override void Method() { } } ```

Certain bitwise operations are not needed and should not be performed because their results are predictable.

Specifically, using & -1 with any value always results in the original value.

That is because the binary representation of -1 on a integral numeric type supporting negative numbers, such as int or long, is based on two’s complement and made of all 1s: 0b111…​111.

Performing & between a value and 0b111…​111 means applying the & operator to each bit of the value and the bit 1, resulting in a value equal to the provided one, bit by bit.

```csharp Bad theme={null} anyValue & -1 // Noncompliant anyValue // Compliant ``` ```csharp Fix theme={null} anyValue | 0 // Noncompliant anyValue // Compliant ```

Certain mathematical comparisons will always return the same value, and should not be performed.

Specifically, the following comparisons will return either always true or always false depending on the kind of comparison:

  • comparing a char with a numeric constant that is outside of the range of char

  • comparing a float with a numeric constant that is outside of the range of float

  • comparing a long with a numeric constant that is outside of the range of long

  • comparing a ulong with a numeric constant that is outside of the range of ulong

  • etc.

```csharp Bad theme={null} float f = 42.0f; if (f <= double.MaxValue) { } // Noncompliant: always true if (f > double.MaxValue) { } // Noncompliant: always false ``` ```csharp Fix theme={null} ```

Calling ToString() on an object should always return a string. Thus, overriding the ToString method should never return null, as it breaks the method’s implicit contract, and as a result the consumer’s expectations.

```csharp Bad theme={null} public override string ToString () { if (this.collection.Count == 0) { return null; // Noncompliant } else { // ... } } ``` ```csharp Fix theme={null} public override string ToString () { if (this.collection.Count == 0) { return string.Empty; } else { // ... } } ```

The switch statement is a conditional statement that executes a sequence of instructions based on patterns matching the provided value.

```csharp Bad theme={null} switch (temperatureInCelsius) { case < 35.0: Console.WriteLine("Hypothermia"); break; case >= 36.5 and <= 37.5: Console.WriteLine("Normal"); break; case > 37.5 and <= 40.0: Console.WriteLine("Fever or hyperthermia"); break; case > 40.0: Console.WriteLine("Hyperpyrexia"); break; } ``` ```csharp Fix theme={null} switch (gradeLetter) { case "A+": case "A": case "A-": Console.WriteLine("Excellent"); break; case "B+": case "B": Console.WriteLine("Very Good"); break; case "B-": case "C+": Console.WriteLine("Good"); break; case "C": Console.WriteLine("Pass"); break; case "F": Console.WriteLine("Fail"); break; default: Console.WriteLine("Invalid grade letter!"); break; } ```

Transparency attributes can be declared at several levels. If two different attributes are declared at two different levels, the attribute that prevails is the one in the highest level. For example, you can declare that a class is SecuritySafeCritical and that a method of this class is SecurityCritical. In this case, the method will be SecuritySafeCritical and the SecurityCritical attribute attached to it is ignored.

```csharp Bad theme={null} using System; using System.Security; namespace MyLibrary { [SecuritySafeCritical] public class Foo { [SecurityCritical] // Noncompliant public void Bar() { } } } ``` ```csharp Fix theme={null} using System; using System.Security; namespace MyLibrary { public class Foo { [SecurityCritical] public void Bar() { } } } ```

When a private static method is only invoked by a nested class, there’s no reason not to move it into that class. It will still have the same access to the outer class' static members, but the outer class will be clearer and less cluttered.

```csharp Bad theme={null} public class Outer { private const int base = 42; private static void Print(int num) // Noncompliant - static method is only used by the nested class, should be moved there { Console.WriteLine(num + base); } public class Nested { public void SomeMethod() { Outer.Print(1); } } } ``` ```csharp Fix theme={null} public class Outer { private const int base = 42; public class Nested { public void SomeMethod() { Print(1); } private static void Print(int num) { Console.WriteLine(num + base); } } } ```

Properties are accessed like fields which makes them easier to use.

This rule raises an issue when the name of a public or protected method starts with Get, takes no parameter, and returns a value that is not an array.

```csharp Bad theme={null} using System; namespace MyLibrary { public class Foo { private string name; public string GetName() // Noncompliant { return name; } } } ``` ```csharp Fix theme={null} using System; namespace MyLibrary { public class Foo { private string name; public string Name { get { return name; } } } } ```

Synchronization can be expensive in terms of time when multiple threads need to pass through the same bottleneck (method with \`\[MethodImpl(MethodImplOptions.Synchronized)]).

If you have a piece of code calling a method with \[MethodImpl(MethodImplOptions.Synchronized)] attribute once, then it only has to wait its turn to pass through the bottleneck once. But call it in a loop, and your code has to get back in line for the bottleneck over and over.

Instead, it would be better to get into the bottleneck, and then do the looping. I.e. consider refactoring the code to perform the loop inside the method.

This rule raises an issue when a method with \[MethodImpl(MethodImplOptions.Synchronized)]\` is called in a loop.

```csharp Bad theme={null} public void doSomething(int max) { for (int i = 0; i < max; i++) { doSynchronized(i); // Noncompliant } } [MethodImpl(MethodImplOptions.Synchronized)] public void doSynchronized(int val) { // ... } ``` ```csharp Fix theme={null} ```

When you annotate a field with the ThreadStatic attribute, it is an indication that the value of this field is unique for each thread. But if you don’t mark the field as static, then the ThreadStatic attribute is ignored.

The ThreadStatic attribute should either be removed or replaced with the use of ThreadLocal\ class, which gives a similar behavior for non-static fields.

```csharp Bad theme={null} public class MyClass { [ThreadStatic] // Noncompliant private int count = 0; // ... } ``` ```csharp Fix theme={null} public class MyClass { private int count = 0; // ... } ```

Finalizers come with a performance cost due to the overhead of tracking the life cycle of objects. An empty one is consequently costly with no benefit or justification.

```csharp Bad theme={null} public class Foo { ~Foo() // Noncompliant { } } ``` ```csharp Fix theme={null} ```

Properties and Get method should have names that makes them clearly distinguishable.

This rule raises an issue when the name of a public or protected member starts with 'Get' and otherwise matches the name of a public or protected property.

```csharp Bad theme={null} using System; namespace MyLibrary { public class Foo { public DateTime Date { get { return DateTime.Today; } } public string GetDate() // Noncompliant { return this.Date.ToString(); } } } ``` ```csharp Fix theme={null} using System; namespace MyLibrary { public class Foo { public DateTime Date { get { return DateTime.Today; } } public string GetDateAsString() { return this.Date.ToString(); } } } ```

In C#, delegates can be added together to chain their execution, and subtracted to remove their execution from the chain.

Subtracting a chain of delegates from another one might yield unexpected results as shown hereunder - and is likely to be a bug.

```csharp Bad theme={null} MyDelegate first, second, third, fourth; first = () => Console.Write("1"); second = () => Console.Write("2"); third = () => Console.Write("3"); fourth = () => Console.Write("4"); MyDelegate chain1234 = first + second + third + fourth; // Compliant - chain sequence = "1234" MyDelegate chain12 = chain1234 - third - fourth; // Compliant - chain sequence = "12" MyDelegate chain14 = first + fourth; // creates a new MyDelegate instance which is a list under the covers MyDelegate chain23 = chain1234 - chain14; // Noncompliant; (first + fourth) doesn't exist in chain1234 // The chain sequence of "chain23" will be "1234" instead of "23"! // Indeed, the sequence "1234" does not contain the subsequence "14", so nothing is subtracted // (but note that "1234" contains both the "1" and "4" subsequences) chain23 = chain1234 - (first + fourth); // Noncompliant chain23(); // will print "1234"! ``` ```csharp Fix theme={null} MyDelegate chain23 = chain1234 - first - fourth; // Compliant - "1" is first removed, followed by "4" chain23(); // will print "23" ```

Once you modify a closure, any use of it could provide unexpected results.

```csharp Bad theme={null} var x = 0; Func f1 = () => x; // Noncompliant x = 1; Console.WriteLine(f1()); var input = new[] { 1, 2, 3 }; var fs = new List>(); for (var i = 0; i < input.Length; i++) { Func f = () => input[i]; // Noncompliant fs.Add(f); } Console.WriteLine(fs[0]()); //Access to modified closure yields Exception ``` ```csharp Fix theme={null} var x = 0; var xx = x; Func f = () => xx; x = 1; Console.WriteLine(f()); var input = new[] { 1, 2, 3 }; var fs = new List>(); for (var i = 0; i < input.Length; i++) { var ii = i; Func f = () => input[ii]; fs.Add(f); } Console.WriteLine(fs[0]()); ```

When a \`System.Globalization.CultureInfo or IFormatProvider object is not supplied, the default value that is supplied by the overloaded member might not have the effect that you want in all locales.

You should supply culture-specific information according to the following guidelines:

  • If the value will be displayed to the user, use the current culture. See CultureInfo.CurrentCulture.

  • If the value will be stored and accessed by software (persisted to a file or database), use the invariant culture. See CultureInfo.InvariantCulture.

  • If you do not know the destination of the value, have the data consumer or provider specify the culture.

This rule raises an issue when a method or constructor calls one or more members that have overloads that accept a System.IFormatProvider parameter, and the method or constructor does not call the overload that takes the IFormatProvider parameter. This rule ignores calls to .NET Framework methods that are documented as ignoring the IFormatProvider parameter as well as the following methods:

  • Activator.CreateInstance

  • ResourceManager.GetObject

  • ResourceManager.GetString\`

```csharp Bad theme={null} using System; namespace MyLibrary { public class Foo { public void Bar(String string1) { if(string.Compare(string1, string2, false) == 0) // Noncompliant { Console.WriteLine(string3.ToLower()); // Noncompliant } } } } ``` ```csharp Fix theme={null} using System; using System.Globalization; namespace MyLibrary { public class Foo { public void Bar(String string1, String string2, String string3) { if(string.Compare(string1, string2, false, CultureInfo.InvariantCulture) == 0) { Console.WriteLine(string3.ToLower(CultureInfo.CurrentCulture)); } } } } ```

In C#, the Object.ReferenceEquals method is used to compare two reference type variables. If you use this method to compare two value types, such as int, float, or bool you will not get the expected results because value type variables contain an instance of the type and not a reference to it.

Due to value type variables containing directly an instance of the type, they can’t have the same reference, and using Object.ReferenceEquals to compare them will always return false even if the compared variables have the same value.

```csharp Bad theme={null} using System; struct MyStruct { int valueA; int valueB; } static class MyClass { public static void Method(MyStruct struct1, MyStruct struct2) { if (Object.ReferenceEquals(struct1, struct2)) // Noncompliant: this will be always false { // ... } } } ``` ```csharp Fix theme={null} using System; struct MyStruct : IEquatable { int valueA; int valueB; public bool Equals(MyStruct other) => valueA == other.valueA && valueB == other.valueB; public override bool Equals(object obj) => obj is MyStruct other && Equals(other); public override int GetHashCode() => HashCode.Combine(valueA, valueB); public static bool operator ==(MyStruct lhs, MyStruct rhs) => lhs.Equals(rhs); public static bool operator !=(MyStruct lhs, MyStruct rhs) => !(lhs == rhs); } static class MyClass { public static void Method(MyStruct struct1, MyStruct struct2) { if (struct1.Equals(struct2)) // Compliant: value are compared { // ... } } } ```

After an \`awaited Task has executed, you can continue execution in the original, calling thread or any arbitrary thread. Unless the rest of the code needs the context from which the Task was spawned, Task.ConfigureAwait(false) should be used to keep execution in the Task thread to avoid the need for context switching and the possibility of deadlocks.

This rule raises an issue when code in a class library targeting .Net Framework awaits a Task and continues execution in the original calling thread.

The rule does not raise for .Net Core libraries as there is no SynchronizationContext\` in .Net Core.

```csharp Bad theme={null} var response = await httpClient.GetAsync(url); // Noncompliant ``` ```csharp Fix theme={null} var response = await httpClient.GetAsync(url).ConfigureAwait(false); ```

When you create a \`DataTable or DataSet, you should set the locale explicitly. By default, the locale for these types is the current culture. For data that is stored in a database or file and is shared globally, the locale should ordinarily be set to the invariant culture (CultureInfo.InvariantCulture).

This rule raises an issue when System.Data.DataTable or System.Data.DataSet instances are created without explicitly setting the locale property (DataTable.Locale or DataSet.Locale\`).

```csharp Bad theme={null} using System; using System.Data; namespace MyLibrary { public class Foo { public DataTable CreateTable() { DataTable table = new DataTable("Customers"); // Noncompliant table.Locale not set DataColumn key = table.Columns.Add("ID", typeof(Int32)); key.AllowDBNull = false; key.Unique = true; table.Columns.Add("LastName", typeof(String)); table.Columns.Add("FirstName", typeof(String)); return table; } } } ``` ```csharp Fix theme={null} using System; using System.Data; using System.Globalization; namespace MyLibrary { public class Foo { public DataTable CreateTable() { DataTable table = new DataTable("Customers"); table.Locale = CultureInfo.InvariantCulture; DataColumn key = table.Columns.Add("ID", typeof(Int32)); key.AllowDBNull = false; key.Unique = true; table.Columns.Add("LastName", typeof(String)); table.Columns.Add("FirstName", typeof(String)); return table; } } } ```

Because the is operator performs a cast if the object is not null, using is to check type and then casting the same argument to that type, necessarily performs two casts. The same result can be achieved more efficiently with a single cast using as, followed by a null-check.

```csharp Bad theme={null} if (x is Fruit) // Noncompliant { var f = (Fruit)x; // or x as Fruit // ... } ``` ```csharp Fix theme={null} // C# 6 var f = x as Fruit; if (f != null) { // ... } // C# 7 if (x is Fruit fruit) { // ... } ```

Numbers can be shifted with the \<\[/code> and \> operators, but the right operand of the operation needs to be an int or a type that has an implicit conversion to int. However, when the left operand is dynamic, the compiler’s type checking is turned off, so you can pass anything to the right of a shift operator and have it compile. And if the argument can’t be implicitly converted to int at runtime, then a RuntimeBinderException will be raised.

```csharp Bad theme={null} dynamic d = 5; var x = d >> 5.4; // Noncompliant x = d << null; // Noncompliant x <<= new object(); // Noncompliant ``` ```csharp Fix theme={null} ```

Using the same value on both sides of certain operators is a code defect. In the case of logical operators, it is either a copy/paste error and, therefore, a bug, or it is simply duplicated code and should be simplified. For bitwise operators and most binary mathematical operators, having the same value on both sides of an operator yields predictable results and should be simplified as well to avoid further code defects.

This rule raises for the following operators.

```csharp Bad theme={null} if ( a == a ) // always true { doZ(); } if ( a != a ) // always false { doY(); } if ( a == b && a == b ) // if the first one is true, the second one is too { doX(); } if ( a == b || a == b ) // if the first one is true, the second one is too { doW(); } int j = 5 / 5; // always 1 int k = 5 - 5; // always 0 c.Equals(c); // always true Object.Equals(c, c); // always true ``` ```csharp Fix theme={null} ```

Floating point numbers in C# (and in most other programming languages) are not precise. They are a binary approximation of the actual value. This means that even if two floating point numbers appear to be equal, they might not be due to the tiny differences in their binary representation.

Even simple floating point assignments are not simple:

```csharp Bad theme={null} float f = 0.100000001f; // 0.1 double d = 0.10000000000000001; // 0.1 ``` ```csharp Fix theme={null} float myNumber = 3.146f; if (myNumber == 3.146f) //Noncompliant: due to floating point imprecision, this will likely be false { // ... } if (myNumber < 4 || myNumber > 4) // Noncompliant: indirect inequality test { // ... } ```

An async method with a void return type does not follow the task asynchronous programming (TAP) model since the return type should be Task or Task\

Doing so prevents control over the asynchronous execution, such as:

  • waiting for the execution to complete

  • catching any exception that might occur during execution

  • testing execution behavior

```csharp Bad theme={null} public async void button1_Click(object sender, EventArgs e) { await DoSomethingAsync(); } ``` ```csharp Fix theme={null} public async void OnClick(EventContext data) { await DoSomethingAsync(); } ```

The \`IEquatable\ interface has only one method in it: Equals(\). If you’ve already written Equals(T), there’s no reason not to explicitly implement IEquatable\. Doing so expands the utility of your class by allowing it to be used where an IEquatable is called for.

Note: Classes that implement IEquatable\ should also be sealed\`.

```csharp Bad theme={null} class MyClass // Noncompliant { public bool Equals(MyClass other) { //... } } ``` ```csharp Fix theme={null} sealed class MyClass : IEquatable { public override bool Equals(object other) { return Equals(other as MyClass); } public bool Equals(MyClass other) { //... } } ```

A chain of if/else if statements is evaluated from top to bottom. At most, only one branch will be executed: the first statement with a condition that evaluates to true. Therefore, duplicating a condition leads to unreachable code inside the duplicated condition block. Usually, this is due to a copy/paste error.

The result of such duplication can lead to unreachable code or even to unexpected behavior.

```csharp Bad theme={null} if (param == 1) { OpenWindow(); } else if (param == 2) { CloseWindow(); } else if (param == 1) // Noncompliant: condition has already been checked { MoveWindowToTheBackground(); // unreachable code } ``` ```csharp Fix theme={null} if (param == 1) { OpenWindow(); } else if (param == 2) { CloseWindow(); } else if (param == 3) { MoveWindowToTheBackground(); } ```

Having an infinite loop or recursion will lead to a program failure or a program never finishing the execution.

```csharp Bad theme={null} public int Sum() { var i = 0; var result = 0; while (true) // Noncompliant: the program will never stop { result += i; i++; } return result; } ``` ```csharp Fix theme={null} int Pow(int num, int exponent) { return num * Pow(num, exponent - 1); // Noncompliant: no condition under which Pow isn't re-called } ```

String literals embedded in the source code will not be localized properly.

This rule raises an issue when a literal string is passed as a parameter or property and one or more of the following cases is true:

  • The \`LocalizableAttribute attribute of the parameter or property is set to true.

  • The parameter or property name contains "Text", "Message", or "Caption".

  • The name of the string parameter that is passed to a Console.Write or Console.WriteLine\` method is either "value" or "format".

```csharp Bad theme={null} using System; using System.Globalization; using System.Reflection; using System.Windows.Forms; [assembly: NeutralResourcesLanguageAttribute("en-US")] namespace MyLibrary { public class Foo { public void SetHour(int hour) { if (hour < 0 || hour > 23) { MessageBox.Show("The valid range is 0 - 23."); // Noncompliant } } } } ``` ```csharp Fix theme={null} using System; using System.Globalization; using System.Reflection; using System.Resources; using System.Windows.Forms; [assembly: NeutralResourcesLanguageAttribute("en-US")] namespace MyLibrary { public class Foo { ResourceManager rm; public Foo() { rm = new ResourceManager("en-US", Assembly.GetExecutingAssembly()); } public void SetHour(int hour) { if (hour < 0 || hour > 23) { MessageBox.Show( rm.GetString("OutOfRangeMessage", CultureInfo.CurrentUICulture)); } } } } ```

A jagged array is an array whose elements are arrays. It is recommended over a multidimensional array because the arrays that make up the elements can be of different sizes, which avoids wasting memory space.

```csharp Bad theme={null} int [,] myArray = // Noncompliant { {1,2,3,4}, {5,6,7,0}, {8,0,0,0}, {9,0,0,0} }; // ... myArray[1,1] = 0; ``` ```csharp Fix theme={null} int[][] myArray = { new int[] {1,2,3,4}, new int[] {5,6,7}, new int[] {8}, new int[] {9} }; // ... myArray[1][1] = 0; ```

There’s no point in chaining multiple \`OrderBy calls in a LINQ; only the last one will be reflected in the result because each subsequent call completely reorders the list. Thus, calling OrderBy multiple times is a performance issue as well, because all of the sorting will be executed, but only the result of the last sort will be kept.

Instead, use ThenBy\` for each call after the first.

```csharp Bad theme={null} var x = personList .OrderBy(person => person.Age) .OrderBy(person => person.Name) // Noncompliant .ToList(); // x is sorted by Name, not sub-sorted ``` ```csharp Fix theme={null} var x = personList .OrderBy(person => person.Age) .ThenBy(person => person.Name) .ToList(); ```

The use of ref or out in combination with Optional attribute is both confusing and contradictory. \[Optional] indicates that the parameter doesn’t have to be provided, while out and ref mean that the parameter will be used to return data to the caller (ref additionally indicates that the parameter may also be used to pass data into the method).

Thus, making it \[Optional] to provide the parameter in which you will be passing back the method results doesn’t make sense. In fact, the compiler will raise an error on such code. Unfortunately, it raises the error on method calls where the \[Optional] parameter has been omitted, not the source of the problem, the method declaration.

```csharp Bad theme={null} class MyClass { public void DoStuff([Optional] ref int i) // Noncompliant { Console.WriteLine(i); } public static void Main() { new MyClass().DoStuff(); // Compilation Error [CS7036] } } ``` ```csharp Fix theme={null} class MyClass { public void DoStuff(ref int i) { Console.WriteLine(i); } public static void Main() { var i = 42; new MyClass().DoStuff(ref i); } } ```

Shared naming conventions allow teams to collaborate efficiently.

This rule raises an issue when a type name is not PascalCased.

For example, the classes

```csharp Bad theme={null} class my_class {...} class SOMEName42 {...} ``` ```csharp Fix theme={null} class MyClass {...} class SomeName42 {...} ```

DefaultValue does not make the compiler set the default value, as its name may suggest. What you probably wanted to use is DefaultParameterValue.

The DefaultValue attribute from the System.ComponentModel namespace, is sometimes used to declare a member’s default value. This can be used, for instance, by the reset feature of a visual designer or by a code generator.

```csharp Bad theme={null} public void DoStuff([DefaultValue(4)] int i) { // i is not automatically assigned 4 } ``` ```csharp Fix theme={null} public void DoStuff([Optional] int i) { // i would be assigned default(int) = 0 } ```

Field-like events are events that do not have explicit add and remove accessors.

```csharp Bad theme={null} public event EventHandler MyEvent; // No add and remove accessors ``` ```csharp Fix theme={null} abstract class Car { public virtual event EventHandler OnRefuel; // Noncompliant public void Refuel() { // This OnRefuel will always be null if (OnRefuel != null) { OnRefuel(this, EventArgs.Empty); } } } class R2 : Car { public override event EventHandler OnRefuel; } class Program { static void Main(string[] args) { var r2 = new R2(); r2.OnRefuel += (o, a) => { Console.WriteLine("This event will be called"); }; r2.Refuel(); } } ```

The compiler automatically initializes class fields, auto-properties and events to their default values before setting them with any initialization values, so there is no need to explicitly set a member to its default value. Further, under the logic that cleaner code is better code, it’s considered poor style to do so.

```csharp Bad theme={null} class X { public int field = 0; // Noncompliant public object o = null; // Noncompliant public object MyProperty { get; set; } = null; // Noncompliant public event EventHandler MyEvent = null; // Noncompliant } ``` ```csharp Fix theme={null} class X { public int field; public object o; public object MyProperty { get; set; } public event EventHandler MyEvent; } ```

An inheritance list entry is redundant if:

  • It is \`Object - all classes extend Object implicitly.

  • It is int for an enum\`

  • It is a base class of another listed inheritance.

Such redundant declarations should be removed because they needlessly clutter the code and can be confusing.

```csharp Bad theme={null} public class MyClass : Object // Noncompliant enum MyEnum : int // Noncompliant ``` ```csharp Fix theme={null} public class MyClass enum MyEnum ```

The parameter to \`Assembly.Load includes the full specification of the dll to be loaded. Use another method, and you might end up with a dll other than the one you expected.

This rule raises an issue when Assembly.LoadFrom, Assembly.LoadFile, or Assembly.LoadWithPartialName\` is called.

```csharp Bad theme={null} static void Main(string[] args) { Assembly.LoadFrom(...); // Noncompliant Assembly.LoadFile(...); // Noncompliant Assembly.LoadWithPartialName(...); // Noncompliant + deprecated } ``` ```csharp Fix theme={null} ```

This rule addresses the issue of incomplete assertions that can occur when using certain test frameworks. Incomplete assertions can lead to tests that do not effectively verify anything. The rule enforces the use of complete assertions in specific cases, namely:

  • Fluent Assertions: Should() is not followed by an assertion invocation.

```csharp Bad theme={null} string actual = "Using Fluent Assertions"; actual.Should(); // Noncompliant ``` ```csharp Fix theme={null} string actual = "Using NFluent"; Check.That(actual); // Noncompliant ```

Debug statements are always useful during development. But include them in production code - particularly in code that runs client-side - and you run the risk of inadvertently exposing sensitive information.

```csharp Bad theme={null} private void DoSomething() { // ... Console.WriteLine("so far, so good..."); // Noncompliant // ... } ``` ```csharp Fix theme={null} ```

In Blazor, when a route parameter constraint is applied, the value is automatically cast to the corresponding component parameter type. If the constraint type does not match the component parameter type, it can lead to confusion and potential runtime errors due to unsuccessful casting. Therefore, it is crucial to ensure that the types of route parameters and component parameters match to prevent such issues and maintain code clarity.

```csharp Bad theme={null} @page "/my-route/{Param:datetime}" @code { [Parameter] public string Param { get; set; } // Noncompliant } ``` ```csharp Fix theme={null} @page "/my-route/{Param:datetime}" @code { [Parameter] public DateTime Param { get; set; } // Compliant } ```

\`switch statements and expressions are useful when there are many different cases depending on the value of the same expression.

When a switch statement or expression is simple enough, the code will be more readable with a single if, if-else\` or ternary conditional operator.

```csharp Bad theme={null} switch (variable) { case 0: doSomething(); break; default: doSomethingElse(); break; } var foo = variable switch { 0 => doSomething(), _ => doSomethingElse(), } ``` ```csharp Fix theme={null} if (variable == 0) { doSomething(); } else { doSomethingElse(); } var foo = variable == 0 ? doSomething() : doSomethingElse(); ```

There’s no point in having a public member in a non-public type because objects that can’t access the type will never have the chance to access the member.

This rule raises an issue when a type has methods, fields, or inner types with higher visibility than the type itself has.

```csharp Bad theme={null} internal class MyClass { public static decimal PI = 3.14m; // Noncompliant public int GetOne() // Noncompliant { return 1; } protected record NestedType // Noncompliant: outer class is internal { public bool FlipCoin() // Noncompliant: outer class is internal { return false; } // ... } } ``` ```csharp Fix theme={null} public class MyClass // Class visibility upgrade makes members compliant { public static decimal PI = 3.14m; public int GetOne() { return 1; } protected record NestedType { public bool FlipCoin() // Outer type is public { return false; } // ... } } ```

The abstract modifier in a class declaration is used to indicate that a class is intended only to be a base class of other classes, not instantiated on its own.

Since abstract classes cannot be instantiated, there is no need for public or internal constructors. If there is basic initialization logic that should run when an extending class instance is created, you can add it in a private, private protected or protected constructor.

```csharp Bad theme={null} abstract class Base { public Base() // Noncompliant: should be private, private protected or protected. { //... } } ``` ```csharp Fix theme={null} abstract class Base { protected Base() { //... } } ```

When working with anonymous functions, it is important to keep in mind that each time you create one, it is a completely new instance.

In this example, even though the same lambda expression is used, the expressions are stored separately in the memory and are therefore not equal or the same.

```csharp Bad theme={null} Func lambda1 = x => x + 1; Func lambda2 = x => x + 1; var result = lambda1 == lambda2; // result is false here ``` ```csharp Fix theme={null} event EventHandler myEvent; void DoWork() { myEvent += (s, e) => Console.WriteLine($"Event raised with sender {s} and arguments {e}!"); // ... myEvent -= (s, e) => Console.WriteLine($"Event raised with sender {s} and arguments {e}!"); // Noncompliant: this callback was never subscribed } ```

The value of a \`static readonly field is computed at runtime while the value of a const field is calculated at compile time, which improves performance.

This rule raises an issue when a static readonly\` field is initialized with a value that is computable at compile time.

As specified by Microsoft, the list of types that can have a constant value are:

C# type .Net Fwk type

bool

System.Boolean

byte

System.Byte

sbyte

System.SByte

char

System.Char

decimal

System.Decimal

double

System.Double

float

System.Single

int

System.Int32

uint

System.UInt32

long

System.Int64

ulong

System.UInt64

short

System.Int16

ushort

System.UInt16

string

System.String
```csharp Bad theme={null} namespace myLib { public class Foo { static readonly int x = 1; // Noncompliant static readonly int y = x + 4; // Noncompliant static readonly string s = "Bar"; // Noncompliant } } ``` ```csharp Fix theme={null} namespace myLib { public class Foo { const int x = 1; const int y = x + 4; const string s = "Bar"; } } ```

Non-encoded control characters and whitespace characters are often injected in the source code because of a bad manipulation. They are either invisible or difficult to recognize, which can result in bugs when the string is not what the developer expects. If you actually need to use a control character use their encoded version:

  • ASCII, for example \n and \t

  • Unicode, for example U+000D and U+0009

This rule raises an issue when the following characters are seen in a string literal:

```csharp Bad theme={null} string tabInside = "A B"; // Noncompliant: contains a tabulation string zeroWidthSpaceInside = "foo​bar"; // Noncompliant: contains a U+200B character inside Console.WriteLine(zeroWidthSpaceInside); // Prints "foo?bar" ``` ```csharp Fix theme={null} string tabInside = "A\tB"; // Compliant: escaped value string zeroWidthSpaceInside = "foo\u200Bbar"; // Compliant: escaped value Console.WriteLine(zeroWidthSpaceInside); // Prints "foo?bar" ```

A method is identified as a test method if it is marked with one of the following attributes:

  • \[TestMethod] or \[DataTestMethod] (for MSTest).

  • \[Fact] or \[Theory] (for xUnit).

  • \[Test], \[TestCase], \[TestCaseSource], or \[Theory] (for NUnit).

However, non-public methods are not considered test methods and will not be executed, regardless of whether they have a test attribute. Additionally, methods with the async void modifier or methods that contain generics \ anywhere in their signatures are also excluded from being recognized as tests and will not be executed.

```csharp Bad theme={null} [TestMethod] void TestNullArg() // Noncompliant, method is not public { /* ... */ } [TestMethod] public async void MyIgnoredTestMethod() // Noncompliant, this is an 'async void' method { /* ... */ } [TestMethod] public void MyIgnoredGenericTestMethod(T foo) // Noncompliant, method has generics in its signature { /* ... */ } ``` ```csharp Fix theme={null} [TestMethod] public void TestNullArg() { /* ... */ } [TestMethod] public async Task MyIgnoredTestMethod() { /* ... */ } [TestMethod] public void MyIgnoredGenericTestMethod(int foo) { /* ... */ } ```

In C#, the throw statement can be used in two different ways:

  • by specifying an expression

  • without specifying an expression

```csharp Bad theme={null} try { } catch(Exception exception) { // code that uses the exception throw exception; // The exception stack trace is cleared up to this point. } ``` ```csharp Fix theme={null} try { } catch(Exception exception) { // code that uses the exception throw; // The stack trace of the initial exception is preserved. } ```

The size of a collection and the length of an array are always greater than or equal to zero. Testing it doesn’t make sense, since the result is always true.

```csharp Bad theme={null} if(collection.Count >= 0){...} // Noncompliant: always true if(array.Length >= 0){...} // Noncompliant: always true ``` ```csharp Fix theme={null} if(enumerable.Count() < 0){...} // Noncompliant: always false ```

With the advent of .NET framework version 2, certain practices have become obsolete.

In particular, exceptions should now extend \`System.Exception instead of System.ApplicationException. Similarly, generic collections should be used instead of the older, non-generic, ones. Finally when creating an XML view, you should not extend System.Xml.XmlDocument.

This rule raises an issue when an externally visible type extends one of these types:

  • System.ApplicationException

  • System.Xml.XmlDocument

  • System.Collections.CollectionBase

  • System.Collections.DictionaryBase

  • System.Collections.Queue

  • System.Collections.ReadOnlyCollectionBase

  • System.Collections.SortedList

  • System.Collections.Stack\`

```csharp Bad theme={null} using System; using System.Collections; namespace MyLibrary { public class MyCollection : CollectionBase // Noncompliant { } } ``` ```csharp Fix theme={null} using System; using System.Collections; namespace MyLibrary { public class MyCollection : Collection { } } ```

Re-assigning a variable to itself is a defect as it has no actual effect and indicates meaning to do something else. It usually means that:

  • The statement is redundant and should be removed

  • The re-assignment is a mistake, and another value or variable was intended for the assignment instead

```csharp Bad theme={null} public class Choice { private bool selected; public void MakeChoice(bool selected) { selected = selected; // Noncompliant } } ``` ```csharp Fix theme={null} public class Choice { private bool selected; public void MakeChoice(bool selected) { this.selected = selected; // Compliant } } ```

GC.Collect is a method that forces or suggests to the garbage collector to run a collection of objects in the managed heap that are no longer being used and free their memory.

Calling GC.Collect is rarely necessary and can significantly affect application performance. That’s because it is a tracing garbage collector and needs to examine every object in memory for cleanup and analyze all reachable objects from every application’s root (static fields, local variables on thread stacks, etc.).

To perform tracing and memory releasing correctly, the garbage collection may need to block all threads currently in execution. That is why, as a general rule, the performance implications of calling GC.Collect far outweigh the benefits.

This rule raises an issue when any overload of Collect is invoked.

```csharp Bad theme={null} static void Main(string[] args) { // ... GC.Collect(); // Noncompliant GC.Collect(2, GCCollectionMode.Optimized); // Noncompliant } ``` ```csharp Fix theme={null} ```

When you implement \`IComparable or IComparable\ on a class you should also override Equals(object) and overload the comparison operators (==, !=, \<, \<=, >, >=). That’s because the CLR cannot automatically call your CompareTo implementation from Equals(object) or from the base comparison operator implementations. Additionally, it is best practice to override GetHashCode along with Equals.

This rule raises an issue when a class implements IComparable without also overriding Equals(object)\` and the comparison operators.

```csharp Bad theme={null} public class Foo: IComparable // Noncompliant { public int CompareTo(object obj) { /* ... */ } } ``` ```csharp Fix theme={null} public class Foo: IComparable { public int CompareTo(object obj) { /* ... */ } public override bool Equals(object obj) { var other = obj as Foo; if (object.ReferenceEquals(other, null)) { return false; } return this.CompareTo(other) == 0; } public int GetHashCode() { /* ... */ } public static bool operator == (Foo left, Foo right) { if (object.ReferenceEquals(left, null)) { return object.ReferenceEquals(right, null); } return left.Equals(right); } public static bool operator > (Foo left, Foo right) { return Compare(left, right) > 0; } public static bool operator < (Foo left, Foo right) { return Compare(left, right) < 0; } public static bool operator != (Foo left, Foo right) { return !(left == right); } } ```

Default arguments are determined by the static type of the object.

```csharp Bad theme={null} class Base { public virtual void Run(int distance = 42) { /* ... */ } } class Derived : Base { public override void Run(int distance = 5) { /* ... */ } } Base x = new Base(); x.Run(); // Here the default value of distance is 42 Derived d = new Derived(); d.Run(); // Here the default value of distance is 5 Base b = new Derived(); b.Run(); // Here the default value of distance is 42, not 5 ``` ```csharp Fix theme={null} interface IRunner { void Run(int distance = 42) { /* ... */ } } class Runner : IRunner { void IRunner.Run(int distance = 5) { /* ... */ } } IRunner x = new Runner(); x.Run(); // Here the default value of distance is 42 Runner d = new Runner(); d.Run(); // This will not compile as the Run method is only visible through the specified interface ```

The \`IDisposable interface is a mechanism to release unmanaged resources, if not implemented correctly this could result in resource leaks or more severe bugs.

This rule raises an issue when the recommended dispose pattern, as defined by Microsoft, is not adhered to. See the Compliant Solution section for examples.

Satisfying the rule’s conditions will enable potential derived classes to correctly dispose the members of your class:

  • sealed classes are not checked.

  • If a base class implements IDisposable your class should not have IDisposable in the list of its interfaces. In such cases it is recommended to override the base class’s protected virtual void Dispose(bool) method or its equivalent.

  • The class should not implement IDisposable explicitly, e.g. the Dispose() method should be public.

  • The class should contain protected virtual void Dispose(bool) method. This method allows the derived classes to correctly dispose the resources of this class.

  • The content of the Dispose() method should be invocation of Dispose(true) followed by GC.SuppressFinalize(this)

  • If the class has a finalizer, i.e. a destructor, the only code in its body should be a single invocation of Dispose(false).

  • If the class inherits from a class that implements IDisposable it must call the Dispose, or Dispose(bool) method of the base class from within its own implementation of Dispose or Dispose(bool)\`, respectively. This ensures that all resources from the base class are properly released.

```csharp Bad theme={null} public class Foo1 : IDisposable // Noncompliant - provide protected overridable implementation of Dispose(bool) on Foo or mark the type as sealed. { public void Dispose() // Noncompliant - should contain only a call to Dispose(true) and then GC.SuppressFinalize(this) { // Cleanup } } public class Foo2 : IDisposable { void IDisposable.Dispose() // Noncompliant - Dispose() should be public { Dispose(true); GC.SuppressFinalize(this); } public virtual void Dispose() // Noncompliant - Dispose() should be sealed { Dispose(true); GC.SuppressFinalize(this); } } public class Foo3 : IDisposable { public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } protected virtual void Dispose(bool disposing) { // Cleanup } ~Foo3() // Noncompliant - Modify Foo.~Foo() so that it calls Dispose(false) and then returns. { // Cleanup } }{code} ``` ```csharp Fix theme={null} // Sealed class public sealed class Foo1 : IDisposable { public void Dispose() { // Cleanup } } // Simple implementation public class Foo2 : IDisposable { public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } protected virtual void Dispose(bool disposing) { // Cleanup } } // Implementation with a finalizer public class Foo3 : IDisposable { public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } protected virtual void Dispose(bool disposing) { // Cleanup } ~Foo3() { Dispose(false); } } // Base disposable class public class Foo4 : DisposableBase { protected override void Dispose(bool disposing) { // Cleanup // Do not forget to call base base.Dispose(disposing); } } ```

One of the possible ways of performing type-testing is via the is operator: food is Pizza.

The is operator is often used before a direct cast to the target type, as a more flexible and powerful alternative to the as operator, especially when used to perform pattern matching.

```csharp Bad theme={null} if (food is Pizza pizza) ``` ```csharp Fix theme={null} public class Food { public void DoSomething() { if (this is Pizza) // Noncompliant { // Code specific to Pizza... } } } ```

By convention, namespaces within a project should start with the project default namespace, and end with the file’s position within the project. Anything else may confuse maintainers and callers.

```csharp Bad theme={null} // file path: Gui/Screen.cs namespace Green // Noncompliant { class Screen { } } ``` ```csharp Fix theme={null} // file path: Gui/Screen.cs namespace Gui { class Screen { } } ```

Shared naming conventions allow teams to collaborate efficiently. This rule checks that all enum names match a provided regular expression.

The default configuration is the one recommended by Microsoft:

  • Pascal casing, starting with an upper case character, e.g. BackColor

  • Short abbreviations of 2 letters can be capitalized, e.g. GetID

  • Longer abbreviations need to be lower case, e.g. GetHtml

  • If the enum is marked as \[Flags] then its name should be plural (e.g. MyOptions), otherwise, names should be singular (e.g. MyOption)

```csharp Bad theme={null} public enum foo // Noncompliant { FooValue = 0 } ``` ```csharp Fix theme={null} [Flags] public enum Option // Noncompliant { None = 0, Option1 = 1, Option2 = 2 } ```

Because serialization constructors allocate and initialize objects, security checks that are present on regular constructors must also be present on a serialization constructor. Failure to do so would allow callers that could not otherwise create an instance to use the serialization constructor to do this.

This rule raises an issue when a type implements the System.Runtime.Serialization.ISerializable interface, is not a delegate or interface, is declared in an assembly that allows partially trusted callers and has a constructor that takes a System.Runtime.Serialization.SerializationInfo object and a System.Runtime.Serialization.StreamingContext object which is not secured by a security check, but one or more of the regular constructors in the type is secured.

```csharp Bad theme={null} using System; using System.IO; using System.Runtime.Serialization; using System.Runtime.Serialization.Formatters.Binary; using System.Security; using System.Security.Permissions; [assembly: AllowPartiallyTrustedCallersAttribute()] namespace MyLibrary { [Serializable] public class Foo : ISerializable { private int n; [FileIOPermissionAttribute(SecurityAction.Demand, Unrestricted = true)] public Foo() { n = -1; } protected Foo(SerializationInfo info, StreamingContext context) // Noncompliant { n = (int)info.GetValue("n", typeof(int)); } void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) { info.AddValue("n", n); } } } ``` ```csharp Fix theme={null} using System; using System.IO; using System.Runtime.Serialization; using System.Runtime.Serialization.Formatters.Binary; using System.Security; using System.Security.Permissions; [assembly: AllowPartiallyTrustedCallersAttribute()] namespace MyLibrary { [Serializable] public class Foo : ISerializable { private int n; [FileIOPermissionAttribute(SecurityAction.Demand, Unrestricted = true)] public Foo() { n = -1; } [FileIOPermissionAttribute(SecurityAction.Demand, Unrestricted = true)] protected Foo(SerializationInfo info, StreamingContext context) { n = (int)info.GetValue("n", typeof(int)); } void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) { info.AddValue("n", n); } } } ```

Using the base keyword to access a member in anonymous methods, iterator results, and lambda and query expressions results in the compiler creating extra classes under the covers. Those extra classes are "unverifiable", meaning that the under some trust levels, the code will not be allowed to run.

Instead, the access should be made from a helper method.

```csharp Bad theme={null} public Person GetBasePerson() { return delegate () { base.GetThePerson(); } // Noncompliant } ``` ```csharp Fix theme={null} string BasePerson() { return base.GetThePerson(); } public Person GetBasePerson() { return delegate () { BasePerson(); } // Noncompliant } ```

GetHashCode is used to file an object in a Dictionary or Hashtable. If GetHashCode uses non-readonly fields and those fields change after the object is stored, the object immediately becomes mis-filed in the Hashtable. Any subsequent test to see if the object is in the Hashtable will return a false negative.

```csharp Bad theme={null} public class Person { public int age; public string name; public override int GetHashCode() { int hash = 12; hash += this.age.GetHashCode(); // Noncompliant hash += this.name.GetHashCode(); // Noncompliant return hash; } ``` ```csharp Fix theme={null} public class Person { public readonly DateTime birthday; public string name; public override int GetHashCode() { int hash = 12; hash += this.birthday.GetHashCode(); return hash; } ```

Creating an extension method that extends object is not recommended because it makes the method available on every type. Extensions should be applied at the most specialized level possible, and that is very unlikely to be object.

```csharp Bad theme={null} public static class MyExtensions { public static void SomeExtension(this object obj) // Noncompliant { // ... } } ``` ```csharp Fix theme={null} ```

When a static constructor serves no other purpose that initializing static fields, it comes with an unnecessary performance cost because the compiler generates a check before each static method or instance constructor invocation.

Instead, inline initialization is highly recommended.

```csharp Bad theme={null} namespace myLib { public class Foo { static int i; static string s; static Foo() // Noncompliant { i = 3; ResourceManager sm = new ResourceManager("strings", Assembly.GetExecutingAssembly()); s = sm.GetString("mystring"); } } } ``` ```csharp Fix theme={null} namespace myLib { public class Foo { static int i =3; static string s = InitString(); static string InitString() { ResourceManager sm = new ResourceManager("strings", Assembly.GetExecutingAssembly()); return sm.GetString("mystring"); } } } ```

An enumeration can be decorated with the FlagsAttribute to indicate that it can be used as a bit field: a set of flags, that can be independently set and reset.

For example, the following definition of the day of the week:

```csharp Bad theme={null} [Flags] enum Days { Monday = 1, // 0b00000001 Tuesday = 2, // 0b00000010 Wednesday = 4, // 0b00000100 Thursday = 8, // 0b00001000 Friday = 16, // 0b00010000 Saturday = 32, // 0b00100000 Sunday = 64 // 0b01000000 } ``` ```csharp Fix theme={null} [Flags] enum Days { // ... None = 0, // 0b00000000 Weekdays = Monday | Tuesday | Wednesday | Thursday | Friday, // 0b00011111 Weekend = Saturday | Sunday, // 0b01100000 All = Weekdays | Weekend // 0b01111111 } ```

In C#, the type of a variable can often be inferred by the compiler. The use of the \[var keyword]\([https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/implicitly-typed-local-variables](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/implicitly-typed-local-variables)) allows you to avoid repeating the type name in a variable declaration and object instantiation because the declared type can often be inferred by the compiler.

Additionally, initializations providing the default value can also be omitted, helping to make the code more concise and readable.

Unnecessarily verbose declarations and initializations should be simplified. Specifically, the following should be omitted when they can be inferred:

  • array element type

  • array size

  • \`new DelegateType

  • new Nullable\\`

  • object or collection initializers (\{})

  • type of lambda expression parameters

  • parameter declarations of anonymous methods when the parameters are not used.

```csharp Bad theme={null} var l = new List() {}; // Noncompliant, {} can be removed var o = new object() {}; // Noncompliant, {} can be removed var ints = new int[] {1, 2, 3}; // Noncompliant, int can be omitted ints = new int[3] {1, 2, 3}; // Noncompliant, the size specification can be removed int? i = new int?(5); // Noncompliant new int? could be omitted, it can be inferred from the declaration, and there's implicit conversion from T to T? var j = new int?(5); Func f1 = (int i) => 1; //Noncompliant, can be simplified class Class { private event EventHandler MyEvent; public Class() { MyEvent += new EventHandler((a,b)=>{ }); // Noncompliant, needlessly verbose } } ``` ```csharp Fix theme={null} var l = new List(); var o = new object(); var ints = new [] {1, 2, 3}; ints = new [] {1, 2, 3}; int? i = 5; var j = new int?(5); Func f1 = (i) => 1; class Class { private event EventHandler MyEvent; public Class() { MyEvent += (a,b)=>{ }; } } ```

Shared naming conventions allow teams to collaborate efficiently.

This rule raises an issue when a method or a property name is not PascalCased.

For example, the method

```csharp Bad theme={null} public int doSomething() {...} // Noncompliant ``` ```csharp Fix theme={null} public int DoSomething() {...} ```

StringBuilder instances that never build a string clutter the code and worse are a drag on performance. Either they should be removed, or the missing ToString() call should be added.

```csharp Bad theme={null} public void DoSomething(List strings) { var sb = new StringBuilder(); // Noncompliant sb.Append("Got: "); foreach(var str in strings) { sb.Append(str).Append(", "); // ... } } ``` ```csharp Fix theme={null} public void DoSomething(List strings) { foreach(var str in strings) { // ... } } ```

Nested control flow statements if, switch, for, foreach, while, do, and try are often key ingredients in creating what’s known as "Spaghetti code". This code smell can make your program difficult to understand and maintain.

When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.

```csharp Bad theme={null} if (condition1) // Compliant - depth = 1 { /* ... */ if (condition2) // Compliant - depth = 2 { /* ... */ for (int i = 0; i < 10; i++) // Compliant - depth = 3 { /* ... */ if (condition4) // Noncompliant - depth = 4, which exceeds the limit { if (condition5) // Depth = 5, exceeding the limit, but issues are only reported on depth = 4 { /* ... */ } return; } } } } ``` ```csharp Fix theme={null} if (!condition1) { return; } /* ... */ if (!condition2) { return; } for (int i = 0; i < 10; i++) { /* ... */ if (condition4) { if (condition5) { /* ... */ } return; } } ```

Creating objects that are not used is a vulnerability that can lead to unexpected behavior.

If this was done intentionally due to side effects in the object’s constructor, the code should be moved to a dedicated method.

```csharp Bad theme={null} public void Method(MyObject myObject) { if (myObject is null) { new MyObject(); // Noncompliant } if (myObject.IsCorrupted) { new ArgumentException($"{nameof(myObject)} is corrupted"); // Noncompliant } // ... } ``` ```csharp Fix theme={null} public void Method(MyObject myObject) { if (myObject is null) { myObject = new MyObject(); // Compliant } if (myObject.IsCorrupted) { throw new ArgumentException($"{nameof(myObject)} is corrupted"); // Compliant } // ... } ```

If an enum member’s name contains the word "reserved" it implies it is not currently used and will be change in the future. However changing an enum member is a breaking change and can create significant problems. There is no need to reserve an enum member since a new member can be added in the future, and such an addition will usually not be a breaking change.

This rule raises an issue when the name of an enumeration member contains "reserved".

```csharp Bad theme={null} using System; namespace MyLibrary { public enum Color { None, Red, Orange, Yellow, ReservedColor // Noncompliant } } ``` ```csharp Fix theme={null} ```

When writing managed code, there is no need to worry about memory allocation or deallocation as it is taken care of by the garbage collector. However, certain objects, such as Bitmap, utilize unmanaged memory for specific purposes like pointer arithmetic. These objects may have substantial unmanaged memory footprints while having minimal managed footprints. Unfortunately, the garbage collector only recognizes the small managed footprint and does not promptly reclaim the corresponding unmanaged memory (by invoking the finalizer method of Bitmap) for efficiency reasons.

In addition, it’s essential to manage other system resources besides memory. The operating system has limits on the number of file descriptors (e.g., FileStream) or sockets (e.g., WebClient) that can remain open simultaneously. Therefore, it’s crucial to Dispose of these resources promptly when they are no longer required, instead of relying on the garbage collector to invoke the finalizers of these objects at an unpredictable time in the future.

This rule keeps track of private fields and local variables of specific types that implement IDisposable or IAsyncDisposable. It identifies instances of these types that are not properly disposed, closed, aliased, returned, or passed to other methods. This applies to instances that are either directly created using the new operator or instantiated through a predefined list of factory methods.

Here is the list of predefined factory methods tracked by this rule:

  • System.IO.File.Create()

  • System.IO.File.Open()

  • System.Drawing.Image.FromFile()

  • System.Drawing.Image.FromStream()

```csharp Bad theme={null} public Stream WriteToFile(string path, string text) { var fs = new FileStream(path, FileMode.Open); // Compliant: it is returned var bytes = Encoding.UTF8.GetBytes(text); fs.Write(bytes, 0, bytes.Length); return fs; } public void ReadFromStream(Stream s) { var sr = new StreamReader(s); // Compliant: it would close the underlying stream. // ... } ``` ```csharp Fix theme={null} public class ResourceHolder { private FileStream fs; // Noncompliant: dispose or close are never called public void OpenResource(string path) { this.fs = new FileStream(path, FileMode.Open); } public void WriteToFile(string path, string text) { var fs = new FileStream(path, FileMode.Open); // Noncompliant: not disposed, returned or initialized with another disposable object var bytes = Encoding.UTF8.GetBytes(text); fs.Write(bytes, 0, bytes.Length); } } ```

Recursion is a technique used to define a problem in terms of the problem itself, usually in terms of a simpler version of the problem itself.

For example, the implementation of the generator for the n-th value of the Fibonacci sequence comes naturally from its mathematical definition, when recursion is used:

```csharp Bad theme={null} int NthFibonacciNumber(int n) { if (n <= 1) { return 1; } else { return NthFibonacciNumber(n - 1) + NthFibonacciNumber(n - 2); } } ``` ```csharp Fix theme={null} int NthFibonacciNumber(int n) { int previous = 0; int last = 1; for (var i = 0; i < n; i++) { (previous, last) = (last, last + previous); } return last; } ```

A field marked \`readonly can only be assigned as part of its declaration or in a constructor. While readonly reference types (e.g. classes) can still have their state changed subsequently, the same is not true of value types such as struct s. Thus, calling a method that updates object state on a readonly value type field simply has no effect (but runs without error!). The result is code that probably doesn’t do what you thought it did.

This rule raises an issue when a method that is not marked \[Pure] is invoked on a readonly\` value type field.

```csharp Bad theme={null} public struct S1 { public int value; public void SetValue() { value = 10; } } class Test { static readonly S1 first; static S1 second; static void Main() { first.SetValue(); // Noncompliant second.SetValue(); Console.WriteLine(first.value); // Surprise! This writes 0 Console.WriteLine(second.value); // This writes 10 } } ``` ```csharp Fix theme={null} ```

The foreach statement was introduced in the C# language prior to generics to make it easier to work with the non-generic collections available at that time such as ArrayList. The foreach statements allow you to downcast elements of a collection of Objects to any other type.

The problem is that to achieve the cast, the foreach statements silently perform explicit type conversion, which at runtime can result in an InvalidCastException.

C# code iterating on generic collections or arrays should not rely on foreach statement’s silent explicit conversions.

```csharp Bad theme={null} public class Fruit { } public class Orange : Fruit { } public class Apple : Fruit { } class MyTest { public void Test() { var fruitBasket = new List(); fruitBasket.Add(new Orange()); fruitBasket.Add(new Orange()); fruitBasket.Add(new Apple()); foreach (Orange orange in fruitBasket) // Noncompliant { //... } } } ``` ```csharp Fix theme={null} public class Fruit { } public class Orange : Fruit { } public class Apple : Fruit { } class MyTest { public void Test() { var fruitBasket = new List(); fruitBasket.Add(new Orange()); fruitBasket.Add(new Orange()); fruitBasket.Add(new Apple()); foreach (Orange orange in fruitBasket.OfType()) { //... } } } ```

Decreasing the accessibility level of an inherited method that is not overridable to private will shadow the name of the base method and can lead to confusion.

```csharp Bad theme={null} public class Base { public void SomeMethod(int count) { } } public class Derived : Base { private void SomeMethod(int count) { } // Noncompliant } class Program { public void DoWork() { var derived = new Derived(); derived.SomeMethod(42); // Base.SomeMethod is accessed here } } ``` ```csharp Fix theme={null} public class Base { public void SomeMethod(int count) { } } public class Derived : Base { private void SomeMethod(int count) { } // Noncompliant } public class SecondDerived : Derived { public void DoWork() { SomeMethod(42); // Base.SomeMethod is accessed here } } ```

Calling an overridable method from a constructor could result in failures or strange behaviors when instantiating a subclass which overrides the method.

When constructing an object of a derived class, the constructor of the parent class is invoked first, and only then the constructor of the derived class is called. This sequential construction process applies to multiple levels of inheritance as well, starting from the base class and progressing to the most derived class.

If an overridable method is called within the constructor of the parent class, it may inadvertently invoke an overridden implementation in the derived class. This can lead to unexpected failures or strange behaviors because the object’s construction is still in progress and may not have reached a fully initialized state. Consequently, the overridden method may rely on uninitialized members or have assumptions about the object’s state that are not yet valid.

For example:

```csharp Bad theme={null} public class Parent { public Parent() { DoSomething(); // Noncompliant } public virtual void DoSomething() // can be overridden { ... } } public class Child : Parent { private string foo; public Child(string foo) // leads to call DoSomething() in Parent constructor which triggers a NullReferenceException as foo has not yet been initialized { this.foo = foo; } public override void DoSomething() { Console.WriteLine(this.foo.Length); } } ``` ```csharp Fix theme={null} ```

Nullable value types can hold either a value or null.

The value held in the nullable type can be accessed with the Value property or by casting it to the underlying type. Still, both operations throw an InvalidOperationException when the value is null. A nullable type should always be tested before accessing the value to avoid raising exceptions.

```csharp Bad theme={null} void Sample(bool condition) { int? nullableValue = condition ? 42 : null; Console.WriteLine(nullableValue.Value); // Noncompliant: InvalidOperationException is raised int? nullableCast = condition ? 42 : null; Console.WriteLine((int)nullableCast); // Noncompliant: InvalidOperationException is raised } ``` ```csharp Fix theme={null} void Sample(bool condition) { int? nullableValue = condition ? 42 : null; if (nullableValue.HasValue) { Console.WriteLine(nullableValue.Value); } int? nullableCast = condition ? 42 : null; if (nullableCast is not null) { Console.WriteLine((int)nullableCast); } } ```

Trivial properties, which include no logic but setting and getting a backing field should be converted to auto-implemented properties, yielding cleaner and more readable code.

```csharp Bad theme={null} public class Car { private string _make; public string Make // Noncompliant { get { return _make; } set { _make = value; } } } ``` ```csharp Fix theme={null} public class Car { public string Make { get; set; } } ```

Throwing general exceptions such as \`Exception, SystemException and ApplicationException will have a negative impact on any code trying to catch these exceptions.

From a consumer perspective, it is generally a best practice to only catch exceptions you intend to handle. Other exceptions should ideally be let to propagate up the stack trace so that they can be dealt with appropriately. When a general exception is thrown, it forces consumers to catch exceptions they do not intend to handle, which they then have to re-throw.

Besides, when working with a general type of exception, the only way to distinguish between multiple exceptions is to check their message, which is error-prone and difficult to maintain. Legitimate exceptions may be unintentionally silenced and errors may be hidden.

For instance, if an exception such as StackOverflowException is caught and not re-thrown, it may prevent the program from terminating gracefully.

When throwing an exception, it is therefore recommended to throw the most specific exception possible so that it can be handled intentionally by consumers.

Additionally, some reserved exceptions should not be thrown manually. Exceptions such as IndexOutOfRangeException, NullReferenceException, OutOfMemoryException or ExecutionEngineException\` will be thrown automatically by the runtime when the corresponding error occurs. Many of them indicate serious errors, which the application may not be able to recover from. It is therefore recommended to avoid throwing them as well as using them as base classes.

```csharp Bad theme={null} public void DoSomething(object obj) { if (obj == null) { throw new NullReferenceException("obj"); // Noncompliant: This reserved exception should not be thrown manually } // ... } ``` ```csharp Fix theme={null} public void DoSomething(object obj) { if (obj == null) { throw new ArgumentNullException("obj"); // Compliant: this is a specific and non-reserved exception type } // ... } ```

When implementing operator overloads, it is very important to make sure that all related operators and methods are consistent in their implementation.

The following guidelines should be followed:

  • When providing \`operator == you should also provide operator != and vice-versa.

  • When providing operator == you should also provide Equals(Object) and GetHashCode().

  • When providing operator +, -, \*, / or % you should also provide operator ==\`, respecting previous guidelines.

This rule raises an issue when any of these guidelines are not followed on publicly-visible type (public, protected or protected internal).

```csharp Bad theme={null} using System; namespace MyLibrary { public class Foo // Noncompliant { private int left; private int right; public Foo(int l, int r) { this.left = l; this.right = r; } public static Foo operator +(Foo a, Foo b) { return new Foo(a.left + b.left, a.right + b.right); } public static Foo operator -(Foo a, Foo b) { return new Foo(a.left - b.left, a.right - b.right); } } } ``` ```csharp Fix theme={null} using System; namespace MyLibrary { public class Foo { private int left; private int right; public Foo(int l, int r) { this.left = l; this.right = r; } public static Foo operator +(Foo a, Foo b) { return new Foo(a.left + b.left, a.right + b.right); } public static Foo operator -(Foo a, Foo b) { return new Foo(a.left - b.left, a.right - b.right); } public static bool operator ==(Foo a, Foo b) { return (a.left == b.left && a.right == b.right); } public static bool operator !=(Foo a, Foo b) { return !(a == b); } public override bool Equals(Object obj) { Foo a = obj as Foo; if (a == null) return false; return this == a; } public override int GetHashCode() { return (this.left * 10) + this.right; } } } ```

In Blazor, using lambda expressions as event handlers when the UI elements are rendered in a loop can lead to negative user experiences and performance issues. This is particularly noticeable when rendering a large number of elements.

The reason behind this is that Blazor rebuilds all lambda expressions within the loop every time the UI elements are rendered.

```csharp Bad theme={null} @for (var i = 1; i < 100; i++) { var buttonNumber = i; } @code { private void DoAction(MouseEventArgs e, int button) { // Do something here } } ``` ```csharp Fix theme={null} @foreach (var button in Buttons) { } @code { private List