Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deeper spotbugs checks #240

Merged
merged 11 commits into from Sep 27, 2022
Merged

Deeper spotbugs checks #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
46d13d8
Deeper spotbugs checks
dheerajodha Jan 30, 2022
e4b09ea
Attempt to fix SpotBugs errors
dheerajodha Feb 3, 2022
805bec7
Merge branch 'master' into update-spotbugs
MarkEWaite Sep 27, 2022
2f4a208
Remove redundant null check
MarkEWaite Sep 27, 2022
d51059b
Add two more spotbugs exclusions
MarkEWaite Sep 27, 2022
83fc30a
Remove unused import
MarkEWaite Sep 27, 2022
0145183
Use consistent indentation
MarkEWaite Sep 27, 2022
0b03d4d
Prevent possible NPE in BlockableBuildTriggerConfig.getNodes()
MarkEWaite Sep 27, 2022
16ca711
unused field is suppressed in the source file
MarkEWaite Sep 27, 2022
4091bd0
Resolve spotbugs null on return path warning
MarkEWaite Sep 27, 2022
77c40ab
Remove an unused suppression
MarkEWaite Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion pom.xml
View file
Open in desktop
Expand Up @@ -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>
5 changes: 4 additions & 1 deletion src/main/java/hudson/plugins/parameterizedtrigger/BlockableBuildTriggerConfig.java
View file
Open in desktop
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/hudson/plugins/parameterizedtrigger/BuildTriggerConfig.java
View file
Open in desktop
Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/hudson/plugins/parameterizedtrigger/NodeParameters.java
View file
Open in desktop
Expand Up @@ -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);
}

Expand Down
3 changes: 3 additions & 0 deletions ...main/java/hudson/plugins/parameterizedtrigger/PredefinedPropertiesBuildTriggerConfig.java
View file
Open in desktop
@@ -1,5 +1,7 @@
package hudson.plugins.parameterizedtrigger;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

import java.util.ArrayList;
import java.util.List;

Expand All @@ -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() {
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/hudson/plugins/parameterizedtrigger/SubProjectData.java
View file
Open in desktop
Expand Up @@ -24,6 +24,8 @@

package hudson.plugins.parameterizedtrigger;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

import hudson.model.Job;

import java.util.Comparator;
Expand All @@ -42,6 +44,7 @@
*
* @author JO Sivtoft
*/
@SuppressFBWarnings(value="SIC_INNER_SHOULD_BE_STATIC_ANON")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add the "justification" field here inside the SuppressFBWarnings? If yes, then can anyone please tell me what exactly should I write here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this refer to the customComparator? If so, the dumb fix would be to make it a static constant. More idiomatic in Java 8 would be to use https://docs.oracle.com/javase/8/docs/api/java/util/Comparator.html#comparing-java.util.function.Function-

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jglick thank you for looking into this PR!
However, I'd need a couple of days to get back to this as I was (and am a little) preoccupied with job interviews + GSoC's application phase. I appreciate your help

public class SubProjectData {

private final Comparator<Job> customComparator = new Comparator<Job>() {
Expand Down
26 changes: 7 additions & 19 deletions suppressed-spotbug-errors.xml
View file
Open in desktop
Expand Up @@ -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>