Merge pull request #6021 from basil/spotbugs

Fix some SpotBugs violations

core/src/main/java/hudson/FilePath.java

@@ -1035,7 +1035,7 @@ public final class FilePath implements SerializableOnlyOverRemoting {
     }
 
     // this reads from arbitrary URL
-    private final class Unpack extends MasterToSlaveFileCallable<Void> {
+    private static final class Unpack extends MasterToSlaveFileCallable<Void> {
         private final URL archive;
         Unpack(URL archive) {
             this.archive = archive;
@@ -2938,7 +2938,7 @@ public final class FilePath implements SerializableOnlyOverRemoting {
     public @CheckForNull String validateAntFileMask(final String fileMasks, final int bound, final boolean caseSensitive) throws IOException, InterruptedException {
         return act(new ValidateAntFileMask(fileMasks, caseSensitive, bound));
     }
-    private class ValidateAntFileMask extends MasterToSlaveFileCallable<String> {
+    private static class ValidateAntFileMask extends MasterToSlaveFileCallable<String> {
         private final String fileMasks;
         private final boolean caseSensitive;
         private final int bound;

core/src/main/java/hudson/PluginManager.java

@@ -88,6 +88,7 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.ServiceLoader;
 import java.util.Set;
@@ -271,10 +272,6 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
                     }
                 }
                 LOGGER.log(WARNING, String.format("Provided custom plugin manager [%s] does not provide any of the suitable constructors. Using default.", pmClassName));
-            } catch(NullPointerException e) {
-                // Class.forName and Class.getConstructor are supposed to never return null though a broken ClassLoader
-                // could break the contract. Just in case we introduce this specific catch to avoid polluting the logs with NPEs.
-                LOGGER.log(WARNING, String.format("Unable to instantiate custom plugin manager [%s]. Using default.", pmClassName));
             } catch(ClassCastException e) {
                 LOGGER.log(WARNING, String.format("Provided class [%s] does not extend PluginManager. Using default.", pmClassName));
             } catch(Exception e) {
@@ -647,7 +644,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
                 }
 
                 names.add(fileName);
-                copyBundledPlugin(url, fileName);
+                copyBundledPlugin(Objects.requireNonNull(url), fileName);
                 copiedPlugins.add(url);
                 try {
                     addDependencies(url, fromPath, dependencies);
@@ -910,9 +907,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
             plugins.add(p);
             if (p.isActive())
                 activePlugins.add(p);
-            synchronized (((UberClassLoader) uberClassLoader).loaded) {
-                ((UberClassLoader) uberClassLoader).loaded.clear();
-            }
+            ((UberClassLoader) uberClassLoader).loaded.clear();
 
             // TODO antimodular; perhaps should have a PluginListener to complement ExtensionListListener?
             CustomClassFilter.Contributed.load();
@@ -1743,7 +1738,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
         void cleanup();
     }
 
-    class FileUploadPluginCopier implements PluginCopier {
+    static class FileUploadPluginCopier implements PluginCopier {
         private FileItem fileItem;
 
         FileUploadPluginCopier(FileItem fileItem) {
@@ -1761,7 +1756,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
         }
     }
 
-    class UrlPluginCopier implements PluginCopier {
+    static class UrlPluginCopier implements PluginCopier {
         private String url;
 
         UrlPluginCopier(String url) {

core/src/main/java/hudson/TcpSlaveAgentListener.java

@@ -134,7 +134,7 @@ public final class TcpSlaveAgentListener extends Thread {
         }
         try {
             return new URL(Jenkins.get().getRootUrl()).getHost();
-        } catch (MalformedURLException | NullPointerException e) {
+        } catch (MalformedURLException e) {
             throw new IllegalStateException("Could not get TcpSlaveAgentListener host name", e);
         }
     }

core/src/main/java/hudson/diagnosis/MemoryUsageMonitor.java

@@ -51,7 +51,7 @@ public final class MemoryUsageMonitor extends PeriodicWork {
     /**
      * A memory group is conceptually a set of memory pools. 
      */
-    public final class MemoryGroup {
+    public static final class MemoryGroup {
         private final List<MemoryPoolMXBean> pools = new ArrayList<>();
 
         /**

core/src/main/java/hudson/model/Queue.java

@@ -2454,7 +2454,7 @@ public class Queue extends ResourceController implements Saveable {
     @Restricted(NoExternalUse.class)
     @ExportedBean(defaultVisibility = 999)
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "it is exported, so it might be used")
-    public class StubItem {
+    public static class StubItem {
 
         @Exported public StubTask task;
 

core/src/main/java/hudson/model/Run.java

@@ -100,6 +100,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -814,6 +815,7 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
         return state ==State.NOT_STARTED;
     }
 
+    @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification = "see JENKINS-45892")
     @Override
     public String toString() {
         if (project == null) {
@@ -1911,7 +1913,7 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
                 }
 
                 // even if the main build fails fatally, try to run post build processing
-                job.post(listener);
+                job.post(Objects.requireNonNull(listener));
 
             } catch (ThreadDeath t) {
                 throw t;

core/src/main/java/hudson/model/queue/CauseOfBlockage.java

@@ -9,6 +9,7 @@ import hudson.model.Node;
 import hudson.model.Queue.Task;
 import hudson.model.TaskListener;
 import hudson.slaves.Cloud;
+import java.util.Objects;
 import org.jvnet.localizer.Localizable;
 
 /**
@@ -45,7 +46,7 @@ public abstract class CauseOfBlockage {
      * Obtains a simple implementation backed by {@link Localizable}.
      */
     public static CauseOfBlockage fromMessage(@NonNull final Localizable l) {
-        l.getKey(); // null check
+        Objects.requireNonNull(l);
         return new CauseOfBlockage() {
             @Override
             public String getShortDescription() {

core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java

@@ -23,6 +23,7 @@
  */
 package hudson.security;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.model.User;
 import java.io.IOException;
 import java.util.logging.Level;
@@ -52,6 +53,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 @Restricted(NoExternalUse.class)
 public final class AuthenticationProcessingFilter2 extends UsernamePasswordAuthenticationFilter {
 
+    @SuppressFBWarnings(value = "HARD_CODE_PASSWORD", justification = "This is a password parameter, not a password")
     public AuthenticationProcessingFilter2(String authenticationGatewayUrl) {
         setRequiresAuthenticationRequestMatcher(new AntPathRequestMatcher("/" + authenticationGatewayUrl, "POST"));
         // Jenkins/login.jelly & SetupWizard/authenticate-security-token.jelly
@@ -59,6 +61,7 @@ public final class AuthenticationProcessingFilter2 extends UsernamePasswordAuthe
         setPasswordParameter("j_password");
     }
 
+    @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT", justification = "request.getSession(true) does in fact have a side effect")
     @Override
     protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain, Authentication authResult) throws IOException, ServletException {
         if (SystemProperties.getInteger(SecurityRealm.class.getName() + ".sessionFixationProtectionMode", 1) == 2) {

core/src/main/java/hudson/tasks/LogRotator.java

@@ -62,7 +62,7 @@ public class LogRotator extends BuildDiscarder {
     
     /** @deprecated Replaced by more generic {@link CompositeIOException}. */
     @Deprecated
-    public class CollatedLogRotatorException extends IOException {
+    public static class CollatedLogRotatorException extends IOException {
         private static final long serialVersionUID = 5944233808072651101L;
         
         public final Collection<Exception> collated;

core/src/main/java/hudson/triggers/SlowTriggerAdminMonitor.java

@@ -94,7 +94,7 @@ public class SlowTriggerAdminMonitor extends AdministrativeMonitor {
         return HttpResponses.redirectViaContextPath("/manage");
     }
 
-    public class Value {
+    public static class Value {
 
         private final LocalDateTime time;
         private Class<? extends TriggerDescriptor> trigger;

core/src/main/java/hudson/util/RemotingDiagnostics.java

@@ -148,6 +148,7 @@ public final class RemotingDiagnostics {
             }
             return out.toString();
         }
+        private static final long serialVersionUID = 1L;
     }
 
     /**

core/src/main/java/jenkins/FilePathFilterAggregator.java

@@ -30,7 +30,7 @@ class FilePathFilterAggregator extends FilePathFilter {
 
         @Override
         public int compareTo(Entry that) {
-            double result = Double.compare(this.ordinal, that.ordinal);
+            int result = Double.compare(this.ordinal, that.ordinal);
 
             if (result < 0) return -1;
             if (result > 0) return 1;

core/src/main/java/jenkins/model/Jenkins.java

@@ -2432,11 +2432,6 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      */
     public @Nullable String getRootUrl() throws IllegalStateException {
         final JenkinsLocationConfiguration config = JenkinsLocationConfiguration.get();
-        if (config == null) {
-            // Try to get standard message if possible
-            final Jenkins j = Jenkins.get();
-            throw new IllegalStateException("Jenkins instance " + j + " has been successfully initialized, but JenkinsLocationConfiguration is undefined.");
-        }
         String url = config.getUrl();
         if(url!=null) {
             return Util.ensureEndsWith(url,"/");
@@ -2453,7 +2448,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
     @CheckForNull
     public String getConfiguredRootUrl() {
         JenkinsLocationConfiguration config = JenkinsLocationConfiguration.get();
-        return config != null ? config.getUrl() : null;
+        return config.getUrl();
     }
 
     /**

core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java

@@ -43,6 +43,7 @@ import hudson.widgets.HistoryWidget;
 import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
+import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.RunIdMigrator;
@@ -362,10 +363,7 @@ public abstract class LazyBuildMixIn<JobT extends Job<JobT,RunT> & Queue.Task &
 
                 if (r == null) {
                     // having two neighbors pointing to each other is important to make RunMap.removeValue work
-                    JobT _parent = asRun().getParent();
-                    if (_parent == null) {
-                        throw new IllegalStateException("no parent for " + asRun().number);
-                    }
+                    JobT _parent = Objects.requireNonNull(asRun().getParent(), "no parent for " + asRun().number);
                     RunT pb = _parent.getLazyBuildMixIn()._getRuns().search(asRun().number - 1, AbstractLazyLoadRunMap.Direction.DESC);
                     if (pb != null) {
                         pb.getRunMixIn().nextBuildR = createReference();   // establish bi-di link

core/src/main/java/jenkins/security/NonSerializableSecurityContext.java

@@ -42,7 +42,9 @@ import org.springframework.security.core.userdetails.UserDetails;
  * @see hudson.security.HttpSessionContextIntegrationFilter2
  * @since 1.509
  */
-@SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "It is not intended to be serialized. Default values will be used in case of deserialization")
+@SuppressFBWarnings(
+        value = {"SE_NO_SERIALVERSIONID", "SE_TRANSIENT_FIELD_NOT_RESTORED"},
+        justification = "It is not intended to be serialized. Default values will be used in case of deserialization")
 @Restricted(NoExternalUse.class)
 public class NonSerializableSecurityContext implements SecurityContext {
     private transient Authentication authentication;

core/src/main/java/jenkins/security/s2m/AdminWhitelistRule.java

@@ -65,12 +65,16 @@ public class AdminWhitelistRule implements StaplerProxy {
         // overwrite 30-default.conf with what we think is the best from the core.
         // this file shouldn't be touched by anyone. For local customization, use other files in the conf dir.
         // 0-byte file is used as a signal from the admin to prevent this overwriting
-        placeDefaultRule(
-                new File(jenkins.getRootDir(), "secrets/whitelisted-callables.d/default.conf"),
-                getClass().getResourceAsStream("callable.conf"));
-        placeDefaultRule(
-                new File(jenkins.getRootDir(), "secrets/filepath-filters.d/30-default.conf"),
-                transformForWindows(getClass().getResourceAsStream("filepath-filter.conf")));
+        try (InputStream callable = getClass().getResourceAsStream("callable.conf")) {
+            placeDefaultRule(
+                    new File(jenkins.getRootDir(), "secrets/whitelisted-callables.d/default.conf"),
+                    callable);
+        }
+        try (InputStream filepathFilter = getClass().getResourceAsStream("filepath-filter.conf")) {
+            placeDefaultRule(
+                    new File(jenkins.getRootDir(), "secrets/filepath-filters.d/30-default.conf"),
+                    transformForWindows(filepathFilter));
+        }
 
         this.whitelisted = new CallableWhitelistConfig(
                 new File(jenkins.getRootDir(),"secrets/whitelisted-callables.d/gui.conf"));

core/src/main/java/jenkins/slaves/StandardOutputSwapper.java

@@ -66,6 +66,7 @@ public class StandardOutputSwapper extends ComputerListener {
             }
         }
 
+        @SuppressFBWarnings(value = "OBL_UNSATISFIED_OBLIGATION", justification = "the obligation is satisfied with libc(7)")
         private void swap(StandardOutputStream stdout) throws IOException, NoSuchMethodException, InstantiationException, IllegalAccessException, InvocationTargetException, UnsatisfiedLinkError {
             // duplicate the OS file descriptor and create FileOutputStream around it
             int out = GNUCLibrary.LIBC.dup(1);

src/spotbugs/spotbugs-excludes.xml

@@ -82,10 +82,6 @@
   <Match>
     <Confidence value="2"/>
     <Or>
-      <And>
-        <Bug pattern="CO_COMPARETO_INCORRECT_FLOATING"/>
-        <Class name="jenkins.FilePathFilterAggregator$Entry"/>
-      </And>
       <And>
         <Bug pattern="COMMAND_INJECTION"/>
         <Or>
@@ -95,13 +91,6 @@
           <Class name="jenkins.slaves.restarter.WinswSlaveRestarter"/>
         </Or>
       </And>
-      <And>
-        <Bug pattern="DCN_NULLPOINTER_EXCEPTION"/>
-        <Or>
-          <Class name="hudson.PluginManager"/>
-          <Class name="hudson.TcpSlaveAgentListener"/>
-        </Or>
-      </And>
       <And>
         <Bug pattern="DM_NUMBER_CTOR"/>
         <Class name="hudson.scheduler.CrontabLexer"/>
@@ -184,10 +173,6 @@
           <Class name="jenkins.util.groovy.GroovyHookScript"/>
         </Or>
       </And>
-      <And>
-        <Bug pattern="HARD_CODE_PASSWORD"/>
-        <Class name="hudson.security.AuthenticationProcessingFilter2"/>
-      </And>
       <And>
         <Bug pattern="HTTP_PARAMETER_POLLUTION"/>
         <Class name="hudson.ProxyConfiguration$DescriptorImpl"/>
@@ -236,10 +221,6 @@
           <Class name="hudson.model.ListView$Listener"/>
         </Or>
       </And>
-      <And>
-        <Bug pattern="JLM_JSR166_UTILCONCURRENT_MONITORENTER"/>
-        <Class name="hudson.PluginManager"/>
-      </And>
       <And>
         <Bug pattern="LI_LAZY_INIT_STATIC"/>
         <Or>
@@ -400,31 +381,10 @@
           <Class name="jenkins.util.VirtualFile$FileVF"/>
         </Or>
       </And>
-      <And>
-        <Bug pattern="NP_NULL_PARAM_DEREF"/>
-        <Or>
-          <Class name="hudson.model.Run"/>
-          <Class name="hudson.PluginManager"/>
-        </Or>
-      </And>
       <And>
         <Bug pattern="NP_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD"/>
         <Class name="jenkins.security.apitoken.LegacyApiTokenAdministrativeMonitor"/>
       </And>
-      <And>
-        <Bug pattern="OBL_UNSATISFIED_OBLIGATION"/>
-        <Or>
-          <Class name="jenkins.security.s2m.AdminWhitelistRule"/>
-          <Class name="jenkins.slaves.StandardOutputSwapper$ChannelSwapper"/>
-        </Or>
-      </And>
-      <And>
-        <Bug pattern="OS_OPEN_STREAM"/>
-        <Or>
-          <Class name="hudson.console.ConsoleNote"/>
-          <Class name="jenkins.slaves.DefaultJnlpSlaveReceiver"/>
-        </Or>
-      </And>
       <And>
         <Bug pattern="PATH_TRAVERSAL_IN"/>
         <Or>
@@ -522,12 +482,7 @@
       </And>
       <And>
         <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
-        <Or>
-          <Class name="hudson.model.Run"/>
-          <Class name="hudson.tasks.Fingerprinter"/>
-          <Class name="jenkins.model.Jenkins"/>
-          <Class name="jenkins.model.lazy.LazyBuildMixIn$RunMixIn"/>
-        </Or>
+        <Class name="hudson.tasks.Fingerprinter"/>
       </And>
       <And>
         <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE"/>
@@ -624,11 +579,7 @@
       </And>
       <And>
         <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
-        <Or>
-          <Class name="hudson.model.queue.BackFiller"/>
-          <Class name="hudson.model.queue.CauseOfBlockage"/>
-          <Class name="hudson.security.AuthenticationProcessingFilter2"/>
-        </Or>
+        <Class name="hudson.model.queue.BackFiller"/>
       </And>
       <And>
         <Bug pattern="SBSC_USE_STRINGBUFFER_CONCATENATION"/>
@@ -648,30 +599,6 @@
           <Class name="hudson.util.ProcessTree$SerializedProcess"/>
         </Or>
       </And>
-      <And>
-        <Bug pattern="SE_NO_SERIALVERSIONID"/>
-        <Or>
-          <Class name="hudson.util.RemotingDiagnostics$Script"/>
-          <Class name="jenkins.security.NonSerializableSecurityContext"/>
-        </Or>
-      </And>
-      <And>
-        <Bug pattern="SIC_INNER_SHOULD_BE_STATIC"/>
-        <Or>
-          <Class name="hudson.diagnosis.MemoryUsageMonitor$MemoryGroup"/>
-          <Class name="hudson.FilePath$Unpack"/>
-          <Class name="hudson.FilePath$ValidateAntFileMask"/>
-          <Class name="hudson.model.Queue$StubItem"/>
-          <Class name="hudson.PluginManager$FileUploadPluginCopier"/>
-          <Class name="hudson.PluginManager$UrlPluginCopier"/>
-          <Class name="hudson.tasks.LogRotator$CollatedLogRotatorException"/>
-          <Class name="hudson.triggers.SlowTriggerAdminMonitor$Value"/>
-        </Or>
-      </And>
-      <And>
-        <Bug pattern="SR_NOT_CHECKED"/>
-        <Class name="hudson.org.apache.tools.tar.TarInputStream"/>
-      </And>
       <And>
         <Bug pattern="STATIC_IV"/>
         <Or>