Skip to content

Conversation

@Njeriiii
Copy link
Owner

@Njeriiii Njeriiii commented Jun 12, 2024

What does this implement/fix? Explain your changes.

This class contains the inspection tool for checking if Closeable clients are properly closed.
It checks if the Closeable client is declared in a try-with-resources block or if it is closed in the code block.
If the Closeable client is not closed, it registers a problem with the ProblemsHolder.
currently can identify closeable clients in the PSI tree

Has this been tested?

  • Tested

Any relevant logs, screenshots, error output, etc.?

image

image

image


System.out.println("Printing resourceList");
System.out.println(resourceList);
if (resourceList != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this working now?
Also, did you figure out unit tests?

@Njeriiii Njeriiii marked this pull request as ready for review July 24, 2024 23:10
@Njeriiii Njeriiii changed the title draft close closeable clients rule close closeable clients rule Jul 24, 2024
* This method checks if the variable is closed elsewhere in the source code.
* It checks if the variable is closed in the finally block of a try statement or if it is closed elsewhere in the source code.
*/
private static boolean isResourceClosed(PsiVariable variable, PsiElement element) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it detect issues if I do this:

void doStuff() {
    Client client = createClient();
    // it's ok if we can't detect all possible cases when variable is reassigned multiple times
}

Client createClient() {
  Client c = new Client();
  ...
  return  c;
}

or

void doStuff() {
    Client client = createClient();
    // important that we don't report issue here
    client.close();
}

Client createClient() {
  Client c = new Client();
  ...
  return  c;
}

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants