Modify rule S2107: Expand and adjust for LaYC

I moved and closed the Java rule description because they do not plan to
implement it, but it is nice to preserve the description for posterity.

rules/S2107/cfamily/metadata.json

@@ -1,5 +1,7 @@
 {
   "title": "Member variables should be initialized",
+  "type": "BUG",
+  "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "10min"
@@ -9,9 +11,21 @@
     "bad-practice",
     "pitfall"
   ],
+  "extra": {
+    "replacementRules": [
+
+    ],
+    "legacyKeys": [
+
+    ]
+  },
   "defaultSeverity": "Critical",
+  "ruleSpecification": "RSPEC-2107",
+  "sqKey": "S2107",
+  "scope": "Main",
   "defaultQualityProfiles": [
     "Sonar way",
     "MISRA C++ 2008 recommended"
-  ]
+  ],
+  "quickfix": "infeasible"
 }

rules/S2107/cfamily/rule.adoc

@@ -1,22 +1,174 @@
+Partially-initialized objects are surprising to the `class` users
+and might lead to hard-to-catch bugs.
+``class``es with constructors are expected to have all members initialized after their constructor finishes.
+
+This rule raises an issue when some members are left uninitialized after a constructor finishes.
+
 == Why is this an issue?
 
-In a {cpp} class, all member variables of a type with an implicit or explicit constructor will be automatically initialized. This is not the case for the others: if no initialization is explicitly written, the variable will be left uninitialized.
+In the following example, `PartInit::x` is left uninitialized after the constructor finishes.
+
+[source,cpp]
+----
+struct AutomaticallyInitialized {
+  int x;
+  AutomaticallyInitialized() : x(0) {}
+};
+
+struct PartInit {
+  AutomaticallyInitialized ai;
+  int x;
+  int y;
+  PartInit(int n) :y(n) {
+    // this->ai is initialized
+    // this->y is initialized
+    // Noncompliant: this->x is left uninitialized
+  }
+};
+----
+
+This leads to undefined behavior in benign-looking code like on the example below.
+In this particular case, garbage value may be printed,
+or a compiler may optimize away the print statement completely.
+
+[source,cpp]
+----
+PartInit pi(1);
+std::cout <<pi.y; // Undefined behavior
+----
+
+For this reason, constructors should always initialize all data members of a class.
+
+While in some cases data members are initialized by their default constructor,
+in others, they are left with garbage.
+
+Types with a ``++default++``ed or implicit trivial default constructor follow the aggregate initialization syntax:
+if you omit them in the initialization list, they will not be initialized.
+
+[source,cpp]
+----
+struct Trivial {
+  int x;
+  int y;
+};
+struct Container {
+  Trivial t;
+  int arr[2];
+  Container() {
+    // Noncompliant
+    // this->t is not initialized
+    // this->t.x and this->t.y contain garbage
+    // this->arr contains garbage
+  }
+  Container(int) :t{}, arr{} {
+    // Compliant
+    // this->t.x and this->t.y are initialized to 0
+    // this->arr is initialized to {0, 0}
+  }
+  Container(int, int) :t{1}, arr{1} {
+    // Compliant
+    // this->t.x is 1
+    // this->t.y is 0
+    // this->arr is initialized to {1, 0}
+  }
+};
+struct DefaultedContainer {
+  Trivial t;
+  int arr[2];
+  DefaultedContainer() = default; // Noncompliant
+  // this->t and this->arr are not initialized
+};
+----
+
+The same is true for a ``++default++``ed default constructor.
+[source,cpp]
+----
+struct Defaulted {
+  int x;
+  Defaulted() = default;
+};
+struct ContainerDefaulted {
+  Defaulted d;
+  ContainerDefaulted() {
+    // Noncompliant this->d.x is not initialized
+  }
+};
+----
 
 
-This comes with all the risks associated with uninitialized variables, and these risks propagate to all the classes using  the faulty class as a type. This is all the more surprising that most programmers expect a constructor to correctly initialize the members of its class (this is its raison d'être after all).
+Even if some of the members have class initializers,
+the other members are still not initialized by default.
 
+[source,cpp]
+----
+struct Partial {
+  int x;
+  int y = 1;
+  int z;
+};
+struct ContainerPartial {
+  Partial p;
+  ContainrePartial() {
+    // Noncompliant
+    // this->p.x is not initialized
+    // this->p.y is initialized to 1
+    // this->p.z is not initialized
+  }
+  ContainrePartial(bool) :p{3} {
+    // Compliant
+    // this->p.x is initialized to 3
+    // this->p.y is initialized to 1
+    // this->p.z is initialized to 0
+  }
+};
+----
 
-To avoid such situations, all non class type fields should always be initialized (in order of preference):
+=== What is the potential impact?
+
+It is a common expectation that an object be in a fully-initialized state after its construction.
+A partially initialized object breaks this assumption.
+
+This comes with all the risks associated with uninitialized variables,
+and these risks propagate to all the classes using the faulty class as a type
+as a base class or a data member.
+This is all the more surprising
+that most programmers expect a constructor to correctly initialize the members of its class.
+
+include::../../../shared_content/cfamily/garbage_value_impact.adoc[]
+
+=== Exceptions
+
+Aggregate classes do not initialize most of their data members
+(unless you explicitly value initialize them with `x{}` or `x()`)
+but allow their users to use nice and flexible initialization syntax.
+They will be ignored by this rule (but are the subject of rule S5558).
+
+== How to fix it
+
+To avoid partially-initialized objects,
+all non `+class+`-type fields should always be initialized (in order of preference):
 
 * With an in-class initializer
 * In the initialization list of a constructor
 * In the constructor body
 
-See S3230 for more details about this order.
+See rule S3230 for more details about this order.
 
-=== Noncompliant code example
+If none of the above places is a good fit for a data member,
+ask yourself whether it belongs to this class.
+Instead of a data member, it might be more appropriate to keep the value
 
-[source,cpp]
+- as part of the class of a data member
+  (delegating initialization to the constructor of the member type),
+- in a derived class (delegating initialization to the derived class),
+- in a (potentially `static`) local variable of a member function, or
+- in a parameter of a member function.
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,cpp,diff-id=1,diff-type=noncompliant]
 ----
 class C {
   int val = 42;
@@ -48,9 +200,9 @@ public:
 };
 ----
 
