Fix/ignore major + minor code smells reported by SonarCloud

Author: SvenWoltmann

google_checks.xml

@@ -321,19 +321,11 @@
                value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/>
     </module>
     <module name="JavadocMethod">
-      <property name="accessModifiers" value="public"/>
       <property name="allowMissingParamTags" value="true"/>
       <property name="allowMissingReturnTag" value="true"/>
       <property name="allowedAnnotations" value="Override, Test"/>
       <property name="tokens" value="METHOD_DEF, CTOR_DEF, ANNOTATION_FIELD_DEF, COMPACT_CTOR_DEF"/>
     </module>
-    <module name="MissingJavadocMethod">
-      <property name="scope" value="public"/>
-      <property name="minLineCount" value="2"/>
-      <property name="allowedAnnotations" value="Override, Test"/>
-      <property name="tokens" value="METHOD_DEF, CTOR_DEF, ANNOTATION_FIELD_DEF,
-                                   COMPACT_CTOR_DEF"/>
-    </module>
     <module name="MissingJavadocType">
       <property name="scope" value="protected"/>
       <property name="tokens"

pom.xml

@@ -21,6 +21,7 @@
         <google-java-format.version>1.15.0</google-java-format.version>
         <jacoco.version>0.8.8</jacoco.version>
         <spotbugs.version>4.7.1.0</spotbugs.version>
+        <spotbugs-annotations.version>4.7.1</spotbugs-annotations.version>
         <findsecbugs.version>1.12.0</findsecbugs.version>
         <pmd-plugin.version>3.17.0</pmd-plugin.version>
         <pmd.version>6.47.0</pmd.version>
@@ -37,16 +38,24 @@
         <sonar.java.checkstyle.reportPaths>./target/checkstyle-result.xml</sonar.java.checkstyle.reportPaths>
         <sonar.coverage.jacoco.xmlReportPaths>${project.reporting.outputDirectory}/jacoco-ut/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
 
-        <!-- Exclude "Cognitive Complexity of methods should not be too high" from "method" package -->
-        <sonar.issue.ignore.multicriteria>e1</sonar.issue.ignore.multicriteria>
+        <sonar.issue.ignore.multicriteria>e1,e2</sonar.issue.ignore.multicriteria>
+        <!-- Ignore "Cognitive Complexity of methods should not be too high" warnings in "method" package -->
         <sonar.issue.ignore.multicriteria.e1.ruleKey>java:S3776</sonar.issue.ignore.multicriteria.e1.ruleKey>
         <sonar.issue.ignore.multicriteria.e1.resourceKey>**/eu/happycoders/sort/method/**/*.*</sonar.issue.ignore.multicriteria.e1.resourceKey>
+        <!-- Ignore "Weak Cryptography" warnings - we're just sorting random numbers -->
+        <sonar.issue.ignore.multicriteria.e2.ruleKey>java:S2245</sonar.issue.ignore.multicriteria.e2.ruleKey>
+        <sonar.issue.ignore.multicriteria.e2.resourceKey>**/*.*</sonar.issue.ignore.multicriteria.e2.resourceKey>
 
         <!-- Exclude "method" package from duplicate check -->
         <sonar.cpd.exclusions>**/eu/happycoders/sort/method/**/*.*</sonar.cpd.exclusions>
     </properties>
 
     <dependencies>
+        <dependency>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-annotations</artifactId>
+            <version>${spotbugs-annotations.version}</version>
+        </dependency>
         <dependency>
             <scope>test</scope>
             <groupId>org.junit.jupiter</groupId>

ruleset.xml

@@ -10,6 +10,7 @@
   <rule ref="category/java/bestpractices.xml">
     <exclude name="AvoidReassigningParameters"/>  <!-- In Java, this is OK, IMO -->
     <exclude name="SwitchStmtsShouldHaveDefault"/><!-- PMD doesn't recognize Java's new switch statements -->
+    <exclude name="UseVarargs"/>                  <!-- PMD would suggest varargs wherever we pass an array to be sorted -->
   </rule>
 
   <rule ref="category/java/codestyle.xml">
@@ -21,6 +22,8 @@
     <exclude name="OnlyOneReturn"/>               <!-- This rule might have made sense decades ago when methods where hundreds of lines long -->
     <exclude name="ShortClassName"/>              <!-- Wouldn't allow class names like "User" -->
     <exclude name="ShortMethodName"/>             <!-- Wouldn't allow method names like "of" -->
