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..0330547 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[] { "test061"}; // TESTS_NUMBERS = new int[] { 50 }; // TESTS_RANGE = new int[] { 11, -1 }; } @@ -5354,7 +5359,373 @@ "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 bis2 = 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 19)\n" + + " BufferedInputStream doubleWrap2 = new BufferedInputStream(bis2);\n" + + " ^^^^^^^^^^^\n" + + "Potential resource leak: \'doubleWrap2\' may not be closed\n" + + "----------\n", + null, + true, + options); +} 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..0a64324 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 @@ -45,7 +45,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, this.resolvedType); flowInfo = this.arguments[i] .analyseCode(currentScope, flowContext, flowInfo) 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..ebf4e92 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 @@ -50,14 +50,14 @@ .analyseAssignment(currentScope, flowContext, flowInfo, this, false) .unconditionalInits(); if (local != null) { - LocalVariableBinding previousTrackerBinding = null; + FakedTrackingVariable previousTracker = null; if (local.closeTracker != null) { // Assigning to a variable already holding an AutoCloseable, has it been closed before? - previousTrackerBinding = local.closeTracker.binding; + previousTracker = local.closeTracker; if (!flowInfo.isDefinitelyNull(local)) // only if previous value may be non-null - local.closeTracker.recordErrorLocation(this, flowInfo.nullStatus(previousTrackerBinding)); + local.closeTracker.recordErrorLocation(this, flowInfo.nullStatus(previousTracker.binding)); } - FakedTrackingVariable.handleResourceAssignment(flowInfo, this, this.expression, local, previousTrackerBinding); + FakedTrackingVariable.handleResourceAssignment(flowInfo, this, this.expression, local, previousTracker); } int nullStatus = this.expression.nullStatus(flowInfo); if (local != null && (local.type.tagBits & TagBits.IsBaseType) == 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 ee13047..60053e3 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 @@ -48,9 +48,11 @@ 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; /** - * 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; @@ -58,10 +60,12 @@ public LocalVariableBinding originalBinding; // the real local being tracked - HashMap recordedLocations; // initially null, ASTNode -> Integer + HashMap recordedLocations; // initially null, ASTNode -> Integer + + public FakedTrackingVariable innerTracker; // chained tracking variable of a chained (wrapped) resource - public FakedTrackingVariable(LocalVariableBinding original, Statement location) { + public FakedTrackingVariable(LocalVariableBinding original, ASTNode location) { super(original.name, location.sourceStart, location.sourceEnd); this.type = new SingleTypeReference( TypeConstants.OBJECT, @@ -131,8 +135,8 @@ * 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. */ - public static void handleResourceAssignment(FlowInfo flowInfo, Statement location, Expression rhs, LocalVariableBinding local, - LocalVariableBinding previousTrackerBinding) + public static void handleResourceAssignment(FlowInfo flowInfo, ASTNode location, Expression rhs, LocalVariableBinding local, + FakedTrackingVariable previousTracker) { if (isAutoCloseable(rhs.resolvedType)) { // new value is AutoCloseable, start tracking, possibly re-using existing tracker var: @@ -141,11 +145,15 @@ 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? + } else if (previousTracker != null) { // 2. re-use tracking variable from the LHS? // re-assigning from a fresh, mark as not-closed again: - flowInfo.markAsDefinitelyNull(previousTrackerBinding); + 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); + rhsTrackVar = analyseCloseableExpression(flowInfo, local, location, rhs, null); + if (rhsTrackVar == null) + return; + local.closeTracker = rhsTrackVar; // a fresh resource, mark as not-closed: flowInfo.markAsDefinitelyNull(local.closeTracker.binding); // TODO(stephan): this might be useful, but I could not find a test case for it: @@ -153,6 +161,115 @@ // flowContext.initsOnFinally.markAsDefinitelyNonNull(trackerBinding); } } + } + /** + * analyze structure of a closeable expression, matching (chained) resources against our white lists. + * See Bug 358903 - Filter practically unimportant resource leak warnings + * @param flowInfo used when recursing back into {@link #handleResourceAssignment} + * @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.resolvedType instanceof ReferenceBinding) { + ReferenceBinding resourceType = (ReferenceBinding) expression.resolvedType; + + if (resourceType.hasTypeBit(TypeIds.BitResourceFreeCloseable)) { + // (a) resource-free closeable: -> null + return null; + } + + if (resourceType.hasTypeBit(TypeIds.BitWrapperCloseable)) { + // (b) wrapper + Expression innerExpression = expression; + if (innerExpression instanceof Assignment) + innerExpression = ((Assignment)innerExpression).expression; + if (innerExpression instanceof AllocationExpression) { + Expression[] args = ((AllocationExpression) innerExpression).arguments; + if (args != null && args.length == 1) { + // (b.1) wrapper allocation with argument + return analyseCloseableAllocationArgument(flowInfo, local, location, args[0], previousTracker); + } + } + LocalVariableBinding innerLocal = innerExpression.localVariableBinding(); + if (innerLocal != null && innerLocal.closeTracker != null) { + FakedTrackingVariable outerTracker = previousTracker != null ? previousTracker : new FakedTrackingVariable(local, location); + outerTracker.innerTracker = innerLocal.closeTracker; + innerLocal.closeTracker.globalClosingState |= WRAPPED; + return outerTracker; + } + // (b.2) wrapper with irrelevant inner: -> null + return null; + } + } + if (local.closeTracker != null) + // (c): inner has already been analysed: -> re-use track var + return local.closeTracker; + // (d): normal resource: -> normal tracking var + if (previousTracker != null) + return previousTracker; // (d.1): re-use existing tracking var + return new FakedTrackingVariable(local, location); + } + + // an outer allocation expression has an argument, recursively analyze whether the arg is closeable + // return (1) a possible nested tracker for the outer expression or (2) null signaling no relevant resource contained + static FakedTrackingVariable analyseCloseableAllocationArgument(FlowInfo flowInfo, LocalVariableBinding outerLocal, ASTNode outerLocation, + Expression arg, FakedTrackingVariable previousTracker) + { + if (arg instanceof Assignment) { + Assignment assign = (Assignment)arg; + LocalVariableBinding innerLocal = assign.localVariableBinding(); + if (innerLocal != null) { + // recurse from the top for nested assignment: + handleResourceAssignment(flowInfo, assign, assign.expression, innerLocal, innerLocal.closeTracker); + return innerLocal.closeTracker; // FIXME do we need a nested tracker here? + } 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? + return getTrackingVarForNested(flowInfo, outerLocal, outerLocation, (LocalVariableBinding)ref.binding, ref, ref, previousTracker); + } + } else if (arg instanceof AllocationExpression && arg.resolvedType instanceof ReferenceBinding) { + // nested allocation + ReferenceBinding innerType = (ReferenceBinding)arg.resolvedType; + if (innerType.hasTypeBit(TypeIds.BitResourceFreeCloseable)) { + return null; // leaf of wrapper-chain is irrelevant + } else if (innerType.hasTypeBit(TypeIds.BitWrapperCloseable)) { + // nested wrapper, nested tracking variables may skip this level as it is not bound to a local variable + Expression[] args = ((AllocationExpression) arg).arguments; + if (args != null && args.length > 0) + return analyseCloseableAllocationArgument(flowInfo, outerLocal, arg, args[0], previousTracker); + return null; // wrapper with no arg? shouldn't occur actually + } else { + // (c) wrapper alloc with direct nested alloc of regular: -> normal track var (no local represents inner) + return previousTracker != null ? previousTracker : new FakedTrackingVariable(outerLocal, outerLocation); + } + } + return null; + } + + // an outer allocation expression has an argument, create/link outer and inner tracking variable and return the outer + // return null if inner is not tracked + private static FakedTrackingVariable getTrackingVarForNested(FlowInfo flowInfo, LocalVariableBinding outerLocal, ASTNode outerLocation, + LocalVariableBinding innerLocal, ASTNode innerLocation, Expression innerExpression, FakedTrackingVariable previousTracker) + { + FakedTrackingVariable innerTracker = analyseCloseableExpression(flowInfo, innerLocal, innerLocation, innerExpression, null); + if (innerTracker != null) { + FakedTrackingVariable outerTracker = previousTracker != null ? previousTracker : new FakedTrackingVariable(outerLocal, outerLocation); + outerTracker.innerTracker = innerTracker; + innerTracker.globalClosingState |= WRAPPED; + return outerTracker; + } + return null; } /** Answer wither the given type binding is a subtype of java.lang.AutoCloseable. */ @@ -180,7 +297,11 @@ * (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) { trackVar.globalClosingState |= PASSED_TO_OUTSIDE; @@ -201,9 +322,14 @@ } public boolean reportRecordedErrors(Scope scope) { - if (this.globalClosingState == 0) { - reportError(scope.problemReporter(), null, FlowInfo.NULL); - return true; + FakedTrackingVariable current = this; + while (current.globalClosingState == 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 +344,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 +353,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/MessageSend.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java index 46d4540..eae974b 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 @@ -97,7 +97,7 @@ 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 = FakedTrackingVariable.markPassedToOutside(currentScope, this.arguments[i], flowInfo, null); flowInfo = this.arguments[i].analyseCode(currentScope, flowContext, flowInfo).unconditionalInits(); } 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/lookup/BinaryTypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java index 9294726..d082d29 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.hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable)) + 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..0187139 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 @@ -1003,10 +1003,25 @@ 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, + // consider wrappers (per white list) encapsulating an inner resource. + int status = FlowInfo.UNKNOWN; + FakedTrackingVariable currentTracker = trackingVar; + while (currentTracker != null) { + LocalVariableBinding currentVar = currentTracker.binding; + int currentStatus = getNullStatusAggressively(currentVar, flowInfo); + if (locationScope != null) // only check at method exit points + currentStatus = locationScope.mergeCloseStatus(currentStatus, currentVar, this); + 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; + } + if (status == FlowInfo.NULL) { // definitely unclosed: highest priority reportResourceLeak(trackingVar, location, status); diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java index 3eb3cca..2b32dc2 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ClassScope.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 @@ -909,7 +909,10 @@ } else { // only want to reach here when no errors are reported sourceType.superclass = superclass; - sourceType.typeBits |= superclass.typeBits; + sourceType.typeBits |= (superclass.typeBits & TypeIds.InheritableBits); + // further analysis against white lists for the unlikely case we are compiling java.io.*: + if (sourceType.hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable)) + sourceType.typeBits |= sourceType.applyCloseableWhitelists(); return true; } } @@ -1025,7 +1028,7 @@ noProblems &= superInterfaceRef.resolvedType.isValidBinding(); } // only want to reach here when no errors are reported - sourceType.typeBits |= superInterface.typeBits; + sourceType.typeBits |= (superInterface.typeBits & TypeIds.InheritableBits); interfaceBindings[count++] = superInterface; } // hold onto all correctly resolved superinterfaces diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java index 75de854..057fb4f 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java @@ -1469,4 +1469,31 @@ public FieldBinding[] unResolvedFields() { return Binding.NO_FIELDS; } + +/* + * If a type - known to be a Closeable - is mentioned in one of our white lists + * answer the typeBit for the white list (BitWrapperCloseable or BitResourceFreeCloseable). + */ +protected int applyCloseableWhitelists() { + switch (this.compoundName.length) { + case 3: + if (CharOperation.equals(TypeConstants.JAVA, this.compoundName[0])) { + if (CharOperation.equals(TypeConstants.IO, this.compoundName[1])) { + char[] simpleName = this.compoundName[2]; + int l = TypeConstants.JAVA_IO_WRAPPER_CLOSEABLES.length; + for (int i = 0; i < l; i++) { + if (CharOperation.equals(simpleName, TypeConstants.JAVA_IO_WRAPPER_CLOSEABLES[i])) + return TypeIds.BitWrapperCloseable; + } + l = TypeConstants.JAVA_IO_RESOURCE_FREE_CLOSEABLES.length; + for (int i = 0; i < l; i++) { + if (CharOperation.equals(simpleName, TypeConstants.JAVA_IO_RESOURCE_FREE_CLOSEABLES[i])) + return TypeIds.BitResourceFreeCloseable; + } + } + } + break; + } + return 0; +} } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java index 0bbef47..ef77f4c 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java @@ -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 @@ -154,6 +154,23 @@ }; char[][] JAVA_LANG_AUTOCLOSEABLE = {JAVA, LANG, "AutoCloseable".toCharArray()}; //$NON-NLS-1$ char[] CLOSE = "close".toCharArray(); //$NON-NLS-1$ + // white lists of closeables: + char[][] JAVA_IO_WRAPPER_CLOSEABLES = new char[][] { + "BufferedInputStream".toCharArray(), //$NON-NLS-1$ + "BufferedOutputStream".toCharArray(), //$NON-NLS-1$ + "BufferedReader".toCharArray(), //$NON-NLS-1$ + "BufferedWriter".toCharArray(), //$NON-NLS-1$ + "InputStreamReader".toCharArray(), //$NON-NLS-1$ + "PrintWriter".toCharArray(), //$NON-NLS-1$ + }; + char[][] JAVA_IO_RESOURCE_FREE_CLOSEABLES = new char[][] { + "StringReader".toCharArray(), //$NON-NLS-1$ + "StringWriter".toCharArray(), //$NON-NLS-1$ + "ByteArrayInputStream".toCharArray(), //$NON-NLS-1$ + "ByteArrayOutputStream".toCharArray(), //$NON-NLS-1$ + "CharArrayReader".toCharArray(), //$NON-NLS-1$ + "CharArrayWriter".toCharArray(), //$NON-NLS-1$ + }; // Constraints for generic type argument inference int CONSTRAINT_EQUAL = 0; // Actual = Formal diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeIds.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeIds.java index 7fff434..d6ec945 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeIds.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeIds.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 @@ -207,4 +207,19 @@ * @see ReferenceBinding#hasTypeBit(int) */ final int BitCloseable = 2; + /** + * Bit for members of a white list: + * Subtypes of Closeable that wrap another resource without directly holding any OS resources. + */ + final int BitWrapperCloseable = 4; + /** + * Bit for members of a white list: + * Subtypes of Closeable that do not hold an OS resource that needs to be released. + */ + final int BitResourceFreeCloseable = 8; + + /** + * Set of type bits that should be inherited by any sub types. + */ + final int InheritableBits = BitAutoCloseable | BitCloseable; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java index 995f30e..bd698f3 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.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 @@ -316,11 +316,11 @@ // initialize from bounds this.typeBits = 0; if (this.superclass != null && this.superclass.hasTypeBit(~TypeIds.BitUninitialized)) - this.typeBits |= this.superclass.typeBits; + this.typeBits |= (this.superclass.typeBits & TypeIds.InheritableBits); if (this.superInterfaces != null) for (int i = 0, l = this.superInterfaces.length; i < l; i++) if (this.superInterfaces[i].hasTypeBit(~TypeIds.BitUninitialized)) - this.typeBits |= this.superInterfaces[i].typeBits; + this.typeBits |= (this.superInterfaces[i].typeBits & TypeIds.InheritableBits); } return (this.typeBits & bit) != 0; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java index 01f369e..b887196 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/WildcardBinding.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2011 IBM Corporation and others. + * Copyright (c) 2005, 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 @@ -429,11 +429,11 @@ // initialize from upper bounds this.typeBits = 0; if (this.superclass != null && this.superclass.hasTypeBit(~TypeIds.BitUninitialized)) - this.typeBits |= this.superclass.typeBits; + this.typeBits |= (this.superclass.typeBits & TypeIds.InheritableBits); if (this.superInterfaces != null) for (int i = 0, l = this.superInterfaces.length; i < l; i++) if (this.superInterfaces[i].hasTypeBit(~TypeIds.BitUninitialized)) - this.typeBits |= this.superInterfaces[i].typeBits; + this.typeBits |= (this.superInterfaces[i].typeBits & TypeIds.InheritableBits); } return (this.typeBits & bit) != 0; }