SONARJAVA-2202 Update rule description and bug/code smell (#1368)

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

@@ -1,5 +1,7 @@
 <p>If a <code>private</code> field is declared but not used in the program, it can be considered dead code and should therefore be removed. This will
 improve maintainability because developers will not wonder what the variable is used for.</p>
+<p>Note that this rule does not take reflection into account, which means that issues will be raised on <code>private</code> fields that are only
+accessed using the reflection API.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
 public class MyClass {
@@ -30,7 +32,7 @@ <h2>Exceptions</h2>
   private static final long serialVersionUID = 42L;
 }
 </pre>
-<p>Moreover, this rule doesn't raise any issue on annotated fields.</p>
+<p>Moreover, this rule doesn't raise any issue on annotated fields. </p>
 <h2>See</h2>
 <ul>
   <li> <a href="https://www.securecoding.cert.org/confluence/x/SIIyAQ">CERT, MSC12-CPP.</a> - Detect and remove code that has no effect </li>

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

@@ -1,19 +1,18 @@
 {
   "title": "Empty statements should be removed",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "2min"
   },
   "tags": [
-    "bug",
     "misra",
     "cert",
     "unused"
   ],
   "standards": [
     "CERT"
   ],
-  "defaultSeverity": "Minor"
+  "defaultSeverity": "Critical"
 }

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

@@ -7,7 +7,7 @@
     "constantCost": "1min"
   },
   "tags": [
-    "convention"
+    "style"
   ],
   "defaultSeverity": "Minor"
 }

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

@@ -13,11 +13,19 @@ <h2>Compliant Solution</h2>
 }
 </pre>
 <h2>Exceptions</h2>
-<p>Generic exceptions in the signatures of overriding methods are ignored.</p>
+<p>Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration
+in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the
+analysis).</p>
 <pre>
 @Override
 public void myMethod() throws Exception {...}
 </pre>
+<p>Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.</p>
+<pre>
+public void myOtherMethod throws Exception {
+  doTheThing();  // this method throws Exception
+}
+</pre>
 <h2>See</h2>
 <ul>
   <li> <a href="http://cwe.mitre.org/data/definitions/397.html">MITRE, CWE-397</a> - Declaration of Throws for Generic Exception </li>

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

@@ -1,6 +1,6 @@
 {
   "title": "Generic exceptions should never be thrown",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
@@ -9,7 +9,6 @@
   "tags": [
     "cwe",
     "error-handling",
-    "bug",
     "cert"
   ],
   "standards": [

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

@@ -1,5 +1,7 @@
 <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>
 <h2>Noncompliant Code Example</h2>
 <pre>
 public class Foo implements Serializable
@@ -30,7 +32,7 @@ <h2>Compliant Solution</h2>
 }
 </pre>
 <h2>Exceptions</h2>
-<p>This rule doesn't raise any issue on annotated methods.</p>
+<p>This rule doesn't raise any issue on annotated methods. </p>
 <h2>See</h2>
 <ul>
   <li> <a href="https://www.securecoding.cert.org/confluence/x/OYIyAQ">CERT, MSC07-CPP.</a> - Detect and remove dead code </li>

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

@@ -1,14 +1,13 @@
 {
   "title": "Exit methods should not be called",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "30min"
   },
   "tags": [
     "cwe",
-    "bug",
     "cert",
     "suspicious"
   ],

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

@@ -1,14 +1,13 @@
 {
   "title": "Synchronized classes Vector, Hashtable, Stack and StringBuffer should not be used",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "20min"
   },
   "tags": [
-    "performance",
-    "bug"
+    "performance"
   ],
   "defaultSeverity": "Major"
 }

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

@@ -7,7 +7,6 @@
     "constantCost": "2min"
   },
   "tags": [
-    "bug",
     "clumsy"
   ],
   "defaultSeverity": "Minor"

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

@@ -3,8 +3,6 @@
   <li> It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production. </li>
   <li> It is not yet, or never will be, supported. In this case an <code>UnsupportedOperationException</code> should be thrown. </li>
   <li> The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override. </li>
-  <li> There is a desire to provide a public, no-args constructor. In this case, it can simply be omitted from the code; a default constructor will
-  automatically be generated. </li>
 </ul>
 <h2>Noncompliant Code Example</h2>
 <pre>
