HBASE-27373 Fix new spotbugs warnings after upgrading spotbugs to 4.7…

….2 (apache#4787) (apache#4791)

Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>

hbase-client/src/main/java/org/apache/hadoop/hbase/exceptions/ClientExceptionsUtil.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.exceptions;
 
+import com.google.errorprone.annotations.RestrictedApi;
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.SyncFailedException;
@@ -140,6 +141,10 @@ public static boolean isCallDroppedException(Throwable t) {
    * For test only. Usually you should use the {@link #isConnectionException(Throwable)} method
    * below.
    */
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+      allowedOnPath = ".*/src/test/.*")
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "test only")
   public static Set<Class<? extends Throwable>> getConnectionExceptionTypes() {
     return CONNECTION_EXCEPTION_TYPES;
   }

hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java

@@ -1473,18 +1473,12 @@ public static ServerInfo getServerInfo(final RpcController controller,
     }
   }
 
-  /**
-   * @see #buildGetServerInfoRequest()
-   */
-  private static GetServerInfoRequest GET_SERVER_INFO_REQUEST =
-    GetServerInfoRequest.newBuilder().build();
-
   /**
    * Create a new GetServerInfoRequest
    * @return a GetServerInfoRequest
    */
   public static GetServerInfoRequest buildGetServerInfoRequest() {
-    return GET_SERVER_INFO_REQUEST;
+    return GetServerInfoRequest.getDefaultInstance();
   }
 
   public static ScanMetrics toScanMetrics(final byte[] bytes) {

hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java

@@ -1032,32 +1032,20 @@ public static CompactRegionRequest buildCompactRegionRequest(byte[] regionName,
     return builder.build();
   }
 
-  /**
-   * @see #buildRollWALWriterRequest()
-   */
-  private static RollWALWriterRequest ROLL_WAL_WRITER_REQUEST =
-    RollWALWriterRequest.newBuilder().build();
-
   /**
    * Create a new RollWALWriterRequest
    * @return a ReplicateWALEntryRequest
    */
   public static RollWALWriterRequest buildRollWALWriterRequest() {
-    return ROLL_WAL_WRITER_REQUEST;
+    return RollWALWriterRequest.getDefaultInstance();
   }
 
-  /**
-   * @see #buildGetServerInfoRequest()
-   */
-  private static GetServerInfoRequest GET_SERVER_INFO_REQUEST =
-    GetServerInfoRequest.newBuilder().build();
-
   /**
    * Create a new GetServerInfoRequest
    * @return a GetServerInfoRequest
    */
   public static GetServerInfoRequest buildGetServerInfoRequest() {
-    return GET_SERVER_INFO_REQUEST;
+    return GetServerInfoRequest.getDefaultInstance();
   }
 
   /**
@@ -1438,18 +1426,12 @@ public static GetClusterStatusRequest buildGetClusterStatusRequest(EnumSet<Optio
       .addAllOptions(ClusterMetricsBuilder.toOptions(options)).build();
   }
 
-  /**
-   * @see #buildCatalogScanRequest
-   */
-  private static final RunCatalogScanRequest CATALOG_SCAN_REQUEST =
-    RunCatalogScanRequest.newBuilder().build();
-
   /**
    * Creates a request for running a catalog scan
    * @return A {@link RunCatalogScanRequest}
    */
   public static RunCatalogScanRequest buildCatalogScanRequest() {
-    return CATALOG_SCAN_REQUEST;
+    return RunCatalogScanRequest.getDefaultInstance();
   }
 
   /**
@@ -1460,32 +1442,20 @@ public static EnableCatalogJanitorRequest buildEnableCatalogJanitorRequest(boole
     return EnableCatalogJanitorRequest.newBuilder().setEnable(enable).build();
   }
 
-  /**
-   * @see #buildIsCatalogJanitorEnabledRequest()
-   */
-  private static final IsCatalogJanitorEnabledRequest IS_CATALOG_JANITOR_ENABLED_REQUEST =
-    IsCatalogJanitorEnabledRequest.newBuilder().build();
-
   /**
    * Creates a request for querying the master whether the catalog janitor is enabled
    * @return A {@link IsCatalogJanitorEnabledRequest}
    */
   public static IsCatalogJanitorEnabledRequest buildIsCatalogJanitorEnabledRequest() {
-    return IS_CATALOG_JANITOR_ENABLED_REQUEST;
+    return IsCatalogJanitorEnabledRequest.getDefaultInstance();
   }
 
-  /**
-   * @see #buildRunCleanerChoreRequest()
-   */
-  private static final RunCleanerChoreRequest CLEANER_CHORE_REQUEST =
-    RunCleanerChoreRequest.newBuilder().build();
-
   /**
    * Creates a request for running cleaner chore
    * @return A {@link RunCleanerChoreRequest}
    */
   public static RunCleanerChoreRequest buildRunCleanerChoreRequest() {
-    return CLEANER_CHORE_REQUEST;
+    return RunCleanerChoreRequest.getDefaultInstance();
   }
 
   /**
@@ -1496,18 +1466,12 @@ public static SetCleanerChoreRunningRequest buildSetCleanerChoreRunningRequest(b
     return SetCleanerChoreRunningRequest.newBuilder().setOn(on).build();
   }
 
-  /**
-   * @see #buildIsCleanerChoreEnabledRequest()
-   */
-  private static final IsCleanerChoreEnabledRequest IS_CLEANER_CHORE_ENABLED_REQUEST =
-    IsCleanerChoreEnabledRequest.newBuilder().build();
-
   /**
    * Creates a request for querying the master whether the cleaner chore is enabled
    * @return A {@link IsCleanerChoreEnabledRequest}
    */
   public static IsCleanerChoreEnabledRequest buildIsCleanerChoreEnabledRequest() {
-    return IS_CLEANER_CHORE_ENABLED_REQUEST;
+    return IsCleanerChoreEnabledRequest.getDefaultInstance();
   }
 
   /**
@@ -1727,34 +1691,25 @@ public static ClearCompactionQueuesRequest buildClearCompactionQueuesRequest(Set
     return builder.build();
   }
 
-  private static final GetSpaceQuotaRegionSizesRequest GET_SPACE_QUOTA_REGION_SIZES_REQUEST =
-    GetSpaceQuotaRegionSizesRequest.newBuilder().build();
-
   /**
    * Returns a {@link GetSpaceQuotaRegionSizesRequest} object.
    */
   public static GetSpaceQuotaRegionSizesRequest buildGetSpaceQuotaRegionSizesRequest() {
-    return GET_SPACE_QUOTA_REGION_SIZES_REQUEST;
+    return GetSpaceQuotaRegionSizesRequest.getDefaultInstance();
   }
 
-  private static final GetSpaceQuotaSnapshotsRequest GET_SPACE_QUOTA_SNAPSHOTS_REQUEST =
-    GetSpaceQuotaSnapshotsRequest.newBuilder().build();
-
   /**
    * Returns a {@link GetSpaceQuotaSnapshotsRequest} object.
    */
   public static GetSpaceQuotaSnapshotsRequest buildGetSpaceQuotaSnapshotsRequest() {
-    return GET_SPACE_QUOTA_SNAPSHOTS_REQUEST;
+    return GetSpaceQuotaSnapshotsRequest.getDefaultInstance();
   }
 
-  private static final GetQuotaStatesRequest GET_QUOTA_STATES_REQUEST =
-    GetQuotaStatesRequest.newBuilder().build();
-
   /**
    * Returns a {@link GetQuotaStatesRequest} object.
    */
   public static GetQuotaStatesRequest buildGetQuotaStatesRequest() {
-    return GET_QUOTA_STATES_REQUEST;
+    return GetQuotaStatesRequest.getDefaultInstance();
   }
 
   public static DecommissionRegionServersRequest

hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/CryptoCipherProvider.java

@@ -30,6 +30,8 @@ public final class CryptoCipherProvider implements CipherProvider {
 
   private static CryptoCipherProvider instance;
 
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "singleton pattern")
   public static CryptoCipherProvider getInstance() {
     if (instance != null) {
       return instance;

hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/DefaultCipherProvider.java

@@ -30,6 +30,8 @@ public final class DefaultCipherProvider implements CipherProvider {
 
   private static DefaultCipherProvider instance;
 
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "singleton pattern")
   public static DefaultCipherProvider getInstance() {
     if (instance != null) {
       return instance;

hbase-common/src/main/java/org/apache/hadoop/hbase/security/Superusers.java

@@ -19,14 +19,14 @@
 
 import java.io.IOException;
 import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.AuthUtil;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet;
+
 /**
  * Keeps lists of superusers and super groups loaded from HBase configuration, checks if certain
  * user is regarded as superuser.
@@ -38,8 +38,8 @@ public final class Superusers {
   /** Configuration key for superusers */
   public static final String SUPERUSER_CONF_KEY = "hbase.superuser"; // Not getting a name
 
-  private static Set<String> superUsers;
-  private static Set<String> superGroups;
+  private static ImmutableSet<String> superUsers;
+  private static ImmutableSet<String> superGroups;
   private static User systemUser;
 
   private Superusers() {
@@ -53,8 +53,8 @@ private Superusers() {
    * @throws IllegalStateException if current user is null
    */
   public static void initialize(Configuration conf) throws IOException {
-    superUsers = new HashSet<>();
-    superGroups = new HashSet<>();
+    ImmutableSet.Builder<String> superUsersBuilder = ImmutableSet.builder();
+    ImmutableSet.Builder<String> superGroupsBuilder = ImmutableSet.builder();
     systemUser = User.getCurrent();
 
     if (systemUser == null) {
@@ -64,17 +64,19 @@ public static void initialize(Configuration conf) throws IOException {
 
     String currentUser = systemUser.getShortName();
     LOG.trace("Current user name is {}", currentUser);
-    superUsers.add(currentUser);
+    superUsersBuilder.add(currentUser);
 
     String[] superUserList = conf.getStrings(SUPERUSER_CONF_KEY, new String[0]);
     for (String name : superUserList) {
       if (AuthUtil.isGroupPrincipal(name)) {
         // Let's keep the '@' for distinguishing from user.
-        superGroups.add(name);
+        superGroupsBuilder.add(name);
       } else {
-        superUsers.add(name);
+        superUsersBuilder.add(name);
       }
     }
+    superUsers = superUsersBuilder.build();
+    superGroups = superGroupsBuilder.build();
   }
 
   /**
@@ -111,14 +113,20 @@ public static boolean isSuperUser(String user) {
     return superUsers.contains(user) || superGroups.contains(user);
   }
 
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "immutable")
   public static Collection<String> getSuperUsers() {
     return superUsers;
   }
 
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "immutable")
   public static Collection<String> getSuperGroups() {
     return superGroups;
   }
 
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "by design")
   public static User getSystemUser() {
     return systemUser;
   }

hbase-common/src/main/java/org/apache/hadoop/hbase/trace/SpanReceiverHost.java

@@ -59,6 +59,8 @@ public static SpanReceiverHost getInstance(Configuration conf) {
 
   }
 
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "by design")
   public static Configuration getConfiguration() {
     synchronized (SingletonHolder.INSTANCE.lock) {
       if (SingletonHolder.INSTANCE.host == null || SingletonHolder.INSTANCE.host.conf == null) {

hbase-metrics/pom.xml

@@ -76,6 +76,12 @@
       <groupId>io.dropwizard.metrics</groupId>
       <artifactId>metrics-core</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.github.stephenc.findbugs</groupId>
+      <artifactId>findbugs-annotations</artifactId>
+      <scope>compile</scope>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>

hbase-metrics/src/main/java/org/apache/hadoop/hbase/metrics/impl/FastLongHistogram.java

@@ -138,6 +138,8 @@ public void add(long value, long count) {
     /**
      * Computes the quantiles give the ratios.
      */
+    @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "FL_FLOATS_AS_LOOP_COUNTERS",
+        justification = "valid usage")
     public long[] getQuantiles(double[] quantiles) {
       if (!hasData) {
         // No data yet.
@@ -266,10 +268,6 @@ public FastLongHistogram(int numOfBins, long min, long max) {
     this.bins = new Bins(bins, numOfBins, 0.01, 0.999);
   }
 
-  private FastLongHistogram(Bins bins) {
-    this.bins = bins;
-  }
-
   /**
    * Adds a value to the histogram.
    */

hbase-protocol/pom.xml

@@ -42,6 +42,12 @@
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>com.github.stephenc.findbugs</groupId>
+      <artifactId>findbugs-annotations</artifactId>
+      <scope>compile</scope>
+      <optional>true</optional>
+    </dependency>
   </dependencies>
   <build>
     <plugins>

hbase-protocol/src/main/java/com/google/protobuf/HBaseZeroCopyByteString.java

@@ -65,6 +65,8 @@ public static ByteString wrap(final byte[] array, int offset, int length) {
    *            of a {@code LiteralByteString}.
    * @return byte[] representation
    */
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "by design")
   public static byte[] zeroCopyGetBytes(final ByteString buf) {
     if (buf instanceof LiteralByteString) {
       return ((LiteralByteString) buf).bytes;

hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java

@@ -53,6 +53,8 @@ UserGroupInformation getRealUser() {
   }
 
   /** Returns the RESTServlet singleton instance */
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "singleton pattern")
   public synchronized static RESTServlet getInstance() {
     assert (INSTANCE != null);
     return INSTANCE;
@@ -66,8 +68,10 @@ public ConnectionCache getConnectionCache() {
   /**
    * @param conf         Existing configuration to use in rest servlet
    * @param userProvider the login user provider
-   * @return the RESTServlet singleton instance n
+   * @return the RESTServlet singleton instance
    */
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "MS_EXPOSE_REP",
+      justification = "singleton pattern")
   public synchronized static RESTServlet getInstance(Configuration conf, UserProvider userProvider)
     throws IOException {
     if (INSTANCE == null) {