From 37030a89dd327a3aab9f34c55ae1c55f96395a6f Mon Sep 17 00:00:00 2001
From: Zsolt Kolbay
 <121798625+zsolt-kolbay-sonarsource@users.noreply.github.com>
Date: Fri, 13 Oct 2023 12:26:37 +0200
Subject: [PATCH] Modify S1854: Migrate to LayC (#3274)

---
 rules/S1854/cfamily/rule.adoc    | 12 +++---
 rules/S1854/compliant.adoc       |  7 ----
 rules/S1854/csharp/rule.adoc     | 50 +++++++++++++++++++-----
 rules/S1854/description.adoc     |  1 -
 rules/S1854/exceptions.adoc      |  3 --
 rules/S1854/howtofixit.adoc      |  3 ++
 rules/S1854/java/rule.adoc       | 43 ++++++++++++++++-----
 rules/S1854/javascript/rule.adoc | 65 +++++++++++++++++++++-----------
 rules/S1854/metadata.json        |  2 +-
 rules/S1854/noncompliant.adoc    |  7 ----
 rules/S1854/php/rule.adoc        | 32 ++++++++++------
 rules/S1854/plsql/rule.adoc      | 27 ++++++++-----
 rules/S1854/python/rule.adoc     | 35 +++++++++--------
 rules/S1854/rule.adoc            | 11 ------
 rules/S1854/see.adoc             |  4 +-
 rules/S1854/swift/rule.adoc      | 41 ++++++++++++++++----
 rules/S1854/why.adoc             |  4 ++
 17 files changed, 223 insertions(+), 124 deletions(-)
 delete mode 100644 rules/S1854/compliant.adoc
 delete mode 100644 rules/S1854/description.adoc
 delete mode 100644 rules/S1854/exceptions.adoc
 create mode 100644 rules/S1854/howtofixit.adoc
 delete mode 100644 rules/S1854/noncompliant.adoc
 delete mode 100644 rules/S1854/rule.adoc
 create mode 100644 rules/S1854/why.adoc

diff --git a/rules/S1854/cfamily/rule.adoc b/rules/S1854/cfamily/rule.adoc
index 515bd5bf0b..1615601802 100644
--- a/rules/S1854/cfamily/rule.adoc
+++ b/rules/S1854/cfamily/rule.adoc
@@ -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[]
 
diff --git a/rules/S1854/compliant.adoc b/rules/S1854/compliant.adoc
deleted file mode 100644
index 5781b9e4c5..0000000000
--- a/rules/S1854/compliant.adoc
+++ /dev/null
@@ -1,7 +0,0 @@
-=== Compliant solution
-
-[source,text]
-----
-i = a + b;
-i += compute();
-----
diff --git a/rules/S1854/csharp/rule.adoc b/rules/S1854/csharp/rule.adoc
index e392b370b7..0d663b8b27 100644
--- a/rules/S1854/csharp/rule.adoc
+++ b/rules/S1854/csharp/rule.adoc
@@ -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[]
 
 '''
diff --git a/rules/S1854/description.adoc b/rules/S1854/description.adoc
deleted file mode 100644
index 5a8e268e47..0000000000
--- a/rules/S1854/description.adoc
+++ /dev/null
@@ -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.
diff --git a/rules/S1854/exceptions.adoc b/rules/S1854/exceptions.adoc
deleted file mode 100644
index 654ed311f6..0000000000
--- a/rules/S1854/exceptions.adoc
+++ /dev/null
@@ -1,3 +0,0 @@
-=== Exceptions
-
-This rule ignores initializations to -1, 0, 1, ``++null++``, ``++true++``, ``++false++`` and ``++""++``.
diff --git a/rules/S1854/howtofixit.adoc b/rules/S1854/howtofixit.adoc
new file mode 100644
index 0000000000..e379fddb21
--- /dev/null
+++ b/rules/S1854/howtofixit.adoc
@@ -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).
diff --git a/rules/S1854/java/rule.adoc b/rules/S1854/java/rule.adoc
index 453899c354..f371d1fa8c 100644
--- a/rules/S1854/java/rule.adoc
+++ b/rules/S1854/java/rule.adoc
@@ -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[]
 
diff --git a/rules/S1854/javascript/rule.adoc b/rules/S1854/javascript/rule.adoc
index 27b5b7fe78..5629012043 100644
--- a/rules/S1854/javascript/rule.adoc
+++ b/rules/S1854/javascript/rule.adoc
@@ -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[]
diff --git a/rules/S1854/metadata.json b/rules/S1854/metadata.json
index 88dce851cc..d6ee2c1b86 100644
--- a/rules/S1854/metadata.json
+++ b/rules/S1854/metadata.json
@@ -10,7 +10,7 @@
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
-    "constantCost": "15min"
+    "constantCost": "1min"
   },
   "tags": [
     "cwe",
diff --git a/rules/S1854/noncompliant.adoc b/rules/S1854/noncompliant.adoc
deleted file mode 100644
index 725dae33cc..0000000000
--- a/rules/S1854/noncompliant.adoc
+++ /dev/null
@@ -1,7 +0,0 @@
-=== Noncompliant code example
-
-[source,text]
-----
-i = a + b; // Noncompliant; calculation result not used before value is overwritten
-i = compute();
-----
diff --git a/rules/S1854/php/rule.adoc b/rules/S1854/php/rule.adoc
index c0d996a426..dd82e1062f 100644
--- a/rules/S1854/php/rule.adoc
+++ b/rules/S1854/php/rule.adoc
@@ -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[]
 
 '''
diff --git a/rules/S1854/plsql/rule.adoc b/rules/S1854/plsql/rule.adoc
index ef3d88621d..c921bbb4ba 100644
--- a/rules/S1854/plsql/rule.adoc
+++ b/rules/S1854/plsql/rule.adoc
@@ -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);
@@ -26,14 +32,17 @@ declare
 begin
   my_user := user();
   my_date := sysdate();
-  dbms_output.put_line('User:' || my_user || ', date: ' || my_date); 
+  dbms_output.put_line('User:' || my_user || ', date: ' || my_date);
 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[]
 
 '''
diff --git a/rules/S1854/python/rule.adoc b/rules/S1854/python/rule.adoc
index 36ac1072a8..1ad8ea9256 100644
--- a/rules/S1854/python/rule.adoc
+++ b/rules/S1854/python/rule.adoc
@@ -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[]
diff --git a/rules/S1854/rule.adoc b/rules/S1854/rule.adoc
deleted file mode 100644
index 228040a1e2..0000000000
--- a/rules/S1854/rule.adoc
+++ /dev/null
@@ -1,11 +0,0 @@
-== Why is this an issue?
-
-include::description.adoc[]
-
-include::noncompliant.adoc[]
-
-include::compliant.adoc[]
-
-include::exceptions.adoc[]
-
-include::see.adoc[]
diff --git a/rules/S1854/see.adoc b/rules/S1854/see.adoc
index 676c7cbd6b..a57fe8a745 100644
--- a/rules/S1854/see.adoc
+++ b/rules/S1854/see.adoc
@@ -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')]
\ No newline at end of file
diff --git a/rules/S1854/swift/rule.adoc b/rules/S1854/swift/rule.adoc
index 0ed76f4d96..2ed00d9338 100644
--- a/rules/S1854/swift/rule.adoc
+++ b/rules/S1854/swift/rule.adoc
@@ -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[]
 
 '''
diff --git a/rules/S1854/why.adoc b/rules/S1854/why.adoc
new file mode 100644
index 0000000000..56db57209b
--- /dev/null
+++ b/rules/S1854/why.adoc
@@ -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.
\ No newline at end of file