Skip to content

Commit a91c586

Browse files
committed
Python: Infer expression contexts in QL rather than in the extractor
Adds a simple recursive scheme for inferring expression contexts. This means we no longer need to do this in the extractor, but the present commit does not attempt to remove that code. Overall this is an improvement to the readability of the code. Instead of writing `py_expr_contexts(_, 2, foo)`, we can write `foo.isDeleteContext()`. I validated this by running the newly added `ExprContextParity` query on `python/cpython`, where it had zero results (meaning all of the inferred contexts match exactly to the ones generated by the extractor). Finally, the `UselessClass` query used `ctx instanceof AugAssign`, which never did anything in the first place since `AugAssign` isn't a valid context type. I therefore refactored it to (presumably) capture the original intent of also matching augmented assignments.
1 parent 9b72529 commit a91c586

14 files changed

Lines changed: 201 additions & 30 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: feature
3+
---
4+
* Added `Expr.isLoadContext()`, `Expr.isStoreContext()`, `Expr.isParameterContext()`, and
5+
`Expr.isDeleteContext()` to classify expression contexts from the AST structure.

‎python/ql/lib/semmle/python/Exprs.qll‎

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,77 @@ module;
44
private import python
55
private import semmle.python.internal.CachedStages
66

7+
private newtype TContext =
8+
TDel() or
9+
TLoad() or
10+
TParam() or
11+
TStore()
12+
13+
private predicate hasExpressionContext(Expr e) {
14+
e instanceof Attribute or
15+
e instanceof List or
16+
e instanceof Name or
17+
e instanceof PlaceHolder or
18+
e instanceof Starred or
19+
e instanceof Subscript or
20+
e instanceof TemplateDottedNotation_ or
21+
e instanceof Tuple
22+
}
23+
24+
private predicate propagatesExpressionContext(Expr parent, Expr child) {
25+
child in [parent.(List).getAnElt(), parent.(Tuple).getAnElt(), parent.(Starred).getValue()]
26+
}
27+
28+
private predicate hasNonLoadExpressionContextSeed(Expr e, TContext context) {
29+
context = TStore() and
30+
e in [
31+
any(Assign a).getATarget(),
32+
any(AnnAssign a).getTarget(),
33+
any(AssignExpr a).getTarget(),
34+
any(For f).getTarget(),
35+
any(Comprehension c).getTarget(),
36+
any(With w).getOptionalVars(),
37+
any(ExceptStmt ex).getName(),
38+
any(ExceptGroupStmt ex).getName(),
39+
any(Alias a).getAsname(),
40+
any(MatchAsPattern p).getAlias(),
41+
any(MatchCapturePattern p).getVariable(),
42+
any(TypeAlias t).getName(),
43+
any(TypeVar t).getName(),
44+
any(TypeVarTuple t).getName(),
45+
any(ParamSpec p).getName()
46+
]
47+
or
48+
context = TParam() and
49+
(
50+
e = any(Function f).getAnArg()
51+
or
52+
e in [
53+
any(Function f).getVararg(),
54+
any(Function f).getAKwonlyarg(),
55+
any(Function f).getKwarg()
56+
]
57+
)
58+
or
59+
context = TDel() and
60+
e = any(Delete d).getATarget()
61+
}
62+
63+
private predicate inferredNonLoadExpressionContext(Expr e, TContext context) {
64+
exists(Expr root |
65+
hasNonLoadExpressionContextSeed(root, context) and
66+
propagatesExpressionContext*(root, e)
67+
)
68+
}
69+
70+
private predicate inferredExpressionContext(Expr e, TContext context) {
71+
inferredNonLoadExpressionContext(e, context)
72+
or
73+
context = TLoad() and
74+
hasExpressionContext(e) and
75+
not inferredNonLoadExpressionContext(e, _)
76+
}
77+
778
/** An expression */
879
class Expr extends Expr_, AstNode {
980
/** Gets the scope of this expression */
@@ -54,6 +125,18 @@ class Expr extends Expr_, AstNode {
54125
/** Gets an immediate (non-nested) sub-expression of this expression */
55126
Expr getASubExpression() { none() }
56127

128+
/** Whether this expression occurs in a load context. */
129+
predicate isLoadContext() { inferredExpressionContext(this, TLoad()) }
130+
131+
/** Whether this expression occurs in a store context. */
132+
predicate isStoreContext() { inferredExpressionContext(this, TStore()) }
133+
134+
/** Whether this expression occurs in a parameter context. */
135+
predicate isParameterContext() { inferredExpressionContext(this, TParam()) }
136+
137+
/** Whether this expression occurs in a deletion context. */
138+
predicate isDeleteContext() { inferredExpressionContext(this, TDel()) }
139+
57140
override AstNode getAChildNode() { result = this.getASubExpression() }
58141
}
59142

@@ -432,10 +515,10 @@ class Name extends Name_ {
432515

433516
/** Whether this expression is a definition */
434517
predicate isDefinition() {
435-
py_expr_contexts(_, 5, this)
518+
this.isStoreContext()
436519
or
437520
/* Treat Param as a definition (which it is) */
438-
py_expr_contexts(_, 4, this)
521+
this.isParameterContext()
439522
or
440523
/* The target in an augmented assignment is also a definition (and a use) */
441524
exists(AugAssign aa | aa.getTarget() = this)
@@ -451,7 +534,7 @@ class Name extends Name_ {
451534
}
452535

453536
/** Whether this expression is a deletion */
454-
predicate isDeletion() { py_expr_contexts(_, 2, this) }
537+
predicate isDeletion() { this.isDeleteContext() }
455538

456539
/**
457540
* Whether this expression deletes variable `v`.
@@ -463,7 +546,7 @@ class Name extends Name_ {
463546
}
464547

465548
/** Whether this expression is a use */
466-
predicate isUse() { py_expr_contexts(_, 3, this) }
549+
predicate isUse() { this.isLoadContext() }
467550

468551
/**
469552
* Whether this expression is a use of variable `v`

‎python/ql/lib/semmle/python/Flow.qll‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,19 @@ private Py::AstNode toAst(ControlFlowNode n) { py_flow_bb_node(n, result, _, _)
3535
class ControlFlowNode extends @py_flow_node {
3636
/** Whether this control flow node is a load (including those in augmented assignments) */
3737
predicate isLoad() {
38-
exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 3, e) and not augstore(_, this))
38+
exists(Py::Expr e | e = toAst(this) | e.isLoadContext() and not augstore(_, this))
3939
}
4040

4141
/** Whether this control flow node is a store (including those in augmented assignments) */
4242
predicate isStore() {
43-
exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 5, e) or augstore(_, this))
43+
exists(Py::Expr e | e = toAst(this) | e.isStoreContext() or augstore(_, this))
4444
}
4545

