Owning fields should not be compared with string containment #6280
Conversation
| private boolean expressionEqualsField(String e, VariableElement field) { | ||
| try { | ||
| JavaExpression je = StringToJavaExpression.atFieldDecl(e, field, this.checker); | ||
| return je instanceof FieldAccess && ((FieldAccess) je).getField().equals(field); |
There was a problem hiding this comment.
Do we also need to check if the receiver is this?
There was a problem hiding this comment.
StringToJavaExpression should handle that for us: it uses the supplied location (field, the declaration of the owning field, in this case) and the given string to find the appropriate program element, and associates that with the result.
A potential point of confusion in my implementation is that field is used for two purposes:
- it's the element corresponding to the owning field, so the object that
StringToJavaExpressionconverts the input string into is compared against it - it is used as the location at which
StringToJavaExpressionshould consider the stringeto occur in the code: that is, the context
There was a problem hiding this comment.
Could I write something like
@MustCall("close")
class SneakyDestructor {
private final @Owning Closeable resource;
// ...
@EnsuresCalledMethods(value = "#1.resource", methods = "close")
void close(SneakyDestructor other) {
other.close();
}
}?
In that case, the field referred to by the expression really is SneakyDestructor.resource, but the destructor clearly doesn't close its own @Owning field.
There was a problem hiding this comment.
I understand what you mean now, thanks. I fleshed out your test case and added it to this PR, and a bit to my surprise the tests are passing.
It's no problem. I'm happy to see this fixed. :) |
… into owning-field-string-comparison
fixes #6276
sorry in advance for the mildly annoying merge with #6271 @Calvin-L