Skip to content

Commit 13568b7

Browse files
RasmusWLRasmusWL
committed
Python: Modernise Statements/ queries
Almost. Left out a few things marked with TODO
1 parent 83d40f1 commit 13568b7

22 files changed

+129
-110
lines changed

‎python/ql/src/Statements/DocStrings.ql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ predicate needs_docstring(Scope s) {
2727
}
2828

2929
predicate function_needs_docstring(Function f) {
30-
not exists(FunctionObject fo, FunctionObject base | fo.overrides(base) and fo.getFunction() = f |
31-
not function_needs_docstring(base.getFunction())
30+
not exists(FunctionValue fo, FunctionValue base | fo.overrides(base) and fo.getScope() = f |
31+
not function_needs_docstring(base.getScope())
3232
) and
3333
f.getName() != "lambda" and
3434
(f.getMetrics().getNumberOfLinesOfCode() - count(f.getADecorator())) > 2 and

‎python/ql/src/Statements/IterableStringOrSequence.ql‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,15 @@
1313

1414
import python
1515

16-
predicate is_a_string_type(ClassObject seqtype) {
17-
seqtype = theBytesType() and major_version() = 2
18-
or
19-
seqtype = theUnicodeType()
20-
}
2116

2217
from
23-
For loop, ControlFlowNode iter, Object str, Object seq, ControlFlowNode seq_origin,
24-
ClassObject strtype, ClassObject seqtype, ControlFlowNode str_origin
18+
For loop, ControlFlowNode iter, Value str, Value seq, ControlFlowNode seq_origin, ControlFlowNode str_origin
2519
where
2620
loop.getIter().getAFlowNode() = iter and
27-
iter.refersTo(str, strtype, str_origin) and
28-
iter.refersTo(seq, seqtype, seq_origin) and
29-
is_a_string_type(strtype) and
30-
seqtype.isIterable() and
31-
not is_a_string_type(seqtype)
32-
select loop, "Iteration over $@, of class " + seqtype.getName() + ", may also iterate over $@.",
21+
iter.pointsTo(str, str_origin) and
22+
iter.pointsTo(seq, seq_origin) and
23+
str.getClass() = ClassValue::str() and
24+
seq.getClass().isIterable() and
25+
not seq.getClass() = ClassValue::str()
26+
select loop, "Iteration over $@, of class " + seq.getClass().getName() + ", may also iterate over $@.",
3327
seq_origin, "sequence", str_origin, "string"

‎python/ql/src/Statements/MismatchInMultipleAssignment.ql‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ predicate mismatched(Assign a, int lcount, int rcount, Location loc, string sequ
3636
}
3737

3838
predicate mismatched_tuple_rhs(Assign a, int lcount, int rcount, Location loc) {
39-
exists(ExprList l, TupleObject r, AstNode origin |
39+
exists(ExprList l, TupleValue r, AstNode origin |
4040
(
4141
a.getATarget().(Tuple).getElts() = l or
4242
a.getATarget().(List).getElts() = l
4343
) and
44-
a.getValue().refersTo(r, origin) and
44+
a.getValue().pointsTo(r, origin) and
4545
loc = origin.getLocation() and
4646
lcount = len(l) and
47-
rcount = r.getLength() and
47+
rcount = r.length() and
4848
lcount != rcount and
4949
not exists(Starred s | l.getAnItem() = s)
5050
)

‎python/ql/src/Statements/ModificationOfLocals.ql‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,17 @@
1212

1313
import python
1414

15-
Object aFunctionLocalsObject() {
16-
exists(Call c, Name n, GlobalVariable v |
17-
c = result.getOrigin() and
18-
n = c.getFunc() and
19-
n.getVariable() = v and
20-
v.getId() = "locals" and
21-
c.getScope() instanceof FastLocalsFunction
22-
)
15+
predicate originIsLocals(ControlFlowNode n) {
16+
n.pointsTo(_, _, Value::named("locals").getACall())
2317
}
2418

2519
predicate modification_of_locals(ControlFlowNode f) {
26-
f.(SubscriptNode).getObject().refersTo(aFunctionLocalsObject()) and
20+
originIsLocals(f.(SubscriptNode).getObject()) and
2721
(f.isStore() or f.isDelete())
2822
or
2923
exists(string mname, AttrNode attr |
3024
attr = f.(CallNode).getFunction() and
31-
attr.getObject(mname).refersTo(aFunctionLocalsObject(), _)
25+
originIsLocals(attr.getObject(mname))
3226
|
3327
mname = "pop" or
3428
mname = "popitem" or

‎python/ql/src/Statements/NonIteratorInForLoop.qhelp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ for addressing the defect. </p>
1717
</recommendation>
1818
<example>
1919
<p>
20-
In this example, the loop may attempt to iterate over <code>None</code>, which is not an iterator.
20+
In this example, the loop may attempt to iterate over <code>None</code>, which is not an iterable.
2121
It is likely that the programmer forgot to test for <code>None</code> before the loop.
2222
</p>
2323
<sample src="NonIteratorInForLoop.py" />

‎python/ql/src/Statements/NonIteratorInForLoop.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ where
2222
not t.failedInference(_) and
2323
not v = Value::named("None") and
2424
not t.isDescriptorType()
25-
select loop, "$@ of class '$@' may be used in for-loop.", origin, "Non-iterator", t, t.getName()
25+
select loop, "$@ of class '$@' may be used in for-loop.", origin, "Non-iterable", t, t.getName()

‎python/ql/src/Statements/RedundantAssignment.ql‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,30 @@ predicate maybe_defined_in_outer_scope(Name n) {
3737
exists(SsaVariable v | v.getAUse().getNode() = n | v.maybeUndefined())
3838
}
3939

40-
Variable relevant_var(Name n) {
41-
n.getVariable() = result and
42-
(corresponding(n, _) or corresponding(_, n))
40+
/* Protection against FPs in projects that offer compatibility between Python 2 and 3,
41+
* since many of them make assignments such as
42+
*
43+
* if PY2:
44+
* bytes = str
45+
* else:
46+
* bytes = bytes
47+
*
48+
*/
49+
predicate isBuiltin(string name) {
50+
exists(Value v | v = Value::named(name) and v.isBuiltin())
4351
}
4452

4553
predicate same_name(Name n1, Name n2) {
4654
corresponding(n1, n2) and
47-
relevant_var(n1) = relevant_var(n2) and
48-
not exists(Object::builtin(n1.getId())) and
55+
n1.getVariable() = n2.getVariable() and
56+
not isBuiltin(n1.getId()) and
4957
not maybe_defined_in_outer_scope(n2)
5058
}
5159

5260
ClassObject value_type(Attribute a) { a.getObject().refersTo(_, result, _) }
5361

5462
predicate is_property_access(Attribute a) {
63+
// TODO: We need something to model PropertyObject in the Value API
5564
value_type(a).lookupAttribute(a.getName()) instanceof PropertyObject
5665
}
5766

@@ -77,9 +86,9 @@ predicate pyflakes_commented(AssignStmt assignment) {
7786
}
7887

7988
predicate side_effecting_lhs(Attribute lhs) {
80-
exists(ClassObject cls, ClassObject decl |
81-
lhs.getObject().refersTo(_, cls, _) and
82-
decl = cls.getAnImproperSuperType() and
89+
exists(ClassValue cls, ClassValue decl |
90+
lhs.getObject().pointsTo().getClass() = cls and
91+
decl = cls.getASuperType() and
8392
not decl.isBuiltin()
8493
|
8594
decl.declaresAttribute("__setattr__")
@@ -90,6 +99,7 @@ from AssignStmt a, Expr left, Expr right
9099
where
91100
assignment(a, left, right) and
92101
same_value(left, right) and
102+
// some people use self-assignment to shut Pyflakes up, such as `ok = ok # Pyflakes`
93103
not pyflakes_commented(a) and
94104
not side_effecting_lhs(left)
95105
select a, "This assignment assigns a variable to itself."

‎python/ql/src/Statements/ShouldUseWithStatement.ql‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ predicate only_stmt_in_finally(Try t, Call c) {
2222
)
2323
}
2424

25-
predicate points_to_context_manager(ControlFlowNode f, ClassObject cls) {
26-
cls.isContextManager() and
27-
forex(Object obj | f.refersTo(obj) | f.refersTo(obj, cls, _))
25+
predicate points_to_context_manager(ControlFlowNode f, ClassValue cls) {
26+
forex(Value v | f.pointsTo(v) | v.getClass() = cls) and
27+
cls.isContextManager()
2828
}
2929

30-
from Call close, Try t, ClassObject cls
30+
from Call close, Try t, ClassValue cls
3131
where
3232
only_stmt_in_finally(t, close) and
3333
calls_close(close) and

‎python/ql/src/Statements/StatementNoEffect.ql‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,78 +13,78 @@
1313

1414
import python
1515

16-
predicate understood_attribute(Attribute attr, ClassObject cls, ClassObject attr_cls) {
16+
predicate understood_attribute(Attribute attr, ClassValue cls, ClassValue attr_cls) {
1717
exists(string name | attr.getName() = name |
18-
attr.getObject().refersTo(_, cls, _) and
19-
cls.attributeRefersTo(name, _, attr_cls, _)
18+
attr.getObject().pointsTo().getClass() = cls and
19+
cls.attr(name).getClass() = attr_cls
2020
)
2121
}
2222

2323
/* Conservative estimate of whether attribute lookup has a side effect */
2424
predicate side_effecting_attribute(Attribute attr) {
25-
exists(ClassObject cls, ClassObject attr_cls |
25+
exists(ClassValue cls, ClassValue attr_cls |
2626
understood_attribute(attr, cls, attr_cls) and
2727
side_effecting_descriptor_type(attr_cls)
2828
)
2929
}
3030

3131
predicate maybe_side_effecting_attribute(Attribute attr) {
32-
not understood_attribute(attr, _, _) and not attr.refersTo(_)
32+
not understood_attribute(attr, _, _) and not attr.pointsTo(_)
3333
or
3434
side_effecting_attribute(attr)
3535
}
3636

37-
predicate side_effecting_descriptor_type(ClassObject descriptor) {
37+
predicate side_effecting_descriptor_type(ClassValue descriptor) {
3838
descriptor.isDescriptorType() and
3939
/*
4040
* Technically all descriptor gets have side effects,
4141
* but some are indicative of a missing call and
4242
* we want to treat them as having no effect.
4343
*/
4444

45-
not descriptor = thePyFunctionType() and
46-
not descriptor = theStaticMethodType() and
47-
not descriptor = theClassMethodType()
45+
not descriptor = ClassValue::function() and
46+
not descriptor = ClassValue::staticmethod() and
47+
not descriptor = ClassValue::classmethod()
4848
}
4949

5050
/**
5151
* Side effecting binary operators are rare, so we assume they are not
5252
* side-effecting unless we know otherwise.
5353
*/
5454
predicate side_effecting_binary(Expr b) {
55-
exists(Expr sub, ClassObject cls, string method_name |
55+
exists(Expr sub, ClassValue cls, string method_name |
5656
binary_operator_special_method(b, sub, cls, method_name)
5757
or
5858
comparison_special_method(b, sub, cls, method_name)
5959
|
6060
method_name = special_method() and
6161
cls.hasAttribute(method_name) and
62-
not exists(ClassObject declaring |
62+
not exists(ClassValue declaring |
6363
declaring.declaresAttribute(method_name) and
64-
declaring = cls.getAnImproperSuperType() and
64+
declaring = cls.getASuperType() and
6565
declaring.isBuiltin() and
66-
not declaring = theObjectType()
66+
not declaring = ClassValue::object()
6767
)
6868
)
6969
}
7070

7171
pragma[nomagic]
7272
private predicate binary_operator_special_method(
73-
BinaryExpr b, Expr sub, ClassObject cls, string method_name
73+
BinaryExpr b, Expr sub, ClassValue cls, string method_name
7474
) {
7575
method_name = special_method() and
7676
sub = b.getLeft() and
7777
method_name = b.getOp().getSpecialMethodName() and
78-
sub.refersTo(_, cls, _)
78+
sub.pointsTo().getClass() = cls
7979
}
8080

8181
pragma[nomagic]
82-
private predicate comparison_special_method(Compare b, Expr sub, ClassObject cls, string method_name) {
82+
private predicate comparison_special_method(Compare b, Expr sub, ClassValue cls, string method_name) {
8383
exists(Cmpop op |
8484
b.compares(sub, op, _) and
8585
method_name = op.getSpecialMethodName()
8686
) and
87-
sub.refersTo(_, cls, _)
87+
sub.pointsTo().getClass() = cls
8888
}
8989

9090
private string special_method() {
@@ -102,9 +102,8 @@ predicate is_notebook(File f) {
102102
/** Expression (statement) in a jupyter/ipython notebook */
103103
predicate in_notebook(Expr e) { is_notebook(e.getScope().(Module).getFile()) }
104104

105-
FunctionObject assertRaises() {
106-
result =
107-
ModuleObject::named("unittest").attr("TestCase").(ClassObject).lookupAttribute("assertRaises")
105+
FunctionValue assertRaises() {
106+
result = Value::named("unittest.TestCase").(ClassValue).lookup("assertRaises")
108107
}
109108

110109
/** Holds if expression `e` is in a `with` block that tests for exceptions being raised. */
@@ -124,6 +123,7 @@ predicate python2_print(Expr e) {
124123
}
125124

126125
predicate no_effect(Expr e) {
126+
// strings can be used as comments
127127
not e instanceof StrConst and
128128
not e.hasSideEffects() and
129129
forall(Expr sub | sub = e.getASubExpression*() |

‎python/ql/src/Statements/StringConcatenationInLoop.qhelp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
"qhelp.dtd">
44
<qhelp>
55
<overview>
6-
<p>If you concatenate strings in a loop then the time taken by the loop is quadratic in the number
6+
<p>If you concatenate strings in a loop then the time taken by the loop is quadratic in the number
77
of iterations.</p>
88

99
</overview>
1010
<recommendation>
1111

12-
<p>Initialize an empty list before the start of the list.
12+
<p>Initialize an empty list before the start of the loop.
1313
During the loop append the substrings to the list.
1414
At the end of the loop, convert the list to a string by using <code>''.join(list)</code>.</p>
1515

0 commit comments

Comments
 (0)