Skip to content

Commit

Permalink
Fix shadowed variables in various places - part 1 (#77555)
Browse files Browse the repository at this point in the history
Part of #19752.

Fix a number of locations where local variables or parameters are shadowing a field
that is defined in the same class.
  • Loading branch information
pugnascotia committed Sep 13, 2021
1 parent c1fcf89 commit 934691e
Show file tree
Hide file tree
Showing 43 changed files with 237 additions and 214 deletions.
11 changes: 11 additions & 0 deletions build-tools-internal/src/main/resources/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,17 @@
lines up with the directory structure. -->
<module name="PackageDeclaration" />

<!-- Checks that a local variable or a parameter does not shadow a field that is defined in the same class. -->
<!-- Disabled until existing violations are fixed -->
<!--
<module name="HiddenField">
<property name="ignoreConstructorParameter" value="true" />
<property name="ignoreSetter" value="true" />
<property name="setterCanReturnItsClass" value="true"/>
<property name="ignoreFormat" value="^(threadPool)$"/>
</module>
-->

<!-- We don't use Java's builtin serialization and we suppress all warning
about it. The flip side of that coin is that we shouldn't _try_ to use
it. We can't outright ban it with ForbiddenApis because it complain about
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public SearchBenchmarkTask(SearchRequestExecutor searchRequestExecutor, String b
}

@Override
public void setUp(SampleRecorder sampleRecorder) throws Exception {
this.sampleRecorder = sampleRecorder;
public void setUp(SampleRecorder recorder) throws Exception {
this.sampleRecorder = recorder;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ public int getBufferLimit() {
}

@Override
protected void onResponseReceived(HttpResponse response) throws HttpException, IOException {
this.response = response;
protected void onResponseReceived(HttpResponse httpResponse) throws HttpException, IOException {
this.response = httpResponse;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ public void setEntity(HttpEntity entity) {
* If you need a different content type then use
* {@link #setEntity(HttpEntity)}.
*/
public void setJsonEntity(String entity) {
setEntity(entity == null ? null : new NStringEntity(entity, ContentType.APPLICATION_JSON));
public void setJsonEntity(String body) {
setEntity(body == null ? null : new NStringEntity(body, ContentType.APPLICATION_JSON));
}

/**
Expand Down
52 changes: 26 additions & 26 deletions client/rest/src/main/java/org/elasticsearch/client/RestClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,10 @@ public Response performRequest(Request request) throws IOException {
return performRequest(nextNodes(), internalRequest, null);
}

private Response performRequest(final NodeTuple<Iterator<Node>> nodeTuple,
private Response performRequest(final NodeTuple<Iterator<Node>> tuple,
final InternalRequest request,
Exception previousException) throws IOException {
RequestContext context = request.createContextForNextAttempt(nodeTuple.nodes.next(), nodeTuple.authCache);
RequestContext context = request.createContextForNextAttempt(tuple.nodes.next(), tuple.authCache);
HttpResponse httpResponse;
try {
httpResponse = client.execute(context.requestProducer, context.asyncResponseConsumer, context.context, null).get();
Expand All @@ -282,8 +282,8 @@ private Response performRequest(final NodeTuple<Iterator<Node>> nodeTuple,
onFailure(context.node);
Exception cause = extractAndWrapCause(e);
addSuppressedException(previousException, cause);
if (nodeTuple.nodes.hasNext()) {
return performRequest(nodeTuple, request, cause);
if (tuple.nodes.hasNext()) {
return performRequest(tuple, request, cause);
}
if (cause instanceof IOException) {
throw (IOException) cause;
Expand All @@ -298,8 +298,8 @@ private Response performRequest(final NodeTuple<Iterator<Node>> nodeTuple,
return responseOrResponseException.response;
}
addSuppressedException(previousException, responseOrResponseException.responseException);
if (nodeTuple.nodes.hasNext()) {
return performRequest(nodeTuple, request, responseOrResponseException.responseException);
if (tuple.nodes.hasNext()) {
return performRequest(tuple, request, responseOrResponseException.responseException);
}
throw responseOrResponseException.responseException;
}
Expand Down Expand Up @@ -362,11 +362,11 @@ public Cancellable performRequestAsync(Request request, ResponseListener respons
}
}

private void performRequestAsync(final NodeTuple<Iterator<Node>> nodeTuple,
private void performRequestAsync(final NodeTuple<Iterator<Node>> tuple,
final InternalRequest request,
final FailureTrackingResponseListener listener) {
request.cancellable.runIfNotCancelled(() -> {
final RequestContext context = request.createContextForNextAttempt(nodeTuple.nodes.next(), nodeTuple.authCache);
final RequestContext context = request.createContextForNextAttempt(tuple.nodes.next(), tuple.authCache);
client.execute(context.requestProducer, context.asyncResponseConsumer, context.context, new FutureCallback<HttpResponse>() {
@Override
public void completed(HttpResponse httpResponse) {
Expand All @@ -375,9 +375,9 @@ public void completed(HttpResponse httpResponse) {
if (responseOrResponseException.responseException == null) {
listener.onSuccess(responseOrResponseException.response);
} else {
if (nodeTuple.nodes.hasNext()) {
if (tuple.nodes.hasNext()) {
listener.trackFailure(responseOrResponseException.responseException);
performRequestAsync(nodeTuple, request, listener);
performRequestAsync(tuple, request, listener);
} else {
listener.onDefinitiveFailure(responseOrResponseException.responseException);
}
Expand All @@ -392,9 +392,9 @@ public void failed(Exception failure) {
try {
RequestLogger.logFailedRequest(logger, request.httpRequest, context.node, failure);
onFailure(context.node);
if (nodeTuple.nodes.hasNext()) {
if (tuple.nodes.hasNext()) {
listener.trackFailure(failure);
performRequestAsync(nodeTuple, request, listener);
performRequestAsync(tuple, request, listener);
} else {
listener.onDefinitiveFailure(failure);
}
Expand All @@ -421,9 +421,9 @@ public void cancelled() {
* @throws IOException if no nodes are available
*/
private NodeTuple<Iterator<Node>> nextNodes() throws IOException {
NodeTuple<List<Node>> nodeTuple = this.nodeTuple;
Iterable<Node> hosts = selectNodes(nodeTuple, blacklist, lastNodeIndex, nodeSelector);
return new NodeTuple<>(hosts.iterator(), nodeTuple.authCache);
NodeTuple<List<Node>> tuple = this.nodeTuple;
Iterable<Node> hosts = selectNodes(tuple, blacklist, lastNodeIndex, nodeSelector);
return new NodeTuple<>(hosts.iterator(), tuple.authCache);
}

/**
Expand Down Expand Up @@ -639,17 +639,17 @@ void onSuccess(Response response) {
/**
* Tracks one last definitive failure and returns to the caller by notifying the wrapped listener
*/
void onDefinitiveFailure(Exception exception) {
trackFailure(exception);
void onDefinitiveFailure(Exception e) {
trackFailure(e);
responseListener.onFailure(this.exception);
}

/**
* Tracks an exception, which caused a retry hence we should not return yet to the caller
*/
void trackFailure(Exception exception) {
addSuppressedException(this.exception, exception);
this.exception = exception;
void trackFailure(Exception e) {
addSuppressedException(this.exception, e);
this.exception = e;
}
}

Expand Down Expand Up @@ -752,26 +752,26 @@ private class InternalRequest {
RestClient.this.warningsHandler : request.getOptions().getWarningsHandler();
}

private void setHeaders(HttpRequest httpRequest, Collection<Header> requestHeaders) {
private void setHeaders(HttpRequest req, Collection<Header> requestHeaders) {
// request headers override default headers, so we don't add default headers if they exist as request headers
final Set<String> requestNames = new HashSet<>(requestHeaders.size());
for (Header requestHeader : requestHeaders) {
httpRequest.addHeader(requestHeader);
req.addHeader(requestHeader);
requestNames.add(requestHeader.getName());
}
for (Header defaultHeader : defaultHeaders) {
if (requestNames.contains(defaultHeader.getName()) == false) {
httpRequest.addHeader(defaultHeader);
req.addHeader(defaultHeader);
}
}
if (compressionEnabled) {
httpRequest.addHeader("Accept-Encoding", "gzip");
req.addHeader("Accept-Encoding", "gzip");
}
}

private void setRequestConfig(HttpRequestBase httpRequest, RequestConfig requestConfig) {
private void setRequestConfig(HttpRequestBase requestBase, RequestConfig requestConfig) {
if (requestConfig != null) {
httpRequest.setConfig(requestConfig);
requestBase.setConfig(requestConfig);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public void testOnSuccess() {

final Response response = mockResponse();
listener.onSuccess(response);
assertSame(response, responseListener.response.get());
assertNull(responseListener.exception.get());
assertSame(response, responseListener.lastResponse.get());
assertNull(responseListener.lastException.get());
}

public void testOnFailure() {
Expand All @@ -56,20 +56,20 @@ public void testOnFailure() {
RuntimeException runtimeException = new RuntimeException("test" + i);
expectedExceptions[i] = runtimeException;
listener.trackFailure(runtimeException);
assertNull(responseListener.response.get());
assertNull(responseListener.exception.get());
assertNull(responseListener.lastResponse.get());
assertNull(responseListener.lastException.get());
}

if (randomBoolean()) {
Response response = mockResponse();
listener.onSuccess(response);
assertSame(response, responseListener.response.get());
assertNull(responseListener.exception.get());
assertSame(response, responseListener.lastResponse.get());
assertNull(responseListener.lastException.get());
} else {
RuntimeException runtimeException = new RuntimeException("definitive");
listener.onDefinitiveFailure(runtimeException);
assertNull(responseListener.response.get());
Throwable exception = responseListener.exception.get();
assertNull(responseListener.lastResponse.get());
Throwable exception = responseListener.lastException.get();
assertSame(runtimeException, exception);

int i = numIters - 1;
Expand All @@ -83,19 +83,19 @@ public void testOnFailure() {
}

private static class MockResponseListener implements ResponseListener {
private final AtomicReference<Response> response = new AtomicReference<>();
private final AtomicReference<Exception> exception = new AtomicReference<>();
private final AtomicReference<Response> lastResponse = new AtomicReference<>();
private final AtomicReference<Exception> lastException = new AtomicReference<>();

@Override
public void onSuccess(Response response) {
if (this.response.compareAndSet(null, response) == false) {
if (this.lastResponse.compareAndSet(null, response) == false) {
throw new IllegalStateException("onSuccess was called multiple times");
}
}

@Override
public void onFailure(Exception exception) {
if (this.exception.compareAndSet(null, exception) == false) {
if (this.lastException.compareAndSet(null, exception) == false) {
throw new IllegalStateException("onFailure was called multiple times");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
* {@link RestClient.FailureListener} impl that allows to track when it gets called for which host.
*/
class HostsTrackingFailureListener extends RestClient.FailureListener {
private volatile Set<HttpHost> hosts = new HashSet<>();
private volatile Set<HttpHost> httpHosts = new HashSet<>();

@Override
public void onFailure(Node node) {
hosts.add(node.getHost());
httpHosts.add(node.getHost());
}

void assertCalled(List<Node> nodes) {
Expand All @@ -49,12 +49,12 @@ void assertCalled(List<Node> nodes) {
}

void assertCalled(HttpHost... hosts) {
assertEquals(hosts.length, this.hosts.size());
assertThat(this.hosts, containsInAnyOrder(hosts));
this.hosts.clear();
assertEquals(hosts.length, this.httpHosts.size());
assertThat(this.httpHosts, containsInAnyOrder(hosts));
this.httpHosts.clear();
}

void assertNotCalled() {
assertEquals(0, hosts.size());
assertEquals(0, httpHosts.size());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ public void startHttpServer() throws Exception {
}

private HttpServer createHttpServer() throws Exception {
HttpServer httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
httpServer.start();
HttpServer mockServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
mockServer.start();
//returns a different status code depending on the path
for (int statusCode : getAllStatusCodes()) {
httpServer.createContext(pathPrefix + "/" + statusCode, new ResponseHandler(statusCode));
mockServer.createContext(pathPrefix + "/" + statusCode, new ResponseHandler(statusCode));
}
waitForCancelHandler = new WaitForCancelHandler();
httpServer.createContext(pathPrefix + "/wait", waitForCancelHandler);
return httpServer;
mockServer.createContext(pathPrefix + "/wait", waitForCancelHandler);
return mockServer;
}

private static class WaitForCancelHandler implements HttpHandler {
Expand Down Expand Up @@ -540,12 +540,12 @@ private Response bodyTest(final String method) throws Exception {
return bodyTest(restClient, method);
}

private Response bodyTest(final RestClient restClient, final String method) throws Exception {
private Response bodyTest(final RestClient client, final String method) throws Exception {
int statusCode = randomStatusCode(getRandom());
return bodyTest(restClient, method, statusCode, new Header[0]);
return bodyTest(client, method, statusCode, new Header[0]);
}

private Response bodyTest(RestClient restClient, String method, int statusCode, Header[] headers) throws Exception {
private Response bodyTest(RestClient client, String method, int statusCode, Header[] headers) throws Exception {
String requestBody = "{ \"field\": \"value\" }";
Request request = new Request(method, "/" + statusCode);
request.setJsonEntity(requestBody);
Expand All @@ -556,7 +556,7 @@ private Response bodyTest(RestClient restClient, String method, int statusCode,
request.setOptions(options);
Response esResponse;
try {
esResponse = RestClientSingleHostTests.performRequestSyncOrAsync(restClient, request);
esResponse = RestClientSingleHostTests.performRequestSyncOrAsync(client, request);
} catch(ResponseException e) {
esResponse = e.getResponse();
}
Expand Down
6 changes: 3 additions & 3 deletions libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public enum Verbosity {
}

/** The current verbosity for the terminal, defaulting to {@link Verbosity#NORMAL}. */
private Verbosity verbosity = Verbosity.NORMAL;
private Verbosity currentVerbosity = Verbosity.NORMAL;

/** The newline used when calling println. */
private final String lineSeparator;
Expand All @@ -60,7 +60,7 @@ protected Terminal(String lineSeparator) {

/** Sets the verbosity of the terminal. */
public void setVerbosity(Verbosity verbosity) {
this.verbosity = verbosity;
this.currentVerbosity = verbosity;
}

/** Reads clear text from the terminal input. See {@link Console#readLine()}. */
Expand Down Expand Up @@ -128,7 +128,7 @@ public final void errorPrintln(Verbosity verbosity, String msg) {

/** Checks if is enough {@code verbosity} level to be printed */
public final boolean isPrintable(Verbosity verbosity) {
return this.verbosity.ordinal() >= verbosity.ordinal();
return this.currentVerbosity.ordinal() >= verbosity.ordinal();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/
public class Streams {

private static final ThreadLocal<byte[]> buffer = ThreadLocal.withInitial(() -> new byte[8 * 1024]);
private static final ThreadLocal<byte[]> LOCAL_BUFFER = ThreadLocal.withInitial(() -> new byte[8 * 1024]);

private Streams() {

Expand Down Expand Up @@ -63,7 +63,7 @@ public static long copy(final InputStream in, final OutputStream out, byte[] buf
* @see #copy(InputStream, OutputStream, byte[], boolean)
*/
public static long copy(final InputStream in, final OutputStream out, boolean close) throws IOException {
return copy(in, out, buffer.get(), close);
return copy(in, out, LOCAL_BUFFER.get(), close);
}

/**
Expand All @@ -77,6 +77,6 @@ public static long copy(final InputStream in, final OutputStream out, byte[] buf
* @see #copy(InputStream, OutputStream, byte[], boolean)
*/
public static long copy(final InputStream in, final OutputStream out) throws IOException {
return copy(in, out, buffer.get(), true);
return copy(in, out, LOCAL_BUFFER.get(), true);
}
}
Loading

0 comments on commit 934691e

Please sign in to comment.