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 9660125..6217175 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 @@ -33,7 +33,7 @@ import junit.framework.Test; public class TryWithResourcesStatementTest extends AbstractRegressionTest { static { -// TESTS_NAMES = new String[] { "test061a"}; +// TESTS_NAMES = new String[] { "test061m"}; // TESTS_NUMBERS = new int[] { 50 }; // TESTS_RANGE = new int[] { 11, -1 }; } @@ -5911,6 +5911,54 @@ public void test061l() throws IOException { options, null); } +// Bug 358903 - Filter practically unimportant resource leak warnings +// a closeable is passed to another method in a return statement +// example constructed after org.eclipse.equinox.internal.p2.artifact.repository.simple.SimpleArtifactRepository#getArtifact(..) +public void test061m() 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.InputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " BufferedInputStream stream;\n" + + " BufferedInputStream foo(File file) throws IOException {\n" + + " FileInputStream s = new FileInputStream(file);\n" + + " return check(new BufferedInputStream(s));\n" + + " }\n" + + " BufferedInputStream foo2(FileInputStream s, File file) throws IOException {\n" + + " s = new FileInputStream(file);\n" + + " return check(s);\n" + + " }\n" + + " BufferedInputStream foo3(InputStream s) throws IOException {\n" + + " s = check(s);\n" + + " return check(s);\n" + + " }\n" + + " BufferedInputStream check(InputStream s) { return null; }\n" + + "}\n" + }, + // TODO: also these warnings *might* be avoidable by detecting check(s) as a wrapper creation?? + "----------\n" + + "1. ERROR in X.java (at line 14)\n" + + " return check(s);\n" + + " ^^^^^^^^^^^^^^^^\n" + + "Potential resource leak: \'s\' may not be closed at this location\n" + + "----------\n" + + "2. ERROR in X.java (at line 18)\n" + + " return check(s);\n" + + " ^^^^^^^^^^^^^^^^\n" + + "Potential resource leak: \'s\' may not be closed at this location\n" + + "----------\n", + null, + true, + options); +} // Bug 362331 - Resource leak not detected when closeable not assigned to variable // a resource is never assigned public void test062a() throws IOException { @@ -6123,6 +6171,51 @@ public void test063c() throws IOException { options, null); } +// Bug 362332 - Only report potential leak when closeable not created in the local scope +// a resource is obtained as a method argument and/or assigned with a cast +public void test063d() throws IOException { + Map options = getCompilerOptions(); + options.put(CompilerOptions.OPTION_ReportUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportPotentiallyUnclosedCloseable, CompilerOptions.ERROR); + options.put(CompilerOptions.OPTION_ReportExplicitlyClosedAutoCloseable, CompilerOptions.ERROR); + this.runNegativeTest( + new String[] { + "X.java", + "import java.io.FileInputStream;\n" + + "import java.io.BufferedInputStream;\n" + + "import java.io.InputStream;\n" + + "import java.io.IOException;\n" + + "public class X {\n" + + " void foo( InputStream input) throws IOException {\n" + + " FileInputStream input1 = (FileInputStream)input;\n" + + " System.out.println(input1.read());\n" + + " input.close();\n" + // don't propose t-w-r for argument + " }\n" + + " void foo() throws IOException {\n" + + " InputStream input = new FileInputStream(\"somefile\");\n" + + " FileInputStream input1 = (FileInputStream)input;\n" + + " System.out.println(input1.read());\n" + + " input.close();\n" + // do propose t-w-r, not from a method argument + " }\n" + + " void foo3( InputStream input, InputStream input2) throws IOException {\n" + + " FileInputStream input1 = (FileInputStream)input;\n" + // still don't claim because obtained from outside + " System.out.println(input1.read());\n" + + " BufferedInputStream bis = new BufferedInputStream(input2);\n" + + " System.out.println(bis.read());\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 12)\n" + + " InputStream input = new FileInputStream(\"somefile\");\n" + + " ^^^^^\n" + + "Resource \'input\' should be managed by try-with-resource\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 71217ce..67eb8f5 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 @@ -60,7 +60,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl analyseArguments(currentScope, flowContext, flowInfo, this.binding, this.arguments); } - if (FakedTrackingVariable.isAutoCloseable(this.resolvedType)) + if (FakedTrackingVariable.isAnyCloseable(this.resolvedType)) FakedTrackingVariable.analyseCloseableAllocation(currentScope, flowInfo, this); // record some dependency information for exception types 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 539a661..e328f12 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 @@ -51,7 +51,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl FlowInfo preInitInfo = null; boolean shouldAnalyseResource = local != null && flowInfo.reachMode() == FlowInfo.REACHABLE - && (FakedTrackingVariable.isAutoCloseable(this.expression.resolvedType) + && (FakedTrackingVariable.isAnyCloseable(this.expression.resolvedType) || this.expression.resolvedType == TypeBinding.NULL); if (shouldAnalyseResource) { preInitInfo = flowInfo.unconditionalCopy(); 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 b35a67c..afa2381 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 @@ -43,17 +43,19 @@ public class FakedTrackingVariable extends LocalDeclaration { private static final int CLOSE_SEEN = 1; // the resource was passed to outside code (arg in method/ctor call or as a return value from this method): private static final int PASSED_TO_OUTSIDE = 2; + // the resource was obtained from the outside (argument of the current method, or via a field read) + private static final int OBTAINED_FROM_OUTSIDE = 4; // If close() is invoked from a nested method (inside a local type) report remaining problems only as potential: - private static final int CLOSED_IN_NESTED_METHOD = 4; + private static final int CLOSED_IN_NESTED_METHOD = 8; // a location independent issue has been reported already against this resource: - private static final int REPORTED = 8; + private static final int REPORTED = 16; // a resource is wrapped in another resource: - private static final int WRAPPED = 16; + private static final int WRAPPED = 32; - private static final int DOUBT_MASK = CLOSE_SEEN | PASSED_TO_OUTSIDE | CLOSED_IN_NESTED_METHOD | REPORTED; // not WRAPPED + private static final int DOUBT_MASK = CLOSE_SEEN | PASSED_TO_OUTSIDE | OBTAINED_FROM_OUTSIDE | CLOSED_IN_NESTED_METHOD | REPORTED; // not WRAPPED /** - * Bitset of {@link #CLOSE_SEEN}, {@link #PASSED_TO_OUTSIDE}, {@link #CLOSED_IN_NESTED_METHOD}, {@link #REPORTED} and {@link #WRAPPED}. + * Bitset of {@link #CLOSE_SEEN}, {@link #PASSED_TO_OUTSIDE}, {@link #OBTAINED_FROM_OUTSIDE}, {@link #CLOSED_IN_NESTED_METHOD}, {@link #REPORTED} and {@link #WRAPPED}. */ private int globalClosingState = 0; @@ -116,18 +118,31 @@ public class FakedTrackingVariable extends LocalDeclaration { * @return a new {@link FakedTrackingVariable} or null. */ public static FakedTrackingVariable getCloseTrackingVariable(Expression expression) { + while (true) { + if (expression instanceof CastExpression) + expression = ((CastExpression) expression).expression; + else if (expression instanceof Assignment) + expression = ((Assignment) expression).expression; + else + break; + } if (expression instanceof SingleNameReference) { SingleNameReference name = (SingleNameReference) expression; if (name.binding instanceof LocalVariableBinding) { LocalVariableBinding local = (LocalVariableBinding)name.binding; if (local.closeTracker != null) return local.closeTracker; - if (local.isParameter() || !isAutoCloseable(expression.resolvedType)) + if (!isAnyCloseable(expression.resolvedType)) return null; // tracking var doesn't yet exist. This happens in finally block // which is analyzed before the corresponding try block Statement location = local.declaration; - return local.closeTracker = new FakedTrackingVariable(local, location); + local.closeTracker = new FakedTrackingVariable(local, location); + if (local.isParameter()) { + local.closeTracker.globalClosingState |= OBTAINED_FROM_OUTSIDE; + // status of this tracker is now UNKNOWN + } + return local.closeTracker; } } else if (expression instanceof AllocationExpression) { // return any preliminary tracking variable from analyseCloseableAllocation @@ -152,7 +167,7 @@ public class FakedTrackingVariable extends LocalDeclaration { if (rhs instanceof AllocationExpression) { closeTracker = local.closeTracker; if (closeTracker == null) { - if (isAutoCloseable(rhs.resolvedType)) { + if (isAnyCloseable(rhs.resolvedType)) { closeTracker = new FakedTrackingVariable(local, location); } } @@ -178,7 +193,7 @@ public class FakedTrackingVariable extends LocalDeclaration { } else if (((ReferenceBinding)allocation.resolvedType).hasTypeBit(TypeIds.BitWrapperCloseable)) { if (allocation.arguments != null && allocation.arguments.length > 0) { // find the wrapped resource represented by its tracking var: - FakedTrackingVariable innerTracker = analyseCloseableAllocationArgument(scope, flowInfo, allocation, allocation.arguments[0]); + FakedTrackingVariable innerTracker = findCloseTracker(scope, flowInfo, allocation, allocation.arguments[0]); if (innerTracker != null) { if (innerTracker == allocation.closeTracker) return; // self wrap (res = new Res(res)) -> neither change (here) nor remove (below) @@ -212,7 +227,7 @@ public class FakedTrackingVariable extends LocalDeclaration { } /** Find an existing tracking variable for the argument of an allocation for a resource wrapper. */ - public static FakedTrackingVariable analyseCloseableAllocationArgument(BlockScope scope, FlowInfo flowInfo, AllocationExpression allocation, Expression arg) + private static FakedTrackingVariable findCloseTracker(BlockScope scope, FlowInfo flowInfo, AllocationExpression allocation, Expression arg) { while (arg instanceof Assignment) { Assignment assign = (Assignment)arg; @@ -276,14 +291,15 @@ public class FakedTrackingVariable extends LocalDeclaration { // 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); + if ((previousTracker.globalClosingState & (PASSED_TO_OUTSIDE|OBTAINED_FROM_OUTSIDE)) == 0) + flowInfo.markAsDefinitelyNull(previousTracker.binding); local.closeTracker = analyseCloseableExpression(flowInfo, local, location, rhs, previousTracker); } else { // 3. no re-use, create a fresh tracking variable: 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) + if ((rhsTrackVar.globalClosingState & (PASSED_TO_OUTSIDE|OBTAINED_FROM_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) @@ -333,15 +349,15 @@ public class FakedTrackingVariable extends LocalDeclaration { // we *might* be responsible for the resource obtained FakedTrackingVariable tracker = new FakedTrackingVariable(local, location); tracker.globalClosingState |= PASSED_TO_OUTSIDE; - flowInfo.markPotentiallyNullBit(tracker.binding); + flowInfo.markPotentiallyNullBit(tracker.binding); // shed some doubt return tracker; } else if (expression instanceof FieldReference || expression instanceof QualifiedNameReference) { // responsibility for this resource probably lies at a higher level FakedTrackingVariable tracker = new FakedTrackingVariable(local, location); - tracker.globalClosingState |= PASSED_TO_OUTSIDE; - flowInfo.markPotentiallyNonNullBit(tracker.binding); + tracker.globalClosingState |= OBTAINED_FROM_OUTSIDE; + // leave state as UNKNOWN, the bit OBTAINED_FROM_OUTSIDE will prevent spurious warnings return tracker; } @@ -385,7 +401,7 @@ public class FakedTrackingVariable extends LocalDeclaration { } /** Answer wither the given type binding is a subtype of java.lang.AutoCloseable. */ - public static boolean isAutoCloseable(TypeBinding typeBinding) { + public static boolean isAnyCloseable(TypeBinding typeBinding) { return typeBinding instanceof ReferenceBinding && ((ReferenceBinding)typeBinding).hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable); } @@ -544,7 +560,7 @@ public class FakedTrackingVariable extends LocalDeclaration { } public void reportExplicitClosing(ProblemReporter problemReporter) { - if ((this.globalClosingState & REPORTED) == 0) { + if ((this.globalClosingState & (OBTAINED_FROM_OUTSIDE|REPORTED)) == 0) { // can't use t-w-r for OBTAINED_FROM_OUTSIDE this.globalClosingState |= REPORTED; problemReporter.explicitlyClosedAutoCloseable(this); } 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 3781553..239ef15 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 @@ -78,7 +78,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl FlowInfo preInitInfo = null; boolean shouldAnalyseResource = this.binding != null && flowInfo.reachMode() == FlowInfo.REACHABLE - && FakedTrackingVariable.isAutoCloseable(this.initialization.resolvedType); + && FakedTrackingVariable.isAnyCloseable(this.initialization.resolvedType); if (shouldAnalyseResource) { preInitInfo = flowInfo.unconditionalCopy(); // analysis of resource leaks needs additional context while analyzing the RHS: 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 1890315..5e346e9 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 @@ -977,8 +977,7 @@ public int registerTrackingVariable(FakedTrackingVariable fakedTrackingVariable) this.trackingVariables = new ArrayList(3); this.trackingVariables.add(fakedTrackingVariable); MethodScope outerMethodScope = outerMostMethodScope(); - return outerMethodScope.analysisIndex + (outerMethodScope.trackVarCount++); - + return outerMethodScope.analysisIndex++; } /** When are no longer interested in this tracking variable - remove it. */ public void removeTrackingVar(FakedTrackingVariable trackingVariable) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodScope.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodScope.java index 52c7258..bfb4f2b 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodScope.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodScope.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,9 +50,6 @@ public class MethodScope extends BlockScope { // inner-emulation public SyntheticArgumentBinding[] extraSyntheticArguments; - // count number of tracking variables, see FakedTrackingVariable - int trackVarCount = 0; - public MethodScope(ClassScope parent, ReferenceContext context, boolean isStatic) { super(METHOD_SCOPE, parent); this.locals = new LocalVariableBinding[5];