Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #5472 #5476

Merged
merged 5 commits into from
Dec 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions checker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -956,19 +956,14 @@ private void removeObligationsAtOwnershipTransferToParameters(

// check if parameter has an @Owning annotation
VariableElement parameter = parameters.get(i);
Set<AnnotationMirror> 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);
}
}
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)?
*
* <p>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)?
*
* <p>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;
}
}
7 changes: 7 additions & 0 deletions checker/tests/resourceleak/JavaEETest.java
Original file line number Diff line number Diff line change
@@ -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();
}
}