TRUE/FALSE_RETURNS at odds with NAB_NEEDLESS_BOOLEAN_CONSTANT_CONVERSION from fb-contrib

[davidburstrom]

With Pitest 1.7.2:

Given

Boolean getValue() {
  return Boolean.TRUE;
}

the TRUE_RETURNS mutator will produce the mutation

Boolean getValue() {
  return true;
}

which will always survive as the true will be auto-boxed to a Boolean instance (unless we're testing Boolean instance identity, which seems pointless). The problem is that changing the production code to match the mutation is at odds with the NAB_NEEDLESS_BOOLEAN_CONSTANT_CONVERSION bug type from fb-contrib (See http://fb-contrib.sourceforge.net/bugdescriptions.html).

I might be missing some edge case here, but I think that the TRUE_RETURNS mutator shouldn't try to return true in lieu of Boolean.TRUE, due to the auto-boxing. I'd be happy to understand if it is indeed a relevant case.

The corresponding argument can be made for FALSE_RETURNS, of course.

[hcoles]

It is highly unlikely that the autoboxing in return true will have any measurable impact on performance/object allocation etc, so personally I would always use this more readable version, and not enable static analysis rules such as this.

Having said that, codebases and opinions differ, and there may be scenarios where the boxing does make a difference.

I'll take a look out at filtering mutatants of this form in the next release.

[davidburstrom]

@mebigfatguy My apologies for roping you into this, but maybe you have some input on the performance benefits of the linter in question?

[hcoles]

Released in 1.7.3

[davidburstrom]

Thank you very much for addressing it so fast :)

[hcoles]

This repo has 2 response times: 3 years or 3 days.

[davidburstrom]

I think you decreased the average response time with this one :)