QL: add unused-field query #7763
Conversation
erik-krogh
force-pushed
the
unused-field
branch
3 times, most recently
from
e2fe297 to
7956b67
Compare
erik-krogh
force-pushed
the
unused-field
branch
from
7956b67 to
f3ca6bb
Compare
The expected output changed after I fixed it, so I pushed the updated output. |
| // The `implClz` does not override `field` with a more specific type. | ||
| not exists(FieldDecl override | | ||
| override = implClz.getDeclaration().getAField() and | ||
| override.getName() = field.getName() and | ||
| override.hasAnnotation("override") and | ||
| override.getVarDecl().getType() != field.getVarDecl().getType() | ||
| ) |
There was a problem hiding this comment.
Is this relevant? The field should still be mentioned in a char-pred, right? Merely overriding the type doesn't prevent the cartesian product.
There was a problem hiding this comment.
You're right, that should not be relevant.
Removing that restriction produces 1 extra alert here:
But that result is an FP for other reasons (the field is bound in
DeclarationEntryNode).
I'll look into it.
There was a problem hiding this comment.
I fixed the FP by just looking at the name of the field, instead of checking that the FieldDecl was the same.
(The FieldDecl was different because of an override).
| // The field is not accessed in the charpred (of any of the classes) | ||
| not exists(FieldAccess access | | ||
| access.getEnclosingPredicate() = | ||
| [clz.getDeclaration().getCharPred(), implClz.getDeclaration().getCharPred()] |
There was a problem hiding this comment.
isn't clz implied by implClz, as you're using the * operator in L16?
There was a problem hiding this comment.
If not, this can be [clz, implClz].getDeclaration().getCharPred(), right?
There was a problem hiding this comment.
isn't
clzimplied byimplClz, as you're using the * operator in L16?
Not necessarily. There are other restrictions in the query that makes it so clz and implClz must sometimes be different classes. (I don't have any examples to link to though).
If not, this can be [clz, implClz].getDeclaration().getCharPred(), right?
Yes it can. I'll do that.
| @@ -96,8 +96,6 @@ private class IntentFlagsOrDataChangedSanitizer extends IntentUriPermissionManip | |||
| * ``` | |||
| */ | |||
| private class IntentFlagsOrDataCheckedGuard extends IntentUriPermissionManipulationGuard { | |||
| Expr condition; | |||
Inspired by a recent code-review.
Detects when a field is not bound by the charpred.
Also handles the cases where the field is defined in an abstract super-class.
I fixed all five errors detected by the query. They were pretty benign except for the Python warning in
Flask.qll.(I think the error in
Flask.qllwas from not copy-pasting a line from the above class, so I did that).