Modify S1854: Migrate to LayC (#3274)

This commit is contained in:
Zsolt Kolbay 2023-10-13 12:26:37 +02:00 committed by GitHub
parent 828e2d54cc
commit 37030a89dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 223 additions and 124 deletions

View File

@ -38,7 +38,6 @@ Unused values typically showcase a discrepancy between what a developer intended
Remove unused values and superfluous code.
=== Code examples
==== Noncompliant code example
@ -182,12 +181,11 @@ void caller() {
=== Related rules
* S1763 detects unreachable code
* S2583 ensures that conditionally executed code is reachable
* S2589 detects gratuitous boolean expressions
* S3516 ensures that a function returns non-invariant
* S3626 identifies redundant jump statements
* S1763 - All code should be reachable
* S2583 - Conditionally executed code should be reachable
* S2589 - Boolean expressions should not be gratuitous
* S3516 - Methods returns should not be invariant
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]

View File

@ -1,7 +0,0 @@
=== Compliant solution
[source,text]
----
i = a + b;
i += compute();
----

View File

@ -1,22 +1,52 @@
== Why is this an issue?
include::../description.adoc[]
include::../noncompliant.adoc[]
include::../compliant.adoc[]
include::../why.adoc[]
=== Exceptions
No issue is reported when
* the analyzed method body contains ``++try++`` blocks,
* a lambda expression captures the local variables, or
* the analyzed method body contains `try` blocks
* a lambda expression captures the local variable
* the variable is unused (case covered by Rule S1481)
* initializations to ``++-1++``, ``++0++``, ``++1++``, ``++null++``, ``++true++``, ``++false++``, ``++""++`` and ``++string.Empty++``.
* it's an initialization to `-1`, `0`, `1`, `null`, `true`, `false`, `""` or `string.Empty`
include::../howtofixit.adoc[]
You can also use https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/functional/discards[discards] (rather than a variable) to express that result of a method call is ignored on purpose.
=== Code examples
==== Noncompliant code example
[source,csharp,diff-id=1,diff-type=noncompliant]
----
int Foo(int y)
{
int x = 100; // Noncompliant: dead store
x = 150; // Noncompliant: dead store
x = 200;
return x + y;
}
----
==== Compliant solution
[source,csharp,diff-id=1,diff-type=compliant]
----
int Foo(int y)
{
int x = 200; // Compliant: no unnecessary assignment
return x + y;
}
----
include::../see.adoc[]
=== Related rules
* S2583 - Conditionally executed code should be reachable
* S2589 - Boolean expressions should not be gratuitous
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]
'''

View File

@ -1 +0,0 @@
A dead store happens when a local variable is assigned a value that is not read by any subsequent instruction. Calculating or retrieving a value only to then overwrite it or throw it away, could indicate a serious error in the code. Even if it's not an error, it is at best a waste of resources. Therefore all calculated values should be used.

View File

@ -1,3 +0,0 @@
=== Exceptions
This rule ignores initializations to -1, 0, 1, ``++null++``, ``++true++``, ``++false++`` and ``++""++``.

View File

@ -0,0 +1,3 @@
== How to fix it
Remove the unnecesarry assignment, then test the code to make sure that the right-hand side of a given assignment had no side effects (e.g. a method that writes certain data to a file and returns the number of written bytes).

View File

@ -1,18 +1,43 @@
== Why is this an issue?
include::../why.adoc[]
include::../description.adoc[]
=== Exceptions
include::../noncompliant.adoc[]
This rule ignores initializations to `-1`, `0`, `1`, `null`, `true`, `false` and `""`.
include::../compliant.adoc[]
include::../howtofixit.adoc[]
include::../exceptions.adoc[]
=== Code examples
== Resources
==== Noncompliant code example
* https://cwe.mitre.org/data/definitions/563[MITRE, CWE-563] - Assignment to Variable without Use ('Unused Variable')
* https://wiki.sei.cmu.edu/confluence/x/39UxBQ[CERT, MSC13-C.] - Detect and remove unused values
* https://wiki.sei.cmu.edu/confluence/x/9DZGBQ[CERT, MSC56-J.] - Detect and remove superfluous code and values
[source,java,diff-id=1,diff-type=noncompliant]
----
int foo(int y) {
int x = 100; // Noncompliant: dead store
x = 150; // Noncompliant: dead store
x = 200;
return x + y;
}
----
==== Compliant solution
[source,java,diff-id=1,diff-type=compliant]
----
int foo(int y) {
int x = 200; // Compliant: no unnecessary assignment
return x + y;
}
----
include::../see.adoc[]
=== Related rules
* S2583 - Conditionally executed code should be reachable
* S2589 - Boolean expressions should not be gratuitous
* S3516 - Methods returns should not be invariant
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]

View File

@ -1,30 +1,59 @@
== Why is this an issue?
include::../description.adoc[]
include::../noncompliant.adoc[]
include::../compliant.adoc[]
include::../why.adoc[]
=== Exceptions
* This rule ignores initializations to -1, 0, 1, ``++undefined++``, [], {}, ``++true++``, ``++false++`` and ``++""++``.
* Variables that start with an underscore (e.g. \'``++_unused++``') are ignored.
* Assignment of ``++null++`` is ignored because it is sometimes used to help garbage collection
The rule ignores
* Initializations to `-1`, `0`, `1`, `undefined`, `[]`, `{}`, `true`, `false` and `""`.
* Variables that start with an underscore (e.g. \'`_unused`') are ignored.
* Assignment of `null` is ignored because it is sometimes used to help garbage collection
* Increment and decrement expressions are ignored because they are often used idiomatically instead of `x+1`
* This rule also ignores variables declared with object destructuring using rest syntax (used to exclude some properties from object):
* This rule also ignores variables declared with object destructuring using rest syntax (used to exclude some properties from object)
[source,javascript]
----
let {a, b, ...rest} = obj; // 'a' and 'b' are ok
let {a, b, ...rest} = obj; // 'a' and 'b' are compliant
doSomething(rest);
let [x1, x2, x3] = arr; // but 'x1' is noncompliant, as omitting syntax can be used: "let [, x2, x3] = arr;"
let [x1, x2, x3] = arr; // 'x1' is noncompliant, as omitting syntax can be used: "let [, x2, x3] = arr;"
doSomething(x2, x3);
----
include::../howtofixit.adoc[]
=== Code examples
==== Noncompliant code example
[source,javascript,diff-id=1,diff-type=noncompliant]
----
function foo(y) {
let x = 100; // Noncompliant: dead store
x = 150; // Noncompliant: dead store
x = 200;
return x + y;
}
----
==== Compliant solution
[source,javascript,diff-id=1,diff-type=compliant]
----
function foo(y) {
let x = 200; // Compliant: no unnecessary assignment
return x + y;
}
----
include::../see.adoc[]
=== Related rules
* S1763 - All code should be reachable
* S2589 - Boolean expressions should not be gratuitous
* S3516 - Function returns should not be invariant
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]
'''
@ -33,16 +62,6 @@ ifdef::env-github,rspecator-view[]
include::../message.adoc[]
'''
== Comments And Links
(visible only on this page)
=== on 29 Apr 2015, 07:49:23 Ann Campbell wrote:
assigned for review of expansion
=== on 29 Apr 2015, 08:55:42 Linda Martin wrote:
Reviewed!
include::../comments-and-links.adoc[]
endif::env-github,rspecator-view[]

View File

@ -10,7 +10,7 @@
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "15min"
"constantCost": "1min"
},
"tags": [
"cwe",

View File

@ -1,7 +0,0 @@
=== Noncompliant code example
[source,text]
----
i = a + b; // Noncompliant; calculation result not used before value is overwritten
i = compute();
----

View File

@ -1,29 +1,39 @@
== Why is this an issue?
include::../why.adoc[]
include::../description.adoc[]
=== Exceptions
=== Noncompliant code example
This rule ignores initializations to `-1`, `0`, `1`, `null`, `true`, `false`, `""`, `[]` and `array()`.
[source,php]
include::../howtofixit.adoc[]
=== Code examples
==== Noncompliant code example
[source,php,diff-id=1,diff-type=noncompliant]
----
$i = $a + $b; // Noncompliant; calculation result not used before value is overwritten
$i = $a + $b; // Noncompliant - calculation result is not used before value is overwritten
$i = compute();
----
=== Compliant solution
==== Compliant solution
[source,php]
[source,php,diff-id=1,diff-type=compliant]
----
$i = $a + $b;
$i += compute();
----
=== Exceptions
This rule ignores initializations to -1, 0, 1, ``++null++``, ``++true++``, ``++false++``, ``++""++``, ``++[]++`` and ``++array()++``.
include::../see.adoc[]
=== Related rules
* S1763 - All code should be reachable
* S3626 - Jump statements should not be redundant
* S1763 detects unreachable code
* S3626 identifies redundant jump statements
ifdef::env-github,rspecator-view[]
'''

