Skip to content

Commit 95cd5d8

Browse files
Update RSPECs as part of the LayC migration (sixth batch) (#7454)
1 parent ecb72c6 commit 95cd5d8

27 files changed

+427
-176
lines changed

analyzers/rspec/cs/S108.html

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
<h2>Why is this an issue?</h2>
2-
<p>Most of the time a block of code is empty when a piece of code is really missing. So such empty block must be either filled or removed.</p>
3-
<h3>Noncompliant code example</h3>
2+
<p>An empty code block is confusing. It will require some effort from maintainers to determine if it is intentional or indicates the implementation is
3+
incomplete.</p>
44
<pre>
5-
for (int i = 0; i &lt; 42; i++){} // Empty on purpose or missing piece of code ?
5+
for (int i = 0; i &lt; 42; i++){} // Noncompliant: is the block empty on purpose, or is code missing?
66
</pre>
7+
<p>Removing or filling the empty code blocks takes away ambiguity and generally results in a more straightforward and less surprising code.</p>
78
<h3>Exceptions</h3>
8-
<p>When a block contains a comment, this block is not considered to be empty.</p>
9+
<p>The rule ignores code blocks that contain comments.</p>
910

analyzers/rspec/cs/S1110.html

+2-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ <h2>Why is this an issue?</h2>
22
<p>The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. But
33
redundant pairs of parentheses could be misleading, and should be removed.</p>
44
<h3>Noncompliant code example</h3>
5-
<pre>
5+
<pre data-diff-id="1" data-diff-type="noncompliant">
66
if (a &amp;&amp; ((x + y &gt; 0))) // Noncompliant
77
{
88
//...
@@ -11,14 +11,12 @@ <h3>Noncompliant code example</h3>
1111
return ((x + 1)); // Noncompliant
1212
</pre>
1313
<h3>Compliant solution</h3>
14-
<pre>
14+
<pre data-diff-id="1" data-diff-type="compliant">
1515
if (a &amp;&amp; (x + y &gt; 0))
1616
{
1717
//...
1818
}
1919

2020
return x + 1;
21-
22-
return (x + 1);
2321
</pre>
2422

analyzers/rspec/cs/S1117.html

+12-5
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
11
<h2>Why is this an issue?</h2>
2-
<p>Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of
3-
code. Further, it could lead maintainers to introduce bugs because they think they’re using one variable but are really using another.</p>
4-
<h3>Noncompliant code example</h3>
2+
<p>Overriding or shadowing a field or a property declared in an outer scope can strongly impact the readability, and therefore the maintainability, of
3+
a piece of code. Developers may mistakenly assume they are modifying or accessing the class field/property when, in fact, they are working with the
4+
local variable.</p>
55
<pre>
66
class Foo
77
{
88
public int myField;
9+
public int MyProperty { get; set; }
910

1011
public void DoSomething()
1112
{
12-
int myField = 0; // Noncompliant
13-
...
13+
int myField = 0; // Noncompliant
14+
int MyProperty = 0; // Noncompliant
1415
}
1516
}
1617
</pre>
18+
<h2>Resources</h2>
19+
<h3>Documentation</h3>
20+
<ul>
21+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/fields">Fields</a> </li>
22+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties">Properties</a> </li>
23+
</ul>
1724

analyzers/rspec/cs/S1117.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"title": "Local variables should not shadow class fields",
2+
"title": "Local variables should not shadow class fields or properties",
33
"type": "CODE_SMELL",
44
"status": "ready",
55
"remediation": {
@@ -14,5 +14,5 @@
1414
"ruleSpecification": "RSPEC-1117",
1515
"sqKey": "S1117",
1616
"scope": "All",
17-
"quickfix": "unknown"
17+
"quickfix": "infeasible"
1818
}

analyzers/rspec/cs/S1479.html

+35-14
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
11
<h2>Why is this an issue?</h2>
2-
<p>When <code>switch</code> statements have large sets of case clauses, it is usually an attempt to map two sets of data. A <code>Dictionary</code>
3-
should be used instead to make the code more readable and maintainable.</p>
4-
<h3>Noncompliant code example</h3>
5-
<p>With a "Maximum number of case" set to 4</p>
6-
<pre>
2+
<p>When <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement">switch</a>
3+
statements have large sets of case clauses, it is usually an attempt to map two sets of data. A <a
4+
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2">Dictionary</a> should be used instead to make the code
5+
more readable and maintainable.</p>
6+
<h3>Exceptions</h3>
7+
<p>This rule ignores <code>switch</code> statements over <code>Enum</code> arguments and empty, fall-through cases.</p>
8+
<h2>How to fix it</h2>
9+
<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
10+
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue">TryGetValue</a> method.</p>
11+
<h3>Code examples</h3>
12+
<p>The example below are using the "Maximum number of case" property set to <code>4</code>.</p>
13+
<h4>Noncompliant code example</h4>
14+
<pre data-diff-id="1" data-diff-type="noncompliant">
715
public class TooManyCase
816
{
9-
public int switchCase(char ch)
17+
public int mapValues(char ch)
1018
{
11-
switch(ch) { // Noncompliant
19+
switch(ch) { // Noncompliant: 5 cases, "default" excluded, more than maximum = 4
1220
case 'a':
1321
return 1;
1422
case 'b':
@@ -28,19 +36,25 @@ <h3>Noncompliant code example</h3>
2836
}
2937
}
3038
</pre>
31-
<h3>Compliant solution</h3>
32-
<pre>
39+
<h4>Compliant solution</h4>
40+
<pre data-diff-id="1" data-diff-type="compliant">
3341
using System.Collections.Generic;
3442

3543
public class TooManyCase
3644
{
3745
Dictionary&lt;char, int&gt; matching = new Dictionary&lt;char, int&gt;()
3846
{
39-
{'a', 1}, {'b', 2}, {'c', 2}, {'d', 3},
40-
{'e', 4}, {'f', 5}, {'g', 5}, {'h', 5}
47+
{ 'a', 1 },
48+
{ 'b', 2 },
49+
{ 'c', 2 },
50+
{ 'd', 3 },
51+
{ 'e', 4 },
52+
{ 'f', 5 },
53+
{ 'g', 5 },
54+
{ 'h', 5 }
4155
};
4256

43-
public int withDictionary(char ch)
57+
public int mapValues(char ch)
4458
{
4559
int value;
4660
if (this.matching.TryGetValue(ch, out value)) {
@@ -51,6 +65,13 @@ <h3>Compliant solution</h3>
5165
}
5266
}
5367
</pre>
54-
<h3>Exceptions</h3>
55-
<p>This rule ignores <code>switch</code>es over <code>Enum</code>s and empty, fall-through cases.</p>
68+
<h2>Resources</h2>
69+
<h3>Documentation</h3>
70+
<ul>
71+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2">Dictionary&lt;TKey,TValue&gt; Class</a> </li>
72+
<li> <a
73+
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>
74+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement">The
75+
<code>switch</code> statement</a> </li>
76+
</ul>
5677

analyzers/rspec/cs/S1479.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@
1313
"ruleSpecification": "RSPEC-1479",
1414
"sqKey": "S1479",
1515
"scope": "All",
16-
"quickfix": "unknown"
16+
"quickfix": "infeasible"
1717
}

analyzers/rspec/cs/S2190.html

+142-32
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,160 @@
11
<h2>Why is this an issue?</h2>
2-
<p>Recursion happens when control enters a loop that has no exit. This can happen a method invokes itself, when a pair of methods invoke each other,
3-
or when <code>goto</code> statements are used to move between two segments of code. It can be a useful tool, but unless the method includes a
4-
provision to break out of the recursion and <code>return</code>, the recursion will continue until the stack overflows and the program crashes.</p>
5-
<h3>Noncompliant code example</h3>
2+
<p>Having an infinite loop or recursion will lead to a program failure or a program never finishing the execution.</p>
63
<pre>
7-
int Pow(int num, int exponent) // Noncompliant; no condition under which pow isn't re-called
4+
public int Sum()
85
{
9-
num = num * Pow(num, exponent-1);
10-
return num; // this is never reached
6+
var i = 0;
7+
var result = 0;
8+
while (true) // Noncompliant: the program will never stop
9+
{
10+
result += i;
11+
i++;
12+
}
13+
return result;
1114
}
12-
13-
void WhileLoop() // Noncompliant; no condition under which while loop would exit
15+
</pre>
16+
<p>This can happen in multiple scenarios.</p>
17+
<h3>Loop statements</h3>
18+
<p><code>while</code> and <code>for</code> loops with no <code>break</code> or <code>return</code> statements and with the exit condition always
19+
<code>false</code> will be indefinitely executed.</p>
20+
<h3>"goto" statements</h3>
21+
<p><code>goto</code> statement with nothing that stops it from being executed over and over again will prevent the program from the completion.</p>
22+
<h3>Recursion</h3>
23+
<p>When a <a href="https://en.wikipedia.org/wiki/Recursion_(computer_science)">recursive</a> method call chain lacks an exit condition, the <a
24+
href="https://en.wikipedia.org/wiki/Call_stack">call stack</a> will reach its limit and the program will crash due to a <a
25+
href="https://learn.microsoft.com/en-us/dotnet/api/system.stackoverflowexception">StackOverflowException</a>.</p>
26+
<pre>
27+
int Pow(int num, int exponent)
1428
{
15-
while (true)
16-
{
17-
var line = Console.ReadLine();
18-
Console.WriteLine(line);
19-
}
29+
return num * Pow(num, exponent - 1); // Noncompliant: no condition under which Pow isn't re-called
2030
}
21-
22-
void InternalRecursion(int i)
31+
</pre>
32+
<p>In this example, <code>Pow</code> will keep calling <code>Pow</code> with <code>exponent - 1</code> forever, until the program crashes with a
33+
StackOverflowException.</p>
34+
<p>Recursion provides some benefits.</p>
35+
<ul>
36+
<li> <strong>Simplified code</strong>: recursion can often lead to more concise and elegant code by breaking down complex problems into smaller,
37+
more manageable parts. </li>
38+
<li> <strong>Improved code readability</strong>: compared to iterative solutions, recursive solutions can be easier to understand and reason about.
39+
</li>
40+
</ul>
41+
<p>However, it has disadvantages as well.</p>
42+
<ul>
43+
<li> <strong>Stack overflow</strong>: Recursive functions can lead to <a
44+
href="https://learn.microsoft.com/en-us/dotnet/api/system.stackoverflowexception">stack overflow</a> if the recursion is too deep, potentially
45+
causing the program to crash. </li>
46+
<li> <strong>Performance overhead</strong>: Recursive function calls can lead to poor performance due to the need to push and pop <a
47+
href="https://en.citizendium.org/wiki/Stack_frame#:~:text=In%20computer%20science%2C%20a%20stack,only%20exist%20at%20run%2Dtime">stack frames</a>,
48+
making them potentially slower than iterative solutions. </li>
49+
<li> <strong>Difficulty in debugging</strong>: Debugging recursive code can be challenging, as multiple recursive calls can make it harder to track
50+
the flow of execution and identify logical errors. </li>
51+
<li> <strong>Space complexity</strong>: Recursive algorithms may require more memory compared to iterative approaches, as each recursive call adds a
52+
new frame to the call stack. </li>
53+
<li> <strong>Lack of control</strong>: Recursion can sometimes lead to infinite loops or unexpected behavior if not properly implemented or
54+
terminated, making it crucial to have proper base cases and exit conditions. </li>
55+
</ul>
56+
<h2>How to fix it</h2>
57+
<p>The program’s logic should incorporate a mechanism to break out of the control flow loop. Here are some examples.</p>
58+
<h3>Code examples</h3>
59+
<ul>
60+
<li> Use a loop condition which eventually evaluates to <code>false</code> </li>
61+
</ul>
62+
<h4>Noncompliant code example</h4>
63+
<pre data-diff-id="1" data-diff-type="noncompliant">
64+
public int Sum()
2365
{
24-
start:
25-
goto end;
26-
end:
27-
goto start; // Noncompliant; there's no way to break out of this method
66+
var i = 0;
67+
var result = 0;
68+
while (true) // Noncompliant: the program will never stop
69+
{
70+
result += i;
71+
i++;
72+
}
73+
return result;
2874
}
2975
</pre>
30-
<h3>Compliant solution</h3>
31-
<pre>
76+
<h4>Compliant solution</h4>
77+
<pre data-diff-id="1" data-diff-type="compliant">
78+
public int Sum()
79+
{
80+
var i = 0;
81+
var result = 0;
82+
while (result &lt; 1000)
83+
{
84+
result += i;
85+
i++;
86+
}
87+
return result;
88+
}
89+
</pre>
90+
<ul>
91+
<li> As <a href="https://rules.sonarsource.com/csharp/RSPEC-907">{rule:csharpsquid:S907}</a> generally suggests, avoid using <code>goto</code>
92+
statements. Instead, you can use a loop statement or explicit recursion. </li>
93+
</ul>
94+
<h4>Noncompliant code example</h4>
95+
<pre data-diff-id="2" data-diff-type="noncompliant">
96+
public int Sum()
97+
{
98+
var result = 0;
99+
var i = 0;
100+
iteration:
101+
// Noncompliant: program never ends
102+
result += i;
103+
i++;
104+
goto iteration;
105+
return result;
106+
}
107+
</pre>
108+
<h4>Compliant solution</h4>
109+
<pre data-diff-id="2" data-diff-type="compliant">
110+
public int Sum()
111+
{
112+
var i = 0;
113+
var result = 0;
114+
while (result &lt; 1000)
115+
{
116+
result += i;
117+
i++;
118+
}
119+
return result;
120+
}
121+
</pre>
122+
<ul>
123+
<li> For a recursion make sure there is a base case when the recursive method is not re-called. </li>
124+
</ul>
125+
<h4>Noncompliant code example</h4>
126+
<pre data-diff-id="3" data-diff-type="noncompliant">
32127
int Pow(int num, int exponent)
33128
{
34-
if (exponent &gt; 1) // recursion now conditional and stop-able
35-
{
36-
num = num * Pow(num, exponent-1);
37-
}
38-
return num;
129+
return num * Pow(num, exponent - 1); // Noncompliant: no condition under which Pow isn't re-called
39130
}
40-
41-
void WhileLoop()
131+
</pre>
132+
<h4>Compliant solution</h4>
133+
<pre data-diff-id="3" data-diff-type="compliant">
134+
int Pow(int num, int exponent)
42135
{
43-
string line;
44-
while ((line = Console.ReadLine()) != null) // loop has clear exit condition
136+
if (exponent &gt; 1) // recursion is now conditional and stoppable
45137
{
46-
Console.WriteLine(line);
138+
num = num * Pow(num, exponent - 1);
47139
}
140+
return num;
48141
}
49142
</pre>
143+
<h2>Resources</h2>
144+
<h3>Documentation</h3>
145+
<ul>
146+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-for-statement">The "for"
147+
statement</a> </li>
148+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-while-statement">The "while"
149+
statement</a> </li>
150+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/jump-statements#the-goto-statement">The "goto"
151+
statement</a> </li>
152+
<li> <a href="https://en.wikipedia.org/wiki/Recursion_(computer_science)">Recursion - wiki</a> </li>
153+
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.stackoverflowexception?view=net-7.0">StackOverflowException class</a> </li>
154+
<li> <a href="https://rules.sonarsource.com/csharp/RSPEC-907">{rule:csharpsquid:S907}: "goto" statement should not be used</a> </li>
155+
</ul>
156+
<h3>Articles &amp; blog posts</h3>
157+
<ul>
158+
<li> <a href="https://www.cs.utexas.edu/users/EWD/transcriptions/EWD02xx/EWD215.html">Edsger Dijkstra: A Case against the GO TO Statement</a> </li>
159+
</ul>
50160

analyzers/rspec/cs/S2190.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"title": "Recursion should not be infinite",
2+
"title": "Loops and recursions should not be infinite",
33
"type": "BUG",
44
"status": "ready",
55
"remediation": {
@@ -13,5 +13,5 @@
1313
"ruleSpecification": "RSPEC-2190",
1414
"sqKey": "S2190",
1515
"scope": "All",
16-
"quickfix": "unknown"
16+
"quickfix": "infeasible"
1717
}

0 commit comments

Comments
 (0)