Bug 541759 - [resource] don't suggest using t-w-r for a foreach elementI20181205-0600I20181204-1800I20181204-0600I20181203-1800

variable

Change-Id: I18f766ec36fa8e2c4bc25fafaa0e01bc3d40e388

3 files changed, 75 insertions, 7 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 765e10dfac..0d2d7a5b82 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
@@ -5437,4 +5437,63 @@ public void testBug473317() {
 		"----------\n",
 		compilerOptions);
 }
+public void testBug541705() {
+	if (this.complianceLevel < ClassFileConstants.JDK1_7) return; // uses diamond
+	Runner runner = new Runner();
+	runner.customOptions = getCompilerOptions();
+	runner.customOptions.put(CompilerOptions.OPTION_ReportExplicitlyClosedAutoCloseable, CompilerOptions.ERROR);
+	runner.testFiles = new String[] {
+		"Test.java",
+		"import java.util.*;\n" +
+		"import java.util.zip.*;\n" +
+		"import java.io.*;\n" +
+		"public class Test {\n" +
+		"	private static HashMap<String, ZipFile> fgZipFileCache = new HashMap<>(5);\n" + 
+		"	public static void closeArchives() {\n" + 
+		"		synchronized (fgZipFileCache) {\n" + 
+		"			for (ZipFile file : fgZipFileCache.values()) {\n" +
+		"				synchronized (file) {\n" + 
+		"					try {\n" + 
+		"						file.close();\n" + 
+		"					} catch (IOException e) {\n" + 
+		"						System.out.println(e);\n" + 
+		"					}\n" + 
+		"				}\n" + 
+		"			}\n" + 
+		"			fgZipFileCache.clear();\n" + 
+		"		}\n" + 
+		"	}\n" +
+		"}\n"
+	};
+	runner.runConformTest();
+}
+public void testBug541705b() {
+	if (this.complianceLevel < ClassFileConstants.JDK9) return; // variable used in t-w-r
+	Runner runner = new Runner();
+	runner.customOptions = getCompilerOptions();
+	runner.customOptions.put(CompilerOptions.OPTION_ReportExplicitlyClosedAutoCloseable, CompilerOptions.ERROR);
+	runner.testFiles = new String[] {
+		"Test.java",
+		"import java.util.*;\n" +
+		"import java.util.zip.*;\n" +
+		"import java.io.*;\n" +
+		"public class Test {\n" +
+		"	private static HashMap<String, ZipFile> fgZipFileCache = new HashMap<>(5);\n" + 
+		"	public static void closeArchives() {\n" + 
+		"		synchronized (fgZipFileCache) {\n" + 
+		"			for (ZipFile file : fgZipFileCache.values()) {\n" + 
+		"				synchronized (file) {\n" + 
+		"					try (file) {\n" + 
+		"					} catch (IOException e) {\n" + 
+		"						System.out.println(e);\n" + 
+		"					}\n" + 
+		"				}\n" + 
+		"			}\n" + 
+		"			fgZipFileCache.clear();\n" + 
+		"		}\n" + 
+		"	}\n" +
+		"}\n"
+	};
+	runner.runConformTest();
+}
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java
index d09322585c..8498b31c09 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java
@@ -79,7 +79,9 @@ public class FakedTrackingVariable extends LocalDeclaration {
 	private static final int REPORTED_POTENTIAL_LEAK = 32;
 	// a location independent definitive problem has been reported against this resource:
 	private static final int REPORTED_DEFINITIVE_LEAK = 64;
-	
+	// a local declarations that acts as the element variable of a foreach loop (should never suggest to use t-w-r):
+	private static final int FOREACH_ELEMENT_VAR = 128;
+
 	public static boolean TEST_372319 = false; // see https://bugs.eclipse.org/372319
 
 	/**
@@ -449,9 +451,9 @@ public class FakedTrackingVariable extends LocalDeclaration {
 					if (rhsTrackVar.originalBinding != null)
 						local.closeTracker = rhsTrackVar;			//		a.: let fresh LHS share it
 					if (rhsTrackVar.currentAssignment == location) {
-						// pre-set tracker from lhs - passed from outside?
+						// pre-set tracker from lhs - passed from outside (or foreach)?
 						// now it's a fresh resource
-						rhsTrackVar.globalClosingState &= ~(SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE);
+						rhsTrackVar.globalClosingState &= ~(SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE|FOREACH_ELEMENT_VAR);
 					}
 				} else {
 					if (rhs instanceof AllocationExpression || rhs instanceof ConditionalExpression) {
@@ -484,7 +486,7 @@ public class FakedTrackingVariable extends LocalDeclaration {
 						}
 					}
 					// re-assigning from a fresh value, mark as not-closed again:
-					if ((previousTracker.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE)) == 0
+					if ((previousTracker.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE|FOREACH_ELEMENT_VAR)) == 0
 							&& flowInfo.hasNullInfoFor(previousTracker.binding)) // avoid spilling info into a branch that doesn't see the corresponding resource
 						flowInfo.markAsDefinitelyNull(previousTracker.binding);
 					local.closeTracker = analyseCloseableExpression(flowInfo, flowContext, local, location, rhs, previousTracker);
@@ -494,7 +496,7 @@ public class FakedTrackingVariable extends LocalDeclaration {
 				if (rhsTrackVar != null) {
 					local.closeTracker = rhsTrackVar;
 					// a fresh resource, mark as not-closed:
-					if ((rhsTrackVar.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE)) == 0)
+					if ((rhsTrackVar.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE|FOREACH_ELEMENT_VAR)) == 0)
 						flowInfo.markAsDefinitelyNull(rhsTrackVar.binding);
 // TODO(stephan): this might be useful, but I could not find a test case for it: 
 //					if (flowContext.initsOnFinally != null)
@@ -753,6 +755,12 @@ public class FakedTrackingVariable extends LocalDeclaration {
 		return flowInfo;
 	}
 
+	public static void markForeachElementVar(LocalDeclaration local) {
+		if (local.binding != null && local.binding.closeTracker != null) {
+			local.binding.closeTracker.globalClosingState |= FOREACH_ELEMENT_VAR;
+		}
+	}
+
 	/**
 	 * Iterator for a set of FakedTrackingVariable, which dispenses the elements 
 	 * according to the priorities defined by enum {@link Stage}.
@@ -987,7 +995,7 @@ public class FakedTrackingVariable extends LocalDeclaration {
 	}
 
 	public void reportExplicitClosing(ProblemReporter problemReporter) {
-		if ((this.globalClosingState & (OWNED_BY_OUTSIDE|REPORTED_EXPLICIT_CLOSE)) == 0) { // can't use t-w-r for OWNED_BY_OUTSIDE
+		if ((this.globalClosingState & (OWNED_BY_OUTSIDE|REPORTED_EXPLICIT_CLOSE|FOREACH_ELEMENT_VAR)) == 0) { // can't use t-w-r for OWNED_BY_OUTSIDE
 			this.globalClosingState |= REPORTED_EXPLICIT_CLOSE;
 			problemReporter.explicitlyClosedAutoCloseable(this);
 		}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java
index 26e30f772b..fe438b0220 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java
@@ -99,7 +99,7 @@ public class ForeachStatement extends Statement {
 		int initialComplaintLevel = (flowInfo.reachMode() & FlowInfo.UNREACHABLE) != 0 ? Statement.COMPLAINED_FAKE_REACHABLE : Statement.NOT_COMPLAINED;
 
 		// process the element variable and collection
-		flowInfo = this.elementVariable.analyseCode(this.scope, flowContext, flowInfo);		
+		flowInfo = this.elementVariable.analyseCode(this.scope, flowContext, flowInfo);
 		FlowInfo condInfo = this.collection.analyseCode(this.scope, flowContext, flowInfo.copy());
 		this.collection.checkNPE(currentScope, flowContext, condInfo.copy(), 1);
 		LocalVariableBinding elementVarBinding = this.elementVariable.binding;
@@ -132,6 +132,7 @@ public class ForeachStatement extends Statement {
 			if (this.action.complainIfUnreachable(actionInfo, this.scope, initialComplaintLevel, true) < Statement.COMPLAINED_UNREACHABLE) {
 				actionInfo = this.action.analyseCode(this.scope, loopingContext, actionInfo).unconditionalCopy();
 				if (this.action instanceof Block) {
+					FakedTrackingVariable.markForeachElementVar(this.elementVariable);
 					// action.analyseCode() missed the following check due to identical scopes of ForeachStatement and Block:
 					this.scope.checkUnclosedCloseables(actionInfo, loopingContext, null, null);
 				}