Fix several CheckStyle and PMD warnings.

The new 8.x release of analysis-pom contains an upgrade to PMD 7 with modified rules and some additional CheckStyle rules. Maven will now correctly break the build locally if there are warnings. Fixes #99
jenkinsci · May 17, 2024 · a2ef3eb · a2ef3eb
1 parent 2bca96d
commit a2ef3eb
Show file tree

Hide file tree

Showing 12 changed files with 37 additions and 91 deletions.
diff --git a/pom.xml b/pom.xml
@@ -156,35 +156,7 @@
         <groupId>org.revapi</groupId>
         <artifactId>revapi-maven-plugin</artifactId>
         <configuration>
-          <versionFormat>\d+\.\d+\.\d+</versionFormat>
-          <checkDependencies>false</checkDependencies>
-          <analysisConfiguration>
-            <revapi.ignore combine.children="append">
-              <item>
-                <code>java.field.serialVersionUIDUnchanged</code>
-                <old>field io.jenkins.plugins.checks.steps.PublishChecksStep.serialVersionUID</old>
-                <serialVersionUID>1</serialVersionUID>
-                <justification>Adding actions list with an initial empty value should not break the compatibility.</justification>
-              </item>
-              <item>
-                <code>java.field.serialVersionUIDUnchanged</code>
-                <old>field io.jenkins.plugins.checks.steps.WithChecksStep.serialVersionUID</old>
-                <serialVersionUID>1</serialVersionUID>
-                <justification>Boolean value added, will have no impact on backwards compat.</justification>
-              </item>
-              <item>
-                <regex>true</regex>
-                <code>java.annotation.*</code>
-                <annotation>@edu.umd.cs.findbugs.annotations.*</annotation>
-                <justification>Annotation should be safe to change.</justification>
-              </item>
-              <item>
-                <regex>true</regex>
-                <code>java.missing.*</code>
-                <justification>Dependencies are not being checked, so they are reported as missing</justification>
-              </item>
-            </revapi.ignore>
-          </analysisConfiguration>
+          <skip>true</skip>
         </configuration>
       </plugin>
       <plugin>

diff --git a/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java b/src/main/java/io/jenkins/plugins/checks/api/TruncatedString.java
@@ -25,10 +25,7 @@
  * </p>
  */
 public class TruncatedString {
-
-    @NonNull
     private final List<String> chunks;
-    @NonNull
     private final String truncationText;
     private final boolean truncateStart;
     private final boolean chunkOnNewlines;

diff --git a/src/main/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisher.java b/src/main/java/io/jenkins/plugins/checks/status/BuildStatusChecksPublisher.java
@@ -41,6 +41,7 @@
  * A publisher which publishes different statuses through the checks API based on the stage of the {@link Queue.Item}
  * or {@link Run}.
  */
+@SuppressWarnings("PMD.CouplingBetweenObjects")
 public final class BuildStatusChecksPublisher {
     private static final Logger LOGGER = Logger.getLogger(BuildStatusChecksPublisher.class.getName());
 
@@ -244,7 +245,6 @@ public void onNewHead(final FlowNode node) {
                 getChecksName(run).ifPresent(checksName -> publish(ChecksPublisherFactory.fromRun(run, TaskListener.NULL),
                         ChecksStatus.IN_PROGRESS, ChecksConclusion.NONE, checksName, getOutput(run, node.getExecution())));
             }
-
         }
     }
 }
