discussing rule AEM-20 "ResourceResolver can be closed using try-with-resources"

[joerghoh]

I think that the current implementation displays quite a lot of false positives, which diminish the value of this rule right now.

Look at this example (https://github.com/Adobe-Consulting-Services/acs-aem-commons/blob/4b3da3f5fdf0b59049b966bea9306d24c87d0293/bundle/src/main/java/com/adobe/acs/commons/audit_log_search/impl/AuditLogSearchServlet.java#L64)

protected final void doGet(SlingHttpServletRequest request, SlingHttpServletResponse response)
            throws ServletException, IOException {
[...]
ResourceResolver resolver = request.getResourceResolver();
[...]

This line is pointed out with this rule.
But actually it does not apply here, as this is a simple assignment, and no ResourecResolver has been opened here. When you close the RR as suggested (by whatever means) you are doing wrong. Because you did not open the RR here, but you just did an assignment to a local variable.

This rule as it is today is not helpful, because blindly following the rules will cause developers to introduce functional issues in their code.

Instead only methods, which clearly open a new RR should be considered. Given the number of patterns out there, which are used for it, the rules are likely to get much more complex. Consider these patterns:

ResourceResolver rr = resourceResolverFactory.login[...];

and

ResourceResolver rr = HelperService.getNewResourceResolver();

and

ResourceResolver getNewResourceResolver() {
  [...]
  ResourceResolver rr = resourceResolverFactory.login[...]
  return rr;
}

I would be great if you could point any issue clearly; even if that means, that we cannot detect all possible situations where a RR will remains unclosed.

[toniedzwiedz]

Hi @joerghoh, I agree that the rule is somewhat troublesome for a number of reasons, some of which I've described in #188

I'm of a mind to get rid of it. If other rules pertaining to Autocloseable resources fail to pick up Sling-specific examples, we can revisit bringing the rule back and make sure to use the examples you provided as unit tests to avoid false positives arising.