View File

@ -1,10 +1,16 @@
== Why is this an issue?
include::../why.adoc[]
include::../description.adoc[]
=== Exceptions
=== Noncompliant code example
This rule ignores initializations to `-1`, `0`, `1`, `NULL`, `TRUE`, `FALSE` and `""`.
[source,sql]
include::../howtofixit.adoc[]
=== Code examples
==== Noncompliant code example
[source,sql,diff-id=1,diff-type=noncompliant]
----
declare
my_user VARCHAR2(30);
@ -16,9 +22,9 @@ begin
end;
----
=== Compliant solution
==== Compliant solution
[source,sql]
[source,sql,diff-id=1,diff-type=compliant]
----
declare
my_user VARCHAR2(30);
@ -30,10 +36,13 @@ begin
end;
----
include::../exceptions.adoc[]
include::../see.adoc[]
=== Related rules
* S1763 - All code should be reachable
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]
'''

View File

@ -1,10 +1,17 @@
== Why is this an issue?
include::../why.adoc[]
include::../description.adoc[]
=== Exceptions
=== Noncompliant code example
This rule ignores initializations to `-1`, `0`, `1`, `None`, `True`, `False` and `""`.
No issue will be raised on unpacked variables.
[source,python]
include::../howtofixit.adoc[]
=== Code examples
==== Noncompliant code example
[source,python,diff-id=1,diff-type=noncompliant]
----
def func(a, b, compute):
i = a + b # Noncompliant; calculation result not used before value is overwritten
@ -12,9 +19,9 @@ def func(a, b, compute):
return i
----
=== Compliant solution
==== Compliant solution
[source,python]
[source,python,diff-id=1,diff-type=compliant]
----
def func(a, b, compute):
i = a + b
@ -22,15 +29,14 @@ def func(a, b, compute):
return i
----
=== Exceptions
This rule ignores initializations to -1, 0, 1, ``++None++``, ``++True++``, ``++False++`` and ``++""++``.
No issue will be raised on unpacked variables.
include::../see.adoc[]
=== Related rules
* S1763 - All code should be reachable
* S3516 - Functions returns should not be invariant
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]
'''
@ -43,9 +49,6 @@ include::../message.adoc[]
== Comments And Links
(visible only on this page)
=== on 13 Nov 2019, 06:56:28 Alexandre Gigleux wrote:
\[~pierre-yves.nicolas] Rule S1854 is not raised for the "Noncompliant Code". Instead S1481 "Unused local variables should be removed" is raised.
include::../comments-and-links.adoc[]
endif::env-github,rspecator-view[]