+    <exclude name="ShortVariable"/>               <!-- This is usually a valid check; however, in the search algorithms, we often use variables "i" and "j"Wouldn't allow method names like "of" -->
+    <exclude name="CallSuperInConstructor"/>      <!-- IMO, this would be noise -->
   </rule>
 
   <rule ref="category/java/codestyle.xml/ClassNamingConventions">
@@ -37,6 +40,9 @@
 
   <rule ref="category/java/design.xml">
     <exclude name="LawOfDemeter"/>                <!-- We don't want to apply this law -->
+    <exclude name="CognitiveComplexity"/>         <!-- This is usually a valid check; however, search algorithms *are* complex (we could make the code easier by extracting several methods, but I prefer to have it as close to the actual algorithm description as possible) -->
+    <exclude name="CyclomaticComplexity"/>        <!-- This is usually a valid check; however, search algorithms *are* complex (we could make the code easier by extracting several methods, but I prefer to have it as close to the actual algorithm description as possible) -->
+    <exclude name="NPathComplexity"/>             <!-- This is usually a valid check; however, search algorithms *are* complex (we could make the code easier by extracting several methods, but I prefer to have it as close to the actual algorithm description as possible) -->
   </rule>
 
   <rule ref="category/java/documentation.xml">
@@ -50,11 +56,13 @@
 
   <rule ref="category/java/errorprone.xml/AvoidLiteralsInIfCondition">
     <properties>
-      <property name="ignoreMagicNumbers" value="-1,0,1" /><!-- Default is only -1 and 0; add 1 -->
+      <property name="ignoreMagicNumbers" value="-1,0,1,2" /><!-- Default is only -1 and 0; add 1, 2 -->
     </properties>
   </rule>
 
-  <rule ref="category/java/multithreading.xml"/>
+  <rule ref="category/java/multithreading.xml">
+    <exclude name="DoNotUseThreads"/>             <!-- This warning is for Java EE applications -->
+  </rule>
 
   <rule ref="category/java/performance.xml"/>
 

src/main/java/eu/happycoders/sort/CountOperations.java

@@ -4,7 +4,7 @@
 import eu.happycoders.sort.method.SortAlgorithm;
 import eu.happycoders.sort.utils.ArrayUtils;
 import java.util.Locale;
