[apex] New Rule: AvoidInterfaceAsMapKeyRule by JonnyPower · Pull Request #6493 · pmd/pmd

JonnyPower · 2026-03-08T23:52:38Z

Describe the PR

Apex has a dispatch bug in the implementation of Map that can result in difficult errors. This PMD rule looks for the structure that results in Map's bad dispatch, when a Map is using an interface as a key, the rule uses ApexLink to find implementations of the interface, then checking if any implementation is abstract & overrides equals / hashCode - this will result in incorrect dispatching from Map's methods.

This was largely implemented with AI, confirmed resulting jar finds violations against https://github.com/Traction-Rec/map-eq-dispatch-bug but doesn't trigger violation just because an interface is being used.

Ran this rule against relatively large internal project and it completed succesfully.

Related issues

Fix [apex] New rule: Prevent use of interface -> abstract class with equals/hashCode as key in Map #6492

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)

- rely on ApexLink for cross file analysis Closes pmd#6492

- discovered that this dispatch bug happens even if abstract class isn't implementing the equals/hashCode methods

- anyTypeMatches does seem useful, but I ended up not using it, so makes sense to remove

adangel

Thanks!

I'll fix the issues myself.

adangel · 2026-04-24T06:56:39Z

+ * to the root of your SFDX project (where {@code sfdx-project.json} lives). Without it the
+ * rule produces no violations.
+ *
+ * @see <a href="https://github.com/pmd/pmd/issues/6492">Issue 6492</a>


Suggested change

* @see <a href="https://github.com/pmd/pmd/issues/6492">Issue 6492</a>

* @see <a href="https://github.com/pmd/pmd/issues/6492">[apex] New rule: Prevent use of interface -> abstract class with equals/hashCode as key in Map #6492</a>

* @since 7.24.0

adangel · 2026-04-24T06:59:14Z

+     * Returns an unmodifiable list of all type summaries in the org.
+     * Returns an empty list when multifile analysis is unavailable.
+     * This enables rules to perform complex cross-type analysis.
+     */


Suggested change

*/

* @since 7.24.0

*/

adangel · 2026-04-24T07:00:56Z

        return multifileAnalysis.getFileIssues(fileId.getAbsolutePath());
    }

