Bug 381445 - [compiler][resource] Can the resource leak check be made

aware of Closeables.closeQuietly?

3 files changed, 216 insertions, 8 deletions

diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java
index e679c99096..aa32c91b73 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java
@@ -25,6 +25,14 @@ import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 
 public class ResourceLeakTests extends AbstractRegressionTest {
 
+// well-known helper class:
+private static final String GUAVA_CLOSEABLES_JAVA = "com/google/common/io/Closeables.java";
+private static final String GUAVA_CLOSEABLES_CONTENT = "package com.google.common.io;\n" +
+	"public class Closeables {\n" +
+	"    public static void closeQuietly(java.io.Closeable closeable) {}\n" +
+	"    public static void close(java.io.Closeable closeable, boolean flag) {}\n" +
+	"}\n";
+
 static {
 //	TESTS_NAMES = new String[] { "testBug394768" };
 //	TESTS_NUMBERS = new int[] { 50 };
@@ -4011,4 +4019,165 @@ public void testBug394768_1() {
 		options,
 		null);
 }
+
+// Bug 381445 - [compiler][resource] Can the resource leak check be made aware of Closeables.closeQuietly?
+// A resource is closed using various known close helpers
+public void testBug381445_1() {
+	Map options = getCompilerOptions();
+	options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR);
+	options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR);
+	runNegativeTest(
+		new String[] {
+			GUAVA_CLOSEABLES_JAVA,
+			GUAVA_CLOSEABLES_CONTENT,
+			"org/apache/commons/io/IOUtils.java",
+			"package org.apache.commons.io;\n" +
+			"public class IOUtils {\n" +
+			"    public static void closeQuietly(java.io.Closeable closeable) {}\n" +
+			"}\n",
+			"Bug381445.java",
+			"import java.io.File;\n" +
+			"import java.io.FileInputStream;\n" +
+			"import java.io.InputStream;\n" +
+			"\n" +
+			"public class Bug381445 {\n" +
+			"	public void readFile(String path) throws Exception {\n" +
+			"		File file = new File(path);\n" +
+			"		InputStream stream1 = new FileInputStream(path);\n" +
+			"		InputStream stream2 = new FileInputStream(path);\n" +
+			"		InputStream stream3 = new FileInputStream(path);\n" +
+			"		InputStream stream4 = new FileInputStream(path);\n" +
+			"		try {\n" +
+			"			// Use the opened streams here\n" +
+			"			stream1.read();\n" +
+			"			stream2.read();\n" +
+			"			stream3.read();\n" +
+			"			stream4.read();\n" +
+			"		} finally {\n" +
+			"			com.google.common.io.Closeables.closeQuietly(stream1);\n" +
+			"			com.google.common.io.Closeables.close(stream2, false);\n" +
+			"			org.apache.commons.io.IOUtils.closeQuietly(stream3);\n" +
+			"			Closeables.closeQuietly(stream4);\n" +
+			"		}\n" +
+			"	}\n" +
+			"}\n" +
+			"class Closeables {\n" + // fake, should not be recognized
+			"	public static void closeQuietly(java.io.Closeable closeable) {}\n" +
+			"}\n"
+		},
+		"----------\n" + 
+		"1. ERROR in Bug381445.java (at line 11)\n" + 
+		"	InputStream stream4 = new FileInputStream(path);\n" + 
+		"	            ^^^^^^^\n" + 
+		"Potential resource leak: \'stream4\' may not be closed\n" + 
+		"----------\n",
+		null,
+		true,
+		options,
+		null);	
+}
+
+// Bug 381445 - [compiler][resource] Can the resource leak check be made aware of Closeables.closeQuietly?
+// A resource is closed in different places of the flow
+public void testBug381445_2() {
+	Map options = getCompilerOptions();
+	options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR);
+	options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR);
+	runNegativeTest(
+		new String[] {
+			GUAVA_CLOSEABLES_JAVA,
+			GUAVA_CLOSEABLES_CONTENT,
+			"Bug381445.java",
+			"import java.io.File;\n" +
+			"import java.io.FileInputStream;\n" +
+			"import java.io.InputStream;\n" +
+			"import com.google.common.io.Closeables;\n" +
+			"\n" +
+			"public class Bug381445 {\n" +
+			"	public void readFile(String path) throws Exception {\n" +
+			"		File file = new File(path);\n" +
+			"		InputStream stream1 = new FileInputStream(path);\n" +
+			"		InputStream stream2 = new FileInputStream(path);\n" +
+			"		InputStream stream3 = new FileInputStream(path);\n" +
+			"		try {\n" +
+			"			// Use the opened streams here\n" +
+			"			stream1.read();\n" +
+			"			Closeables.closeQuietly(stream1);\n" +
+			"			stream2.read();\n" +
+			"			if (path.length() > 2)\n" +
+			"				Closeables.closeQuietly(stream2);\n" + // close inside if is too weak
+			"			stream3.read();\n" +
+			"		} finally {\n" +
+			"		}\n" +
+			"		Closeables.closeQuietly(stream3);\n" +
+			"	}\n" +
+			"}\n"
+		},
+		"----------\n" + 
+		"1. ERROR in Bug381445.java (at line 10)\n" + 
+		"	InputStream stream2 = new FileInputStream(path);\n" + 
+		"	            ^^^^^^^\n" + 
+		"Potential resource leak: \'stream2\' may not be closed\n" + 
+		"----------\n",
+		null,
+		true,
+		options,
+		null);	
+}
+
+// Bug 381445 - [compiler][resource] Can the resource leak check be made aware of Closeables.closeQuietly?
+// A close helper is referenced in various ways:
+public void testBug381445_3() {
+	if (this.complianceLevel < ClassFileConstants.JDK1_5) return; // using static import
+	Map options = getCompilerOptions();
+	options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR);
+	options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR);
+	runConformTest(
+		new String[] {
+			GUAVA_CLOSEABLES_JAVA,
+			GUAVA_CLOSEABLES_CONTENT,
+			"Bug381445a.java",
+			"import java.io.File;\n" +
+			"import java.io.FileInputStream;\n" +
+			"import java.io.InputStream;\n" +
+			"import static com.google.common.io.Closeables.closeQuietly;\n" +
+			"\n" +
+			"public class Bug381445a {\n" +
+			"	public void readFile(String path) throws Exception {\n" +
+			"		File file = new File(path);\n" +
+			"		InputStream stream = new FileInputStream(path);\n" +
+			"		try {\n" +
+			"			// Use the opened stream here\n" +
+			"			stream.read();\n" +
+			"		} finally {\n" +
+			"			closeQuietly(stream);\n" + // via static import
+			"		}\n" +
+			"	}\n" +
+			"}\n",
+			"Bug381445b.java",
+			"import java.io.File;\n" +
+			"import java.io.FileInputStream;\n" +
+			"import java.io.InputStream;\n" +
+			"import com.google.common.io.Closeables;\n" +
+			"\n" +
+			"public class Bug381445b extends Closeables {\n" +
+			"	public void readFile(String path) throws Exception {\n" +
+			"		File file = new File(path);\n" +
+			"		InputStream stream = new FileInputStream(path);\n" +
+			"		try {\n" +
+			"			// Use the opened streams here\n" +
+			"			stream.read();\n" +
+			"		} finally {\n" +
+			"			closeQuietly(stream);\n" + // via super class
+			"		}\n" +
+			"	}\n" +
+			"}\n",
+		},
+		"",
+		null,
+		true,
+		null, 
+		options,
+		null);	
+}
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java
index cefefa9808..1fd060e2ed 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java
@@ -25,6 +25,7 @@
  *								bug 388281 - [compiler][null] inheritance of null annotations as an option
  *								bug 392862 - [1.8][compiler][null] Evaluate null annotations on array types
  *								bug 394768 - [compiler][resource] Incorrect resource leak warning when creating stream in conditional
