SONARJAVA-4662 Update Rules Metadata and External Linters Metadata (#4527)

Author: irina-batinic-sonarsource

external-reports/src/main/resources/org/sonar/l10n/java/rules/spotbugs/spotbugs-rules.json

@@ -2276,7 +2276,8 @@
   },
   {
     "key": "PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_INNER_CLASS_NAMES",
-    "name": "Do not reuse public identifiers from JSL as inner name",
+    "name": "Bad practice - Do not reuse public identifiers from JSL as inner name",
+    "type": "CODE_SMELL",
     "severity": "MAJOR",
     "url": "https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html#pi-do-not-reuse-public-identifiers-inner-class-names"
   },

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S113.html

@@ -1,5 +1,5 @@
 <h2>Why is this an issue?</h2>
-<p>Some tools work better when files end with an empty line.</p>
+<p>Some tools work better when files end with a newline.</p>
 <p>This rule simply generates an issue if it is missing.</p>
 <p>For example, a Git diff looks like this if the empty line is missing at the end of the file:</p>
 <pre>

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S113.json

@@ -1,5 +1,5 @@
 {
-  "title": "Files should contain an empty newline at the end",
+  "title": "Files should end with a newline",
   "type": "CODE_SMELL",
   "code": {
     "impacts": {

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S1144.html

@@ -1,35 +1,34 @@
+<p>This rule raises an issue when a private method is never referenced in the code.</p>
 <h2>Why is this an issue?</h2>
-<p><code>private</code> methods that are never executed are dead code: unnecessary, inoperative code that should be removed. Cleaning out dead code
-decreases the size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced.</p>
-<p>Note that this rule does not take reflection into account, which means that issues will be raised on <code>private</code> methods that are only
-accessed using the reflection API.</p>
+<p>A method that is never called is dead code, and should be removed. Cleaning out dead code decreases the size of the maintained codebase, making it
+easier to understand the program and preventing bugs from being introduced.</p>
+<p>This rule detects methods that are never referenced from inside a translation unit, and cannot be referenced from the outside.</p>
+<h3>Code examples</h3>
 <h3>Noncompliant code example</h3>
-<pre>
+<pre data-diff-id="1" data-diff-type="noncompliant">
 public class Foo implements Serializable
 {
-  private Foo(){}     //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
-  public static void doSomething(){
+  public static void doSomething() {
     Foo foo = new Foo();
     ...
   }
-  private void unusedPrivateMethod(){...}
-  private void writeObject(ObjectOutputStream s){...}  //Compliant, relates to the java serialization mechanism
-  private void readObject(ObjectInputStream in){...}  //Compliant, relates to the java serialization mechanism
+
+  private void unusedPrivateMethod() {...}
+  private void writeObject(ObjectOutputStream s) {...}  //Compliant, relates to the java serialization mechanism
+  private void readObject(ObjectInputStream in) {...}  //Compliant, relates to the java serialization mechanism
 }
 </pre>
 <h3>Compliant solution</h3>
-<pre>
+<pre data-diff-id="1" data-diff-type="compliant">
 public class Foo implements Serializable
 {
-  private Foo(){}     //Compliant, private empty constructor intentionally used to prevent any direct instantiation of a class.
   public static void doSomething(){
     Foo foo = new Foo();
     ...
   }
 
-  private void writeObject(ObjectOutputStream s){...}  //Compliant, relates to the java serialization mechanism
-
-  private void readObject(ObjectInputStream in){...}  //Compliant, relates to the java serialization mechanism
+  private void writeObject(ObjectOutputStream s) {...}  //Compliant, relates to the java serialization mechanism
+  private void readObject(ObjectInputStream in) {...}  //Compliant, relates to the java serialization mechanism
 }
 </pre>
 <h3>Exceptions</h3>
@@ -38,4 +37,6 @@ <h3>Exceptions</h3>
   <li> annotated methods </li>
   <li> methods with parameters that are annotated with <code>@javax.enterprise.event.Observes</code> </li>
 </ul>
+<p>The rule does not take reflection into account, which means that issues will be raised on <code>private</code> methods that are only accessed using
+the reflection API.</p>
 

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S1144.json

@@ -10,7 +10,7 @@
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
-    "constantCost": "5min"
+    "constantCost": "2min"
   },
   "tags": [
     "unused"

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S1860.html

@@ -1,6 +1,34 @@
 <h2>Why is this an issue?</h2>
-<p>Instances of value-based classes, which are pooled and potentially reused, should not be used for synchronization. If they are, it can cause
-unrelated threads to deadlock with unhelpful stacktraces.</p>
+<p>In Java, value-based classes are those for which instances are final and immutable, like <code>String</code>, <code>Integer</code> and so on, and
+their identity relies on their value and not their reference. When a variable of one of these types is instantiated, the JVM caches its value, and the
+variable is just a reference to that value. For example, multiple <code>String</code> variables with the same value "Hello world!" will refer to the
+same cached string literal in memory.</p>
+<p>The <code>synchronized</code> keyword tells the JVM to only allow the execution of the code contained in the following block to one
+<code>Thread</code> at a time. This mechanism relies on the identity of the object that is being synchronized between threads, to prevent that if
+object X is locked, it will still be possible to lock another object Y.</p>
+<p>It means that the JVM will fail to correctly synchronize threads on instances of the aforementioned value-based classes, for instance:</p>
+<pre>
+// These variables "a" and "b" will effectively reference the same object in memory
+Integer a = 0;
+Integer b = 0;
+
+// This means that in the following code, the JVM could try to lock and execute
+// on the variable "a" because "b" was notified to be released, as the two Integer variables
+// are the same object to the JVM
+void syncMethod(int x) {
+    synchronized (a) {
+        if (a == x) {
+        // ... do something here
+        }
+    }
+    synchronized (b) {
+        if (b == x) {
+        // ... do something else
+        }
+    }
+}
+</pre>
+<p>This behavior can cause unrelated threads to deadlock with unclear stacktraces.</p>
 <p>Within the JDK, types which should not be used for synchronization include:</p>
 <ul>
   <li> <code>String</code> literals </li>
@@ -21,8 +49,8 @@ <h2>Why is this an issue?</h2>
 </ul>
 <h2>How to fix it</h2>
 <p>Replace instances of value-based classes with a new object instance to synchronize on.</p>
-<h2>Code examples</h2>
-<h3>Noncompliant code example</h3>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
 <pre data-diff-id="1" data-diff-type="noncompliant">
 private static final Boolean bLock = Boolean.FALSE;
 private static final Integer iLock = Integer.valueOf(0);
@@ -44,33 +72,33 @@ <h3>Noncompliant code example</h3>
       ...
   }
 </pre>
-<h3>Compliant solution</h3>
+<h4>Compliant solution</h4>
 <pre data-diff-id="1" data-diff-type="compliant">
 private static final Object lock1 = new Object();
-private static final Object lock2 = new Object();
-private static final Object lock3 = new Object();
-private static final Object lock4 = new Object();
+private static final Integer iLock = new Integer(42);
+private static final String sLock = new String("A brand new string in memory!");
+private static final List&lt;String&gt; listLock = new ArrayList&lt;&gt;();
 
 public void doSomething() {
 
   synchronized(lock1) { // Compliant
       ...
   }
-  synchronized(lock2) { // Compliant
+  synchronized(iLock) { // Compliant
       ...
   }
-  synchronized(lock3) { // Compliant
+  synchronized(sLock) { // Compliant
       ...
   }
-  synchronized(lock4) { // Compliant
+  synchronized(listLock) { // Compliant
       ...
   }
 </pre>
 <h2>Resources</h2>
 <ul>
-  <li> <a href="https://wiki.sei.cmu.edu/confluence/x/1zdGBQ">Do not synchronize on objects that may be reused - CERT, LCK01-J.</a> </li>
-  <li> <a href="https://openjdk.java.net/jeps/390">JEP 390: Warnings for Value-Based Classes - OpenJDK</a> </li>
-  <li> <a href="https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html">Value-based Classes - Java SE 17 API
-  Documentation</a> </li>
+  <li> CERT - <a href="https://wiki.sei.cmu.edu/confluence/x/1zdGBQ">Do not synchronize on objects that may be reused</a> </li>
+  <li> OpenJDK - <a href="https://openjdk.java.net/jeps/390">JEP 390: Warnings for Value-Based Classes</a> </li>
+  <li> Java Documentation - <a href="https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html">Value-based
+  Classes</a> </li>
 </ul>
 

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S1871.html

@@ -1,7 +1,25 @@
 <h2>Why is this an issue?</h2>
+<p>When the same code is duplicated in two or more separate branches of a conditional, it can make the code harder to understand, maintain, and can
+potentially introduce bugs if one instance of the code is changed but others are not.</p>
 <p>Having two <code>cases</code> in a <code>switch</code> statement or two branches in an <code>if</code> chain with the same implementation is at
 best duplicate code, and at worst a coding error.</p>
 <pre data-diff-id="1" data-diff-type="noncompliant">
+if (a &gt;= 0 &amp;&amp; a &lt; 10) {
+  doFirstThing();
+  doTheThing();
+}
+else if (a &gt;= 10 &amp;&amp; a &lt; 20) {
+  doTheOtherThing();
+}
+else if (a &gt;= 20 &amp;&amp; a &lt; 50) {
+  doFirstThing();
+  doTheThing();  // Noncompliant; duplicates first condition
+}
+else {
+  doTheRest();
+}
+</pre>
+<pre data-diff-id="2" data-diff-type="noncompliant">
 switch (i) {
   case 1:
     doFirstThing();
@@ -18,27 +36,11 @@ <h2>Why is this an issue?</h2>
     doTheRest();
 }
 </pre>
-<pre data-diff-id="2" data-diff-type="noncompliant">
-if (a &gt;= 0 &amp;&amp; a &lt; 10) {
-  doFirstThing();
-  doTheThing();
-}
-else if (a &gt;= 10 &amp;&amp; a &lt; 20) {
-  doTheOtherThing();
-}
-else if (a &gt;= 20 &amp;&amp; a &lt; 50) {
-  doFirstThing();
-  doTheThing();  // Noncompliant; duplicates first condition
-}
-else {
-  doTheRest();
-}
-</pre>
 <p>If the same logic is truly needed for both instances, then:</p>
 <ul>
   <li> in an <code>if</code> chain they should be combined </li>
 </ul>
-<pre data-diff-id="2" data-diff-type="compliant">
+<pre data-diff-id="1" data-diff-type="compliant">
 if ((a &gt;= 0 &amp;&amp; a &lt; 10) || (a &gt;= 20 &amp;&amp; a &lt; 50)) { // Compliant
   doFirstThing();
   doTheThing();
@@ -51,9 +53,9 @@ <h2>Why is this an issue?</h2>
 }
 </pre>
 <ul>
-  <li> for a <code>switch</code>, one should fall through to the other. </li>
+  <li> for a <code>switch</code>, one should fall through to the other </li>
 </ul>
-<pre data-diff-id="1" data-diff-type="compliant">
+<pre data-diff-id="2" data-diff-type="compliant">
 switch (i) {
   case 1:
   case 3: // Compliant
@@ -74,7 +76,7 @@ <h3>Exceptions</h3>
 <code>switch</code> statement that contains a single line of code with or without a following <code>break</code>.</p>
 <pre>
 if (a == 1) {
-  doSomething();  //no issue, usually this is done on purpose to increase the readability
+  doSomething();  // Compliant, usually this is done on purpose to increase the readability
 } else if (a == 2) {
   doSomethingElse();
 } else {

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S1905.html

@@ -1,6 +1,6 @@
 <h2>Why is this an issue?</h2>
 <p>Casting expressions are utilized to convert one data type to another, such as transforming an integer into a string. This is especially crucial in
-strongly typed languages like <code>C</code>, <code>C++</code>, <code>C#</code>, <code>Java</code>, <code>Python</code>, and others.</p>
+strongly typed languages like C, C++, C#, Java, Python, and others.</p>
 <p>However, there are instances where casting expressions are not needed. These include situations like:</p>
 <ul>
   <li> casting a variable to its own type </li>