4646
/** Whether this control flow node is a delete */
47-
predicate isDelete() { exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 2, e)) }
47+
predicate isDelete() { exists(Py::Expr e | e = toAst(this) | e.isDeleteContext()) }
4848

4949
/** Whether this control flow node is a parameter */
50-
predicate isParameter() { exists(Py::Expr e | e = toAst(this) | py_expr_contexts(_, 4, e)) }
50+
predicate isParameter() { exists(Py::Expr e | e = toAst(this) | e.isParameterContext()) }
5151

5252
/** Whether this control flow node is a store in an augmented assignment */
5353
predicate isAugStore() { augstore(_, this) }
@@ -848,9 +848,7 @@ class NameNode extends ControlFlowNode {
848848
this.isLoad() and
849849
exists(Py::Name u | this.getNode() = u and u.uses(v))
850850
or
851-
exists(Py::PlaceHolder u |
852-
this.getNode() = u and u.getVariable() = v and u.getCtx() instanceof Py::Load
853-
)
851+
exists(Py::PlaceHolder u | this.getNode() = u and u.getVariable() = v and u.isLoadContext())
854852
or
855853
Scopes::use_of_global_variable(this, v.getScope(), v.getId())
856854
}

‎python/ql/lib/semmle/python/SelfAttribute.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ private predicate self_attribute(Attribute attr, Class cls) {
3535
/** A helper class for UndefinedClassAttribute.ql & MaybeUndefinedClassAttribute.ql */
3636
class SelfAttributeRead extends SelfAttribute {
3737
SelfAttributeRead() {
38-
this.getCtx() instanceof Load and
38+
this.isLoadContext() and
3939
// Be stricter for loads.
4040
// We want to generous as to what is defined (i.e. stores),
4141
// but strict as to what needs to be defined (i.e. loads).
@@ -68,7 +68,7 @@ class SelfAttributeRead extends SelfAttribute {
6868
}
6969

7070
class SelfAttributeStore extends SelfAttribute {
71-
SelfAttributeStore() { this.getCtx() instanceof Store }
71+
SelfAttributeStore() { this.isStoreContext() }
7272

7373
Expr getAssignedValue() { exists(Assign a | a.getATarget() = this | result = a.getValue()) }
7474
}

‎python/ql/lib/semmle/python/controlflow/internal/Cfg.qll‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,16 @@ class ControlFlowNode extends CfgImpl::ControlFlowNode {
122122
* which holds on the destination of compound and unary assignments
123123
* even though the destination is also a write.
124124
*/
125-
predicate isLoad() { py_expr_contexts(_, 3, this.asPyExpr()) }
125+
predicate isLoad() { this.asPyExpr().isLoadContext() }
126126

127127
/** Holds if this flow node is a store (including those in augmented assignments). */
128-
predicate isStore() { py_expr_contexts(_, 5, this.asPyExpr()) or augstore(_, this) }
128+
predicate isStore() { this.asPyExpr().isStoreContext() or augstore(_, this) }
129129

130130
/** Holds if this flow node is a delete. */
131-
predicate isDelete() { py_expr_contexts(_, 2, this.asPyExpr()) }
131+
predicate isDelete() { this.asPyExpr().isDeleteContext() }
132132

133133
/** Holds if this flow node is a parameter. */
134-
predicate isParameter() { py_expr_contexts(_, 4, this.asPyExpr()) }
134+
predicate isParameter() { this.asPyExpr().isParameterContext() }
135135

136136
/** Holds if this flow node is a store in an augmented assignment. */
137137
predicate isAugStore() { augstore(_, this) }
@@ -379,9 +379,7 @@ class NameNode extends ControlFlowNode {
379379
this.isLoad() and
380380
exists(Py::Name u | u = this.getNode() and u.uses(v))
381381
or
382-
exists(Py::PlaceHolder u |
383-
u = this.getNode() and u.getVariable() = v and u.getCtx() instanceof Py::Load
384-
)
382+
exists(Py::PlaceHolder u | u = this.getNode() and u.getVariable() = v and u.isLoadContext())
385383
}
386384

387385
/** Gets the identifier of this name node. */

‎python/ql/src/Classes/ClassAttributes.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class CheckClass extends ClassObject {
6666
private predicate monkeyPatched(string name) {
6767
exists(Attribute a, ControlFlowNode objCfg |
6868
objCfg.getNode() = a.getObject() and
69-
a.getCtx() instanceof Store and
69+
a.isStoreContext() and
7070
PointsTo::points_to(objCfg, _, this, _, _) and
7171
a.getName() = name
7272
)

‎python/ql/src/Classes/OverwritingAttributeInSuperClass.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ predicate self_write_stmt(Stmt s, string attr) {
4343
self = a.getObject() and
4444
s.contains(a) and
4545
self.getId() = "self" and
46-
a.getCtx() instanceof Store and
46+
a.isStoreContext() and
4747
a.getName() = attr
4848
)
4949
}

‎python/ql/src/Classes/UselessClass.ql‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,20 @@ predicate no_inheritance(Class c) {
3434

3535
predicate is_decorated(Class c) { exists(c.getADecorator()) }
3636

37+
private predicate is_stateful_store(Expr e) {
38+
e.isStoreContext()
39+
or
40+
e = any(AugAssign a).getTarget()
41+
}
42+
3743
predicate is_stateful(Class c) {
38-
exists(Function method, ExprContext ctx |
44+
exists(Function method |
3945
method.getScope() = c and
40-
(ctx instanceof Store or ctx instanceof AugStore)
41-
|
42-
exists(Subscript s | s.getScope() = method and s.getCtx() = ctx)
43-
or
44-
exists(Attribute a | a.getScope() = method and a.getCtx() = ctx)
46+
(
47+
exists(Subscript s | s.getScope() = method and is_stateful_store(s))
48+
or
49+
exists(Attribute a | a.getScope() = method and is_stateful_store(a))
50+
)
4551
)
4652
or
4753
exists(Function method, Call call, Attribute a, string name |

‎python/ql/src/analysis/RatioOfDefinitions.ql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ predicate want_to_have_definition(ExprWithPointsTo e) {
1010
/* not builtin object like len, tuple, etc. */
1111
not exists(Value builtin | e.pointsTo(builtin) and builtin.isBuiltin()) and
1212
(
13-
e instanceof Name and e.(Name).getCtx() instanceof Load
13+
e instanceof Name and e.isLoadContext()
1414
or
15-
e instanceof Attribute and e.(Attribute).getCtx() instanceof Load
15+
e instanceof Attribute and e.isLoadContext()
1616
or
1717
e instanceof ImportMember
1818
or

‎python/ql/test/library-tests/expr-context-parity/ExprContextParity.expected‎

Whitespace-only changes.

0 commit comments

Comments
 (0)