Skip to content

Commit

Permalink
Update RSPEC before 9.7 release (#7724)
Browse files Browse the repository at this point in the history
  • Loading branch information
martin-strecker-sonarsource authored Aug 4, 2023
1 parent 28c59a4 commit d9025b5
Show file tree
Hide file tree
Showing 11 changed files with 195 additions and 45 deletions.
19 changes: 16 additions & 3 deletions analyzers/rspec/cs/S1135.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
<h2>Why is this an issue?</h2>
<p><code>TODO</code> tags are commonly used to mark places where some more code is required, but which the developer wants to implement later.</p>
<p>Sometimes the developer will not have the time or will simply forget to get back to that tag.</p>
<p>This rule is meant to track those tags and to ensure that they do not go unnoticed.</p>
<p>Developers often use <code>TOOO</code> tags to mark areas in the code where additional work or improvements are needed but are not implemented
immediately. However, these <code>TODO</code> tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This code smell
class aims to identify and address such unattended <code>TODO</code> tags to ensure a clean and maintainable codebase. This description will explore
why this is a problem and how it can be fixed to improve the overall code quality.</p>
<h3>What is the potential impact?</h3>
<p>Unattended <code>TODO</code> tags in code can have significant implications for the development process and the overall codebase.</p>
<p>Incomplete Functionality: When developers leave <code>TODO</code> tags without implementing the corresponding code, it results in incomplete
functionality within the software. This can lead to unexpected behavior or missing features, adversely affecting the end-user experience.</p>
<p>Missed Bug Fixes: If developers do not promptly address <code>TODO</code> tags, they might overlook critical bug fixes and security updates.
Delayed bug fixes can result in more severe issues and increase the effort required to resolve them later.</p>
<p>Impact on Collaboration: In team-based development environments, unattended <code>TODO</code> tags can hinder collaboration. Other team members
might not be aware of the intended changes, leading to conflicts or redundant efforts in the codebase.</p>
<p>Codebase Bloat: Accumulation of unattended <code>TODO</code> tags over time can clutter the codebase and make it difficult to distinguish between
work in progress and completed code. This bloat can make it challenging to maintain an organized and efficient codebase.</p>
<p>Addressing this code smell is essential to ensure a maintainable, readable, reliable codebase and promote effective collaboration among
developers.</p>
<h3>Noncompliant code example</h3>
<pre>
private void DoSomething()
Expand Down
2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S125.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@
"ruleSpecification": "RSPEC-125",
"sqKey": "S125",
"scope": "All",
"quickfix": "unknown"
"quickfix": "partial"
}
30 changes: 17 additions & 13 deletions analyzers/rspec/cs/S2190.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ <h2>Why is this an issue?</h2>
</pre>
<p>This can happen in multiple scenarios.</p>
<h3>Loop statements</h3>
<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
<code>false</code> will be indefinitely executed.</p>
<p><code>while</code> and <code>for</code> loops with no <code>break</code> or <code>return</code> statements that have exit conditions which are
always <code>false</code> will be indefinitely executed.</p>
<h3>"goto" statements</h3>
<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>
<h3>Recursion</h3>
Expand Down Expand Up @@ -88,8 +88,8 @@ <h4>Compliant solution</h4>
}
</pre>
<ul>
<li> As <a href="https://rules.sonarsource.com/csharp/RSPEC-907">{rule:csharpsquid:S907}</a> generally suggests, avoid using <code>goto</code>
statements. Instead, you can use a loop statement or explicit recursion. </li>
<li> As {rule:csharpsquid:S907} generally suggests, avoid using <code>goto</code> statements. Instead, you can use a loop statement or explicit
recursion. </li>
</ul>
<h4>Noncompliant code example</h4>
<pre data-diff-id="2" data-diff-type="noncompliant">
Expand Down Expand Up @@ -143,18 +143,22 @@ <h4>Compliant solution</h4>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-for-statement">The "for"
statement</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-while-statement">The "while"
statement</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/jump-statements#the-goto-statement">The "goto"
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-for-statement">The "for" statement</a>
</li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/iteration-statements#the-while-statement">The "while"
statement</a> </li>
<li> <a href="https://en.wikipedia.org/wiki/Recursion_(computer_science)">Recursion - wiki</a> </li>
<li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.stackoverflowexception?view=net-7.0">StackOverflowException class</a> </li>
<li> <a href="https://rules.sonarsource.com/csharp/RSPEC-907">{rule:csharpsquid:S907}: "goto" statement should not be used</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/jump-statements#the-goto-statement">The "goto" statement</a>
</li>
<li> Wikipedia <a href="https://en.wikipedia.org/wiki/Recursion_(computer_science)">Recursion - wiki</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/api/system.stackoverflowexception?view=net-7.0">StackOverflowException
class</a> </li>
<li> {rule:csharpsquid:S907} - "goto" statement should not be used </li>
</ul>
<h3>Articles &amp; blog posts</h3>
<ul>
<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>
<li> Edsger Dijkstra - <a href="https://www.cs.utexas.edu/users/EWD/transcriptions/EWD02xx/EWD215.html">A Case against the GO TO Statement</a> </li>
</ul>