-=== Compliant solution
+==== Compliant solution
 
-[source,cpp]
+[source,cpp,diff-id=1,diff-type=compliant]
 ----
 class C {
   int val = 42;
@@ -68,7 +220,7 @@ public:
 
 class T_fixed {
 public:
-  S _fixed s;
+  S_fixed s;
 
   T_fixed() : s() {}  // Compliant
   T_fixed(bool b) : s(b) {}
@@ -82,13 +234,55 @@ public:
 };
 ----
 
-=== Exceptions
+==== Noncompliant code example
 
-Aggregate classes do not initialize most of their data members, but allow their users to use nice and flexible initialization syntax. They will be ignored by this rule (but are the subject of S5558).
+[source,cpp,diff-id=2,diff-type=noncompliant]
+----
+struct Partial {
+  int x;
+  int y = 1;
+};
+struct ContainerPartial {
+  Partial p;
+  ContainrePartial() {
+    // Noncompliant: this->p.x is not initialized
+  }
+};
+----
+
+==== Compliant solution
+
+[source,cpp,diff-id=2,diff-type=compliant]
+----
+struct Partial {
+  int x = 0;
+  int y = 1;
+};
+struct ContainerPartial {
+  Partial p;
+  ContainrePartial() {
+    // Compliant
+  }
+};
+----
 
 == Resources
 
-* https://github.com/isocpp/CppCoreGuidelines/blob/036324/CppCoreGuidelines.md#c41-a-constructor-should-create-a-fully-initialized-object[{cpp} Core Guidelines C.41]: A constructor should create a fully initialized object
+=== External coding guidelines
+
+* {cpp} Core Guidelines - https://github.com/isocpp/CppCoreGuidelines/blob/036324/CppCoreGuidelines.md#c41-a-constructor-should-create-a-fully-initialized-object[C.41: A constructor should create a fully initialized object]
+
+=== Related rules
+
+* S836 detects the uses of uninitialized variables.
+* S3230 describes the preferred place for initializing class data members.
+
+=== Documentation
+
+* {cpp} reference - https://en.cppreference.com/w/cpp/language/aggregate_initialization[Aggregate initialization]
+* {cpp} reference - https://en.cppreference.com/w/cpp/language/direct_initialization[Direct initialization]
+* {cpp} reference - https://en.cppreference.com/w/cpp/language/value_initialization[Value initialization]
+
 
 ifdef::env-github,rspecator-view[]
 

rules/S2107/java/metadata.json

@@ -0,0 +1,24 @@
+{
+  "title": "Class fields should be initialized",
+  "type": "BUG",
+  "status": "closed",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "15min"
+  },
+  "tags": [
+    "pitfall"
+  ],
+  "extra": {
+    "replacementRules": [],
+    "legacyKeys": []
+  },
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-2107",
+  "sqKey": "S2107",
+  "scope": "Main",
+  "defaultQualityProfiles": [
+    "Sonar way"
+  ],
+  "quickfix": "unknown"
+}

