Fail build if we define equals but not hashCode

That is like some kind of cardinal sin or something, right?

We had two violations though they weren't super likely to be keys in a hashmap
any time soon.
This commit is contained in:
Nik Everett 2016-02-09 17:13:13 -05:00
parent 3fcca2c516
commit f76366909a
9 changed files with 98 additions and 52 deletions

View File

@ -37,6 +37,8 @@
hard to distinguish from the digit 1 (one). -->
<module name="UpperEll"/>
<module name="EqualsHashCode" />
<!-- 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

View File

@ -116,6 +116,11 @@ public class FieldValueFactorFunction extends ScoreFunction {
Objects.equals(this.modifier, fieldValueFactorFunction.modifier);
}
@Override
protected int doHashCode() {
return Objects.hash(boostFactor, field, modifier);
}
/**
* The Type class encapsulates the modification types that can be applied
* to the score/value product.

View File

@ -25,6 +25,8 @@ import org.elasticsearch.index.fielddata.AtomicFieldData;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import java.util.Objects;
/**
* Pseudo randomly generate a score for each {@link LeafScoreFunction#score}.
*/
@ -92,4 +94,9 @@ public class RandomScoreFunction extends ScoreFunction {
return this.originalSeed == randomScoreFunction.originalSeed &&
this.saltedSeed == randomScoreFunction.saltedSeed;
}
@Override
protected int doHashCode() {
return Objects.hash(originalSeed, saltedSeed);
}
}

View File

@ -66,4 +66,15 @@ public abstract class ScoreFunction {
* Indicates whether some other {@link ScoreFunction} object of the same type is "equal to" this one.
*/
protected abstract boolean doEquals(ScoreFunction other);
@Override
public final int hashCode() {
/*
* Override hashCode here and forward to an abstract method to force extensions of this class to override hashCode in the same
* way that we force them to override equals. This also prevents false positives in CheckStyle's EqualsHashCode check.
*/
return Objects.hash(scoreCombiner, doHashCode());
}
protected abstract int doHashCode();
}

View File

@ -133,4 +133,9 @@ public class ScriptScoreFunction extends ScoreFunction {
ScriptScoreFunction scriptScoreFunction = (ScriptScoreFunction) other;
return Objects.equals(this.sScript, scriptScoreFunction.sScript);
}
@Override
protected int doHashCode() {
return Objects.hash(sScript);
}
}

View File

@ -93,6 +93,11 @@ public class WeightFactorFunction extends ScoreFunction {
Objects.equals(this.scoreFunction, weightFactorFunction.scoreFunction);
}
@Override
protected int doHashCode() {
return Objects.hash(weight, scoreFunction);
}
private static class ScoreOne extends ScoreFunction {
protected ScoreOne(CombineFunction scoreCombiner) {
@ -123,5 +128,10 @@ public class WeightFactorFunction extends ScoreFunction {
protected boolean doEquals(ScoreFunction other) {
return true;
}
@Override
protected int doHashCode() {
return 0;
}
}
}

View File

@ -383,6 +383,11 @@ public abstract class DecayFunctionBuilder<DFB extends DecayFunctionBuilder> ext
return super.doEquals(other) &&
Objects.equals(this.origin, geoFieldDataScoreFunction.origin);
}
@Override
protected int doHashCode() {
return Objects.hash(super.doHashCode(), origin);
}
}
static class NumericFieldDataScoreFunction extends AbstractDistanceScoreFunction {
@ -533,5 +538,10 @@ public abstract class DecayFunctionBuilder<DFB extends DecayFunctionBuilder> ext
Objects.equals(this.func, distanceScoreFunction.func) &&
Objects.equals(this.getFieldName(), distanceScoreFunction.getFieldName());
}
@Override
protected int doHashCode() {
return Objects.hash(scale, offset, mode, func, getFieldName());
}
}
}

View File

