Skip to content

Commit

Permalink
Updated rules metadata (#1629)
Browse files Browse the repository at this point in the history
  • Loading branch information
joke1196 authored Nov 1, 2023
1 parent 75c1690 commit cbda730
Show file tree
Hide file tree
Showing 55 changed files with 654 additions and 330 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,34 @@
<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:
if condition2: # Noncompliant
# ...
</pre>
<h3>Compliant solution</h3>
<p>Will be more readable as</p>
<pre>
if condition1 and condition2:
if condition1 and condition2: # Compliant
# ...
</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 file.isValid():
if file.isfile() or file.isdir(): # Noncompliant
# ...
</pre>
<h4>Compliant solution</h4>
<pre>
def isFileOrDirectory(File file):
return file.isFile() or file.isDirectory()

if file.isValid() and isFileOrDirectory(file): # Compliant
# ...
</pre>

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,6 +1,10 @@
<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>Parentheses can disambiguate the order of operations in complex expressions and make the code easier to understand.</p>
<pre>
a = (b * c) + (d * e) # Compliant: the intent is clear.
</pre>
<p>Redundant parentheses are parenthesis that do not change the behavior of the code, and do not clarify the intent. They can mislead and complexify
the code. They should be removed.</p>
<h3>Noncompliant code example</h3>
<pre>
return ((3)) # Noncompliant
Expand Down
Original file line number Diff line number Diff line change
@@ -1,92 +1,40 @@
<p>This rule raises an issue when a generic exception (such as <code>Exception</code> or <code>BaseException</code>) is raised.</p>
<h2>Why is this an issue?</h2>
<p>Raising instances of <a href="https://docs.python.org/3/library/exceptions.html#Exception"><code>Exception</code></a> and <a
href="https://docs.python.org/3/library/exceptions.html#BaseException"><code>BaseException</code></a> will have a negative impact on any code trying
to catch these exceptions.</p>
<p>First, the only way to handle differently multiple Exceptions is to check their message, which is error-prone and difficult to maintain.</p>
<p>What’s more, it becomes difficult to catch only your exception. The best practice is to catch only exceptions which require a specific handling.
When you raise <code>Exception</code> or <code>BaseException</code> in a function the caller will have to add an <code>except Exception</code> or
<code>except BaseException</code> and re-raise all exceptions which were unintentionally caught. This can create tricky bugs when the caller forgets
to re-raise exceptions such as <code>SystemExit</code> and the software cannot be stopped.</p>
<p>It is recommended to either:</p>
<p>From a consumer perspective, it is generally a best practice to only catch exceptions you intend to handle. Other exceptions should ideally not be
caught and let to propagate up the stack trace so that they can be dealt with appropriately. When a generic exception is thrown, it forces consumers
to catch exceptions they do not intend to handle, which they then have to re-raise.</p>
<p>Besides, when working with a generic type of exception, the only way to distinguish between multiple exceptions is to check their message, which is
error-prone and difficult to maintain. Legitimate exceptions may be unintentionally silenced and errors may be hidden.</p>
<p>For instance, if an exception such as <code>SystemExit</code> is caught and not re-raised, it will prevent the program from stopping.</p>
<p>When raising an exception, it is therefore recommended to raising the most specific exception possible so that it can be handled intentionally by
consumers.</p>
<h2>How to fix it</h2>
<p>To fix this issue, make sure to throw specific exceptions that are relevant to the context in which they arise. It is recommended to either:</p>
<ul>
<li> raise a more specific <a href="https://docs.python.org/3/library/exceptions.html">Built-in exception</a> when one matches. For example
<li> Raise a specific <a href="https://docs.python.org/3/library/exceptions.html">Built-in exception</a> when one matches. For example
<code>TypeError</code> should be raised when the type of a parameter is not the one expected. </li>
<li> create a custom exception class deriving from <code>Exception</code> or one of its subclasses. A common practice for libraries is to have one
custom root exception class from which every other custom exception class inherits. It enables other projects using this library to catch all errors
coming from the library with a single "except" statement </li>
<li> Create a custom exception class deriving from <code>Exception</code> or one of its subclasses. </li>
</ul>
<p>This rule raises an issue when <code>Exception</code> or <code>BaseException</code> are raised.</p>
<h3>Noncompliant code example</h3>
<pre>
def process1():
raise BaseException("Wrong user input for field X") # Noncompliant

def process2():
raise BaseException("Wrong configuration") # Noncompliant

def process3(param):
if not isinstance(param, int):
raise Exception("param should be an integer") # Noncompliant

def caller():
try:
process1()
process2()
process3()
except BaseException as e:
if e.args[0] == "Wrong user input for field X":
# process error
pass
elif e.args[0] == "Wrong configuration":
# process error
pass
else:
# re-raise other exceptions
raise
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
def check_value(value):
if value &lt; 0:
raise BaseException("Value cannot be negative") # Noncompliant: this will be difficult for consumers to handle
</pre>
<h3>Compliant solution</h3>
<pre>
class MyProjectError(Exception):
"""Exception class from which every exception in this library will derive.
It enables other projects using this library to catch all errors coming
from the library with a single "except" statement
"""
pass

class BadUserInputError(MyProjectError):
"""A specific error"""
pass

class ConfigurationError(MyProjectError):
"""A specific error"""
pass

def process1():
raise BadUserInputError("Wrong user input for field X")

def process2():
raise ConfigurationError("Wrong configuration")

def process3(param):
if not isinstance(param, int):
raise TypeError("param should be an integer")

def caller():
try:
process1()
process2()
process3()
except BadUserInputError as e:
# process error
pass
except ConfigurationError as e:
# process error
pass
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
def check_value(value):
if value &lt; 0:
raise ValueError("Value cannot be negative") # Compliant
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> PEP 352 - <a href="https://www.python.org/dev/peps/pep-0352/#exception-hierarchy-changes">Required Superclass for Exceptions</a> </li>
<li> Python Documentation - <a href="https://docs.python.org/3/library/exceptions.html#BaseException">Built-in exceptions</a> </li>
<li> <a href="https://cwe.mitre.org/data/definitions/397">MITRE, CWE-397</a> - Declaration of Throws for Generic Exception </li>
<li> PEP 352 - <a href="https://www.python.org/dev/peps/pep-0352/#exception-hierarchy-changes">Required Superclass for Exceptions</a> </li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "2min"
"constantCost": "1min"
},
"tags": [
"unused"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"title": "Files should contain an empty newline at the end",
"title": "Files should end with a newline",
"type": "CODE_SMELL",
"code": {
"impacts": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<h2>Why is this an issue?</h2>
<p>Trailing whitespaces are simply useless and should not stay in code. They may generate noise when comparing different versions of the same
file.</p>
<p>If you encounter issues from this rule, this probably means that you are not using an automated code formatter - which you should if you have the
opportunity to do so.</p>
<p>Trailing whitespaces bring no information, they may generate noise when comparing different versions of the same file, and they can create bugs
when they appear after a <code>\</code> marking a line continuation. They should be systematically removed.</p>
<p>An automated code formatter allows to completely avoid this family of issues and should be used wherever possible.</p>

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<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
<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 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>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<p>This rule raises an issue when a "class-private" method is never called inside the class where it was declared.</p>
<h2>Why is this an issue?</h2>
<p>"Class-Private" methods that are never executed inside their enclosing class 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>
<p>A method 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>Python has no real private methods. Every method is accessible. There are however two conventions indicating that a method is not meant to be
"public":</p>
<ul>
Expand All @@ -15,7 +15,8 @@ <h2>Why is this an issue?</h2>
</ul>
<p>This rule raises an issue when a class-private method (two leading underscores, max one underscore at the end) is never called inside the class.
Class methods, static methods and instance methods will all raise an issue.</p>
<h3>Noncompliant code example</h3>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre>
class Noncompliant:

Expand All @@ -30,7 +31,7 @@ <h3>Noncompliant code example</h3>
def __mangled_instance_method(self): # Noncompliant
print("__mangled_instance_method")
</pre>
<h3>Compliant solution</h3>
<h4>Compliant solution</h4>
<pre>
class Compliant:

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
@@ -1,15 +1,22 @@
<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-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-z0-9]*$</code>, the noncompliant code below:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
class MyClass:
myField = 1
</pre>
<h3>Compliant solution</h3>
<pre>
<p>Should be replaced with:</p>
<pre data-diff-id="1" data-diff-type="compliant">
class MyClass:
my_field = 1
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Python Enhancement Proposals - <a href="https://peps.python.org/pep-0008/#naming-conventions">PEP8 - Naming Conventions</a> </li>
<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,10 +1,69 @@
<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>
<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> Reduced Readability: 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> Difficulty in Identifying Variables: 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> Increased Risk of Errors: 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> Collaboration Difficulties: in a team setting, inconsistent naming conventions lead to confusion and miscommunication among team members. </li>
<li> Difficulty in Code Maintenance: 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>
<h3>Exceptions</h3>
<p>Loop counters are ignored by this rule.</p>
<p>Loop counters of one letter are ignored by this rule.</p>
<pre>
for i in range(limit): # Compliant
print(i)
</pre>
<p>Local variables matching regular expression <code>^[_A-Z][A-Z0-9_]*$</code> are considered as constant and ignored by this rule.<br> Function
parameters are not concerned by this exception.</p>
<pre>
def print_something(important_param):
LOCAL_VARIABLE = "" # Compliant
print(important_param + LOCAL_VARIABLE)
</pre>
<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-z0-9_]*$</code>:</p>
<pre data-diff-id="1" data-diff-type="noncompliant">
def print_something(IMPORTANT_PARAM): # Noncompliant
localVariable = "" # Noncompliant
print(IMPORTANT_PARAM + localVariable)
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
def print_something(important_param):
local_variable = ""
print(important_param + local_variable)
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Python Enhancement Proposals - <a href="https://peps.python.org/pep-0008/#naming-conventions">PEP8 - Naming Conventions</a> </li>
<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:python:S100} - Method names should comply with a naming convention </li>
<li> {rule:python:S101} - Class names should comply with a naming convention </li>
<li> {rule:python:S116} - Field names should comply with a naming convention </li>
<li> {rule:python:S1542} - Function names should comply with a naming convention </li>
<li> {rule:python:S1578} - Module names should comply with a naming convention </li>
<li> {rule:python:S2710} - The first argument to class methods should follow the naming convention </li>
</ul>

Loading

0 comments on commit cbda730

Please sign in to comment.