There should be a way to opt out of "Significant fields"

[pmogren]

"Significant fields: equals relies on foo, but hashCode does not"

Satisfying this check might be considered a sane default (though I'm not even sure I agree with that). However, it is by no means a requirement for the implementation of the two methods to be compatible with each other and the spec. There should be a way to suppress this check.

Thanks for your consideration.

[jqno]

An important design goal of EqualsVerifier is to err on the side of caution, which is why this check is there. Maybe you're right, and it should be possible to suppress. Do you have a specific use case that caused you to file this issue, or is it more of a principle thing for you?

[pmogren]

It's not just a principle thing, I often write hashCode this way. I filed the issue in response to doing so on https://github.com/commercehub-oss/clouseau/blob/master/clouseau-api/src/main/java/com/commercehub/clouseau/api/Session.java; I had to abandon EqualsVerifier for this class.

[jqno]

Causing you to abandon EqualsVerifier is obviously not a design goal :). Just to try to understand the situation better, though: is there a reason why only the sessionId can be used in hashCode(), and not the other fields that are used in equals()? You'll achieve much better spread if you include those other fields as well. Abandoning EqualsVerifier seems to imply it's a deal-breaker to add those fields to hashCode(), and I don't understand why that would be the case here.

[pmogren]

I think at some point in this code's history, that approach presented an issue. It seems viable now in this case. I'll give you some other conceptual cases:

1. Sometimes we are in the position of adding test coverage to a class in a legacy application that doesn't do it that way for whatever reasons (probably unknown). Wanting to use a test convenience like EqualsVerifier might not be sufficient justification to change the class under test, because of associated risk or testing cost. Indeed in this scenario the class doesn't have sufficient test coverage to provide confidence in making a change; that is the reason for what we are doing, and we might not be willing to change the class under test until after we have successfully added tests capturing the current behavior. (Also there are people that never want to change production code for tests' sake, but let's not debate that). I would like to be able to use EqualsVerifier in this scenario.
2. Sometimes the implementation of equals() is complex or unusual, and in comparison to a minimally correct implementation of hashCode(), an implementation that covers all of the fields can be considerably more difficult, or considerably more computationally expensive. If you are of the opinion that EqualsVerifier is not well-suited to use this scenario, I would understand.

[pmogren]

Another case:

(3.) Sometimes we have objects which have equality defined in terms of immutable identity properties and mutable value properties, and these objects get added to hash-based collections. Then either:
a. The value fields are later mutated. By implementing hashCode() using only the identity fields, the hashCode() will not change out from under the collection, which would cause it to misbehave.
b. Another version of an object in the collection (same identity, different values) is added to the collection, with the expectation that the collection will match up the two objects having the same identity and respond accordingly.

You might be able to make an argument that hash-based collections should not have been used, but I've seen enough occurrences of these idioms to not want them excluded.

Erring on the side of caution is great for defaults, but a suppression option would expand the potential applicability of EqualsVerifier.

[jqno]

Hi Paul,

Thanks for the elaborate reply, but you already had me by your first example :). I don't do a lot of legacy work these days so I didn't think of this scenario immediately. I think this is a very compelling use case, so I'll add a switch to the next version. Note that I have a couple other issues that I'm also working on, so it won't be done right away. I'll update this ticket when I release something.

[pmogren]

Thanks

[jqno]

I've added Warning.STRICT_HASHCODE, which you can suppress to get the behaviour you want. It's available in version 2.1.