@@ -27,10 +25,10 @@ <h2>Compliant Solution</h2>
 }
 </pre>
 <h2>Exceptions</h2>
-<p>An abstract class may have empty methods, in order to provide default implementations for child classes.</p>
+<p>Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.</p>
 <pre>
 public abstract class Animal {
-  void speak() {
+  void speak() {  // default implementation ignored
   }
 }
 </pre>

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

@@ -1,10 +1,7 @@
-<p>While it is technically correct to assign to parameters from within method bodies, it is typically done in error, with the intent to assign a
-parameter value to a field of the same name, (and <code>this</code> was forgotten). </p>
-<p>If it is done on purpose, a better course would be to use temporary variables to store intermediate results. Allowing parameters to be assigned to
-also reduces code readability because developers won't be able to tell whether the original parameter or some temporary variable is being accessed
-without going through the whole method. Moreover, some developers might also expect assignments of method parameters to be visible to callers, which
-is not the case, and this lack of visibility could confuse them. Instead, all parameters, caught exceptions, and foreach parameters should be treated
-as <code>final</code>.</p>
+<p>While it is technically correct to assign to parameters from within method bodies, it reduces code readability because developers won't be able to
+tell whether the original parameter or some temporary variable is being accessed without going through the whole method. Moreover, some developers
+might also expect assignments of method parameters to be visible to callers, which is not the case, and this lack of visibility could confuse them.
+Instead, all parameters, caught exceptions, and foreach parameters should be treated as <code>final</code>.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
 class MyClass {

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

@@ -1,13 +1,12 @@
 {
   "title": "Method parameters, caught exceptions and foreach variables should not be reassigned",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "5min"
   },
   "tags": [
-    "bug",
     "misra",
     "pitfall"
   ],

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

@@ -1,5 +1,5 @@
-<p>When only the condition expression is defined in a <code>for</code> loop, but the init and increment expressions are missing, a <code>while</code>
-loop should be used instead to increase readability. </p>
+<p>When only the condition expression is defined in a <code>for</code> loop, and the initialization and increment expressions are missing, a
+<code>while</code> loop should be used instead to increase readability. </p>
 <h2>Noncompliant Code Example</h2>
 <pre>
 for (;condition;) { /*...*/ }

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

@@ -10,5 +10,5 @@
   "tags": [
     "brain-overload"
   ],
-  "defaultSeverity": "Major"
+  "defaultSeverity": "Minor"
 }

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

@@ -1,3 +1,6 @@
 <p>The cyclomatic complexity of methods should not exceed a defined threshold.</p>
 <p>Complex code can perform poorly and will in any case be difficult to understand and therefore to maintain.</p>
+<h2>Exceptions</h2>
+<p>While having a large number of fields in a class may indicate that it should be split, this rule nonetheless ignores high complexity in
+<code>equals</code> and <code>hashCode</code> methods.</p>
 

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

@@ -1,14 +1,13 @@
 {
   "title": "Maps with keys that are enum values should be replaced with EnumMap",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "5min"
   },
   "tags": [
-    "performance",
-    "bug"
+    "performance"
   ],
   "defaultSeverity": "Minor"
 }

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

@@ -1,14 +1,13 @@
 {
   "title": "Sets with elements that are enum values should be replaced with EnumSet",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "5min"
   },
   "tags": [
-    "performance",
-    "bug"
+    "performance"
   ],
   "defaultSeverity": "Minor"
 }

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

@@ -1,14 +1,13 @@
 {
   "title": "Strings should not be concatenated using '+' in a loop",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "10min"
   },
   "tags": [
-    "performance",
-    "bug"
+    "performance"
   ],
   "defaultSeverity": "Minor"
 }

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

@@ -7,7 +7,6 @@
     "constantCost": "3min"
   },
   "tags": [
-    "bug",
     "cert"
   ],
   "defaultSeverity": "Major"

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

@@ -41,8 +41,28 @@
       <li> <code>PrintWriter(OutputStream out, boolean autoFlush)</code> </li>
       <li> <code>PrintWriter(String fileName)</code> </li>
     </ul> </li>
