Merge pull request #139 from jimklimov/spotbugs

pom.xml: set spotbugs settings
jenkinsci · Nov 30, 2022 · 495fdd4 · 495fdd4
2 parents 0530ad4 + d58803f
commit 495fdd4
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 9 deletions.
diff --git a/pom.xml b/pom.xml
@@ -34,6 +34,8 @@
         <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
         <jenkins.version>2.361.1</jenkins.version>
         <!-- Note: keep in sync with io.jenkins.tools.bom version below -->
+        <spotbugs.effort>Max</spotbugs.effort>
+        <spotbugs.threshold>Low</spotbugs.threshold>
     </properties>
 
     <dependencyManagement>

diff --git a/src/main/java/hudson/plugins/ircbot/IrcPublisher.java b/src/main/java/hudson/plugins/ircbot/IrcPublisher.java
@@ -142,7 +142,7 @@ public static final class DescriptorImpl extends BuildStepDescriptor<Publisher>
 
         private static final String PREFIX = "irc_publisher.";
 
-        public static final String[] CHARSETS;
+        private static final String[] CHARSETS;
 
         static {
             SortedMap<String, Charset> availableCharsets = Charset.availableCharsets();
@@ -157,6 +157,10 @@ public static final class DescriptorImpl extends BuildStepDescriptor<Publisher>
             CHARSETS = cs;
         }
 
+        public String[] getCharsets() {
+            return CHARSETS;
+        }
+
         boolean enabled = false;
 
         String hostname = null;
@@ -237,6 +241,9 @@ public static final class DescriptorImpl extends BuildStepDescriptor<Publisher>
          * Check boxes values are not passed in the posted form when they are unchecked.
          * The workaround consists in acceding these values via the JSON representation.
          */
+        @Deprecated
+        @SuppressFBWarnings(value="UPM_UNCALLED_PRIVATE_METHOD",
+            justification="Here just in case for UI and config loads")
         private static List<JSONObject> fillChannelsFromJSON(JSONObject root){
             throw new UnsupportedOperationException();
         }
@@ -604,8 +611,8 @@ public String getCharset() {
             return this.charset;
         }
 
-
-
+        @SuppressFBWarnings(value = "SIC_INNER_SHOULD_BE_STATIC_ANON",
+            justification = "No idea how to solve")
         public ParameterNames getParamNames() {
             return new ParameterNames() {
                 @Override
@@ -623,7 +630,7 @@ protected Integer getMessageRateFromSystemProperty() {
             try {
                 return Integer.parseInt(System.getProperty("hudson.plugins.ircbot.messageRate", "500"));
             } catch (NumberFormatException nfe) {
-                return new Integer(500);
+                return Integer.valueOf(500);
             }
         }
 

diff --git a/src/main/java/hudson/plugins/ircbot/steps/IrcNotifyStep.java b/src/main/java/hudson/plugins/ircbot/steps/IrcNotifyStep.java
@@ -8,6 +8,7 @@
  */
 
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 import hudson.Extension;
 import hudson.FilePath;
@@ -184,6 +185,7 @@ private static class IrcNotifyStepExecution extends SynchronousNonBlockingStepEx
         // (e.g. serializable fields change)
         private static final long serialVersionUID = 2;
 
+        @SuppressFBWarnings({"SE_TRANSIENT_FIELD_NOT_RESTORED"})
         private transient final IrcNotifyStep step;
 
         public IrcNotifyStepExecution(@NonNull IrcNotifyStep step, @NonNull StepContext context) {

diff --git a/src/main/java/hudson/plugins/ircbot/v2/IRCConnection.java b/src/main/java/hudson/plugins/ircbot/v2/IRCConnection.java
@@ -75,8 +75,8 @@ public class IRCConnection implements IMConnection, JoinListener, InviteListener
     private final Map<String, Bot> privateChats = new HashMap<String, Bot>();
 
 
-    @SuppressFBWarnings(value="UR_UNINIT_READ",
-        justification="TODO: this is probably a geniune problem but I don't know why")
+    @SuppressFBWarnings(value={"UR_UNINIT_READ", "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR"},
+        justification="UR_UNINIT_READ: TODO: this is probably a genuine problem but I don't know why; MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR: Need ecosystem change to separate IRCConnection construction from PircListener connection")
     public IRCConnection(DescriptorImpl descriptor, AuthenticationHolder authentication) {
         Builder config = new Configuration.Builder();
 
@@ -139,6 +139,10 @@ public IRCConnection(DescriptorImpl descriptor, AuthenticationHolder authenticat
         config.setEncoding(Charset.forName(this.descriptor.getCharset()));
 
         this.listener = new PircListener(this.pircConnection, this.descriptor.getNick());
+        // MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR
+        // https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html
+        // Overridable method is called from constructor
+        // It may also leak the "this" reference of the partially constructed object.
         this.listener.addJoinListener(this);
         this.listener.addInviteListener(this);
         this.listener.addPartListener(this);
@@ -186,6 +190,8 @@ public boolean isConnected() {
     }
 
     //@Override
+    @SuppressFBWarnings(value = "SIC_INNER_SHOULD_BE_STATIC_ANON",
+        justification = "Commented below, no idea how to solve")
     public boolean connect() {
         try {
 
@@ -199,7 +205,9 @@ public boolean connect() {
 
             final CountDownLatch connectLatch = new CountDownLatch(1);
 
-
+            // SIC_INNER_SHOULD_BE_STATIC_ANON.... whatever that means:
+            // The class hudson.plugins.im.build_notify.PrintFailingTestsBuildToChatNotifier$1
+            // could be refactored into a named _static_ inner class.
             ListenerAdapter connectListener = new ListenerAdapter() {
 
                 @Override

diff --git a/src/main/java/hudson/plugins/ircbot/v2/IRCConnectionProvider.java b/src/main/java/hudson/plugins/ircbot/v2/IRCConnectionProvider.java
@@ -1,5 +1,7 @@
 package hudson.plugins.ircbot.v2;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import hudson.plugins.im.IMConnection;
 import hudson.plugins.im.IMConnectionProvider;
 import hudson.plugins.im.IMException;
@@ -19,6 +21,8 @@ public static final synchronized void setDesc(IMPublisherDescriptor desc) throws
         INSTANCE.releaseConnection();
     }
 
+    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR",
+        justification = "Yes we do generally init() ourselves")
     private IRCConnectionProvider() {
         super();
         init();

diff --git a/src/main/resources/hudson/plugins/ircbot/IrcNotifyStep/global.jelly b/src/main/resources/hudson/plugins/ircbot/IrcNotifyStep/global.jelly
@@ -84,7 +84,7 @@
 
         <f:entry title="Notification charset" description="The character set to use for notifications">
             <select name="irc_publisher.charset">
-               <j:forEach var="cs" items="${descriptor.CHARSETS}">
+               <j:forEach var="cs" items="${descriptor.getCharsets()}">
                   <f:option selected="${instance.charset.toString()==cs}">${cs}</f:option>
                </j:forEach>
             </select>

diff --git a/src/main/resources/hudson/plugins/ircbot/IrcPublisher/global.jelly b/src/main/resources/hudson/plugins/ircbot/IrcPublisher/global.jelly
@@ -85,7 +85,7 @@
 
         <f:entry title="Notification charset" description="The character set to use for notifications">
             <select class="setting-input" name="irc_publisher.charset">
-               <j:forEach var="cs" items="${descriptor.CHARSETS}">
+               <j:forEach var="cs" items="${descriptor.getCharsets()}">
                   <f:option selected="${instance.charset.toString()==cs}">${cs}</f:option>
                </j:forEach>
             </select>