Skip to content

Commit

Permalink
Update RSPECs as part of the LayC second migration (batch#4) (#7540)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregory-paidis-sonarsource authored Jul 5, 2023
1 parent 5b9856d commit 5735c1f
Show file tree
Hide file tree
Showing 17 changed files with 170 additions and 75 deletions.
48 changes: 32 additions & 16 deletions analyzers/rspec/cs/S1048.html
Original file line number Diff line number Diff line change
@@ -1,38 +1,54 @@
<h2>Why is this an issue?</h2>
<p>Finalizers (historically referred to as destructors) are used to perform any necessary final clean-up when the garbage collector is collecting a
class instance. The programmer has no control over when the destructor is called; the garbage collector decides when to call it, and the destructor
implicitly calls <code>Finalize</code> on the object’s base class.</p>
<p>When you create a destructor, you should never throw an exception in it, as you are risking having your application terminated without a graceful
cleanup.</p>
<p>If a destructor throws an exception, and the runtime is not hosted by an application that overrides the default policy and the behavior of the
<code>UnhandledExceptionEventHandler</code>, then the runtime terminates the process immediately without graceful cleanup (<code>finally</code> blocks
and <code>Finalizer</code> methods are not executed).</p>
<p>The rule raises an issue on <code>throw</code> statements used in destructors.</p>
<h3>Noncompliant code example</h3>
<p>The <a href="https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/finalizers">finalizers</a> are used to perform
<a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/fundamentals#unmanaged-resources">any necessary final clean-up</a> when
the garbage collector is collecting a class instance. The programmer has no control over when the finalizer is called; the garbage collector decides
when to call it.</p>
<p>When creating a finalizer, it should never throw an exception, as there is a high risk of having the application terminated leaving unmanaged
resources without a graceful cleanup.</p>
<p>The rule raises an issue on <code>throw</code> statements used in a finalizer.</p>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
class MyClass
{
~MyClass()
{
throw new NotImplementedException(); // Noncompliant
throw new NotImplementedException(); // Noncompliant: finalizer throws an exception
}
}
</pre>
<h3>Compliant solution</h3>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
class MyClass
{
~MyClass()
{

// Compliant: finalizer does not throw an exception
}
}
</pre>
<h3>Going the extra mile</h3>
<p>In general object finalization can be a complex and error-prone operation and should not be implemented except within the <a
href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose">dispose pattern</a>.</p>
<p>When <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/unmanaged">cleaning up unmanaged resources</a>, it is
recommended to implement the dispose pattern or, to cover uncalled <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose"><code>Dispose</code></a> method by the consumer, implement <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.safehandle"><code>SafeHandle</code></a>.</p>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/finalizers">Finalizers (destructors)</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.appdomain">App Domain</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.appdomain.unhandledexception">AppDomain.UnhandledException Event</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/fundamentals">Fundamentals of garbage
collection</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/unmanaged">Cleaning up unmanaged resources</a>
</li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose">Implement a Dispose
method</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.safehandle"><code>SafeHandle</code>
Class</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose"><code>IDisposable.Dispose</code> Method</a>
</li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/finalizers">Finalizers
(destructors)</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S1048.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Destructors should not throw exceptions",
"title": "Finalizers should not throw exceptions",
"type": "BUG",
"status": "ready",
"remediation": {
Expand Down
48 changes: 36 additions & 12 deletions analyzers/rspec/cs/S1848.html
Original file line number Diff line number Diff line change
@@ -1,16 +1,40 @@
<h2>Why is this an issue?</h2>
<p>There is no good reason to create a new object to not do anything with it. Most of the time, this is due to a missing piece of code and so could
lead to an unexpected behavior in production.</p>
<p>If it was done on purpose because the constructor has side-effects, then that side-effect code should be moved into a separate, static method and
called directly.</p>
<h3>Noncompliant code example</h3>
<pre>
if (x &lt; 0)
new ArgumentException("x must be nonnegative");
<p>Creating objects that are not used is a vulnerability that can lead to unexpected behavior.</p>
<p>If this was done intentionally due to side effects in the object’s constructor, the code should be moved to a dedicated method.</p>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public void Method(MyObject myObject)
{
if (myObject is null)
{
new MyObject(); // Noncompliant
}

if (myObject.IsCorrupted)
{
new ArgumentException($"{nameof(myObject)} is corrupted"); // Noncompliant
}

// ...
}
</pre>
<h3>Compliant solution</h3>
<pre>
if (x &lt; 0)
throw new ArgumentException("x must be nonnegative");
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public void Method(MyObject myObject)
{
if (myObject is null)
{
myObject = new MyObject(); // Compliant
}

if (myObject.IsCorrupted)
{
throw new ArgumentException($"{nameof(myObject)} is corrupted"); // Compliant
}

// ...
}
</pre>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S1848.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
"ruleSpecification": "RSPEC-1848",
"sqKey": "S1848",
"scope": "Main",
"quickfix": "unknown"
"quickfix": "infeasible"
}
24 changes: 17 additions & 7 deletions analyzers/rspec/cs/S3984.html
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
<h2>Why is this an issue?</h2>
<p>Creating a new <code>Exception</code> without actually throwing it is useless and is probably due to a mistake.</p>
<h3>Noncompliant code example</h3>
<pre>
<p>Creating a new <a href="https://learn.microsoft.com/en-us/dotnet/api/system.exception"><code>Exception</code></a> without actually throwing does
not achieve the intended purpose.</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
if (x &lt; 0)
{
new ArgumentException("x must be nonnegative");
new ArgumentException("x must be nonnegative");
}
</pre>
<h3>Compliant solution</h3>
<pre>
<p>Ensure to throw the <code>Exception</code> with a <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/exception-handling-statements#the-throw-statement"><code>throw</code>
statement</a>.</p>
<pre data-diff-id="1" data-diff-type="compliant">
if (x &lt; 0)
{
throw new ArgumentException("x must be nonnegative");
throw new ArgumentException("x must be nonnegative");
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.exception"><code>Exception</code> Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/exception-handling-statements">Exception-handling statements -
<code>throw</code>, <code>try-catch</code>, <code>try-finally</code>, and <code>try-catch-finally</code></a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S3984.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@
"ruleSpecification": "RSPEC-3984",
"sqKey": "S3984",
"scope": "All",
"quickfix": "unknown"
"quickfix": "targeted"
}
30 changes: 22 additions & 8 deletions analyzers/rspec/cs/S4260.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
<h2>Why is this an issue?</h2>
<p>When creating a custom Markup Extension that accepts parameters in WPF, the <code>ConstructorArgument</code> markup must be used to identify the
discrete properties that match these parameters. However since this is done via a string, the compiler will not notice if there are typos.</p>
<p>When creating a custom <a href="https://learn.microsoft.com/en-us/dotnet/desktop/wpf/advanced/markup-extensions-and-wpf-xaml">Markup Extension</a>
that accepts parameters in WPF, the <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.markup.constructorargumentattribute"><code>ConstructorArgument</code></a> markup
must be used to identify the discrete properties that match these parameters. However since this is done via a string, the compiler won’t give you any
warning in case there are typos.</p>
<p>This rule raises an issue when the string argument to <code>ConstructorArgumentAttribute</code> doesn’t match any parameter of any constructor.</p>
<h3>Noncompliant code example</h3>
<pre>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
using System;

namespace myLibrary
namespace MyLibrary
{
public class MyExtension : MarkupExtension
{
Expand All @@ -22,11 +27,11 @@ <h3>Noncompliant code example</h3>
}
}
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
using System;

namespace myLibrary
namespace MyLibrary
{
public class MyExtension : MarkupExtension
{
Expand All @@ -42,4 +47,13 @@ <h3>Compliant solution</h3>
}
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/desktop/wpf/advanced/markup-extensions-and-wpf-xaml">Markup Extensions and
WPF XAML</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.markup.markupextension">MarkupExtension Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.markup.constructorargumentattribute">ConstructorArgumentAttribute Class</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S6588.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<h2>Why is this an issue?</h2>
<p>With .NET core the <code>UnixEpoch</code> field was introduced to <code>DateTime</code> and <code>DateTimeOffset</code> types. Using this field
<p>With .NET Core the <code>UnixEpoch</code> field was introduced to <code>DateTime</code> and <code>DateTimeOffset</code> types. Using this field
clearly states that the intention is to use the beginning of the Unix epoch.</p>
<h3>What is the potential impact?</h3>
<p>You should not use the <code>DateTime</code> or <code>DateTimeOffset</code> constructors to set the time to the 1st of January 1970 to represent
Expand Down
2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S6588.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
"ruleSpecification": "RSPEC-6588",
"sqKey": "S6588",
"scope": "All",
"quickfix": "targeted"
"quickfix": "covered"
}
2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S6610.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@
"ruleSpecification": "RSPEC-6610",
"sqKey": "S6610",
"scope": "All",
"quickfix": "targeted"
"quickfix": "covered"
}
45 changes: 31 additions & 14 deletions analyzers/rspec/vbnet/S1048.html
Original file line number Diff line number Diff line change
@@ -1,32 +1,49 @@
<h2>Why is this an issue?</h2>
<p>The Finalize method is used to perform cleanup operations on unmanaged resources held by the current object before the object is destroyed. The
method is protected and therefore is accessible only through this class or through a derived class. When you override <code>Finalize</code>, you
should never throw an exception in it, as you are risking having your application terminated without a graceful cleanup.</p>
<p>If <code>Finalize</code> or an override of <code>Finalize</code> throws an exception and the runtime is not hosted by an application that overrides
the default policy and the behavior of the <code>UnhandledExceptionEventHandler</code>, then the runtime terminates the process immediately without
graceful cleanup (<code>finally</code> blocks and <code>Finalizer</code> methods are not executed).</p>
<p>The rule reports on throw statements used in finalizers.</p>
<h3>Noncompliant code example</h3>
<p>The <a href="https://learn.microsoft.com/en-us/dotnet/api/system.object.finalize">Finalize methods</a> are used to perform <a
href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/fundamentals#unmanaged-resources">any necessary final clean-up</a> when the
garbage collector is collecting a class instance. The programmer has no control over when the Finalize method is called; the garbage collector decides
when to call it.</p>
<p>When creating a Finalize method, it should never throw an exception, as there is a high risk of having the application terminated leaving unmanaged
resources without a graceful cleanup.</p>
<p>The rule raises an issue on <code>throw</code> statements used in a Finalize method.</p>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
Class MyClass
Protected Overrides Sub Finalize()
Throw New NotImplementedException() ' Noncompliant
Throw New NotImplementedException() ' Noncompliant: Finalize method throws an exception
End Sub
End Class
</pre>
<h3>Compliant solution</h3>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
Class MyClass
Protected Overrides Sub Finalize()