@ -352,7 +352,7 @@ public final class PhraseSuggestionBuilder extends SuggestionBuilder<PhraseSugge
}
@Override
public final int hashCode() {
protected final int doHashCode() {
return Objects.hash(discount);
}
@ -442,7 +442,7 @@ public final class PhraseSuggestionBuilder extends SuggestionBuilder<PhraseSugge
}
@Override
public final int hashCode() {
protected final int doHashCode() {
return Objects.hash(alpha);
}
@ -489,11 +489,19 @@ public final class PhraseSuggestionBuilder extends SuggestionBuilder<PhraseSugge
if (obj == null || getClass() != obj.getClass()) {
return false;
}
@SuppressWarnings("unchecked")
SmoothingModel other = (SmoothingModel) obj;
return doEquals(other);
}
@Override
public final int hashCode() {
/*
* Override hashCode here and forward to an abstract method to force extensions of this class to override hashCode in the same
* way that we force them to override equals. This also prevents false positives in CheckStyle's EqualsHashCode check.
*/
return doHashCode();
}
public abstract SmoothingModel fromXContent(QueryParseContext parseContext) throws IOException;
public abstract WordScorerFactory buildWordScorerFactory();
@ -503,6 +511,8 @@ public final class PhraseSuggestionBuilder extends SuggestionBuilder<PhraseSugge
*/
protected abstract boolean doEquals(SmoothingModel other);
protected abstract int doHashCode();
protected abstract XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException;
}
@ -592,7 +602,7 @@ public final class PhraseSuggestionBuilder extends SuggestionBuilder<PhraseSugge
}
@Override
public final int hashCode() {
protected final int doHashCode() {
return Objects.hash(trigramLambda, bigramLambda, unigramLambda);
}

View File

@ -459,6 +459,11 @@ public class FunctionScoreTests extends ESTestCase {
protected boolean doEquals(ScoreFunction other) {
return false;
}
@Override
protected int doHashCode() {
return 0;
}
}
public void testSimpleWeightedFunction() throws IOException, ExecutionException, InterruptedException {
@ -615,21 +620,7 @@ public class FunctionScoreTests extends ESTestCase {
Float minScore = randomBoolean() ? null : 1.0f;
CombineFunction combineFunction = randomFrom(CombineFunction.values());
float maxBoost = randomBoolean() ? Float.POSITIVE_INFINITY : randomFloat();
ScoreFunction function = randomBoolean() ? null : new ScoreFunction(combineFunction) {
@Override
public LeafScoreFunction getLeafScoreFunction(LeafReaderContext ctx) throws IOException {
return null;
}
@Override
public boolean needsScores() {
return false;
}
@Override
protected boolean doEquals(ScoreFunction other) {
return other == this;
}
};
ScoreFunction function = randomBoolean() ? null : new DummyScoreFunction(combineFunction);
FunctionScoreQuery q = new FunctionScoreQuery(new TermQuery(new Term("foo", "bar")), function, minScore, combineFunction, maxBoost);
FunctionScoreQuery q1 = new FunctionScoreQuery(new TermQuery(new Term("foo", "bar")), function, minScore, combineFunction, maxBoost);
@ -640,23 +631,7 @@ public class FunctionScoreTests extends ESTestCase {
FunctionScoreQuery diffQuery = new FunctionScoreQuery(new TermQuery(new Term("foo", "baz")), function, minScore, combineFunction, maxBoost);
FunctionScoreQuery diffMinScore = new FunctionScoreQuery(q.getSubQuery(), function, minScore == null ? 1.0f : null, combineFunction, maxBoost);
ScoreFunction otherFunciton = function == null ? new ScoreFunction(combineFunction) {
@Override
public LeafScoreFunction getLeafScoreFunction(LeafReaderContext ctx) throws IOException {
return null;
}
@Override
public boolean needsScores() {
return false;
}
@Override
protected boolean doEquals(ScoreFunction other) {
return other == this;
}
} : null;
ScoreFunction otherFunciton = function == null ? new DummyScoreFunction(combineFunction) : null;
FunctionScoreQuery diffFunction = new FunctionScoreQuery(q.getSubQuery(), otherFunciton, minScore, combineFunction, maxBoost);
FunctionScoreQuery diffMaxBoost = new FunctionScoreQuery(new TermQuery(new Term("foo", "bar")), function, minScore, combineFunction, maxBoost == 1.0f ? 0.9f : 1.0f);
q1.setBoost(3.0f);
@ -685,22 +660,7 @@ public class FunctionScoreTests extends ESTestCase {
public void testFilterFunctionScoreHashCodeAndEquals() {
ScoreMode mode = randomFrom(ScoreMode.values());
CombineFunction combineFunction = randomFrom(CombineFunction.values());
ScoreFunction scoreFunction = new ScoreFunction(combineFunction) {
@Override
public LeafScoreFunction getLeafScoreFunction(LeafReaderContext ctx) throws IOException {
return null;
}
@Override
public boolean needsScores() {
return false;
}
@Override
protected boolean doEquals(ScoreFunction other) {
return other == this;
}
};
ScoreFunction scoreFunction = new DummyScoreFunction(combineFunction);
Float minScore = randomBoolean() ? null : 1.0f;
Float maxBoost = randomBoolean() ? Float.POSITIVE_INFINITY : randomFloat();
@ -742,4 +702,30 @@ public class FunctionScoreTests extends ESTestCase {
}
}
}
private static class DummyScoreFunction extends ScoreFunction {
protected DummyScoreFunction(CombineFunction scoreCombiner) {
super(scoreCombiner);
}
@Override
public LeafScoreFunction getLeafScoreFunction(LeafReaderContext ctx) throws IOException {
return null;
}
@Override
public boolean needsScores() {
return false;
}
@Override
protected boolean doEquals(ScoreFunction other) {
return other == this;
}
@Override
protected int doHashCode() {
return 0;
};
}
}