Deeper spotbugs checks (#240)

* Deeper spotbugs checks - Increase chance of detecting bugs * Remove redundant null check * Add two more spotbugs exclusions and simplify exclusions Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
jenkinsci · Sep 27, 2022 · 8091841 · 8091841
1 parent fb05de4
commit 8091841
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 26 deletions.
diff --git a/pom.xml b/pom.xml
@@ -226,9 +226,10 @@
     <revision>2.46</revision>
     <changelist>-SNAPSHOT</changelist>
     <gitHubRepo>jenkinsci/parameterized-trigger-plugin</gitHubRepo>
-    <java.level>8</java.level>
     <jenkins.version>2.332.1</jenkins.version>
+    <spotbugs.effort>Max</spotbugs.effort>
     <spotbugs.failOnError>true</spotbugs.failOnError>
+    <spotbugs.threshold>Low</spotbugs.threshold>
     <spotbugs.excludeFilterFile>suppressed-spotbug-errors.xml</spotbugs.excludeFilterFile>
   </properties>
 </project>
diff --git a/src/main/java/hudson/plugins/parameterizedtrigger/BlockableBuildTriggerConfig.java b/src/main/java/hudson/plugins/parameterizedtrigger/BlockableBuildTriggerConfig.java
@@ -15,6 +15,7 @@
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ListMultimap;
+import hudson.model.Label;
 import hudson.model.TaskListener;
 import hudson.model.queue.QueueTaskFuture;
 
@@ -94,7 +95,9 @@ protected QueueTaskFuture schedule(AbstractBuild<?, ?> build, Job project, List<
     }
 
     public Collection<Node> getNodes() {
-        return Jenkins.get().getLabel("asrt").getNodes();
+        Label label = Jenkins.get().getLabel("asrt");
+        if (label==null) return Collections.emptyList();
+        return label.getNodes();
     }
 
     @Extension

diff --git a/src/main/java/hudson/plugins/parameterizedtrigger/BuildTriggerConfig.java b/src/main/java/hudson/plugins/parameterizedtrigger/BuildTriggerConfig.java
@@ -549,9 +549,7 @@ protected QueueTaskFuture schedule(@Nonnull AbstractBuild<?, ?> build, @Nonnull
         // From https://github.com/jenkinsci/jenkins/pull/1771
         Cause cause = createUpstreamCause(build);
         List<Action> queueActions = new ArrayList<>(list);
-        if (cause != null) {
-            queueActions.add(new CauseAction(cause));
-        }
+        queueActions.add(new CauseAction(cause));
 
         // Includes both traditional projects via AbstractProject and Workflow Job
         if (project instanceof ParameterizedJobMixIn.ParameterizedJob) {

diff --git a/src/main/java/hudson/plugins/parameterizedtrigger/NodeParameters.java b/src/main/java/hudson/plugins/parameterizedtrigger/NodeParameters.java
@@ -29,13 +29,16 @@ public NodeParameters() {
 	public Action getAction(AbstractBuild<?, ?> build, TaskListener listener) throws IOException, InterruptedException, DontTriggerException {
 		String nodeName = build.getBuiltOnStr();
 		Label nodeLabel;
-		// master does not return a node name so add it explicitly.
+                String nodeDisplayName;
+		// Controller does not return a node name so add it explicitly.
 		if( StringUtils.isEmpty( nodeName ) ) {
 			nodeLabel = Jenkins.get().getSelfLabel();
+                        nodeDisplayName = nodeLabel.getDisplayName();
 		} else {
 			nodeLabel = Label.get(nodeName);
+                        nodeDisplayName = nodeLabel != null ? nodeLabel.getDisplayName() : "null label of " + nodeName;
 		}
-		listener.getLogger().println("Returning node parameter for " + nodeLabel.getDisplayName());
+		listener.getLogger().println("Returning node parameter for " + nodeDisplayName);
 		return new NodeAction(nodeLabel);
 	}
 

diff --git a/...main/java/hudson/plugins/parameterizedtrigger/PredefinedPropertiesBuildTriggerConfig.java b/...main/java/hudson/plugins/parameterizedtrigger/PredefinedPropertiesBuildTriggerConfig.java
@@ -1,5 +1,7 @@
 package hudson.plugins.parameterizedtrigger;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -19,6 +21,7 @@ public PredefinedPropertiesBuildTriggerConfig(String projects,
     private ResultCondition condition;
     private boolean triggerWithNoParameters;
     private boolean includeCurrentParameters;
+    @SuppressFBWarnings(value = "UUF_UNUSED_FIELD", justification = "Do not risk compatibility")
     private String batchCondition;
 
     public Object readResolve() {

diff --git a/src/main/java/hudson/plugins/parameterizedtrigger/SubProjectData.java b/src/main/java/hudson/plugins/parameterizedtrigger/SubProjectData.java
@@ -24,6 +24,8 @@
 
 package hudson.plugins.parameterizedtrigger;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import hudson.model.Job;
 
 import java.util.Comparator;
@@ -42,6 +44,7 @@
  *
  * @author JO Sivtoft
  */
+@SuppressFBWarnings(value="SIC_INNER_SHOULD_BE_STATIC_ANON")
 public class SubProjectData {
 
     private final Comparator<Job> customComparator = new Comparator<Job>() {

diff --git a/suppressed-spotbug-errors.xml b/suppressed-spotbug-errors.xml
@@ -5,24 +5,12 @@
     for an explanation of the technique
  -->
 <FindBugsFilter>
-    <Match>
-    <Or>
-        <And>
-            <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
-            <Class name="hudson.plugins.parameterizedtrigger.BlockableBuildTriggerConfig"/>
-        </And>
-        <And>
-            <Bug pattern="UUF_UNUSED_FIELD"/>
-            <Class name="hudson.plugins.parameterizedtrigger.FileBuildTriggerConfig"/>
-      </And>
-        <And>
-            <Bug pattern="UUF_UNUSED_FIELD"/>
-            <Class name="hudson.plugins.parameterizedtrigger.PredefinedPropertiesBuildTriggerConfig"/>
-      </And>
-        <And>
-            <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
-            <Class name="hudson.plugins.parameterizedtrigger.NodeParameters"/>
-      </And>
-    </Or>
+  <Match>
+    <Bug pattern="SIC_INNER_SHOULD_BE_STATIC_ANON"/>
+    <Class name="hudson.plugins.parameterizedtrigger.BuildTriggerConfig"/>
+  </Match>
+  <Match>
+    <Bug pattern="DP_DO_INSIDE_DO_PRIVILEGED"/>
+    <Class name="hudson.plugins.parameterizedtrigger.BinaryFileParameterFactory"/>
   </Match>
 </FindBugsFilter>