AEM-Rules-for-SonarQube icon indicating copy to clipboard operation
AEM-Rules-for-SonarQube copied to clipboard

Revise AEM-20 (ResourceResolver can be closed using try-with-resources Java 7 feature) Java 9+ support and possible redundancy.

Open toniedzwiedz opened this issue 4 years ago • 1 comments

During the code review of https://github.com/Cognifide/AEM-Rules-for-SonarQube/pull/181, we talked about try-with-resources statements and the implementation details of AEM-20 (ResourceResolver can be closed using try-with-resources Java 7 feature).

We noticed a suspicious cast

tree.resourceList().stream()
        .map(resource -> ((VariableTreeImpl) resource).simpleName().name())

and recommended the cast should be preceded by resource being checked using instanceof.

I've read into it a little further and it seems there's a new way to write a try-with-resources block in Java 9 and above.

The type check we proposed helps us allow a class cast exception but we now know of a case that AEM-6 doesn't cover, which could be encountered in AEM 6.5 projects (due to Java 11 support).

The rule should be revised, taking into account:

  • whether or not it provides any benefits over the rule java:S2095, Resources should be closed, which comes with the built-in Sonar way profile in SonarQube LTS 7.9.3 and could, in theory, highlight the misuse of ResourceResolver instances.
  • if it does provide an extra benefit, make it work for Java 11 AEM projects: -- Provide unit test cases with examples of new syntax -- Modify ResourceResolverTryWithResourcesCheck accordingly

toniedzwiedz avatar Apr 09 '20 15:04 toniedzwiedz

Also consider how this affects AEM-6

toniedzwiedz avatar May 22 '20 08:05 toniedzwiedz