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

Code clean-up #185

Merged
merged 9 commits into from
Jan 12, 2022
Merged

Code clean-up #185

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d01f456
Replace deprecated method calls
offa Dec 8, 2021
9a181b1
Fix some unchecked warnings and raw types
offa Dec 8, 2021
4ee389f
Make members final
offa Dec 8, 2021
b094fa6
Cleanup exception declarations, imports and toString() calls
offa Dec 8, 2021
e27aa3d
Replace anonymous classes with lambdas
offa Dec 8, 2021
efc1c89
Refactoring
offa Dec 8, 2021
e9be4c8
Add missing nullability annotations
offa Dec 8, 2021
a5808c5
Use UTF-8 charset
offa Jan 11, 2022
bacc03a
Use matcher instead of comparison
offa Jan 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/java/org/jenkinsci/plugins/workflow/FilePathUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import hudson.remoting.Channel;
import hudson.remoting.VirtualChannel;
import hudson.slaves.ComputerListener;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
Expand Down Expand Up @@ -143,7 +142,7 @@ static Collection<String> getChannelNames() {
addChannel(c.getChannel(), c.getName());
}
}
@Override public void preOnline(Computer c, Channel channel, FilePath root, TaskListener listener) throws IOException, InterruptedException {
@Override public void preOnline(Computer c, Channel channel, FilePath root, TaskListener listener) {
addChannel(channel, c.getName());
}

Expand Down
12 changes: 6 additions & 6 deletions src/main/java/org/jenkinsci/plugins/workflow/actions/ArgumentsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,13 @@ public Object getArgumentValue(@NonNull String argumentName) {
public Object getArgumentValueOrReason(@NonNull String argumentName) {
Object ob = getArgumentsInternal().get(argumentName);
if (ob instanceof Map) {
return Collections.unmodifiableMap((Map)ob);
return Collections.unmodifiableMap((Map<?, ?>)ob);
} else if (ob instanceof Set) {
return Collections.unmodifiableSet((Set)ob);
return Collections.unmodifiableSet((Set<?>)ob);
} else if (ob instanceof List) {
return Collections.unmodifiableList((List)ob);
return Collections.unmodifiableList((List<?>)ob);
} else if (ob instanceof Collection) {
return Collections.unmodifiableCollection((Collection)ob);
return Collections.unmodifiableCollection((Collection<?>)ob);
}
return ob;
}
Expand Down Expand Up @@ -302,8 +302,8 @@ public boolean isUnmodifiedArguments() {
return args;
}
// helper method to capture generic type
private static <S extends Step> UninstantiatedDescribable resolve(DescribableModel<S> model, Map<String, Object> arguments) throws Exception {
return model.uninstantiate2(model.instantiate(arguments));
private static <S extends Step> UninstantiatedDescribable resolve(DescribableModel<S> model, Map<String, Object> arguments) {
return model.uninstantiate2(model.instantiate(arguments, null));
}


Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/actions/TagsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class TagsAction implements PersistentAction {
private static final String displayName = "Tags";
private static final String urlSuffix = "tags";

private LinkedHashMap<String, String> tags = new LinkedHashMap<>();
private final LinkedHashMap<String, String> tags = new LinkedHashMap<>();

/**
* Add a tag key:value pair to this FlowNode, null or empty values are ignored
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/flow/FlowCopier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static abstract class ByRun extends FlowCopier {
Queue.Executable originalExec = original.getExecutable();
Queue.Executable copyExec = copy.getExecutable();
if (originalExec instanceof Run && copyExec instanceof Run) {
copy((Run) originalExec, (Run) copyExec, copy.getListener());
copy((Run<?, ?>) originalExec, (Run<?, ?>) copyExec, copy.getListener());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public FlowDurabilityHint getDurabilityHint() {
/**
* Should be called by the flow owner after it is deserialized.
*/
public /*abstract*/ void onLoad(FlowExecutionOwner owner) throws IOException {
Copy link
Member

@dwnusbaum dwnusbaum Feb 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Changes to throws clauses like this break source compatibility with CpsFlowExecution in https://github.com/jenkinsci/workflow-cps-plugin and need to be reverted:

-------------------------------------------------------------
COMPILATION ERROR : 
-------------------------------------------------------------
workflow-cps-plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java:[721,17] onLoad(org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner) in org.jenkinsci.plugins.workflow.cps.CpsFlowExecution cannot override onLoad(org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner) in org.jenkinsci.plugins.workflow.flow.FlowExecution
  overridden method does not throw java.io.IOException
1 error
-------------------------------------------------------------

Copy link
Member

Choose a reason for hiding this comment

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

Indeed this is wrong and should be reverted.

Copy link
Member

Choose a reason for hiding this comment

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

@dwnusbaum if you approve #201 I can cut a release to fix this.

public /*abstract*/ void onLoad(FlowExecutionOwner owner) {
if (Util.isOverridden(FlowExecution.class, getClass(), "onLoad")) {
onLoad();
}
Expand Down
14 changes: 5 additions & 9 deletions src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,7 @@ private synchronized void saveLater() {
final List<FlowExecutionOwner> copy = new ArrayList<>(runningTasks.getView());
LOGGER.log(Level.FINE, "scheduling save of {0}", copy);
try {
executor.submit(new Runnable() {
@Override public void run() {
save(copy);
}
});
executor.submit(() -> save(copy));
} catch (RejectedExecutionException x) {
LOGGER.log(Level.FINE, "could not schedule save, perhaps because Jenkins is shutting down; saving immediately", x);
save(copy);
Expand Down Expand Up @@ -210,7 +206,7 @@ public ListenableFuture<?> apply(final Function<StepExecution, Void> f) {
all.add(execs);
Futures.addCallback(execs,new FutureCallback<List<StepExecution>>() {
@Override
public void onSuccess(List<StepExecution> result) {
public void onSuccess(@NonNull List<StepExecution> result) {
for (StepExecution e : result) {
try {
f.apply(e);
Expand All @@ -221,7 +217,7 @@ public void onSuccess(List<StepExecution> result) {
}

@Override
public void onFailure(Throwable t) {
public void onFailure(@NonNull Throwable t) {
LOGGER.log(Level.WARNING, null, t);
}
}, MoreExecutors.directExecutor());
Expand Down Expand Up @@ -260,7 +256,7 @@ public static class ResumeStepExecutionListener extends FlowExecutionListener {
public void onResumed(@NonNull FlowExecution e) {
Futures.addCallback(e.getCurrentExecutions(false), new FutureCallback<List<StepExecution>>() {
@Override
public void onSuccess(List<StepExecution> result) {
public void onSuccess(@NonNull List<StepExecution> result) {
if (e.isComplete()) {
// WorkflowRun.onLoad will not fire onResumed if the serialized execution was already
// complete when loaded, but right now (at least for workflow-cps), the execution resumes
Expand Down Expand Up @@ -289,7 +285,7 @@ public void onSuccess(List<StepExecution> result) {
}

@Override
public void onFailure(Throwable t) {
public void onFailure(@NonNull Throwable t) {
if (t instanceof CancellationException) {
LOGGER.log(Level.FINE, "Cancelled load of " + e, t);
} else {
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionOwner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ public static FlowExecutionOwner dummyOwner() {

private static class DummyOwner extends FlowExecutionOwner {
DummyOwner() {}
@NonNull
@Override public FlowExecution get() throws IOException {
throw new IOException("not implemented");
}
Expand All @@ -174,7 +175,8 @@ private static class DummyOwner extends FlowExecutionOwner {
@Override public int hashCode() {
return 0;
}
@Override public TaskListener getListener() throws IOException {
@NonNull
@Override public TaskListener getListener() {
return TaskListener.NULL;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/flow/GlobalDefaultFlowDurabilityLevel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public DescriptorImpl() {
/** Null to use the platform default, which may change over time as enhanced options are available. */
private FlowDurabilityHint durabilityHint = null;

@NonNull
@Override
public String getDisplayName() {
return "Global Default Pipeline Durability Level";
Expand All @@ -48,7 +49,7 @@ public void setDurabilityHint(FlowDurabilityHint hint){
}

@Override
public boolean configure(StaplerRequest req, JSONObject json) throws FormException {
public boolean configure(StaplerRequest req, JSONObject json) {
// TODO verify if this is covered by permissions checks or we need an explicit check here.
Object ob = json.opt("durabilityHint");
FlowDurabilityHint hint = null;
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/org/jenkinsci/plugins/workflow/flow/StashManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,8 @@ public interface StashAwareArtifactManager /* extends ArtifactManager */ {
Map<String,String> files = new HashMap<>();
for (String path : srcroot.list("**/*", null, false)) {
files.put(path, path);
InputStream in = srcroot.child(path).open();
try {
try(InputStream in = srcroot.child(path).open()) {
dstDir.child(path).copyFrom(in);
} finally {
IOUtils.closeQuietly(in);
Copy link
Member

Choose a reason for hiding this comment

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

Fix imports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are no unused imports left or do you mean reordering / grouping?

Copy link
Member

Choose a reason for hiding this comment

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

I did not check, I just guessed that IOUtils became unused.

}
}
if (!files.isEmpty()) {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graph/FlowNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ private synchronized void loadActions() {
}

@Exported
@NonNull
@SuppressWarnings("deprecation") // of override
@Override
@SuppressFBWarnings(value = "UG_SYNC_SET_UNSYNC_GET", justification = "CopyOnWrite ArrayList, and field load & modification is synchronized")
Expand Down Expand Up @@ -466,7 +467,7 @@ public int size() {
}

@Override
public boolean removeAll(Collection<?> c) {
public boolean removeAll(@NonNull Collection<?> c) {
boolean changed = actions.removeAll(c);
if (changed) {
persistSafe();
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/jenkinsci/plugins/workflow/graph/FlowStartNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package org.jenkinsci.plugins.workflow.graph;

import edu.umd.cs.findbugs.annotations.NonNull;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;

import java.util.List;
Expand All @@ -44,6 +45,7 @@ public FlowStartNode(FlowExecution exec, String id) {
* @deprecated
* Why are you calling a method that always return empty list?
*/
@NonNull
@Override
public List<FlowNode> getParents() {
return super.getParents();
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/FilteratorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ class FilteratorImpl<T> implements Filterator<T> {
private Iterator<T> wrapped = null;
private Predicate<T> matchCondition = null;

@NonNull
@Override
public FilteratorImpl<T> filter(Predicate<T> matchCondition) {
public FilteratorImpl<T> filter(@NonNull Predicate<T> matchCondition) {
return new FilteratorImpl<>(this, matchCondition);
}

Expand Down
7 changes: 1 addition & 6 deletions src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowScanningUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,7 @@ private FlowScanningUtils() {}
*/
@NonNull
public static Predicate<FlowNode> hasActionPredicate(@NonNull final Class<? extends Action> actionClass) {
return new Predicate<FlowNode>() {
@Override
public boolean apply(FlowNode input) {
return (input != null && input.getAction(actionClass) != null);
}
};
return input -> (input != null && input.getAction(actionClass) != null);
}

// Default predicates, which may be used for common conditions
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ ArrayDeque<ParallelBlockStart> leastCommonAncestor(@NonNull final Set<FlowNode>
ArrayDeque<Fork> parallelForks = new ArrayDeque<>(); // Tracks the discovered forks in order of encounter

Predicate<FlowNode> notAHead = new Predicate<FlowNode>() { // Filter out pre-existing heads
Collection<FlowNode> checkHeads = convertToFastCheckable(heads);
final Collection<FlowNode> checkHeads = convertToFastCheckable(heads);

@Override
public boolean apply(FlowNode input) { return !checkHeads.contains(input); }
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearBlockHoppingScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ protected FlowNode jumpBlockScan(@CheckForNull FlowNode node, @NonNull Collectio
}

@Override
protected FlowNode next(@NonNull FlowNode current, @NonNull Collection<FlowNode> blackList) {
protected FlowNode next(@CheckForNull FlowNode current, @NonNull Collection<FlowNode> blackList) {
if (current == null) {
return null;
}
Expand Down
15 changes: 9 additions & 6 deletions src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.common.base.Predicate;
import org.jenkinsci.plugins.workflow.graph.FlowNode;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import javax.annotation.concurrent.NotThreadSafe;
import java.util.Collection;
Expand Down Expand Up @@ -78,7 +79,7 @@ protected void setHeads(@NonNull Collection<FlowNode> heads) {
}

@Override
protected FlowNode next(FlowNode current, @NonNull Collection<FlowNode> blackList) {
protected FlowNode next(@CheckForNull FlowNode current, @NonNull Collection<FlowNode> blackList) {
if (current == null) {
return null;
}
Expand All @@ -98,8 +99,9 @@ protected FlowNode next(FlowNode current, @NonNull Collection<FlowNode> blackLis
* @deprecated prefer {@link #filteredNodes(FlowNode, Predicate)}
*/
@Deprecated
@NonNull
@Override
public List<FlowNode> filteredNodes(Collection<FlowNode> heads, Predicate<FlowNode> matchPredicate) {
public List<FlowNode> filteredNodes(Collection<FlowNode> heads, @NonNull Predicate<FlowNode> matchPredicate) {
return super.filteredNodes(heads, matchPredicate);
}

Expand All @@ -109,6 +111,7 @@ public List<FlowNode> filteredNodes(Collection<FlowNode> heads, Predicate<FlowNo
* {@inheritDoc}
* @param heads Nodes to start iterating backward from by visiting their parents. <strong>Do not pass multiple heads.</strong>
*/
@NonNull
@Override
public List<FlowNode> filteredNodes(Collection<FlowNode> heads, Collection<FlowNode> blackList, Predicate<FlowNode> matchCondition) {
return super.filteredNodes(heads, blackList, matchCondition);
Expand All @@ -120,7 +123,7 @@ public List<FlowNode> filteredNodes(Collection<FlowNode> heads, Collection<FlowN
*/
@Deprecated
@Override
public FlowNode findFirstMatch(Collection<FlowNode> heads, Predicate<FlowNode> matchPredicate) {
public FlowNode findFirstMatch(Collection<FlowNode> heads, @NonNull Predicate<FlowNode> matchPredicate) {
return super.findFirstMatch(heads, matchPredicate);
}

Expand All @@ -142,7 +145,7 @@ public FlowNode findFirstMatch(Collection<FlowNode> heads, Collection<FlowNode>
* @param heads <strong>Do not pass multiple heads.</strong>
*/
@Override
public void visitAll(Collection<FlowNode> heads, FlowNodeVisitor visitor) {
public void visitAll(Collection<FlowNode> heads, @NonNull FlowNodeVisitor visitor) {
super.visitAll(heads, visitor);
}

Expand All @@ -153,7 +156,7 @@ public void visitAll(Collection<FlowNode> heads, FlowNodeVisitor visitor) {
* @param heads Nodes to start walking the DAG backwards from. <strong>Do not pass multiple heads.</strong>
*/
@Override
public void visitAll(Collection<FlowNode> heads, Collection<FlowNode> blackList, FlowNodeVisitor visitor) {
public void visitAll(Collection<FlowNode> heads, Collection<FlowNode> blackList, @NonNull FlowNodeVisitor visitor) {
super.visitAll(heads, blackList, visitor);
}

Expand All @@ -163,7 +166,7 @@ public void visitAll(Collection<FlowNode> heads, Collection<FlowNode> blackList,
*/
@Deprecated
@Override
public FlowNode findFirstMatch(FlowExecution exec, Predicate<FlowNode> matchPredicate) {
public FlowNode findFirstMatch(FlowExecution exec, @NonNull Predicate<FlowNode> matchPredicate) {
return super.findFirstMatch(exec, matchPredicate);
}

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/MemoryFlowChunk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public MemoryFlowChunk() {

}

@NonNull
@Override
public FlowNode getFirstNode() {
return firstNode;
Expand All @@ -61,7 +62,7 @@ public void setFirstNode(FlowNode firstNode) {
this.firstNode = firstNode;
}


@NonNull
@Override
public FlowNode getLastNode() {
return lastNode;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ParallelFlowChunk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ public interface ParallelFlowChunk <ChunkType extends FlowChunk> extends FlowChu
@NonNull
Map<String, ChunkType> getBranches();

@NonNull
void setBranch(@NonNull String branchName, @NonNull ChunkType branchBlock);
}
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ParallelMemoryFlowChunk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
public class ParallelMemoryFlowChunk extends MemoryFlowChunk implements ParallelFlowChunk<MemoryFlowChunk> {

// LinkedHashMap to preserve insert order
private LinkedHashMap<String, MemoryFlowChunk> branches = new LinkedHashMap<>();
private final LinkedHashMap<String, MemoryFlowChunk> branches = new LinkedHashMap<>();

public ParallelMemoryFlowChunk(@NonNull FlowNode firstNode, @NonNull FlowNode lastNode) {
super (null,firstNode, lastNode, null);
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/org/jenkinsci/plugins/workflow/log/BrokenLogStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package org.jenkinsci.plugins.workflow.log;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Functions;
import hudson.console.AnnotatedLargeText;
import hudson.model.BuildListener;
Expand All @@ -47,20 +48,24 @@ public final class BrokenLogStorage implements LogStorage {
public BrokenLogStorage(Throwable x) {
this.x = x;
}

@Override public BuildListener overallListener() throws IOException, InterruptedException {

@NonNull
@Override public BuildListener overallListener() throws IOException {
throw new IOException(x);
}

@Override public TaskListener nodeListener(FlowNode node) throws IOException, InterruptedException {

@NonNull
@Override public TaskListener nodeListener(@NonNull FlowNode node) throws IOException {
throw new IOException(x);
}

@Override public AnnotatedLargeText<FlowExecutionOwner.Executable> overallLog(FlowExecutionOwner.Executable build, boolean complete) {

@NonNull
@Override public AnnotatedLargeText<FlowExecutionOwner.Executable> overallLog(@NonNull FlowExecutionOwner.Executable build, boolean complete) {
return new BrokenAnnotatedLargeText<>();
}

@Override public AnnotatedLargeText<FlowNode> stepLog(FlowNode node, boolean complete) {

@NonNull
@Override public AnnotatedLargeText<FlowNode> stepLog(@NonNull FlowNode node, boolean complete) {
return new BrokenAnnotatedLargeText<>();
}

Expand Down
Loading