View File

@ -1,11 +0,0 @@
== Why is this an issue?
include::description.adoc[]
include::noncompliant.adoc[]
include::compliant.adoc[]
include::exceptions.adoc[]
include::see.adoc[]

View File

@ -1,3 +1,5 @@
== Resources
* https://cwe.mitre.org/data/definitions/563[MITRE, CWE-563] - Assignment to Variable without Use ('Unused Variable')
=== Standards
* CWE - https://cwe.mitre.org/data/definitions/563[563 - Assignment to Variable without Use ('Unused Variable')]

View File

@ -1,17 +1,42 @@
== Why is this an issue?
include::../description.adoc[]
include::../noncompliant.adoc[]
include::../compliant.adoc[]
include::../why.adoc[]
=== Exceptions
This rule ignores initializations to 0, 1, ``++nil++``, ``++true++``, ``++false++`` and ``++""++``.
This rule ignores initializations to `0`, `1`, `nil`, `true`, `false` and `""`.
include::../howtofixit.adoc[]
=== Code examples
==== Noncompliant code example
[source,swift,diff-id=1,diff-type=noncompliant]
----
func foo(y: Int) -> Int {
var x = 100 // Noncompliant: dead store
x = 150 // Noncompliant: dead store
x = 200
return x + y
}
----
==== Compliant solution
[source,swift,diff-id=1,diff-type=compliant]
----
func foo(y: Int) -> Int {
var x = 200 // Compliant: no unnecessary assignment
return x + y
}
----
include::../see.adoc[]
=== Related rules
* S1763 - All code should be reachable
* S3626 - Jump statements should not be redundant
ifdef::env-github,rspecator-view[]
'''

4
rules/S1854/why.adoc Normal file
View File

@ -0,0 +1,4 @@
== Why is this an issue?
Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten. Such assignments are unnecessary and don't contribute to the functionality or clarity of the code. They may even negatively impact performance. Removing them enhances code cleanliness and readability.
Even if the unnecessary operations do not do any harm in terms of the program's correctness, they are - at best - a waste of computing resources.