diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryWithResourcesStatementTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryWithResourcesStatementTest.java index f21a57f..9ec10f6 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryWithResourcesStatementTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryWithResourcesStatementTest.java @@ -14,8 +14,13 @@ *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; +import java.io.IOException; +import java.net.URL; import java.util.Map; +import org.eclipse.core.runtime.FileLocator; +import org.eclipse.core.runtime.Path; +import org.eclipse.core.runtime.Platform; import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; @@ -23,7 +28,7 @@ public class TryWithResourcesStatementTest extends AbstractRegressionTest { static { -// TESTS_NAMES = new String[] { "test056throw"}; +// TESTS_NAMES = new String[] { "test063c"}; // TESTS_NUMBERS = new int[] { 50 }; // TESTS_RANGE = new int[] { 11, -1 }; } @@ -3713,11 +3718,12 @@ options); } // Bug 349326 - [1.7] new warning for missing try-with-resources +// Bug 362332 - Only report potential leak when closeable not created in the local scope // one method returns an AutoCleasble, a second method uses this object without ever closing it. public void test056e() { Map options = getCompilerOptions(); options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); - options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.WARNING); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); this.runNegativeTest( new String[] { "X.java", @@ -3744,7 +3750,7 @@ "1. ERROR in X.java (at line 11)\n" + " FileReader reader = getReader(\"somefile\");\n" + " ^^^^^^\n" + - "Resource leak: 'reader' is never closed\n" + + "Potential resource leak: \'reader\' may not be closed\n" + "----------\n", null, true, @@ -4358,11 +4364,12 @@ null/*requestor*/); } // Bug 349326 - [1.7] new warning for missing try-with-resources +// Bug 362332 - Only report potential leak when closeable not created in the local scope // a method uses an AutoCloseable without ever closing it, type from a type variable public void test056p() { Map options = getCompilerOptions(); options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); - options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.WARNING); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); this.runNegativeTest( new String[] { "X.java", @@ -4389,7 +4396,7 @@ "1. ERROR in X.java (at line 8)\n" + " T fileReader = newReader(file);\n" + " ^^^^^^^^^^\n" + - "Resource leak: 'fileReader' is never closed\n" + + "Potential resource leak: \'fileReader\' may not be closed\n" + "----------\n", null, true, @@ -4444,7 +4451,7 @@ options); } // Bug 349326 - [1.7] new warning for missing try-with-resources -// closed in dead code +// properly closed, dead code in between public void test056r() { Map options = getCompilerOptions(); options.put(JavaCore.COMPILER_PB_UNCLOSED_CLOSEABLE, CompilerOptions.ERROR); @@ -5354,7 +5361,726 @@ "X::~X\n" + "true"); } - +// Bug 358903 - Filter practically unimportant resource leak warnings +// Bug 360908 - Avoid resource leak warning when the underlying/chained resource is closed explicitly +// a resource wrapper is not closed but the underlying resource is +public void test061a() { + 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.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream);\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " System.out.println(bis.available());\n" + + " fileStream.close();\n" + + " }\n" + + " void inline() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " FileInputStream fileStream;\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream = new FileInputStream(file));\n" + + " System.out.println(bis.available());\n" + + " fileStream.close();\n" + + " }\n" + + " public static void main(String[] args) throws IOException {\n" + + " try {\n" + + " new X().foo();\n" + + " } catch (IOException ex) {" + + " System.out.println(\"Got IO Exception\");\n" + + " }\n" + + " }\n" + + "}\n" + }, + "Got IO Exception", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a closeable without OS resource is not closed +public void test061b() { + 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.StringReader;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " StringReader string = new StringReader(\"content\");\n" + + " System.out.println(string.read());\n" + + " }\n" + + " public static void main(String[] args) throws IOException {\n" + + " new X().foo();\n" + + " }\n" + + "}\n" + }, + "99", // character 'c' + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a resource wrapper is not closed but the underlying closeable is resource-free +public void test061c() { + 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.BufferedReader;\n" + + "import java.io.StringReader;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " StringReader input = new StringReader(\"content\");\n" + + " BufferedReader br = new BufferedReader(input);\n" + + " BufferedReader doubleWrap = new BufferedReader(br);\n" + + " System.out.println(br.read());\n" + + " }\n" + + " void inline() throws IOException {\n" + + " BufferedReader br = new BufferedReader(new StringReader(\"content\"));\n" + + " System.out.println(br.read());\n" + + " }\n" + + " public static void main(String[] args) throws IOException {\n" + + " new X().foo();\n" + + " }\n" + + "}\n" + }, + "99", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a resource wrapper is not closed neither is the underlying resource +public void test061d() { + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.WARNING); + this.runNegativeTest( + new String[] { + "X.java", + "import java.io.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream);\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " System.out.println(bis.available());\n" + + " }\n" + + " void inline() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " BufferedInputStream bis2 = new BufferedInputStream(new FileInputStream(file));\n" + + " System.out.println(bis2.available());\n" + + " }\n" + + " public static void main(String[] args) throws IOException {\n" + + " try {\n" + + " new X().foo();\n" + + " } catch (IOException ex) {" + + " System.out.println(\"Got IO Exception\");\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 10)\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " ^^^^^^^^^^\n" + + "Resource leak: \'doubleWrap\' is never closed\n" + + "----------\n" + + "2. ERROR in X.java (at line 15)\n" + + " BufferedInputStream bis2 = new BufferedInputStream(new FileInputStream(file));\n" + + " ^^^^\n" + + "Resource leak: \'bis2\' is never closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// Bug 361073 - Avoid resource leak warning when the top level resource is closed explicitly +// a resource wrapper is closed closing also the underlying resource +public void test061e() { + 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.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " FileInputStream fis;" + + " void foo() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream);\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " System.out.println(bis.available());\n" + + " bis.close();\n" + + " }\n" + + " void inline() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " BufferedInputStream bis2 = new BufferedInputStream(fis = new FileInputStream(file));\n" + // field assignment + " System.out.println(bis2.available());\n" + + " bis2.close();\n" + + " FileInputStream fileStream = null;\n" + + " BufferedInputStream bis3 = new BufferedInputStream(fileStream = new FileInputStream(file));\n" + + " System.out.println(bis3.available());\n" + + " bis3.close();\n" + + " }\n" + + " public static void main(String[] args) throws IOException {\n" + + " try {\n" + + " new X().foo();\n" + + " } catch (IOException ex) {" + + " System.out.println(\"Got IO Exception\");\n" + + " }\n" + + " }\n" + + "}\n" + }, + "Got IO Exception", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// Bug 361073 - Avoid resource leak warning when the top level resource is closed explicitly +// a resource wrapper is closed closing also the underlying resource - original test case +public void test061f() throws IOException { + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); + URL url = FileLocator.toFileURL(FileLocator.find(Platform.getBundle("org.eclipse.jdt.core.tests.compiler"), new Path("META-INF/MANIFEST.MF"), null)); + this.runConformTest( + new String[] { + "X.java", + "import java.io.InputStream;\n" + + "import java.io.InputStreamReader;\n" + + "import java.io.BufferedReader;\n" + + "import java.io.IOException;\n" + + "import java.net.URL;\n" + + "public class X {\n" + + " boolean loadURL(final URL url) throws IOException {\n" + + " InputStream stream = null;\n" + + " BufferedReader reader = null;\n" + + " try {\n" + + " stream = url.openStream();\n" + + " reader = new BufferedReader(new InputStreamReader(stream));\n" + + " System.out.println(reader.readLine());\n" + + " } finally {\n" + + " try {\n" + + " if (reader != null)\n" + + " reader.close();\n" + + " } catch (IOException x) {\n" + + " }\n" + + " }\n" + + " return false; // 'stream' may not be closed at this location\n" + + " }\n" + + " public static void main(String[] args) throws IOException {\n" + + " try {\n" + + " new X().loadURL(new URL(\""+url.toString()+"\"));\n" + + " } catch (IOException ex) {\n" + + " System.out.println(\"Got IO Exception\"+ex);\n" + + " }\n" + + " }\n" + + "}\n" + }, + "Manifest-Version: 1.0", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// Bug 360908 - Avoid resource leak warning when the underlying/chained resource is closed explicitly +// Different points in a resource chain are closed +public void test061g() { + 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.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void closeMiddle() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream);\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " System.out.println(bis.available());\n" + + " bis.close();\n" + + " }\n" + + " void closeOuter() throws IOException {\n" + + " File file2 = new File(\"somefile\");\n" + + " FileInputStream fileStream2 = new FileInputStream(file2);\n" + + " BufferedInputStream bis2 = new BufferedInputStream(fileStream2);\n" + + " BufferedInputStream doubleWrap2 = new BufferedInputStream(bis2);\n" + + " System.out.println(bis2.available());\n" + + " doubleWrap2.close();\n" + + " }\n" + + " void neverClosed() throws IOException {\n" + + " File file3 = new File(\"somefile\");\n" + + " FileInputStream fileStream3 = new FileInputStream(file3);\n" + + " BufferedInputStream bis3 = new BufferedInputStream(fileStream3);\n" + + " BufferedInputStream doubleWrap3 = new BufferedInputStream(bis3);\n" + + " System.out.println(doubleWrap3.available());\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 26)\n" + + " BufferedInputStream doubleWrap3 = new BufferedInputStream(bis3);\n" + + " ^^^^^^^^^^^\n" + + "Resource leak: \'doubleWrap3\' is never closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// Bug 360908 - Avoid resource leak warning when the underlying/chained resource is closed explicitly +// Different points in a resource chain are potentially closed +public void test061h() { + 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.File;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void closeMiddle(boolean b) throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " FileInputStream fileStream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(fileStream);\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " System.out.println(bis.available());\n" + + " if (b)\n" + + " bis.close();\n" + + " }\n" + + " void closeOuter(boolean b) throws IOException {\n" + + " File file2 = new File(\"somefile\");\n" + + " FileInputStream fileStream2 = new FileInputStream(file2);\n" + + " BufferedInputStream dummy;\n" + + " BufferedInputStream bis2 = (dummy = new BufferedInputStream(fileStream2));\n" + + " BufferedInputStream doubleWrap2 = new BufferedInputStream(bis2);\n" + + " System.out.println(bis2.available());\n" + + " if (b)\n" + + " doubleWrap2.close();\n" + + " }\n" + + " void potAndDef(boolean b) throws IOException {\n" + + " File file3 = new File(\"somefile\");\n" + + " FileInputStream fileStream3 = new FileInputStream(file3);\n" + + " BufferedInputStream bis3 = new BufferedInputStream(fileStream3);\n" + + " BufferedInputStream doubleWrap3 = new BufferedInputStream(bis3);\n" + + " System.out.println(doubleWrap3.available());\n" + + " if (b) bis3.close();\n" + + " fileStream3.close();\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 10)\n" + + " BufferedInputStream doubleWrap = new BufferedInputStream(bis);\n" + + " ^^^^^^^^^^\n" + + "Potential resource leak: \'doubleWrap\' may not be closed\n" + + "----------\n" + + "2. ERROR in X.java (at line 20)\n" + + " BufferedInputStream doubleWrap2 = new BufferedInputStream(bis2);\n" + + " ^^^^^^^^^^^\n" + + "Potential resource leak: \'doubleWrap2\' may not be closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// local var is re-used for two levels of wrappers +public void test061i() { + 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.File;\n" + + "import java.io.InputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void closeMiddle() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " InputStream stream = new FileInputStream(file);\n" + + " stream = new BufferedInputStream(stream);\n" + + " InputStream middle;\n" + + " stream = new BufferedInputStream(middle = stream);\n" + + " System.out.println(stream.available());\n" + + " middle.close();\n" + + " }\n" + + " void closeOuter() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " InputStream stream2 = new FileInputStream(file);\n" + + " stream2 = new BufferedInputStream(stream2);\n" + + " stream2 = new BufferedInputStream(stream2);\n" + + " System.out.println(stream2.available());\n" + + " stream2.close();\n" + + " }\n" + + " void neverClosed() throws IOException {\n" + + " File file = new File(\"somefile\");\n" + + " InputStream stream3 = new FileInputStream(file);\n" + + " stream3 = new BufferedInputStream(stream3);\n" + + " stream3 = new BufferedInputStream(stream3);\n" + + " System.out.println(stream3.available());\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 26)\n" + + " InputStream stream3 = new FileInputStream(file);\n" + + " ^^^^^^^\n" + + "Resource leak: \'stream3\' is never closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// self-wrapping a method argument (caused NPE UnconditionalFlowInfo.markAsDefinitelyNull(..)). +public void test061j() { + 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.InputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo(InputStream stream) throws IOException {\n" + + " stream = new BufferedInputStream(stream);\n" + + " System.out.println(stream.available());\n" + + " stream.close();\n" + + " }\n" + + " void boo(InputStream stream2) throws IOException {\n" + + " stream2 = new BufferedInputStream(stream2);\n" + + " System.out.println(stream2.available());\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a wrapper is created in a return statement +public void test061k() throws IOException { + 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.File;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " BufferedInputStream getReader(File file) throws IOException {\n" + + " FileInputStream stream = new FileInputStream(file);\n" + + " return new BufferedInputStream(stream);\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} +// Bug 358903 - Filter practically unimportant resource leak warnings +// a closeable is assigned to a field +public void test061l() throws IOException { + 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.File;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " BufferedInputStream stream;\n" + + " void foo(File file) throws IOException {\n" + + " FileInputStream s = new FileInputStream(file);\n" + + " stream = new BufferedInputStream(s);\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 { + 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.File;\n" + + "import java.io.FileOutputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " new FileOutputStream(new File(\"C:\\temp\\foo.txt\")).write(1);\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 6)\n" + + " new FileOutputStream(new File(\"C:\\temp\\foo.txt\")).write(1);\n" + + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + + "Resource leak: \'\' is never closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 362331 - Resource leak not detected when closeable not assigned to variable +// a freshly allocated resource is immediately closed +public void test062b() throws IOException { + 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.File;\n" + + "import java.io.FileOutputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " new FileOutputStream(new File(\"C:\\temp\\foo.txt\")).close();\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} +// Bug 362331 - Resource leak not detected when closeable not assigned to variable +// a resource is directly passed to another method +public void test062c() throws IOException { + 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.File;\n" + + "import java.io.FileOutputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " writeIt(new FileOutputStream(new File(\"C:\\temp\\foo.txt\")));\n" + + " }\n" + + " void writeIt(FileOutputStream fos) throws IOException {\n" + + " fos.write(1);\n" + + " fos.close();\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} +// Bug 362331 - Resource leak not detected when closeable not assigned to variable +// a resource is not used +public void test062d() throws IOException { + 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.File;\n" + + "import java.io.FileOutputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo() throws IOException {\n" + + " new FileOutputStream(new File(\"C:\\temp\\foo.txt\"));\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 6)\n" + + " new FileOutputStream(new File(\"C:\\temp\\foo.txt\"));\n" + + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + + "Resource leak: \'\' is never closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 362332 - Only report potential leak when closeable not created in the local scope +// a wrapper is obtained from another method +public void test063a() throws IOException { + 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.File;\n" + + "import java.io.FileInputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void read(File file) throws IOException {\n" + + " FileInputStream stream = new FileInputStream(file);\n" + + " BufferedInputStream bis = new BufferedInputStream(stream); // never since reassigned\n" + + " FileInputStream stream2 = new FileInputStream(file); // unsure since passed to method\n" + + " bis = getReader(stream2); // unsure since obtained from method\n" + + " bis.available();\n" + + " }\n" + + " BufferedInputStream getReader(FileInputStream stream) throws IOException {\n" + + " return new BufferedInputStream(stream);\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 8)\n" + + " BufferedInputStream bis = new BufferedInputStream(stream); // never since reassigned\n" + + " ^^^\n" + + "Resource leak: \'bis\' 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, + options); +} +// Bug 362332 - Only report potential leak when closeable not created in the local scope +// a wrapper is obtained from a field read +public void test063b() throws IOException { + 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.FileInputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " FileInputStream stream;\n" + + " void read() throws IOException {\n" + + " FileInputStream s = this.stream;\n" + + " BufferedInputStream bis = new BufferedInputStream(s); // unsure since s is obtained from a field\n" + + " bis.available();\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 8)\n" + + " BufferedInputStream bis = new BufferedInputStream(s); // unsure since s is obtained from a field\n" + + " ^^^\n" + + "Potential resource leak: \'bis\' may not be closed\n" + + "----------\n", + null, + true, + options); +} +// Bug 362332 - Only report potential leak when closeable not created in the local scope +// a wrapper is assigned to a field +public void test063c() throws IOException { + 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.BufferedInputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " BufferedInputStream stream;\n" + + " void read() throws IOException {\n" + + " FileInputStream s = new FileInputStream(\"somefile\");\n" + + " BufferedInputStream bis = new BufferedInputStream(s);\n" + + " this.stream = bis;\n" + + " }\n" + + "}\n" + }, + "", + null, + true, + null, + options, + null); +} public static Class testClass() { return TryWithResourcesStatementTest.class; } 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 e811ea9..20b96f4 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -37,6 +37,8 @@ protected TypeBinding typeExpected; // for <> inference public boolean inferredReturnType; + public FakedTrackingVariable closeTracker; // when allocation a Closeable store a pre-liminary tracking variable here + public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) { // check captured variables are initialized in current context (26134) checkCapturedLocalInitializationIfNecessary((ReferenceBinding)this.binding.declaringClass.erasure(), currentScope, flowInfo); @@ -44,18 +46,22 @@ // process arguments if (this.arguments != null) { for (int i = 0, count = this.arguments.length; i < count; i++) { - // if argument is an AutoCloseable insert info that it *may* be closed (by the target method, i.e.) - flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo); 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.) + flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo, this.resolvedType); if ((this.arguments[i].implicitConversion & TypeIds.UNBOXING) != 0) { this.arguments[i].checkNPE(currentScope, flowContext, flowInfo); } } analyseArguments(currentScope, flowContext, flowInfo, this.binding, this.arguments); } + + if (FakedTrackingVariable.isAutoCloseable(this.resolvedType)) + FakedTrackingVariable.analyseCloseableAllocation(currentScope, flowInfo, this); + // record some dependency information for exception types ReferenceBinding[] thrownExceptions; if (((thrownExceptions = this.binding.thrownExceptions).length) != 0) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java index 970fc24..97464a1 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -46,19 +46,27 @@ if ((this.expression.implicitConversion & TypeIds.UNBOXING) != 0) { this.expression.checkNPE(currentScope, flowContext, flowInfo); } + + FlowInfo preInitInfo = null; + boolean shouldAnalyseResource = local != null + && flowInfo.reachMode() == FlowInfo.REACHABLE + && (FakedTrackingVariable.isAutoCloseable(this.expression.resolvedType) + || this.expression.resolvedType == TypeBinding.NULL); + if (shouldAnalyseResource) { + preInitInfo = flowInfo.unconditionalCopy(); + // analysis of resource leaks needs additional context while analyzing the RHS: + FakedTrackingVariable.preConnectTrackerAcrossAssignment(this, local, this.expression); + } + flowInfo = ((Reference) this.lhs) .analyseAssignment(currentScope, flowContext, flowInfo, this, false) .unconditionalInits(); - if (local != null) { - LocalVariableBinding previousTrackerBinding = null; - if (local.closeTracker != null) { - // Assigning to a variable already holding an AutoCloseable, has it been closed before? - previousTrackerBinding = local.closeTracker.binding; - if (!flowInfo.isDefinitelyNull(local)) // only if previous value may be non-null - local.closeTracker.recordErrorLocation(this, flowInfo.nullStatus(previousTrackerBinding)); - } - FakedTrackingVariable.handleResourceAssignment(flowInfo, this, this.expression, local, previousTrackerBinding); - } + + if (shouldAnalyseResource) + FakedTrackingVariable.handleResourceAssignment(preInitInfo, flowInfo, this, this.expression, local); + else + FakedTrackingVariable.cleanUpAfterAssignment(currentScope, this.lhs.bits, this.expression); + int nullStatus = this.expression.nullStatus(flowInfo); if (local != null && (local.type.tagBits & TagBits.IsBaseType) == 0) { if (nullStatus == FlowInfo.NULL) { 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 ee13047..c4579b1 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 @@ -15,7 +15,6 @@ import java.util.Map; import java.util.Map.Entry; -import org.eclipse.jdt.core.compiler.CharOperation; import org.eclipse.jdt.internal.compiler.codegen.CodeStream; import org.eclipse.jdt.internal.compiler.flow.FlowContext; import org.eclipse.jdt.internal.compiler.flow.FlowInfo; @@ -48,20 +47,28 @@ private static final int CLOSED_IN_NESTED_METHOD = 4; // a location independent issue has been reported already against this resource: private static final int REPORTED = 8; - + // a resource is wrapped in another resource: + private static final int WRAPPED = 16; + + private static final int DOUBT_MASK = CLOSE_SEEN | PASSED_TO_OUTSIDE | CLOSED_IN_NESTED_METHOD | REPORTED; // not WRAPPED + /** - * Bitset of {@link #CLOSE_SEEN}, {@link #PASSED_TO_OUTSIDE}, {@link #CLOSED_IN_NESTED_METHOD} and {@link #REPORTED}. + * Bitset of {@link #CLOSE_SEEN}, {@link #PASSED_TO_OUTSIDE}, {@link #CLOSED_IN_NESTED_METHOD}, {@link #REPORTED} and {@link #WRAPPED}. */ private int globalClosingState = 0; + public LocalVariableBinding originalBinding; // the real local being tracked, can be null for preliminary track vars for allocation expressions + + public FakedTrackingVariable innerTracker; // chained tracking variable of a chained (wrapped) resource + MethodScope methodScope; // designates the method declaring this variable - - public LocalVariableBinding originalBinding; // the real local being tracked - - HashMap recordedLocations; // initially null, ASTNode -> Integer + private HashMap recordedLocations; // initially null, ASTNode -> Integer - public FakedTrackingVariable(LocalVariableBinding original, Statement location) { + // temporary storage while analyzing "res = new Res();": + private ASTNode currentAssignment; // temporarily store the assignment as the location for error reporting + + public FakedTrackingVariable(LocalVariableBinding original, ASTNode location) { super(original.name, location.sourceStart, location.sourceEnd); this.type = new SingleTypeReference( TypeConstants.OBJECT, @@ -69,6 +76,17 @@ this.methodScope = original.declaringScope.methodScope(); this.originalBinding = original; resolve(original.declaringScope); + } + + /* Create an unassigned tracking variable while analyzing an allocation expression: */ + private FakedTrackingVariable(BlockScope scope, ASTNode location) { + super("".toCharArray(), location.sourceStart, location.sourceEnd); //$NON-NLS-1$ + this.type = new SingleTypeReference( + TypeConstants.OBJECT, + ((long)this.sourceStart <<32)+this.sourceEnd); + this.methodScope = scope.methodScope(); + this.originalBinding = null; + resolve(scope); } public void generateCode(BlockScope currentScope, CodeStream codeStream) @@ -88,8 +106,12 @@ } /** - * If expression resolves to a local variable binding of type AutoCloseable, - * answer the variable that tracks closing of that local, creating it if needed. + * If expression resolves to a value of type AutoCloseable answer the variable that tracks closing of that local. + * Covers two cases: + *
    + *
  • value is a local variable reference, create tracking variable it if needed. + *
  • value is an allocation expression, return a preliminary tracking variable if set. + *
* @param expression * @return a new {@link FakedTrackingVariable} or null. */ @@ -107,51 +129,248 @@ Statement location = local.declaration; return local.closeTracker = new FakedTrackingVariable(local, location); } - } + } else if (expression instanceof AllocationExpression) { + // return any preliminary tracking variable from analyseCloseableAllocation + return ((AllocationExpression) expression).closeTracker; + } return null; } - /** if 'invocationSite' is a call to close() that has a registered tracking variable, answer that variable's binding. */ - public static LocalVariableBinding getTrackerForCloseCall(ASTNode invocationSite) { - if (invocationSite instanceof MessageSend) { - MessageSend send = (MessageSend) invocationSite; - if (CharOperation.equals(TypeConstants.CLOSE, send.selector) && send.receiver instanceof SingleNameReference) { - Binding receiverBinding = ((SingleNameReference)send.receiver).binding; - if (receiverBinding instanceof LocalVariableBinding) { - FakedTrackingVariable trackingVariable = ((LocalVariableBinding)receiverBinding).closeTracker; - if (trackingVariable != null) - return trackingVariable.binding; + /** + * Before analyzing an assignment of this shape: singleName = new Allocation() + * connect any tracking variable of the LHS with the allocation on the RHS. + * Also the assignment is temporarily stored in the tracking variable in case we need to + * report errors because the assignment leaves the old LHS value unclosed. + * In this case the assignment should be used as the error location. + * + * @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. + * @return a tracking variable with temporarily set 'currentAssignment' or null. + */ + public static FakedTrackingVariable preConnectTrackerAcrossAssignment(ASTNode location, LocalVariableBinding local, Expression rhs) { + FakedTrackingVariable closeTracker = null; + if (rhs instanceof AllocationExpression) { + closeTracker = local.closeTracker; + if (closeTracker == null) { + if (isAutoCloseable(rhs.resolvedType)) { + closeTracker = new FakedTrackingVariable(local, location); + } + } + if (closeTracker != null) { + closeTracker.currentAssignment = location; + ((AllocationExpression)rhs).closeTracker = closeTracker; + } + } + return closeTracker; + } + + /** Compute/assign a tracking variable for a freshly allocated closeable value, using information from our white lists. */ + public static void analyseCloseableAllocation(BlockScope scope, FlowInfo flowInfo, AllocationExpression allocation) { + if (((ReferenceBinding)allocation.resolvedType).hasTypeBit(TypeIds.BitResourceFreeCloseable)) { + // remove unnecessary attempts (closeable is not relevant) + if (allocation.closeTracker != null) { + scope.removeTrackingVar(allocation.closeTracker); + allocation.closeTracker = null; + } + } else if (((ReferenceBinding)allocation.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable)) { + if (allocation.arguments != null && allocation.arguments.length > 0) { + FakedTrackingVariable innerTracker = analyseCloseableAllocationArgument(scope, flowInfo, allocation, allocation.arguments[0]); + if (innerTracker != null) { + if (innerTracker == allocation.closeTracker) + return; // self wrap -> neither change (here) nor remove (below) + if (allocation.closeTracker == null) { + allocation.closeTracker = new FakedTrackingVariable(scope, allocation); // no local available, closeable is unassigned + } + allocation.closeTracker.innerTracker = innerTracker; + innerTracker.globalClosingState |= WRAPPED; + flowInfo.markAsDefinitelyNull(allocation.closeTracker.binding); + return; // keep chaining wrapper } } + // remove unnecessary attempts (wrapper has no relevant inner) + if (allocation.closeTracker != null) { + scope.removeTrackingVar(allocation.closeTracker); + allocation.closeTracker = null; + } + } else { + FakedTrackingVariable presetTracker = allocation.closeTracker; + if (presetTracker != null && presetTracker.originalBinding != null) { + int closeStatus = flowInfo.nullStatus(presetTracker.binding); + if (closeStatus != FlowInfo.NON_NULL + && !flowInfo.isDefinitelyNull(presetTracker.originalBinding) + && !(presetTracker.currentAssignment instanceof LocalDeclaration)) + allocation.closeTracker.recordErrorLocation(presetTracker.currentAssignment, closeStatus); + } else { + allocation.closeTracker = new FakedTrackingVariable(scope, allocation); // no local available, closeable is unassigned + } + flowInfo.markAsDefinitelyNull(allocation.closeTracker.binding); + } + } + + /** Analyze the argument of an allocation for a resource wrapper. */ + public static FakedTrackingVariable analyseCloseableAllocationArgument(BlockScope scope, FlowInfo flowInfo, AllocationExpression allocation, Expression arg) + { + if (arg instanceof Assignment) { + Assignment assign = (Assignment)arg; + LocalVariableBinding innerLocal = assign.localVariableBinding(); + if (innerLocal != null) { + // nested assignment has already been processed + return innerLocal.closeTracker; // FIXME do we need a nested tracker here? see test061a/e + } else { + arg = assign.expression; // unwrap assignment and fall through + } + } + if (arg instanceof SingleNameReference) { + SingleNameReference ref = (SingleNameReference) arg; + if (ref.binding instanceof LocalVariableBinding) { + // allocation arg is a reference to an existing closeable? + LocalVariableBinding innerLocal = (LocalVariableBinding)ref.binding; + if (innerLocal.closeTracker != null) + return innerLocal.closeTracker; + if (innerLocal.isParameter() && isAutoCloseable(innerLocal.type)) { + FakedTrackingVariable tracker = new FakedTrackingVariable(innerLocal, allocation); + tracker.globalClosingState = PASSED_TO_OUTSIDE; + flowInfo.markAsDefinitelyNonNull(tracker.binding); // assume we're not responsible for parameter value + return tracker; + } + } + } else if (arg instanceof AllocationExpression && arg.resolvedType instanceof ReferenceBinding) { + // nested allocation + return ((AllocationExpression)arg).closeTracker; } return null; } /** * Check if the rhs of an assignment or local declaration is an (Auto)Closeable. - * If so create or re-use a tracking variable, and wire and initialize everything. + * If so create or re-use a tracking variable, and wire and initialize everything. + * @param upstreamInfo info without analysis of the rhs, use this to determine the status of a resource being disconnected + * @param flowInfo info with analysis of the rhs, use this for recording resource status because this will be passed downstream + * @param location where to report warnigs/errors against + * @param rhs the right hand side of the assignment, this expression is to be analyzed. + * The caller has already checked that the rhs is either of a closeable type or null. + * @param local the local variable into which the rhs is being assigned */ - public static void handleResourceAssignment(FlowInfo flowInfo, Statement location, Expression rhs, LocalVariableBinding local, - LocalVariableBinding previousTrackerBinding) + public static void handleResourceAssignment(FlowInfo upstreamInfo, FlowInfo flowInfo, ASTNode location, Expression rhs, LocalVariableBinding local) { - if (isAutoCloseable(rhs.resolvedType)) { + // does the LHS (local) already have a tracker, indicating we may leak a resource by the assignment? + FakedTrackingVariable previousTracker = null; + FakedTrackingVariable disconnectedTracker = null; + if (local.closeTracker != null) { + // assigning to a variable already holding an AutoCloseable, has it been closed before? + previousTracker = local.closeTracker; + int status = upstreamInfo.nullStatus(local); + if (status != FlowInfo.NULL && status != FlowInfo.UNKNOWN) // only if previous value may be relevant + disconnectedTracker = previousTracker; // report error below, unless we have a self-wrap assignment + } + + if (rhs.resolvedType != TypeBinding.NULL) { // new value is AutoCloseable, start tracking, possibly re-using existing tracker var: - FakedTrackingVariable rhsTrackVar = getCloseTrackingVariable(rhs); - if (rhsTrackVar != null) { // 1. share tracking variable with RHS? - local.closeTracker = rhsTrackVar; - // keep null-status unchanged across this assignment - } else if (previousTrackerBinding != null) { // 2. re-use tracking variable from the LHS? - // re-assigning from a fresh, mark as not-closed again: - flowInfo.markAsDefinitelyNull(previousTrackerBinding); + if (rhsTrackVar != null) { // 1. if RHS has a tracking variable... + if (local.closeTracker == null) { + // null shouldn't occur but let's play safe + if (rhsTrackVar.originalBinding != null) + local.closeTracker = rhsTrackVar; // a.: let fresh LHS share it + } else { + if (rhsTrackVar == disconnectedTracker && rhs instanceof AllocationExpression) + return; // b.: self wrapper: res = new Wrap(res); -> done! + local.closeTracker = rhsTrackVar; // c.: conflicting LHS and RHS, proceed with recordErrorLocation below + } + // keep close-status of RHS unchanged across this assignment + } else if (previousTracker != null) { // 2. re-use tracking variable from the LHS? + // re-assigning from a fresh value, mark as not-closed again: + flowInfo.markAsDefinitelyNull(previousTracker.binding); + local.closeTracker = analyseCloseableExpression(flowInfo, local, location, rhs, previousTracker); } else { // 3. no re-use, create a fresh tracking variable: - local.closeTracker = new FakedTrackingVariable(local, location); - // a fresh resource, mark as not-closed: - flowInfo.markAsDefinitelyNull(local.closeTracker.binding); + rhsTrackVar = analyseCloseableExpression(flowInfo, local, location, rhs, null); + if (rhsTrackVar != null) { + local.closeTracker = rhsTrackVar; + // a fresh resource, mark as not-closed: + if ((rhsTrackVar.globalClosingState & PASSED_TO_OUTSIDE) == 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) -// flowContext.initsOnFinally.markAsDefinitelyNonNull(trackerBinding); +// if (flowContext.initsOnFinally != null) +// flowContext.initsOnFinally.markAsDefinitelyNonNull(trackerBinding); + } } + } + + if (disconnectedTracker != null) + disconnectedTracker.recordErrorLocation(location, upstreamInfo.nullStatus(disconnectedTracker.binding)); + } + /** + * Analyze structure of a closeable expression, matching (chained) resources against our white lists. + * See Bug 358903 - Filter practically unimportant resource leak warnings + * @param flowInfo where to record close status + * @param local local variable to which the closeable is being assigned + * @param location where to flag errors/warnings against + * @param expression expression to be analyzed + * @param previousTracker when analyzing a re-assignment we may already have a tracking variable for local, + * which we should then re-use + * @return a tracking variable associated with local or null if no need to track + */ + private static FakedTrackingVariable analyseCloseableExpression(FlowInfo flowInfo, LocalVariableBinding local, + ASTNode location, Expression expression, FakedTrackingVariable previousTracker) + { + if (expression instanceof AllocationExpression) { + // allocation expressions already have their tracking variables analyzed by analyseCloseableAllocation(..) + FakedTrackingVariable tracker = ((AllocationExpression) expression).closeTracker; + if (tracker != null && tracker.originalBinding == null) { + // tracker without original binding (unassigned closeable) shouldn't reach here but let's play safe + return null; + } + return tracker; + } + if (expression.resolvedType instanceof ReferenceBinding) { + ReferenceBinding resourceType = (ReferenceBinding) expression.resolvedType; + + if (expression instanceof CastExpression) + expression = ((CastExpression) expression).expression; + + if (expression instanceof Assignment) { + Expression innerExpression = ((Assignment)expression).expression; + if (innerExpression instanceof AllocationExpression) { + // allocation inside assignment has already been analyzed fully + return ((AllocationExpression) innerExpression).closeTracker; + } + } else if (expression instanceof MessageSend + || expression instanceof FieldReference + || expression instanceof QualifiedNameReference + || expression instanceof ArrayReference) + { + FakedTrackingVariable tracker = new FakedTrackingVariable(local, location); + tracker.globalClosingState |= PASSED_TO_OUTSIDE; + flowInfo.markPotentiallyNullBit(tracker.binding); + return tracker; + } + if (resourceType.hasTypeBit(TypeIds.BitResourceFreeCloseable)) { + // (a) resource-free closeable: -> null + return null; + } + } + if (local.closeTracker != null) + // (c): inner has already been analyzed: -> re-use track var + return local.closeTracker; + return new FakedTrackingVariable(local, location); + } + + public static void cleanUpAfterAssignment(BlockScope currentScope, int lhsBits, Expression expression) { + // remove all remaining track vars with no original binding + while (expression instanceof Assignment) + expression = ((Assignment) expression).expression; + if (expression instanceof AllocationExpression) { + FakedTrackingVariable tracker = ((AllocationExpression) expression).closeTracker; + if (tracker != null && tracker.originalBinding == null) { + currentScope.removeTrackingVar(tracker); + ((AllocationExpression) expression).closeTracker = null; + } + } else { + // assignment passing a local into a field? + LocalVariableBinding local = expression.localVariableBinding(); + if (local != null && local.closeTracker != null && ((lhsBits & Binding.FIELD) != 0)) + currentScope.removeTrackingVar(local.closeTracker); } } @@ -159,6 +378,76 @@ public static boolean isAutoCloseable(TypeBinding typeBinding) { return typeBinding instanceof ReferenceBinding && ((ReferenceBinding)typeBinding).hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable); + } + + public int findMostSpecificStatus(FlowInfo flowInfo, BlockScope currentScope, BlockScope locationScope) { + int status = FlowInfo.UNKNOWN; + FakedTrackingVariable currentTracker = this; + // loop as to consider wrappers (per white list) encapsulating an inner resource. + while (currentTracker != null) { + LocalVariableBinding currentVar = currentTracker.binding; + int currentStatus = getNullStatusAggressively(currentVar, flowInfo); + if (locationScope != null) // only check at method exit points + currentStatus = mergeCloseStatus(locationScope, currentStatus, currentVar, currentScope); + if (currentStatus == FlowInfo.NON_NULL) { + status = currentStatus; + break; // closed -> stop searching + } else if (status == FlowInfo.NULL || status == FlowInfo.UNKNOWN) { + status = currentStatus; // improved although not yet safe -> keep searching for better + } + currentTracker = currentTracker.innerTracker; + } + return status; + } + + /** + * Get the null status looking even into unreachable flows + * @param local + * @param flowInfo + * @return one of the constants FlowInfo.{NULL,POTENTIALLY_NULL,POTENTIALLY_NON_NULL,NON_NULL}. + */ + private int getNullStatusAggressively(LocalVariableBinding local, FlowInfo flowInfo) { + int reachMode = flowInfo.reachMode(); + int status = 0; + try { + // unreachable flowInfo is too shy in reporting null-issues, temporarily forget reachability: + if (reachMode != FlowInfo.REACHABLE) + flowInfo.tagBits &= ~FlowInfo.UNREACHABLE; + status = flowInfo.nullStatus(local); + } finally { + // reset + flowInfo.tagBits |= reachMode; + } + // at this point some combinations are not useful so flatten to a single bit: + if ((status & FlowInfo.NULL) != 0) { + if ((status & (FlowInfo.NON_NULL | FlowInfo.POTENTIALLY_NON_NULL)) != 0) + return FlowInfo.POTENTIALLY_NULL; // null + doubt = pot null + return FlowInfo.NULL; + } else if ((status & FlowInfo.NON_NULL) != 0) { + if ((status & FlowInfo.POTENTIALLY_NULL) != 0) + return FlowInfo.POTENTIALLY_NULL; // non-null + doubt = pot null + return FlowInfo.NON_NULL; + } else if ((status & FlowInfo.POTENTIALLY_NULL) != 0) + return FlowInfo.POTENTIALLY_NULL; + return status; + } + + public int mergeCloseStatus(BlockScope currentScope, int status, LocalVariableBinding local, BlockScope outerScope) { + // get the most suitable null status representing whether resource 'binding' has been closed + // start at 'currentScope' and potentially travel out until 'outerScope' + // at each scope consult any recorded 'finallyInfo'. + if (status != FlowInfo.NON_NULL) { + if (currentScope.finallyInfo != null) { + int finallyStatus = currentScope.finallyInfo.nullStatus(local); + if (finallyStatus == FlowInfo.NON_NULL) + return finallyStatus; + if (finallyStatus != FlowInfo.NULL) // neither is NON_NULL, but not both are NULL => call it POTENTIALLY_NULL + status = FlowInfo.POTENTIALLY_NULL; + } + if (currentScope != outerScope && currentScope.parent instanceof BlockScope) + return mergeCloseStatus(((BlockScope) currentScope.parent), status, local, outerScope); + } + return status; } /** Mark that this resource is closed locally. */ @@ -180,9 +469,18 @@ * (as argument to a method/ctor call or as a return value from the current method), * and thus should be considered as potentially closed. */ - public static FlowInfo markPassedToOutside(BlockScope scope, Expression expression, FlowInfo flowInfo) { + public static FlowInfo markPassedToOutside(BlockScope scope, Expression expression, FlowInfo flowInfo, TypeBinding allocatedType) { + if ((allocatedType instanceof ReferenceBinding) + && ((ReferenceBinding) allocatedType).hasTypeBit(TypeIds.BitWrapperCloseable)) + return flowInfo; // wrapped closeables are analyzed separately + FakedTrackingVariable trackVar = getCloseTrackingVariable(expression); if (trackVar != null) { + if (trackVar.originalBinding == null) { + // an allocation that never was assigned to a local variable -> drop it completely as we're not responsible + scope.removeTrackingVar(trackVar); + return flowInfo; + } trackVar.globalClosingState |= PASSED_TO_OUTSIDE; if (scope.methodScope() != trackVar.methodScope) trackVar.globalClosingState |= CLOSED_IN_NESTED_METHOD; @@ -201,9 +499,14 @@ } public boolean reportRecordedErrors(Scope scope) { - if (this.globalClosingState == 0) { - reportError(scope.problemReporter(), null, FlowInfo.NULL); - return true; + FakedTrackingVariable current = this; + while ((current.globalClosingState & DOUBT_MASK) == 0) { + current = current.innerTracker; + if (current == null) { + // no relevant state found -> report: + reportError(scope.problemReporter(), null, FlowInfo.NULL); + return true; + } } boolean hasReported = false; if (this.recordedLocations != null) { @@ -218,6 +521,8 @@ } public void reportError(ProblemReporter problemReporter, ASTNode location, int nullStatus) { + if ((this.globalClosingState & WRAPPED) != 0) + return; if (nullStatus == FlowInfo.NULL) { if ((this.globalClosingState & CLOSED_IN_NESTED_METHOD) != 0) problemReporter.potentiallyUnclosedCloseable(this, location); @@ -225,7 +530,7 @@ problemReporter.unclosedCloseable(this, location); } else if (nullStatus == FlowInfo.POTENTIALLY_NULL) { problemReporter.potentiallyUnclosedCloseable(this, location); - } + } } public void reportExplicitClosing(ProblemReporter problemReporter) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java index dac029c..082d20a 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -74,11 +74,26 @@ this.initialization.checkNPE(currentScope, flowContext, flowInfo); } + FlowInfo preInitInfo = null; + boolean shouldAnalyseResource = this.binding != null + && flowInfo.reachMode() == FlowInfo.REACHABLE + && FakedTrackingVariable.isAutoCloseable(this.initialization.resolvedType); + if (shouldAnalyseResource) { + preInitInfo = flowInfo.unconditionalCopy(); + // analysis of resource leaks needs additional context while analyzing the RHS: + FakedTrackingVariable.preConnectTrackerAcrossAssignment(this, this.binding, this.initialization); + } + flowInfo = this.initialization .analyseCode(currentScope, flowContext, flowInfo) .unconditionalInits(); - FakedTrackingVariable.handleResourceAssignment(flowInfo, this, this.initialization, this.binding, null); + + if (shouldAnalyseResource) + FakedTrackingVariable.handleResourceAssignment(preInitInfo, flowInfo, this, this.initialization, this.binding); + else + FakedTrackingVariable.cleanUpAfterAssignment(currentScope, Binding.LOCAL, this.initialization); + int nullStatus = this.initialization.nullStatus(flowInfo); if (!flowInfo.isDefinitelyAssigned(this.binding)){// for local variable debug attributes this.bits |= FirstAssignmentToLocal; 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 46d4540..bc781f3 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -96,9 +96,9 @@ if ((this.arguments[i].implicitConversion & TypeIds.UNBOXING) != 0) { this.arguments[i].checkNPE(currentScope, flowContext, flowInfo); } - // if argument is an AutoCloseable insert info that it *may* be closed (by the target method, i.e.) - flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo); 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.) + flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo, null); } analyseArguments(currentScope, flowContext, flowInfo, this.binding, this.arguments); } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java index 3a2a0ed..7169032 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java @@ -81,7 +81,7 @@ if (this.arguments != null) { for (int i = 0, count = this.arguments.length; i < count; i++) { // if argument is an AutoCloseable insert info that it *may* be closed (by the target method, i.e.) - flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo); + flowInfo = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo, null); flowInfo = this.arguments[i].analyseCode(currentScope, flowContext, flowInfo); if ((this.arguments[i].implicitConversion & TypeIds.UNBOXING) != 0) { this.arguments[i].checkNPE(currentScope, flowContext, flowInfo); diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java index d625d5b..41385c1 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java index 9fae009..c623113 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java @@ -134,7 +134,7 @@ if (resourceBinding.closeTracker != null) { // this was false alarm, we don't need to track the resource this.tryBlock.scope.removeTrackingVar(resourceBinding.closeTracker); - resourceBinding.closeTracker = null; + // keep the tracking variable in the resourceBinding in order to prevent creating a new one while analyzing the try block } TypeBinding type = resourceBinding.type; if (type != null && type.isValidBinding()) { @@ -276,7 +276,7 @@ if (resourceBinding.closeTracker != null) { // this was false alarm, we don't need to track the resource this.tryBlock.scope.removeTrackingVar(resourceBinding.closeTracker); - resourceBinding.closeTracker = null; + // keep the tracking variable in the resourceBinding in order to prevent creating a new one while analyzing the try block } TypeBinding type = resourceBinding.type; if (type != null && type.isValidBinding()) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java index 9294726..e65acae 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -1275,7 +1275,9 @@ this.environment.mayTolerateMissingType = wasToleratingMissingTypeProcessingAnnotations; } } - this.typeBits |= this.superclass.typeBits; + this.typeBits |= (this.superclass.typeBits & TypeIds.InheritableBits); + if ((this.typeBits & (TypeIds.BitAutoCloseable|TypeIds.BitCloseable)) != 0) // avoid the side-effects of hasTypeBit()! + this.typeBits |= applyCloseableWhitelists(); return this.superclass; } // NOTE: superInterfaces of binary types are resolved when needed @@ -1298,7 +1300,7 @@ this.environment.mayTolerateMissingType = wasToleratingMissingTypeProcessingAnnotations; } } - this.typeBits |= this.superInterfaces[i].typeBits; + this.typeBits |= (this.superInterfaces[i].typeBits & TypeIds.InheritableBits); } this.tagBits &= ~TagBits.HasUnresolvedSuperinterfaces; return this.superInterfaces; 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 c93c9d9..6d44ea6 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -981,6 +981,10 @@ } /** When are no longer interested in this tracking variable - remove it. */ public void removeTrackingVar(FakedTrackingVariable trackingVariable) { + if (trackingVariable.innerTracker != null) { + removeTrackingVar(trackingVariable.innerTracker); + trackingVariable.innerTracker = null; + } if (this.trackingVariables != null) if (this.trackingVariables.remove(trackingVariable)) return; @@ -1003,10 +1007,10 @@ FakedTrackingVariable trackingVar = (FakedTrackingVariable) this.trackingVariables.get(i); 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 - int status = getNullStatusAggressively(trackingVar.binding, flowInfo); - // try to improve info if a close() inside finally was observed: - if (locationScope != null) // only check at method exit points - status = locationScope.mergeCloseStatus(status, trackingVar.binding, this); + + // compute the most specific null status for this resource, + int status = trackingVar.findMostSpecificStatus(flowInfo, this, locationScope); + if (status == FlowInfo.NULL) { // definitely unclosed: highest priority reportResourceLeak(trackingVar, location, status); @@ -1033,24 +1037,6 @@ this.locals[i].closeTracker = null; this.trackingVariables = null; } -} - -private int mergeCloseStatus(int status, LocalVariableBinding binding, BlockScope outerScope) { - // get the most suitable null status representing whether resource 'binding' has been closed - // start at this scope and potentially travel out until 'outerScope' - // at each scope consult any recorded 'finallyInfo'. - if (status != FlowInfo.NON_NULL) { - if (this.finallyInfo != null) { - int finallyStatus = this.finallyInfo.nullStatus(binding); - if (finallyStatus == FlowInfo.NON_NULL) - return finallyStatus; - if (finallyStatus != FlowInfo.NULL) // neither is NON_NULL, but not both are NULL => call it POTENTIALLY_NULL - status = FlowInfo.POTENTIALLY_NULL; - } - if (this != outerScope && this.parent instanceof BlockScope) - return ((BlockScope) this.parent).mergeCloseStatus(status, binding, outerScope); - } - return status; } private void reportResourceLeak(FakedTrackingVariable trackingVar, ASTNode location, int nullStatus) { @@ -1083,6 +1069,8 @@ if (this.trackingVariables != null) { for (int i=0; i