Skip to content

Commit

Permalink
New rule S6609: "Min/Max" properties of "Set" types should be used in…
Browse files Browse the repository at this point in the history
…stead of the "Enumerable" extension methods (#7202)
  • Loading branch information
gregory-paidis-sonarsource authored May 16, 2023
1 parent e99c3ad commit 9e2a97a
Show file tree
Hide file tree
Showing 21 changed files with 821 additions and 4 deletions.
13 changes: 13 additions & 0 deletions analyzers/its/expected/Net7/Net7--net7.0-S1144.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@
},
{
"id": "S1144",
"message": "Remove the unused private method 'Method'.",
"location": {
"uri": "sources\Net7\Net7\features\SetPropertiesInsteadOfLinqMethods.cs",
"region": {
"startLine": 8,
"startColumn": 20,
"endLine": 8,
"endColumn": 26
}
}
},
{
"id": "S1144",
"message": "Remove the unused internal class 'lowercasename'.",
"location": {
"uri": "sources\Net7\Net7\features\WarningWave7.cs",
Expand Down
13 changes: 13 additions & 0 deletions analyzers/its/expected/Net7/Net7--net7.0-S1451.json
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@
"id": "S1451",
"message": "Add or update the header of this file.",
"location": {
"uri": "sources\Net7\Net7\features\SetPropertiesInsteadOfLinqMethods.cs",
"region": {
"startLine": 1,
"startColumn": 1,
"endLine": 1,
"endColumn": 1
}
}
},
{
"id": "S1451",
"message": "Add or update the header of this file.",
"location": {
"uri": "sources\Net7\Net7\features\StaticVirtualMembersInInterfaces.cs",
"region": {
"startLine": 1,
Expand Down
17 changes: 17 additions & 0 deletions analyzers/its/expected/Net7/Net7--net7.0-S6609.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"issues": [
{
"id": "S6609",
"message": ""Min" property of Set type should be used instead of the "Min()" extension method.",
"location": {
"uri": "sources\Net7\Net7\features\SetPropertiesInsteadOfLinqMethods.cs",
"region": {
"startLine": 11,
"startColumn": 24,
"endLine": 11,
"endColumn": 27
}
}
}
]
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
namespace Net7.features
{
public static class Foo
public static class OrderByBeforeWhere
{
public static List<int> Bar()
public static List<int> Method()
{
var list = new List<int>();
return list.OrderBy(x => x).Where(x => true).ToList();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System.Linq;
using System.Collections.Generic;

namespace Net7.features
{
public static class SetPropertiesInsteadOfLinqMethods
{
static int Method()
{
var set = new SortedSet<int>();
return set.Min();
}
}
}
150 changes: 150 additions & 0 deletions analyzers/rspec/cs/S6609.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<h2>Why is this an issue?</h2>
<p>Both the <code>Enumerable.Max</code> extension method and the <code>SortedSet&lt;T&gt;.Max</code> property can be used to find the maximum value in
a <code>SortedSet&lt;T&gt;</code>. However, <code>SortedSet&lt;T&gt;.Max</code> is much faster than <code>Enumerable.Max</code>. For small
collections, the performance difference may be minor, but for large collections, it can be noticeable. The same applies for the <code>Min</code>
property as well.</p>
<p><code>Max</code> and <code>Min</code> in <code>SortedSet&lt;T&gt;</code> exploit the fact that the set is implemented via a <code>Red-Black
tree</code>. The algorithm to find the <code>Max</code>/<code>Min</code> is "go left/right whenever possible". The operation has the time complexity
of <code>O(h)</code> which becomes <code>O(ln(n))</code> due to the fact that the tree is balanced. This is much better than the <code>O(n)</code>
time complexity of extension methods.</p>
<p><code>Max</code> and <code>Min</code> in <code>ImmutableSortedSet&lt;T&gt;</code> exploits a tree augmentation technique, storing the
<code>Min</code>, <code>Max</code> and <code>Count</code> values on each node of the data structure. The time complexity in this case is
<code>O(1)</code> that is significantly better than <code>O(n)</code> of extension methods.</p>
<p><strong>Applies to:</strong></p>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.sortedset-1.max">SortedSet&lt;T&gt;.Max</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.sortedset-1.min">SortedSet&lt;T&gt;.Min</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablesortedset-1.max">ImmutableSortedSet&lt;T&gt;.Max</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablesortedset-1.min">ImmutableSortedSet&lt;T&gt;.Min</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablesortedset-1.builder.max">ImmutableSortedSet&lt;T&gt;.Builder.Max</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablesortedset-1.builder.min">ImmutableSortedSet&lt;T&gt;.Builder.Min</a> </li>
</ul>
<h3>What is the potential impact?</h3>
<p>We measured a significant improvement both in execution time and memory allocation. For more details see the <code>Benchmarks</code> section from
the <code>More info</code> tab.</p>
<h2>How to fix it</h2>
<p>The <code>Min</code> and <code>Max</code> properties are defined on the following classes, and the extension method call can be replaced by calling
the propery instead:</p>
<ul>
<li> <code>SortedSet&lt;T&gt;</code> </li>
<li> <code>ImmutableSortedSet&lt;T&gt;</code> </li>
<li> <code>ImmutableSortedSet&lt;T&gt;.Builder</code> </li>
</ul>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
int GetMax(SortedSet&lt;int&gt; data) =&gt;
data.Max();
</pre>
<pre data-diff-id="2" data-diff-type="noncompliant">
int GetMin(SortedSet&lt;int&gt; data) =&gt;
data.Min();
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
int GetMax(SortedSet&lt;int&gt; data) =&gt;
data.Max;
</pre>
<pre data-diff-id="2" data-diff-type="compliant">
int GetMin(SortedSet&lt;int&gt; data) =&gt;
data.Min;
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.sortedset-1">SortedSet&lt;T&gt;</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablesortedset-1">ImmutableSortedSet&lt;T&gt;</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablesortedset-1.builder">ImmutableSortedSet&lt;T&gt;.Builder</a> </li>
</ul>
<h3>Benchmarks</h3>
<table>
<colgroup>
<col style="width: 20%;">
<col style="width: 20%;">
<col style="width: 20%;">
<col style="width: 20%;">
<col style="width: 20%;">
</colgroup>
<thead>
<tr>
<th>Method</th>
<th>Runtime</th>
<th>Mean</th>
<th>StdDev</th>
<th>Allocated</th>
</tr>
</thead>
<tbody>
<tr>
<td><p>MaxMethod</p></td>
<td><p>.NET 7.0</p></td>
<td><p>68,961.483 us</p></td>
<td><p>499.6623 us</p></td>
<td><p>248063 B</p></td>
</tr>
<tr>
<td><p>MaxProperty</p></td>
<td><p>.NET 7.0</p></td>
<td><p>4.638 us</p></td>
<td><p>0.0634 us</p></td>
<td><p>-</p></td>
</tr>
<tr>
<td><p>MaxMethod</p></td>
<td><p>.NET Framework 4.6.2</p></td>
<td><p>85,827.359 us</p></td>
<td><p>1,531.1611 us</p></td>
<td><p>281259 B</p></td>
</tr>
<tr>
<td><p>MaxProperty</p></td>
<td><p>.NET Framework 4.6.2</p></td>
<td><p>67.682 us</p></td>
<td><p>0.3757 us</p></td>
<td><p>312919 B</p></td>
</tr>
</tbody>
</table>
<p>The results were generated by running the following snippet with <a href="https://github.com/dotnet/BenchmarkDotNet">BenchmarkDotNet</a>:</p>
<pre>
private SortedSet&lt;string&gt; data;

[Params(1_000)]
public int Iterations;

[GlobalSetup]
public void Setup() =&gt;
data = new SortedSet&lt;string&gt;(Enumerable.Range(0, Iterations).Select(x =&gt; Guid.NewGuid().ToString()));

[Benchmark(Baseline = true)]
public void MaxMethod()
{
for (var i = 0; i &lt; Iterations; i++)
{
_ = data.Max(); // Max() extension method
}
}

[Benchmark]
public void MaxProperty()
{
for (var i = 0; i &lt; Iterations; i++)
{
_ = data.Max; // Max property
}
}
</pre>
<p>Hardware configuration:</p>
<pre>
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
[Host] : .NET Framework 4.8 (4.8.4614.0), X64 RyuJIT VectorSize=256
.NET 7.0 : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
.NET Framework 4.6.2 : .NET Framework 4.8 (4.8.4614.0), X64 RyuJIT VectorSize=256
</pre>

17 changes: 17 additions & 0 deletions analyzers/rspec/cs/S6609.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"title": "\"Min\/Max\" properties of \"Set\" types should be used instead of the \"Enumerable\" extension methods",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"performance"
],
"defaultSeverity": "Minor",
"ruleSpecification": "RSPEC-6609",
"sqKey": "S6609",
"scope": "All",
"quickfix": "targeted"
}
1 change: 1 addition & 0 deletions analyzers/rspec/cs/Sonar_way_profile.json
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@
"S6605",
"S6607",
"S6608",
"S6609",
"S6610",
"S6613"
]
Expand Down
Loading

0 comments on commit 9e2a97a

Please sign in to comment.