diff --git a/checker/build.gradle b/checker/build.gradle index a6a7bf2268b..d09b9f95256 100644 --- a/checker/build.gradle +++ b/checker/build.gradle @@ -62,6 +62,8 @@ dependencies { testImplementation 'com.amazonaws:aws-java-sdk-kms' // The AWS SDK is used for testing the Called Methods Checker. testImplementation platform('com.amazonaws:aws-java-sdk-bom:1.12.293') + // For the Resource Leak Checker's support for JavaEE. + testImplementation 'javax.servlet:javax.servlet-api:3.1.0' testImplementation group: 'junit', name: 'junit', version: '4.13.2' testImplementation project(':framework-test') diff --git a/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java b/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java index ecf526952ea..4600c40ba1a 100644 --- a/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java +++ b/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java @@ -640,14 +640,14 @@ private boolean isValidCreatesMustCallForExpression( if (expression instanceof FieldAccess) { Element elt = ((FieldAccess) expression).getField(); if (!checker.hasOption(MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP) - && typeFactory.getDeclAnnotation(elt, Owning.class) != null) { + && typeFactory.hasOwning(elt)) { // The expression is an Owning field. This satisfies case 1. return true; } } else if (expression instanceof LocalVariable) { Element elt = ((LocalVariable) expression).getElement(); if (!checker.hasOption(MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP) - && typeFactory.getDeclAnnotation(elt, Owning.class) != null) { + && typeFactory.hasOwning(elt)) { // The expression is an Owning formal parameter. Note that this cannot actually // be a local variable (despite expressions's type being LocalVariable) because // the @Owning annotation can only be written on methods, parameters, and fields; @@ -956,19 +956,14 @@ private void removeObligationsAtOwnershipTransferToParameters( // check if parameter has an @Owning annotation VariableElement parameter = parameters.get(i); - Set annotationMirrors = typeFactory.getDeclAnnotations(parameter); - for (AnnotationMirror anno : annotationMirrors) { - if (AnnotationUtils.areSameByName( - anno, "org.checkerframework.checker.mustcall.qual.Owning")) { - Obligation localObligation = getObligationForVar(obligations, local); - // Passing to an owning parameter is not sufficient to resolve the - // obligation created from a MustCallAlias parameter, because the containing - // method must actually return the value. - if (!localObligation.derivedFromMustCallAlias()) { - // Transfer ownership! - obligations.remove(localObligation); - break; - } + if (typeFactory.hasOwning(parameter)) { + Obligation localObligation = getObligationForVar(obligations, local); + // Passing to an owning parameter is not sufficient to resolve the + // obligation created from a MustCallAlias parameter, because the containing + // method must actually return the value. + if (!localObligation.derivedFromMustCallAlias()) { + // Transfer ownership! + obligations.remove(localObligation); } } } @@ -1032,7 +1027,7 @@ private boolean isTransferOwnershipAtReturn(ControlFlowGraph cfg) { // not be transferred. MethodTree method = ((UnderlyingAST.CFGMethod) underlyingAST).getMethod(); ExecutableElement executableElement = TreeUtils.elementFromDeclaration(method); - return typeFactory.getDeclAnnotation(executableElement, NotOwning.class) == null; + return !typeFactory.hasNotOwning(executableElement); } return false; } @@ -1061,7 +1056,7 @@ private void updateObligationsForAssignment( if (lhsElement.getKind() == ElementKind.FIELD) { boolean isOwningField = !checker.hasOption(MustCallChecker.NO_LIGHTWEIGHT_OWNERSHIP) - && typeFactory.getDeclAnnotation(lhsElement, Owning.class) != null; + && typeFactory.hasOwning(lhsElement); // Check that the must-call obligations of the lhs have been satisfied, if the field is // non-final and owning. if (isOwningField @@ -1116,7 +1111,7 @@ private boolean hasAtMostOneOwningField(TypeElement element) { // Has an owning field already been encountered? boolean hasOwningField = false; for (VariableElement field : fields) { - if (typeFactory.getDeclAnnotation(field, Owning.class) != null) { + if (typeFactory.hasOwning(field)) { if (hasOwningField) { return false; } else { @@ -1635,7 +1630,7 @@ private boolean shouldTrackReturnType(MethodInvocationNode node) { return false; } // check for absence of @NotOwning annotation - return (typeFactory.getDeclAnnotation(executableElement, NotOwning.class) == null); + return !typeFactory.hasNotOwning(executableElement); } /** diff --git a/checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnnotatedTypeFactory.java b/checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnnotatedTypeFactory.java index c12f15993e6..0e65fabd9e3 100644 --- a/checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnnotatedTypeFactory.java +++ b/checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakAnnotatedTypeFactory.java @@ -23,6 +23,8 @@ import org.checkerframework.checker.mustcall.qual.CreatesMustCallFor; import org.checkerframework.checker.mustcall.qual.MustCall; import org.checkerframework.checker.mustcall.qual.MustCallAlias; +import org.checkerframework.checker.mustcall.qual.NotOwning; +import org.checkerframework.checker.mustcall.qual.Owning; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.common.basetype.BaseTypeChecker; import org.checkerframework.dataflow.cfg.ControlFlowGraph; @@ -355,4 +357,34 @@ public ExecutableElement getCreatesMustCallForValueElement() { public ExecutableElement getCreatesMustCallForListValueElement() { return createsMustCallForListValueElement; } + + /** + * Does the given element have an {@code @NotOwning} annotation (including in stub files)? + * + *

Prefer this method to calling {@link #getDeclAnnotation(Element, Class)} on the type factory + * directly, which won't find this annotation in stub files (it only considers stub files loaded + * by this checker, not subcheckers). + * + * @param elt an element + * @return whether there is a NotOwning annotation on the given element + */ + public boolean hasNotOwning(Element elt) { + MustCallAnnotatedTypeFactory mcatf = getTypeFactoryOfSubchecker(MustCallChecker.class); + return mcatf.getDeclAnnotation(elt, NotOwning.class) != null; + } + + /** + * Does the given element have an {@code @Owning} annotation (including in stub files)? + * + *

Prefer this method to calling {@link #getDeclAnnotation(Element, Class)} on the type factory + * directly, which won't find this annotation in stub files (it only considers stub files loaded + * by this checker, not subcheckers). + * + * @param elt an element + * @return whether there is an Owning annotation on the given element + */ + public boolean hasOwning(Element elt) { + MustCallAnnotatedTypeFactory mcatf = getTypeFactoryOfSubchecker(MustCallChecker.class); + return mcatf.getDeclAnnotation(elt, Owning.class) != null; + } } diff --git a/checker/tests/resourceleak/JavaEETest.java b/checker/tests/resourceleak/JavaEETest.java new file mode 100644 index 00000000000..bb1f7431b49 --- /dev/null +++ b/checker/tests/resourceleak/JavaEETest.java @@ -0,0 +1,7 @@ +// Test for https://github.com/typetools/checker-framework/issues/5472 + +class JavaEETest { + static void foo(javax.servlet.ServletResponse s) throws java.io.IOException { + s.getWriter(); + } +}