AEM-17 is claiming replace method is not mutable?

[holdsworthr]

Hi

I'm trying to get rid of this code smell but it appears this sonar rule believes the replace method is not mutable

[toniedzwiedz]

Hi @holdsworthr, thanks for reporting this. Looking https://github.com/wttech/AEM-Rules-for-SonarQube/blob/master/src/main/java/com/cognifide/aemrules/java/checks/ModifiableValueMapUsageCheck.java#L63, the rule only recognises put, putAll and remove as mutable methods.

I'm guessing that the implementation follows the ModifiableValueMap Javadoc, which says:

The modifiable value map is only changeable through one of these methods

• #put(String, Object)
• Map.putAll(java.util.Map)
• Map.remove(Object)

This is odd, because other methods inherited from java.util.Map have the effect of changing the values of the properties of the ValueMap's underlying resource.

Unless there's something I'm missing, both the AEM-17 Rule, and the Sling Javadoc could need a change. There may be a reason the Javadoc does not recommend using replace or replaceAll. If there's a good reason not to use those, we may be looking at a new rule. Let me search the Apache Sling Jira to find out if this has been discussed.

[toniedzwiedz]

https://issues.apache.org/jira/browse/SLING-11878 has been raised

[toniedzwiedz]

@holdsworthr it looks like these methods are valid. I think we'll improve AEM-17 to no longer report them. In the meantime, I would suggest resolving these issues as a won't fix or false positive on your Sonar.

[toniedzwiedz]

apache/sling-org-apache-sling-api#48 has been merged. This means we should update the rule to allow the following methods in addition to the ones it currently supports:

• #replace(Object, Object)
• #replace(Object, Object, Object)
• #replaceAll(java.util.function.BiFunction)
• #compute(Object, java.util.function.BiFunction)
• #computeIfAbsent(Object, java.util.function.Function)
• #computeIfPresent(Object, java.util.function.BiFunction)
• #merge(Object, Object, java.util.function.BiFunction)