Make some classes final to avoid suppressing "this-escape" warning (#…

…101699)

* Avoid "this-escape" by making classes final

The "this-escape" compiler warning is intended to alert
developers to potential bugs in object initialization due to
subclassing. This class of bugs cannot occur when a class is
final. Here, we take cases where a class has no implementations
but generates a "this-escape" warning, and we make those
classes final rather than suppressing the compiler warning.
This makes the remaining suppressions more meaningful, since
they now indicate places where we may want to look for
initialization bugs.

In a few cases, making a class final meant changing some of its
protected fields and methods to private or default
accessibility.

Some classes with no implementations are mocked in testing.
Since making those classes final would involve non-trivial
rewrites of tests, I've left them alone.

* Spotless, remove redundant modifiers, clean up "protected" usage

* Revert a few more mocked classes

libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslConfigurationLoaderTests.java

@@ -29,9 +29,8 @@
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 
-public class SslConfigurationLoaderTests extends ESTestCase {
+public final class SslConfigurationLoaderTests extends ESTestCase {
 
-    @SuppressWarnings("this-escape")
     private final Path certRoot = getDataPath("/certs/ca1/ca.crt").getParent().getParent();
 
     private Settings settings;

libs/tdigest/src/main/java/org/elasticsearch/tdigest/Centroid.java

@@ -26,7 +26,7 @@
 /**
  * A single centroid which represents a number of data points.
  */
-public class Centroid implements Comparable<Centroid> {
+public final class Centroid implements Comparable<Centroid> {
     private static final AtomicInteger uniqueCount = new AtomicInteger(1);
 
     private double centroid = 0;
@@ -40,19 +40,16 @@ private Centroid() {
         id = uniqueCount.getAndIncrement();
     }
 
-    @SuppressWarnings("this-escape")
     public Centroid(double x) {
         this();
         start(x, 1, uniqueCount.getAndIncrement());
     }
 
-    @SuppressWarnings("this-escape")
     public Centroid(double x, long w) {
         this();
         start(x, w, uniqueCount.getAndIncrement());
     }
 
-    @SuppressWarnings("this-escape")
     public Centroid(double x, long w, int id) {
         this();
         start(x, w, id);

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java

@@ -39,7 +39,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-public class CustomMustacheFactory extends DefaultMustacheFactory {
+public final class CustomMustacheFactory extends DefaultMustacheFactory {
     static final String V7_JSON_MEDIA_TYPE_WITH_CHARSET = "application/json; charset=UTF-8";
     static final String JSON_MEDIA_TYPE_WITH_CHARSET = "application/json;charset=utf-8";
     static final String JSON_MEDIA_TYPE = "application/json";
@@ -63,7 +63,6 @@ public class CustomMustacheFactory extends DefaultMustacheFactory {
 
     private final Encoder encoder;
 
-    @SuppressWarnings("this-escape")
     public CustomMustacheFactory(String mediaType) {
         super();
         setObjectHandler(new CustomReflectionObjectHandler());

modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/RestMultiSearchTemplateActionTests.java

@@ -23,8 +23,7 @@
 import java.util.List;
 import java.util.Map;
 
-public class RestMultiSearchTemplateActionTests extends RestActionTestCase {
-    @SuppressWarnings("this-escape")
+public final class RestMultiSearchTemplateActionTests extends RestActionTestCase {
     final List<String> contentTypeHeader = Collections.singletonList(compatibleMediaType(XContentType.VND_JSON, RestApiVersion.V_7));
 
     @Before

modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/RestSearchTemplateActionTests.java

@@ -21,8 +21,7 @@
 import java.util.List;
 import java.util.Map;
 
-public class RestSearchTemplateActionTests extends RestActionTestCase {
-    @SuppressWarnings("this-escape")
+public final class RestSearchTemplateActionTests extends RestActionTestCase {
     final List<String> contentTypeHeader = Collections.singletonList(randomCompatibleMediaType(RestApiVersion.V_7));
 
     @Before

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java

@@ -26,7 +26,7 @@
 /**
  * Request to perform a search ranking evaluation.
  */
-public class RankEvalRequest extends ActionRequest implements IndicesRequest.Replaceable {
+public final class RankEvalRequest extends ActionRequest implements IndicesRequest.Replaceable {
 
     private RankEvalSpec rankingEvaluationSpec;
 
@@ -35,7 +35,6 @@ public class RankEvalRequest extends ActionRequest implements IndicesRequest.Rep
 
     private SearchType searchType = SearchType.DEFAULT;
 
-    @SuppressWarnings("this-escape")
     public RankEvalRequest(RankEvalSpec rankingEvaluationSpec, String[] indices) {
         this.rankingEvaluationSpec = Objects.requireNonNull(rankingEvaluationSpec, "ranking evaluation specification must not be null");
         indices(indices);

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/TransportRankEvalActionTests.java

@@ -30,10 +30,9 @@
 
 import static org.mockito.Mockito.mock;
 
-public class TransportRankEvalActionTests extends ESTestCase {
+public final class TransportRankEvalActionTests extends ESTestCase {
 
-    @SuppressWarnings("this-escape")
-    private Settings settings = Settings.builder()
+    private final Settings settings = Settings.builder()
         .put("path.home", createTempDir().toString())
         .put("node.name", "test-" + getTestName())
         .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())

modules/reindex/src/test/java/org/elasticsearch/reindex/RestDeleteByQueryActionTests.java

@@ -23,9 +23,8 @@
 import java.util.List;
 import java.util.Map;
 
-public class RestDeleteByQueryActionTests extends RestActionTestCase {
+public final class RestDeleteByQueryActionTests extends RestActionTestCase {
 
-    @SuppressWarnings("this-escape")
     final List<String> contentTypeHeader = Collections.singletonList(compatibleMediaType(XContentType.VND_JSON, RestApiVersion.V_7));
 
     @Before

modules/reindex/src/test/java/org/elasticsearch/reindex/RestUpdateByQueryActionTests.java

@@ -23,9 +23,8 @@
 import java.util.List;
 import java.util.Map;
 
-public class RestUpdateByQueryActionTests extends RestActionTestCase {
+public final class RestUpdateByQueryActionTests extends RestActionTestCase {
 
-    @SuppressWarnings("this-escape")
     final List<String> contentTypeHeader = Collections.singletonList(compatibleMediaType(XContentType.VND_JSON, RestApiVersion.V_7));
 
     @Before

qa/packaging/src/test/java/org/elasticsearch/packaging/util/Packages.java

@@ -310,15 +310,14 @@ public static void restartElasticsearch(Shell sh, Installation installation) thr
      * when instantiated, and advancing that cursor when the {@code clear()}
      * method is called.
      */
-    public static class JournaldWrapper {
+    public static final class JournaldWrapper {
         private Shell sh;
         private String cursor;
 
         /**
          * Create a new wrapper for Elasticsearch JournalD logs.
          * @param sh A shell with appropriate permissions.
          */
-        @SuppressWarnings("this-escape")
         public JournaldWrapper(Shell sh) {
             this.sh = sh;
             clear();

server/src/main/java/org/elasticsearch/action/NoShardAvailableActionException.java

@@ -16,7 +16,7 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 
-public class NoShardAvailableActionException extends ElasticsearchException {
+public final class NoShardAvailableActionException extends ElasticsearchException {
 
     private static final StackTraceElement[] EMPTY_STACK_TRACE = new StackTraceElement[0];
 
@@ -28,22 +28,18 @@ public static NoShardAvailableActionException forOnShardFailureWrapper(String ms
         return new NoShardAvailableActionException(null, msg, null, true);
     }
 
-    @SuppressWarnings("this-escape")
     public NoShardAvailableActionException(ShardId shardId) {
         this(shardId, null, null, false);
     }
 
-    @SuppressWarnings("this-escape")
     public NoShardAvailableActionException(ShardId shardId, String msg) {
         this(shardId, msg, null, false);
     }
 
-    @SuppressWarnings("this-escape")
     public NoShardAvailableActionException(ShardId shardId, String msg, Throwable cause) {
         this(shardId, msg, cause, false);
     }
 
-    @SuppressWarnings("this-escape")
     private NoShardAvailableActionException(ShardId shardId, String msg, Throwable cause, boolean onShardFailureWrapper) {
         super(msg, cause);
         setShard(shardId);

server/src/main/java/org/elasticsearch/action/RoutingMissingException.java

@@ -18,11 +18,10 @@
 import java.io.IOException;
 import java.util.Objects;
 
-public class RoutingMissingException extends ElasticsearchException {
+public final class RoutingMissingException extends ElasticsearchException {
 
     private final String id;
 
-    @SuppressWarnings("this-escape")
     public RoutingMissingException(String index, String id) {
         super("routing is required for [" + index + "]/[" + id + "]");
         Objects.requireNonNull(index, "index must not be null");

server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/NodesInfoRequest.java

@@ -20,7 +20,7 @@
 /**
  * A request to get node (cluster) level information.
  */
-public class NodesInfoRequest extends BaseNodesRequest<NodesInfoRequest> {
+public final class NodesInfoRequest extends BaseNodesRequest<NodesInfoRequest> {
 
     private final NodesInfoMetrics nodesInfoMetrics;
 
@@ -39,7 +39,6 @@ public NodesInfoRequest(StreamInput in) throws IOException {
      * Get information from nodes based on the nodes ids specified. If none are passed, information
      * for all nodes will be returned.
      */
-    @SuppressWarnings("this-escape")
     public NodesInfoRequest(String... nodesIds) {
         super(nodesIds);
         nodesInfoMetrics = new NodesInfoMetrics();

server/src/main/java/org/elasticsearch/action/admin/cluster/shards/ClusterSearchShardsRequest.java

@@ -20,7 +20,9 @@
 import java.io.IOException;
 import java.util.Objects;
 
-public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSearchShardsRequest> implements IndicesRequest.Replaceable {
+public final class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSearchShardsRequest>
+    implements
+        IndicesRequest.Replaceable {
 
     private String[] indices = Strings.EMPTY_ARRAY;
     @Nullable
@@ -31,7 +33,6 @@ public class ClusterSearchShardsRequest extends MasterNodeReadRequest<ClusterSea
 
     public ClusterSearchShardsRequest() {}
 
-    @SuppressWarnings("this-escape")
     public ClusterSearchShardsRequest(String... indices) {
         indices(indices);
     }

server/src/main/java/org/elasticsearch/action/admin/indices/analyze/AnalyzeAction.java

@@ -59,7 +59,7 @@ public static class Fields {
      * A request to analyze a text associated with a specific index. Allow to provide
      * the actual analyzer name to perform the analysis with.
      */
-    public static class Request extends SingleShardRequest<Request> {
+    public static final class Request extends SingleShardRequest<Request> {
 
         private String[] text;
         private String analyzer;
@@ -91,7 +91,6 @@ public Request() {}
          *
          * @param index The text to analyze
          */
-        @SuppressWarnings("this-escape")
         public Request(String index) {
             this.index(index);
         }

server/src/main/java/org/elasticsearch/action/admin/indices/close/TransportVerifyShardBeforeCloseAction.java

@@ -163,7 +163,7 @@ public void markShardCopyAsStaleIfNeeded(
         }
     }
 
-    public static class ShardRequest extends ReplicationRequest<ShardRequest> {
+    public static final class ShardRequest extends ReplicationRequest<ShardRequest> {
 
         private final ClusterBlock clusterBlock;
 
@@ -175,7 +175,6 @@ public static class ShardRequest extends ReplicationRequest<ShardRequest> {
             phase1 = in.readBoolean();
         }
 
-        @SuppressWarnings("this-escape")
         public ShardRequest(final ShardId shardId, final ClusterBlock clusterBlock, final boolean phase1, final TaskId parentTaskId) {
             super(shardId);
             this.clusterBlock = Objects.requireNonNull(clusterBlock);

server/src/main/java/org/elasticsearch/action/admin/indices/readonly/TransportVerifyShardIndexBlockAction.java

@@ -157,7 +157,7 @@ public void markShardCopyAsStaleIfNeeded(
         }
     }
 
-    public static class ShardRequest extends ReplicationRequest<ShardRequest> {
+    public static final class ShardRequest extends ReplicationRequest<ShardRequest> {
 
         private final ClusterBlock clusterBlock;
 
@@ -166,7 +166,6 @@ public static class ShardRequest extends ReplicationRequest<ShardRequest> {
             clusterBlock = new ClusterBlock(in);
         }
 
-        @SuppressWarnings("this-escape")
         public ShardRequest(final ShardId shardId, final ClusterBlock clusterBlock, final TaskId parentTaskId) {
             super(shardId);
             this.clusterBlock = Objects.requireNonNull(clusterBlock);

server/src/main/java/org/elasticsearch/action/admin/indices/stats/CommonStatsFlags.java

@@ -24,7 +24,7 @@
  * The SHARD_LEVEL flags are for stat fields that can be calculated at the shard level and then may be later aggregated at the index level
  * along with index-level flag stat fields (e.g., Mappings).
  */
-public class CommonStatsFlags implements Writeable, Cloneable {
+public final class CommonStatsFlags implements Writeable, Cloneable {
 
     public static final CommonStatsFlags ALL = new CommonStatsFlags().all();
     public static final CommonStatsFlags SHARD_LEVEL = new CommonStatsFlags().all().set(Flag.Mappings, false);
@@ -40,7 +40,6 @@ public class CommonStatsFlags implements Writeable, Cloneable {
     /**
      * @param flags flags to set. If no flags are supplied, default flags will be set.
      */
-    @SuppressWarnings("this-escape")
     public CommonStatsFlags(Flag... flags) {
         if (flags.length > 0) {
             clear();

server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/ValidateQueryRequest.java

@@ -29,7 +29,7 @@
  * <p>
  * The request requires the query to be set using {@link #query(QueryBuilder)}
  */
-public class ValidateQueryRequest extends BroadcastRequest<ValidateQueryRequest> implements ToXContentObject {
+public final class ValidateQueryRequest extends BroadcastRequest<ValidateQueryRequest> implements ToXContentObject {
 
     public static final IndicesOptions DEFAULT_INDICES_OPTIONS = IndicesOptions.fromOptions(false, false, true, false);
 
@@ -65,7 +65,6 @@ public ValidateQueryRequest(StreamInput in) throws IOException {
      * Constructs a new validate request against the provided indices. No indices provided means it will
      * run against all indices.
      */
-    @SuppressWarnings("this-escape")
     public ValidateQueryRequest(String... indices) {
         super(indices);
         indicesOptions(DEFAULT_INDICES_OPTIONS);

server/src/main/java/org/elasticsearch/action/bulk/BulkShardRequest.java

@@ -24,7 +24,10 @@
 import java.io.IOException;
 import java.util.Set;
 
-public class BulkShardRequest extends ReplicatedWriteRequest<BulkShardRequest> implements Accountable, RawIndexingDataTransportRequest {
+public final class BulkShardRequest extends ReplicatedWriteRequest<BulkShardRequest>
+    implements
+        Accountable,
+        RawIndexingDataTransportRequest {
 
     private static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(BulkShardRequest.class);
 
@@ -35,7 +38,6 @@ public BulkShardRequest(StreamInput in) throws IOException {
         items = in.readArray(i -> i.readOptionalWriteable(inpt -> new BulkItemRequest(shardId, inpt)), BulkItemRequest[]::new);
     }
 
-    @SuppressWarnings("this-escape")
     public BulkShardRequest(ShardId shardId, RefreshPolicy refreshPolicy, BulkItemRequest[] items) {
         super(shardId);
         this.items = items;

server/src/main/java/org/elasticsearch/action/search/SearchResponse.java

@@ -455,7 +455,7 @@ public String toString() {
      * and how many of them were skipped and further details in a Map of Cluster objects
      * (when doing a cross-cluster search).
      */
-    public static class Clusters implements ToXContentFragment, Writeable {
+    public static final class Clusters implements ToXContentFragment, Writeable {
 
         public static final Clusters EMPTY = new Clusters(0, 0, 0);
 
@@ -538,7 +538,6 @@ public Clusters(int total, int successful, int skipped) {
             this.clusterInfo = Collections.emptyMap();  // will never be used if created from this constructor
         }
 
-        @SuppressWarnings("this-escape")
         public Clusters(StreamInput in) throws IOException {
             this.total = in.readVInt();
             int successfulTemp = in.readVInt();

server/src/main/java/org/elasticsearch/action/support/broadcast/BroadcastShardOperationFailedException.java

@@ -20,7 +20,7 @@
  *
  *
  */
-public class BroadcastShardOperationFailedException extends ElasticsearchException implements ElasticsearchWrapperException {
+public final class BroadcastShardOperationFailedException extends ElasticsearchException implements ElasticsearchWrapperException {
 
     public BroadcastShardOperationFailedException(ShardId shardId, String msg) {
         this(shardId, msg, null);
@@ -30,7 +30,6 @@ public BroadcastShardOperationFailedException(ShardId shardId, Throwable cause)
         this(shardId, "", cause);
     }
 
-    @SuppressWarnings("this-escape")
     public BroadcastShardOperationFailedException(ShardId shardId, String msg, Throwable cause) {
         super(msg, cause);
         setShard(shardId);

server/src/main/java/org/elasticsearch/action/support/replication/ReplicationOperation.java

@@ -661,13 +661,11 @@ public interface ReplicaResponse {
 
     }
 
-    public static class RetryOnPrimaryException extends ElasticsearchException {
-        @SuppressWarnings("this-escape")
+    public static final class RetryOnPrimaryException extends ElasticsearchException {
         public RetryOnPrimaryException(ShardId shardId, String msg) {
             this(shardId, msg, null);
         }
 
-        @SuppressWarnings("this-escape")
         RetryOnPrimaryException(ShardId shardId, String msg, Throwable cause) {
             super(msg, cause);
             setShard(shardId);

server/src/main/java/org/elasticsearch/action/support/replication/TransportReplicationAction.java

@@ -605,9 +605,8 @@ protected Releasable checkReplicaLimits(final ReplicaRequest request) {
         return () -> {};
     }
 
-    public static class RetryOnReplicaException extends ElasticsearchException {
+    public static final class RetryOnReplicaException extends ElasticsearchException {
 
-        @SuppressWarnings("this-escape")
         public RetryOnReplicaException(ShardId shardId, String msg) {
             super(msg);
             setShard(shardId);