Skip to content

Commit

Permalink
Use "enclosing" rather than "containing"
Browse files Browse the repository at this point in the history
  • Loading branch information
mernst authored May 6, 2024
1 parent 7ee1d07 commit bd54383
Show file tree
Hide file tree
Showing 19 changed files with 74 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public Void visitMethodInvocation(MethodInvocationTree tree, Void p) {
*
* @param fc an invocation of a format method
* @param enclosingMethod the method that contains the call
* @return true if {@code fc} is a call to a format method that forwards its containing method's
* @return true if {@code fc} is a call to a format method that forwards its enclosing method's
* arguments
*/
private boolean isWrappedFormatCall(FormatCall fc, @Nullable MethodTree enclosingMethod) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,14 @@ public Void visitMethodInvocation(MethodInvocationTree methodInvocationTree, Voi
}

if (enclosingMethodElement != null) {
SideEffectAnnotation seaOfContainingMethod =
SideEffectAnnotation seaOfEnclosingMethod =
atypeFactory.methodSideEffectAnnotation(enclosingMethodElement, false);

if (seaOfInvokedMethod.isWeakerThan(seaOfContainingMethod)) {
if (seaOfInvokedMethod.isWeakerThan(seaOfEnclosingMethod)) {
checker.reportError(
methodInvocationTree,
"method.guarantee.violated",
seaOfContainingMethod.getNameOfSideEffectAnnotation(),
seaOfEnclosingMethod.getNameOfSideEffectAnnotation(),
enclosingMethodElement.getSimpleName(),
methodElement.getSimpleName(),
seaOfInvokedMethod.getNameOfSideEffectAnnotation());
Expand Down Expand Up @@ -807,12 +807,11 @@ public Void visitSynchronized(SynchronizedTree tree, Void p) {
if (enclosingMethod != null) {
methodElement = TreeUtils.elementFromDeclaration(enclosingMethod);

SideEffectAnnotation seaOfContainingMethod =
SideEffectAnnotation seaOfEnclosingMethod =
atypeFactory.methodSideEffectAnnotation(methodElement, false);

if (!seaOfContainingMethod.isWeakerThan(SideEffectAnnotation.LOCKINGFREE)) {
checker.reportError(
tree, "synchronized.block.in.lockingfree.method", seaOfContainingMethod);
if (!seaOfEnclosingMethod.isWeakerThan(SideEffectAnnotation.LOCKINGFREE)) {
checker.reportError(tree, "synchronized.block.in.lockingfree.method", seaOfEnclosingMethod);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public Void visitReturn(ReturnTree tree, Void p) {
public Void visitAssignment(AssignmentTree tree, Void p) {
// This code implements the following rule:
// * It is always safe to assign a MustCallAlias parameter of a constructor
// to an owning field of the containing class.
// to an owning field of the enclosing class.
// It is necessary to special case this because MustCallAlias is translated
// into @PolyMustCall, so the common assignment check will fail when assigning
// an @MustCallAlias parameter to an owning field: the parameter is polymorphic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ protected TransferResult<NullnessValue, NullnessStore> strengthenAnnotationOfEqu
thenStore = thenStore == null ? res.getThenStore() : thenStore;
elseStore = elseStore == null ? res.getElseStore() : elseStore;
// TODO: methodTree is null for lambdas. Handle that case. See Issue3850.java.
MethodTree methodTree = analysis.getContainingMethod(secondNode.getTree());
MethodTree methodTree = analysis.getEnclosingMethod(secondNode.getTree());
ExecutableElement methodElem =
methodTree == null ? null : TreeUtils.elementFromDeclaration(methodTree);
if (notEqualTo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public void handleTernaryIsPresentGet(ConditionalExpressionTree tree) {
tree,
"prefer.map.and.orelse",
receiver,
// The literal "CONTAININGCLASS::" is gross.
// The literal "ENCLOSINGCLASS::" is gross.
// TODO: add this to the error message.
// ElementUtils.getQualifiedClassName(ele);
ele.getSimpleName(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
### Error messages for the Optional Checker
prefer.map.and.orelse=It is better style to use map and orElse.%nConsider changing to:%n%s.map(CONTAININGCLASS::%s).orElse(%s)
prefer.map.and.orelse=It is better style to use map and orElse.%nConsider changing to:%n%s.map(ENCLOSINGCLASS::%s).orElse(%s)
prefer.ifpresent=It is better style to use ifPresent.%nConsider changing to:%n%s.ifPresent(%s)
introduce.eliminate=It is bad style to create an Optional just to chain methods to get a non-optional value.
optional.as.element.type=Don't use Optional as the element type in a collection.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,8 @@ public int hashCode() {
*
* <ul>
* <li>it is passed to another method or constructor in an @MustCallAlias position, and then
* the containing method returns that method’s result, or the call is a super()
* constructor call annotated with {@link MustCallAlias}, or
* the enclosing method returns that method’s result, or the call is a super() constructor
* call annotated with {@link MustCallAlias}, or
* <li>it is stored in an owning field of the class under analysis
* </ul>
*/
Expand Down Expand Up @@ -1066,7 +1066,7 @@ private void removeObligationsAtOwnershipTransferToParameters(
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.
// enclosing method must actually return the value.
if (!localObligation.derivedFromMustCallAlias()) {
// Transfer ownership!
obligations.remove(localObligation);
Expand Down Expand Up @@ -1178,9 +1178,8 @@ private void updateObligationsForAssignment(

LocalVariableNode rhsVar = (LocalVariableNode) rhs;

MethodTree containingMethod = cfg.getContainingMethod(assignmentNode.getTree());
boolean inConstructor =
containingMethod != null && TreeUtils.isConstructor(containingMethod);
MethodTree enclosingMethod = cfg.getEnclosingMethod(assignmentNode.getTree());
boolean inConstructor = enclosingMethod != null && TreeUtils.isConstructor(enclosingMethod);

// Determine which obligations this field assignment can clear. In a constructor,
// assignments to `this.field` only clears obligations on normal return, since
Expand Down Expand Up @@ -1489,7 +1488,7 @@ private void checkReassignmentToField(Set<Obligation> obligations, AssignmentNod

// TODO: it would be better to defer getting the path until after checking
// for a CreatesMustCallFor annotation, because getting the path can be expensive.
// It might be possible to exploit the CFG structure to find the containing
// It might be possible to exploit the CFG structure to find the enclosing
// method (rather than using the path, as below), because if a method is being
// analyzed then it should be the root of the CFG (I think).
TreePath currentPath = typeFactory.getPath(node.getTree());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,12 +501,12 @@ public Set<EnsuresCalledMethodOnExceptionContract> getExceptionalPostconditions(
* @return whether that method is one of the must-call methods for its enclosing class
*/
private boolean isMustCallMethod(ExecutableElement elt) {
TypeElement containingClass = ElementUtils.enclosingTypeElement(elt);
TypeElement enclosingClass = ElementUtils.enclosingTypeElement(elt);
MustCallAnnotatedTypeFactory mustCallAnnotatedTypeFactory =
getTypeFactoryOfSubchecker(MustCallChecker.class);
AnnotationMirror mcAnno =
mustCallAnnotatedTypeFactory
.getAnnotatedType(containingClass)
.getAnnotatedType(enclosingClass)
.getPrimaryAnnotationInHierarchy(mustCallAnnotatedTypeFactory.TOP);
List<String> mcValues =
AnnotationUtils.getElementValueArray(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,25 @@ public IdentityHashMap<Node, V> getNodeValues() {
*
* @param t the given tree
* @return the contained method tree of the given tree
* @deprecated use {@link #getEnclosingMethod}
*/
@Deprecated // 2024-05-01
public @Nullable MethodTree getContainingMethod(Tree t) {
return getEnclosingMethod(t);
}

/**
* Get the {@link MethodTree} of the current CFG if the argument {@link Tree} maps to a {@link
* Node} in the CFG or {@code null} otherwise.
*
* @param t the given tree
* @return the contained method tree of the given tree
*/
public @Nullable MethodTree getEnclosingMethod(Tree t) {
if (cfg == null) {
return null;
}
return cfg.getContainingMethod(t);
return cfg.getEnclosingMethod(t);
}

/**
Expand All @@ -318,12 +331,25 @@ public IdentityHashMap<Node, V> getNodeValues() {
*
* @param t the given tree
* @return the contained class tree of the given tree
* @deprecated use {@link #getEnclosingClass}
*/
@Deprecated // 2024-05-01
public @Nullable ClassTree getContainingClass(Tree t) {
return getEnclosingClass(t);
}

/**
* Get the {@link ClassTree} of the current CFG if the argument {@link Tree} maps to a {@link
* Node} in the CFG or {@code null} otherwise.
*
* @param t the given tree
* @return the contained class tree of the given tree
*/
public @Nullable ClassTree getEnclosingClass(Tree t) {
if (cfg == null) {
return null;
}
return cfg.getContainingClass(t);
return cfg.getEnclosingClass(t);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public UnmodifiableIdentityHashMap<UnaryTree, BinaryTree> getPostfixNodeLookup()
* @param t a tree that might correspond to a node in the CFG
* @return the method that contains {@code t}'s Node, or null
*/
public @Nullable MethodTree getContainingMethod(Tree t) {
public @Nullable MethodTree getEnclosingMethod(Tree t) {
if (treeLookup.containsKey(t) && underlyingAST.getKind() == UnderlyingAST.Kind.METHOD) {
UnderlyingAST.CFGMethod cfgMethod = (UnderlyingAST.CFGMethod) underlyingAST;
return cfgMethod.getMethod();
Expand All @@ -421,7 +421,7 @@ public UnmodifiableIdentityHashMap<UnaryTree, BinaryTree> getPostfixNodeLookup()
* @param t a tree that might be within a class
* @return the class that contains the given tree, or null
*/
public @Nullable ClassTree getContainingClass(Tree t) {
public @Nullable ClassTree getEnclosingClass(Tree t) {
if (treeLookup.containsKey(t) && underlyingAST.getKind() == UnderlyingAST.Kind.METHOD) {
UnderlyingAST.CFGMethod cfgMethod = (UnderlyingAST.CFGMethod) underlyingAST;
return cfgMethod.getClassTree();
Expand Down
14 changes: 10 additions & 4 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ Version 3.43.1 (June 3, 2024)

**Implementation details:**

Renamed `CFAbstractStore#methodValues` to
`CFAbstractStore#methodCallExpressions`.
Renamed methods:
* `CFAbstractStore.methodValues()` => `methodCallExpressions()`

Deprecated methods:
* `AbstractAnalysis.getContainingMethod()` => `getEnclosingMethod()`
* `AbstractAnalysis.getContainingClass()` => `getEnclosingMethod()`
* `ControlFlowGraph.getContainingMethod()` => `getEnclosingMethod()`
* `ControlFlowGraph.getContainingClass()` => `getEnclosingClass()`
* `JavaExpression.isUnassignableByOtherCode()` => `isAssignableByOtherCode()`
* `JavaExpression.isUnmodifiableByOtherCode()` => `isModifiableByOtherCode()`

Deprecated `isUnassignableByOtherCode()` in favor of `isAssignableByOtherCode()`.
Deprecated `isUnmodifiableByOtherCode()` in favor of `isModifiableByOtherCode()`.

**Closed issues:**

Expand Down
2 changes: 1 addition & 1 deletion docs/manual/signature-checker.tex
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
An extension of binary name format to represent primitives and arrays.
It is like \refqualclass{checker/signature/qual}{FullyQualifiedName}, but using
``\<\$>'' instead of ``\<.>'' to separate nested classes from their
containing classes. For example, \<"pkg.Outer\$Inner"> or
enclosing classes. For example, \<"pkg.Outer\$Inner"> or
\<"pkg.Outer\$Inner[][]"> or \<"int[]">.
\item[\refqualclass{checker/signature/qual}{CanonicalName}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ private static int printClassDefinitions(
*
* @param innermostTypeElt the innermost type element: either an inner class or an outer class
* without any inner classes that should be printed
* @param classNames the names of the containing classes, from outer to inner
* @param classNames the names of the enclosing classes, from outer to inner
* @return an array of TypeElements whose entry at a given index represents the type named at that
* index in {@code classNames}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,14 +416,14 @@ public void updateContracts(

// This code handles fields of "this" and method parameters (including the receiver
// parameter "this"), for now. In the future, extend it to other expressions.
TypeElement containingClass = (TypeElement) methodElt.getEnclosingElement();
ThisReference thisReference = new ThisReference(containingClass.asType());
ClassName classNameReceiver = new ClassName(containingClass.asType());
TypeElement enclosingClass = (TypeElement) methodElt.getEnclosingElement();
ThisReference thisReference = new ThisReference(enclosingClass.asType());
ClassName classNameReceiver = new ClassName(enclosingClass.asType());
// Fields of "this":
for (VariableElement fieldElement :
ElementFilter.fieldsIn(containingClass.getEnclosedElements())) {
ElementFilter.fieldsIn(enclosingClass.getEnclosedElements())) {
if (atypeFactory.wpiOutputFormat == OutputFormat.JAIF
&& containingClass.getNestingKind().isNested()) {
&& enclosingClass.getNestingKind().isNested()) {
// Don't infer facts about fields of inner classes, because IndexFileWriter
// places the annotations incorrectly on the class declarations.
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -867,15 +867,15 @@ public TransferResult<V, S> visitReturn(ReturnNode n, TransferInput<V, S> p) {

if (shouldPerformWholeProgramInference(n.getTree())) {
// Retrieves class containing the method
ClassTree classTree = analysis.getContainingClass(n.getTree());
ClassTree classTree = analysis.getEnclosingClass(n.getTree());
// classTree is null e.g. if this is a return statement in a lambda.
if (classTree == null) {
return result;
}
ClassSymbol classSymbol = (ClassSymbol) TreeUtils.elementFromDeclaration(classTree);

ExecutableElement methodElem =
TreeUtils.elementFromDeclaration(analysis.getContainingMethod(n.getTree()));
TreeUtils.elementFromDeclaration(analysis.getEnclosingMethod(n.getTree()));

Map<AnnotatedDeclaredType, ExecutableElement> overriddenMethods =
AnnotatedTypes.overriddenMethods(
Expand All @@ -886,7 +886,7 @@ public TransferResult<V, S> visitReturn(ReturnNode n, TransferInput<V, S> p) {
.atypeFactory
.getWholeProgramInference()
.updateFromReturn(
n, classSymbol, analysis.getContainingMethod(n.getTree()), overriddenMethods);
n, classSymbol, analysis.getEnclosingMethod(n.getTree()), overriddenMethods);
}

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -910,8 +910,8 @@ private String javaParserNodeToStringTruncated(Node n, int length) {
* removed after processing the type's members. Otherwise, this method removes them.
*
* @param typeDecl the type declaration to process
* @param outerTypeName the name of the containing class, when processing a nested class;
* otherwise null
* @param outerTypeName the name of the enclosing class, when processing a nested class; otherwise
* null
* @param classTree the tree corresponding to typeDecl if processing an ajava file, null otherwise
* @return a list of types variables for {@code typeDecl}. Only non-null if processing an ajava
* file, in which case the contents should be removed from {@link #typeParameters} after
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ public static Map<AnnotatedDeclaredType, ExecutableElement> overriddenMethods(
}

/**
* Given a method and all supertypes (recursively) of the method's containing class, returns the
* Given a method and all supertypes (recursively) of the method's enclosing class, returns the
* methods that the method overrides.
*
* @param method the overriding method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ public static String findPathTo(@Nullable Class<?> cls, boolean errIfFromDirecto
}
String name = cls.getName();
String classFileName;
/* name is something like pakkage.name.ContainingClass$ClassName. We need to turn this into ContainingClass$ClassName.class. */
/* name is something like pakkage.name.EnclosingClass$ClassName. We need to turn this into EnclosingClass$ClassName.class. */
{
int idx = name.lastIndexOf('.');
classFileName = (idx == -1 ? name : name.substring(idx + 1)) + ".class";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ protected boolean isAccepted() {
*
* @param methodChildElem some element that is a child of a method typeDeclaration (e.g. a
* parameter or return type)
* @return the MethodSymbol of the method containing methodChildElem
* @return the MethodSymbol of the method enclosing methodChildElem
*/
public static Symbol.MethodSymbol getParentMethod(Element methodChildElem) {
if (!(methodChildElem.getEnclosingElement() instanceof Symbol.MethodSymbol)) {
Expand Down

0 comments on commit bd54383

Please sign in to comment.