mirror of
https://github.com/github/codeql.git
synced 2026-04-30 19:26:02 +02:00
Initial commit of Python queries and QL libraries.
This commit is contained in:
committed by
Mark Shannon
parent
90c75cd362
commit
5f58824d1b
19
python/ql/src/Functions/ConsistentReturns.py
Normal file
19
python/ql/src/Functions/ConsistentReturns.py
Normal file
@@ -0,0 +1,19 @@
|
||||
def check_state1(state, interactive=True):
|
||||
if not state['good'] or not state['bad']:
|
||||
if (good or bad or skip or reset) and interactive:
|
||||
return # implicitly return None
|
||||
if not state['good']:
|
||||
raise util.Abort(_('cannot bisect (no known good revisions)'))
|
||||
else:
|
||||
raise util.Abort(_('cannot bisect (no known bad revisions)'))
|
||||
return True
|
||||
|
||||
def check_state2(state, interactive=True):
|
||||
if not state['good'] or not state['bad']:
|
||||
if (good or bad or skip or reset) and interactive:
|
||||
return False # return an explicit value
|
||||
if not state['good']:
|
||||
raise util.Abort(_('cannot bisect (no known good revisions)'))
|
||||
else:
|
||||
raise util.Abort(_('cannot bisect (no known bad revisions)'))
|
||||
return True
|
||||
37
python/ql/src/Functions/ConsistentReturns.qhelp
Normal file
37
python/ql/src/Functions/ConsistentReturns.qhelp
Normal file
@@ -0,0 +1,37 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>When a function contains both explicit returns (<code>return value</code>) and implicit returns
|
||||
(where code falls off the end of a function) this often indicates that a return
|
||||
statement has been forgotten. It is best to return an explicit return value even when returning
|
||||
<code>None</code> because this makes it easier for other developers to read your code.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Add an explicit return at the end of the function.</p>
|
||||
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In the <code>check_state1</code> function, the developer probably did intend to use an implicit
|
||||
return value of <code>None</code> as this equates to <code>False</code>. However, the function in
|
||||
<code>check_state2</code> is easier to read.</p>
|
||||
|
||||
<sample src="ConsistentReturns.py" />
|
||||
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/2/reference/compound_stmts.html#function">Function definitions</a>.
|
||||
</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
32
python/ql/src/Functions/ConsistentReturns.ql
Normal file
32
python/ql/src/Functions/ConsistentReturns.ql
Normal file
@@ -0,0 +1,32 @@
|
||||
/**
|
||||
* @name Explicit returns mixed with implicit (fall through) returns
|
||||
* @description Mixing implicit and explicit returns indicates a likely error as implicit returns always return 'None'.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* maintainability
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision high
|
||||
* @id py/mixed-returns
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
predicate explicitly_returns_non_none(Function func) {
|
||||
exists(Return return | return.getScope() = func and
|
||||
exists(Expr val |
|
||||
val= return.getValue() |
|
||||
not val instanceof None
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
predicate has_implicit_return(Function func) {
|
||||
exists(ControlFlowNode fallthru | fallthru = func.getFallthroughNode() and not fallthru.unlikelyReachable()) or
|
||||
exists(Return return | return.getScope() = func and not exists(return.getValue()))
|
||||
}
|
||||
|
||||
|
||||
from Function func
|
||||
where explicitly_returns_non_none(func) and has_implicit_return(func)
|
||||
select func, "Mixing implicit and explicit returns may indicate an error as implicit returns always return None."
|
||||
37
python/ql/src/Functions/DeprecatedSliceMethod.qhelp
Normal file
37
python/ql/src/Functions/DeprecatedSliceMethod.qhelp
Normal file
@@ -0,0 +1,37 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The <code>__getslice__</code>, <code>__setslice__</code> and <code>__delslice__</code> methods have been deprecated since Python 2.0.
|
||||
In general, no class should implement these methods.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
The only exceptions to this rule are classes that inherit from <code>list</code> and override <code>__getitem__</code>,
|
||||
<code>__setitem__</code> or <code>__delitem__</code>.
|
||||
Since <code>list</code> implements the slicing methods any class inheriting from <code>list</code> must implement the
|
||||
the slicing methods to ensure correct behavior of <code>__getitem__</code>, <code>__setitem__</code> and <code>__delitem__</code>.
|
||||
These exceptions to the rule will not be treated as violations.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>
|
||||
Delete the slicing method. Any functionality should be moved to the equivalent <code>__xxxitem__</code> method:
|
||||
</p>
|
||||
<ul>
|
||||
<li><code>__getslice__</code> should be replaced with <code>__getitem__</code></li>
|
||||
<li><code>__setslice__</code> should be replaced with <code>__setitem__</code></li>
|
||||
<li><code>__delslice__</code> should be replaced with <code>__delitem__</code></li>
|
||||
</ul>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="https://docs.python.org/2/reference/datamodel.html#additional-methods-for-emulation-of-sequence-types">
|
||||
Additional methods for emulation of sequence types</a>.
|
||||
</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
24
python/ql/src/Functions/DeprecatedSliceMethod.ql
Normal file
24
python/ql/src/Functions/DeprecatedSliceMethod.ql
Normal file
@@ -0,0 +1,24 @@
|
||||
/**
|
||||
* @name Deprecated slice method
|
||||
* @description Defining special methods for slicing has been deprecated since Python 2.0.
|
||||
* @kind problem
|
||||
* @tags maintainability
|
||||
* @problem.severity warning
|
||||
* @sub-severity high
|
||||
* @precision very-high
|
||||
* @id py/deprecated-slice-method
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
predicate slice_method_name(string name) {
|
||||
name = "__getslice__" or name = "__setslice__" or name = "__delslice__"
|
||||
}
|
||||
|
||||
from PyFunctionObject f, string meth
|
||||
|
||||
where f.getFunction().isMethod() and not f.isOverridingMethod() and
|
||||
slice_method_name(meth) and f.getName() = meth
|
||||
|
||||
|
||||
select f, meth + " method has been deprecated since Python 2.0"
|
||||
4
python/ql/src/Functions/ExplicitReturnInInit.py
Normal file
4
python/ql/src/Functions/ExplicitReturnInInit.py
Normal file
@@ -0,0 +1,4 @@
|
||||
class ExplicitReturnInInit(object):
|
||||
def __init__(self, i):
|
||||
self.i = i
|
||||
return self
|
||||
28
python/ql/src/Functions/ExplicitReturnInInit.qhelp
Normal file
28
python/ql/src/Functions/ExplicitReturnInInit.qhelp
Normal file
@@ -0,0 +1,28 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The <code>__init__</code> method of a class is used to initialize new objects,
|
||||
not create them. As such, it should not return any value. Returning <code>None</code>
|
||||
is correct in the sense that no runtime error will occur,
|
||||
but it suggests that the returned value is meaningful, which it is not.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Convert the <code>return expr</code> statement to a plain <code>return</code> statement,
|
||||
or omit it altogether if it is at the end of the method.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example, the <code>__init__</code> method attempts to return the newly created
|
||||
object. This is an error and the return method should be removed.</p>
|
||||
<sample src="ExplicitReturnInInit.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python: <a href="http://docs.python.org/2.7/reference/datamodel.html#object.__init__">The __init__ method</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
23
python/ql/src/Functions/ExplicitReturnInInit.ql
Normal file
23
python/ql/src/Functions/ExplicitReturnInInit.ql
Normal file
@@ -0,0 +1,23 @@
|
||||
/**
|
||||
* @name __init__ method returns a value
|
||||
* @description Explicitly returning a value from an __init__ method will raise a TypeError.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision very-high
|
||||
* @id py/explicit-return-in-init
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
from Return r
|
||||
where exists(Function init | init.isInitMethod() and
|
||||
r.getScope() = init and exists(r.getValue())) and
|
||||
not r.getValue() instanceof None and
|
||||
not exists(FunctionObject f | f.getACall() = r.getValue().getAFlowNode() |
|
||||
f.neverReturns()
|
||||
) and
|
||||
not exists(Attribute meth | meth = ((Call)r.getValue()).getFunc() | meth.getName() = "__init__")
|
||||
select r, "Explicit return in __init__ method."
|
||||
16
python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py
Normal file
16
python/ql/src/Functions/IncorrectRaiseInSpecialMethod.py
Normal file
@@ -0,0 +1,16 @@
|
||||
#Incorrect unhashable class
|
||||
class MyMutableThing(object):
|
||||
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
def __hash__(self):
|
||||
raise NotImplementedError("%r is unhashable" % self)
|
||||
|
||||
#Make class unhashable in the standard way
|
||||
class MyCorrectMutableThing(object):
|
||||
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
__hash__ = None
|
||||
71
python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp
Normal file
71
python/ql/src/Functions/IncorrectRaiseInSpecialMethod.qhelp
Normal file
@@ -0,0 +1,71 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>User-defined classes interact with the Python virtual machine via special methods (also called "magic methods").
|
||||
For example, for a class to support addition it must implement the <code>__add__</code> and <code>__radd__</code> special methods.
|
||||
When the expression <code>a + b</code> is evaluated the Python virtual machine will call <code>type(a).__add__(a, b)</code> and if that
|
||||
is not implemented it will call <code>type(b).__radd__(b, a)</code>.</p>
|
||||
<p>
|
||||
Since the virtual machine calls these special methods for common expressions, users of the class will expect these operations to raise standard exceptions.
|
||||
For example, users would expect that the expression <code>a.b</code> might raise an <code>AttributeError</code>
|
||||
if the object <code>a</code> does not have an attribute <code>b</code>.
|
||||
If a <code>KeyError</code> were raised instead,
|
||||
then this would be unexpected and may break code that expected an <code>AttributeError</code>, but not a <code>KeyError</code>.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
Therefore, if a method is unable to perform the expected operation then its response should conform to the standard protocol, described below.
|
||||
</p>
|
||||
|
||||
<ul>
|
||||
<li>Attribute access, <code>a.b</code>: Raise <code>AttributeError</code></li>
|
||||
<li>Arithmetic operations, <code>a + b</code>: Do not raise an exception, return <code>NotImplemented</code> instead.</li>
|
||||
<li>Indexing, <code>a[b]</code>: Raise <code>KeyError</code>.</li>
|
||||
<li>Hashing, <code>hash(a)</code>: Use <code>__hash__ = None</code> to indicate that an object is unhashable.</li>
|
||||
<li>Equality methods, <code>a != b</code>: Never raise an exception, always return <code>True</code> or <code>False</code>.</li>
|
||||
<li>Ordering comparison methods, <code>a < b</code>: Raise a <code>TypeError</code> if the objects cannot be ordered.</li>
|
||||
<li>Most others: Ideally, do not implement the method at all, otherwise raise <code>TypeError</code> to indicate that the operation is unsupported.</li>
|
||||
</ul>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>If the method is meant to be abstract, then declare it so using the <code>@abstractmethod</code> decorator.
|
||||
Otherwise, either remove the method or ensure that the method raises an exception of the correct type.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
|
||||
<p>
|
||||
This example shows two unhashable classes. The first class is unhashable in a non-standard way which may cause maintenance problems.
|
||||
The second, corrected, class uses the standard idiom for unhashable classes.
|
||||
</p>
|
||||
<sample src="IncorrectRaiseInSpecialMethod.py" />
|
||||
<p>
|
||||
In this example, the first class is implicitly abstract; the <code>__add__</code> method is unimplemented,
|
||||
presumably with the expectation that it will be implemented by sub-classes.
|
||||
The second class makes this explicit with an <code>@abstractmethod</code> decoration on the unimplemented <code>__add__</code> method.
|
||||
</p>
|
||||
<sample src="IncorrectRaiseInSpecialMethod2.py" />
|
||||
<p>
|
||||
In this last example, the first class implements a collection backed by the file store.
|
||||
However, should an <code>IOError</code> be raised in the <code>__getitem__</code> it will propagate to the caller.
|
||||
The second class handles any <code>IOError</code> by reraising a <code>KeyError</code> which is the standard exception for
|
||||
the <code>__getitem__</code> method.
|
||||
</p>
|
||||
|
||||
<sample src="IncorrectRaiseInSpecialMethod3.py" />
|
||||
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/dev/reference/datamodel.html#special-method-names">Special Method Names</a>.</li>
|
||||
<li>Python Library Reference: <a href="https://docs.python.org/2/library/exceptions.html">Exceptions</a>.</li>
|
||||
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
112
python/ql/src/Functions/IncorrectRaiseInSpecialMethod.ql
Normal file
112
python/ql/src/Functions/IncorrectRaiseInSpecialMethod.ql
Normal file
@@ -0,0 +1,112 @@
|
||||
/**
|
||||
* @name Non-standard exception raised in special method
|
||||
* @description Raising a non-standard exception in a special method alters the expected interface of that method.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* maintainability
|
||||
* convention
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision very-high
|
||||
* @id py/unexpected-raise-in-special-method
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
private predicate attribute_method(string name) {
|
||||
name = "__getattribute__" or name = "__getattr__" or name = "__setattr__"
|
||||
}
|
||||
|
||||
private predicate indexing_method(string name) {
|
||||
name = "__getitem__" or name = "__setitem__" or name = "__delitem__"
|
||||
}
|
||||
|
||||
private predicate arithmetic_method(string name) {
|
||||
name = "__add__" or name = "__sub__" or name = "__div__" or
|
||||
name = "__pos__" or name = "__abs__" or name = "__floordiv__" or
|
||||
name = "__div__" or name = "__divmod__" or name = "__lshift__" or
|
||||
name = "__and__" or name = "__or__"or name = "__xor__" or name = "__rshift__" or
|
||||
name = "__pow__" or name = "__mul__" or name = "__neg__" or
|
||||
name = "__radd__" or name = "__rsub__" or name = "__rdiv__" or
|
||||
name = "__rfloordiv__" or name = "__rdiv__" or name = "__rlshift__" or
|
||||
name = "__rand__" or name = "__ror__"or name = "__rxor__" or name = "__rrshift__" or
|
||||
name = "__rpow__" or name = "__rmul__" or name = "__truediv__" or name = "__rtruediv__" or
|
||||
name = "__iadd__" or name = "__isub__" or name = "__idiv__" or
|
||||
name = "__ifloordiv__" or name = "__idiv__" or name = "__ilshift__" or
|
||||
name = "__iand__" or name = "__ior__"or name = "__ixor__" or name = "__irshift__" or
|
||||
name = "__ipow__" or name = "__imul__" or name = "__itruediv__"
|
||||
}
|
||||
|
||||
private predicate ordering_method(string name) {
|
||||
name = "__lt__" or name = "__le__" or name = "__gt__" or name = "__ge__" or
|
||||
name = "__cmp__" and major_version() = 2
|
||||
}
|
||||
|
||||
private predicate cast_method(string name) {
|
||||
name = "__nonzero__" and major_version() = 2 or
|
||||
name = "__bool__" or
|
||||
name = "__int__" or name = "__float__" or
|
||||
name = "__long__" or
|
||||
name = "__trunc__" or
|
||||
name = "__complex__"
|
||||
}
|
||||
|
||||
predicate correct_raise(string name, ClassObject ex) {
|
||||
ex.getAnImproperSuperType() = theTypeErrorType()
|
||||
and
|
||||
(
|
||||
name = "__copy__" or
|
||||
name = "__deepcopy__" or
|
||||
name = "__call__" or
|
||||
indexing_method(name) or
|
||||
attribute_method(name)
|
||||
)
|
||||
or
|
||||
preferred_raise(name, ex)
|
||||
or
|
||||
preferred_raise(name, ex.getASuperType())
|
||||
}
|
||||
|
||||
predicate preferred_raise(string name, ClassObject ex) {
|
||||
attribute_method(name) and ex = theAttributeErrorType()
|
||||
or
|
||||
indexing_method(name) and ex = builtin_object("LookupError")
|
||||
or
|
||||
ordering_method(name) and ex = theTypeErrorType()
|
||||
or
|
||||
arithmetic_method(name) and ex = builtin_object("ArithmeticError")
|
||||
}
|
||||
|
||||
predicate no_need_to_raise(string name, string message) {
|
||||
name = "__hash__" and message = "use __hash__ = None instead"
|
||||
or
|
||||
cast_method(name) and message = "there is no need to implement the method at all."
|
||||
}
|
||||
|
||||
predicate is_abstract(FunctionObject func) {
|
||||
((Name)func.getFunction().getADecorator()).getId().matches("%abstract%")
|
||||
}
|
||||
|
||||
predicate always_raises(FunctionObject f, ClassObject ex) {
|
||||
ex = f.getARaisedType() and
|
||||
strictcount(f.getARaisedType()) = 1 and
|
||||
not exists(f.getFunction().getANormalExit()) and
|
||||
/* raising StopIteration is equivalent to a return in a generator */
|
||||
not ex = theStopIterationType()
|
||||
}
|
||||
|
||||
from FunctionObject f, ClassObject cls, string message
|
||||
where f.getFunction().isSpecialMethod() and
|
||||
not is_abstract(f) and
|
||||
always_raises(f, cls) and
|
||||
(
|
||||
no_need_to_raise(f.getName(), message) and not cls.getName() = "NotImplementedError"
|
||||
or
|
||||
not correct_raise(f.getName(), cls) and not cls.getName() = "NotImplementedError"
|
||||
and
|
||||
exists(ClassObject preferred |
|
||||
preferred_raise(f.getName(), preferred) |
|
||||
message = "raise " + preferred.getName() + " instead"
|
||||
)
|
||||
)
|
||||
select f, "Function always raises $@; " + message, cls, cls.toString()
|
||||
15
python/ql/src/Functions/IncorrectRaiseInSpecialMethod2.py
Normal file
15
python/ql/src/Functions/IncorrectRaiseInSpecialMethod2.py
Normal file
@@ -0,0 +1,15 @@
|
||||
|
||||
#Abstract base class, but don't declare it.
|
||||
class ImplicitAbstractClass(object):
|
||||
|
||||
def __add__(self, other):
|
||||
raise NotImplementedError()
|
||||
|
||||
#Make abstractness explicit.
|
||||
class ExplicitAbstractClass:
|
||||
__metaclass__ = ABCMeta
|
||||
|
||||
@abstractmethod
|
||||
def __add__(self, other):
|
||||
raise NotImplementedError()
|
||||
|
||||
27
python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py
Normal file
27
python/ql/src/Functions/IncorrectRaiseInSpecialMethod3.py
Normal file
@@ -0,0 +1,27 @@
|
||||
|
||||
#Incorrect file-backed table
|
||||
class FileBackedTable(object):
|
||||
|
||||
def __getitem__(self, key):
|
||||
if key not in self.index:
|
||||
raise IOError("Key '%s' not in table" % key)
|
||||
else:
|
||||
#May raise an IOError
|
||||
return self.backing.get_row(key)
|
||||
|
||||
#Correct by transforming exception
|
||||
class ObjectLikeFileBackedTable(object):
|
||||
|
||||
def get_from_key(self, key):
|
||||
if key not in self.index:
|
||||
raise IOError("Key '%s' not in table" % key)
|
||||
else:
|
||||
#May raise an IOError
|
||||
return self.backing.get_row(key)
|
||||
|
||||
def __getitem__(self, key):
|
||||
try:
|
||||
return self.get_from_key(key)
|
||||
except IOError:
|
||||
raise KeyError(key)
|
||||
|
||||
41
python/ql/src/Functions/IncorrectlyOverriddenMethod.qhelp
Normal file
41
python/ql/src/Functions/IncorrectlyOverriddenMethod.qhelp
Normal file
@@ -0,0 +1,41 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p> There is a call to the overridden method, and potentially the overriding method,
|
||||
with arguments that are not legal for the overriding method.
|
||||
This will cause an error if the overriding method is called and is a
|
||||
violation of the Liskov substitution principle.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure that the overriding method accepts all the parameters that are legal for the
|
||||
overridden method.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example there is a mismatch between the legal parameters for the base
|
||||
class method <code>(self, source, filename, symbol)</code> and the extension method
|
||||
<code>(self, source)</code>. The extension method can be used to override the base
|
||||
method as long as values are not specified for the <code>filename</code> and (optional)
|
||||
<code>symbol</code> parameters. If the extension method was passed the additional
|
||||
parameters accepted by the base method then an error would occur.</p>
|
||||
|
||||
<sample src="SignatureIncorrectlyOverriddenMethod.py" />
|
||||
|
||||
<p>The extension method should be updated to support the <code>filename</code> and
|
||||
<code>symbol</code> parameters supported by the overridden method.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Liskov_substitution_principle">Liskov Substitution Principle</a>, <a href="http://en.wikipedia.org/wiki/Method_overriding#Python">Method overriding</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
27
python/ql/src/Functions/IncorrectlyOverriddenMethod.ql
Normal file
27
python/ql/src/Functions/IncorrectlyOverriddenMethod.ql
Normal file
@@ -0,0 +1,27 @@
|
||||
/**
|
||||
* @name Mismatch between signature and use of an overriding method
|
||||
* @description Method has a different signature from the overridden method and, if it were called, would be likely to cause an error.
|
||||
* @kind problem
|
||||
* @tags maintainability
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/inheritance/incorrect-overriding-signature
|
||||
*/
|
||||
|
||||
import python
|
||||
import Expressions.CallArgs
|
||||
|
||||
from Call call, FunctionObject func, FunctionObject overridden, string problem
|
||||
where
|
||||
func.overrides(overridden) and (
|
||||
wrong_args(call, func, _, problem) and correct_args_if_called_as_method(call, overridden)
|
||||
or
|
||||
exists(string name |
|
||||
illegally_named_parameter(call, func, name) and problem = "an argument named '" + name + "'" and
|
||||
overridden.getFunction().getAnArg().(Name).getId() = name
|
||||
)
|
||||
)
|
||||
|
||||
select func, "Overriding method signature does not match $@, where it is passed " + problem + ". Overridden method $@ is correctly specified.",
|
||||
call, "here", overridden, overridden.descriptiveString()
|
||||
@@ -0,0 +1,39 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p> There is a call to the overriding method, and potentially the overridden method,
|
||||
with arguments that are not legal for the overridden method.
|
||||
This will cause an error if the overridden method is called and is a
|
||||
violation of the Liskov substitution principle.
|
||||
</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure that the overridden method accepts all the parameters that are legal for
|
||||
overriding method(s).</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example there is a mismatch between the legal parameters for the base
|
||||
class method <code>(self, source, filename)</code> and the extension method
|
||||
<code>(self, source)</code>. Since there is a call that uses the signature of the extension method
|
||||
then it can be inferred that the base signature is erroneous and should be updated to
|
||||
match that of the extension method.
|
||||
</p>
|
||||
|
||||
<sample src="SignatureOverridingMethod.py" />
|
||||
|
||||
<p>The base method should be updated to either remove the <code>filename</code> parameters, or add a default value for it.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Liskov_substitution_principle">Liskov Substitution Principle</a>, <a href="http://en.wikipedia.org/wiki/Method_overriding#Python">Method overriding</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,35 @@
|
||||
/**
|
||||
* @name Mismatch between signature and use of an overridden method
|
||||
* @description Method has a signature that differs from both the signature of its overriding methods and
|
||||
* the arguments with which it is called, and if it were called, would be likely to cause an error.
|
||||
* @kind problem
|
||||
* @tags maintainability
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/inheritance/incorrect-overridden-signature
|
||||
*/
|
||||
|
||||
import python
|
||||
import Expressions.CallArgs
|
||||
|
||||
from Call call, FunctionObject func, FunctionObject overriding, string problem
|
||||
where
|
||||
not func.getName() = "__init__" and
|
||||
overriding.overrides(func) and
|
||||
call = overriding.getAMethodCall().getNode() and
|
||||
correct_args_if_called_as_method(call, overriding) and
|
||||
(
|
||||
arg_count(call)+1 < func.minParameters() and problem = "too few arguments"
|
||||
or
|
||||
arg_count(call) >= func.maxParameters() and problem = "too many arguments"
|
||||
or
|
||||
exists(string name | call.getAKeyword().getArg() = name and
|
||||
overriding.getFunction().getAnArg().(Name).getId() = name and
|
||||
not func.getFunction().getAnArg().(Name).getId() = name and
|
||||
problem = "an argument named '" + name + "'"
|
||||
)
|
||||
)
|
||||
|
||||
select func, "Overridden method signature does not match $@, where it is passed " + problem + ". Overriding method $@ matches the call.",
|
||||
call, "call", overriding, overriding.descriptiveString()
|
||||
3
python/ql/src/Functions/InitIsGenerator.py
Normal file
3
python/ql/src/Functions/InitIsGenerator.py
Normal file
@@ -0,0 +1,3 @@
|
||||
class InitIsGenerator(object):
|
||||
def __init__(self, i):
|
||||
yield i
|
||||
28
python/ql/src/Functions/InitIsGenerator.qhelp
Normal file
28
python/ql/src/Functions/InitIsGenerator.qhelp
Normal file
@@ -0,0 +1,28 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The <code>__init__</code> method of a class is used to initialize new objects,
|
||||
not create them. As such, it should not return any value.
|
||||
By including a <code>yield</code> expression in the method turns it into a generator method.
|
||||
On calling it will return a generator resulting in a runtime error.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>The presence of a <code>yield</code> expression in an <code>__init__</code> method
|
||||
suggests a logical error, so it is not possible to suggest a general fix.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example the <code>__init__</code> method contains a yield expression. This is
|
||||
not logical in the context of an initializer.</p>
|
||||
<sample src="InitIsGenerator.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python: <a href="http://docs.python.org/2.7/reference/datamodel.html#object.__init__">The __init__ method</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
18
python/ql/src/Functions/InitIsGenerator.ql
Normal file
18
python/ql/src/Functions/InitIsGenerator.ql
Normal file
@@ -0,0 +1,18 @@
|
||||
/**
|
||||
* @name __init__ method is a generator
|
||||
* @description __init__ method is a generator.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision very-high
|
||||
* @id py/init-method-is-generator
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
from Function f
|
||||
where f.isInitMethod() and
|
||||
(exists(Yield y | y.getScope() = f) or exists(YieldFrom y| y.getScope() = f))
|
||||
select f, "__init__ method is a generator."
|
||||
18
python/ql/src/Functions/IterReturnsNonIterator.py
Normal file
18
python/ql/src/Functions/IterReturnsNonIterator.py
Normal file
@@ -0,0 +1,18 @@
|
||||
class MyRange(object):
|
||||
def __init__(self, low, high):
|
||||
self.current = low
|
||||
self.high = high
|
||||
|
||||
def __iter__(self):
|
||||
return self
|
||||
|
||||
#Fixed version
|
||||
class MyRange(object):
|
||||
def __init__(self, low, high):
|
||||
self.current = low
|
||||
self.high = high
|
||||
|
||||
def __iter__(self):
|
||||
while self.current < self.high:
|
||||
yield self.current
|
||||
self.current += 1
|
||||
37
python/ql/src/Functions/IterReturnsNonIterator.qhelp
Normal file
37
python/ql/src/Functions/IterReturnsNonIterator.qhelp
Normal file
@@ -0,0 +1,37 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The <code>__iter__</code> method of a class should return an iterator.
|
||||
|
||||
Iteration in Python relies on this behavior and attempting to iterate over an
|
||||
instance of a class with an incorrect <code>__iter__</code> method will raise a TypeError.
|
||||
</p>
|
||||
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Make the <code>__iter__</code> return a new iterator, either as an instance of
|
||||
a separate class or as a generator.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example the <code>MyRange</code> class's <code>__iter__</code> method does not
|
||||
return an iterator. This will cause the program to fail when anyone attempts
|
||||
to use the iterator in a <code>for</code> loop or <code>in</code> statement.
|
||||
</p>
|
||||
|
||||
<p>The fixed version implements the <code>__iter__</code> method as a generator function.</p>
|
||||
|
||||
<sample src="IterReturnsNonIterator.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/2.7/reference/datamodel.html#object.__iter__">object.__iter__</a>.</li>
|
||||
<li>Python Standard Library: <a href="http://docs.python.org/2/library/stdtypes.html#typeiter">Iterator Types</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
32
python/ql/src/Functions/IterReturnsNonIterator.ql
Normal file
32
python/ql/src/Functions/IterReturnsNonIterator.ql
Normal file
@@ -0,0 +1,32 @@
|
||||
/**
|
||||
* @name __iter__ method returns a non-iterator
|
||||
* @description The '__iter__' method returns a non-iterator which, if used in a 'for' loop, would raise a 'TypeError'.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/iter-returns-non-iterator
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
FunctionObject iter_method(ClassObject t) {
|
||||
result = t.lookupAttribute("__iter__")
|
||||
}
|
||||
|
||||
cached ClassObject return_type(FunctionObject f) {
|
||||
exists(ControlFlowNode n, Return ret |
|
||||
ret.getScope() = f.getFunction() and ret.getValue() = n.getNode() and
|
||||
n.refersTo(_, result, _)
|
||||
)
|
||||
}
|
||||
|
||||
from ClassObject t, FunctionObject iter
|
||||
where exists(ClassObject ret_t | iter = iter_method(t) and
|
||||
ret_t = return_type(iter) and
|
||||
not ret_t.isIterator()
|
||||
)
|
||||
|
||||
select iter, "The '__iter__' method of iterable class $@ does not return an iterator.", t, t.getName()
|
||||
13
python/ql/src/Functions/IterReturnsNonSelf.py
Normal file
13
python/ql/src/Functions/IterReturnsNonSelf.py
Normal file
@@ -0,0 +1,13 @@
|
||||
class MyRange(object):
|
||||
def __init__(self, low, high):
|
||||
self.current = low
|
||||
self.high = high
|
||||
|
||||
def __iter__(self):
|
||||
return self.current
|
||||
|
||||
def next(self):
|
||||
if self.current > self.high:
|
||||
raise StopIteration
|
||||
self.current += 1
|
||||
return self.current - 1
|
||||
37
python/ql/src/Functions/IterReturnsNonSelf.qhelp
Normal file
37
python/ql/src/Functions/IterReturnsNonSelf.qhelp
Normal file
@@ -0,0 +1,37 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The <code>__iter__</code> method of an iterator should return self.
|
||||
This is important so that iterators can be used as sequences in any context
|
||||
that expect a sequence. To do so requires that <code>__iter__</code> is
|
||||
idempotent on iterators.</p>
|
||||
|
||||
<p>
|
||||
Note that sequences and mapping should return a new iterator, it is just the returned
|
||||
iterator that must obey this constraint.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Make the <code>__iter__</code> return self unless the class should not be an iterator,
|
||||
in which case rename the <code>next</code> (Python 2) or <code>__next__</code> (Python 3)
|
||||
to something else.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example the <code>Counter</code> class's <code>__iter__</code> method does not
|
||||
return self (or even an iterator). This will cause the program to fail when anyone attempts
|
||||
to use the iterator in a <code>for</code> loop or <code>in</code> statement.</p>
|
||||
<sample src="IterReturnsNonSelf.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/2.7/reference/datamodel.html#object.__iter__">object.__iter__</a>.</li>
|
||||
<li>Python Standard Library: <a href="http://docs.python.org/2/library/stdtypes.html#typeiter">Iterators</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
33
python/ql/src/Functions/IterReturnsNonSelf.ql
Normal file
33
python/ql/src/Functions/IterReturnsNonSelf.ql
Normal file
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* @name Iterator does not return self from __iter__ method
|
||||
* @description Iterator does not return self from __iter__ method, violating the iterator protocol.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/iter-returns-non-self
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
Function iter_method(ClassObject t) {
|
||||
result = ((FunctionObject)t.lookupAttribute("__iter__")).getFunction()
|
||||
}
|
||||
|
||||
predicate is_self(Name value, Function f) {
|
||||
value.getVariable() = ((Name)f.getArg(0)).getVariable()
|
||||
}
|
||||
|
||||
predicate returns_non_self(Function f) {
|
||||
exists(f.getFallthroughNode())
|
||||
or
|
||||
exists(Return r | r.getScope() = f and not is_self(r.getValue(), f))
|
||||
or
|
||||
exists(Return r | r.getScope() = f and not exists(r.getValue()))
|
||||
}
|
||||
|
||||
from ClassObject t, Function iter
|
||||
where t.isIterator() and iter = iter_method(t) and returns_non_self(iter)
|
||||
select t, "Class " + t.getName() + " is an iterator but its $@ method does not return 'self'.", iter, iter.getName()
|
||||
@@ -0,0 +1,7 @@
|
||||
|
||||
def __init__(self, name, choices=[], default=[], shortDesc=None,
|
||||
longDesc=None, hints=None, allowNone=1): # 'default' parameter assigned a value
|
||||
self.choices = choices
|
||||
if choices and not default:
|
||||
default.append(choices[0][1]) # value of 'default' parameter modified
|
||||
Argument.__init__(self, name, default, shortDesc, longDesc, hints, allowNone=allowNone)
|
||||
@@ -0,0 +1,44 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p> The default value of a parameter is computed once when the function is
|
||||
created, not for every invocation. The "pre-computed" value is then used for every
|
||||
subsequent call to the function. Consequently, if you modify the default
|
||||
value for a parameter this "modified" default value is used for the parameter
|
||||
in future calls to the function. This means that the function may not behave as
|
||||
expected in future calls and also makes the function more difficult to understand.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>If a parameter has a default value, do not modify the default value. When
|
||||
you use a mutable object as a default value, you should use a placeholder value
|
||||
instead of modifying the default value. This is a particular problem when you
|
||||
work with lists and dictionaries but there are standard methods of avoiding
|
||||
modifying the default parameter (see References).</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In the following example, the <code>default</code> parameter is set with a default
|
||||
value of an empty list. Other commands in the function then append values to the
|
||||
list. The next time the function is called, the list will contain values, which
|
||||
may not have been intended.</p>
|
||||
<sample src="ModificationOfParameterWithDefault.py" />
|
||||
|
||||
<p>The recommended workaround is use a placeholder value. That is, define the
|
||||
function with a default of <code>default=None</code>, check if the parameter is
|
||||
<code>None</code> and then set the parameter to a list.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Effbot: <a href="http://effbot.org/zone/default-values.htm">Default Parameter Values in Python</a>.</li>
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/2/reference/compound_stmts.html#function-definitions">Function definitions</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,61 @@
|
||||
/**
|
||||
* @name Modification of parameter with default
|
||||
* @description Modifying the default value of a parameter can lead to unexpected
|
||||
* results.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* maintainability
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/modification-of-default-value
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
predicate safe_method(string name) {
|
||||
name = "count" or name = "index" or name = "copy" or name = "get" or name = "has_key" or
|
||||
name = "items" or name = "keys" or name = "values" or name = "iteritems" or name = "iterkeys" or name = "itervalues"
|
||||
}
|
||||
|
||||
predicate maybe_parameter(SsaVariable var, Function f, Parameter p) {
|
||||
p = var.getAnUltimateDefinition().getDefinition().getNode() and
|
||||
f.getAnArg() = p
|
||||
}
|
||||
|
||||
Name use_of_parameter(Parameter p) {
|
||||
exists(SsaVariable var |
|
||||
p = var.getAnUltimateDefinition().getDefinition().getNode() and
|
||||
var.getAUse().getNode() = result
|
||||
)
|
||||
}
|
||||
|
||||
predicate modifying_call(Call c, Parameter p) {
|
||||
exists(Attribute a |
|
||||
c.getFunc() = a |
|
||||
a.getObject() = use_of_parameter(p) and
|
||||
not safe_method(a.getName())
|
||||
)
|
||||
}
|
||||
|
||||
predicate is_modification(AstNode a, Parameter p) {
|
||||
modifying_call(a, p)
|
||||
or
|
||||
a.(AugAssign).getTarget() = use_of_parameter(p)
|
||||
}
|
||||
|
||||
predicate has_mutable_default(Parameter p) {
|
||||
exists(SsaVariable v, FunctionExpr f | maybe_parameter(v, f.getInnerScope(), p) and
|
||||
exists(int i, int def_cnt, int arg_cnt |
|
||||
def_cnt = count(f.getArgs().getADefault()) and
|
||||
arg_cnt = count(f.getInnerScope().getAnArg()) and
|
||||
i in [1 .. arg_cnt] and
|
||||
(f.getArgs().getDefault(def_cnt - i) instanceof Dict or f.getArgs().getDefault(def_cnt - i) instanceof List) and
|
||||
f.getInnerScope().getArgName(arg_cnt - i) = v.getId()
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
from AstNode a, Parameter p
|
||||
where has_mutable_default(p) and is_modification(a, p)
|
||||
select a, "Modification of parameter $@, which has mutable default value.", p, p.asName().getId()
|
||||
4
python/ql/src/Functions/NonCls.py
Normal file
4
python/ql/src/Functions/NonCls.py
Normal file
@@ -0,0 +1,4 @@
|
||||
class Entry(object):
|
||||
@classmethod
|
||||
def make(klass):
|
||||
return Entry()
|
||||
35
python/ql/src/Functions/NonCls.qhelp
Normal file
35
python/ql/src/Functions/NonCls.qhelp
Normal file
@@ -0,0 +1,35 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p> The first argument of a class method, a new method or any metaclass method
|
||||
should be called <code>cls</code>. This makes the purpose of the argument clear to other developers.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Change the name of the first argument to <code>cls</code> as recommended by the style guidelines
|
||||
in PEP 8.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In the example, the first parameter to <code>make()</code> is <code>klass</code> which should be changed to <code>cls</code>
|
||||
for ease of comprehension.
|
||||
</p>
|
||||
|
||||
<sample src="NonCls.py" />
|
||||
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python PEP 8: <a href="http://www.python.org/dev/peps/pep-0008/#function-and-method-arguments">Function and method arguments</a>.</li>
|
||||
<li>Python Tutorial: <a href="http://docs.python.org/2/tutorial/classes.html">Classes</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
47
python/ql/src/Functions/NonCls.ql
Normal file
47
python/ql/src/Functions/NonCls.ql
Normal file
@@ -0,0 +1,47 @@
|
||||
/**
|
||||
* @name First parameter of a class method is not named 'cls'
|
||||
* @description Using an alternative name for the first argument of a class method makes code more
|
||||
* difficult to read; PEP8 states that the first argument to class methods should be 'cls'.
|
||||
* @kind problem
|
||||
* @tags maintainability
|
||||
* readability
|
||||
* convention
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision high
|
||||
* @id py/not-named-cls
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
predicate first_arg_cls(Function f) {
|
||||
exists(string argname | argname = f.getArgName(0) |
|
||||
argname = "cls" or
|
||||
/* Not PEP8, but relatively common */
|
||||
argname = "mcls"
|
||||
)
|
||||
}
|
||||
|
||||
predicate is_type_method(Function f) {
|
||||
exists(ClassObject c | c.getPyClass() = f.getScope() and c.getASuperType() = theTypeType())
|
||||
}
|
||||
|
||||
predicate classmethod_decorators_only(Function f) {
|
||||
forall(Expr decorator |
|
||||
decorator = f.getADecorator() |
|
||||
((Name) decorator).getId() = "classmethod")
|
||||
}
|
||||
|
||||
from Function f, string message
|
||||
where (f.getADecorator().(Name).getId() = "classmethod" or is_type_method(f)) and
|
||||
not first_arg_cls(f) and classmethod_decorators_only(f) and
|
||||
not f.getName() = "__new__" and
|
||||
(
|
||||
if exists(f.getArgName(0)) then
|
||||
message = "Class methods or methods of a type deriving from type should have 'cls', rather than '" +
|
||||
f.getArgName(0) + "', as their first argument."
|
||||
else
|
||||
message = "Class methods or methods of a type deriving from type should have 'cls' as their first argument."
|
||||
)
|
||||
|
||||
select f, message
|
||||
9
python/ql/src/Functions/NonSelf.py
Normal file
9
python/ql/src/Functions/NonSelf.py
Normal file
@@ -0,0 +1,9 @@
|
||||
class Point:
|
||||
def __init__(val, x, y): # first argument is mis-named 'val'
|
||||
val._x = x
|
||||
val._y = y
|
||||
|
||||
class Point2:
|
||||
def __init__(self, x, y): # first argument is correctly named 'self'
|
||||
self._x = x
|
||||
self._y = y
|
||||
38
python/ql/src/Functions/NonSelf.qhelp
Normal file
38
python/ql/src/Functions/NonSelf.qhelp
Normal file
@@ -0,0 +1,38 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p> Normal methods should have at least one parameter and the first parameter should be called <code>self</code>.
|
||||
This makes the purpose of the parameter clear to other developers.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>If there is at least one parameter, then change the name of the first parameter to <code>self</code> as recommended by the style guidelines
|
||||
in PEP 8.</p>
|
||||
<p>If there are no parameters, then it cannot be a normal method. It may need to be marked as a <code>staticmethod</code>
|
||||
or it could be moved out of the class as a normal function.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>The following methods can both be used to assign values to variables in a <code>point</code>
|
||||
object. The second method makes the association clearer because the <code>self</code> parameter is
|
||||
used.</p>
|
||||
<sample src="NonSelf.py" />
|
||||
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python PEP 8: <a href="http://www.python.org/dev/peps/pep-0008/#function-and-method-arguments">Function and
|
||||
method arguments</a>.</li>
|
||||
<li>Python Tutorial: <a href="http://docs.python.org/2/tutorial/classes.html">Classes</a>.</li>
|
||||
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
54
python/ql/src/Functions/NonSelf.ql
Normal file
54
python/ql/src/Functions/NonSelf.ql
Normal file
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* @name First argument of a method is not named 'self'
|
||||
* @description Using an alternative name for the first argument of an instance method makes
|
||||
* code more difficult to read; PEP8 states that the first argument to instance
|
||||
* methods should be 'self'.
|
||||
* @kind problem
|
||||
* @tags maintainability
|
||||
* readability
|
||||
* convention
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision very-high
|
||||
* @id py/not-named-self
|
||||
*/
|
||||
|
||||
import python
|
||||
import semmle.python.libraries.Zope
|
||||
|
||||
predicate first_arg_self(Function f) {
|
||||
f.getArgName(0) = "self"
|
||||
}
|
||||
|
||||
predicate is_type_method(FunctionObject f) {
|
||||
exists(ClassObject c | c.lookupAttribute(_) = f and c.getASuperType() = theTypeType())
|
||||
}
|
||||
|
||||
predicate used_in_defining_scope(FunctionObject f) {
|
||||
exists(Call c |
|
||||
c.getScope() = f.getFunction().getScope() and
|
||||
c.getFunc().refersTo(f)
|
||||
)
|
||||
}
|
||||
|
||||
from Function f, PyFunctionObject func, string message
|
||||
where
|
||||
exists(ClassObject cls, string name |
|
||||
cls.declaredAttribute(name) = func and cls.isNewStyle() and
|
||||
not name = "__new__" and
|
||||
not name = "__metaclass__" and
|
||||
/* declared in scope */
|
||||
f.getScope() = cls.getPyClass()
|
||||
) and
|
||||
not first_arg_self(f) and not is_type_method(func) and
|
||||
func.getFunction() = f and not f.getName() = "lambda" and
|
||||
not used_in_defining_scope(func) and
|
||||
(
|
||||
if exists(f.getArgName(0)) then
|
||||
message = "Normal methods should have 'self', rather than '" + f.getArgName(0) + "', as their first parameter."
|
||||
else
|
||||
message = "Normal methods should have at least one parameter (the first of which should be 'self')." and not f.hasVarArg()
|
||||
) and
|
||||
not func instanceof ZopeInterfaceMethod
|
||||
|
||||
select f, message
|
||||
24
python/ql/src/Functions/OverlyComplexDelMethod.py
Normal file
24
python/ql/src/Functions/OverlyComplexDelMethod.py
Normal file
@@ -0,0 +1,24 @@
|
||||
|
||||
#Relies on __del__ being called by the garbage collector.
|
||||
class CachedPreferencesFile
|
||||
|
||||
...
|
||||
|
||||
def __del__(self):
|
||||
for key, value in self.preferences.items():
|
||||
self.write_pair(key, value)
|
||||
self.backing.close()
|
||||
|
||||
|
||||
#Better version
|
||||
class CachedPreferencesFile
|
||||
|
||||
...
|
||||
|
||||
def close(self):
|
||||
for key, value in self.preferences.items():
|
||||
self.write_pair(key, value)
|
||||
self.backing.close()
|
||||
|
||||
def __del__(self):
|
||||
self.close()
|
||||
42
python/ql/src/Functions/OverlyComplexDelMethod.qhelp
Normal file
42
python/ql/src/Functions/OverlyComplexDelMethod.qhelp
Normal file
@@ -0,0 +1,42 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>The <code>__del__</code> method exists to release any resources held by an object when that object is deleted.
|
||||
The <code>__del__</code> is called only by the garbage collector which may call it after an indefinite delay or
|
||||
never.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
Consequently, <code>__del__</code> method should not be relied on to release resources, such as file descriptors.
|
||||
Rather, these resources should be released explicitly.
|
||||
</p>
|
||||
|
||||
<p>The existence of a complex <code>__del__</code> method suggests that this is the main or only way to release resources
|
||||
associated with the object.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>In order to ensure correct cleanup of the object add an explicit close(), or similar,
|
||||
method. Possibly make the object a context manager.</p>
|
||||
|
||||
<p>The __del__ method should just call close()</p>
|
||||
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>The first example below shows a class which relies on <code>__del__</code> to release resources.
|
||||
The second example shows an improved version of the class where <code>__del__</code> simply calls close.</p>
|
||||
|
||||
<sample src="OverlyComplexDelMethod.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Standard Library: <a href="http://docs.python.org/library/stdtypes.html#context-manager-types">Context manager</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
21
python/ql/src/Functions/OverlyComplexDelMethod.ql
Normal file
21
python/ql/src/Functions/OverlyComplexDelMethod.ql
Normal file
@@ -0,0 +1,21 @@
|
||||
/**
|
||||
* @name Overly complex __del__ method
|
||||
* @description __del__ methods may be called at arbitrary times, perhaps never called at all, and should be simple.
|
||||
* @kind problem
|
||||
* @tags efficiency
|
||||
* maintainability
|
||||
* complexity
|
||||
* statistical
|
||||
* non-attributable
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/overly-complex-delete
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
from FunctionObject method
|
||||
where exists(ClassObject c | c.declaredAttribute("__del__") = method and
|
||||
method.getFunction().getMetrics().getCyclomaticComplexity() > 3)
|
||||
select method, "Overly complex '__del__' method."
|
||||
15
python/ql/src/Functions/ReturnConsistentTupleSizes.py
Normal file
15
python/ql/src/Functions/ReturnConsistentTupleSizes.py
Normal file
@@ -0,0 +1,15 @@
|
||||
def sum_length_product1(l):
|
||||
if l == []:
|
||||
return 0, 0 # this tuple has the wrong length
|
||||
else:
|
||||
val = l[0]
|
||||
restsum, restlength, restproduct = sum_length_product1(l[1:])
|
||||
return restsum + val, restlength + 1, restproduct * val
|
||||
|
||||
def sum_length_product2(l):
|
||||
if l == []:
|
||||
return 0, 0, 1 # this tuple has the correct length
|
||||
else:
|
||||
val = l[0]
|
||||
restsum, restlength, restproduct = sum_length_product2(l[1:])
|
||||
return restsum + val, restlength + 1, restproduct * val
|
||||
39
python/ql/src/Functions/ReturnConsistentTupleSizes.qhelp
Normal file
39
python/ql/src/Functions/ReturnConsistentTupleSizes.qhelp
Normal file
@@ -0,0 +1,39 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
A common pattern for functions returning multiple arguments is to return a
|
||||
single tuple containing said arguments. If the function has multiple return
|
||||
points, care must be taken to ensure that the tuples returned have the same
|
||||
length.
|
||||
</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure that the function returns tuples of similar lengths.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>
|
||||
In this example, the <code>sum_length_product1</code> function
|
||||
simultaneously calculates the sum, length, and product of the values in the
|
||||
given list. For empty lists, however, the returned tuple only contains the
|
||||
sum and length of the list. In <code>sum_length_product2</code> this error
|
||||
has been corrected.
|
||||
</p>
|
||||
<sample src="ReturnConsistentTupleSizes.py" />
|
||||
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/2/reference/compound_stmts.html#function">Function definitions</a>.
|
||||
</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
29
python/ql/src/Functions/ReturnConsistentTupleSizes.ql
Normal file
29
python/ql/src/Functions/ReturnConsistentTupleSizes.ql
Normal file
@@ -0,0 +1,29 @@
|
||||
/**
|
||||
* @name Returning tuples with varying lengths
|
||||
* @description A function that potentially returns tuples of different lengths may indicate a problem.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* maintainability
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision high
|
||||
* @id py/mixed-tuple-returns
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
predicate returns_tuple_of_size(Function func, int size, AstNode origin) {
|
||||
exists(Return return, TupleObject val |
|
||||
return.getScope() = func and
|
||||
return.getValue().refersTo(val, origin) |
|
||||
size = val.getLength()
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
from Function func, int s1, int s2, AstNode t1, AstNode t2
|
||||
where
|
||||
returns_tuple_of_size(func, s1, t1) and
|
||||
returns_tuple_of_size(func, s2, t2) and
|
||||
s1 < s2
|
||||
select func, func.getQualifiedName() + " returns $@ and $@.", t1, "tuple of size " + s1, t2, "tuple of size " + s2
|
||||
21
python/ql/src/Functions/ReturnValueIgnored.py
Normal file
21
python/ql/src/Functions/ReturnValueIgnored.py
Normal file
@@ -0,0 +1,21 @@
|
||||
|
||||
from third_party import get_resource
|
||||
|
||||
def ignore_error(name):
|
||||
rsc = get_resource(name)
|
||||
rsc.initialize()
|
||||
try:
|
||||
use_resource(rsc)
|
||||
finally:
|
||||
rsc.close()
|
||||
|
||||
#Fixed
|
||||
def do_not_ignore_error(name):
|
||||
rsc = get_resource(name)
|
||||
success = rsc.initialize()
|
||||
if not success:
|
||||
raise Error("Could not initialize resource")
|
||||
try:
|
||||
use_resource(rsc)
|
||||
finally:
|
||||
rsc.close()
|
||||
45
python/ql/src/Functions/ReturnValueIgnored.qhelp
Normal file
45
python/ql/src/Functions/ReturnValueIgnored.qhelp
Normal file
@@ -0,0 +1,45 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>When a function returns a non-trivial value, that value should not be ignored. Doing so may result in errors being ignored or
|
||||
information being thrown away.</p>
|
||||
|
||||
<p>A return value is considered to be trivial if it is <code>None</code> or it is a parameter (parameters, usually <code>self</code> are often
|
||||
returned to assist with method chaining, but can be ignored).
|
||||
A return value is also assumed to be trivial if it is ignored for 75% or more of calls.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Act upon all non-trivial return values, either propagating each value or recording it.
|
||||
If a return value should be ignored, then ensure that it is ignored consistently.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
If you have access to the source code of the called function, then consider modifying it so that it does not return pointless values.
|
||||
</p>
|
||||
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
|
||||
<p>
|
||||
In the <code>ignore_error</code> function the error condition is ignored.
|
||||
Ideally the <code>Resource.initialize()</code> function would raise an exception if it failed, but as it does not, the caller must deal with the error.
|
||||
The <code>do_not_ignore_error</code> function checks the error condition and raises an exception if <code>Resource.initialize()</code> fails.
|
||||
</p>
|
||||
|
||||
<sample src="ReturnValueIgnored.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/2/reference/compound_stmts.html#function">Function definitions</a>.
|
||||
</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
72
python/ql/src/Functions/ReturnValueIgnored.ql
Normal file
72
python/ql/src/Functions/ReturnValueIgnored.ql
Normal file
@@ -0,0 +1,72 @@
|
||||
/**
|
||||
* @name Ignored return value
|
||||
* @description Ignoring return values may result in discarding errors or loss of information.
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* readability
|
||||
* convention
|
||||
* statistical
|
||||
* non-attributable
|
||||
* external/cwe/cwe-252
|
||||
* @problem.severity recommendation
|
||||
* @sub-severity high
|
||||
* @precision medium
|
||||
* @id py/ignored-return-value
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
predicate meaningful_return_value(Expr val) {
|
||||
val instanceof Name
|
||||
or
|
||||
val instanceof BooleanLiteral
|
||||
or
|
||||
exists(FunctionObject callee | val = callee.getACall().getNode() and returns_meaningful_value(callee))
|
||||
or
|
||||
not exists(FunctionObject callee | val = callee.getACall().getNode()) and not val instanceof Name
|
||||
}
|
||||
|
||||
/* Value is used before returning, and thus its value is not lost if ignored */
|
||||
predicate used_value(Expr val) {
|
||||
exists(LocalVariable var, Expr other | var.getAnAccess() = val and other = var.getAnAccess() and not other = val)
|
||||
}
|
||||
|
||||
predicate returns_meaningful_value(FunctionObject f) {
|
||||
not exists(f.getFunction().getFallthroughNode())
|
||||
and
|
||||
(
|
||||
exists(Return ret, Expr val | ret.getScope() = f.getFunction() and val = ret.getValue() |
|
||||
meaningful_return_value(val) and
|
||||
not used_value(val)
|
||||
)
|
||||
or
|
||||
/* Is f a builtin function that returns something other than None?
|
||||
* Ignore __import__ as it is often called purely for side effects */
|
||||
f.isC() and f.getAnInferredReturnType() != theNoneType() and not f.getName() = "__import__"
|
||||
)
|
||||
}
|
||||
|
||||
/* If a call is wrapped tightly in a try-except then we assume it is being executed for the exception. */
|
||||
predicate wrapped_in_try_except(ExprStmt call) {
|
||||
exists(Try t |
|
||||
exists(t.getAHandler()) and
|
||||
strictcount(Call c | t.getBody().contains(c)) = 1 and
|
||||
call = t.getAStmt()
|
||||
)
|
||||
}
|
||||
|
||||
from ExprStmt call, FunctionObject callee, float percentage_used, int total
|
||||
where call.getValue() = callee.getACall().getNode() and returns_meaningful_value(callee) and
|
||||
not wrapped_in_try_except(call) and
|
||||
exists(int unused |
|
||||
unused = count(ExprStmt e | e.getValue().getAFlowNode() = callee.getACall()) and
|
||||
total = count(callee.getACall()) |
|
||||
percentage_used = (100.0*(total-unused)/total).floor()
|
||||
) and
|
||||
/* Report an alert if we see at least 5 calls and the return value is used in at least 3/4 of those calls. */
|
||||
percentage_used >= 75 and
|
||||
total >= 5
|
||||
|
||||
select call, "Call discards return value of function $@. The result is used in " + percentage_used.toString() + "% of calls.",
|
||||
callee, callee.getName()
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
|
||||
class BaseClass(object):
|
||||
|
||||
def run(self, source, filename, symbol="single"):
|
||||
... # Definition
|
||||
|
||||
def load_and_run(self, filename):
|
||||
source = self.load(filename)
|
||||
self.run(source, filename) # Matches signature in this class, but not in the derived class.
|
||||
|
||||
class DerivedClass(BaseClass):
|
||||
|
||||
def run(self, source):
|
||||
... # Definition
|
||||
9
python/ql/src/Functions/SignatureOverriddenMethod.py
Normal file
9
python/ql/src/Functions/SignatureOverriddenMethod.py
Normal file
@@ -0,0 +1,9 @@
|
||||
|
||||
# Base class method
|
||||
def runsource(self, source, filename="<input>", symbol="single"):
|
||||
... # Definition
|
||||
|
||||
|
||||
# Extend base class method
|
||||
def runsource(self, source):
|
||||
... # Definition
|
||||
41
python/ql/src/Functions/SignatureOverriddenMethod.qhelp
Normal file
41
python/ql/src/Functions/SignatureOverriddenMethod.qhelp
Normal file
@@ -0,0 +1,41 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
|
||||
<overview>
|
||||
<p> There are one (or more) legal parameters for an overridden method that are
|
||||
not legal for an overriding method. This will cause an error when the overriding
|
||||
method is called with a number of parameters that is legal for the overridden method.
|
||||
This violates the Liskov substitution principle.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure that the overriding method accepts all the parameters that are legal for
|
||||
overridden method.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example there is a mismatch between the legal parameters for the base
|
||||
class method <code>(self, source, filename, symbol)</code> and the extension method
|
||||
<code>(self, source)</code>. The extension method can be used to override the base
|
||||
method as long as values are not specified for the <code>filename</code> and
|
||||
<code>symbol</code> parameters. If the extension method was passed the additional
|
||||
parameters accepted by the base method then an error would occur.</p>
|
||||
|
||||
<sample src="SignatureOverriddenMethod.py" />
|
||||
|
||||
<p>The extension method should be updated to support the <code>filename</code> and
|
||||
<code>symbol</code> parameters supported by the overridden method.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Liskov_substitution_principle">Liskov Substitution Principle</a>, <a href="http://en.wikipedia.org/wiki/Method_overriding#Python">Method overriding</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
35
python/ql/src/Functions/SignatureOverriddenMethod.ql
Normal file
35
python/ql/src/Functions/SignatureOverriddenMethod.ql
Normal file
@@ -0,0 +1,35 @@
|
||||
/**
|
||||
* @name Signature mismatch in overriding method
|
||||
* @description Overriding a method without ensuring that both methods accept the same
|
||||
* number and type of parameters has the potential to cause an error when there is a mismatch.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* @problem.severity warning
|
||||
* @sub-severity high
|
||||
* @precision very-high
|
||||
* @id py/inheritance/signature-mismatch
|
||||
*/
|
||||
|
||||
import python
|
||||
import Expressions.CallArgs
|
||||
|
||||
from FunctionObject base, PyFunctionObject derived
|
||||
where
|
||||
not exists(base.getACall()) and
|
||||
not exists(FunctionObject a_derived |
|
||||
a_derived.overrides(base) and
|
||||
exists(a_derived.getACall())
|
||||
) and
|
||||
not derived.getFunction().isSpecialMethod() and
|
||||
derived.getName() != "__init__" and
|
||||
derived.isNormalMethod() and
|
||||
not derived.getFunction().isSpecialMethod() and
|
||||
// call to overrides distributed for efficiency
|
||||
(
|
||||
(derived.overrides(base) and derived.minParameters() > base.maxParameters())
|
||||
or
|
||||
(derived.overrides(base) and derived.maxParameters() < base.minParameters())
|
||||
)
|
||||
select derived, "Overriding method '" + derived.getName() + "' has signature mismatch with $@.", base, "overridden method"
|
||||
14
python/ql/src/Functions/SignatureOverridingMethod.py
Normal file
14
python/ql/src/Functions/SignatureOverridingMethod.py
Normal file
@@ -0,0 +1,14 @@
|
||||
|
||||
class BaseClass(object):
|
||||
|
||||
def run(self, source, filename, symbol="single"):
|
||||
... # Definition
|
||||
|
||||
def load_and_run(self, filename):
|
||||
source = self.load(filename)
|
||||
self.run(source) # Matches signature in derived class, but not in this class.
|
||||
|
||||
class DerivedClass(BaseClass):
|
||||
|
||||
def run(self, source):
|
||||
... # Definition
|
||||
18
python/ql/src/Functions/SignatureSpecialMethods.py
Normal file
18
python/ql/src/Functions/SignatureSpecialMethods.py
Normal file
@@ -0,0 +1,18 @@
|
||||
#-*- coding: utf-8 -*-
|
||||
|
||||
class Point(object):
|
||||
|
||||
def __init__(self, x, y):
|
||||
self.x
|
||||
self.y
|
||||
|
||||
def __add__(self, other):
|
||||
if not isinstance(other, Point):
|
||||
return NotImplemented
|
||||
return Point(self.x + other.x, self.y + other.y)
|
||||
|
||||
def __str__(self, style): #Spurious extra parameter
|
||||
if style == 'polar':
|
||||
u"%s @ %s\u00b0" % (abs(self), self.angle())
|
||||
else:
|
||||
return "[%s, %s]" % (self.x, self.y)
|
||||
34
python/ql/src/Functions/SignatureSpecialMethods.qhelp
Normal file
34
python/ql/src/Functions/SignatureSpecialMethods.qhelp
Normal file
@@ -0,0 +1,34 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>Special methods (sometimes also called magic methods) are how user defined classes interact with the Python virtual machine.
|
||||
For example, for a class to support addition it must implement the <code>__add__</code> and <code>__radd__</code> special methods.
|
||||
When the expression <code>a + b</code> is evaluated the Python virtual machine will call <code>type(a).__add__(a, b)</code> and if that
|
||||
is not implemented it will call <code>type(b).__radd__(b, a)</code>.</p>
|
||||
<p>
|
||||
Since these special methods are always called by the virtual machine with a fixed number of parameters, if the method is implemented with
|
||||
a different number of parameters it will fail at runtime with a <code>TypeError</code>.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Ensure that the method has the correct number of parameters</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In the example the <code>__str__</code> method has an extra parameter. This means that if <code>str(p)</code> is called when <code>p</code>
|
||||
is a <code>Point</code> then it will fail with a <code>TypeError</code>.
|
||||
</p>
|
||||
|
||||
<sample src="SignatureSpecialMethods.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/dev/reference/datamodel.html#special-method-names">Special Method Names</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
200
python/ql/src/Functions/SignatureSpecialMethods.ql
Normal file
200
python/ql/src/Functions/SignatureSpecialMethods.ql
Normal file
@@ -0,0 +1,200 @@
|
||||
/**
|
||||
* @name Special method has incorrect signature
|
||||
* @description Special method has incorrect signature
|
||||
* @kind problem
|
||||
* @tags reliability
|
||||
* correctness
|
||||
* @problem.severity error
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/special-method-wrong-signature
|
||||
*/
|
||||
|
||||
import python
|
||||
|
||||
|
||||
predicate is_unary_op(string name) {
|
||||
name = "__del__" or
|
||||
name = "__repr__" or
|
||||
name = "__str__" or
|
||||
name = "__hash__" or
|
||||
name = "__bool__" or
|
||||
name = "__nonzero__" or
|
||||
name = "__unicode__" or
|
||||
name = "__len__" or
|
||||
name = "__iter__" or
|
||||
name = "__reversed__" or
|
||||
name = "__neg__" or
|
||||
name = "__pos__" or
|
||||
name = "__abs__" or
|
||||
name = "__invert__" or
|
||||
name = "__complex__" or
|
||||
name = "__int__" or
|
||||
name = "__float__" or
|
||||
name = "__long__" or
|
||||
name = "__oct__" or
|
||||
name = "__hex__" or
|
||||
name = "__index__" or
|
||||
name = "__enter__"
|
||||
}
|
||||
|
||||
predicate is_binary_op(string name) {
|
||||
name = "__lt__" or
|
||||
name = "__le__" or
|
||||
name = "__eq__" or
|
||||
name = "__ne__" or
|
||||
name = "__gt__" or
|
||||
name = "__ge__" or
|
||||
name = "__cmp__" or
|
||||
name = "__rcmp__" or
|
||||
name = "__getattr___" or
|
||||
name = "__getattribute___" or
|
||||
name = "__delattr__" or
|
||||
name = "__delete__" or
|
||||
name = "__instancecheck__" or
|
||||
name = "__subclasscheck__" or
|
||||
name = "__getitem__" or
|
||||
name = "__delitem__" or
|
||||
name = "__contains__" or
|
||||
name = "__add__" or
|
||||
name = "__sub__" or
|
||||
name = "__mul__" or
|
||||
name = "__floordiv__" or
|
||||
name = "__div__" or
|
||||
name = "__truediv__" or
|
||||
name = "__mod__" or
|
||||
name = "__divmod__" or
|
||||
name = "__lshift__" or
|
||||
name = "__rshift__" or
|
||||
name = "__and__" or
|
||||
name = "__xor__" or
|
||||
name = "__or__" or
|
||||
name = "__radd__" or
|
||||
name = "__rsub__" or
|
||||
name = "__rmul__" or
|
||||
name = "__rfloordiv__" or
|
||||
name = "__rdiv__" or
|
||||
name = "__rtruediv__" or
|
||||
name = "__rmod__" or
|
||||
name = "__rdivmod__" or
|
||||
name = "__rpow__" or
|
||||
name = "__rlshift__" or
|
||||
name = "__rrshift__" or
|
||||
name = "__rand__" or
|
||||
name = "__rxor__" or
|
||||
name = "__ror__" or
|
||||
name = "__iadd__" or
|
||||
name = "__isub__" or
|
||||
name = "__imul__" or
|
||||
name = "__ifloordiv__" or
|
||||
name = "__idiv__" or
|
||||
name = "__itruediv__" or
|
||||
name = "__imod__" or
|
||||
name = "__idivmod__" or
|
||||
name = "__ipow__" or
|
||||
name = "__ilshift__" or
|
||||
name = "__irshift__" or
|
||||
name = "__iand__" or
|
||||
name = "__ixor__" or
|
||||
name = "__ior__" or
|
||||
name = "__coerce__"
|
||||
}
|
||||
|
||||
predicate is_ternary_op(string name) {
|
||||
name = "__setattr__" or
|
||||
name = "__set__" or
|
||||
name = "__setitem__" or
|
||||
name = "__getslice__" or
|
||||
name = "__delslice__"
|
||||
}
|
||||
|
||||
predicate is_quad_op(string name) {
|
||||
name = "__setslice__" or name = "__exit__"
|
||||
}
|
||||
|
||||
int argument_count(PyFunctionObject f, string name, ClassObject cls) {
|
||||
cls.declaredAttribute(name) = f and
|
||||
(
|
||||
is_unary_op(name) and result = 1
|
||||
or
|
||||
is_binary_op(name) and result = 2
|
||||
or
|
||||
is_ternary_op(name) and result = 3
|
||||
or
|
||||
is_quad_op(name) and result = 4
|
||||
)
|
||||
}
|
||||
|
||||
predicate incorrect_special_method_defn(PyFunctionObject func, string message, boolean show_counts, string name, ClassObject owner) {
|
||||
exists(int required |
|
||||
required = argument_count(func, name, owner) |
|
||||
/* actual_non_default <= actual */
|
||||
if required > func.maxParameters() then
|
||||
(message = "Too few parameters" and show_counts = true)
|
||||
else if required < func.minParameters() then
|
||||
(message = "Too many parameters" and show_counts = true)
|
||||
else if (func.minParameters() < required and not func.getFunction().hasVarArg()) then
|
||||
(message = (required -func.minParameters()) + " default values(s) will never be used" and show_counts = false)
|
||||
else
|
||||
none()
|
||||
)
|
||||
}
|
||||
|
||||
predicate incorrect_pow(FunctionObject func, string message, boolean show_counts, ClassObject owner) {
|
||||
owner.declaredAttribute("__pow__") = func and
|
||||
(
|
||||
func.maxParameters() < 2 and message = "Too few parameters" and show_counts = true
|
||||
or
|
||||
func.minParameters() > 3 and message = "Too many parameters" and show_counts = true
|
||||
or
|
||||
func.minParameters() < 2 and message = (2 - func.minParameters()) + " default value(s) will never be used" and show_counts = false
|
||||
or
|
||||
func.minParameters() = 3 and message = "Third parameter to __pow__ should have a default value" and show_counts = false
|
||||
)
|
||||
}
|
||||
|
||||
predicate incorrect_get(FunctionObject func, string message, boolean show_counts, ClassObject owner) {
|
||||
owner.declaredAttribute("__get__") = func and
|
||||
(
|
||||
func.maxParameters() < 3 and message = "Too few parameters" and show_counts = true
|
||||
or
|
||||
func.minParameters() > 3 and message = "Too many parameters" and show_counts = true
|
||||
or
|
||||
func.minParameters() < 2 and not func.getFunction().hasVarArg() and
|
||||
message = (2 - func.minParameters()) + " default value(s) will never be used" and show_counts = false
|
||||
)
|
||||
}
|
||||
|
||||
string should_have_parameters(PyFunctionObject f, string name, ClassObject owner) {
|
||||
exists(int i | i = argument_count(f, name, owner) |
|
||||
result = i.toString()
|
||||
)
|
||||
or
|
||||
owner.declaredAttribute(name) = f and (name = "__get__" or name = "__pow__") and result = "2 or 3"
|
||||
}
|
||||
|
||||
string has_parameters(PyFunctionObject f) {
|
||||
exists(int i | i = f.minParameters() |
|
||||
i = 0 and result = "no parameters"
|
||||
or
|
||||
i = 1 and result = "1 parameter"
|
||||
or
|
||||
i > 1 and result = i.toString() + " parameters"
|
||||
)
|
||||
}
|
||||
|
||||
from PyFunctionObject f, string message, string sizes, boolean show_counts, string name, ClassObject owner
|
||||
where
|
||||
(
|
||||
incorrect_special_method_defn(f, message, show_counts, name, owner)
|
||||
or
|
||||
incorrect_pow(f, message, show_counts, owner) and name = "__pow__"
|
||||
or
|
||||
incorrect_get(f, message, show_counts, owner) and name = "__get__"
|
||||
)
|
||||
and
|
||||
(
|
||||
show_counts = false and sizes = "" or
|
||||
show_counts = true and sizes = ", which has " + has_parameters(f) + ", but should have " + should_have_parameters(f, name, owner)
|
||||
)
|
||||
select f, message + " for special method " + name + sizes + ", in class $@.", owner, owner.getName()
|
||||
17
python/ql/src/Functions/UseImplicitNoneReturnValue.py
Normal file
17
python/ql/src/Functions/UseImplicitNoneReturnValue.py
Normal file
@@ -0,0 +1,17 @@
|
||||
|
||||
import sys
|
||||
|
||||
def my_print(*args):
|
||||
print (args)
|
||||
|
||||
def main():
|
||||
err = my_print(sys.argv)
|
||||
if err:
|
||||
sys.exit(err)
|
||||
|
||||
|
||||
#FIXED VERSION
|
||||
def main():
|
||||
my_print(sys.argv)
|
||||
#The rest of the code can be removed as None as always false
|
||||
|
||||
32
python/ql/src/Functions/UseImplicitNoneReturnValue.qhelp
Normal file
32
python/ql/src/Functions/UseImplicitNoneReturnValue.qhelp
Normal file
@@ -0,0 +1,32 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>All functions in Python return a value.
|
||||
If a function has no <code>return</code> statements or none of the <code>return</code> statements return a value
|
||||
then the function will return <code>None</code>. However, this value has no meaning and should be ignored.</p>
|
||||
|
||||
<p>Using the return value of such a 'procedure' is confusing to the reader as it suggests
|
||||
that the value is significant.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Do not use the return value of a procedure; replace <code>x = proc()</code> with <code>proc()</code>
|
||||
and replace any use of the value with <code>None</code>.</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>In this example, the <code>my_print</code> function is a procedure as it returns no value of any meaning.
|
||||
Using the return value is misleading in subsequent code.
|
||||
</p>
|
||||
<sample src="UseImplicitNoneReturnValue.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>Python Library Reference: <a href="http://docs.python.org/library/constants.html#None">None</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
34
python/ql/src/Functions/UseImplicitNoneReturnValue.ql
Normal file
34
python/ql/src/Functions/UseImplicitNoneReturnValue.ql
Normal file
@@ -0,0 +1,34 @@
|
||||
/**
|
||||
* @name Use of the return value of a procedure
|
||||
* @description The return value of a procedure (a function that does not return a value) is used. This is confusing to the reader as the value (None) has no meaning.
|
||||
* @kind problem
|
||||
* @tags maintainability
|
||||
* @problem.severity warning
|
||||
* @sub-severity low
|
||||
* @precision high
|
||||
* @id py/procedure-return-value-used
|
||||
*/
|
||||
|
||||
import python
|
||||
import Testing.Mox
|
||||
|
||||
predicate is_used(Call c) {
|
||||
exists(Expr outer | outer != c and outer.containsInScope(c) | outer instanceof Call or outer instanceof Attribute or outer instanceof Subscript)
|
||||
or
|
||||
exists(Stmt s |
|
||||
c = s.getASubExpression() and
|
||||
not s instanceof ExprStmt and
|
||||
/* Ignore if a single return, as def f(): return g() is quite common. Covers implicit return in a lambda. */
|
||||
not (s instanceof Return and strictcount(Return r | r.getScope() = s.getScope()) = 1)
|
||||
)
|
||||
}
|
||||
|
||||
from Call c, FunctionObject func
|
||||
where
|
||||
/* Call result is used, but callee is a procedure */
|
||||
is_used(c) and c.getFunc().refersTo(func) and func.getFunction().isProcedure() and
|
||||
/* All callees are procedures */
|
||||
forall(FunctionObject callee | c.getFunc().refersTo(callee) | callee.getFunction().isProcedure()) and
|
||||
/* Mox return objects have an `AndReturn` method */
|
||||
not useOfMoxInModule(c.getEnclosingModule())
|
||||
select c, "The result of '$@' is used even though it is always None.", func, func.getQualifiedName()
|
||||
Reference in New Issue
Block a user