Fix JsonPrimitive.hashCode to honor the equals/hashCode contract (#992)

[earfman]

Fixes #992.

Problem

JsonPrimitive.equals considers numbers with the same value but different Number backing types equal — e.g. new JsonPrimitive(42), a lazily-parsed "42" (the type produced by round-tripping a number through JSON), and new JsonPrimitive(42.0) are all equal to one another. But hashCode branched on isIntegral(this):

if (isIntegral(this)) {
  long value = getAsNumber().longValue();
  return (int) (value ^ (value >>> 32));
}
if (value instanceof Number) {
  long value = Double.doubleToLongBits(getAsNumber().doubleValue());
  return (int) (value ^ (value >>> 32));
}

An integral primitive (42, hashed as a long) and an equal non-integral one (lazily-parsed "42", hashed via doubleToLongBits) therefore get different hash codes, violating the Object.hashCode contract. In practice this breaks HashMap/HashSet: a value stored under new JsonPrimitive(42) can't be found using the equal primitive you get back after parsing the same number from JSON.

Fix

Derive the hash code solely from the double value that every branch of equals ultimately compares:

• Mathematically-integral values within long range are hashed as the long (this keeps the hash unchanged for the common integral types, and normalizes -0.0 to 0, which equals treats as equal to 0).
• Everything else is hashed via doubleToLongBits(doubleValue) (which canonicalizes NaN, matching equals).

equals is left untouched.

Scope / disclosure

• Some hash codes change — by design. The old hashCode used two different algorithms (integral → hash the long; otherwise → hash doubleToLongBits), which is the root cause of the bug. Now anything integral-valued and in long range hashes as the long. Concretely: an integral-valued floating primitive like 42.0 now hashes like 42; -0.0 now hashes like 0; and an integral value with magnitude > 2^53 (a Long/BigInteger a double can't represent exactly) now hashes by its double value, so 9007199254740993L matches 9007199254740992.0. In every one of those cases the new hash agrees with another primitive that equals already considers equal. Purely fractional Double/Float/BigDecimal values are unchanged. Hash codes aren't part of gson's serialized contract, but I'm flagging the change explicitly.
• Pre-existing, out of scope: equals itself is non-transitive around the 2^53 boundary (two distinct longs that both round to the same double are each "equal" to that double, but not to each other). This PR does not touch equals and does not try to fix that; it only makes hashCode consistent with equals as currently defined. Happy to file a separate issue if that's useful.

Testing

• New testEqualsAndHashCodeAcrossNumberTypes asserts equal-implies-equal-hashcode across Integer/LazilyParsedNumber/Double/BigDecimal/BigInteger, the JSON round-trip case from the issue, -0.0 vs 0, and the > 2^53 long/double case.
• Full gson module test suite passes (mvn test); Spotless clean.

_{Authored with AI assistance; the change was independently reviewed and verified before submission (including a fuzz check confirming no equals-equal pair produces differing hash codes). I'll sign the Google CLA.}

[eamonnmcmanus]

Thanks for looking into this! I agree that it is a problem that the current JsonPrimitive.hashCode() violates the contract for hashCode().

Would it be possible to structure the code of the rewritten hashCode so it is more similar to the code of equals? That way it would be easier to see that the rewritten code does indeed respect the contract. So in other words it would have basically the same conditions in the same order (isIntegral { instanceof BigInteger / getAsNumber().longValue() } etc).

[eamonnmcmanus]

I don't think this approach is going to work. Not all BigDecimal or BigInteger values will fit in a double. I suggested logic parallel to the equals logic because I think that is necessary.

For BigDecimal, I think canonicalizing via stripTrailingZeros() should be enough to ensure that two values for which equals returns true will have the same hashCode.

[earfman]

Thanks — you're right that not every BigDecimal/BigInteger fits in a double, and I think that's exactly the crux, so I want to lay out carefully why the double-based hash is forced by the current equals, and where that leaves your stripTrailingZeros() suggestion.

Why the hash must be a function of the double value (given equals as it stands): every numeric JsonPrimitive is equals() to some primitive whose comparison runs through the double path. Cross-type pairs (BigDecimal vs Double, BigInteger vs Double, etc.) skip both the integral-integral branch and the both-BigDecimal branch, and compare getAsDouble(). So, for example, on current main:

• new JsonPrimitive(new BigDecimal("42")) equals new JsonPrimitive(42) — via the double path. With stripTrailingZeros().hashCode() for the BigDecimal branch these would hash 1302 vs 42, a contract violation on the most ordinary input.
• new JsonPrimitive(new BigDecimal("0.1")) equals new JsonPrimitive(0.1) (same double), but stripTrailingZeros hashes the exact decimal 0.1, not the double — again unequal hashes for equal values.

I did check the suggestion empirically before replying: adding stripTrailingZeros().hashCode() for BigDecimal on top of this PR's structure produces 39 equal-but-different-hash ordered pairs on a ~26-value all-pairs matrix plus 300k randomized cross-type pairs (current main scores 75 on the same harness; this PR scores 0). Happy to share the harness or fold its cases into the test.

Beyond-double values can't hash distinctly under the current equals either: BigDecimal("1e400") and BigDecimal("2e400") are each equals() to JsonParser.parseString("1e400") (a LazilyParsedNumber, whose doubleValue() is +Infinity) — so by the contract they're transitively forced to share one hash even though compareTo distinguishes them. That's parser-produced, so it can't be avoided by rejecting infinite Doubles at construction. Under this PR they simply collide (legal for hashCode, and only reachable for magnitudes beyond ~1.8e308).

One behavior note I should make explicit: for the same reason, this PR changes the hashes of Long/BigInteger values above 2^53 (e.g. Long(2^53+1)). That's forced: on main, Long(2^53+1) equals Double(2^53), so they must share a hash.

The fork in the road: if you'd like values beyond double precision to hash distinctly, I believe equals itself has to be refined first (e.g. exact numeric comparison for all Number pairs) — at which point stripTrailingZeros-style hashing becomes exactly right. That would be a visible behavior change (Double(0.1) would stop equaling BigDecimal("0.1")), but it would also fix a pre-existing non-transitivity: today Long(2^53+1) ~ Double(2^53) ~ Long(2^53) while Long(2^53+1) ≠ Long(2^53). I deliberately kept equals untouched in this PR, but I'm happy to take that on here or in a follow-up if you'd prefer that direction — your call on scope.