2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S2995.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ <h2>How to fix it</h2>
<p>Note that in the case of <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/struct">structure types</a>, it
is recommended to <a
href="https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/statements-expressions-operators/how-to-define-value-equality-for-a-type#struct-example">implement
value equality</a>. If not, <a href="https://rules.sonarsource.com/csharp/RSPEC-3898">{rule:csharpsquid:S3898}</a> might raise.</p>
value equality</a>. If not, {rule:csharpsquid:S3898} might raise.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
Expand Down
2 changes: 1 addition & 1 deletion analyzers/rspec/cs/S3456.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<h2>Why is this an issue?</h2>
<p>The <code>string</code> type offers an indexer property that allows you to treat it as a <code>char</code> array. Therefore, if you just need to
access a specific character or iterate over all of them, the <code>ToCharArray</code> call should be omitted. For these cases, not omitting makes the
code harder to read and less performant as <code>ToCharArray</code> copies the characters from the <code>string</code> object into a new Unicode
code harder to read and less efficient as <code>ToCharArray</code> copies the characters from the <code>string</code> object into a new Unicode
character array.</p>
<p>The same principle applies to <a href="https://devblogs.microsoft.com/dotnet/csharp-11-preview-updates/#utf-8-string-literals">utf-8 literals
types</a> (<code>ReadOnlySpan&lt;byte&gt;</code>, <code>Span&lt;byte&gt;</code>) and the <a
Expand Down
38 changes: 37 additions & 1 deletion analyzers/rspec/cs/S3925.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,47 @@ <h2>Why is this an issue?</h2>
<p>Classes that inherit from <a href="https://learn.microsoft.com/en-us/dotnet/api/system.exception"><code>Exception</code></a> are implementing
<code>ISerializable</code>. Make sure the <code>[Serializable]</code> attribute is used and that <code>ISerializable</code> is correctly implemented.
Even if you don’t plan to explicitly serialize the object yourself, it might still require serialization, for instance when crossing the boundary of
an <code>AppDomain</code>.</p>
an <a href="https://learn.microsoft.com/en-us/dotnet/api/system.appdomain"><code>AppDomain</code></a>.</p>
<p>This rule only raises an issue on classes that indicate that they are interested in serialization (see the <em>Exceptions</em> section). That is to
reduce noise because a lot of classes in the base class library are implementing <code>ISerializable</code>, including the following classes: <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.exception"><code>Exception</code></a>, <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.uri"><code>Uri</code></a>, <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.hashtable"><code>Hashtable</code></a>, <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2"><code>Dictionary&lt;TKey,TValue&gt;</code></a>, <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.data.dataset"><code>DataSet</code></a>, <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.net.httpwebrequest"><code>HttpWebRequest</code></a>, <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex"><code>Regex</code></a> <a
href="https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.treenode"><code>TreeNode</code></a>, and others. There is often no need to add
serialization support in classes derived from these types.</p>
<h3>Exceptions</h3>
<ul>
<li> Classes in test projects are not checked. </li>
<li> Classes need to indicate that they are interested in serialization support by either
<ol>
<li> Applying the <code>[Serializable]</code> attribute </li>
<li> Having <code>ISerializable</code> in their base type list </li>
<li> Declaring a <a
href="https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/serialization#supporting-runtime-serialization">serialization
constructor</a> </li>
</ol> </li>
</ul>
<pre>
[Serializable] // 1.
public class SerializationOptIn_Attribute
{
}

public class SerializationOptIn_Interface : ISerializable // 2.
{
}

