findbugs complains about Eclipse's auto-generated code

Question

Here's the hashCode() method that Eclipse kindly generated for me:

@Override
public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + (int) (id ^ (id >>> 32));
    return result;
}

When I run findbugs on this, it complains about the last line:

Method ...hashCode() stores return result in local before immediately returning it [Scariest(2), Normal confidence]

Who is right here? Findbugs or Eclipse? Is this dodgy?

I cannot for the life of me see why this is anything for findbugs to get upset about. The code is perfectly clear; storing it in a local before returning it doesn't make it harder to read or harder to maintain; and unless the compiler is very badly written then it won't make any difference to performance either.

And yet this is categorised as Scariest!

Am I missing something?

(Clearly the code could be simplified in some respects, and it's come out that way because as far as Eclipse is concerned, there might have been other fields going into the hash function. But it's specifically the issue of storing a value and then immediately returning it that I'm asking about here, because that's what findbugs is complaining about.)

Answer 1

Who is right here? Findbugs or Eclipse? Is this dodgy?

Nighter. Code style is mostly a matter of personal taste. It's useful to have uniform code style across project or even whole world, but as long there is no official guideline or rules generally adopted by whole community there is no better or worse style.

Sometimes some rules have practical benefits. In some cases Eclipse's style is more readable (if you have multiple fields). On the other hand as mentioned by @RomanC firebug's rule have advantage of fitting contract of not using variables if not necessary.

Sometimes some inspection rules in code style software are too officious, but usually you can turn them off or reduce their importance level. Also IDE I'm using usually puts code stale warning on code it itself generates.

Note: Code-style checking tools are configurable. You may enable only those rules which would reduce costs of maintaining code for you, your team or organization.

Answer 2

The code should be simplified because it's doing unnecessary things that you have been informed by firebug. This inspection is valid.

@Override
public int hashCode() {
    return 31 + (int) (id ^ (id >>> 32));
}

Answer 3

Eclipse code is not that bad but it is unnecessarily clumsy. In such trivial cases it is often better to write it by hand. Your code is essentially equivalent to this:

@Override public int hashCode() {
    return Long.hashCode(id);
}

Answer 4

It's less about simplification and more about the possibility that you forgot something. If you store the result, then that's most likely because you want to do something with it. If not, you'd just return it.

FindBugs is warning you that you most likely meant to do something with the stored value, but probably forgot.

public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + (int) (id ^ (id >>> 32));

    // I meant to do something more here with result, but forgot...

    return result;
}