-import java.util.function.Function;
+import java.util.function.IntFunction;
 
 /**
  * Measures the performance of all sorting algorithms for various input sizes.
@@ -43,7 +43,7 @@ private void countOps(
       SortAlgorithm algorithm,
       boolean sorted,
       String inputOrder,
-      Function<Integer, int[]> arraySupplier) {
+      IntFunction<int[]> arraySupplier) {
     System.out.printf("%n--- %s (order: %s) ---%n", algorithm.getName(), inputOrder);
 
     // Sort until sorting takes more than MAX_SORTING_TIME_SECS
@@ -74,7 +74,7 @@ private void countOps(
 
   private Counters countOps(SortAlgorithm algorithm, int[] elements) {
     Counters counters = new Counters();
-    algorithm.sort(elements, counters);
+    algorithm.sortWithCounters(elements, counters);
     return counters;
   }
 }

src/main/java/eu/happycoders/sort/UltimateTest.java

@@ -21,7 +21,7 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
-import java.util.function.Function;
+import java.util.function.IntFunction;
 
 /**
  * Measures the performance of all sorting algorithms for various input sizes.
@@ -31,29 +31,28 @@
 @SuppressWarnings({"PMD.SystemPrintln", "java:S106"})
 public class UltimateTest {
 
-  static final SortAlgorithm[] ALGORITHMS =
-      new SortAlgorithm[] {
-        new InsertionSort(),
-        new SelectionSort(),
-        new BubbleSortOpt1(),
-
-        // Quicksort
-        new QuicksortVariant1(PivotStrategy.RIGHT),
-        new QuicksortVariant1(PivotStrategy.MIDDLE),
-        new QuicksortVariant1(PivotStrategy.MEDIAN3),
-        new QuicksortImproved(48, new QuicksortVariant1(PivotStrategy.MIDDLE)),
-        new DualPivotQuicksort(DualPivotQuicksort.PivotStrategy.THIRDS),
-        new DualPivotQuicksortImproved(64, DualPivotQuicksort.PivotStrategy.THIRDS),
-        new MergeSort(),
-
-        // Heapsort
-        new Heapsort(),
-        new BottomUpHeapsort(),
-        new HeapsortSlowComparisons(),
-        new BottomUpHeapsortSlowComparisons(),
-        new CountingSort(),
-        new JavaArraysSort()
-      };
+  static final SortAlgorithm[] ALGORITHMS = {
+    new InsertionSort(),
+    new SelectionSort(),
+    new BubbleSortOpt1(),
+
+    // Quicksort
+    new QuicksortVariant1(PivotStrategy.RIGHT),
+    new QuicksortVariant1(PivotStrategy.MIDDLE),
+    new QuicksortVariant1(PivotStrategy.MEDIAN3),
+    new QuicksortImproved(48, new QuicksortVariant1(PivotStrategy.MIDDLE)),
+    new DualPivotQuicksort(DualPivotQuicksort.PivotStrategy.THIRDS),
+    new DualPivotQuicksortImproved(64, DualPivotQuicksort.PivotStrategy.THIRDS),
+    new MergeSort(),
+
+    // Heapsort
+    new Heapsort(),
+    new BottomUpHeapsort(),
+    new HeapsortSlowComparisons(),
+    new BottomUpHeapsortSlowComparisons(),
+    new CountingSort(),
+    new JavaArraysSort()
+  };
 
   private static final int WARM_UPS = 2;
 
@@ -65,6 +64,7 @@ public class UltimateTest {
 
   private static final boolean TEST_SORTED_INPUT = true;
 
+  @SuppressWarnings("PMD.UseConcurrentHashMap") // Not accessed concurrently
   private final Map<String, Scorecard> scorecards = new HashMap<>();
 
   public static void main(String[] args) {
@@ -108,7 +108,7 @@ private void test(SortAlgorithm algorithm, boolean warmingUp) {
   private void test(
       SortAlgorithm algorithm,
       InputOrder inputOrder,
-      Function<Integer, int[]> arraySupplier,
+      IntFunction<int[]> arraySupplier,
       boolean warmingUp) {
     System.out.printf("%n--- %s (order: %s) ---%n", algorithm.getName(), inputOrder);
 
@@ -199,7 +199,7 @@ boolean isSorted() {
 
     @Override
     public String toString() {
-      return name().toLowerCase();
+      return name().toLowerCase(Locale.US);
     }
   }
 }

src/main/java/eu/happycoders/sort/comparisons/CompareImprovedDualPivotQuicksort.java

@@ -20,6 +20,7 @@ public static void main(String[] args) {
     new CompareImprovedDualPivotQuicksort().run();
   }
 
+  @SuppressWarnings("PMD.AvoidLiteralsInIfCondition")
   private void run() {
     List<SortAlgorithm> algorithms = new ArrayList<>();
     algorithms.add(new DualPivotQuicksort(DualPivotQuicksort.PivotStrategy.THIRDS));

src/main/java/eu/happycoders/sort/comparisons/CompareImprovedQuicksort.java

@@ -23,6 +23,7 @@ public static void main(String[] args) {
     new CompareImprovedQuicksort().run();
   }
 
+  @SuppressWarnings("PMD.AvoidLiteralsInIfCondition")
   private void run() {
     List<SortAlgorithm> algorithms = new ArrayList<>();
     algorithms.add(new QuicksortVariant1(PivotStrategy.MIDDLE));

src/main/java/eu/happycoders/sort/comparisons/CompareMergeSorts.java

@@ -19,8 +19,7 @@ public static void main(String[] args) {
   }
 
   private void run() {
-    SortAlgorithm[] algorithms =
-        new SortAlgorithm[] {new MergeSort(), new MergeSort2(), new MergeSort3()};
+    SortAlgorithm[] algorithms = {new MergeSort(), new MergeSort2(), new MergeSort3()};
 
     runTest(algorithms, SIZE);
   }

src/main/java/eu/happycoders/sort/comparisons/DirectComparison.java

@@ -19,11 +19,12 @@ public class DirectComparison {
   private static final int ITERATIONS = 100;
   private static final int PRINT_RESULTS_ALL_X_ITERATIONS = 10;
 
+  @SuppressWarnings("PMD.UseConcurrentHashMap") // Not accessed concurrently
   private final Map<String, Scorecard> scorecards = new HashMap<>();
 
   private int longestNameLength;
 
-  public void runTest(SortAlgorithm[] algorithms, int size) {
+  void runTest(SortAlgorithm[] algorithms, int size) {
     longestNameLength = Scorecard.findLongestAlgorithmName(algorithms);
 
     int numAlgorithms = algorithms.length;
@@ -78,11 +79,9 @@ private long measure(SortAlgorithm sortAlgorithm, int[] elements) {
     long time = System.nanoTime();
     sortAlgorithm.sort(elements);
     time = System.nanoTime() - time;
-    System.out.printf(
-        Locale.US,
-        "  %-" + longestNameLength + "s -> time = %,10.3f ms",
-        sortAlgorithm.getName(),
-        (time / 1_000_000.0));
+
+    String format = "  %-" + longestNameLength + "s -> time = %,10.3f ms";
+    System.out.printf(Locale.US, format, sortAlgorithm.getName(), time / 1_000_000.0);
 
     // Validate that's sorted (and make sure the sorting wasn't optimized away!)
     if (!ArrayUtils.isSorted(elements)) {

src/main/java/eu/happycoders/sort/method/Counters.java

@@ -1,12 +1,14 @@
 package eu.happycoders.sort.method;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Locale;
 
 /**
  * Counter for sort operations.
  *
  * @author <a href="[email protected]">Sven Woltmann</a>
  */
