Merge pull request #125 from jimklimov/spotbugs

pom.xml: set spotbugs settings
jenkinsci · Nov 30, 2022 · d902607 · d902607
2 parents 27f57b6 + 086ab15
commit d902607
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 4 deletions.
diff --git a/pom.xml b/pom.xml
@@ -33,6 +33,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/im/bot/Bot.java b/src/main/java/hudson/plugins/im/bot/Bot.java
@@ -3,6 +3,7 @@
  */
 package hudson.plugins.im.bot;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Extension;
 import hudson.plugins.im.AuthenticationHolder;
 import hudson.plugins.im.IMChat;
@@ -77,6 +78,8 @@ public String getHelp() {
 
     private final AuthenticationHolder authentication;
 
+    @SuppressFBWarnings(value = "MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR",
+        justification = "Need ecosystem change to separate Bot construction from IMChat connection")
     public Bot(IMChat chat, String nick, String imServer,
             String commandPrefix, AuthenticationHolder authentication
             ) {
@@ -92,6 +95,10 @@ public Bot(IMChat chat, String nick, String imServer,
                 this.cmdsAndAliases.put(name,cmd);
         }
 
+        // MC_OVERRIDABLE_METHOD_CALL_IN_CONSTRUCTOR
+        // https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html
+        // Overridable method addMessageListener is called from constructor
+        // It may also leak the "this" reference of the partially constructed object.
         chat.addMessageListener(this);
     }
 

diff --git a/src/main/java/hudson/plugins/im/bot/BuildCommand.java b/src/main/java/hudson/plugins/im/bot/BuildCommand.java
@@ -180,7 +180,7 @@ List<ParameterValue> parseBuildParameters(String[] args,
         List<ParameterValue> parameters = new ArrayList<ParameterValue>();
         ParametersDefinitionProperty propDefs = project.getProperty(ParametersDefinitionProperty.class);
         for (ParameterDefinition pd : propDefs.getParameterDefinitions()) {
-            if (pd.getName() != null && parsedParameters.containsKey(pd.getName())) {
+            if (parsedParameters.containsKey(pd.getName())) {
                 if (pd instanceof SimpleParameterDefinition) {
                     SimpleParameterDefinition spd = (SimpleParameterDefinition) pd;
                     parameters.add(spd.createValue(parsedParameters.get(pd.getName())));

diff --git a/src/main/java/hudson/plugins/im/bot/CurrentlyBuildingCommand.java b/src/main/java/hudson/plugins/im/bot/CurrentlyBuildingCommand.java
@@ -107,9 +107,7 @@ public void executeCommand(Bot bot, IMChat chat, IMMessage message,
         String rootUrl = null;
         if (reportUrls && !reportCountOnly) {
             JenkinsLocationConfiguration cfg = JenkinsLocationConfiguration.get();
-            if (cfg != null) {
-                rootUrl = cfg.getUrl();
-            }
+            rootUrl = cfg.getUrl();
             if (rootUrl == null) {
                 msg.append("\n- WARNING: Could not determine Jenkins URL for reporting.\n");
             } else {

diff --git a/src/main/java/hudson/plugins/im/build_notify/PrintFailingTestsBuildToChatNotifier.java b/src/main/java/hudson/plugins/im/build_notify/PrintFailingTestsBuildToChatNotifier.java
@@ -1,5 +1,6 @@
 package hudson.plugins.im.build_notify;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Extension;
 import hudson.model.Run;
 import hudson.model.TaskListener;
@@ -61,13 +62,21 @@ public String upstreamCommitterMessage(IMPublisher publisher,
         return msg + getFailedTestsReport(build);
     }
 
+    @SuppressFBWarnings(value = "SIC_INNER_SHOULD_BE_STATIC_ANON",
+        justification = "Commented below, no idea how to solve")
     private CharSequence getFailedTestsReport(Run<?, ?> build) {
         AbstractTestResultAction testResultAction = build.getAction(AbstractTestResultAction.class);
         if (testResultAction == null || testResultAction.getFailCount() == 0) {
             return "";
         }
 
         StringBuilder buf = new StringBuilder();
+        // 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.
+        // IDEA says: Unchecked assignment: 'java.util.List' to 'java.util.List<hudson.tasks.junit.CaseResult>'.
+        //   Reason: 'testResultAction' has raw type, so result of getFailedTests is erased.
+        // and suggests to change to "raw" List, then complains it is raw :\
         List<CaseResult> failedTests = testResultAction.getFailedTests();
 
         Collections.sort(failedTests, new Comparator<CaseResult>() {