private classes and records aren’t visible outside of their assemblies anyway, so if they’re not extended inside the assemblies, they
-should be made explicitly non-extensible with the addition of the sealed keyword.
++Classes and records with either
+privateorfileaccess modifiers aren’t visible outside of their assemblies or files, so +if they’re not extended inside their scope, they should be made explicitly non-extensible with the addition of thesealedkeyword.What is the potential impact?
+We measured at least 4x improvement in execution time. For more details see the
+Benchmarkssection from theMore info+tab.How to fix it
+The code can be improved by adding the
+sealedkeyword in front of theclassorrecordtypes that have no +inheritors.Code examples
+Noncompliant code example
+private class MyClass // Noncompliant { // ... @@ -13,8 +20,19 @@-Noncompliant code example
// ... }Compliant solution
-++file class MyClass // Noncompliant +{ + // ... +} + +file record MyRecord // Noncompliant +{ + // ... +} ++Compliant solution
+private sealed class MyClass { // ... @@ -25,4 +43,139 @@+Compliant solution
// ... }+file sealed class MyClass +{ + // ... +} + +file sealed record MyRecord +{ + // ... +} ++Resources
+Documentation
+
| Method | +Runtime | +Mean | +StdDev | +Ratio | +
|---|---|---|---|---|
UnsealedType |
+ .NET 5.0 |
+ 918.7 us |
+ 10.72 us |
+ 1.00 |
+
SealedType |
+ .NET 5.0 |
+ 231.2 us |
+ 3.20 us |
+ 0.25 |
+
UnsealedType |
+ .NET 6.0 |
+ 867.9 us |
+ 5.65 us |
+ 1.00 |
+
SealedType |
+ .NET 6.0 |
+ 218.4 us |
+ 0.59 us |
+ 0.25 |
+
UnsealedType |
+ .NET 7.0 |
+ 1,074.5 us |
+ 3.15 us |
+ 1.00 |
+
SealedType |
+ .NET 7.0 |
+ 216.1 us |
+ 1.19 us |
+ 0.20 |
+
The results were generated by running the following snippet with BenchmarkDotNet:
+
+[Params(1_000_000)]
+public int Iterations { get; set; }
+
+private readonly UnsealedClass unsealedType = new UnsealedClass();
+private readonly SealedClass sealedType = new SealedClass();
+
+[Benchmark(Baseline = true)]
+public void UnsealedType()
+{
+ for (int i = 0; i < Iterations; i++)
+ {
+ unsealedType.DoNothing();
+ }
+}
+
+[Benchmark]
+public void SealedType()
+{
+ for (int i = 0; i < Iterations; i++)
+ {
+ sealedType.DoNothing();
+ }
+}
+
+private class BaseType
+{
+ public virtual void DoNothing() { }
+}
+
+private class UnsealedClass : BaseType
+{
+ public override void DoNothing() { }
+}
+
+private sealed class SealedClass : BaseType
+{
+ public override void DoNothing() { }
+}
+
+Hardware Configuration:
++BenchmarkDotNet=v0.13.5, OS=Windows 10 (10.0.19045.2846/22H2/2022Update) +12th Gen Intel Core i7-12800H, 1 CPU, 20 logical and 14 physical cores +.NET SDK=7.0.203 + [Host] : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2 + .NET 5.0 : .NET 5.0.17 (5.0.1722.21314), X64 RyuJIT AVX2 + .NET 6.0 : .NET 6.0.16 (6.0.1623.17311), X64 RyuJIT AVX2 + .NET 7.0 : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2 +diff --git a/analyzers/rspec/cs/S3972.html b/analyzers/rspec/cs/S3972.html index b9a7ad8f6a8..47fe8e7e556 100644 --- a/analyzers/rspec/cs/S3972.html +++ b/analyzers/rspec/cs/S3972.html @@ -1,9 +1,8 @@
Code is clearest when each statement has its own line. Nonetheless, it is a common pattern to combine on the same line an if and its
-resulting then statement. However, when an if is placed on the same line as the closing } from a preceding
-then, else or else if part, it is either an error - else is missing - or the invitation to a future error as
-maintainers fail to understand that the two statements are unconnected.
When an if is placed on the same line as the closing } from a preceding if, else or else
+if part, it is either an error - else is missing - or the invitation to a future error as maintainers fail to understand that the
+two statements are unconnected.
This code is confusing
if (condition1) {
// ...
@@ -11,21 +10,21 @@ Noncompliant code example
//...
}
-Either the two conditions are unrelated and they should be visually separated
if (condition1) {
// ...
-} else if (condition2) {
+}
+
+if (condition2) {
//...
}
-Or
+Or they were supposed to be exclusive and you should use else if instead
if (condition1) {
// ...
-}
-
-if (condition2) {
+} else if (condition2) {
//...
}
diff --git a/analyzers/rspec/cs/S3973.html b/analyzers/rspec/cs/S3973.html
index e8f38ed74ef..76572a985a8 100644
--- a/analyzers/rspec/cs/S3973.html
+++ b/analyzers/rspec/cs/S3973.html
@@ -1,25 +1,31 @@
In the absence of enclosing curly braces, the line immediately after a conditional is the one that is conditionally executed. By both convention -and good practice, such lines are indented. In the absence of both curly braces and indentation the intent of the original programmer is entirely -unclear and perhaps not actually what is executed. Additionally, such code is highly likely to be confusing to maintainers.
-When the line immediately after a conditional has neither curly braces nor indentation, the intent of the code is unclear and perhaps not what is +executed. Additionally, such code is confusing to maintainers.
if (condition) // Noncompliant DoTheThing(); - -DoTheOtherThing(); -SomethingElseEntirely(); - -Foo(); +DoTheOtherThing(); // Was the intent to call this function unconditionally?-
It becomes even more confusing and bug-prone if lines get commented out.
++if (condition) // Noncompliant +// DoTheThing(); +DoTheOtherThing(); // Was the intent to call this function conditionally? ++
Indentation alone or together with curly braces makes the intent clear.
if (condition)
DoTheThing();
+DoTheOtherThing(); // Clear intent to call this function unconditionally
-DoTheOtherThing();
-SomethingElseEntirely();
+// or
-Foo();
+if (condition)
+{
+ DoTheThing();
+}
+DoTheOtherThing(); // Clear intent to call this function unconditionally
+This rule raises an issue if the line controlled by a conditional has the same indentation as the conditional and is not enclosed in curly +braces.
diff --git a/analyzers/rspec/cs/S4144.html b/analyzers/rspec/cs/S4144.html index e45fc3566e0..06c0bad32e6 100644 --- a/analyzers/rspec/cs/S4144.html +++ b/analyzers/rspec/cs/S4144.html @@ -1,9 +1,8 @@When two methods have the same implementation, either it was a mistake - something else was intended - or the duplication was intentional, but may -be confusing to maintainers. In the latter case, one implementation should invoke the other.
--private const string CODE = "bounteous"; +Two methods having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which +becomes a maintenance burden.
++private const string CODE = "secret"; private int callCount = 0; public string GetCode() @@ -12,15 +11,16 @@-Noncompliant code example
return CODE; } -public string GetName() // Noncompliant +public string GetName() // Noncompliant: duplicates GetCode { callCount++; return CODE; }Compliant solution
--private const string CODE = "bounteous"; +If the identical logic is intentional, the code should be refactored to avoid duplication. For example, by having both methods call the same method +or by having one implementation invoke the other.
++private const string CODE = "secret"; private int callCount = 0; public string GetCode() @@ -29,7 +29,7 @@Compliant solution
return CODE; } -public string GetName() +public string GetName() // Intent is clear { return GetCode(); } diff --git a/analyzers/rspec/cs/S6605.html b/analyzers/rspec/cs/S6605.html index e389b43a2ca..0458efc7f8a 100644 --- a/analyzers/rspec/cs/S6605.html +++ b/analyzers/rspec/cs/S6605.html @@ -13,6 +13,11 @@What is the potential impact?
We measured at least 3x improvement in execution time. For more details see the
Benchmarkssection from theMore infotab.Also, no memory allocations were needed for the
+Existsmethod, since the search is done in-place.Exceptions
+Since
LINQ to +Entitiesrelies a lot onSystem.Linqfor query conversion, +this rule won’t raise when used within LINQ to Entities syntaxes.How to fix it
The
@@ -44,6 +49,7 @@Existsmethod is defined on the collection class, and it has the same signature asAnyextension method if a predicate is used. The method can be replaced in place.Documentation
| Method | Runtime | -SampleSize | -LoopSize | Mean | -Error | StdDev |
|---|---|---|---|---|---|---|
ElementAt |
.NET 7.0 |
- 1000000 |
- 1000 |
15,193.1 ns |
- 249.59 ns |
233.47 ns |
Index |
.NET 7.0 |
- 1000000 |
- 1000 |
9,465.6 ns |
- 158.40 ns |
148.16 ns |
First |
.NET 7.0 |
- 1000000 |
- 1000 |
7,790.2 ns |
- 149.08 ns |
165.70 ns |
First_Index |
.NET 7.0 |
- 1000000 |
- 1000 |
398.5 ns |
- 5.73 ns |
5.36 ns |
Last |
.NET 7.0 |
- 1000000 |
- 1000 |
7,398.2 ns |
- 142.51 ns |
152.48 ns |
Last_Index |
.NET 7.0 |
- 1000000 |
- 1000 |
347.3 ns |
- 6.18 ns |
5.47 ns |
ElementAt |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
12,205.7 ns |
- 236.02 ns |
298.49 ns |
Index |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
8,917.8 ns |
- 55.11 ns |
51.55 ns |
First |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
5,109.1 ns |
- 101.95 ns |
100.13 ns |
First_Index |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
566.0 ns |
- 7.40 ns |
6.56 ns |
Last |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
5,052.7 ns |
- 81.27 ns |
76.02 ns |
Last_Index |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
680.7 ns |
- 10.22 ns |
9.56 ns |
Configuration used for running the benchmarks:
+The results were generated by running the following snippet with BenchmarkDotNet:
+
+private List<byte> data;
+private Random random;
+
+[Params(1_000_000)]
+public int SampleSize;
+
+[Params(1_000)]
+public int LoopSize;
+
+[GlobalSetup]
+public void Setup()
+{
+ random = new Random(42);
+ var bytes = new byte[SampleSize];
+ random.NextBytes(bytes);
+ data = bytes.ToList();
+}
+
+[Benchmark]
+public void ElementAt()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ var index = random.Next(0, SampleSize);
+ _ = data.ElementAt(index);
+ }
+}
+
+[Benchmark]
+public void Index()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ var index = random.Next(0, SampleSize);
+ _ = data[index];
+ }
+}
+
+[Benchmark]
+public void First()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data.First();
+ }
+}
+
+[Benchmark]
+public void First_Index()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data[0];
+ }
+}
+
+[Benchmark]
+public void Last()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data.Last();
+ }
+}
+
+[Benchmark]
+public void Last_Index()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data[data.Count - 1];
+ }
+}
+
+Hardware configuration:
BenchmarkDotNet=v0.13.5, OS=Windows 10 (10.0.19045.2846/22H2/2022Update) 11th Gen Intel Core i7-11850H 2.50GHz, 1 CPU, 16 logical and 8 physical cores @@ -227,43 +226,4 @@-What is the potential impact?
.NET 7.0 : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2 .NET Framework 4.6.2 : .NET Framework 4.8.1 (4.8.9139.0), X64 RyuJIT VectorSize=256
If the type you are using implements IList, IList<T> or IReadonlyList<T>, it implements
-this[int index]. This means calls to First, Last, or ElementAt(index) can be replaced with
-indexing at 0, Count-1 and index respectively.
-int GetAt(List<int> data, int index) - => data.ElementAt(index); --
-int GetFirst(List<int> data) - => data.First(); --
-int GetLast(List<int> data) - => data.Last(); --
-int GetAt(List<int> data, int index) - => data[index]; --
-int GetFirst(List<int> data) - => data[0]; --
-int GetLast(List<int> data) - => data[data.Count-1]; --
We measured a significant improvement both in execution time and memory allocation. For more details see the Benchmarks section from
the More info tab.
Since LINQ to
+Entities relies a lot on System.Linq for query conversion,
+this rule won’t raise when used within LINQ to Entities syntaxes.
Contains is a method defined on the ICollection<T> interface and takes a T item argument.
Any is an extension method defined on the IEnumerable<T> interface and takes a predicate argument. Therefore, calls
@@ -48,6 +53,7 @@
When two methods have the same implementation, either it was a mistake - something else was intended - or the duplication was intentional, but may -be confusing to maintainers. In the latter case, one implementation should invoke the other.
-+Two methods having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which +becomes a maintenance burden.
+Private Const CODE As String = "bounteous" Private callCount As Integer = 0 @@ -11,13 +10,14 @@-Noncompliant code example
Return CODE End Function -Public Function GetName() As String ' Noncompliant +Public Function GetName() As String ' Noncompliant: duplicates GetCode callCount = callCount + 1 Return CODE End FunctionCompliant solution
-+If the identical logic is intentional, the code should be refactored to avoid duplication. For example, by having both methods call the same method +or by having one implementation invoke the other.
+Private Const CODE As String = "bounteous" Private callCount As Integer = 0 @@ -26,7 +26,7 @@diff --git a/analyzers/rspec/vbnet/S6605.html b/analyzers/rspec/vbnet/S6605.html index 49b8fdc7a6a..385701d0b2f 100644 --- a/analyzers/rspec/vbnet/S6605.html +++ b/analyzers/rspec/vbnet/S6605.html @@ -13,6 +13,11 @@Compliant solution
Return CODE End Function -Public Function GetName() As String +Public Function GetName() As String ' Intent is clear Return GetCode() End FunctionWhat is the potential impact?
We measured at least 3x improvement in execution time. For more details see the
Benchmarkssection from theMore infotab.Also, no memory allocations were needed for the
+Existsmethod, since the search is done in-place.Exceptions
+Since
LINQ to +Entitiesrelies a lot onSystem.Linqfor query conversion, +this rule won’t raise when used within LINQ to Entities syntaxes.How to fix it
The
@@ -48,6 +53,7 @@Existsmethod is defined on the collection class, and it has the same signature asAnyextension method if a predicate is used. The method can be replaced in place.Documentation
| Method | Runtime | -SampleSize | -LoopSize | Mean | -Error | StdDev |
|---|---|---|---|---|---|---|
ElementAt |
.NET 7.0 |
- 1000000 |
- 1000 |
15,193.1 ns |
- 249.59 ns |
233.47 ns |
Index |
.NET 7.0 |
- 1000000 |
- 1000 |
9,465.6 ns |
- 158.40 ns |
148.16 ns |
First |
.NET 7.0 |
- 1000000 |
- 1000 |
7,790.2 ns |
- 149.08 ns |
165.70 ns |
First_Index |
.NET 7.0 |
- 1000000 |
- 1000 |
398.5 ns |
- 5.73 ns |
5.36 ns |
Last |
.NET 7.0 |
- 1000000 |
- 1000 |
7,398.2 ns |
- 142.51 ns |
152.48 ns |
Last_Index |
.NET 7.0 |
- 1000000 |
- 1000 |
347.3 ns |
- 6.18 ns |
5.47 ns |
ElementAt |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
12,205.7 ns |
- 236.02 ns |
298.49 ns |
Index |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
8,917.8 ns |
- 55.11 ns |
51.55 ns |
First |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
5,109.1 ns |
- 101.95 ns |
100.13 ns |
First_Index |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
566.0 ns |
- 7.40 ns |
6.56 ns |
Last |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
5,052.7 ns |
- 81.27 ns |
76.02 ns |
Last_Index |
.NET Framework 4.6.2 |
- 1000000 |
- 1000 |
680.7 ns |
- 10.22 ns |
9.56 ns |
Configuration used for running the benchmarks:
+The results were generated by running the following snippet with BenchmarkDotNet:
+
+private List<byte> data;
+private Random random;
+
+[Params(1_000_000)]
+public int SampleSize;
+
+[Params(1_000)]
+public int LoopSize;
+
+[GlobalSetup]
+public void Setup()
+{
+ random = new Random(42);
+ var bytes = new byte[SampleSize];
+ random.NextBytes(bytes);
+ data = bytes.ToList();
+}
+
+[Benchmark]
+public void ElementAt()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ var index = random.Next(0, SampleSize);
+ _ = data.ElementAt(index);
+ }
+}
+
+[Benchmark]
+public void Index()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ var index = random.Next(0, SampleSize);
+ _ = data[index];
+ }
+}
+
+[Benchmark]
+public void First()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data.First();
+ }
+}
+
+[Benchmark]
+public void First_Index()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data[0];
+ }
+}
+
+[Benchmark]
+public void Last()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data.Last();
+ }
+}
+
+[Benchmark]
+public void Last_Index()
+{
+ for (int i = 0; i < LoopSize; i++)
+ {
+ _ = data[data.Count - 1];
+ }
+}
+
+Hardware configuration:
BenchmarkDotNet=v0.13.5, OS=Windows 10 (10.0.19045.2846/22H2/2022Update) 11th Gen Intel Core i7-11850H 2.50GHz, 1 CPU, 16 logical and 8 physical cores @@ -227,49 +232,4 @@-What is the potential impact?
.NET 7.0 : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2 .NET Framework 4.6.2 : .NET Framework 4.8.1 (4.8.9139.0), X64 RyuJIT VectorSize=256
If the type you are using implements IList, IList<T> or IReadonlyList<T>, it implements
-this[int index]. This means calls to First, Last, or ElementAt(index) can be replaced with
-indexing at 0, Count-1 and index respectively.
-Function GetAt(data As List(Of Integer), index As Integer) As Integer - Return data.ElementAt(index) -End Function --
-Function GetFirst(data As List(Of Integer)) As Integer - Return data.First() -End Function --
-Function GetLast(data As List(Of Integer)) As Integer - Return data.Last() -End Function --
-Function GetAt(data As List(Of Integer), index As Integer) As Integer - Return data(index) -End Function --
-Function GetFirst(data As List(Of Integer)) As Integer - Return data(0) -End Function --
-Function GetLast(data As List(Of Integer)) As Integer - Return data(data.Count-1) -End Function --
We measured a significant improvement both in execution time and memory allocation. For more details see the Benchmarks section from
the More info tab.
Since LINQ to
+Entities relies a lot on System.Linq for query conversion,
+this rule won’t raise when used within LINQ to Entities syntaxes.
Contains is a method defined on the ICollection<T> interface and takes a T item argument.
Any is an extension method defined on the IEnumerable<T> interface and takes a predicate argument. Therefore, calls
@@ -52,6 +57,7 @@