ZOOKEEPER-3364: Compile with strict options in order to check code quality

- Add extra compiler arguments in order to achieve better code quality.
- Fix some minor issues reported by javac
- Extra checks are not enabled in "contrib" module.

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: andor@apache.org

Closes #910 from eolivelli/fix/ZOOKEEPER-3364-javac-strict and squashes the following commits:

9660aff82 [Enrico Olivelli] Fix tests and fix warnings on JDK12
52895ec9d [Enrico Olivelli] ZOOKEEPER-3364 Compile with strict options in order to check code quality

pom.xml

@@ -471,6 +471,15 @@
           <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-compiler-plugin</artifactId>
           <version>3.8.0</version>
+          <configuration>
+             <compilerArgs>
+               <compilerArg>-Werror</compilerArg>
+               <compilerArg>-Xlint:deprecation</compilerArg>
+               <compilerArg>-Xlint:unchecked</compilerArg>
+               <!-- https://issues.apache.org/jira/browse/MCOMPILER-205 -->
+               <compilerArg>-Xpkginfo:always</compilerArg>
+              </compilerArgs>
+            </configuration>
         </plugin>
         <plugin>
           <groupId>org.apache.maven.plugins</groupId>

zookeeper-contrib/pom.xml

@@ -49,10 +49,20 @@
           <artifactId>spotbugs-maven-plugin</artifactId>
           <version>3.1.9</version>
           <configuration>
-            <!-- No spotbugs for contri modules -->
+            <!-- No spotbugs for contrib modules -->
             <skip>true</skip>
           </configuration>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-compiler-plugin</artifactId>
+          <configuration>
+             <!-- no additional args for contrib -->
+             <compilerArgs>
+                 <compilerArg>-Xlint:none</compilerArg>
+             </compilerArgs>
+            </configuration>
+        </plugin>
       </plugins>
     </pluginManagement>
   </build>

zookeeper-server/src/main/java/org/apache/zookeeper/metrics/impl/MetricsProviderBootstrap.java

@@ -17,6 +17,7 @@
  */
 package org.apache.zookeeper.metrics.impl;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.Properties;
 import org.apache.zookeeper.metrics.MetricsProvider;
 import org.apache.zookeeper.metrics.MetricsProviderLifeCycleException;
