rewrite-static-analysis icon indicating copy to clipboard operation
rewrite-static-analysis copied to clipboard

use `try-with-resources` statement

Open Pankraz76 opened this issue 8 months ago • 6 comments

What problem are you trying to solve?

  • https://github.com/apache/maven/pull/2426
  • https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html#usetrywithresources
  • https://mail.openjdk.org/pipermail/coin-dev/2009-April/001503.html

Describe the solution you'd like

  • https://www.baeldung.com/java-try-with-resources

Have you considered any alternatives or workarounds?

ide fix:

Image

Additional context

Are you interested in contributing this feature to OpenRewrite?

Pankraz76 avatar Jun 12 '25 20:06 Pankraz76

@iddeepak if this is approved, which seems likely, lets do this next, if you want.

Pankraz76 avatar Jun 12 '25 20:06 Pankraz76

This would indeed be nice to have. I can provide an implementation and tests that can be used as a starting point / inspiration.

knutwannheden avatar Jun 13 '25 03:06 knutwannheden

dont make like ide:

raw:

public void collectInput() throws IOException {
      final TerminalConnection connection = new TerminalConnection();
      connection.setSignalHandler(interruptionSignalHandler());
      try {
          read(connection, ReadlineBuilder.builder().enableHistory(false).build(), prompts.iterator());
          connection.openBlocking();
      } finally {
          connection.close();
      }
}

ide-fix:

public void collectInput() throws IOException {
      if (prompts.isEmpty()) {
          return;
      }
      final TerminalConnection connection = new TerminalConnection();
      try (connection) {
          connection.setSignalHandler(interruptionSignalHandler());
          read(connection, ReadlineBuilder.builder().enableHistory(false).build(), prompts.iterator());
          connection.openBlocking();
      }
  }

always try to inline.

public void collectInput() throws IOException {
      if (prompts.isEmpty()) {
          return;
      }
      try (TerminalConnection connection = new TerminalConnection()) {
          connection.setSignalHandler(interruptionSignalHandler());
          read(connection, ReadlineBuilder.builder().enableHistory(false).build(), prompts.iterator());
          connection.openBlocking();
      }
  }

Pankraz76 avatar Jun 13 '25 09:06 Pankraz76

edge case need to give (default) return type:

	@Override
	public long contentLength() throws IOException {
		InputStream is = getInputStream();
		try {
			long size = 0;
			byte[] buf = new byte[256];
			int read;
			while ((read = is.read(buf)) != -1) {
				size += read;
			}
			return size;
		}
		finally {
			try {
				is.close();
			}
			catch (IOException ex) {
				debug(() -> "Could not close content-length InputStream for " + getDescription(), ex);
			}
		}
	}

remove whole finally block:

	@Override
	public long contentLength() throws IOException {
		try (InputStream is = getInputStream()) {
			long size = 0;
			byte[] buf = new byte[256];
			int read;
			while ((read = is.read(buf)) != -1) {
				size += read;
			}
			return size;
		}
	}

Pankraz76 avatar Jun 13 '25 09:06 Pankraz76

check for ignored:

  • https://github.com/spring-projects/spring-framework/pull/35046
  • https://github.com/spring-projects/spring-framework/pull/35046/files#r2144621681

Variable 'scope' is never used

Image

Pankraz76 avatar Jun 13 '25 09:06 Pankraz76

There is indeed plenty of different ways this can be done and improved. But note that Java 9 (or is it 10?) is required to allow the resources to reference an existing variable. In Java 1.8 the resource must be declared as a variable inside the resource block.

FWIW, I'll create a PR with my recipe. Then feel free to use parts of it (or not).

knutwannheden avatar Jun 13 '25 11:06 knutwannheden