rules/S2107/java/rule.adoc

@@ -0,0 +1,87 @@
+*This rule was deprecated for Java before it was implemented.*
+
+> in Java the members are initialized to default values (i.e. zero and null), so the rule doesn’t bring much value.
+
+https://discuss.sonarsource.com/t/is-the-rule-s2107-implemented-planned-duplicate-for-java/15217/2[discuss post]
+
+== Why is this an issue?
+
+Class members that are not assigned a default value and are not initialized in a constructor will be set to null by the compiler. Even if code exists to properly set those members, there is a risk that they will be dereferenced before it is called, resulting in a ``++NullPointerException++``. 
+
+
+Because you cannot guarantee that such classes will always be used properly, class members should always be initialized.
+
+
+This rule flags members which have no default value and which are left uninitialized by at least one class constructor, but which are unconditionally dereferenced somewhere in the code.
+
+
+=== Noncompliant code example
+
+[source,text]
+----
+public class Team {
+
+  int limit = 30;
+  List<Player> roster;  // Noncompliant; no default & not initialized by constructor
+  Person coach;
+
+  public Team (Person coach) { // roster is left uninitialized
+    this.coach = coach;
+  }
+
+  public void add(Player p) {
+    if (roster == null) {
+      roster = new ArrayList<Player>();
+    }
+    roster.add(p);
+  }
+
+  public boolean isFull() {  // NPE if called before add()
+    return roster.size() < limit;
+  }
+}
+----
+
+
+=== Compliant solution
+
+[source,text]
+----
+public class Team {
+
+  int limit = 30;
+  List<Player> roster = new ArrayList<Player>();
+  Person coach;
+
+  public Team (Person coach) {
+    this.coach = coach;
+  }
+
+  public void add(Player p) {
+    roster.add(p);
+  }
+
+  // ...
+----
+or 
+
+[source,text]
+----
+public class Team {
+
+  int limit = 30;
+  List<Player> roster;
+  Person coach;
+
+  public Team (Person coach) {
+    this.coach = coach;
+    this.roster = new ArrayList<Player>();
+  }
+
+  public void add(Player p) {
+    roster.add(p);
+  }
+
+  // ...
+----
+

rules/S2107/metadata.json

@@ -1,28 +1,2 @@
 {
-  "title": "Class fields should be initialized",
-  "type": "BUG",
-  "status": "ready",
-  "remediation": {
-    "func": "Constant\/Issue",
-    "constantCost": "15min"
-  },
-  "tags": [
-    "pitfall"
-  ],
-  "extra": {
-    "replacementRules": [
-
-    ],
-    "legacyKeys": [
-
-    ]
-  },
-  "defaultSeverity": "Major",
-  "ruleSpecification": "RSPEC-2107",
-  "sqKey": "S2107",
-  "scope": "Main",
-  "defaultQualityProfiles": [
-    "Sonar way"
-  ],
-  "quickfix": "unknown"
 }

rules/S836/cfamily/rule.adoc

@@ -62,24 +62,7 @@ void usingNew() {
 
 === What is the potential impact?
 
-Using garbage values will cause the program to behave
-nondeterministically at runtime.
-The program may produce a different output or crash depending
-on the run.
-
-In some situations, loading a variable may expose a sensitive
-data, such as a password that was previously stored in the same
-location, leading to a vulnerability that uses such a defect
-as a gadget for extracting information from the instance
-of the program.
-
-Finally, in {cpp}, outside of a few exceptions related to the uses
-of `unsigned char` or `std::byte`, loading data from an uninitialized
-variable causes undefined behavior. This means that the compiler
-is not bound by the language standard anymore, and the program
-has no meaning assigned to it. As a consequence, the impact
-of such a defect is not limited to the use of garbage values.
-
+include::../../../shared_content/cfamily/garbage_value_impact.adoc[]
 
 === Why is there an issue for a class with a default constructor?
 

shared_content/cfamily/garbage_value_impact.adoc

@@ -0,0 +1,17 @@
+
+Using garbage values will cause the program to behave
+nondeterministically at runtime.
+The program may produce a different output or crash depending on the run.
+
+In some situations, loading a variable may expose a sensitive data,
+such as a password that was previously stored in the same location,
+leading to a vulnerability that uses such a defect
+as a gadget for extracting information from the instance
+of the program.
+
+Finally, in {cpp}, outside of a few exceptions related to the uses of `unsigned char` or `std::byte`,
+loading data from an uninitialized variable causes undefined behavior.
+This means that the compiler is not bound by the language standard anymore,
+and the program has no meaning assigned to it.
+As a consequence,
+the impact of such a defect is not limited to the use of garbage values.