Skip to content

Commit

Permalink
SONARFLEX-187 Update rule metadata (#197)
Browse files Browse the repository at this point in the history
  • Loading branch information
yassin-kammoun-sonarsource authored Nov 25, 2024
1 parent bae5be0 commit 005476c
Show file tree
Hide file tree
Showing 43 changed files with 348 additions and 194 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<h2>Why is this an issue?</h2>
<p>Programmers should not comment out code as it bloats programs and reduces readability.</p>
<p>Unused code should be deleted and can be retrieved from source control history if required.</p>
<p>Commented-out code distracts the focus from the actual executed code. It creates a noise that increases maintenance code. And because it is never
executed, it quickly becomes out of date and invalid.</p>
<p>Commented-out code should be deleted and can be retrieved from source control history if required.</p>

Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ <h3>Exceptions</h3>
</pre>
<h2>Resources</h2>
<ul>
<li> <a href="https://cwe.mitre.org/data/definitions/484">MITRE, CWE-484</a> - Omitted Break Statement in Switch </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/484">CWE-484 - Omitted Break Statement in Switch</a> </li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "HIGH"
"MAINTAINABILITY": "BLOCKER"
},
"attribute": "CLEAR"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@
"ruleSpecification": "RSPEC-2260",
"sqKey": "S2260",
"scope": "All",
"quickfix": "unknown"
"quickfix": "infeasible"
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
<h2>Why is this an issue?</h2>
<p>Merging collapsible <code>if</code> statements increases the code’s readability.</p>
<h3>Noncompliant code example</h3>
<p>Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as
possible, by avoiding unnecessary nesting, is considered a good practice.</p>
<p>Merging <code>if</code> statements when possible will decrease the nesting of the code and improve its readability.</p>
<p>Code like</p>
<pre>
if (condition1) {
if (condition2) { // NonCompliant
if (condition2) { // Noncompliant
...
}
}
</pre>
<h3>Compliant solution</h3>
<p>Is more readable as</p>
<pre>
if (condition1 &amp;&amp; condition2) {
...
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Collapsible \"if\" statements should be merged",
"title": "Mergeable \"if\" statements should be combined",
"type": "CODE_SMELL",
"code": {
"impacts": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
<h2>Why is this an issue?</h2>
<p>If a <code>private</code> field is declared but not used in the program, it can be considered dead code and should therefore be removed. This will
improve maintainability because developers will not wonder what the variable is used for.</p>
<h3>Noncompliant code example</h3>
<pre>
public class MyClass {
private var foo:int = 4; //foo is unused

public function compute(a:int):int{
return a * 4;
}
}
</pre>
<h3>Compliant solution</h3>
<p>If a <code>private</code> field is declared but not used locally, its limited visibility makes it dead code.</p>
<p>This is either a sign that some logic is missing or that the code should be cleaned.</p>
<p>Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand and preventing bugs from being introduced.</p>
<pre>
public class MyClass {
private var foo:int = 4; // Noncompliant: foo is unused and should be removed

public function compute(a:int):int{
return a * 4;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<h2>Why is this an issue?</h2>
<p>Functions with a long parameter list are difficult to use, as maintainers must figure out the role of each parameter and keep track of their
<p>Functions with a long parameter list are difficult to use because maintainers must figure out the role of each parameter and keep track of their
position.</p>
<pre>
function setCoordinates(var x1, var y1, var z1, var x2, var y2, var z2) { // Noncompliant
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
<h2>Why is this an issue?</h2>
<p>Empty statements, i.e. <code>;</code>, are usually introduced by mistake, for example because:</p>
<ul>
<li> It was meant to be replaced by an actual statement, but this was forgotten. </li>
<li> There was a typo which lead the semicolon to be doubled, i.e. <code>;;</code>. </li>
</ul>
<p>Empty statements represented by a semicolon <code>;</code> are statements that do not perform any operation. They are often the result of a typo or
a misunderstanding of the language syntax. It is a good practice to remove empty statements since they don’t add value and lead to confusion and
errors.</p>
<h3>Noncompliant code example</h3>
<pre>
function doSomething():void {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
<h2>Why is this an issue?</h2>
<p>Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of
code. Further, it could lead maintainers to introduce bugs because they think they’re using one variable but are really using another.</p>
<p>Shadowing occurs when a local variable has the same name as a variable or a field in an outer scope.</p>
<p>This can lead to three main problems:</p>
<ul>
<li> Confusion: The same name can refer to different variables in different parts of the scope, making the code hard to read and understand. </li>
<li> Unintended Behavior: You might accidentally use the wrong variable, leading to hard-to-detect bugs. </li>
<li> Maintenance Issues: If the inner variable is removed or renamed, the code’s behavior might change unexpectedly because the outer variable is
now being used. </li>
</ul>
<p>To avoid these problems, rename the shadowing, shadowed, or both identifiers to accurately represent their purpose with unique and meaningful
names.</p>
<p>This rule focuses on variables in methods that shadow a field.</p>
<h3>Noncompliant code example</h3>
<pre>
class Foo {
public var myField:int;

public function doSomething():String {
var myField:int = 0; /* Noncompliant */
...
var myField:int = 0; // Noncompliant
//...
}
}
</pre>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"title": "Local variables should not shadow class fields",
"type": "CODE_SMELL",
"code": {
"impacts": {
Expand All @@ -20,5 +19,6 @@
"ruleSpecification": "RSPEC-1117",
"sqKey": "S1117",
"scope": "All",
"quickfix": "unknown"
"quickfix": "unknown",
"title": "Local variables should not shadow class fields"
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
<h2>Why is this an issue?</h2>
<p>Redundant Boolean literals should be removed from expressions to improve readability.</p>
<h3>Noncompliant code example</h3>
<pre>
<p>A boolean literal can be represented in two different ways: <code>true</code> or <code>false</code>. They can be combined with logical operators
(<code>!, &amp;&amp;, ||, ==, !=</code>) to produce logical expressions that represent truth values. However, comparing a boolean literal to a
variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a
boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of
new bugs.</p>
<h2>How to fix it</h2>
<p>Remove redundant boolean literals from expressions to improve readability and make the code more maintainable.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
if (booleanMethod() == true) { /* ... */ }
if (booleanMethod() == false) { /* ... */ }
if (booleanMethod() || false) { /* ... */ }
Expand All @@ -14,8 +21,8 @@ <h3>Noncompliant code example</h3>
booleanVariable = booleanMethod() ? exp : true;
booleanVariable = booleanMethod() ? exp : false;
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
if (booleanMethod()) { /* ... */ }
if (!booleanMethod()) { /* ... */ }
if (booleanMethod()) { /* ... */ }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
<p>This rule raises an issue when a private function is never referenced in the code.</p>
<h2>Why is this an issue?</h2>
<p>Private functions that are never executed are dead code: unnecessary, inoperative code that should be removed. Cleaning out dead code decreases the
size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced.</p>
<h3>Noncompliant code example</h3>
<pre>
<p>A function that is never called is dead code, and should be removed. Cleaning out dead code decreases the size of the maintained codebase, making
it easier to understand the program and preventing bugs from being introduced.</p>
<p>This rule detects functions that are never referenced from inside a translation unit, and cannot be referenced from the outside.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
public class Foo
{
private function Foo(){} //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
public static function doSomething():void
public static function doSomething(): void // Compliant - public function
{
var foo:Foo = new Foo();
...
}
private function unusedPrivateFunction():void {...}

private function unusedPrivateFunction(): void {...} // Noncompliant
}
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public class Foo
{
private function Foo(){} //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
public static function doSomething():void
public static function doSomething(): void // Compliant - public function
{
var foo:Foo = new Foo();
...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
"constantCost": "2min"
},
"tags": [
"unused"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ <h3>Compliant solution</h3>
</pre>
<h2>Resources</h2>
<ul>
<li> <a href="https://cwe.mitre.org/data/definitions/489">MITRE, CWE-489</a> - Active Debug Code </li>
<li> <a href="https://cwe.mitre.org/data/definitions/570">MITRE, CWE-570</a> - Expression is Always False </li>
<li> <a href="https://cwe.mitre.org/data/definitions/571">MITRE, CWE-571</a> - Expression is Always True </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/489">CWE-489 - Active Debug Code</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/570">CWE-570 - Expression is Always False</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/571">CWE-571 - Expression is Always True</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
<p>Constants should be named consistently to communicate intent and improve maintainability. Rename your constants to follow your project’s naming
convention to address this issue.</p>
<h2>Why is this an issue?</h2>
<p>Shared coding conventions allow teams to collaborate efficiently. This rule checks that all constant names match a provided regular expression.</p>
<h3>Noncompliant code example</h3>
<p>With the default regular expression <code>^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$</code>:</p>
<pre>
<p>Constants are variables whose value does not change during the runtime of a program after initialization. Oftentimes, constants are used in
multiple locations across different subroutines.</p>
<p>It is important that the names of constants follow a consistent and easily recognizable pattern. This way, readers immediately understand that the
referenced value does not change, which simplifies debugging.</p>
<p>This rule checks that all constant names match a given regular expression.</p>
<h3>What is the potential impact?</h3>
<p>Ignoring the naming convention for constants makes the code less readable since constants and variables are harder to tell apart. Code that is hard
to understand is also difficult to maintain between different team members.</p>
<h2>How to fix it</h2>
<p>First, familiarize yourself with the particular naming convention of the project in question. Then, update the name of the constant to match the
convention, as well as all usages of the name. For many IDEs, you can use built-in renaming and refactoring features to update all usages of a
constant at once.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<p>The following example assumes that constant names should match the default regular expression <code>^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$</code>:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
public static const first:String = "first";
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public static const FIRST:String = "first";
</pre>

Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
<h2>Why is this an issue?</h2>
<p>Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that field
names match a provided regular expression.</p>
<h3>Noncompliant code example</h3>
<p>With the default regular expression <code>^[_a-z][a-zA-Z0-9]*$</code>:</p>
<pre>
<p>A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes.</p>
<p>The goal of a naming convention is to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures
consistency in the code, especially when multiple developers are working on the same project.</p>
<p>This rule checks that field names match a provided regular expression.</p>
<p>Using the regular expression <code>^[_a-z][a-zA-Z0-9]*$</code>, the noncompliant code below:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
class MyClass {
public var my_field:int;
}
</pre>
<h3>Compliant solution</h3>
<pre>
<p>Should be replaced with:</p>
<pre data-diff-id="1" data-diff-type="compliant">
public class MyClass {
public var myField:int;
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Wikipedia - <a href="https://en.wikipedia.org/wiki/Naming_convention_(programming)">Naming Convention (programming)</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -1,21 +1,61 @@
<p>Local variables and function parameters should be named consistently to communicate intent and improve maintainability. Rename your local variable
or function parameter to follow your project’s naming convention to address this issue.</p>
<h2>Why is this an issue?</h2>
<p>Shared naming conventions allow teams to collaborate effectively. This rule raises an issue when a local variable or function parameter name does
not match the provided regular expression.</p>
<h3>Noncompliant code example</h3>
<p>A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes.<br> Local
variables and function parameters hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily
recognizable pattern.<br> Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to
maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.</p>
<p>This rule checks that local variable and function parameter names match a provided regular expression.</p>
<h3>What is the potential impact?</h3>
<p>Inconsistent naming of local variables and function parameters can lead to several issues in your code:</p>
<ul>
<li> <strong>Reduced Readability</strong>: Inconsistent local variable and function parameter names make the code harder to read and understand;
consequently, it is more difficult to identify the purpose of each variable, spot errors, or comprehend the logic. </li>
<li> <strong>Difficulty in Identifying Variables</strong>: The local variables and function parameters that don’t adhere to a standard naming
convention are challenging to identify; thus, the coding process slows down, especially when dealing with a large codebase. </li>
<li> <strong>Increased Risk of Errors</strong>: Inconsistent or unclear local variable and function parameter names lead to misunderstandings about
what the variable represents. This ambiguity leads to incorrect assumptions and, consequently, bugs in the code. </li>
<li> <strong>Collaboration Difficulties</strong>: In a team setting, inconsistent naming conventions lead to confusion and miscommunication among
team members. </li>
<li> <strong>Difficulty in Code Maintenance</strong>: Inconsistent naming leads to an inconsistent codebase. The code is difficult to understand,
and making changes feels like refactoring constantly, as you face different naming methods. Ultimately, it makes the codebase harder to maintain.
</li>
</ul>
<p>In summary, not adhering to a naming convention for local variables and function parameters can lead to confusion, errors, and inefficiencies,
making the code harder to read, understand, and maintain.</p>
<h2>How to fix it</h2>
<p>First, familiarize yourself with the particular naming convention of the project in question. Then, update the name to match the convention, as
well as all usages of the name. For many IDEs, you can use built-in renaming and refactoring features to update all usages at once.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<p>With the default regular expression <code>^[_a-z][a-zA-Z0-9]*$</code>:</p>
<pre>
<pre data-diff-id="1" data-diff-type="noncompliant">
public function doSomething(my_param:int):void
{
var LOCAL:int;
...
}
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
public function doSomething(myParam):void
{
var local;
...
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Wikipedia - <a href="https://en.wikipedia.org/wiki/Naming_convention_(programming)">Naming Convention (programming)</a> </li>
</ul>
<h3>Related rules</h3>
<ul>
<li> {rule:flex:S100} - Function names should comply with a naming convention </li>
<li> {rule:flex:S101} - Class names should comply with a naming convention </li>
<li> {rule:flex:S115} - Constant names should comply with a naming convention </li>
<li> {rule:flex:S116} - Field names should comply with a naming convention </li>
<li> {rule:flex:S120} - Package names should comply with a naming convention </li>
<li> {rule:flex:S1312} - Loggers should be "private static const" and should share naming convention </li>
</ul>

Loading

0 comments on commit 005476c

Please sign in to comment.