Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
toniedzwiedz opened this issue Apr 9, 2020 · 1 comment
Labels
enhancement rule New rule or modification of existing one.

Comments

@toniedzwiedz
Copy link
Collaborator

toniedzwiedz commented Apr 9, 2020

During the code review of #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 toniedzwiedz mentioned this issue Apr 9, 2020
@toniedzwiedz toniedzwiedz added bug enhancement rule New rule or modification of existing one. and removed bug labels Apr 9, 2020
@toniedzwiedz toniedzwiedz changed the title Revise AEM-6 (ResourceResolver should be closed in finally block.) (Java 9+ support and possible redundancy) Revise AEM-6 (ResourceResolver should be closed in finally block.) Java 9+ support and possible redundancy. Apr 9, 2020
@toniedzwiedz toniedzwiedz changed the title Revise AEM-6 (ResourceResolver should be closed in finally block.) Java 9+ support and possible redundancy. Revise AEM-20 (ResourceResolver can be closed using try-with-resources Java 7 feature.) and AEM-6 (ResourceResolver should be closed in finally block.) Java 9+ support and possible redundancy. May 22, 2020
@toniedzwiedz toniedzwiedz changed the title Revise AEM-20 (ResourceResolver can be closed using try-with-resources Java 7 feature.) and AEM-6 (ResourceResolver should be closed in finally block.) Java 9+ support and possible redundancy. Revise AEM-20 (ResourceResolver can be closed using try-with-resources Java 7 feature) Java 9+ support and possible redundancy. May 22, 2020
@toniedzwiedz
Copy link
Collaborator Author

Also consider how this affects AEM-6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement rule New rule or modification of existing one.
Projects
None yet
Development

No branches or pull requests

1 participant