[java] New Rule: UselessPureMethodCall

[zbynek]

Describe the PR

Add a new rule that verifies that results of pure methods are used. Generalizes both AvoidLosingExceptionInformation and UselessOperationOnImmutableRule, but more generic, since it's not restricted to catch blocks or immutable types.

Deprecates the following rules

• AvoidLosingExceptionInformation
• UselessOperationOnImmutable

Related issues

#5881

Ready?

• Added unit tests for fixed bug/feature
• Passing all unit tests
• Complete build ./mvnw clean verify passes (checked automatically by github actions)
• Added (in-code) documentation (if needed)

[github-actions]

Documentation Preview

Compared to main:
This changeset changes 32 violations,
introduces 860 new violations, 0 new errors and 0 new configuration errors,
removes 391 violations, 0 errors and 0 configuration errors.

Regression Tester Report

(comment created at 2025-08-05 20:30:31+00:00 for b652b17)

[adangel]

Thanks for the PR!

The idea is good, to create a more general rule for useless method calls. However, I think the usage of pure is not related here.

Have you had a look at the implementation of UselessOperationOnImmutableRule? It is basically a whitelist of some types, and it simply checks, whether a method (that doesn't return void) is called, and the result is not used.

If a pure method (pure == side-effect free) returns void, such a method in itself would be non-sense (in the strict sense - but System.out.println would fall into this category). In any case, as it doesn't return a value, it's ok not to use the return value (which doesn't exist).
If a non-pure method returns void, that would be ok. As it doesn't return anything, it's ok not to use the return value (which doesn't exist).
If a pure method returns a value, and that returned value is not used, this would be a violation of this rule.
If a non-pure method returns a value, and that returned value is not sued, this would be a violation of this rule (whether a method should have a side-effect and additionally return a value at the same time is a different topic).

So, I think, we really don't need to guess, whether a method is pure or not (which we can only guess, as we only see the method call).

If we make the rule very simple and just report any method call, that returns a value and whose value is not used, it might have a couple of false positives. I didn't try it, did you? This might be the reason, why UselessOperationOnImmutableRule is implemented with kind of a whitelist (only certain types are considered).

And lastly, if we add this rule, I think, this would overlap with UselessOperationOnImmutable... so we need to decided, how to move forward (e.g. deprecating UselessOperationOnImmutable).
But maybe not: E.g. https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/java/time/temporal/TemporalUnit.java#L179 will be detected by UselessOperationOnImmutable, but not by UselessPureMethodCall ...

[zbynek]

@adangel thanks for the thourough review. To the main point: I don't think it's reasonable to expect that code should handle all return values , the following would trigger a warning on each line

list.remove(1); // returns boolean
list.add(7); // always returns true
stringBuilder.append("hi"); // returns self

the property that ensures that you can remove the method call without changing the code's behavior is really the lack of side effects.

As I learned by checking the regressions caused by this PR, there is no guarantee that even something as simple as Class::getName would be without side effects (spring-projects/spring-framework@b72af54), so you are right, this is just guessing. But since the concept of pure methods was already used by the PrematureDeclaration rule, I thought maybe reusing that approach to generalize UselessOperationOnImmutable and AvoidLosingExceptionInformation would be viable.

Let me know if you think this is worth continuing.

(btw. ErrorProne does have a rule for this: https://errorprone.info/bugpattern/ReturnValueIgnored )

[adangel]

the property that ensures that you can remove the method call without changing the code's behavior is really the lack of side effects.

Yes, you're right.

I thought maybe reusing that approach to generalize UselessOperationOnImmutable and AvoidLosingExceptionInformation would be viable.

From the regression report, it looks good. We don't seem to have much violations for UselessOperationOnImmutable / AvoidLosingExceptionInformation - so, not so many violations have been removed (in openjdk11 and spring at least). A coarse check on the results of the checkstyle project, it also looks fine there (for every removed violation, we have a similar violation of UselessPureMethodCall).
Your new method of course finds more cases, so overall we have more new violations than removed.

Maybe we can improve the rule description of the new rule a bit. For me, I was not so familiar with the term "pure method". I guess, we should give some short explanation there e.g. with "side-effect". And also mention, that having such a code could be either useless (the fix is, to remove it) or a bug (not using the result value - the fix is, to use the result). And provide some prominent examples (like immutable classes, or the exception.getCause call, or List.size()).

Otherwise, I'm fine with this PR. Thank you for contributing!