+ *								bug 381445 - [compiler][resource] Can the resource leak check be made aware of Closeables.closeQuietly?
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.ast;
 
@@ -80,19 +81,40 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 	boolean nonStatic = !this.binding.isStatic();
 	boolean wasInsideAssert = ((flowContext.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING) != 0);
 	flowInfo = this.receiver.analyseCode(currentScope, flowContext, flowInfo, nonStatic).unconditionalInits();
+
 	// recording the closing of AutoCloseable resources:
 	boolean analyseResources = currentScope.compilerOptions().analyseResourceLeaks;
-	if (analyseResources && CharOperation.equals(TypeConstants.CLOSE, this.selector)) 
-	{
-		FakedTrackingVariable trackingVariable = FakedTrackingVariable.getCloseTrackingVariable(this.receiver, flowInfo, flowContext);
-		if (trackingVariable != null) { // null happens if receiver is not a local variable or not an AutoCloseable
-			if (trackingVariable.methodScope == currentScope.methodScope()) {
-				trackingVariable.markClose(flowInfo, flowContext);
-			} else {
-				trackingVariable.markClosedInNestedMethod();
+	if (analyseResources) {
+		Expression closeTarget = null;
+		if (nonStatic) {
+			// closeable.close()
+			if (CharOperation.equals(TypeConstants.CLOSE, this.selector)) {
+				closeTarget = this.receiver;
+			}
+		} else if (this.arguments != null && this.arguments.length > 0 && FakedTrackingVariable.isAnyCloseable(this.arguments[0].resolvedType)) {
+			// Helper.closeMethod(closeable, ..)
+			for (int i=0; i<TypeConstants.closeMethods.length; i++) {
+				CloseMethodRecord record = TypeConstants.closeMethods[i];
+				if (CharOperation.equals(record.selector, this.selector)
+						&& CharOperation.equals(record.typeName, this.binding.declaringClass.compoundName)) 
+				{
+					closeTarget = this.arguments[0];
+					break;
+				}
+			}
+		}
+		if (closeTarget != null) {
+			FakedTrackingVariable trackingVariable = FakedTrackingVariable.getCloseTrackingVariable(closeTarget, flowInfo, flowContext);
+			if (trackingVariable != null) { // null happens if target is not a local variable or not an AutoCloseable
+				if (trackingVariable.methodScope == currentScope.methodScope()) {
+					trackingVariable.markClose(flowInfo, flowContext);
+				} else {
+					trackingVariable.markClosedInNestedMethod();
+				}
 			}
 		}
 	}
+
 	if (nonStatic) {
 		this.receiver.checkNPE(currentScope, flowContext, flowInfo);
 		// https://bugs.eclipse.org/bugs/show_bug.cgi?id=318682
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
index 67e770e017..66569aa02b 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
@@ -14,6 +14,7 @@
  *     Stephan Herrmann - Contributions for
  *								bug 349326 - [1.7] new warning for missing try-with-resources
  *								bug 358903 - Filter practically unimportant resource leak warnings
+ *								bug 381445 - [compiler][resource] Can the resource leak check be made aware of Closeables.closeQuietly?
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.lookup;
 
@@ -165,6 +166,22 @@ public interface TypeConstants {
 	};
 	char[][] JAVA_LANG_AUTOCLOSEABLE =  {JAVA, LANG, "AutoCloseable".toCharArray()}; //$NON-NLS-1$
 	char[] CLOSE = "close".toCharArray(); //$NON-NLS-1$
+	// known helper functions for closing a Closeable (all receive a Closeable as their first argument):
+	public static class CloseMethodRecord {
+		public char[][] typeName;
+		public char[] selector;
+		public CloseMethodRecord(char[][] typeName, char[] selector) {
+			this.typeName = typeName;
+			this.selector = selector;
+		}
+	}
+	char[][] GUAVA_CLOSEABLES = { "com".toCharArray(), "google".toCharArray(), "common".toCharArray(), "io".toCharArray(), "Closeables".toCharArray() }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
+	char[][] APACHE_IOUTILS = { "org".toCharArray(), "apache".toCharArray(), "commons".toCharArray(), "io".toCharArray(), "IOUtils".toCharArray() }; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
+	CloseMethodRecord[] closeMethods = new CloseMethodRecord[] {
+		new CloseMethodRecord(GUAVA_CLOSEABLES, "closeQuietly".toCharArray()), //$NON-NLS-1$
+		new CloseMethodRecord(GUAVA_CLOSEABLES, "close".toCharArray()), //$NON-NLS-1$
+		new CloseMethodRecord(APACHE_IOUTILS, "closeQuietly".toCharArray()) //$NON-NLS-1$
+	};
 	// white lists of closeables:
 	char[][] JAVA_IO_WRAPPER_CLOSEABLES = new char[][] {
 		"BufferedInputStream".toCharArray(), //$NON-NLS-1$