diff --git a/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java b/src/main/java/io/jenkins/plugins/checks/status/FlowExecutionAnalyzer.java
@@ -39,7 +39,7 @@ class FlowExecutionAnalyzer {
 
     private final Run<?, ?> run;
     private final FlowExecution execution;
-    private final Stack<Integer> indentationStack = new Stack<>();
+    private final Stack<Integer> indentationStack = new Stack<>(); // NOPMD TODO: replace with DeQueue
     private final boolean suppressLogs;
 
     FlowExecutionAnalyzer(final Run<?, ?> run, final FlowExecution execution, final boolean suppressLogs) {
@@ -128,8 +128,9 @@ private Pair<String, String> processErrorOrWarningRow(final FlowGraphTable.Row r
 
         nodeTextBuilder.append(String.join("", Collections.nCopies(indentationStack.size() + 1, "  ")));
         if (warningAction == null) {
-            nodeTextBuilder.append(String.format("**Error**: *%s*", errorAction.getDisplayName()));
-            nodeSummaryBuilder.append(String.format("```%n%s%n```%n", errorAction.getDisplayName()));
+            var displayName = errorAction == null ? "[no error action]" : errorAction.getDisplayName();
+            nodeTextBuilder.append(String.format("**Error**: *%s*", displayName));
+            nodeSummaryBuilder.append(String.format("```%n%s%n```%n", displayName));
             if (!suppressLogs) {
                 String log = getLog(flowNode);
                 if (StringUtils.isNotBlank(log)) {
@@ -169,7 +170,7 @@ ChecksOutput extractOutput() {
                         .orElseGet(() -> processErrorOrWarningRow(row, errorAction, warningAction));
 
                 // the last title will be used in the ChecksOutput (if any are found)
-                if (!stageOrBranchName.isPresent()) {
+                if (stageOrBranchName.isEmpty()) {
                     title = getPotentialTitle(flowNode, errorAction);
                 }
 
@@ -206,7 +207,7 @@ private static String getLog(final FlowNode flowNode) {
                 return null;
             }
 
-            String outputString = out.toString(StandardCharsets.UTF_8.toString());
+            String outputString = out.toString(StandardCharsets.UTF_8);
             // strip ansi color codes
             return outputString.replaceAll("\u001B\\[[;\\d]*m", "");
         }

diff --git a/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/PublishChecksStep.java
@@ -1,42 +1,24 @@
 package io.jenkins.plugins.checks.steps;
 
-import java.io.IOException;
-import java.io.Serializable;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
-import java.util.stream.Collectors;
-
-import org.apache.commons.lang3.StringUtils;
-
 import edu.hm.hafner.util.VisibleForTesting;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
-
-import org.kohsuke.stapler.DataBoundConstructor;
-import org.kohsuke.stapler.DataBoundSetter;
-import org.jenkinsci.plugins.workflow.steps.Step;
-import org.jenkinsci.plugins.workflow.steps.StepContext;
-import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
-import org.jenkinsci.plugins.workflow.steps.StepExecution;
-import org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution;
 import hudson.Extension;
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.util.ListBoxModel;
+import io.jenkins.plugins.checks.api.*;
+import org.apache.commons.lang3.StringUtils;
+import org.jenkinsci.plugins.workflow.steps.*;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
 
-import io.jenkins.plugins.checks.api.ChecksAction;
-import io.jenkins.plugins.checks.api.ChecksAnnotation;
-import io.jenkins.plugins.checks.api.ChecksConclusion;
-import io.jenkins.plugins.checks.api.ChecksDetails;
-import io.jenkins.plugins.checks.api.ChecksOutput;
-import io.jenkins.plugins.checks.api.ChecksPublisherFactory;
-import io.jenkins.plugins.checks.api.ChecksStatus;
+import java.io.IOException;
+import java.io.Serializable;
+import java.util.*;
+import java.util.stream.Collectors;
 
 /**
  * Pipeline step to publish customized checks.
@@ -93,8 +75,7 @@ public void setDetailsURL(final String detailsURL) {
      * When the {@code status} is {@link ChecksStatus#QUEUED} or {@link ChecksStatus#IN_PROGRESS},
      * the conclusion will be reset to {@link ChecksConclusion#NONE}
      *
-     * @param status
-     *         the status to be set
+     * @param status the status to be set
      */
     @DataBoundSetter
     public void setStatus(final ChecksStatus status) {
@@ -216,7 +197,9 @@ static class PublishChecksStepExecution extends SynchronousNonBlockingStepExecut
 
         @Override
         protected Void run() throws IOException, InterruptedException {
-            ChecksPublisherFactory.fromRun(getContext().get(Run.class), getContext().get(TaskListener.class))
+            ChecksPublisherFactory.fromRun(
+                            Objects.requireNonNull(getContext().get(Run.class)),
+                            Objects.requireNonNull(getContext().get(TaskListener.class)))
                     .publish(extractChecksDetails());
 
             return null;
@@ -277,14 +260,10 @@ public static class StepChecksAnnotation extends AbstractDescribableImpl<StepChe
         /**
          * Creates an annotation with required parameters.
          *
-         * @param path
-         *         path of the file to annotate
-         * @param startLine
-         *         start line of the annotation
-         * @param endLine
-         *         end line of the annotation
-         * @param message
-         *         annotation message
+         * @param path      path of the file to annotate
+         * @param startLine start line of the annotation
+         * @param endLine   end line of the annotation
+         * @param message   annotation message
          */
         @DataBoundConstructor
         public StepChecksAnnotation(final String path, final int startLine, final int endLine, final String message) {
@@ -424,10 +403,8 @@ public static class StepChecksAction extends AbstractDescribableImpl<StepChecksA
         /**
          * Creates an instance that wraps a newly constructed {@link ChecksAction} with according parameters.
          *
-         * @param label
-         *         label of the action to display in the checks report on SCMs
-         * @param identifier
-         *         identifier for the action, useful to identify which action is requested by users
+         * @param label      label of the action to display in the checks report on SCMs
+         * @param identifier identifier for the action, useful to identify which action is requested by users
          */
         @DataBoundConstructor
         public StepChecksAction(final String label, final String identifier) {

diff --git a/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java b/src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java
@@ -56,7 +56,7 @@ public boolean isIncludeStage() {
     }
 
     @DataBoundSetter
-    public void setIncludeStage(boolean includeStage) {
+    public void setIncludeStage(final boolean includeStage) {
         this.includeStage = includeStage;
     }
 
@@ -93,7 +93,6 @@ public String getDisplayName() {
     }
 
     private static class WithChecksPublishException extends Exception {
-
         public static final long serialVersionUID = 1L;
 
         WithChecksPublishException(final Throwable cause) {
@@ -259,7 +258,6 @@ public void onFailure(final StepContext context, final Throwable t) {
                                     .withTitle("Cancelled")
                                     .withText(summary)
                                     .build());
-
                 }
                 else {
                     builder.withConclusion(ChecksConclusion.FAILURE)

diff --git a/src/main/java/io/jenkins/plugins/checks/utils/FlowNodeUtils.java b/src/main/java/io/jenkins/plugins/checks/utils/FlowNodeUtils.java
@@ -68,4 +68,8 @@ private static boolean isStageNode(@NonNull final FlowNode node) {
             return false;
         }
     }
+
+    private FlowNodeUtils() {
+        // prevent instantiation
+    }
 }
diff --git a/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java b/src/test/java/io/jenkins/plugins/checks/api/TruncatedStringTest.java
@@ -1,7 +1,6 @@
 package io.jenkins.plugins.checks.api;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -28,11 +27,12 @@ private static Stream<Arguments> parameters() {
                 Arguments.of(true, true));
     }
 
-    private TruncatedString.Builder builder;
+    private TruncatedString.Builder builder = makeBuilder();
 
-    @BeforeEach
-    public void makeBuilder() {
+    private TruncatedString.Builder makeBuilder() {
         this.builder = new TruncatedString.Builder().withTruncationText(MESSAGE);
+
+        return builder;
     }
 
     private String build(final boolean chunkOnChars, final int maxSize) {

diff --git a/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepITest.java b/src/test/java/io/jenkins/plugins/checks/steps/PublishChecksStepITest.java
@@ -22,7 +22,6 @@
  * Tests the pipeline step to publish checks.
  */
 class PublishChecksStepITest extends IntegrationTestWithJenkinsPerTest {
-
     private CapturingChecksPublisher.Factory getFactory() {
         return getJenkins().getInstance().getExtensionList(ChecksPublisherFactory.class)
                 .stream()

diff --git a/src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepITest.java b/src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepITest.java
@@ -30,7 +30,6 @@
  * Tests the "withChecks" step.
  */
 class WithChecksStepITest extends IntegrationTestWithJenkinsPerTest {
-
     /**
      * Tests that the step can inject the {@link ChecksInfo} into the closure.
      */

diff --git a/src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepTest.java b/src/test/java/io/jenkins/plugins/checks/steps/WithChecksStepTest.java
@@ -19,7 +19,7 @@ void shouldStartWithCorrectExecution() throws IOException, InterruptedException
         when(context.get(Run.class)).thenReturn(mock(Run.class));
         when(context.get(TaskListener.class)).thenReturn(TaskListener.NULL);
 
-        assertThat(((WithChecksStep.WithChecksStepExecution) (new WithChecksStep("test").start(context)))
+        assertThat(((WithChecksStep.WithChecksStepExecution) new WithChecksStep("test").start(context))
                 .extractChecksInfo())
                 .hasFieldOrPropertyWithValue("name", "test");
     }

diff --git a/src/test/java/io/jenkins/plugins/checks/util/CapturingChecksPublisher.java b/src/test/java/io/jenkins/plugins/checks/util/CapturingChecksPublisher.java
@@ -45,7 +45,6 @@
  * </p>
  */
 public class CapturingChecksPublisher extends ChecksPublisher {
-
     private final List<ChecksDetails> publishedChecks = new ArrayList<>();
 
     @Override
-Original file line number
+Diff line change
@@ Expand Up / @@ -30,7 +30,6 @@ @@
      * Tests the "withChecks" step.
      */
     class WithChecksStepITest extends IntegrationTestWithJenkinsPerTest {
         /**
          * Tests that the step can inject the {@link ChecksInfo} into the closure.
          */
@@ Expand Down @@