Modify S109: Migrate To LayC (#3235)
## Review A dedicated reviewer checked the rule description successfully for: - [x] logical errors and incorrect information - [x] information gaps and missing content - [x] text style and tone - [x] PR summary and labels follow [the guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule)
This commit is contained in:
parent
ccbf7d7edc
commit
bc15ffe77e
@ -1,23 +1,25 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
include::../description.adoc[]
|
include::../description.adoc[]
|
||||||
|
|
||||||
=== Noncompliant code example
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
[source,abap]
|
=== Code examples
|
||||||
|
|
||||||
|
==== Noncompliant code example
|
||||||
|
|
||||||
|
[source,abap,diff-id=1,diff-type=noncompliant]
|
||||||
----
|
----
|
||||||
IF sy-subrc EQ 42.
|
IF sy-subrc EQ 42. " Noncompliant - 5 is a magic number
|
||||||
screen-request = 45.
|
screen-request = 'OK'.
|
||||||
ENDIF.
|
ENDIF.
|
||||||
----
|
----
|
||||||
|
|
||||||
=== Compliant solution
|
==== Compliant solution
|
||||||
|
|
||||||
[source,abap]
|
[source,abap,diff-id=1,diff-type=compliant]
|
||||||
----
|
----
|
||||||
answer = 42.
|
answer = 42.
|
||||||
IF sy-subrc EQ answer.
|
IF sy-subrc EQ answer. " Compliant
|
||||||
screen-request = 45.
|
screen-request = 'OK'.
|
||||||
ENDIF.
|
ENDIF.
|
||||||
----
|
----
|
||||||
|
|
||||||
|
@ -1,46 +1,39 @@
|
|||||||
== Why is this an issue?
|
include::../description.adoc[]
|
||||||
|
|
||||||
A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loops, to test the value of a property, etc.
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
|
This is classically done by using a constant (`constexpr` or `const` if your compiler does not support `constexpr` yet) or an enumeration.
|
||||||
|
Note that since {cpp}20, some well known mathematical constants, such as pi, are defined in the header `<numbers>`, and should be preferred over defining your own version (see S6164).
|
||||||
|
|
||||||
Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.
|
=== Code examples
|
||||||
|
|
||||||
|
==== Noncompliant code example
|
||||||
|
|
||||||
That is why magic numbers must be demystified by first being assigned a name. This is classically done by using a constant (``++constexpr++`` or ``++const++`` if your compiler does not support ``++constexpr++`` yet) or an enumeration.
|
[source,cpp,diff-id=1,diff-type=noncompliant]
|
||||||
|
|
||||||
|
|
||||||
-1, 0 and 1 are not considered magic numbers.
|
|
||||||
|
|
||||||
|
|
||||||
Note that since {cpp}20, some well known mathematical constants, such as pi, are defined in the header ``++<numbers>++``, and should be preferred over defining your own version (see S6164).
|
|
||||||
|
|
||||||
=== Noncompliant code example
|
|
||||||
|
|
||||||
[source,cpp]
|
|
||||||
----
|
----
|
||||||
void doSomething(int var) {
|
void doSomething(int var) {
|
||||||
for(int i = 0; i < 42; i++) { // Noncompliant - 42 is a magic number
|
for (int i = 0; i < 42; i++) { // Noncompliant - 42 is a magic number
|
||||||
// ...
|
// ...
|
||||||
}
|
}
|
||||||
|
|
||||||
if (var == 42) { // Noncompliant - magic number
|
if (42 == var) { // Noncompliant - magic number
|
||||||
// ...
|
// ...
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
----
|
----
|
||||||
|
|
||||||
=== Compliant solution
|
==== Compliant solution
|
||||||
|
|
||||||
[source,cpp]
|
[source,cpp,diff-id=1,diff-type=compliant]
|
||||||
----
|
----
|
||||||
enum Status {
|
enum Status {
|
||||||
STATUS_KO = 0,
|
STATUS_OK = 0,
|
||||||
STATUS_OK = 42,
|
STATUS_ERROR = 42
|
||||||
};
|
};
|
||||||
|
|
||||||
void doSomething(Status var) {
|
void doSomething(Status var) {
|
||||||
constexpr int maxIterations = 42; // Compliant - in a declaration
|
constexpr int maxIterations = 42; // Compliant - in a declaration
|
||||||
for(int i = 0; i < maxIterations ; i++){ // Compliant: 0 is excluded, and maxIterations is a named constant
|
for (int i = 0; i < maxIterations; i++) { // Compliant - 0 is excluded, and maxIterations is a named constant
|
||||||
// ...
|
// ...
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1,10 +1,43 @@
|
|||||||
== Why is this an issue?
|
include::../description.adoc[]
|
||||||
|
|
||||||
A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loops, to test the value of a property, etc.
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.
|
=== Code examples
|
||||||
|
|
||||||
That is why magic numbers must be demystified by first being assigned to clearly named variables before being used.
|
==== Noncompliant code example
|
||||||
|
|
||||||
|
[source,cobol,diff-id=1,diff-type=noncompliant]
|
||||||
|
----
|
||||||
|
DATA DIVISION.
|
||||||
|
WORKING-STORAGE SECTION.
|
||||||
|
01 WS-COUNT PIC 9(1) VALUE 0.
|
||||||
|
PROCEDURE DIVISION.
|
||||||
|
A-PARA.
|
||||||
|
PERFORM B-PARA UNTIL WS-COUNT=5 *> Noncompliant - 5 is a magic number
|
||||||
|
STOP RUN.
|
||||||
|
|
||||||
|
B-PARA.
|
||||||
|
DISPLAY "Count:"WS-COUNT.
|
||||||
|
ADD 1 TO WS-COUNT.
|
||||||
|
----
|
||||||
|
|
||||||
|
==== Compliant solution
|
||||||
|
|
||||||
|
[source,cobol,diff-id=1,diff-type=compliant]
|
||||||
|
----
|
||||||
|
DATA DIVISION.
|
||||||
|
WORKING-STORAGE SECTION.
|
||||||
|
01 WS-COUNT PIC 9(1) VALUE 0.
|
||||||
|
01 WS-NUMBER-OF-CYCLES PIC 9(1) VALUE 5.
|
||||||
|
PROCEDURE DIVISION.
|
||||||
|
A-PARA.
|
||||||
|
PERFORM B-PARA UNTIL WS-COUNT=WS-NUMBER-OF-CYCLES *> Compliant
|
||||||
|
STOP RUN.
|
||||||
|
|
||||||
|
B-PARA.
|
||||||
|
DISPLAY "Count:"WS-COUNT.
|
||||||
|
ADD 1 TO WS-COUNT. *> Compliant - 1 is not considered a magic number
|
||||||
|
----
|
||||||
|
|
||||||
ifdef::env-github,rspecator-view[]
|
ifdef::env-github,rspecator-view[]
|
||||||
|
|
||||||
|
@ -1,35 +1,5 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
include::../description.adoc[]
|
include::../description.adoc[]
|
||||||
|
|
||||||
=== Noncompliant code example
|
|
||||||
|
|
||||||
[source,csharp]
|
|
||||||
----
|
|
||||||
public static void DoSomething()
|
|
||||||
{
|
|
||||||
for(int i = 0; i < 4; i++) // Noncompliant, 4 is a magic number
|
|
||||||
{
|
|
||||||
...
|
|
||||||
}
|
|
||||||
}
|
|
||||||
----
|
|
||||||
|
|
||||||
=== Compliant solution
|
|
||||||
|
|
||||||
[source,csharp]
|
|
||||||
----
|
|
||||||
private const int NUMBER_OF_CYCLES = 4;
|
|
||||||
|
|
||||||
public static void DoSomething()
|
|
||||||
{
|
|
||||||
for(int i = 0; i < NUMBER_OF_CYCLES ; i++) //Compliant
|
|
||||||
{
|
|
||||||
...
|
|
||||||
}
|
|
||||||
}
|
|
||||||
----
|
|
||||||
|
|
||||||
=== Exceptions
|
=== Exceptions
|
||||||
|
|
||||||
This rule doesn't raise an issue when the magic number is used as part of:
|
This rule doesn't raise an issue when the magic number is used as part of:
|
||||||
@ -39,6 +9,39 @@ This rule doesn't raise an issue when the magic number is used as part of:
|
|||||||
- the single argument of an attribute
|
- the single argument of an attribute
|
||||||
- a named argument for a method or attribute
|
- a named argument for a method or attribute
|
||||||
- a constructor call
|
- a constructor call
|
||||||
|
- a default value for a method argument
|
||||||
|
|
||||||
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
|
=== Code examples
|
||||||
|
|
||||||
|
==== Noncompliant code example
|
||||||
|
|
||||||
|
[source,csharp,diff-id=1,diff-type=noncompliant]
|
||||||
|
----
|
||||||
|
public void DoSomething()
|
||||||
|
{
|
||||||
|
for (int i = 0; i < 4; i++) // Noncompliant, 4 is a magic number
|
||||||
|
{
|
||||||
|
...
|
||||||
|
}
|
||||||
|
}
|
||||||
|
----
|
||||||
|
|
||||||
|
==== Compliant solution
|
||||||
|
|
||||||
|
[source,csharp,diff-id=1,diff-type=compliant]
|
||||||
|
----
|
||||||
|
private const int NUMBER_OF_CYCLES = 4;
|
||||||
|
|
||||||
|
public void DoSomething()
|
||||||
|
{
|
||||||
|
for (int i = 0; i < NUMBER_OF_CYCLES; i++) // Compliant
|
||||||
|
{
|
||||||
|
...
|
||||||
|
}
|
||||||
|
}
|
||||||
|
----
|
||||||
|
|
||||||
ifdef::env-github,rspecator-view[]
|
ifdef::env-github,rspecator-view[]
|
||||||
|
|
||||||
|
@ -1,10 +1,7 @@
|
|||||||
A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loop, to test the value of a property, etc.
|
A magic number is a hard-coded numerical value that may lack context or meaning. They should not be used because they can make the code less readable and maintainable.
|
||||||
|
|
||||||
|
== Why is this an issue?
|
||||||
|
|
||||||
Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.
|
Magic numbers make the code more complex to understand as it requires the reader to have knowledge about the global context to understand the number itself.
|
||||||
|
Their usage may seem obvious at the moment you're writing the code, but it may not be the case for another developer or later once the context faded away.
|
||||||
|
-1, 0 and 1 are not considered magic numbers.
|
||||||
That is why magic numbers must be demystified by first being assigned to clearly named variables before being used.
|
|
||||||
|
|
||||||
|
|
||||||
-1, 0 and 1 are not considered magic numbers.
|
|
||||||
|
4
rules/S109/how-to-fix-it.adoc
Normal file
4
rules/S109/how-to-fix-it.adoc
Normal file
@ -0,0 +1,4 @@
|
|||||||
|
== How to fix it
|
||||||
|
|
||||||
|
Replacing them with a constant allows us to provide a meaningful name associated with the value.
|
||||||
|
Instead of adding complexity to the code, it brings clarity and helps to understand the context and the global meaning.
|
@ -1,33 +1,35 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
include::../description.adoc[]
|
include::../description.adoc[]
|
||||||
|
|
||||||
=== Noncompliant code example
|
=== Exceptions
|
||||||
|
|
||||||
[source,java]
|
This rule ignores ``++hashCode++`` methods.
|
||||||
|
|
||||||
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
|
=== Code examples
|
||||||
|
|
||||||
|
==== Noncompliant code example
|
||||||
|
|
||||||
|
[source,java,diff-id=1,diff-type=noncompliant]
|
||||||
----
|
----
|
||||||
public static void doSomething() {
|
public static void doSomething() {
|
||||||
for(int i = 0; i < 4; i++){ // Noncompliant, 4 is a magic number
|
for (int i = 0; i < 4; i++) { // Noncompliant, 4 is a magic number
|
||||||
...
|
|
||||||
}
|
|
||||||
}
|
|
||||||
----
|
|
||||||
|
|
||||||
=== Compliant solution
|
|
||||||
|
|
||||||
[source,java]
|
|
||||||
----
|
|
||||||
public static final int NUMBER_OF_CYCLES = 4;
|
|
||||||
public static void doSomething() {
|
|
||||||
for(int i = 0; i < NUMBER_OF_CYCLES ; i++){
|
|
||||||
...
|
...
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
----
|
----
|
||||||
|
|
||||||
=== Exceptions
|
==== Compliant solution
|
||||||
|
|
||||||
This rule ignores ``++hashCode++`` methods.
|
[source,java,diff-id=1,diff-type=compliant]
|
||||||
|
----
|
||||||
|
public static final int NUMBER_OF_CYCLES = 4;
|
||||||
|
public static void doSomething() {
|
||||||
|
for (int i = 0; i < NUMBER_OF_CYCLES ; i++) { // Compliant
|
||||||
|
...
|
||||||
|
}
|
||||||
|
}
|
||||||
|
----
|
||||||
|
|
||||||
ifdef::env-github,rspecator-view[]
|
ifdef::env-github,rspecator-view[]
|
||||||
|
|
||||||
|
@ -1,25 +1,27 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
include::../description.adoc[]
|
include::../description.adoc[]
|
||||||
|
|
||||||
=== Noncompliant code example
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
[source,javascript]
|
=== Code examples
|
||||||
|
|
||||||
|
==== Noncompliant code example
|
||||||
|
|
||||||
|
[source,javascript,diff-id=1,diff-type=noncompliant]
|
||||||
----
|
----
|
||||||
function doSomething() {
|
function doSomething() {
|
||||||
for (let i = 0; i < 4; i++) { // Noncompliant, 4 is a magic number
|
for (let i = 0; i < 4; i++) { // Noncompliant, 4 is a magic number
|
||||||
// ...
|
// ...
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
----
|
----
|
||||||
|
|
||||||
=== Compliant solution
|
==== Compliant solution
|
||||||
|
|
||||||
[source,javascript]
|
[source,javascript,diff-id=1,diff-type=compliant]
|
||||||
----
|
----
|
||||||
function doSomething() {
|
function doSomething() {
|
||||||
const numberOfCycles = 4;
|
const numberOfCycles = 4;
|
||||||
for (let i = 0; i < numberOfCycles; i++) {
|
for (let i = 0; i < numberOfCycles; i++) { // Compliant
|
||||||
// ...
|
// ...
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1,14 +1,34 @@
|
|||||||
== Why is this an issue?
|
include::../description.adoc[]
|
||||||
|
|
||||||
A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loops, to test the value of a property, etc.
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
|
=== Code examples
|
||||||
|
|
||||||
Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.
|
==== Noncompliant code example
|
||||||
|
|
||||||
That is why magic numbers must be demystified by first being assigned to clearly named variables before being used.
|
[source,sql,diff-id=1,diff-type=noncompliant]
|
||||||
|
----
|
||||||
|
BEGIN
|
||||||
|
FOR l_counter IN 1..5 -- Noncompliant, 5 is a magic number
|
||||||
|
LOOP
|
||||||
|
...
|
||||||
|
END LOOP;
|
||||||
|
END;
|
||||||
|
----
|
||||||
|
|
||||||
|
==== Compliant solution
|
||||||
|
|
||||||
By default, -1, 0 and 1 are not considered magic numbers.
|
[source,sql,diff-id=1,diff-type=compliant]
|
||||||
|
----
|
||||||
|
DECLARE
|
||||||
|
co_number_of_cycles CONSTANT NUMBER := 5;
|
||||||
|
BEGIN
|
||||||
|
FOR l_counter IN 1..co_number_of_cycles -- Compliant
|
||||||
|
LOOP
|
||||||
|
...
|
||||||
|
END LOOP;
|
||||||
|
END;
|
||||||
|
----
|
||||||
|
|
||||||
ifdef::env-github,rspecator-view[]
|
ifdef::env-github,rspecator-view[]
|
||||||
|
|
||||||
|
@ -1,3 +0,0 @@
|
|||||||
{
|
|
||||||
"title": "Named constants should be used for indicators"
|
|
||||||
}
|
|
@ -1,46 +0,0 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
Using a named constant to refer to an indicator makes the content of the field clearer, and therefore makes the code easier to read and maintain.
|
|
||||||
|
|
||||||
=== Noncompliant code example
|
|
||||||
|
|
||||||
[source,rpg]
|
|
||||||
----
|
|
||||||
/Free
|
|
||||||
|
|
||||||
If *In(25); // Noncompliant
|
|
||||||
// Process contents...
|
|
||||||
EndIf;
|
|
||||||
----
|
|
||||||
|
|
||||||
=== Compliant solution
|
|
||||||
|
|
||||||
[source,rpg]
|
|
||||||
----
|
|
||||||
D accountTotal c 25
|
|
||||||
|
|
||||||
/Free
|
|
||||||
|
|
||||||
If *In(accountTotal);
|
|
||||||
// Process contents...
|
|
||||||
EndIf;
|
|
||||||
----
|
|
||||||
|
|
||||||
ifdef::env-github,rspecator-view[]
|
|
||||||
|
|
||||||
'''
|
|
||||||
== Implementation Specification
|
|
||||||
(visible only on this page)
|
|
||||||
|
|
||||||
=== Message
|
|
||||||
|
|
||||||
Use a named constant for this reference to XXX.
|
|
||||||
|
|
||||||
|
|
||||||
'''
|
|
||||||
== Comments And Links
|
|
||||||
(visible only on this page)
|
|
||||||
|
|
||||||
include::../comments-and-links.adoc[]
|
|
||||||
|
|
||||||
endif::env-github,rspecator-view[]
|
|
@ -1,3 +0,0 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
include::description.adoc[]
|
|
@ -1,33 +1,26 @@
|
|||||||
== Why is this an issue?
|
include::../description.adoc[]
|
||||||
|
|
||||||
A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loops, to test the value of a property, etc.
|
include::../how-to-fix-it.adoc[]
|
||||||
|
|
||||||
|
=== Code examples
|
||||||
|
|
||||||
Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.
|
==== Noncompliant code example
|
||||||
|
|
||||||
|
[source,vb6,diff-id=1,diff-type=noncompliant]
|
||||||
That is why magic numbers must be demystified by first being assigned to clearly named constants before being used.
|
|
||||||
|
|
||||||
|
|
||||||
-1, 0 and 1 are not considered magic numbers.
|
|
||||||
|
|
||||||
=== Noncompliant code example
|
|
||||||
|
|
||||||
[source,vb6]
|
|
||||||
----
|
----
|
||||||
Function blnCheckSize(dblParameter As Double) As Boolean
|
Function blnCheckSize(dblParameter As Double) As Boolean
|
||||||
If dblParameter > 1024 Then
|
If dblParameter > 1024 Then ' Noncompliant, 1024 is a magic number
|
||||||
blnCheckSize = True
|
blnCheckSize = True
|
||||||
End If
|
End If
|
||||||
End Function
|
End Function
|
||||||
----
|
----
|
||||||
|
|
||||||
=== Compliant solution
|
==== Compliant solution
|
||||||
|
|
||||||
[source,vb6]
|
[source,vb6,diff-id=1,diff-type=compliant]
|
||||||
----
|
----
|
||||||
Function blnCheckSize(dblParameter As Double) As Boolean
|
Function blnCheckSize(dblParameter As Double) As Boolean
|
||||||
Dim threshold As Integer = 1024
|
Dim threshold As Integer = 1024 ' Compliant
|
||||||
|
|
||||||
If dblParameter > threshold Then
|
If dblParameter > threshold Then
|
||||||
blnCheckSize = True
|
blnCheckSize = True
|
||||||
|
@ -1,3 +0,0 @@
|
|||||||
{
|
|
||||||
|
|
||||||
}
|
|
@ -1,65 +0,0 @@
|
|||||||
== Why is this an issue?
|
|
||||||
|
|
||||||
A magic number is a number that comes out of nowhere, and is directly used in a statement. Magic numbers are often used, for instance to limit the number of iterations of a loops, to test the value of a property, etc.
|
|
||||||
|
|
||||||
|
|
||||||
Using magic numbers may seem obvious and straightforward when you're writing a piece of code, but they are much less obvious and straightforward at debugging time.
|
|
||||||
|
|
||||||
|
|
||||||
That is why magic numbers must be demystified by first being assigned to clearly named constants before being used.
|
|
||||||
|
|
||||||
=== Noncompliant code example
|
|
||||||
|
|
||||||
[source,vbnet]
|
|
||||||
----
|
|
||||||
Class Foo
|
|
||||||
Sub DoSomething(Param As Integer)
|
|
||||||
If Param > 100 Then ' Magic Number
|
|
||||||
' Do something
|
|
||||||
End If
|
|
||||||
End Sub
|
|
||||||
End Class
|
|
||||||
----
|
|
||||||
|
|
||||||
=== Compliant solution
|
|
||||||
|
|
||||||
[source,vbnet]
|
|
||||||
----
|
|
||||||
Class Foo
|
|
||||||
Private Const MaxOfSomething As Integer = 100
|
|
||||||
|
|
||||||
Sub DoSomething(Param As Integer)
|
|
||||||
If Param > MaxOfSomething Then
|
|
||||||
' Do something
|
|
||||||
End If
|
|
||||||
End Sub
|
|
||||||
End Class
|
|
||||||
----
|
|
||||||
|
|
||||||
ifdef::env-github,rspecator-view[]
|
|
||||||
|
|
||||||
'''
|
|
||||||
== Implementation Specification
|
|
||||||
(visible only on this page)
|
|
||||||
|
|
||||||
include::../message.adoc[]
|
|
||||||
|
|
||||||
=== Parameters
|
|
||||||
|
|
||||||
.exceptions
|
|
||||||
****
|
|
||||||
_STRING_
|
|
||||||
|
|
||||||
----
|
|
||||||
-1,0,1,2
|
|
||||||
----
|
|
||||||
****
|
|
||||||
|
|
||||||
|
|
||||||
'''
|
|
||||||
== Comments And Links
|
|
||||||
(visible only on this page)
|
|
||||||
|
|
||||||
include::../comments-and-links.adoc[]
|
|
||||||
|
|
||||||
endif::env-github,rspecator-view[]
|
|
Loading…
x
Reference in New Issue
Block a user