+@SuppressWarnings("PMD.TooManyMethods") // That's OK for this simple counter class
 public class Counters {
 
   private long iterations;
@@ -21,42 +23,42 @@ public void incIterations() {
     iterations++;
   }
 
-  public void addIterations(int x) {
-    iterations += x;
+  public void addIterations(int increment) {
+    iterations += increment;
   }
 
   public void incComparisons() {
     comparisons++;
   }
 
-  public void addComparisons(int x) {
-    comparisons += x;
+  public void addComparisons(int increment) {
+    comparisons += increment;
   }
 
   public void incReads() {
     reads++;
   }
 
-  public void addReads(int x) {
-    reads += x;
+  public void addReads(int increment) {
+    reads += increment;
   }
 
   public void incWrites() {
     writes++;
   }
 
-  public void addWrites(int x) {
-    writes += x;
+  public void addWrites(int increment) {
+    writes += increment;
   }
 
   public void incReadsAndWrites() {
     reads++;
     writes++;
   }
 
-  public void addReadsAndWrites(int x) {
-    reads += x;
-    writes += x;
+  public void addReadsAndWrites(int increment) {
+    reads += increment;
+    writes += increment;
   }
 
   public void incLocalVariableAssignments() {
@@ -69,8 +71,9 @@ public void incLocalVariableAssignments() {
    *
    * <p>Not thread-safe!
    *
-   * @return
+   * @return the second set of counters
    */
+  @SuppressFBWarnings("EI_EXPOSE_REP") // We're intentionally exposing the "phase2" object
   public Counters getPhase2() {
     if (phase2 == null) {
       phase2 = new Counters();

src/main/java/eu/happycoders/sort/method/InsertionSort.java

@@ -5,6 +5,8 @@
  *
  * @author <a href="[email protected]">Sven Woltmann</a>
  */
+// We don't want to use System.arraycopy - we want to demonstrate how the algorithm works
+@SuppressWarnings("PMD.AvoidArrayLoops")
 public class InsertionSort implements SortAlgorithm {
 
   @Override
@@ -27,11 +29,11 @@ public void sort(int[] elements, int fromIndex, int toIndex) {
   }
 
   @Override
-  public void sort(int[] elements, Counters counters) {
-    sort(elements, 0, elements.length, counters);
+  public void sortWithCounters(int[] elements, Counters counters) {
+    sortWithCounters(elements, 0, elements.length, counters);
   }
 
-  public void sort(int[] elements, int fromIndex, int toIndex, Counters counters) {
+  public void sortWithCounters(int[] elements, int fromIndex, int toIndex, Counters counters) {
     for (int i = fromIndex + 1; i < toIndex; i++) {
       counters.incIterations();
 
@@ -44,12 +46,14 @@ public void sort(int[] elements, int fromIndex, int toIndex, Counters counters)
         counters.incIterations();
 
         counters.incComparisons();
-        if (!(number < elements[j - 1])) break;
-
-        elements[j] = elements[j - 1];
-        counters.incReads();
-        counters.incWrites();
-        j--;
+        if (number < elements[j - 1]) {
+          elements[j] = elements[j - 1];
+          counters.incReads();
+          counters.incWrites();
+          j--;
+        } else {
+          break;
+        }
       }
       elements[j] = number;
       counters.incWrites();