' Noncompliant: Finalize method does not throw an exception
End Sub
End Class
</pre>
<h3>Going the extra mile</h3>
<p>In general object finalization can be a complex and error-prone operation and should not be implemented except within the <a
href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose">dispose pattern</a>.</p>
<p>When <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/unmanaged">cleaning up unmanaged resources</a>, it is
recommended to implement the dispose pattern or, to cover uncalled <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose"><code>Dispose</code></a> method by the consumer, implement <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.safehandle"><code>SafeHandle</code></a>.</p>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.object.finalize">Object.Finalize method</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.appdomain">App Domain</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.appdomain.unhandledexception">AppDomain.UnhandledException Event</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/fundamentals">Fundamentals of garbage
collection</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/unmanaged">Cleaning up unmanaged resources</a>
</li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose">Implement a Dispose
method</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.safehandle"><code>SafeHandle</code>
Class</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose"><code>IDisposable.Dispose</code> Method</a>
</li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.object.finalize">Object.Finalize method</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/vbnet/S1048.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Finalizers should not throw exceptions",
"title": "Finalize method should not throw exceptions",
"type": "BUG",
"status": "ready",
"remediation": {
Expand Down
28 changes: 21 additions & 7 deletions analyzers/rspec/vbnet/S4260.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
<h2>Why is this an issue?</h2>
<p>When creating a custom Markup Extension that accepts parameters in WPF, the <code>ConstructorArgument</code> markup must be used to identify the
discrete properties that match these parameters. However since this is done via a string, the compiler will not notice if there are typos.</p>
<p>When creating a custom <a href="https://learn.microsoft.com/en-us/dotnet/desktop/wpf/advanced/markup-extensions-and-wpf-xaml">Markup Extension</a>
that accepts parameters in WPF, the <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.markup.constructorargumentattribute"><code>ConstructorArgument</code></a> markup
must be used to identify the discrete properties that match these parameters. However since this is done via a string, the compiler won’t give you any
warning in case there are typos.</p>
<p>This rule raises an issue when the string argument to <code>ConstructorArgumentAttribute</code> doesn’t match any parameter of any constructor.</p>
<h3>Noncompliant code example</h3>
<pre>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
Imports System

Namespace myLibrary
Namespace MyLibrary
Public Class MyExtension
Inherits MarkupExtension

Expand All @@ -22,8 +27,8 @@ <h3>Noncompliant code example</h3>
End Class
End Namespace
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
Imports System

Namespace MyLibrary
Expand All @@ -42,4 +47,13 @@ <h3>Compliant solution</h3>
End Class
End Namespace
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/desktop/wpf/advanced/markup-extensions-and-wpf-xaml">Markup Extensions and
WPF XAML</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.markup.markupextension">MarkupExtension Class</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.markup.constructorargumentattribute">ConstructorArgumentAttribute Class</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/vbnet/S6588.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<h2>Why is this an issue?</h2>
<p>With .NET core the <code>UnixEpoch</code> field was introduced to <code>DateTime</code> and <code>DateTimeOffset</code> types. Using this field
<p>With .NET Core the <code>UnixEpoch</code> field was introduced to <code>DateTime</code> and <code>DateTimeOffset</code> types. Using this field
clearly states that the intention is to use the beginning of the Unix epoch.</p>
<h3>What is the potential impact?</h3>
<p>You should not use the <code>DateTime</code> or <code>DateTimeOffset</code> constructors to set the time to the 1st of January 1970 to represent
Expand Down
2 changes: 1 addition & 1 deletion analyzers/rspec/vbnet/S6588.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
"ruleSpecification": "RSPEC-6588",
"sqKey": "S6588",
"scope": "All",
"quickfix": "targeted"
"quickfix": "covered"
}
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": "2023-07-04T14:57:11.897522700Z",
"latest-update": "2023-07-05T14:00:08.394905700Z",
"options": {
"no-language-in-filenames": true
}
Expand Down
Loading

0 comments on commit 5735c1f

Please sign in to comment.