Merge pull request #2959 from recdevs:feat/apex-equals-hashcode
[apex] New Rule: override equals and hashcode rule #2959
This commit is contained in:
commit
e114910048
7 files changed
+318
-13
No files matched your search
@@ -14,6 +14,12 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
### New and noteworthy
|
||||
|
||||
#### New Rules
|
||||
|
||||
* The new Apex rule {% rule "apex/errorprone/OverrideBothEqualsAndHashcode" %} brings the well known Java rule
|
||||
to Apex. In Apex the same principle applies: `equals` and `hashCode` should always be overridden
|
||||
together to ensure collection classes such as Maps and Sets work as expected.
|
||||
|
||||
### Fixed Issues
|
||||
|
||||
* core
|
||||
@@ -28,6 +34,7 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
### External Contributions
|
||||
|
||||
* [#2959](https://github.com/pmd/pmd/pull/2959): \[apex] New Rule: override equals and hashcode rule - [recdevs](https://github.com/recdevs)
|
||||
* [#2964](https://github.com/pmd/pmd/pull/2964): \[cs] Update C# grammar for additional C# 7 and C# 8 features - [Maikel Steneker](https://github.com/maikelsteneker)
|
||||
* [#2983](https://github.com/pmd/pmd/pull/2983): \[java] LiteralsFirstInComparisons should consider constant fields - [Ozan Gulle](https://github.com/ozangulle)
|
||||
* [#2994](https://github.com/pmd/pmd/pull/2994): \[core] Fix code climate severity strings - [Vincent Maurin](https://github.com/vmaurin)
|
||||
|
||||
+68
@@ -0,0 +1,68 @@
|
||||
/*
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.apex.rule.errorprone;
|
||||
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTParameter;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
|
||||
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
|
||||
|
||||
public class OverrideBothEqualsAndHashcodeRule extends AbstractApexRule {
|
||||
|
||||
public OverrideBothEqualsAndHashcodeRule() {
|
||||
addRuleChainVisit(ASTUserClass.class);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTUserClass node, Object data) {
|
||||
ApexNode<?> equalsNode = null;
|
||||
ApexNode<?> hashNode = null;
|
||||
for (ASTMethod method : node.findChildrenOfType(ASTMethod.class)) {
|
||||
if (equalsNode == null && isEquals(method)) {
|
||||
equalsNode = method;
|
||||
}
|
||||
if (hashNode == null && isHashCode(method)) {
|
||||
hashNode = method;
|
||||
}
|
||||
if (hashNode != null && equalsNode != null) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (equalsNode != null && hashNode == null) {
|
||||
addViolation(data, equalsNode);
|
||||
} else if (hashNode != null && equalsNode == null) {
|
||||
addViolation(data, hashNode);
|
||||
}
|
||||
|
||||
return data;
|
||||
}
|
||||
|
||||
private boolean isEquals(ASTMethod node) {
|
||||
int numParams = 0;
|
||||
String paramType = null;
|
||||
for (int ix = 0; ix < node.getNumChildren(); ix++) {
|
||||
ApexNode<?> sn = node.getChild(ix);
|
||||
if (sn instanceof ASTParameter) {
|
||||
numParams++;
|
||||
paramType = ((ASTParameter) sn).getType();
|
||||
}
|
||||
}
|
||||
return numParams == 1 && "equals".equalsIgnoreCase(node.getImage()) && "Object".equalsIgnoreCase(paramType);
|
||||
}
|
||||
|
||||
private boolean isHashCode(ASTMethod node) {
|
||||
int numParams = 0;
|
||||
for (int ix = 0; ix < node.getNumChildren(); ix++) {
|
||||
ApexNode<?> sn = node.getChild(ix);
|
||||
if (sn instanceof ASTParameter) {
|
||||
numParams++;
|
||||
}
|
||||
}
|
||||
|
||||
return numParams == 0 && "hashCode".equalsIgnoreCase(node.getImage());
|
||||
}
|
||||
}
|
||||
@@ -111,6 +111,30 @@ public without sharing class Foo {
|
||||
</example>
|
||||
</rule>
|
||||
|
||||
<rule name="AvoidNonExistentAnnotations"
|
||||
language="apex"
|
||||
since="6.5.0"
|
||||
message="Use of non existent annotations will lead to broken Apex code which will not compile in the future."
|
||||
class="net.sourceforge.pmd.lang.apex.rule.errorprone.AvoidNonExistentAnnotationsRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_errorprone.html#avoidnonexistentannotations">
|
||||
<description>
|
||||
Apex supported non existent annotations for legacy reasons.
|
||||
In the future, use of such non-existent annotations could result in broken apex code that will not compile.
|
||||
This will prevent users of garbage annotations from being able to use legitimate annotations added to Apex in the future.
|
||||
A full list of supported annotations can be found at https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_annotation.htm
|
||||
</description>
|
||||
<priority>3</priority>
|
||||
<example>
|
||||
<![CDATA[
|
||||
@NonExistentAnnotation public class ClassWithNonexistentAnnotation {
|
||||
@NonExistentAnnotation public void methodWithNonExistentAnnotation() {
|
||||
// ...
|
||||
}
|
||||
}
|
||||
]]>
|
||||
</example>
|
||||
</rule>
|
||||
|
||||
<rule name="EmptyCatchBlock"
|
||||
language="apex"
|
||||
since="6.0.0"
|
||||
@@ -317,24 +341,39 @@ public class MyClass {
|
||||
</example>
|
||||
</rule>
|
||||
|
||||
<rule name="AvoidNonExistentAnnotations"
|
||||
<rule name="OverrideBothEqualsAndHashcode"
|
||||
language="apex"
|
||||
since="6.5.0"
|
||||
message="Use of non existent annotations will lead to broken Apex code which will not compile in the future."
|
||||
class="net.sourceforge.pmd.lang.apex.rule.errorprone.AvoidNonExistentAnnotationsRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_errorprone.html#avoidnonexistentannotations">
|
||||
since="6.31.0"
|
||||
message="Ensure you override both equals() and hashCode()"
|
||||
class="net.sourceforge.pmd.lang.apex.rule.errorprone.OverrideBothEqualsAndHashcodeRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_errorprone.html#overridebothequalsandhashcode">
|
||||
<description>
|
||||
Apex supported non existent annotations for legacy reasons.
|
||||
In the future, use of such non-existent annotations could result in broken apex code that will not compile.
|
||||
This will prevent users of garbage annotations from being able to use legitimate annotations added to Apex in the future.
|
||||
A full list of supported annotations can be found at https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_annotation.htm
|
||||
Override both `public Boolean equals(Object obj)`, and `public Integer hashCode()`, or override neither.
|
||||
Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly
|
||||
delegating to your superclass.
|
||||
|
||||
This is especially important when [Using Custom Types in Map Keys and Sets](https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/langCon_apex_collections_maps_keys_userdefined.htm).
|
||||
</description>
|
||||
<priority>3</priority>
|
||||
<example>
|
||||
<![CDATA[@NonExistentAnnotation public class ClassWithNonexistentAnnotation {
|
||||
@NonExistentAnnotation public void methodWithNonExistentAnnotation() {
|
||||
// ...
|
||||
}
|
||||
<![CDATA[
|
||||
public class Bar { // poor, missing a hashCode() method
|
||||
public Boolean equals(Object o) {
|
||||
// do some comparison
|
||||
}
|
||||
}
|
||||
public class Baz { // poor, missing an equals() method
|
||||
public Integer hashCode() {
|
||||
// return some hash value
|
||||
}
|
||||
}
|
||||
public class Foo { // perfect, both methods provided
|
||||
public Boolean equals(Object other) {
|
||||
// do some comparison
|
||||
}
|
||||
public Integer hashCode() {
|
||||
// return some hash value
|
||||
}
|
||||
}
|
||||
]]>
|
||||
</example>
|
||||
|
||||
@@ -472,5 +472,6 @@
|
||||
</properties>
|
||||
</rule>
|
||||
<!-- <rule ref="category/apex/bestpractices.xml/UnusedLocalVariable"/> -->
|
||||
<!-- <rule ref="category/apex/errorprone.xml/OverrideBothEqualsAndHashcode" /> -->
|
||||
|
||||
</ruleset>
|
||||
+11
@@ -0,0 +1,11 @@
|
||||
/**
|
||||
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
|
||||
*/
|
||||
|
||||
package net.sourceforge.pmd.lang.apex.rule.errorprone;
|
||||
|
||||
import net.sourceforge.pmd.testframework.PmdRuleTst;
|
||||
|
||||
public class OverrideBothEqualsAndHashcodeTest extends PmdRuleTst {
|
||||
// no additional unit tests
|
||||
}
|
||||
+166
@@ -0,0 +1,166 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<test-data
|
||||
xmlns="http://pmd.sourceforge.net/rule-tests"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
|
||||
|
||||
<test-code>
|
||||
<description>hash code only</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public Integer hashCode() {}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>nested hash code only</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public class Bar {
|
||||
public Integer hashCode() {}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>equals only</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean equals(Object other) {}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>nested equals only, checking case insensitiveness</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public class Bar {
|
||||
public Integer eQuAlS(Object other) {}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>overrides both</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean equals(Object other) {}
|
||||
public Integer hashCode() {}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>nested overrides both</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public class Bar {
|
||||
public boolean equals(Object other) {}
|
||||
public Integer hashCode() {}
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>overrides neither</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>equals sig uses String, not Object</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean equals(String o) {
|
||||
return true;
|
||||
}
|
||||
public Integer hashCode() {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>interface</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public interface Foo {
|
||||
boolean equals(Object o);
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>implements equals but with 2 args, hashCode overloaded as well</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean equals(java.lang.Object o1, java.lang.Object o2) {
|
||||
return true;
|
||||
}
|
||||
public Integer hashCode(java.lang.Object o) {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>overloaded hashCode</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public Integer hashCode(Object o1) { return false; }
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>overloaded both</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean equals(Object o1, Object o2) { return false; }
|
||||
public Integer hashCode(Object o1) { return false; }
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>overloaded hashCode, should fail on equals</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public boolean equals(Object o1) { return false; }
|
||||
public Integer hashCode(Object o1) { return false; }
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>implements hashCode but with args</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
public Integer hashCode(List<Decimal> a) {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
</test-data>
|
||||
@@ -0,0 +1,13 @@
|
||||
<?xml version="1.0"?>
|
||||
|
||||
<ruleset name="6310"
|
||||
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
|
||||
<description>
|
||||
This ruleset contains links to rules that are new in PMD v6.31.0
|
||||
</description>
|
||||
|
||||
<rule ref="category/apex/errorprone.xml/OverrideBothEqualsAndHashcode" />
|
||||
|
||||
</ruleset>
|
||||
Reference in new issue
Block a user