From d396f576cb7f079aa2cef1ded71fcb0f6c7590db Mon Sep 17 00:00:00 2001
From: Marco Borgeaud <marco.borgeaud@sonarsource.com>
Date: Thu, 18 Jan 2024 14:29:13 +0100
Subject: [PATCH] Add description

---
 rules/S6872/cfamily/metadata.json | 19 +++---
 rules/S6872/cfamily/rule.adoc     | 98 +++++++++++++++++++++++--------
 2 files changed, 83 insertions(+), 34 deletions(-)

diff --git a/rules/S6872/cfamily/metadata.json b/rules/S6872/cfamily/metadata.json
index 220b4e18698..45e51d7e58f 100644
--- a/rules/S6872/cfamily/metadata.json
+++ b/rules/S6872/cfamily/metadata.json
@@ -1,25 +1,26 @@
 {
-  "title": "FIXME",
+  "title": "Aggregates should be initialized with braces in non-generic code",
   "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
-    "constantCost": "5min"
+    "constantCost": "1min"
   },
   "tags": [
+    "since-c++20"
   ],
   "defaultSeverity": "Major",
   "ruleSpecification": "RSPEC-6872",
   "sqKey": "S6872",
   "scope": "All",
-  "defaultQualityProfiles": ["Sonar way"],
-  "quickfix": "unknown",
+  "defaultQualityProfiles": [
+    "Sonar way"
+  ],
+  "quickfix": "partial",
   "code": {
     "impacts": {
-      "MAINTAINABILITY": "HIGH",
-      "RELIABILITY": "MEDIUM",
-      "SECURITY": "LOW"
+      "MAINTAINABILITY": "MEDIUM"
     },
-    "attribute": "CONVENTIONAL"
+    "attribute": "CLEAR"
   }
-}
+}
\ No newline at end of file
diff --git a/rules/S6872/cfamily/rule.adoc b/rules/S6872/cfamily/rule.adoc
index 4bd440f87a8..175cfadb72d 100644
--- a/rules/S6872/cfamily/rule.adoc
+++ b/rules/S6872/cfamily/rule.adoc
@@ -1,44 +1,92 @@
-FIXME: add a description
-
-// If you want to factorize the description uncomment the following line and create the file.
-//include::../description.adoc[]
+Except for templated code, brace initialization should be preferred over {cpp}20's parenthesis initialization for aggregates.
 
 == Why is this an issue?
 
-FIXME: remove the unused optional headers (that are commented out)
+With {cpp}20, it is now possible to initialize aggregate types using parentheses.
+This language feature was introduced to simplify writing generic code and consistently initialize objects, whether they are aggregates or not.
+
+For the sake of simplicity, aggregate types include arrays, unions, and structures without user-provided constructors and with only public non-static data members and public bases.
+
+While parentheses are useful when initializing objects in templated code, they have several downsides compared to braces.
+
+ * Firstly, they allow narrowing conversion of arithmetic types that can result in unexpected program behaviors. See also S5276.
+
+ * Secondly, they can result in the most vexing parse.
+   For example, ``++Aggregate a(std::string());++`` declares function, while ``++Aggregate a{std::string()};++`` declares a variable.
+
+ * Furthermore, using braces is idiomatic and consistent with C.
+
+For all these reasons, braces should be preferred for non-generic code when initializing aggregates.
+And the fix is often trivial: replace the parentheses `()` with braces `{}`.
+
+Here is a noncompliant example:
 
-//=== What is the potential impact?
+[source,cpp,diff-id=1,diff-type=noncompliant]
+----
+struct Coordinate {
+  int x;
+  int y;
+};
 
-== How to fix it
-//== How to fix it in FRAMEWORK NAME
+long readInteger();
 
-=== Code examples
+auto readCoordinate() {
+  // Be aware of the narrowing conversions on the next line.
+  return Coordinate(readInteger(), readInteger()); // Noncompliant
+}
+----
 
-==== Noncompliant code example
+There are multiple ways of handling the narrowing conversion; here is one alternative:
 
-[source,text,diff-id=1,diff-type=noncompliant]
+[source,cpp,diff-id=1,diff-type=compliant]
 ----
-FIXME
+struct Coordinate {
+  int x;
+  int y;
+};
+
+long readInteger();
+
+auto readCoordinate() {
+  // Explicitly handle the conversion:
+  // Here, we saturate, but throwing an exception may also be appropriate.
+  auto readInt = []() { return saturate_cast<int>(readInteger()); }
+  return Coordinate{readInt(), readInt()}; // Compliant
+}
 ----
 
-==== Compliant solution
+=== Exceptions
 
-[source,text,diff-id=1,diff-type=compliant]
+Using `()` (i.e., with no arguments) to initialize an aggregate is accepted.
+The arguments listed above do not apply since the intent here is to use _value-initialization_.
+
+// There are also edge cases not worth covering or even mentioning here.
+//
+// Example: having a member with an explicit default constructor does not compile.
+// https://godbolt.org/z/exerMGM9x
+
+[source,cpp]
 ----
-FIXME
+Coordinate x(); // Irrelevant -- this is a function declaration.
+
+auto x = Coordinate(); // Value-initialization - compliant by exception.
+
+struct Object {
+  Coordinate coord;
+  Object() : coord() { } // Value-initialization - compliant by exception.
+};
 ----
 
-//=== How does this work?
+== Resources
 
-//=== Pitfalls
+=== Documentation
 
-//=== Going the extra mile
+ * {cpp} reference -- https://en.cppreference.com/w/cpp/language/aggregate_initialization#Definitions[Aggregate definition]
+ * {cpp} reference -- https://en.cppreference.com/w/cpp/language/value_initialization[Value-initialization]
+ * Wikipedia -- https://en.wikipedia.org/wiki/Most_vexing_parse[Most vexing parse]
 
+=== Related rules
 
-//== Resources
-//=== Documentation
-//=== Articles & blog posts
-//=== Conference presentations
-//=== Standards
-//=== External coding guidelines
-//=== Benchmarks
+ * S835 - Braces should be used to indicate and match the structure in the non-zero initialization of arrays and structures
+// TODO CPP-4792 - Update S835's title and list S6871.
+ * S5276 - Implicit casts should not lower precision