Cleaned-up SpotBugs

ao-apps · Sep 17, 2023 · e76385a · e76385a
1 parent 95e6b5c
commit e76385a
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 49 deletions.
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -41,6 +41,7 @@ def upstreamProjects = [
   // No Jenkins: <groupId>joda-time</groupId><artifactId>joda-time</artifactId>
   'model', // <groupId>com.semanticcms</groupId><artifactId>semanticcms-core-model</artifactId>
   'servlet', // <groupId>com.semanticcms</groupId><artifactId>semanticcms-core-servlet</artifactId>
+  // No Jenkins: <groupId>com.github.spotbugs</groupId><artifactId>spotbugs-annotations</artifactId>
 
   // BOM
   '../../oss/javaee-web-api-bom', // <groupId>com.aoapps</groupId><artifactId>javaee-web-api-bom</artifactId>

diff --git a/pom.xml b/pom.xml
@@ -44,6 +44,7 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
     <sonar.coverage.exclusions>**.*</sonar.coverage.exclusions>
     <!-- Dependencies -->
     <commons-lang3.version>3.13.0</commons-lang3.version>
+    <spotbugs-annotations.version>4.7.3</spotbugs-annotations.version>
   </properties>
 
   <name>SemanticCMS Core Sitemap</name>
@@ -112,18 +113,6 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
     </repository>
   </repositories>
 
-  <build>
-    <plugins>
-      <plugin>
-        <groupId>com.github.spotbugs</groupId><artifactId>spotbugs-maven-plugin</artifactId>
-        <configuration>
-          <!-- TODO: Clean-up SpotBugs -->
-          <failOnError>false</failOnError>
-        </configuration>
-      </plugin>
-    </plugins>
-  </build>
-
   <profiles>
     <profile>
       <id>offlineLinks</id><activation><file><exists>src/main/java</exists></file></activation>
@@ -198,6 +187,11 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
                       <includes>element-list, package-list</includes>
                       <outputDirectory>${project.build.directory}/offlineLinks/com.semanticcms/semanticcms-core-servlet</outputDirectory>
                     </artifactItem>
+                    <artifactItem>
+                      <groupId>com.github.spotbugs</groupId><artifactId>spotbugs-annotations</artifactId><classifier>javadoc</classifier>
+                      <includes>element-list, package-list</includes>
+                      <outputDirectory>${project.build.directory}/offlineLinks/com.github.spotbugs/spotbugs-annotations</outputDirectory>
+                    </artifactItem>
                   </artifactItems>
                 </configuration>
               </execution>
@@ -257,6 +251,10 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
                   <url>https://semanticcms.com/core/servlet/apidocs/</url>
                   <location>${project.build.directory}/offlineLinks/com.semanticcms/semanticcms-core-servlet</location>
                 </offlineLink>
+                <offlineLink>
+                  <url>https://javadoc.io/doc/com.github.spotbugs/spotbugs-annotations/${spotbugs-annotations.version}/</url>
+                  <location>${project.build.directory}/offlineLinks/com.github.spotbugs/spotbugs-annotations</location>
+                </offlineLink>
               </offlineLinks>
             </configuration>
           </plugin>
@@ -352,6 +350,9 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
       <dependency>
         <groupId>com.semanticcms</groupId><artifactId>semanticcms-core-servlet</artifactId><version>1.18.4${POST-SNAPSHOT}</version>
       </dependency>
+      <dependency>
+        <groupId>com.github.spotbugs</groupId><artifactId>spotbugs-annotations</artifactId><version>${spotbugs-annotations.version}</version>
+      </dependency>
       <!-- Transitive -->
       <dependency>
         <groupId>com.aoapps</groupId><artifactId>ao-collections</artifactId><version>3.0.0${POST-SNAPSHOT}</version>
@@ -394,6 +395,9 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
       </dependency>
       <!-- javaee-web-api-bom: <groupId>javax.el</groupId><artifactId>javax.el-api</artifactId> -->
       <!-- javaee-web-api-bom: <groupId>javax.servlet.jsp</groupId><artifactId>javax.servlet.jsp-api</artifactId> -->
+      <dependency>
+        <groupId>com.google.code.findbugs</groupId><artifactId>jsr305</artifactId><version>3.0.2</version>
+      </dependency>
       <!-- Imports -->
       <dependency>
         <groupId>com.aoapps</groupId><artifactId>javaee-web-api-bom</artifactId><version>7.0.1-POST-SNAPSHOT</version>
@@ -444,6 +448,10 @@ along with semanticcms-core-sitemap.  If not, see <https://www.gnu.org/licenses/
     <dependency>
       <groupId>com.semanticcms</groupId><artifactId>semanticcms-core-servlet</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.github.spotbugs</groupId><artifactId>spotbugs-annotations</artifactId>
+      <optional>true</optional>
+    </dependency>
     <!-- Imports (Reactor build order only) -->
     <dependency>
       <groupId>com.aoapps</groupId><artifactId>javaee-web-api-bom</artifactId><version>7.0.1-POST-SNAPSHOT</version>

diff --git a/src/main/java/com/semanticcms/core/sitemap/SiteMapIndexServlet.java b/src/main/java/com/semanticcms/core/sitemap/SiteMapIndexServlet.java
@@ -47,6 +47,7 @@
 import com.semanticcms.core.servlet.ConcurrencyCoordinator;
 import com.semanticcms.core.servlet.SemanticCMS;
 import com.semanticcms.core.servlet.View;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -223,23 +224,26 @@ private static boolean hasSiteMapUrl(
       final Book book,
       PageRef pageRef
   ) throws ServletException, IOException {
-    Boolean result = CapturePage.traversePagesAnyOrder(
-        servletContext,
+    @SuppressWarnings("Convert2Lambda") // Cannot put @SuppressFBWarnings on lambda
+    Boolean result = CapturePage.traversePagesAnyOrder(servletContext,
         req,
         resp,
         pageRef,
-        CaptureLevel.META,
-        page -> {
-          // TODO: Chance for more concurrency here by view?
-          for (View view : views) {
-            if (
-                view.getAllowRobots(servletContext, req, resp, page)
-                    && view.isApplicable(servletContext, req, resp, page)
-            ) {
-              return true;
+        CaptureLevel.META, new CapturePage.PageHandler<>() {
+          @Override
+          @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
+          public Boolean handlePage(Page page) throws ServletException, IOException {
+            // TODO: Chance for more concurrency here by view?
+            for (View view : views) {
+              if (
+                  view.getAllowRobots(servletContext, req, resp, page)
+                  && view.isApplicable(servletContext, req, resp, page)
+                  ) {
+                return true;
+              }
             }
+            return null;
           }
-          return null;
         },
         Page::getChildRefs,
         childPage -> book.equals(childPage.getBook())

diff --git a/src/main/java/com/semanticcms/core/sitemap/SiteMapServlet.java b/src/main/java/com/semanticcms/core/sitemap/SiteMapServlet.java
@@ -36,6 +36,7 @@
 import com.semanticcms.core.servlet.CapturePage;
 import com.semanticcms.core.servlet.SemanticCMS;
 import com.semanticcms.core.servlet.View;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.nio.charset.Charset;
@@ -94,6 +95,7 @@ private static Book getBook(SemanticCMS semanticCms, HttpServletRequest req) {
    *
    * @return  the most recently last modified or {@code null} if unknown
    */
+  @SuppressWarnings("Convert2Lambda")
   static ReadableInstant getLastModified(
       final ServletContext servletContext,
       final HttpServletRequest req,
@@ -104,35 +106,37 @@ static ReadableInstant getLastModified(
     // The most recent is kept here, but set to null the first time a missing
     // per page/view last modified time is found
     final ReadableInstant[] result = new ReadableInstant[1];
-    CapturePage.traversePagesAnyOrder(
-        servletContext,
+    CapturePage.traversePagesAnyOrder(servletContext,
         req,
         resp,
         book.getContentRoot(),
-        CaptureLevel.META,
-        page -> {
-          // TODO: Chance for more concurrency here by view?
-          for (View view : views) {
-            if (
-                view.getAllowRobots(servletContext, req, resp, page)
-                    && view.isApplicable(servletContext, req, resp, page)
-            ) {
-              ReadableInstant lastModified = view.getLastModified(servletContext, req, resp, page);
-              if (lastModified == null) {
-                // Stop searching, return null for this book
-                result[0] = null;
-                return false;
-              } else {
-                if (
-                    result[0] == null
-                        || lastModified.compareTo(result[0]) > 0
-                ) {
-                  result[0] = lastModified;
+        CaptureLevel.META, new CapturePage.PageHandler<>() {
+          @Override
+          @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
+          public Boolean handlePage(Page page) throws ServletException, IOException {
+            // TODO: Chance for more concurrency here by view?
+            for (View view : views) {
+              if (
+                  view.getAllowRobots(servletContext, req, resp, page)
+                  && view.isApplicable(servletContext, req, resp, page)
+                  ) {
+                ReadableInstant lastModified = view.getLastModified(servletContext, req, resp, page);
+                if (lastModified == null) {
+                  // Stop searching, return null for this book
+                  result[0] = null;
+                  return false;
+                } else {
+                  if (
+                      result[0] == null
+                      || lastModified.compareTo(result[0]) > 0
+                      ) {
+                    result[0] = lastModified;
+                  }
                 }
               }
             }
+            return null;
           }
-          return null;
         },
         Page::getChildRefs,
         childPage -> book.equals(childPage.getBook())

diff --git a/src/main/java/com/semanticcms/core/sitemap/SiteMapUrl.java b/src/main/java/com/semanticcms/core/sitemap/SiteMapUrl.java
@@ -24,6 +24,7 @@
 package com.semanticcms.core.sitemap;
 
 import com.aoapps.lang.Strings;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.joda.time.ReadableInstant;
 
 /**
@@ -61,12 +62,13 @@ ReadableInstant getLastmod() {
    * </p>
    */
   @Override
+  @SuppressFBWarnings("EQ_COMPARETO_USE_OBJECT_EQUALS")
   public int compareTo(SiteMapUrl o) {
     if (lastmod != null) {
       if (o.lastmod != null) {
-        int diff = lastmod.compareTo(o.lastmod);
+        int diff = o.lastmod.compareTo(lastmod); // Reversed order for descending
         if (diff != 0) {
-          return -diff;
+          return diff;
         }
       } else {
         // nulls last

diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java
@@ -1,6 +1,6 @@
 /*
  * semanticcms-core-sitemap - Automatic sitemaps for SemanticCMS.
- * Copyright (C) 2021, 2022  AO Industries, Inc.
+ * Copyright (C) 2021, 2022, 2023  AO Industries, Inc.
  *     support@aoindustries.com
  *     7262 Bull Pen Cir
  *     Mobile, AL 36695
@@ -36,6 +36,7 @@
   requires org.joda.time; // <groupId>joda-time</groupId><artifactId>joda-time</artifactId>
   requires com.semanticcms.core.model; // <groupId>com.semanticcms</groupId><artifactId>semanticcms-core-model</artifactId>
   requires com.semanticcms.core.servlet; // <groupId>com.semanticcms</groupId><artifactId>semanticcms-core-servlet</artifactId>
+  requires static com.github.spotbugs.annotations; // <groupId>com.github.spotbugs</groupId><artifactId>spotbugs-annotations</artifactId>
   // Java SE
   requires java.logging;
 }