-  <li> <code>IOUtils</code> methods which accept an encoding argument when that argument is null, and overloads of those methods that omit the
-  encoding argument </li>
+  <li> methods from Apache commons-io library which accept an encoding argument when that argument is null, and overloads of those methods that omit
+  the encoding argument
+    <ul>
+      <li> <code>IOUtils.copy(InputStream, Writer)</code> </li>
+      <li> <code>IOUtils.copy(Reader, OutputStream)</code> </li>
+      <li> <code>IOUtils.readLines(InputStream)</code> </li>
+      <li> <code>IOUtils.toByteArray(Reader)</code> </li>
+      <li> <code>IOUtils.toByteArray(String)</code> </li>
+      <li> <code>IOUtils.toCharArray(InputStream)</code> </li>
+      <li> <code>IOUtils.toInputStream(TypeCriteria.subtypeOf(CharSequence))</code> </li>
+      <li> <code>IOUtils.toString(byte[])</code> </li>
+      <li> <code>IOUtils.toString(URI)</code> </li>
+      <li> <code>IOUtils.toString(URL)</code> </li>
+      <li> <code>IOUtils.write(char[], OutputStream)</code> </li>
+      <li> <code>IOUtils.write(CharSequence, OutputStream)</code> </li>
+      <li> <code>IOUtils.writeLines(Collection, String, OutputStream)</code> </li>
+      <li> <code>FileUtils.readFileToString(File)</code> </li>
+      <li> <code>FileUtils.readLines(File)</code> </li>
+      <li> <code>FileUtils.write(File, CharSequence)</code> </li>
+      <li> <code>FileUtils.write(File, CharSequence, boolean)</code> </li>
+      <li> <code>FileUtils.writeStringToFile(File, String)</code> </li>
+    </ul> </li>
 </ul>
 <h2>See</h2>
 <ul>

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

@@ -1,15 +1,15 @@
 {
   "title": "Classes and methods that rely on the default system encoding should not be used",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",
     "constantCost": "15min"
   },
   "tags": [
-    "bug",
     "unpredictable",
-    "cert"
+    "cert",
+    "i18n"
   ],
   "standards": [
     "CERT"

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

@@ -1,9 +1,18 @@
 <p>The MD5 algorithm and its successor, SHA-1, are no longer considered secure, because it is too easy to create hash collisions with them. That is,
 it takes too little computational effort to come up with a different input that produces the same MD5 or SHA-1 hash, and using the new, same-hash
 value gives an attacker the same access as if he had the originally-hashed value. This applies as well to the other Message-Digest algorithms: MD2,
-MD4, MD6.</p>
-<p>This rule tracks usage of the <code>java.security.MessageDigest</code>, and <code>org.apache.commons.codec.digest.DigestUtils</code> classes to
-instantiate MD or SHA-1 algorithms, and of Guava's <code>com.google.common.hash.Hashing sha1</code> and <code>md5</code> methods. </p>
+MD4, MD6, RIPEMD160.</p>
+<p>The following APIs are tracked for use of obsolete crypto algorithms:</p>
+<p> * <code>java.security.AlgorithmParameters</code> (JDK)</p>
+<p> * <code>java.security.AlgorithmParameterGenerator</code> (JDK)</p>
+<p> * <code>java.security.MessageDigest</code> (JDK)</p>
+<p> * <code>java.security.KeyFactory</code> (JDK)</p>
+<p> * <code>java.security.KeyPairGenerator</code> (JDK)</p>
+<p> * <code>java.security.Signature</code> (JDK)</p>
+<p> * <code>javax.crypto.Mac</code> (JDK)</p>
+<p> * <code>javax.crypto.KeyGenerator</code> (JDK)</p>
+<p> * <code>org.apache.commons.codec.digest.DigestUtils</code> (Apache Commons Codec)</p>
+<p> * <code>com.google.common.hash.Hashing</code> (Guava)</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
 MessageDigest md = MessageDigest.getInstance("SHA1");  // Noncompliant
@@ -12,10 +21,6 @@ <h2>Compliant Solution</h2>
 <pre>
 MessageDigest md = MessageDigest.getInstance("SHA-256");
 </pre>
-<p>or</p>
-<pre>
-Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING");
-</pre>
 <h2>See</h2>
 <ul>
   <li> <a href="http://cwe.mitre.org/data/definitions/328">MITRE, CWE-328</a> - Reversible One-Way Hash </li>