public class SerializationOptIn_Constructor
{
protected SerializationOptIn_Constructor(SerializationInfo info, StreamingContext context) // 3.
{
}
}
</pre>
<h2>How to fix it</h2>
<p>Make sure to follow the <a href="https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/serialization">recommended guidelines</a> when
implementing <code>ISerializable</code>.</p>
Expand Down
112 changes: 96 additions & 16 deletions analyzers/rspec/cs/S4023.html
Original file line number Diff line number Diff line change
@@ -1,27 +1,107 @@
<p>Empty interfaces should be avoided as they do not provide any functional requirements for implementing classes.</p>
<h2>Why is this an issue?</h2>
<p>Empty interfaces are usually used as a marker or a way to identify groups of types. The preferred way to achieve this is to use custom
attributes.</p>
<h3>Noncompliant code example</h3>
<p>Empty interfaces are either useless or used as a <a href="https://en.wikipedia.org/wiki/Marker_interface_pattern">marker</a>. <a
href="https://learn.microsoft.com/en-us/dotnet/standard/attributes/writing-custom-attributes">Custom attributes</a> are a better alternative to marker
interfaces. See the <em>How to fix it</em> section for more information.</p>
<h3>Exceptions</h3>
<p>This rule doesn’t raise in any of the following cases:</p>
<h4>Aggregation of multiple interfaces</h4>
<pre>
using System;
public interface IAggregate: IComparable, IFormattable { } // Compliant: Aggregates two interfaces
</pre>
<h4>Generic specialization</h4>
<p>An empty interface with a single base interface is compliant as long as the resulting interface binds a generic parameter or constrains it.</p>
<pre>
// Compliant: Bound to a concrete type
public interface IStringEquatable: IEquatable&lt;string&gt; { }
// Compliant: Specialized by type parameter constraint
public interface ICreateableEquatable&lt;T&gt;: IEquatable&lt;T&gt; where T: new() { }
</pre>
<h4>Custom attribute</h4>
<p>An empty interface is compliant if a custom attribute is applied to the interface.</p>
<pre>
[Obsolete]
public interface ISorted { } // Compliant: An attribute is applied to the interface declaration
</pre>
<h2>How to fix it</h2>
<p>Do any of the following to fix the issue:</p>
<ul>
<li> Add members to the interface </li>
<li> Remove the useless interface </li>
<li> Replace the usage as a marker interface with custom attributes </li>
</ul>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<p>The empty interface does not add any functionality.</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
public interface IFoo // Noncompliant
{

namespace MyLibrary
}
</pre>
<h4>Compliant solution</h4>
<p>Add members to the interface to be compliant.</p>
<pre data-diff-id="1" data-diff-type="compliant">
public interface IFoo
{
public interface MyInterface // Noncompliant
{
}
void Bar();
}
</pre>
<h3>Compliant solution</h3>
<pre>
using System;
<h4>Noncompliant code example</h4>
<p>A typical use case for marker interfaces is doing type inspection via <a
href="https://learn.microsoft.com/en-us/dotnet/framework/reflection-and-codedom/reflection">reflection</a> as shown below.</p>
<p>The <code>IIncludeFields</code> marker interface is used to configure the JSON serialization of an object.</p>
<pre data-diff-id="2" data-diff-type="noncompliant">
// An example marker interface
public interface IIncludeFields { }

public class OptInToIncludeFields: IIncludeFields { }

Serialize(new OptInToIncludeFields());

void Serialize&lt;T&gt;(T o)
{
// Use reflection to check if the interface is applied to the type
var includeFields = o.GetType()
.GetInterfaces().Any(i =&gt; i == typeof(IIncludeFields));
var options = new JsonSerializerOptions()
{
// Take decisions based on the presence of the marker
IncludeFields = includeFields,
};
}
</pre>
<p>The same example can be rewritten using custom attributes. This approach is preferable because it is more fine-grained, allows parameterization,
and is more flexible in type hierarchies.</p>
<pre data-diff-id="2" data-diff-type="compliant">
// A custom attribute used as a marker
[AttributeUsage(AttributeTargets.Class)]
public sealed class IncludeFieldsAttribute: Attribute { }

[IncludeFields]
public class OptInToIncludeFields { }

Serialize(new OptInToIncludeFields());

