Skip to content

Commit

Permalink
[NO-JIRA] Update rule metadata (#14)
Browse files Browse the repository at this point in the history
  • Loading branch information
ADarko22 authored Sep 13, 2024
1 parent f5d8e45 commit f930b06
Show file tree
Hide file tree
Showing 23 changed files with 270 additions and 82 deletions.
2 changes: 1 addition & 1 deletion sonar-ruby-plugin/sonarpedia.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"languages": [
"RUBY"
],
"latest-update": "2023-10-02T13:52:14.395024Z",
"latest-update": "2024-09-13T13:59:33.064714Z",
"options": {
"no-language-in-filenames": true,
"preserve-filenames": true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,41 @@
<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 !filename.nil?
if File.file?(filename) || File.directory?(filename)
if a then
unless b then # Noncompliant
# ...
end
end

if a then
unless b then
</pre>
<p>Will be more readable as</p>
<pre>
if a &amp;&amp; !b then # Compliant
# ...
end
</pre>
<h2>How to fix it</h2>
<p>If merging the conditions seems to result in a more complex code, extracting the condition or part of it in a named function or variable is a
better approach to fix readability.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre>
if !filename.nil?
if File.file?(filename) || File.directory?(filename) # Noncompliant
# ...
end
end
</pre>
<h3>Compliant solution</h3>
<h4>Compliant solution</h4>
<pre>
def isFileOrDirectory(filename)
File.file?(filename) || File.directory?(filename)
end
# ...

if !filename.nil? &amp;&amp; isFileOrDirectory(filename)
# ...
end

if a &amp;&amp; !b then
if !filename.nil? &amp;&amp; isFileOrDirectory(filename) # Compliant
# ...
end
</pre>
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,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>
def modify(x1, y1, z1, x2, y2, z2) # Noncompliant
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<h2>Why is this an issue?</h2>
<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
redundant pairs of parentheses could be misleading, and should be removed.</p>
<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. However,
redundant pairs of parentheses could be misleading and should be removed.</p>
<h3>Noncompliant code example</h3>
<pre>
x = (y / 2 + 1) # Compliant even if the parenthesis are ignored by the compiler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ <h2>Why is this an issue?</h2>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> <a href="https://cwe.mitre.org/data/definitions/546">MITRE, CWE-546 - Suspicious Comment</a> </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/546">CWE-546 - Suspicious Comment</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<h2>Why is this an issue?</h2>
<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>
<p>Developers often use <code>TODO</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 rule aims to
identify and address unattended <code>TODO</code> tags to ensure a clean and maintainable codebase. This description explores 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
Expand All @@ -11,8 +11,8 @@ <h3>What is the potential impact?</h3>
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>Codebase Bloat: The 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>
Expand All @@ -23,6 +23,6 @@ <h3>Noncompliant code example</h3>
</pre>
<h2>Resources</h2>
<ul>
<li> <a href="https://cwe.mitre.org/data/definitions/546">MITRE, CWE-546</a> - Suspicious Comment </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/546">CWE-546 - Suspicious Comment</a> </li>
</ul>

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,4 +1,55 @@
<p>Functions and block parameters should be named consistently to communicate intent and improve maintainability. Rename your function or block
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 function or block parameter name does not match
the provided regular expression.</p>
<p>A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes.<br> Functions
and block 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 function and block parameter names match a provided regular expression.</p>
<h3>What is the potential impact?</h3>
<p>Inconsistent naming of functions and block parameters can lead to several issues in your code:</p>
<ul>
<li> <strong>Reduced Readability</strong>: Inconsistent function and block 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 functions and block 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 function and block 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 functions and block 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>^(@{0,2}[\da-z_]+[!?=]?)|([*+-/%=!&gt;&lt;~]+)|(\[]=?)$</code>:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
def show_something(text_Param) # Noncompliant
localVar = "" # Noncompliant
puts text_Param + localVar
end
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
def show_something(text_param)
local_var = ""
puts text_param + local_var
end
</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:ruby:S100} - Method names should comply with a naming convention </li>
<li> {rule:ruby:S101} - Class names should comply with a naming convention </li>
</ul>

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "function and block parameter names should comply with a naming convention",
"title": "Function and block parameter names should comply with a naming convention",
"type": "CODE_SMELL",
"code": {
"impacts": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
<h2>Why is this an issue?</h2>
<p>Unused parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same.</p>
<p>A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s
body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to
such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing
function parameters that are not being utilized is considered best practice.</p>

Original file line number Diff line number Diff line change
@@ -1,8 +1,37 @@
<h2>Why is this an issue?</h2>
<p>There are several reasons for a method not to have a method body:</p>
<p>An empty method is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty methods bring no
functionality and are misleading to others as they might think the method implementation fulfills a specific and identified requirement.</p>
<p>There are several reasons for a method not to have a body:</p>
<ul>
<li> It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production. </li>
<li> It is not yet, or never will be, supported. In this case an exception should be thrown. </li>
<li> The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override. </li>
</ul>
<h2>How to fix it</h2>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
def shouldNotBeEmpty() # Noncompliant - method is empty
end

def notImplemented() # Noncompliant - method is empty
end

def emptyOnPurpose() # Noncompliant - method is empty
end
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
def shouldNotBeEmpty()
doSomething()
end

def notImplemented()
raise NotImplementedError, 'notImplemented() cannot be performed because ...'
end

def emptyOnPurpose()
# comment explaining why the method is empty
end
</pre>

Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
<h2>Why is this an issue?</h2>
<p>Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.</p>
<p>On the other hand, constants can be referenced from many places, but only need to be updated in a single place.</p>
<h3>Noncompliant code example</h3>
<p>Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all
occurrences.</p>
<h3>Exceptions</h3>
<p>To prevent generating some false-positives, literals having 5 or less characters are excluded as well as literals containing only letters, digits
and '_'.</p>
<h2>How to fix it</h2>
<p>Use constants to replace the duplicated string literals. Constants can be referenced from many places, but only need to be updated in a single
place.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<p>With the default threshold of 3:</p>
<pre>
<pre data-diff-id="1" data-diff-type="noncompliant">
def foo()
prepare('action random1') #Noncompliant - "action random1" is duplicated 3 times
execute('action random1')
release('action random1')
end
</pre>
<h3>Compliant solution</h3>
<pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
def foo()
action1 = 'action random1'
prepare(action1)
execute(action1)
release(action1)
ACTION1 = 'action random1'
prepare(ACTION1)
execute(ACTION1)
release(ACTION1)
end
</pre>
<h3>Exceptions</h3>
<p>To prevent generating some false-positives, literals having 5 or less characters are excluded as well as literals containing only letters, digits
and '_'.</p>

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ <h3>Compliant solution</h3>
</pre>
<h2>Resources</h2>
<ul>
<li> <a href="https://cwe.mitre.org/data/definitions/478">MITRE, CWE-478</a> - Missing Default Case in Switch Statement </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/478">CWE-478 - Missing Default Case in Switch Statement</a> </li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ <h2>Exceptions</h2>
</ul>
<h2>See</h2>
<ul>
<li> <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">OWASP Top 10 2021 Category A1</a> - Broken Access Control </li>
<li> <a href="https://www.owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure">OWASP Top 10 2017 Category A3</a> - Sensitive Data
Exposure </li>
<li> OWASP - <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">Top 10 2021 Category A1 - Broken Access Control</a> </li>
<li> OWASP - <a href="https://owasp.org/www-project-top-ten/2017/A3_2017-Sensitive_Data_Exposure">Top 10 2017 Category A3 - Sensitive Data
Exposure</a> </li>
</ul>

Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
<h2>Why is this an issue?</h2>
<p>Nested <code>if</code>, <code>for</code>, <code>while</code>, <code>until</code>, <code>case</code> and <code>begin...rescue</code> statements are
key ingredients for making what’s known as "Spaghetti code".</p>
<p>Such code is hard to read, refactor and therefore maintain.</p>
<p>Nested control flow statements <code>if</code>, <code>for</code>, <code>while</code>, <code>until</code>, <code>case</code> and
<code>begin...rescue</code> are often key ingredients in creating what’s known as "Spaghetti code". This code smell can make your program difficult to
understand and maintain.</p>
<p>When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s
readability and maintainability, and it also complicates the testing process.</p>
<h2>Resources</h2>
<ul>
<li> <a href="https://en.wikipedia.org/wiki/Guard_(computer_science)">Guard clauses in programming</a> - one of the approaches to reducing the depth
of nesting </li>
</ul>

Original file line number Diff line number Diff line change
@@ -1,4 +1,38 @@
<h2>Why is this an issue?</h2>
<p>If a local variable is declared but not used, it is dead code and should be removed. Doing so will improve maintainability because developers will
not wonder what the variable is used for.</p>
<p>An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code,
contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain
clarity and efficiency.</p>
<h3>What is the potential impact?</h3>
<p>Having unused local variables in your code can lead to several issues:</p>
<ul>
<li> <strong>Decreased Readability</strong>: Unused variables can make your code more difficult to read. They add extra lines and complexity, which
can distract from the main logic of the code. </li>
<li> <strong>Misunderstanding</strong>: When other developers read your code, they may wonder why a variable is declared but not used. This can lead
to confusion and misinterpretation of the code’s intent. </li>
<li> <strong>Potential for Bugs</strong>: If a variable is declared but not used, it might indicate a bug or incomplete code. For example, if you
declared a variable intending to use it in a calculation, but then forgot to do so, your program might not work as expected. </li>
<li> <strong>Maintenance Issues</strong>: Unused variables can make code maintenance more difficult. If a programmer sees an unused variable, they
might think it is a mistake and try to 'fix' the code, potentially introducing new bugs. </li>
<li> <strong>Memory Usage</strong>: Although modern compilers are smart enough to ignore unused variables, not all compilers do this. In such cases,
unused variables take up memory space, leading to inefficient use of resources. </li>
</ul>
<p>In summary, unused local variables can make your code less readable, more confusing, and harder to maintain, and they can potentially lead to bugs
or inefficient memory use. Therefore, it is best to remove them.</p>
<h2>How to fix it</h2>
<p>The fix for this issue is straightforward. Once you ensure the unused variable is not part of an incomplete implementation leading to bugs, you
just need to remove it.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
def number_of_minutes(hours)
seconds = 0 # Noncompliant - seconds is unused
hours * 60
end
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
def number_of_minutes(hours)
hours * 60
end
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ <h3>Compliant solution</h3>
</pre>
<h2>Resources</h2>
<ul>
<li> <a href="https://cwe.mitre.org/data/definitions/561">MITRE, CWE-561</a> - Dead Code </li>
<li> CWE - <a href="https://cwe.mitre.org/data/definitions/561">CWE-561 - Dead Code</a> </li>
</ul>

Loading

0 comments on commit f930b06

Please sign in to comment.