Skip to content

Commit

Permalink
S1479: Update RSPEC (#9458)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebastien-marichal authored Jun 24, 2024
1 parent 3e340a6 commit fb539c2
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 105 deletions.
139 changes: 91 additions & 48 deletions analyzers/rspec/cs/S1479.html
Original file line number Diff line number Diff line change
@@ -1,77 +1,120 @@
<h2>Why is this an issue?</h2>
<p>When <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement">switch</a>
statements have large sets of case clauses, it is usually an attempt to map two sets of data. A <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2">Dictionary</a> should be used instead to make the code
more readable and maintainable.</p>
statements have large sets of multi-line <code>case</code> clauses, the code becomes hard to read and maintain.</p>
<p>For example, the <a href="https://www.sonarsource.com/docs/CognitiveComplexity.pdf">Cognitive Complexity</a> is going to be particularly high.</p>
<p>In such scenarios, it’s better to refactor the <code>switch</code> to only have single-line case clauses.</p>
<p>When all the <code>case</code> clauses of a <code>switch</code> statement are single-line, the readability of the code is not affected. Moreover,
<code>switch</code> statements with single-line <code>case</code> clauses can easily be converted into <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression"><code>switch</code> expressions</a>, which are
more concise for assignment and avoid the need for <code>break</code> statements.</p>
<h3>Exceptions</h3>
<p>This rule ignores <code>switch</code> statements over <code>Enum</code> arguments and empty, fall-through cases.</p>
<p>This rule ignores:</p>
<ul>
<li> <code>switch</code> statements over <code>Enum</code> arguments </li>
<li> fall-through cases </li>
<li> <code>return</code>, <code>break</code> and <code>throw</code> statements in case clauses </li>
</ul>
<h2>How to fix it</h2>
<p>Store all the cases apart from the <code>default</code> one in a dictionary and try to get the matching value by calling the <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue">TryGetValue</a> method.</p>
<p>Extract the logic of multi-line <code>case</code> clauses into separate methods.</p>
<h3>Code examples</h3>
<p>The example below are using the "Maximum number of case" property set to <code>4</code>.</p>
<p>The examples below use the "Maximum number of case" property set to <code>4</code>.</p>
<p>Note that from C# 8, you can use <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression"><code>switch</code> expression</a>.</p>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public class TooManyCase
public int MapChar(char ch, int value)
{
public int mapValues(char ch)
switch(ch) // Noncompliant
{
switch(ch) { // Noncompliant: 5 cases, "default" excluded, more than maximum = 4
case 'a':
return 1;
case 'b':
case 'c':
return 2;
case 'd':
return 3;
case 'e':
return 4;
case 'f':
case 'g':
case 'h':
return 5;
default:
return 6;
}
case 'a':
return 1;
case 'b':
return 2;
case 'c':
return 3;
// ...
case '-':
if (value &gt; 10)
{
return 42;
}
else if (value &lt; 5 &amp;&amp; value &gt; 1)
{
return 21;
}
return 99;
default:
return 1000;
}
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
using System.Collections.Generic;
public int MapChar(char ch, int value)
{
switch(ch) // Compliant: All 5 cases are single line statements
{
case 'a':
return 1;
case 'b':
return 2;
case 'c':
return 3;
// ...
case '-':
return HandleDash(value);
default:
return 1000;
}
}

public class TooManyCase
private int HandleDash(int value)
{
Dictionary&lt;char, int&gt; matching = new Dictionary&lt;char, int&gt;()
if (value &gt; 10)
{
return 42;
}
else if (value &lt; 5 &amp;&amp; value &gt; 1)
{
{ 'a', 1 },
{ 'b', 2 },
{ 'c', 2 },
{ 'd', 3 },
{ 'e', 4 },
{ 'f', 5 },
{ 'g', 5 },
{ 'h', 5 }
return 21;
}
return 99;
}
</pre>
<p>For this example, a <code>switch</code> expression is more concise and clear:</p>
<pre>
public int MapChar(char ch, int value) =&gt;
ch switch // Compliant
{
'a' =&gt; 1,
'b' =&gt; 2,
'c' =&gt; 3,
// ...
'-' =&gt; HandleDash(value),
_ =&gt; 1000,
};

public int mapValues(char ch)
private int HandleDash(int value)
{
if (value &gt; 10)
{
return 42;
}
else if (value &lt; 5 &amp;&amp; value &gt; 1)
{
int value;
if (this.matching.TryGetValue(ch, out value)) {
return value;
} else {
return 6;
}
return 21;
}
return 99;
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2">Dictionary&lt;TKey,TValue&gt; Class</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue">Dictionary&lt;TKey,TValue&gt;.TryGetValue(TKey, TValue) Method</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement">The
<li> Sonar - <a href="https://www.sonarsource.com/docs/CognitiveComplexity.pdf">Cognitive Complexity</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement">The
<code>switch</code> statement</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression">C#: Switch
Expression</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S1479.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "\"switch\" statements should not have too many \"case\" clauses",
"title": "\"switch\" statements with many \"case\" clauses should have only one statement",
"type": "CODE_SMELL",
"code": {
"impacts": {
Expand Down
111 changes: 58 additions & 53 deletions analyzers/rspec/vbnet/S1479.html
Original file line number Diff line number Diff line change
@@ -1,70 +1,75 @@
<h2>Why is this an issue?</h2>
<p>When <a href="https://learn.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/select-case-statement">Select Case</a> statements
have large sets of case clauses, it is usually an attempt to map two sets of data. A <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2">Dictionary</a> should be used instead to make the code
more readable and maintainable.</p>
have large sets of multi-line <code>Case</code> clauses, the code becomes hard to read and maintain.</p>
<p>For example, the <a href="https://www.sonarsource.com/docs/CognitiveComplexity.pdf">Cognitive Complexity</a> is going to be particularly high.</p>
<p>In such scenarios, it’s better to refactor the <code>Select Case</code> to only have single-line case clauses.</p>
<p>When all the <code>Case</code> clauses of a <code>Select Case</code> statement are single-line, the readability of the code is not affected.</p>
<h3>Exceptions</h3>
<p>This rule ignores <code>Select Case</code> statements over <code>Enum</code> arguments and empty, fall-through cases.</p>
<p>This rule ignores:</p>
<ul>
<li> <code>Select Case</code> statements over <code>Enum</code> arguments </li>
<li> fall-through cases </li>
<li> <code>Return</code> and <code>Throw</code> statements in <code>Case</code> clauses </li>
</ul>
<h2>How to fix it</h2>
<p>Store all the cases apart from the <code>Case Else</code> one in a dictionary and try to get the matching value by calling the <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue">TryGetValue</a> method.</p>
<p>Extract the logic of multi-line <code>Case</code> clauses into separate methods.</p>
<h3>Code examples</h3>
<p>The examples below use the "Maximum number of case" property set to <code>4</code>.</p>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
Public Class TooManyCase

Public Function MapValues(Ch As Char) As Integer
Select Case Ch ' Noncompliant: 5 cases, "Case Else" excluded, more than maximum = 4
Case "a"c
Return 1
Case "b"c, "c"c
Return 2
Case "d"c
Return 3
Case "e"c
Return 4
Case "f"c, "g"c, "h"c
Return 5
Case Else
Return 6
End Select
End Function

End Class
Public Function MapChar(ch As Char, value As Integer) As Integer ' Noncompliant
Select Case ch
Case "a"c
Return 1
Case "b"c
Return 2
Case "c"c
Return 3
' ...
Case "-"c
If value &gt; 10 Then
Return 42
ElseIf value &lt; 5 AndAlso value &gt; 1 Then
Return 21
End If
Return 99
Case Else
Return 1000
End Select
End Function
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
Public Class TooManyCase

Private fMatching As New Dictionary(Of Char, Integer) From {
{ "a"c, 1 },
{ "b"c, 2 },
{ "c"c, 2 },
{ "d"c, 3 },
{ "e"c, 4 },
{ "f"c, 5 },
{ "g"c, 5 },
{ "h"c, 5 },
}

Public Function MapValues(Ch As Char) As Integer
Dim Value As Integer
If fMatching.TryGetValue(Ch, Value) Then
Return Value
Else
Return 6
End If
End Function
Public Function MapChar(ch As Char, value As Integer) As Integer
Select Case ch
Case "a"c
Return 1
Case "b"c
Return 2
Case "c"c
Return 3
' ...
Case "-"c
Return HandleDash(value)
Case Else
Return 1000
End Select
End Function

End Class
Private Function HandleDash(value As Integer) As Integer
If value &gt; 10 Then
Return 42
ElseIf value &lt; 5 AndAlso value &gt; 1 Then
Return 21
End If
Return 99
End Function
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2">Dictionary&lt;TKey,TValue&gt; Class</a> </li>
<li> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue">Dictionary&lt;TKey,TValue&gt;.TryGetValue(TKey, TValue) Method</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/select-case-statement">Select…​Case Statement</a>
</li>
<li> Sonar - <a href="https://www.sonarsource.com/docs/CognitiveComplexity.pdf">Cognitive Complexity</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/select-case-statement">Select…​Case Statement</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/vbnet/S1479.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "\"Select Case\" statements should not have too many \"Case\" clauses",
"title": "\"Select Case\" statement with many \"Case\" clauses should have only one statement",
"type": "CODE_SMELL",
"code": {
"impacts": {
Expand Down
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.CSharp/sonarpedia.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"languages": [
"CSH"
],
"latest-update": "2024-06-24T12:09:48.416439100Z",
"latest-update": "2024-06-24T13:02:32.572091600Z",
"options": {
"no-language-in-filenames": true
}
Expand Down
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.VisualBasic/sonarpedia.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"languages": [
"VBNET"
],
"latest-update": "2024-06-24T12:10:12.271374400Z",
"latest-update": "2024-06-24T13:03:10.676891100Z",
"options": {
"no-language-in-filenames": true
}
Expand Down

0 comments on commit fb539c2

Please sign in to comment.