Skip to content

Commit

Permalink
Replace NOT operator with explicit false check - part 8 (#68625)
Browse files Browse the repository at this point in the history
Part 8.

We have an in-house rule to compare explicitly against `false` instead
of using the logical not operator (`!`). However, this hasn't
historically been enforced, meaning that there are many violations in
the source at present.

We now have a Checkstyle rule that can detect these cases, but before we
can turn it on, we need to fix the existing violations. This is being
done over a series of PRs, since there are a lot to fix.
  • Loading branch information
pugnascotia committed Feb 8, 2021
1 parent 83bad6d commit 1b6ad96
Show file tree
Hide file tree
Showing 122 changed files with 223 additions and 205 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private static List<Metrics> calculateMetricsPerOperation(Map<String, List<Sampl

metrics.add(new Metrics(operationAndMetrics.getKey(),
samples.stream().filter((r) -> r.isSuccess()).count(),
samples.stream().filter((r) -> !r.isSuccess()).count(),
samples.stream().filter((r) -> r.isSuccess() == false).count(),
// throughput calculation is based on the total (Wall clock) time it took to generate all samples
calculateThroughput(samples.size(), latestEnd - firstStart),
// convert ns -> ms without losing precision
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ private static String quoteField(String field) {
}

private static boolean isNotNullOrEmpty(String arg) {
return !Strings.isNullOrEmpty(arg);
return Strings.isNullOrEmpty(arg) == false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void addListener(BiConsumer<T, ? super Exception> listener) {
if (t == null) {
listener.accept(v, null);
} else {
assert !(t instanceof Error) : "Cannot be error";
assert (t instanceof Error) == false: "Cannot be error";
listener.accept(v, (Exception) t);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public DissectParser(String pattern, String appendSeparator) {
}
this.maxMatches = matchPairs.size();
this.maxResults = Long.valueOf(matchPairs.stream()
.filter(dissectPair -> !dissectPair.getKey().skip()).map(KEY_NAME).distinct().count()).intValue();
.filter(dissectPair -> dissectPair.getKey().skip() == false).map(KEY_NAME).distinct().count()).intValue();
if (this.maxMatches == 0 || maxResults == 0) {
throw new DissectException.PatternParse(pattern, "Unable to find any keys or delimiters.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void testRightPaddingModifiers() {
public void testMultipleLeftModifiers() {
String keyName = randomAlphaOfLengthBetween(1, 10);
List<String> validModifiers = EnumSet.allOf(DissectKey.Modifier.class).stream()
.filter(m -> !m.equals(DissectKey.Modifier.NONE))
.filter(m -> m.equals(DissectKey.Modifier.NONE) == false)
.map(DissectKey.Modifier::toString)
.collect(Collectors.toList());
String modifier1 = randomFrom(validModifiers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class Geohash {
for(int i = 1; i <= PRECISION; i++) {
precisionToLatHeight[i] = precisionToLatHeight[i-1] / (even ? 8 : 4);
precisionToLonWidth[i] = precisionToLonWidth[i-1] / (even ? 4 : 8);
even = ! even;
even = even == false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public boolean enabled() {

public void configure(SSLParameters sslParameters) {
// nothing to do here
assert !sslParameters.getWantClientAuth();
assert !sslParameters.getNeedClientAuth();
assert sslParameters.getWantClientAuth() == false;
assert sslParameters.getNeedClientAuth() == false;
}
},
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,6 @@ public TokenFilter includeProperty(String name) {

@Override
protected boolean _includeScalar() {
return !inclusive;
return inclusive == false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus;

import java.io.IOException;
import java.util.Objects;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
Expand Down Expand Up @@ -199,15 +200,17 @@ public static SnapshotIndexShardStatus fromXContent(XContentParser parser, Strin

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
SnapshotIndexShardStatus that = (SnapshotIndexShardStatus) o;

if (stage != that.stage) return false;
if (stats != null ? !stats.equals(that.stats) : that.stats != null) return false;
if (nodeId != null ? !nodeId.equals(that.nodeId) : that.nodeId != null) return false;
return failure != null ? failure.equals(that.failure) : that.failure == null;
return stage == that.stage
&& Objects.equals(stats, that.stats)
&& Objects.equals(nodeId, that.nodeId)
&& Objects.equals(failure, that.failure);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
Expand Down Expand Up @@ -149,15 +150,17 @@ public static SnapshotIndexStatus fromXContent(XContentParser parser) throws IOE

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
SnapshotIndexStatus that = (SnapshotIndexStatus) o;

if (index != null ? !index.equals(that.index) : that.index != null) return false;
if (indexShards != null ? !indexShards.equals(that.indexShards) : that.indexShards != null) return false;
if (shardsStats != null ? !shardsStats.equals(that.shardsStats) : that.shardsStats != null) return false;
return stats != null ? stats.equals(that.stats) : that.stats == null;
return Objects.equals(index, that.index)
&& Objects.equals(indexShards, that.indexShards)
&& Objects.equals(shardsStats, that.shardsStats)
&& Objects.equals(stats, that.stats);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.Map;
import java.util.Objects;

/**
* Represents an alias, to be associated with an index
Expand Down Expand Up @@ -302,9 +303,7 @@ public boolean equals(Object o) {

Alias alias = (Alias) o;

if (name != null ? !name.equals(alias.name) : alias.name != null) return false;

return true;
return Objects.equals(name, alias.name);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (positionLength > 1) {
builder.field(POSITION_LENGTH, positionLength);
}
if (attributes != null && !attributes.isEmpty()) {
if (attributes != null && attributes.isEmpty() == false) {
Map<String, Object> sortedAttributes = new TreeMap<>(attributes);
for (Map.Entry<String, Object> entity : sortedAttributes.entrySet()) {
builder.field(entity.getKey(), entity.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public CommonStatsFlags clear() {
}

public boolean anySet() {
return !flags.isEmpty();
return flags.isEmpty() == false;
}

public Flag[] getFlags() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public ActionRequestValidationException validate() {
validationException = addValidationError("names is null or empty", validationException);
} else {
for (String name : names) {
if (name == null || !Strings.hasText(name)) {
if (name == null || Strings.hasText(name) == false) {
validationException = addValidationError("name is missing", validationException);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public void writeTo(StreamOutput out) throws IOException {
@Override
protected void addCustomXContentFields(XContentBuilder builder, Params params) throws IOException {
builder.field(VALID_FIELD, isValid());
if (getQueryExplanation() != null && !getQueryExplanation().isEmpty()) {
if (getQueryExplanation() != null && getQueryExplanation().isEmpty() == false) {
builder.startArray(EXPLANATIONS_FIELD);
for (QueryExplanation explanation : getQueryExplanation()) {
builder.startObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,14 @@ private void applyGlobalMandatoryParameters(DocWriteRequest<?> request) {
}

private static String valueOrDefault(String value, String globalDefault) {
if (Strings.isNullOrEmpty(value) && !Strings.isNullOrEmpty(globalDefault)) {
if (Strings.isNullOrEmpty(value) && Strings.isNullOrEmpty(globalDefault) == false) {
return globalDefault;
}
return value;
}

private static Boolean valueOrDefault(Boolean value, Boolean globalDefault) {
if (Objects.isNull(value) && !Objects.isNull(globalDefault)) {
if (Objects.isNull(value) && Objects.isNull(globalDefault) == false) {
return globalDefault;
}
return value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void onResponse(BulkResponse bulkItemResponses) {
finishHim();
} else {
if (canRetry(bulkItemResponses)) {
addResponses(bulkItemResponses, (r -> !r.isFailed()));
addResponses(bulkItemResponses, (r -> r.isFailed() == false));
retry(createBulkRequestForRetry(bulkItemResponses));
} else {
addResponses(bulkItemResponses, (r -> true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected GetResponse shardOperation(GetRequest request, ShardId shardId) {
IndexService indexService = indicesService.indexServiceSafe(shardId.getIndex());
IndexShard indexShard = indexService.getShard(shardId.id());

if (request.refresh() && !request.realtime()) {
if (request.refresh() && request.realtime() == false) {
indexShard.refresh("refresh_flag_get");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected MultiGetShardResponse shardOperation(MultiGetShardRequest request, Sha
IndexService indexService = indicesService.indexServiceSafe(shardId.getIndex());
IndexShard indexShard = indexService.getShard(shardId.id());

if (request.refresh() && !request.realtime()) {
if (request.refresh() && request.realtime() == false) {
indexShard.refresh("refresh_flag_mget");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void writeTo(StreamOutput out) throws IOException {
}

public boolean isFound() {
return !pipelines.isEmpty();
return pipelines.isEmpty() == false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public AggregatedDfs aggregateDfs(Collection<DfsSearchResult> results) {

}

assert !lEntry.fieldStatistics().containsKey(null);
assert lEntry.fieldStatistics().containsKey(null) == false;
final Object[] keys = lEntry.fieldStatistics().keys;
final Object[] values = lEntry.fieldStatistics().values;
for (int i = 0; i < keys.length; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static boolean isShardNotAvailableException(final Throwable e) {
* If a failure is already present, should this failure override it or not for read operations.
*/
public static boolean isReadOverrideException(Exception e) {
return !isShardNotAvailableException(e);
return isShardNotAvailableException(e) == false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ protected void onNodeResponse(DiscoveryNode node, int nodeIndex, NodeResponse re

protected void onNodeFailure(DiscoveryNode node, int nodeIndex, Throwable t) {
String nodeId = node.getId();
if (logger.isDebugEnabled() && !(t instanceof NodeShouldNotConnectException)) {
if (logger.isDebugEnabled() && (t instanceof NodeShouldNotConnectException) == false) {
logger.debug(new ParameterizedMessage("failed to execute [{}] on node [{}]", actionName, nodeId), t);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ protected void doStart(ClusterState clusterState) {
retry(clusterState, blockException, newState -> {
try {
ClusterBlockException newException = checkBlock(request, newState);
return (newException == null || !newException.retryable());
return (newException == null || newException.retryable() == false);
} catch (Exception e) {
// accept state as block will be rechecked by doStart() and listener.onFailure() then called
logger.trace("exception occurred during cluster block checking, accepting state", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ private void onOperation(int idx, NodeResponse nodeResponse) {
}

private void onFailure(int idx, String nodeId, Throwable t) {
if (logger.isDebugEnabled() && !(t instanceof NodeShouldNotConnectException)) {
if (logger.isDebugEnabled() && (t instanceof NodeShouldNotConnectException) == false) {
logger.debug(new ParameterizedMessage("failed to execute on node [{}]", nodeId), t);
}
responses.set(idx, new FailedNodeException(nodeId, "Failed node [" + nodeId + "]", t));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ private void onOperation(int idx, NodeTasksResponse nodeResponse) {
}

private void onFailure(int idx, String nodeId, Throwable t) {
if (logger.isDebugEnabled() && !(t instanceof NodeShouldNotConnectException)) {
if (logger.isDebugEnabled() && (t instanceof NodeShouldNotConnectException) == false) {
logger.debug(new ParameterizedMessage("failed to execute on node [{}]", nodeId), t);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ private void setFlag(Flag flag, boolean set) {
flagsEnum.add(flag);
} else if (set == false) {
flagsEnum.remove(flag);
assert (!flagsEnum.contains(flag));
assert flagsEnum.contains(flag) == false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void setFields(Fields termVectorsByField, Set<String> selectedFields, EnumSet<Fl
boolean hasScores = termVectorsFilter != null;

for (String field : termVectorsByField) {
if ((selectedFields != null) && (!selectedFields.contains(field))) {
if (selectedFields != null && selectedFields.contains(field) == false) {
continue;
}

Expand Down Expand Up @@ -85,7 +85,7 @@ void setFields(Fields termVectorsByField, Set<String> selectedFields, EnumSet<Fl
Term term = new Term(field, termBytesRef);

// with filtering we only keep the best terms
if (hasScores && !termVectorsFilter.hasScoreTerm(term)) {
if (hasScores && termVectorsFilter.hasScoreTerm(term) == false) {
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ Tuple<UpdateOpType, Map<String, Object>> executeScriptedUpsert(Map<String, Objec
* {@code IndexRequest} to be executed on the primary and replicas.
*/
Result prepareUpsert(ShardId shardId, UpdateRequest request, final GetResult getResult, LongSupplier nowInMillis) {
if (request.upsertRequest() == null && !request.docAsUpsert()) {
if (request.upsertRequest() == null && request.docAsUpsert() == false) {
throw new DocumentMissingException(shardId, request.type(), request.id());
}
IndexRequest indexRequest = request.docAsUpsert() ? request.doc() : request.upsertRequest();
Expand Down Expand Up @@ -176,7 +176,7 @@ Result prepareUpdateIndexRequest(ShardId shardId, UpdateRequest request, GetResu
final XContentType updateSourceContentType = sourceAndContent.v1();
final Map<String, Object> updatedSourceAsMap = sourceAndContent.v2();

final boolean noop = !XContentHelper.update(updatedSourceAsMap, currentRequest.sourceAsMap(), detectNoop);
final boolean noop = XContentHelper.update(updatedSourceAsMap, currentRequest.sourceAsMap(), detectNoop) == false;

// We can only actually turn the update into a noop if detectNoop is true to preserve backwards compatibility and to handle cases
// where users repopulating multi-fields or adding synonyms, etc.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public boolean isSuccess() {
}

public boolean isFailure() {
return !isSuccess();
return isSuccess() == false;
}

public String getMessage() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ static void log(final Logger logger, final String error) {
static boolean enforceLimits(final BoundTransportAddress boundTransportAddress, final String discoveryType) {
final Predicate<TransportAddress> isLoopbackAddress = t -> t.address().getAddress().isLoopbackAddress();
final boolean bound =
!(Arrays.stream(boundTransportAddress.boundAddresses()).allMatch(isLoopbackAddress) &&
isLoopbackAddress.test(boundTransportAddress.publishAddress()));
return bound && !"single-node".equals(discoveryType);
(Arrays.stream(boundTransportAddress.boundAddresses()).allMatch(isLoopbackAddress) &&
isLoopbackAddress.test(boundTransportAddress.publishAddress())) == false;
return bound && "single-node".equals(discoveryType) == false;
}

// the list of checks to execute
Expand Down Expand Up @@ -578,7 +578,7 @@ static class OnErrorCheck extends MightForkCheck {
@Override
boolean mightFork() {
final String onError = onError();
return onError != null && !onError.equals("");
return onError != null && onError.isEmpty() == false;
}

// visible for testing
Expand All @@ -603,7 +603,7 @@ static class OnOutOfMemoryErrorCheck extends MightForkCheck {
@Override
boolean mightFork() {
final String onOutOfMemoryError = onOutOfMemoryError();
return onOutOfMemoryError != null && !onOutOfMemoryError.equals("");
return onOutOfMemoryError != null && onOutOfMemoryError.isEmpty() == false;
}

// visible for testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv)
throws NodeValidationException, UserException {
try {
Bootstrap.init(!daemonize, pidFile, quiet, initialEnv);
Bootstrap.init(daemonize == false, pidFile, quiet, initialEnv);
} catch (BootstrapException | RuntimeException e) {
// format exceptions to the console in a special way
// to avoid 2MB stacktraces from guice, etc.
Expand Down
Loading

0 comments on commit 1b6ad96

Please sign in to comment.