@@ -34,11 +35,12 @@ public abstract class MetricsProviderBootstrap {
             throws MetricsProviderLifeCycleException {
         try {
             MetricsProvider metricsProvider = (MetricsProvider) Class.forName(metricsProviderClassName,
-                    true, Thread.currentThread().getContextClassLoader()).newInstance();
+                    true, Thread.currentThread().getContextClassLoader()).getConstructor().newInstance();
             metricsProvider.configure(configuration);
             metricsProvider.start();
             return metricsProvider;
-        } catch (ClassNotFoundException | IllegalAccessException | InstantiationException error) {
+        } catch (ClassNotFoundException | IllegalAccessException
+                | InvocationTargetException | NoSuchMethodException | InstantiationException error) {
             LOG.error("Cannot boot MetricsProvider {}", metricsProviderClassName, error);
             throw new MetricsProviderLifeCycleException("Cannot boot MetricsProvider " + metricsProviderClassName,
                     error);

zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java

@@ -19,6 +19,7 @@
 package org.apache.zookeeper.server.metric;
 
 import java.math.BigDecimal;
+import java.math.RoundingMode;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicLong;
@@ -69,7 +70,7 @@ public class AvgMinMaxCounter extends Metric
         if (currentCount > 0) {
             double avgLatency = currentTotal / (double)currentCount;
             BigDecimal bg = new BigDecimal(avgLatency);
-            return bg.setScale(4, BigDecimal.ROUND_HALF_UP).doubleValue();
+            return bg.setScale(4, RoundingMode.HALF_UP).doubleValue();
         }
         return 0;
     }

zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatchManagerFactory.java

@@ -39,7 +39,8 @@ public class WatchManagerFactory {
         }
         try {
             IWatchManager watchManager =
-                    (IWatchManager) Class.forName(watchManagerName).newInstance();
+                    (IWatchManager) Class.forName(watchManagerName)
+                            .getConstructor().newInstance();
             LOG.info("Using {} as watch manager", watchManagerName);
             return watchManager;
         } catch (Exception e) {

zookeeper-server/src/test/java/org/apache/zookeeper/JUnit4ZKTestRunner.java

@@ -40,6 +40,7 @@ public class JUnit4ZKTestRunner extends BlockJUnit4ClassRunner {
         super(klass);
     }
 
+    @SuppressWarnings("unchecked")
     public static List<FrameworkMethod> computeTestMethodsForClass(final Class klass, final List<FrameworkMethod> defaultMethods) {
         List<FrameworkMethod> list = defaultMethods;
         String methodName = System.getProperty("test.method");

zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java

@@ -22,8 +22,8 @@ import org.junit.Assert;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.junit.Rule;
-import org.junit.rules.MethodRule;
-import org.junit.rules.TestWatchman;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
 import org.junit.runner.RunWith;
 import org.junit.runners.model.FrameworkMethod;
 
@@ -45,9 +45,10 @@ public class ZKTestCase {
     }
 
     @Rule
-    public MethodRule watchman = new TestWatchman() {
+    public TestWatcher watchman= new TestWatcher() {
+        
         @Override
-        public void starting(FrameworkMethod method) {
+        public void starting(Description method) {
             // By default, disable starting a JettyAdminServer in tests to avoid
             // accidentally attempting to start multiple admin servers on the
             // same port.
@@ -55,22 +56,22 @@ public class ZKTestCase {
             // ZOOKEEPER-2693 disables all 4lw by default.
             // Here we enable the 4lw which ZooKeeper tests depends.
             System.setProperty("zookeeper.4lw.commands.whitelist", "*");
-            testName = method.getName();
+            testName = method.getMethodName();
             LOG.info("STARTING " + testName);
         }
 
         @Override
-        public void finished(FrameworkMethod method) {
+        public void finished(Description method) {
             LOG.info("FINISHED " + testName);
         }
 
         @Override
-        public void succeeded(FrameworkMethod method) {
+        public void succeeded(Description method) {
             LOG.info("SUCCEEDED " + testName);
         }
 
         @Override
-        public void failed(Throwable e, FrameworkMethod method) {
+        public void failed(Throwable e, Description method) {
             LOG.info("FAILED " + testName, e);
         }
 

zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java

@@ -871,7 +871,7 @@ public class QuorumPeerMainTest extends QuorumPeerTestBase {
 
         // make sure it has a chance to write it to disk
         int sleepTime = 0;
-        Long longLeader = new Long(leader);
+        Long longLeader = Long.valueOf(leader);
         while (!p.qvAcksetPairs.get(0).getAckset().contains(longLeader)) {
             if (sleepTime > 2000) {
                 Assert.fail("Transaction not synced to disk within 1 second " + p.qvAcksetPairs.get(0).getAckset()

zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/MiniKdc.java

@@ -17,7 +17,6 @@
  */
 
 package org.apache.zookeeper.server.quorum.auth;
-import org.apache.commons.io.Charsets;
 import org.apache.kerby.kerberos.kerb.KrbException;
 import org.apache.kerby.kerberos.kerb.server.KdcConfigKey;
 import org.apache.kerby.kerberos.kerb.server.SimpleKdcServer;
@@ -32,6 +31,7 @@ import java.io.FileInputStream;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Locale;
@@ -103,7 +103,7 @@ public class MiniKdc {
         Properties userConf = new Properties();
         InputStreamReader r = null;
         try {
-            r = new InputStreamReader(new FileInputStream(file), Charsets.UTF_8);
+            r = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8);
             userConf.load(r);
         } finally {
             if (r != null) {

zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java

@@ -34,11 +34,10 @@ import org.apache.zookeeper.test.ClientBase;
 import org.apache.zookeeper.test.ClientBase.CountdownWatcher;
 import org.junit.After;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import junit.framework.Assert;
-
 public class QuorumKerberosHostBasedAuthTest extends KerberosSecurityTestcase {
     private static File keytabFile;
     private static String hostServerPrincipal = KerberosTestUtils.getHostServerPrincipal();

zookeeper-server/src/test/java/org/apache/zookeeper/server/watch/WatcherOrBitSetTest.java

@@ -49,7 +49,7 @@ public class WatcherOrBitSetTest extends ZKTestCase {
         WatcherOrBitSet bitSet = new WatcherOrBitSet(bset);
         Assert.assertEquals(0, bitSet.size());
 
-        Integer bit = new Integer(1);
+        Integer bit = 1;
         Assert.assertFalse(bitSet.contains(1));
         Assert.assertFalse(bitSet.contains(bit));
 

zookeeper-server/src/test/java/org/apache/zookeeper/test/CnxManagerTest.java

@@ -245,10 +245,10 @@ public class CnxManagerTest extends ZKTestCase {
         SocketChannel sc = SocketChannel.open();
         sc.socket().connect(peers.get(1L).electionAddr, 5000);
 
-        InetSocketAddress otherAddr = peers.get(new Long(2)).electionAddr;
+        InetSocketAddress otherAddr = peers.get(Long.valueOf(2)).electionAddr;
         DataOutputStream dout = new DataOutputStream(sc.socket().getOutputStream());
         dout.writeLong(QuorumCnxManager.PROTOCOL_VERSION);
-        dout.writeLong(new Long(2));
+        dout.writeLong(2);
         String addr = otherAddr.getHostString()+ ":" + otherAddr.getPort();
         byte[] addr_bytes = addr.getBytes();
         dout.writeInt(addr_bytes.length);

zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java

@@ -44,7 +44,7 @@ public class FourLetterWordsTest extends ClientBase {
         LoggerFactory.getLogger(FourLetterWordsTest.class);
 
     @Rule
-    public Timeout timeout = new Timeout(30000);
+    public Timeout timeout = Timeout.millis(30000);
 
     /** Test the various four letter words */
     @Test

zookeeper-server/src/test/java/org/apache/zookeeper/test/InvalidSnapshotTest.java

@@ -25,7 +25,6 @@ import java.io.File;
 import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.server.LogFormatter;
 import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.apache.zookeeper.server.SnapshotFormatter;
 import org.apache.zookeeper.server.SyncRequestProcessor;
@@ -46,12 +45,13 @@ public class InvalidSnapshotTest extends ZKTestCase{
     /**
      * Verify the LogFormatter by running it on a known file.
      */
+    @SuppressWarnings("deprecation")
     @Test
     public void testLogFormatter() throws Exception {
         File snapDir = new File(testData, "invalidsnap");
         File logfile = new File(new File(snapDir, "version-2"), "log.274");
         String[] args = {logfile.getCanonicalFile().toString()};
-        LogFormatter.main(args);
+        org.apache.zookeeper.server.LogFormatter.main(args);
     }
 
     /**

zookeeper-server/src/test/java/org/apache/zookeeper/test/LoadFromLogTest.java

@@ -179,7 +179,7 @@ public class LoadFromLogTest extends ClientBase {
         String[] tokens = lastPath.split("-");
         String expectedPath = "/invalidsnap/test-"
                 + String.format("%010d",
-                (new Integer(tokens[1])).intValue() + 1);
+                (Integer.parseInt(tokens[1])) + 1);
         ZooKeeperServer zks = getServer(serverFactory);
         long eZxid = zks.getZKDatabase().getDataTreeLastProcessedZxid();
         // force the zxid to be behind the content

zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java

@@ -81,7 +81,7 @@ public class ReconfigMisconfigTest extends ZKTestCase {
             reconfigPort();
             Assert.fail(errorMsg);
         } catch (KeeperException e) {
-            Assert.assertTrue(e.getCode() == KeeperException.Code.NoAuth);
+            Assert.assertTrue(e.code() == KeeperException.Code.NOAUTH);
         }
 
         try {
@@ -89,7 +89,7 @@ public class ReconfigMisconfigTest extends ZKTestCase {
             reconfigPort();
             Assert.fail(errorMsg);
         } catch (KeeperException e) {
-            Assert.assertTrue(e.getCode() == KeeperException.Code.NoAuth);
+            Assert.assertTrue(e.code() == KeeperException.Code.NOAUTH);
         }
     }
 

zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java

@@ -919,7 +919,7 @@ public class ReconfigTest extends ZKTestCase implements DataCallback{
         int leavingIndex = 1;
         int replica2 = 2;
         QuorumPeer peer2 = qu.getPeer(replica2).peer;
-        QuorumServer leavingQS2 = peer2.getView().get(new Long(leavingIndex));
+        QuorumServer leavingQS2 = peer2.getView().get(Long.valueOf(leavingIndex));
         String remotePeerBean2 = MBeanRegistry.DOMAIN
                 + ":name0=ReplicatedServer_id" + replica2 + ",name1=replica."
                 + leavingIndex;
@@ -928,7 +928,7 @@ public class ReconfigTest extends ZKTestCase implements DataCallback{
         // assert remotePeerBean.1 of ReplicatedServer_3
         int replica3 = 3;
         QuorumPeer peer3 = qu.getPeer(replica3).peer;
-        QuorumServer leavingQS3 = peer3.getView().get(new Long(leavingIndex));
+        QuorumServer leavingQS3 = peer3.getView().get(Long.valueOf(leavingIndex));
         String remotePeerBean3 = MBeanRegistry.DOMAIN
                 + ":name0=ReplicatedServer_id" + replica3 + ",name1=replica."
                 + leavingIndex;
@@ -969,11 +969,11 @@ public class ReconfigTest extends ZKTestCase implements DataCallback{
         assertLocalPeerMXBeanAttributes(removedPeer, localPeerBean, true);
 
         // assert remotePeerBean.1 of ReplicatedServer_2
-        leavingQS2 = peer2.getView().get(new Long(leavingIndex));
+        leavingQS2 = peer2.getView().get(Long.valueOf(leavingIndex));
         assertRemotePeerMXBeanAttributes(leavingQS2, remotePeerBean2);
 
         // assert remotePeerBean.1 of ReplicatedServer_3
-        leavingQS3 = peer3.getView().get(new Long(leavingIndex));
+        leavingQS3 = peer3.getView().get(Long.valueOf(leavingIndex));
         assertRemotePeerMXBeanAttributes(leavingQS3, remotePeerBean3);
     }
 
@@ -997,7 +997,7 @@ public class ReconfigTest extends ZKTestCase implements DataCallback{
         int changingIndex = 1;
         int replica2 = 2;
         QuorumPeer peer2 = qu.getPeer(replica2).peer;
-        QuorumServer changingQS2 = peer2.getView().get(new Long(changingIndex));
+        QuorumServer changingQS2 = peer2.getView().get(Long.valueOf(changingIndex));
         String remotePeerBean2 = MBeanRegistry.DOMAIN
                 + ":name0=ReplicatedServer_id" + replica2 + ",name1=replica."
                 + changingIndex;
@@ -1006,7 +1006,7 @@ public class ReconfigTest extends ZKTestCase implements DataCallback{
         // assert remotePeerBean.1 of ReplicatedServer_3
         int replica3 = 3;
         QuorumPeer peer3 = qu.getPeer(replica3).peer;
-        QuorumServer changingQS3 = peer3.getView().get(new Long(changingIndex));
+        QuorumServer changingQS3 = peer3.getView().get(Long.valueOf(changingIndex));
         String remotePeerBean3 = MBeanRegistry.DOMAIN
                 + ":name0=ReplicatedServer_id" + replica3 + ",name1=replica."
                 + changingIndex;
@@ -1042,11 +1042,11 @@ public class ReconfigTest extends ZKTestCase implements DataCallback{
         assertLocalPeerMXBeanAttributes(qp, localPeerBeanName, true);
 
         // assert remotePeerBean.1 of ReplicatedServer_2
-        changingQS2 = peer2.getView().get(new Long(changingIndex));
+        changingQS2 = peer2.getView().get(Long.valueOf(changingIndex));
         assertRemotePeerMXBeanAttributes(changingQS2, remotePeerBean2);
 
         // assert remotePeerBean.1 of ReplicatedServer_3
-        changingQS3 = peer3.getView().get(new Long(changingIndex));
+        changingQS3 = peer3.getView().get(Long.valueOf(changingIndex));
         assertRemotePeerMXBeanAttributes(changingQS3, remotePeerBean3);
     }
 

zookeeper-server/src/test/java/org/apache/zookeeper/test/SyncCallTest.java

@@ -107,12 +107,14 @@ public class SyncCallTest extends ClientBase
     }
 
     @SuppressWarnings("unchecked")
+    @Override
     public void processResult(int rc, String path, Object ctx){
         ((List<Integer>) ctx).add(rc);    
         opsCount.countDown();
     
     }
 
+    @SuppressWarnings("unchecked")
     @Override
     public void processResult(int rc, String path, Object ctx, String name,
         Stat stat) {