ARTEMIS-4345: update to Error Prone 2.20.0 and fix up new errors

artemis-cli/src/test/java/org/apache/activemq/cli/test/FileBrokerTest.java

@@ -48,9 +48,8 @@ public class FileBrokerTest {
    public void startWithoutJMS() throws Exception {
       ServerDTO serverDTO = new ServerDTO();
       serverDTO.configuration = "broker-nojms.xml";
-      FileBroker broker = null;
+      FileBroker broker = new FileBroker(serverDTO, new ActiveMQJAASSecurityManager(), null);
       try {
-         broker = new FileBroker(serverDTO, new ActiveMQJAASSecurityManager(), null);
          broker.start();
          JMSServerManagerImpl jmsServerManager = (JMSServerManagerImpl) broker.getComponents().get("jms");
          Assert.assertNull(jmsServerManager);
@@ -59,7 +58,6 @@ public class FileBrokerTest {
          Assert.assertTrue(activeMQServer.isStarted());
          Assert.assertTrue(broker.isStarted());
       } finally {
-         assert broker != null;
          broker.stop();
       }
    }
@@ -145,7 +143,6 @@ public class FileBrokerTest {
 
          locator.close();
       } finally {
-         assert broker != null;
          broker.stop();
          if (path != null) {
             replacePatternInFile(path, "X", "guest");
@@ -194,7 +191,6 @@ public class FileBrokerTest {
 
          locator.close();
       } finally {
-         assert broker != null;
          broker.stop();
          if (path != null) {
             replacePatternInFile(path, "X", "guest");

artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ActiveMQScheduledComponentTest.java

@@ -106,7 +106,9 @@ public class ActiveMQScheduledComponentTest {
       local.start();
       final long newInitialDelay = 1000;
       //the parameters are valid?
-      assert initialDelay != newInitialDelay && newInitialDelay != period;
+      Assert.assertTrue(initialDelay != newInitialDelay);
+      Assert.assertTrue(newInitialDelay != period);
+
       local.setInitialDelay(newInitialDelay);
       local.stop();
       Assert.assertEquals("the initial dalay can't change", newInitialDelay, local.getInitialDelay());

artemis-commons/src/test/java/org/apache/activemq/artemis/utils/collections/SparseArrayLinkedListTest.java

@@ -160,7 +160,7 @@ public class SparseArrayLinkedListTest {
       for (int i = 1; i < elements; i++) {
          list.add(i);
       }
-      Assert.assertEquals(elements - 1, list.remove(e -> e != zero));
+      Assert.assertEquals(elements - 1, list.remove(e -> !zero.equals(e)));
       final ArrayList<Integer> remaining = new ArrayList<>();
       Assert.assertEquals(1, list.clear(remaining::add));
       Assert.assertEquals(0, list.size());

artemis-ra/src/main/java/org/apache/activemq/artemis/ra/ActiveMQRaUtils.java

@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.UUID;
 
 import org.jgroups.JChannel;
@@ -51,20 +52,8 @@ public final class ActiveMQRaUtils {
     * @param you Second value
     * @return True if object equals else false.
     */
-   @SuppressWarnings("StringEquality")
    public static boolean compare(final String me, final String you) {
-      // If both null or intern equals
+      return Objects.equals(me, you);
-      if (me == you) {
-         return true;
-      }
-
-      // if me null and you are not
-      if (me == null) {
-         return false;
-      }
-
-      // me will not be null, test for equality
-      return me.equals(you);
    }
 
    /**
@@ -75,18 +64,7 @@ public final class ActiveMQRaUtils {
     * @return True if object equals else false.
     */
    public static boolean compare(final Integer me, final Integer you) {
-      // If both null or intern equals
+      return Objects.equals(me, you);
-      if (me == you) {
-         return true;
-      }
-
-      // if me null and you are not
-      if (me == null) {
-         return false;
-      }
-
-      // me will not be null, test for equality
-      return me.equals(you);
    }
 
    /**
@@ -97,18 +75,7 @@ public final class ActiveMQRaUtils {
     * @return True if object equals else false.
     */
    public static boolean compare(final Long me, final Long you) {
-      // If both null or intern equals
+      return Objects.equals(me, you);
-      if (me == you) {
-         return true;
-      }
-
-      // if me null and you are not
-      if (me == null) {
-         return false;
-      }
-
-      // me will not be null, test for equality
-      return me.equals(you);
    }
 
    /**
@@ -119,18 +86,7 @@ public final class ActiveMQRaUtils {
     * @return True if object equals else false.
     */
    public static boolean compare(final Double me, final Double you) {
-      // If both null or intern equals
+      return Objects.equals(me, you);
-      if (me == you) {
-         return true;
-      }
-
-      // if me null and you are not
-      if (me == null) {
-         return false;
-      }
-
-      // me will not be null, test for equality
-      return me.equals(you);
    }
 
    /**
@@ -141,18 +97,7 @@ public final class ActiveMQRaUtils {
     * @return True if object equals else false.
     */
    public static boolean compare(final Boolean me, final Boolean you) {
-      // If both null or intern equals
+      return Objects.equals(me, you);
-      if (me == you) {
-         return true;
-      }
-
-      // if me null and you are not
-      if (me == null) {
-         return false;
-      }
-
-      // me will not be null, test for equality
-      return me.equals(you);
    }
 
    /**

artemis-ra/src/main/java/org/apache/activemq/artemis/ra/inflow/ActiveMQActivationSpec.java

@@ -828,9 +828,9 @@ public class ActiveMQActivationSpec extends ConnectionFactoryProperties implemen
 
       if (acknowledgeMode != null ? !acknowledgeMode.equals(that.acknowledgeMode) : that.acknowledgeMode != null)
          return false;
-      if (subscriptionDurability != that.subscriptionDurability)
+      if (subscriptionDurability != null ? !subscriptionDurability.equals(that.subscriptionDurability) : that.subscriptionDurability != null)
          return false;
-      if (shareSubscriptions != that.shareSubscriptions)
+      if (shareSubscriptions != null ? !shareSubscriptions.equals(that.shareSubscriptions) : that.shareSubscriptions != null)
          return false;
       if (strConnectorClassName != null ? !strConnectorClassName.equals(that.strConnectorClassName) : that.strConnectorClassName != null)
          return false;

artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java

@@ -1410,9 +1410,18 @@ public class ConfigurationImplTest extends ActiveMQTestBase {
 
       Assert.assertEquals(1, configuration.getSecurityRoles().size());
       Assert.assertEquals(1, configuration.getSecurityRoles().get("TEST").size());
-      Assert.assertTrue(configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null).isConsume());
+
-      Assert.assertTrue(configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null).isSend());
+      Role role = configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null);
-      Assert.assertFalse(configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null).isCreateAddress());
+      Assert.assertNotNull(role);
+      Assert.assertTrue(role.isConsume());
+
+      role = configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null);
+      Assert.assertNotNull(role);
+      Assert.assertTrue(role.isSend());
+
+      role = configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null);
+      Assert.assertNotNull(role);
+      Assert.assertFalse(role.isCreateAddress());
    }
 
    @Test
@@ -1450,8 +1459,14 @@ public class ConfigurationImplTest extends ActiveMQTestBase {
       // verify new addition
       Assert.assertEquals(2, configuration.getSecurityRoles().size());
       Assert.assertEquals(1, configuration.getSecurityRoles().get("TEST").size());
-      Assert.assertFalse(configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null).isConsume());
+
-      Assert.assertTrue(configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null).isSend());
+      Role role = configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null);
+      Assert.assertNotNull(role);
+      Assert.assertFalse(role.isConsume());
+
+      role = configuration.getSecurityRoles().get("TEST").stream().findFirst().orElse(null);
+      Assert.assertNotNull(role);
+      Assert.assertTrue(role.isSend());
 
       // verify augmentation
       Assert.assertEquals(2, configuration.getSecurityRoles().get("#").size());

pom.xml

@@ -103,7 +103,7 @@
       <jetty.version>10.0.15</jetty.version>
       <tomcat.servlet-api.version>8.5.78</tomcat.servlet-api.version>
       <jgroups.version>5.2.0.Final</jgroups.version>
-      <errorprone.version>2.10.0</errorprone.version>
+      <errorprone.version>2.20.0</errorprone.version>
       <maven.bundle.plugin.version>5.1.9</maven.bundle.plugin.version>
       <maven.checkstyle.plugin.version>3.2.2</maven.checkstyle.plugin.version>
       <jib.maven.plugin.version>3.3.2</jib.maven.plugin.version>
@@ -1099,7 +1099,7 @@
                               <arg>--add-exports=jdk.unsupported/sun.misc=ALL-UNNAMED</arg>
                               <arg>--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED</arg>
                               <arg>-XDcompilePolicy=simple</arg>
-                              <arg>-Xplugin:ErrorProne -Xep:ThreadLocalUsage:ERROR -Xep:MissingOverride:ERROR -Xep:NonAtomicVolatileUpdate:ERROR -Xep:SynchronizeOnNonFinalField:ERROR -Xep:StaticQualifiedUsingExpression:ERROR -Xep:WaitNotInLoop:ERROR -XepExcludedPaths:.*/generated-sources/.*</arg>
+                              <arg>-Xplugin:ErrorProne -Xep:ThreadLocalUsage:ERROR -Xep:MissingOverride:ERROR -Xep:NonAtomicVolatileUpdate:ERROR -Xep:SynchronizeOnNonFinalField:ERROR -Xep:StaticQualifiedUsingExpression:ERROR -Xep:WaitNotInLoop:ERROR -Xep:BanJNDI:OFF -XepExcludedPaths:.*/generated-sources/.*</arg>
                           </compilerArgs>
                       </configuration>
                   </plugin>
@@ -1133,7 +1133,7 @@
                               <arg>--add-exports=jdk.unsupported/sun.misc=ALL-UNNAMED</arg>
                               <arg>--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED</arg>
                               <arg>-XDcompilePolicy=simple</arg>
-                              <arg>-Xplugin:ErrorProne -Xep:ThreadLocalUsage:ERROR -Xep:MissingOverride:WARN -Xep:NonAtomicVolatileUpdate:ERROR -Xep:SynchronizeOnNonFinalField:ERROR -Xep:StaticQualifiedUsingExpression:ERROR -Xep:WaitNotInLoop:ERROR -XepExcludedPaths:.*/generated-sources/.*</arg>
+                              <arg>-Xplugin:ErrorProne -Xep:ThreadLocalUsage:ERROR -Xep:MissingOverride:WARN -Xep:NonAtomicVolatileUpdate:ERROR -Xep:SynchronizeOnNonFinalField:ERROR -Xep:StaticQualifiedUsingExpression:ERROR -Xep:WaitNotInLoop:ERROR -Xep:BanJNDI:OFF -XepExcludedPaths:.*/generated-sources/.*</arg>
                               <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
                               <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
                               <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/LargeMessageCompressTest.java

@@ -685,7 +685,8 @@ public class LargeMessageCompressTest extends LargeMessageTest {
       int compressed = deflater.deflate(output);
       deflater.end();
 
-      assert compressed > min && compressed < max;
+      Assert.assertTrue(compressed > min);
+      Assert.assertTrue(compressed < max);
    }
 
    /**

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTestBase.java

@@ -33,6 +33,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 
 import org.apache.activemq.artemis.api.core.QueueConfiguration;
@@ -213,11 +214,7 @@ public abstract class StompTestBase extends ActiveMQTestBase {
 
          final String role = "testRole";
          securityManager.getConfiguration().addRole(defUser, role);
-         config.getSecurityRoles().put("#", new HashSet<Role>() {
+         config.getSecurityRoles().put("#", new HashSet<Role>(Set.of(new Role(role, true, true, true, true, true, true, true, true, true, true))));
-            {
-               add(new Role(role, true, true, true, true, true, true, true, true, true, true));
-            }
-         });
       }
 
       return activeMQServer;

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/util/JMSClusteredTestBase.java

@@ -21,6 +21,7 @@ import javax.jms.Queue;
 import javax.jms.Topic;
 import javax.management.MBeanServer;
 import java.util.ArrayList;
+import java.util.List;
 
 import org.apache.activemq.artemis.api.core.TransportConfiguration;
 import org.apache.activemq.artemis.api.jms.ActiveMQJMSClient;
@@ -162,11 +163,7 @@ public class JMSClusteredTestBase extends ActiveMQTestBase {
                                                                                                                                 .setMaxHops(MAX_HOPS)
                                                                                                                                 .setConfirmationWindowSize(1024)
                                                                                                                                 .setMessageLoadBalancingType(MessageLoadBalancingType.ON_DEMAND)
-                                                                                                                                .setStaticConnectors(new ArrayList<String>() {
+                                                                                                                                .setStaticConnectors(new ArrayList<String>(List.of(destinationLabel))));
-                                                                                                                                   {
-                                                                                                                                      add(destinationLabel);
-                                                                                                                                   }
-                                                                                                                                }));
 
       configuration.getAddressSettings().put("#", new AddressSettings().setRedistributionDelay(0));
 

tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/journal/impl/TimedBufferTest.java

@@ -361,7 +361,7 @@ public class TimedBufferTest extends ActiveMQTestBase {
    public void timeoutShouldMatchFlushIOPSWithNotBlockingFlush() {
       //use a large timeout in order to be reactive
       final long timeout = TimeUnit.MILLISECONDS.toNanos(100);
-      assert ((int) timeout) > 0;
+      assertTrue(timeout > 0);
       //it is optimistic: the timeout and the blockingDeviceFlushTime are a perfect match
       final long deviceTime = timeout;
       final int bufferSize = Env.osPageSize();
@@ -374,7 +374,7 @@ public class TimedBufferTest extends ActiveMQTestBase {
          //wait the first flush to happen
          observer.waitUntilFlushIsDone(1);
          //for a not-blocking flush I'm expecting the TimedBuffer has near to finished sleeping now
-         assert observer.flushesDone() == 1;
+         assertEquals(1, observer.flushesDone());
          //issue a new write
          timedBuffer.addBytes(LONG_ENCODER, true, DummyCallback.getInstance());
          //the countdown on the TimedBuffer is already started even before this addBytes
@@ -383,7 +383,7 @@ public class TimedBufferTest extends ActiveMQTestBase {
          observer.waitUntilFlushIsDone(2);
          final long flushDone = System.nanoTime();
          final long elapsedTime = flushDone - endOfWriteRequest;
-         assert observer.flushesDone() == 2;
+         assertEquals(2, observer.flushesDone());
          //it is much more than what is expected!!if it will fail it means that the timed IOPS = 1/(timeout + blockingDeviceFlushTime)!!!!!!
          //while it has to be IOPS = 1/timeout
          logger.debug("elapsed time: {} with timeout: {}", elapsedTime, timeout);
@@ -402,7 +402,7 @@ public class TimedBufferTest extends ActiveMQTestBase {
    public void timeoutShouldMatchFlushIOPSWithBlockingFlush() {
       //use a large timeout in order to be reactive
       final long timeout = TimeUnit.MILLISECONDS.toNanos(100);
-      assert ((int) timeout) > 0;
+      assertTrue(timeout > 0);
       //it is optimistic: the timeout and the blockingDeviceFlushTime are a perfect match
       final long deviceTime = timeout;
       final int bufferSize = Env.osPageSize();
@@ -415,7 +415,7 @@ public class TimedBufferTest extends ActiveMQTestBase {
          //wait the first flush to happen
          observer.waitUntilFlushIsDone(1);
          //for a blocking flush I'm expecting the TimedBuffer has started sleeping now
-         assert observer.flushesDone() == 1;
+         assertEquals(1, observer.flushesDone());
          //issue a new write
          timedBuffer.addBytes(LONG_ENCODER, true, DummyCallback.getInstance());
          //the countdown on the TimedBuffer is already started even before this addBytes
@@ -424,7 +424,7 @@ public class TimedBufferTest extends ActiveMQTestBase {
          observer.waitUntilFlushIsDone(2);
          final long flushDone = System.nanoTime();
          final long elapsedTime = flushDone - endOfWriteRequest;
-         assert observer.flushesDone() == 2;
+         assertEquals(2, observer.flushesDone());
          //it is much more than what is expected!!if it will fail it means that the timed IOPS = 1/(timeout + blockingDeviceFlushTime)!!!!!!
          //while it has to be IOPS = 1/timeout
          logger.debug("elapsed time: {} with timeout: {}", elapsedTime, timeout);