Modify rule S1854: Expand and adjust for LaYC

## Review

A dedicated reviewer checked the rule description successfully for:

- [ ] logical errors and incorrect information
- [ ] information gaps and missing content
- [ ] text style and tone
- [ ] PR summary and labels follow [the
guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule)

---------

Co-authored-by: Arseniy Zaostrovnykh <[email protected]>

rules/S1854/cfamily/rule.adoc

@@ -1,25 +1,193 @@
+Unused assignments should be removed.
+
 == Why is this an issue?
 
-include::../description.adoc[]
+Computing or retrieving a value only to then immediately overwrite it or throw it away indicates a serious logic error in the code.
+
+Assigning a value to a local variable that is not read by any subsequent instruction is called a _dead store_.
+The following code snippet depicts a few dead stores.
+
+[source,cpp]
+----
+int foo() {
+  int x = 0; // Noncompliant: dead store, next line overwrites x
+  x = 100; // Noncompliant: dead store, next line overwrites x
+  x = 200;
+  int y = 0;
+  y += 9001; // Noncompliant: dead store, y is never used
+  int z = 300; // Noncompliant: dead store, next line overwrites z
+  z = 400;
+  return x + z * 2;
+}
+----
+
+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.
+In most cases, these operations have their intended use but it is not expressed correctly in the code.
+Therefore, unused values and superfluous code should be removed to prevent logic errors.
+
+
+== What is the potential impact?
+
+Not only do unused values and superfluous code make the program unnecessary complex, but also indicate significant logic errors.
+And even in the absence of logic errors, they waste computing resources in case the compiler is not able to optimize them away.
+
+Unused values typically showcase a discrepancy between what a developer intended and what is specified in the code and should be removed to uncover and eventually prevent logic errors.
+
+
+== How to fix it
+
+Remove unused values and superfluous code.
+
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,cpp,diff-id=1,diff-type=noncompliant]
+----
+int foo(int y) {
+  int x = 0;
+  x = 100; // Noncompliant: dead store
+  x = 200;
+  return x + y;
+}
+----
+
+==== Compliant solution
+
+[source,cpp,diff-id=1,diff-type=compliant]
+----
+int foo(int y) {
+  int x = 200; // Compliant: no unnecessary assignment
+  return x + y;
+}
+----
+
+==== Noncompliant code example
+
+[source,cpp,diff-id=2,diff-type=noncompliant]
+----
+int bar();
+int buz();
+
+int foo(bool b) {
+  int x = 0;
+  if (b) {
+    x = bar();
+    return x;
+  }
+  if (x != 0) {
+    int y = buz();
+    y += 9001; // Noncompliant: dead store
+  }
+  return x;
+}
+----
 
-include::../noncompliant.adoc[]
+==== Compliant solution
 
-include::../compliant.adoc[]
+[source,cpp,diff-id=2,diff-type=compliant]
+----
+int bar();
+int buz();
 
-=== Exceptions
+int foo(bool b) {
+  int x = 0;
+  if (b) {
+    x = bar();
+    return x;
+  }
+  // Compliant: no more dead stores and superfluous code
+  // Assuming call to buz() had no important side effects
+  return x;
+}
+----
 
-This rule ignores:
 
-* variable declarations initializers
-* prefix and postfix increments and decrements ``x{plus}{plus};``
-* null pointer assignments ``++x = NULL;++``
-* self assignments (i.e. ``++x = x;++``)
+=== Pitfalls
+
+When removing unused values and superfluous code, make sure that the right-hand side of a given assignment has no side effects.
+
+While it is safe to remove the call to `square` in the following code since it has no side effects, removing the call to `fwrite` changes the program's behavior.
+Still, values that are never read such as `n` indicate code smells that should be mitigated.
+In this code example, the return value of `fwrite` should be checked and any potential error should be handled appropriately.
+
+[source,cpp]
+----
+#include <stdio.h>
+#include <string.h>
+
+int square(int n) {
+  return n * n;
+}
+
+int foo(int i) {
+  int sq = square(i); // Noncompliant: dead store, assignment can be removed
+  const char* const str = "Hello, World!\n";
+  // Although `n` is never read, the call to `fwrite` cannot be removed due to side effects
+  size_t n = fwrite(str, sizeof(char), strlen(str), stdout); // Noncompliant: `n` is never read
+  return i + 9001;
+}
+----
+
+
+=== Going the extra mile
+
+In {cpp}17, the `nodiscard` attribute has been introduced which can be used to annotate functions, enumerations and classes.
+
+The attribute serves as a hint to the compiler and to other developers that a function's return value should not be ignored.
+A function that is marked `nodiscard` whose return value is ignored encourages the compiler to issue a warning.
+Example usages of the `nodiscard` attribute are shown in the following:
+
+[source,cpp]
+----
+[[nodiscard]] int foo() { return 100; }
+int bar() { return 200; }
+[[nodiscard("An explanation on why not to discard the return value")]] int buz() { return 300; }
+
+enum class [[nodiscard]] important_error_info { OK, WARN, CRITICAL };
+important_error_info compute() {
+  // More code ...
+  // In case of a critical error, return corresponding error info:
+  return important_error_info::CRITICAL;
+}
+
+void caller() {
+  foo(); // compiler warns on discarding a nodiscard value
+  bar(); // compiler will issue no warning
+  buz(); // compiler warns on discarding a nodiscard value
+  compute(); // compiler warns on discarding a nodiscard value
+}
+----
+
+In case, the return value of a function marked as `nodiscard` should be (exceptionally) ignored, a cast to `void` can be used to silence the compiler warning as shown in the following:
+
+[source,cpp]
+----
+[[nodiscard]] int foo() { return 100; }
+
+void caller() {
+  foo(); // compiler warns on discarding a nodiscard value
+  (void)foo(); // compiler will issue no warning
+}
+----
 
 == Resources
 
-* 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
+=== Standards
+
+* CERT - https://wiki.sei.cmu.edu/confluence/x/39UxBQ[MSC13-C. Detect and remove unused values]
+* CERT - https://wiki.sei.cmu.edu/confluence/x/9DZGBQ[MSC56-J. Detect and remove superfluous code and values]
+* CWE - https://cwe.mitre.org/data/definitions/563[563 - Assignment to Variable without Use ('Unused Variable')]
+
+=== 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
+
 
 ifdef::env-github,rspecator-view[]
 

rules/S1854/metadata.json

@@ -36,5 +36,5 @@
   "defaultQualityProfiles": [
     "Sonar way"
   ],
-  "quickfix": "unknown"
+  "quickfix": "targeted"
 }