[java] CompareObjectsWithEqualsRule: False positive with Enums

[phoenix384]

Affects PMD Version:
6.26.0
Rule:
CompareObjectsWithEquals

Description:
With #885 this rule was fixed to not show a false-positive violation when comparing enums with '=='.
However there is still a way to get a false-positive.

Code Sample demonstrating the issue:

public class EnumTest {
	enum Type {
		A, B;
	}
	
	private final Type type = Type.A;
	
	public String isTypeA(Type param) {
		return param == type ? "Yes" : "No";
	}
}

Expected outcome:
No violation -> false-positive

Running PMD through: Eclipse

[adangel]

Fixed via #2899 for PMD 7.

[uhafner]

I also get a lot of false positives for this rule when upgrading from 6.29.0 to 6.30.0 or 6.31.0:
https://github.com/jenkinsci/warnings-ng-plugin/pull/774/checks?check_run_id=1804343680

What are the plans for 7.0.0? The backlog looks quite large before we can expect a new major release.

[adangel]

What are the plans for 7.0.0? The backlog looks quite large before we can expect a new major release.

That indeed will still take a while. But we are open to fix/backport issues that are already fixed for pmd 7, if there is a demand. We also accept pull requests.

I've looked at this issue again and that's my conclusion:

• This bug here is not reproducible with 6.26.0 - if PMD is used on a compiled project. Then we correctly detect the Enum and don't report it.
• The OP mentioned, he ran PMD through Eclipse. It could be a pmd-eclipse-plugin specific issue, however: I could only reproduce the problem, if I analyse the project without compiling it.
• When the project is not compiled, we actually figure out, that it is a enum since 6.30.0 and don't report it. So, basically, this issue is fixed since 6.30.0
• I'll add a test case for this specific case (the same we use in pmd 7) - there is no specific fix for this issue - it already works.

Then I looked at your false positives:
There are 12 new violations found with "CompareObjectsWithEquals" and one new violations with "LiteralsFirstInComparisons".
The CompareObjectsWithEquals are from two different files:

• https://github.com/jenkinsci/warnings-ng-plugin/blob/master/plugin/src/main/java/io/jenkins/plugins/analysis/core/charts/SeverityPalette.java#L21-L30
• https://github.com/jenkinsci/warnings-ng-plugin/blob/master/plugin/src/main/java/io/jenkins/plugins/analysis/core/util/LocalizedSeverity.java#L20-L29 and https://github.com/jenkinsci/warnings-ng-plugin/blob/master/plugin/src/main/java/io/jenkins/plugins/analysis/core/util/LocalizedSeverity.java#L44-L53
  Both use "==" to compare a ordinary instance. Severity is a class and not a enum. So, this violation is actually correct and not a false positive. The easiest way for you is, to suppress this rule, since it appears only in 3 methods: @SuppressWarnings("PMD.CompareObjectsWithEquals"). Or you could convert Severity into a real enum (it almost looks like a enum, but there is a public constructor...)

The one new violation with "LiteralsFirstInComparisons" appears here: https://github.com/jenkinsci/warnings-ng-plugin/blob/9869a3bff1f20cfc593c2bfe01bacdc55c7dc625/plugin/src/test/java/io/jenkins/plugins/analysis/core/model/PropertyStatisticsTest.java#L94
There you could indeed write string -> KEY.equals(string) ? KEY : "tooltip" and your lambda is null-safe.

[uhafner]

Thanks for clarifying! You are right, the class actually is not an enum it is just used in the same way. It seems that I forgot about it since I haven't looked into that part of my code since a long time...