Merge pull request #16567 from nik9000/checkstyle_equals_hashcode

Fail build if we define equals but not hashCode

buildSrc/src/main/resources/checkstyle.xml

@@ -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

core/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java

@@ -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.

core/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java

@@ -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);
+    }
 }

core/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java

@@ -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();
 }

core/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java

@@ -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);
+    }
 }

core/src/main/java/org/elasticsearch/common/lucene/search/function/WeightFactorFunction.java

@@ -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;
+        }
     }
 }

core/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java

@@ -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());
+        }
     }
 }

core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java

@@ -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);
         }
 

core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java

@@ -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) {
+        ScoreFunction function = randomBoolean() ? null : new DummyScoreFunction(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;
-            }
-        };
 
         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) {
+        ScoreFunction otherFunciton = function == null ? new DummyScoreFunction(combineFunction) : null;
-            @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;
         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) {
+        ScoreFunction scoreFunction = new DummyScoreFunction(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;
-            }
-        };
         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;
+        };
+    }
 }