namespace MyLibrary
void Serialize&lt;T&gt;(T o)
{
public interface MyInterface
{
void Foo();
}
// Use reflection to check if the attribute is applied to the type
var includeFields = o.GetType()
.GetCustomAttributes(typeof(IncludeFieldsAttribute), inherit: true).Any();
var options = new JsonSerializerOptions()
{
// Take decisions based on the presence of the marker
IncludeFields = includeFields,
};
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/types/interfaces">Interfaces - define behavior for
multiple types</a> </li>
<li> Wikipedia - <a href="https://en.wikipedia.org/wiki/Marker_interface_pattern">Marker interface pattern</a> </li>
<li> Microsoft Learn - <a href="https://learn.microsoft.com/en-us/dotnet/standard/attributes/writing-custom-attributes">Writing custom
attributes</a> </li>
</ul>

12 changes: 8 additions & 4 deletions analyzers/rspec/cs/S6422.html
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,14 @@ <h2>Why is this an issue?</h2>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://msdn.microsoft.com/en-us/magazine/jj991977.aspx">Async/Await - Best Practices in Asynchronous Programming</a> </li>
<li> <a href="https://docs.microsoft.com/en-us/azure/azure-functions/performance-reliability#use-async-code-but-avoid-blocking-calls">Improve the
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming">Async/Await - Best
Practices in Asynchronous Programming</a> </li>
<li> Microsoft Learn - <a
href="https://learn.microsoft.com/en-us/azure/azure-functions/performance-reliability#use-async-code-but-avoid-blocking-calls">Improve the
performance and reliability of Azure Functions - Scalability best practices</a> </li>
<li> <a href="https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md">Async Guidance - David Fowl</a> </li>
<li> <a href="https://rules.sonarsource.com/csharp/RSPEC-4462">{rule:csharpsquid:S4462} - a more general version of this rule</a> </li>
<li> Github - <a href="https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md">Async Guidance by David Fowler</a>
</li>
<li> {rule:csharpsquid:S4462} - Calls to "async" methods should not be blocking (a more general version of this rule) </li>
</ul>

19 changes: 16 additions & 3 deletions analyzers/rspec/vbnet/S1135.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
<h2>Why is this an issue?</h2>
<p><code>TODO</code> tags are commonly used to mark places where some more code is required, but which the developer wants to implement later.</p>
<p>Sometimes the developer will not have the time or will simply forget to get back to that tag.</p>
<p>This rule is meant to track those tags and to ensure that they do not go unnoticed.</p>
<p>Developers often use <code>TOOO</code> tags to mark areas in the code where additional work or improvements are needed but are not implemented
immediately. However, these <code>TODO</code> tags sometimes get overlooked or forgotten, leading to incomplete or unfinished code. This code smell
class aims to identify and address such unattended <code>TODO</code> tags to ensure a clean and maintainable codebase. This description will explore
why this is a problem and how it can be fixed to improve the overall code quality.</p>
<h3>What is the potential impact?</h3>
<p>Unattended <code>TODO</code> tags in code can have significant implications for the development process and the overall codebase.</p>
<p>Incomplete Functionality: When developers leave <code>TODO</code> tags without implementing the corresponding code, it results in incomplete
functionality within the software. This can lead to unexpected behavior or missing features, adversely affecting the end-user experience.</p>
<p>Missed Bug Fixes: If developers do not promptly address <code>TODO</code> tags, they might overlook critical bug fixes and security updates.
Delayed bug fixes can result in more severe issues and increase the effort required to resolve them later.</p>
<p>Impact on Collaboration: In team-based development environments, unattended <code>TODO</code> tags can hinder collaboration. Other team members
might not be aware of the intended changes, leading to conflicts or redundant efforts in the codebase.</p>
<p>Codebase Bloat: Accumulation of unattended <code>TODO</code> tags over time can clutter the codebase and make it difficult to distinguish between
work in progress and completed code. This bloat can make it challenging to maintain an organized and efficient codebase.</p>
<p>Addressing this code smell is essential to ensure a maintainable, readable, reliable codebase and promote effective collaboration among
developers.</p>
<h3>Noncompliant code example</h3>
<pre>
Sub DoSomething()
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": "2023-07-24T14:55:21.573727300Z",
"latest-update": "2023-08-04T09:54:32.121596400Z",
"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": "2023-07-24T14:54:55.036971700Z",
"latest-update": "2023-08-04T09:54:42.570864700Z",
"options": {
"no-language-in-filenames": true
}
Expand Down

0 comments on commit d9025b5

Please sign in to comment.