Skip to content

Commit

Permalink
Promote 7 rules to SonarWay (#8806)
Browse files Browse the repository at this point in the history
  • Loading branch information
cristian-ambrosini-sonarsource authored Feb 29, 2024
1 parent c0d6a61 commit 7ae6c9a
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 89 deletions.
6 changes: 4 additions & 2 deletions analyzers/rspec/cs/S1192.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "10min"
"func": "Linear with offset",
"linearDesc": "per duplicate instance",
"linearOffset": "2min",
"linearFactor": "2min"
},
"tags": [
"design"
Expand Down
48 changes: 36 additions & 12 deletions analyzers/rspec/cs/S1244.html
Original file line number Diff line number Diff line change
@@ -1,32 +1,56 @@
<h2>Why is this an issue?</h2>
<p>Floating point math is imprecise because of the challenges of storing such values in a binary representation. Even worse, floating point math is
not associative; push a <code>float</code> or a <code>double</code> through a series of simple mathematical operations and the answer will be
different based on the order of those operation because of the rounding that takes place at each step.</p>
<p>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.</p>
<p>Even simple floating point assignments are not simple:</p>
<pre>
float f = 0.100000001f; // 0.1
double d = 0.10000000000000001; // 0.1
</pre>
<p>(Results will vary based on compiler and compiler settings)</p>
<p>Therefore, the use of the equality (<code>==</code>) and inequality (<code>!=</code>) operators on <code>float</code> or <code>double</code> values
is almost always an error.</p>
<p>This rule checks for the use of direct and indirect equality/inequality tests on floats and doubles.</p>
<h3>Noncompliant code example</h3>
<pre>
<p>(Note: Results may vary based on the compiler and its settings)</p>
<p>This issue is further compounded by the <a href="https://en.wikipedia.org/wiki/Associative_property">non-associative</a> nature of floating point
arithmetic. The order in which operations are performed can affect the outcome due to the rounding that occurs at each step. Consequently, the outcome
of a series of mathematical operations can vary based on the order of operations.</p>
<p>As a result, using the equality (<code>==</code>) or inequality (<code>!=</code>) operators with <code>float</code> or <code>double</code> values
is typically a mistake, as it can lead to unexpected behavior.</p>
<h2>How to fix it</h2>
<p>Consider using a small tolerance value to check if the numbers are "close enough" to be considered equal. This tolerance value, often called
<code>epsilon</code>, should be chosen based on the specifics of your program.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
float myNumber = 3.146f;
if ( myNumber == 3.146f ) //Noncompliant. Because of floating point imprecision, this will be false

if (myNumber == 3.146f) //Noncompliant: due to floating point imprecision, this will likely be false
{
// ...
}

if (myNumber &lt; 4 || myNumber &gt; 4) // Noncompliant: indirect inequality test
{
// ...
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
float myNumber = 3.146f;
float epsilon = 0.0001f; // or some other small value

if (myNumber &lt;= 3.146f &amp;&amp; mNumber &gt;= 3.146f) // Noncompliant indirect equality test
if (Math.Abs(myNumber - 3.146f) &lt; epsilon)
{
// ...
}

if (myNumber &lt; 4 || myNumber &gt; 4) // Noncompliant indirect inequality test
if (myNumber &lt;= 4 - epsilon || myNumber &gt;= 4 + epsilon)
{
// ...
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html">Floating-Point Arithmetic Complexities</a> by David Goldberg </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/floating-point-numeric-types#comparing-floating-point-numbers">Floating-point numeric types</a> </li>
<li> Wikipedia - <a href="https://en.wikipedia.org/wiki/Associative_property">Associative property</a> </li>
</ul>

41 changes: 21 additions & 20 deletions analyzers/rspec/cs/S127.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,34 @@ <h2>Why is this an issue?</h2>
<li> When the stop condition depend upon a method call </li>
<li> When the stop condition depends on an object property, since such properties could change during the execution of the loop. </li>
</ul>
<h3>Noncompliant code example</h3>
<pre>
class Foo
<h2>How to fix it</h2>
<p>It’s generally recommended to only update the loop counter in the loop declaration. If skipping elements or iterating at a different pace based on
a condition is needed, consider using a while loop or a different structure that better fits the needs.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
for (int i = 1; i &lt;= 5; i++)
{
static void Main()
Console.WriteLine(i);
if (condition)
{
for (int i = 1; i &lt;= 5; i++)
{
Console.WriteLine(i);
if (condition)
{
i = 20;
}
}
i = 20;
}
}
</pre>
<h3>Compliant solution</h3>
<pre>
class Foo
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
int i = 1;
while (i &lt;= 5)
{
static void Main()
Console.WriteLine(i);
if (condition)
{
for (int i = 1; i &lt;= 5; i++)
{
Console.WriteLine(i);
}
i = 20;
}
else
{
i++;
}
}
</pre>
Expand Down
41 changes: 21 additions & 20 deletions analyzers/rspec/cs/S1696.html
Original file line number Diff line number Diff line change
@@ -1,43 +1,44 @@
<h2>Why is this an issue?</h2>
<p><code>NullReferenceException</code> should be avoided, not caught. Any situation in which <code>NullReferenceException</code> is explicitly caught
can easily be converted to a <code>null</code> test, and any behavior being carried out in the catch block can easily be moved to the "is null" branch
of the conditional.</p>
<h3>Noncompliant code example</h3>
<pre>
<p>Catching <code>NullReferenceException</code> 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
<code>NullReferenceException</code> 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.</p>
<h2>How to fix it</h2>
<p>Instead of catching NullReferenceException, it’s better to prevent it from happening in the first place. You can do this by using null checks or
null conditional operators (<code>?.</code>) before accessing members of an object.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public int GetLengthPlusTwo(string str)
{
int length = 2;
try
{
length += str.Length;
return str.Length + 2;
}
catch (NullReferenceException e)
{
log.info("argument was null");
return 2;
}
return length;
}
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public int GetLengthPlusTwo(string str)
{
int length = 2;

if (str != null)
{
length += str.Length;
}
else
if (str is null)
{
log.info("argument was null");
return 2;
}
return length;
return str.Length + 2;
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/395">CWE-395 - Use of NullPointerException Catch to Detect NULL Pointer Dereference</a>
</li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.nullreferenceexception">NullReferenceException class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/member-access-operators#null-conditional-operators—​and-">Null-conditional operators ?. and ?[</a>] </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S1696.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@
395
]
},
"quickfix": "unknown"
"quickfix": "infeasible"
}
50 changes: 42 additions & 8 deletions analyzers/rspec/cs/S1994.html
Original file line number Diff line number Diff line change
@@ -1,18 +1,52 @@
<h2>Why is this an issue?</h2>
<p>It can be extremely confusing when a <code>for</code> loop’s counter is incremented outside of its increment clause. In such cases, the increment
should be moved to the loop’s increment clause if at all possible.</p>
<h3>Noncompliant code example</h3>
<pre>
for (i = 0; i &lt; 10; j++) // Noncompliant
<p>The <code>for</code> loop is designed to iterate over a range using a counter variable, with the counter being updated in the loop’s increment
section. Misusing this structure can lead to issues such as infinite loops if the counter is not updated correctly. If this is intentional, use a
<code>while</code> or <code>do while</code> loop instead of a <code>for</code> loop.</p>
<p>Using a for loop for purposes other than its intended use can lead to confusion and potential bugs. If the <code>for</code> loop structure does not
fit your needs, consider using an alternative <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements">iteration statement</a>.</p>
<h2>How to fix it</h2>
<p>Move the counter variable update to the loop’s increment section. If this is impossible, consider using another iteration statement instead.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
int sum = 0;
for (int i = 0; i &lt; 10; sum++) // Noncompliant: `i` is not updated in the increment section
{
// ...
i++;
}
</pre>
<h3>Compliant solution</h3>
<pre>
for (i = 0; i &lt; 10; i++)
<pre data-diff-id="2" data-diff-type="noncompliant">
for (int i = 0;; i++) // Noncompliant: the loop condition is empty although incrementing `i`
{
// ...
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
int sum = 0;
for (int i = 0; i &lt; 10; i++)
{
// ...
sum++;
}
</pre>
<pre data-diff-id="2" data-diff-type="compliant">
int i = 0;
while (true)
{
// ...
i++;
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-for-statement">The <code>for</code>
statement</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements">Iteration
statements - <code>for</code>, <code>foreach</code>, <code>do</code>, and <code>while</code></a> </li>
</ul>

94 changes: 86 additions & 8 deletions analyzers/rspec/cs/S2701.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,89 @@
<h2>Why is this an issue?</h2>
<p>There’s no reason to use literal boolean values in assertions. Doing so is at best confusing for maintainers, and at worst a bug.</p>
<h3>Noncompliant code example</h3>
<pre>
bool b = true;
NUnit.Framework.Assert.AreEqual(true, b);
Xunit.Assert.NotSame(true, b);
Microsoft.VisualStudio.TestTools.UnitTesting.Assert.AreEqual(true, b);
System.Diagnostics.Debug.Assert(true);
<p>Using literal boolean values in assertions can lead to less readable and less informative unit tests. When a test fails, it’s important to have a
clear understanding of what the test was checking and why it failed. Most of the testing frameworks provide more explicit assertion methods that will
provide a more helpful error message if the test fails.</p>
<h3>Exceptions</h3>
<p>In the context of xUnit, <code>Assert.True</code> and <code>Assert.False</code> are not flagged by this rule. This is because
<code>Assert.Fail</code> was only introduced in 2020 with version <code>2.4.2</code>. Prior to this, developers used <code>Assert.True(false,
message)</code> and <code>Assert.False(true, message)</code> as workarounds to simulate the functionality of <code>Assert.Fail()</code>.</p>
<h2>How to fix it in MSTest</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
bool someResult;

Assert.AreEqual(false, someResult); // Noncompliant: use Assert.IsFalse
Assert.AreEqual(true, someResult); // Noncompliant: use Assert.IsTrue
Assert.AreNotEqual(false, someResult); // Noncompliant: use Assert.IsTrue
Assert.AreNotEqual(true, someResult); // Noncompliant: use Assert.IsFalse
Assert.IsFalse(true, "Should not reach this line!"); // Noncompliant: use Assert.Fail
Assert.IsTrue(false, "Should not reach this line!"); // Noncompliant: use Assert.Fail
Assert.IsFalse(false); // Noncompliant: remove it
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
bool someResult;

Assert.IsFalse(someResult);
Assert.IsTrue(someResult);
Assert.IsTrue(someResult);
Assert.IsFalse(someResult);
Assert.Fail("Should not reach this line!");
Assert.Fail("Should not reach this line!");
// Removed
</pre>
<h2>How to fix it in NUnit</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="2" data-diff-type="noncompliant">
bool someResult;

Assert.AreEqual(false, someResult); // Noncompliant: use Assert.False
Assert.AreEqual(true, someResult); // Noncompliant: use Assert.True
Assert.AreNotEqual(false, someResult); // Noncompliant: use Assert.True
Assert.AreNotEqual(true, someResult); // Noncompliant: use Assert.False
Assert.False(true, "Should not reach this line!"); // Noncompliant: use Assert.Fail
Assert.True(false, "Should not reach this line!"); // Noncompliant: use Assert.Fail
Assert.False(false); // Noncompliant: remove it
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="2" data-diff-type="compliant">
bool someResult;

Assert.False(someResult);
Assert.True(someResult);
Assert.True(someResult);
Assert.False(someResult);
Assert.Fail("Should not reach this line!");
Assert.Fail("Should not reach this line!");
// Removed
</pre>
<h2>How to fix it in xUnit</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="3" data-diff-type="noncompliant">
bool someResult;

Assert.Equal(false, someResult); // Noncompliant: use Assert.False
Assert.Equal(true, someResult); // Noncompliant: use Assert.True
Assert.NotEqual(false, someResult); // Noncompliant: use Assert.True
Assert.NotEqual(true, someResult); // Noncompliant: use Assert.False
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="3" data-diff-type="compliant">
bool someResult;

Assert.False(someResult);
Assert.True(someResult);
Assert.True(someResult);
Assert.False(someResult);
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://docs.nunit.org/">NUnit Documentation</a> </li>
<li> <a href="https://xunit.net/docs/getting-started/netcore/cmdline">xUnit Documentation</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-with-mstest">MSTest Documentation</a> </li>
<li> <a href="https://github.com/xunit/xunit/issues/2027">Xunit doesn’t have an Assert.Fail() operation</a> </li>
</ul>

Loading

0 comments on commit 7ae6c9a

Please sign in to comment.