Skip to content

Commit

Permalink
Replace NOT operator with explicit false check (#67817)
Browse files Browse the repository at this point in the history
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 Jan 27, 2021
1 parent 37ec5cf commit e8da7e3
Show file tree
Hide file tree
Showing 119 changed files with 264 additions and 246 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class NoticeTask extends DefaultTask {
if (line.contains('*/')) {
inNotice = false

if (!isPackageInfo) {
if (isPackageInfo == false) {
break
}
} else if (inNotice) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public String toString() {
private final Property<Boolean> failIfUnavailable;
private final Configuration extracted;
private Action<ElasticsearchDistribution> distributionFinalizer;
private boolean froozen = false;
private boolean frozen = false;

ElasticsearchDistribution(
String name,
Expand Down Expand Up @@ -207,10 +207,10 @@ public String toString() {
* runs distribution finalizer logic.
*/
public ElasticsearchDistribution maybeFreeze() {
if (!froozen) {
if (frozen == false) {
finalizeValues();
distributionFinalizer.execute(this);
froozen = true;
frozen = true;
}
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private void registerInternalDistributionResolutions(NamedDomainObjectContainer<
resolutions.register("bwc", distributionResolution -> distributionResolution.setResolver((project, distribution) -> {
BwcVersions.UnreleasedVersionInfo unreleasedInfo = bwcVersions.unreleasedInfo(Version.fromString(distribution.getVersion()));
if (unreleasedInfo != null) {
if (!distribution.getBundledJdk()) {
if (distribution.getBundledJdk() == false) {
throw new GradleException(
"Configuring a snapshot bwc distribution ('"
+ distribution.getName()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void checkInvalidPermissions() throws IOException {
.map(file -> "Source file is executable: " + file)
.collect(Collectors.toList());

if (!failures.isEmpty()) {
if (failures.isEmpty() == false) {
throw new GradleException("Found invalid file permissions:\n" + String.join("\n", failures));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,7 @@ private void tweakJvmOptions(Path configFileRoot) {
String content = new String(Files.readAllBytes(jvmOptions));
Map<String, String> expansions = jvmOptionExpansions();
for (String origin : expansions.keySet()) {
if (!content.contains(origin)) {
if (content.contains(origin) == false) {
throw new IOException("template property " + origin + " not found in template.");
}
content = content.replace(origin, expansions.get(origin));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ public static void mkdirs(File dir) {
return;
}

if (dir.exists() && !dir.isDirectory()) {
if (dir.exists() && dir.isDirectory() == false) {
throw new UncheckedIOException(String.format("Cannot create directory '%s' as it already exists, but is not a directory", dir));
}

List<File> toCreate = new LinkedList<File>();
File parent = dir.getParentFile();
while (!parent.exists()) {
while (parent.exists() == false) {
toCreate.add(parent);
parent = parent.getParentFile();
}
Expand All @@ -55,7 +55,7 @@ public static void mkdirs(File dir) {
continue;
}
File parentDirToCreateParent = parentDirToCreate.getParentFile();
if (!parentDirToCreateParent.isDirectory()) {
if (parentDirToCreateParent.isDirectory() == false) {
throw new UncheckedIOException(
String.format(
"Cannot create parent directory '%s' when creating directory '%s' as '%s' is not a directory",
Expand All @@ -65,13 +65,13 @@ public static void mkdirs(File dir) {
)
);
}
if (!parentDirToCreate.mkdir() && !parentDirToCreate.isDirectory()) {
if (parentDirToCreate.mkdir() == false && parentDirToCreate.isDirectory() == false) {
throw new UncheckedIOException(
String.format("Failed to create parent directory '%s' when creating directory '%s'", parentDirToCreate, dir)
);
}
}
if (!dir.mkdir() && !dir.isDirectory()) {
if (dir.mkdir() == false && dir.isDirectory() == false) {
throw new UncheckedIOException(String.format("Failed to create directory '%s'", dir));
}
}
Expand Down
2 changes: 1 addition & 1 deletion buildSrc/src/main/resources/checkstyle_ide_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<property name="tokens" value="EXPR"/>
<property name="limitedTokens" value="LNOT"/>
<property name="maximumNumber" value="0"/>
<property name="maximumDepth" value="1"/>
<property name="maximumDepth" value="2"/>
<message
key="descendant.token.max"
value="Do not negate boolean expressions with '!', but check explicitly with '== false' as it is more explicit"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private static Map<String, List<Sample>> groupByOperation(Collection<Sample> sam
Map<String, List<Sample>> samplesPerOperation = new HashMap<>();

for (Sample sample : samples) {
if (!samplesPerOperation.containsKey(sample.getOperation())) {
if (samplesPerOperation.containsKey(sample.getOperation()) == false) {
samplesPerOperation.put(sample.getOperation(), new ArrayList<>());
}
samplesPerOperation.get(sample.getOperation()).add(sample);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ static Request deletePipeline(DeletePipelineRequest deletePipelineRequest) {

static Request simulatePipeline(SimulatePipelineRequest simulatePipelineRequest) throws IOException {
RequestConverters.EndpointBuilder builder = new RequestConverters.EndpointBuilder().addPathPartAsIs("_ingest/pipeline");
if (simulatePipelineRequest.getId() != null && !simulatePipelineRequest.getId().isEmpty()) {
if (simulatePipelineRequest.getId() != null && simulatePipelineRequest.getId().isEmpty() == false) {
builder.addPathPart(simulatePipelineRequest.getId());
}
builder.addPathPartAsIs("_simulate");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public String getFollowIndexNamePattern() {
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
if (super.equals(o) == false) return false;
Pattern pattern = (Pattern) o;
return Objects.equals(remoteCluster, pattern.remoteCluster) &&
Objects.equals(leaderIndexPatterns, pattern.leaderIndexPatterns) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
if (super.equals(o) == false) return false;
PutAutoFollowPatternRequest that = (PutAutoFollowPatternRequest) o;
return Objects.equals(name, that.name) &&
Objects.equals(remoteCluster, that.remoteCluster) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public ActiveShardCount waitForActiveShards() {
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
if (super.equals(o) == false) return false;
PutFollowRequest that = (PutFollowRequest) o;
return Objects.equals(waitForActiveShards, that.waitForActiveShards) &&
Objects.equals(remoteCluster, that.remoteCluster) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public String getFollowerIndex() {
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
if (super.equals(o) == false) return false;
ResumeFollowRequest that = (ResumeFollowRequest) o;
return Objects.equals(followerIndex, that.followerIndex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public List<TermVectorsResponse> getTermVectorsResponses() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof MultiTermVectorsResponse)) return false;
if ((obj instanceof MultiTermVectorsResponse) == false) return false;
MultiTermVectorsResponse other = (MultiTermVectorsResponse) obj;
return Objects.equals(responses, other.responses);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.XContentParser;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;

import java.util.Collections;
import java.util.List;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;

public class TermVectorsResponse {
private final String index;
private final String type;
Expand Down Expand Up @@ -142,7 +143,7 @@ public List<TermVector> getTermVectorsList(){
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof TermVectorsResponse)) return false;
if ((obj instanceof TermVectorsResponse) == false) return false;
TermVectorsResponse other = (TermVectorsResponse) obj;
return index.equals(other.index)
&& type.equals(other.type)
Expand Down Expand Up @@ -219,7 +220,7 @@ public FieldStatistics getFieldStatistics() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof TermVector)) return false;
if ((obj instanceof TermVector) == false) return false;
TermVector other = (TermVector) obj;
return fieldName.equals(other.fieldName)
&& Objects.equals(fieldStatistics, other.fieldStatistics)
Expand Down Expand Up @@ -283,7 +284,7 @@ public long getSumTotalTermFreq() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof FieldStatistics)) return false;
if ((obj instanceof FieldStatistics) == false) return false;
FieldStatistics other = (FieldStatistics) obj;
return docCount == other.docCount
&& sumDocFreq == other.sumDocFreq
Expand Down Expand Up @@ -390,7 +391,7 @@ public List<Token> getTokens() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof Term)) return false;
if ((obj instanceof Term) == false) return false;
Term other = (Term) obj;
return term.equals(other.term)
&& termFreq == other.termFreq
Expand Down Expand Up @@ -472,7 +473,7 @@ public String getPayload() {
@Override
public boolean equals(Object obj) {
if (this == obj) return true;
if (!(obj instanceof Token)) return false;
if ((obj instanceof Token) == false) return false;
Token other = (Token) obj;
return Objects.equals(startOffset, other.startOffset)
&& Objects.equals(endOffset,other.endOffset)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;

/**
* A Connection links exactly two {@link Vertex} objects. The basis of a
* A Connection links exactly two {@link Vertex} objects. The basis of a
* connection is one or more documents have been found that contain
* this pair of terms and the strength of the connection is recorded
* as a weight.
Expand Down Expand Up @@ -75,13 +75,13 @@ public double getWeight() {
}

/**
* @return the number of documents in the sampled set that contained this
* @return the number of documents in the sampled set that contained this
* pair of {@link Vertex} objects.
*/
public long getDocCount() {
return docCount;
}

@Override
public boolean equals(Object obj) {
if (this == obj)
Expand All @@ -107,7 +107,7 @@ public int hashCode() {
private static final ParseField TARGET = new ParseField("target");
private static final ParseField WEIGHT = new ParseField("weight");
private static final ParseField DOC_COUNT = new ParseField("doc_count");


void toXContent(XContentBuilder builder, Params params, ObjectIntHashMap<Vertex> vertexNumbers) throws IOException {
builder.field(SOURCE.getPreferredName(), vertexNumbers.get(from));
Expand All @@ -131,10 +131,10 @@ static class UnresolvedConnection {
this.weight = weight;
this.docCount = docCount;
}
public Connection resolve(List<Vertex> vertices) {
public Connection resolve(List<Vertex> vertices) {
return new Connection(vertices.get(fromIndex), vertices.get(toIndex), weight, docCount);
}

private static final ConstructingObjectParser<UnresolvedConnection, Void> PARSER = new ConstructingObjectParser<>(
"ConnectionParser", true,
args -> {
Expand All @@ -150,13 +150,13 @@ public Connection resolve(List<Vertex> vertices) {
PARSER.declareInt(constructorArg(), TARGET);
PARSER.declareDouble(constructorArg(), WEIGHT);
PARSER.declareLong(constructorArg(), DOC_COUNT);
}
}
static UnresolvedConnection fromXContent(XContentParser parser) throws IOException {
return PARSER.apply(parser, null);
}
}
}


/**
* An identifier (implements hashcode and equals) that represents a
* unique key for a {@link Connection}
Expand All @@ -179,9 +179,9 @@ public boolean equals(Object o) {

ConnectionId vertexId = (ConnectionId) o;

if (source != null ? !source.equals(vertexId.source) : vertexId.source != null)
if (source != null ? source.equals(vertexId.source) == false : vertexId.source != null)
return false;
if (target != null ? !target.equals(vertexId.target) : vertexId.target != null)
if (target != null ? target.equals(vertexId.target) == false : vertexId.target != null)
return false;

return true;
Expand All @@ -206,5 +206,5 @@ public VertexId getTarget() {
public String toString() {
return getSource() + "->" + getTarget();
}
}
}
}
Loading

0 comments on commit e8da7e3

Please sign in to comment.