diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java index 0c9324c..e24388f 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java @@ -6020,6 +6020,7 @@ public void test0561_try_catch_unchecked_exception() { public void test0562_try_catch_unchecked_exception() { Map options = getCompilerOptions(); options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.WARNING); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.WARNING); this.runNegativeTest( true, new String[] { @@ -6042,7 +6043,7 @@ public void test0562_try_catch_unchecked_exception() { "1. WARNING in X.java (at line 6)\n" + " o = new LineNumberReader(new FileReader(\"dummy\"));\n" + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + - "Resource leak: 'o' is never closed\n" + + "Potential resource leak: \'o\' may not be closed\n" + "----------\n" + "2. ERROR in X.java (at line 8)\n" + " o.toString();\n" + 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 d0786ca..51d5e74 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 @@ -2112,17 +2112,17 @@ public void test061f2() throws IOException { "import java.io.IOException;\n" + "public class X {\n" + " void zork() throws IOException {\n" + - " try {\n" + - " OutputStream os = null;\n" + - " try {\n" + - " os = new BufferedOutputStream(new FileOutputStream(\"somefile\"));\n" + - " String externalForm = \"externalPath\";\n" + - " } finally {\n" + - " if (os != null)\n" + - " os.close();\n" + - " }\n" + - " } catch (IOException e) {\n" + - " e.printStackTrace();\n" + + " try {\n" + + " OutputStream os = null;\n" + + " try {\n" + + " os = new BufferedOutputStream(new FileOutputStream(\"somefile\"));\n" + + " String externalForm = \"externalPath\";\n" + + " } finally {\n" + + " if (os != null)\n" + + " os.close();\n" + + " }\n" + + " } catch (IOException e) {\n" + + " e.printStackTrace();\n" + " }\n" + " }\n" + "}\n" @@ -2150,25 +2150,25 @@ public void test061f3() throws IOException { "import java.io.InputStream;\n" + "import java.io.BufferedInputStream;\n" + "public class X {\n" + - " String loadProfile(File profileFile) {\n" + - " try {\n" + - " InputStream stream = new BufferedInputStream(new FileInputStream(profileFile));\n" + - " return loadProfile(stream);\n" + - " } catch (FileNotFoundException e) {\n" + - " //null\n" + - " }\n" + - " return null;\n" + - " }\n" + - " private String loadProfile(InputStream stream) {\n" + - " return null;\n" + + " String loadProfile(File profileFile) {\n" + + " try {\n" + + " InputStream stream = new BufferedInputStream(new FileInputStream(profileFile));\n" + + " return loadProfile(stream);\n" + + " } catch (FileNotFoundException e) {\n" + + " //null\n" + + " }\n" + + " return null;\n" + + " }\n" + + " private String loadProfile(InputStream stream) {\n" + + " return null;\n" + " }\n" + "}\n" }, - "----------\n" + - "1. ERROR in X.java (at line 10)\n" + - " return loadProfile(stream);\n" + - " ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + - "Potential resource leak: \'stream\' may not be closed at this location\n" + + "----------\n" + + "1. ERROR in X.java (at line 10)\n" + + " return loadProfile(stream);\n" + + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + + "Potential resource leak: \'stream\' may not be closed at this location\n" + "----------\n", null, true, @@ -2487,11 +2487,11 @@ public void test061n() { " }\n" + "}\n" }, - "----------\n" + - "1. ERROR in X.java (at line 5)\n" + - " PrintWriter writer = new PrintWriter(\"filename\");\n" + - " ^^^^^^\n" + - "Resource leak: \'writer\' is never closed\n" + + "----------\n" + + "1. ERROR in X.java (at line 5)\n" + + " PrintWriter writer = new PrintWriter(\"filename\");\n" + + " ^^^^^^\n" + + "Resource leak: \'writer\' is never closed\n" + "----------\n", null, true, @@ -2511,15 +2511,15 @@ public void test061o() { "import java.io.BufferedInputStream;\n" + "import java.io.IOException;\n" + "public class X {\n" + - " void foo(boolean bar) throws IOException {\n" + - " File file = new File(\"somefil\");\n" + - " FileInputStream fileStream = new FileInputStream(file);\n" + - " BufferedInputStream bis = new BufferedInputStream(fileStream); \n" + - " if (bar) {\n" + - " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + - " doubleWrap.close();\n" + - " }\n" + - " }\n" + + " void foo(boolean bar) throws IOException {\n" + + " File file = new File(\"somefil\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream); \n" + + " if (bar) {\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " doubleWrap.close();\n" + + " }\n" + + " }\n" + "}\n" }, "----------\n" + @@ -2547,13 +2547,13 @@ public void test061f4() throws IOException { "import java.io.InputStream;\n" + "import java.io.BufferedInputStream;\n" + "public class X {\n" + - " void foo(File location, String adviceFilePath) throws FileNotFoundException {\n" + - " InputStream stream = null;\n" + - " if (location.isDirectory()) {\n" + - " File adviceFile = new File(location, adviceFilePath);\n" + - " stream = new BufferedInputStream(new FileInputStream(adviceFile));\n" + - " }\n" + - " }\n" + + " void foo(File location, String adviceFilePath) throws FileNotFoundException {\n" + + " InputStream stream = null;\n" + + " if (location.isDirectory()) {\n" + + " File adviceFile = new File(location, adviceFilePath);\n" + + " stream = new BufferedInputStream(new FileInputStream(adviceFile));\n" + + " }\n" + + " }\n" + "}\n" }, "----------\n" + @@ -2566,6 +2566,128 @@ public void test061f4() throws IOException { true, options); } +// Bug 358903 - Filter practically unimportant resource leak warnings +// a t-w-r wraps an existing resource +public void test061p() { + if (this.complianceLevel < ClassFileConstants.JDK1_7) return; + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); + this.runConformTest( + new String[] { + "X.java", + "import java.io.PrintWriter;\n" + + "import java.io.BufferedWriter;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " PrintWriter writer = new PrintWriter(\"filename\");\n" + + " try (BufferedWriter bw = new BufferedWriter(writer)) {\n" + + " bw.write(1);\n" + + " }\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a t-w-r potentially wraps an existing resource +// DISABLED, fails because we currently don't include t-w-r managed resources in the analysis +public void _test061q() { + if (this.complianceLevel < ClassFileConstants.JDK1_7) return; + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); + this.runNegativeTest( + new String[] { + "X.java", + "import java.io.PrintWriter;\n" + + "import java.io.BufferedWriter;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo(boolean b) throws IOException {\n" + + " PrintWriter writer = new PrintWriter(\"filename\");\n" + + " if (b)\n" + + " try (BufferedWriter bw = new BufferedWriter(writer)) {\n" + + " bw.write(1);\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 6)\n" + + " PrintWriter writer = new PrintWriter(\\\"filename\\\");\n" + + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + + "Potential resource leak: \'writer\' may not be closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// the inner from a wrapper is returned +public void test061r() { + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); + this.runConformTest( + new String[] { + "X.java", + "import java.io.FileInputStream;\n" + + "import java.io.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " FileInputStream foo() throws IOException {\n" + + " File file = new File(\"somefil\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream); \n" + + " return fileStream;\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a wrapper is forgotten, the inner is closed afterwards +public void test061s() { + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); + this.runConformTest( + new String[] { + "X.java", + "import java.io.FileInputStream;\n" + + "import java.io.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " File file = new File(\"somefil\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream);\n" + + " bis = null;\n" + + " fileStream.close();\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} // Bug 362331 - Resource leak not detected when closeable not assigned to variable // a resource is never assigned public void test062a() throws IOException { @@ -2702,21 +2824,21 @@ public void test063a() throws IOException { " }\n" + "}\n" }, - "----------\n" + - "1. ERROR in X.java (at line 7)\n" + - " FileInputStream stream = new FileInputStream(file);\n" + - " ^^^^^^\n" + - "Resource leak: \'stream\' is never closed\n" + - "----------\n" + - "2. ERROR in X.java (at line 9)\n" + - " FileInputStream stream2 = new FileInputStream(file); // unsure since passed to method\n" + - " ^^^^^^^\n" + - "Potential resource leak: \'stream2\' may not be closed\n" + - "----------\n" + - "3. ERROR in X.java (at line 10)\n" + - " bis = getReader(stream2); // unsure since obtained from method\n" + - " ^^^^^^^^^^^^^^^^^^^^^^^^\n" + - "Potential resource leak: \'bis\' may not be closed\n" + + "----------\n" + + "1. ERROR in X.java (at line 7)\n" + + " FileInputStream stream = new FileInputStream(file);\n" + + " ^^^^^^\n" + + "Resource leak: \'stream\' is never closed\n" + + "----------\n" + + "2. ERROR in X.java (at line 9)\n" + + " FileInputStream stream2 = new FileInputStream(file); // unsure since passed to method\n" + + " ^^^^^^^\n" + + "Potential resource leak: \'stream2\' may not be closed\n" + + "----------\n" + + "3. ERROR in X.java (at line 10)\n" + + " bis = getReader(stream2); // unsure since obtained from method\n" + + " ^^^^^^^^^^^^^^^^^^^^^^^^\n" + + "Potential resource leak: \'bis\' may not be closed\n" + "----------\n", null, true, diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java index f5f9ac1..08cdf7e 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java @@ -46,14 +46,15 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl // process arguments if (this.arguments != null) { + boolean hasResourceWrapperType = this.resolvedType instanceof ReferenceBinding + && ((ReferenceBinding)this.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable); for (int i = 0, count = this.arguments.length; i < count; i++) { flowInfo = this.arguments[i] .analyseCode(currentScope, flowContext, flowInfo) .unconditionalInits(); // if argument is an AutoCloseable insert info that it *may* be closed (by the target method, i.e.) - if (!(this.resolvedType instanceof ReferenceBinding - && ((ReferenceBinding)this.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable))) { // allocation of wrapped closeables is analyzed specially + if (!hasResourceWrapperType) { // allocation of wrapped closeables is analyzed specially flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo); } if ((this.arguments[i].implicitConversion & TypeIds.UNBOXING) != 0) { 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 7c2121e..9f5700d 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 @@ -169,6 +169,7 @@ public class FakedTrackingVariable extends LocalDeclaration { * @param location the assignment/local declaration being analyzed * @param local the local variable being assigned to * @param rhs the rhs of the assignment resp. the initialization of the local variable declaration. + * Precondition: client has already checked that the resolved type of this expression is either a closeable type or NULL. */ public static void preConnectTrackerAcrossAssignment(ASTNode location, LocalVariableBinding local, Expression rhs) { FakedTrackingVariable closeTracker = null; @@ -615,6 +616,22 @@ public class FakedTrackingVariable extends LocalDeclaration { return trackingVar; } + /** + * If current is the same as 'returnedResource' or a wrapper thereof, + * mark as reported and return true, otherwise false. + */ + public boolean isResourceBeingReturned(FakedTrackingVariable returnedResource) { + FakedTrackingVariable current = this; + do { + if (current == returnedResource) { + this.globalClosingState |= REPORTED_DEFINITIVE_LEAK; + return true; + } + current = current.innerTracker; + } while (current != null); + return false; + } + public void recordErrorLocation(ASTNode location, int nullStatus) { if (this.recordedLocations == null) this.recordedLocations = new HashMap(); diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java index 18a1bc1..0a34079 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java @@ -52,9 +52,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl if (trackingVariable != null) { if (methodScope != trackingVariable.methodScope) trackingVariable.markClosedInNestedMethod(); - // don't report issues concerning this local, since by returning - // the method passes the responsibility to the caller: - currentScope.removeTrackingVar(trackingVariable); + // by returning the method passes the responsibility to the caller: + flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.expression, flowInfo); } } this.initStateIndex = diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java index 30c8198..33c3b6f 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java @@ -1009,11 +1009,19 @@ public void checkUnclosedCloseables(FlowInfo flowInfo, ASTNode location, BlockSc return; } if (location != null && flowInfo.reachMode() != 0) return; + + FakedTrackingVariable returnVar = (location instanceof ReturnStatement) ? + FakedTrackingVariable.getCloseTrackingVariable(((ReturnStatement)location).expression) : null; + Set varSet = new HashSet(this.trackingVariables); FakedTrackingVariable trackingVar; // pick one outer-most variable from the set at a time while ((trackingVar = FakedTrackingVariable.pickVarForReporting(varSet, this, location != null)) != null) { + if (returnVar != null && trackingVar.isResourceBeingReturned(returnVar)) { + continue; + } + if (location != null && trackingVar.originalBinding != null && flowInfo.isDefinitelyNull(trackingVar.originalBinding)) continue; // reporting against a specific location, resource is null at this flow, don't complain