+    public @NonNull ApexMultifileAnalysis getMultifileAnalysis() {


Instead of this method, I'd return the type summaries directly. For now at least, this fits more the style - like the above method getGlobalIssues.

adangel · 2026-04-24T07:03:40Z

+    @Override
+    public Object visit(ASTApexFile node, Object data) {
+        ApexMultifileAnalysis mfa = node.getMultifileAnalysis();
+        if (mfa.isFailed()) {


I think, this warning should be implemented in ApexMultifileAnalysis directly

adangel · 2026-04-24T07:10:17Z

+    private static final Logger LOG = LoggerFactory.getLogger(AvoidInterfaceAsMapKeyRule.class);
+    private static boolean warnedAboutMissingOrg = false;
+
+    // Cached index - computed once per PMD run, reused across all file visits


Note: This is likely once per thread - if PMD is run multithreaded, each rule is instantiated once per thread.
If we want to cache it once per PMD run, then we would need to move it into ApexMultifileAnalysis. But that is maybe something for later, when we know, what we need. I could imagine, the TypeHierarchyIndex is maybe not rule specific and ApexMultifileAnalysis could have method like getSuperType(type) -> Type, Type.getMethods(), ... something like this.

adangel · 2026-04-24T07:18:12Z

+                typesBySimpleName.computeIfAbsent(simpleName, k -> new HashSet<>()).add(summary);
+                typesByFullName.put(fullName, summary);
+
+                if ("interface".equalsIgnoreCase(nature)) {


It probably makes sense to extract these natures as constants

adangel · 2026-04-24T07:32:14Z

+      In Apex, when a Map uses an interface as key and an abstract class implements that interface
+      and defines equals/hashCode, methods like containsKey do not dispatch to the correct implementation.
+      This rule reports Map declarations (fields, variables, parameters) whose key type is an interface
+      that has at least one abstract implementing class defining equals or hashCode (requires multi-file
+      analysis provided by apex-link).


Suggested change

In Apex, when a Map uses an interface as key and an abstract class implements that interface

and defines equals/hashCode, methods like containsKey do not dispatch to the correct implementation.

This rule reports Map declarations (fields, variables, parameters) whose key type is an interface

that has at least one abstract implementing class defining equals or hashCode (requires multi-file

analysis provided by apex-link).

In Apex, when a Map uses an interface as key and an abstract class implements that interface

and defines equals/hashCode, methods like containsKey do not dispatch to the correct implementation. This results in potentially duplicated map entries or not being able to get entries by key.

For example (`IKey` is an interface and `StringKey` implements IKey and equals/hashCode):

```apex

Map<IKey, String> m = new Map<IKey, String>();

m.put(new StringKey('hello'), 'value');

Boolean exists = m.containsKey(new StringKey('hello')); // might return false

m.put(new StringKey('hello'), 'other value'); // might add a second entry instead of replacing the existing one

```

This rule reports Map declarations (fields, variables, parameters) whose key type is an interface that has at least one abstract implementing class defining equals or hashCode.

[Apex Language Server](https://github.com/apex-dev-tools/apex-ls) is used to make this possible and this needs

additional configuration. The environment variable `PMD_APEX_ROOT_DIRECTORY` needs to be set prior to executing

PMD. With this variable the root directory of the Salesforce metadata, where `sfdx-project.json` resides, is

specified. Apex Language Server can then load all the types in the project and figure out the type hierarchy.

adangel · 2026-04-24T07:33:54Z

+        assertEquals(lineNumber, violation.getBeginLine());
+    }
+
+    private Report runRule(Path testProjectDir) throws IOException {


Looks like this is the same as in UnusedMethodTest - maybe it's time to provide test framework support?

adangel · 2026-04-24T07:35:57Z

+        for (RuleViolation v : report.getViolations()) {
+            assertEquals("MapUser.cls", v.getFileId().getFileName());
+        }


This is not necessary, filename is asserted with assertViolation as well

JonnyPower commented Mar 8, 2026

View reviewed changes

Comment thread pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/multifile/ApexMultifileAnalysis.java Outdated

JonnyPower commented Mar 9, 2026

View reviewed changes

Comment thread pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/multifile/ApexMultifileAnalysis.java Outdated

JonnyPower commented Mar 9, 2026

View reviewed changes

Comment thread pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java

JonnyPower commented Mar 9, 2026

View reviewed changes

Comment thread .../src/main/java/net/sourceforge/pmd/lang/apex/rule/errorprone/AvoidInterfaceAsMapKeyRule.java

feat: add new rule that avoids incorrect Map behavior

58bf4fd

- rely on ApexLink for cross file analysis Closes pmd#6492

JonnyPower force-pushed the feature/6492-apex-interface-key-with-abstract-equals branch from c40f35c to 58bf4fd Compare March 9, 2026 00:18

JonnyPower marked this pull request as draft March 9, 2026 20:35

JonnyPower added 2 commits March 9, 2026 14:06

feat: add tests and cover more cases of apex bug

1ffae1e

- discovered that this dispatch bug happens even if abstract class isn't implementing the equals/hashCode methods

feat: index type summaries once for rule

90be883

JonnyPower force-pushed the feature/6492-apex-interface-key-with-abstract-equals branch from 9928701 to 90be883 Compare March 9, 2026 21:27

chore: note apex-link / mfa dependency for rule

eea22f7

JonnyPower marked this pull request as ready for review March 9, 2026 21:33

JonnyPower changed the title ~~[apex] feat: add new rule that avoids incorrect Map behavior~~ [apex] New Rule: AvoidInterfaceAsMapKeyRule Mar 9, 2026

JonnyPower mentioned this pull request Mar 9, 2026

[apex] New rule: Prevent use of interface -> abstract class with equals/hashCode as key in Map #6492

Closed

JonnyPower added 2 commits March 9, 2026 16:27

feat: add tests for transitive hierarchies

0cb3cb9

chore: remove dead code from earlier iterations

52d3b34

- anyTypeMatches does seem useful, but I ended up not using it, so makes sense to remove

adangel added the a:new-rule Proposal to add a new built-in rule label Mar 24, 2026

adangel added this to the 7.24.0 milestone Mar 24, 2026

JonnyPower commented Mar 24, 2026

View reviewed changes

Comment thread .../src/main/java/net/sourceforge/pmd/lang/apex/rule/errorprone/AvoidInterfaceAsMapKeyRule.java

Comment thread .../src/main/java/net/sourceforge/pmd/lang/apex/rule/errorprone/AvoidInterfaceAsMapKeyRule.java

adangel self-requested a review April 17, 2026 09:58

adangel approved these changes Apr 24, 2026

View reviewed changes

adangel added a commit that referenced this pull request Apr 24, 2026

[doc] Update release notes (#6492, #6493)

4325ba7

adangel merged commit 52d3b34 into pmd:main Apr 24, 2026

adangel added a commit that referenced this pull request Apr 24, 2026

[apex] New Rule: AvoidInterfaceAsMapKeyRule (#6493)

8ef1b90

	* @see <a href="https://github.com/pmd/pmd/issues/6492">Issue 6492</a>
	* @see <a href="https://github.com/pmd/pmd/issues/6492">[apex] New rule: Prevent use of interface -> abstract class with equals/hashCode as key in Map #6492</a>
	* @since 7.24.0

-      In Apex, when a Map uses an interface as key and an abstract class implements that interface
-      and defines equals/hashCode, methods like containsKey do not dispatch to the correct implementation.
-      This rule reports Map declarations (fields, variables, parameters) whose key type is an interface
-      that has at least one abstract implementing class defining equals or hashCode (requires multi-file
-      analysis provided by apex-link).
+In Apex, when a Map uses an interface as key and an abstract class implements that interface
+and defines equals/hashCode, methods like containsKey do not dispatch to the correct implementation. This results in potentially duplicated map entries or not being able to get entries by key.
+For example (`IKey` is an interface and `StringKey` implements IKey and equals/hashCode):
+```apex
+Map<IKey, String> m = new Map<IKey, String>();
+m.put(new StringKey('hello'), 'value');
+Boolean exists = m.containsKey(new StringKey('hello')); // might return false
+m.put(new StringKey('hello'), 'other value'); // might add a second entry instead of replacing the existing one
+```
+This rule reports Map declarations (fields, variables, parameters) whose key type is an interface that has at least one abstract implementing class defining equals or hashCode.
+[Apex Language Server](https://github.com/apex-dev-tools/apex-ls) is used to make this possible and this needs
+additional configuration. The environment variable `PMD_APEX_ROOT_DIRECTORY` needs to be set prior to executing
+PMD. With this variable the root directory of the Salesforce metadata, where `sfdx-project.json` resides, is
+specified. Apex Language Server can then load all the types in the project and figure out the type hierarchy.

Uh oh!

Conversation

JonnyPower commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the PR

Related issues

Ready?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adangel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JonnyPower commented Mar 8, 2026 •

edited

Loading