Merge pull request #493 from markshannon/python-queries

Initial commit of Python queries and QL libraries.

Author: taus-semmle

.lgtm.yml

@@ -10,6 +10,8 @@ path_classifiers:
   - javascript/extractor/tests
   - javascript/ql/src
   - javascript/ql/test
+  - python/ql/src
+  - python/ql/test
 
 queries:
   - include: "*"

python/ql/src/.project

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+    <name>semmlecode-python-queries</name>
+    <comment></comment>
+    <projects>
+    </projects>
+    <buildSpec>
+        <buildCommand>
+            <name>org.python.pydev.PyDevBuilder</name>
+            <arguments>
+            </arguments>
+        </buildCommand>
+    </buildSpec>
+    <natures>
+        <nature>org.python.pydev.pythonNature</nature>
+        <nature>com.semmle.plugin.qdt.core.qlnature</nature>
+    </natures>
+</projectDescription>

python/ql/src/.qlpath

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<ns:qlpath xmlns:ns="https://semmle.com/schemas/qlpath">
+    <librarypath/>
+    <dbscheme>/semmlecode-python-queries/semmlecode.python.dbscheme</dbscheme>
+    <defaultImports><defaultImport>python</defaultImport></defaultImports>
+</ns:qlpath>

python/ql/src/Classes/ConflictingAttributesInBaseClasses.py

@@ -0,0 +1,18 @@
+
+class TCPServer(object):
+    
+    def process_request(self, request, client_address):
+        self.do_work(request, client_address)
+        self.shutdown_request(request)
+
+    
+class ThreadingMixIn:
+    """Mix-in class to handle each request in a new thread."""
+
+    def process_request(self, request, client_address):
+        """Start a new thread to process the request."""
+        t = threading.Thread(target = self.do_work, args = (request, client_address))
+        t.daemon = self.daemon_threads
+        t.start()
+
+class ThreadingTCPServer(ThreadingMixIn, TCPServer): pass

python/ql/src/Classes/ConflictingAttributesInBaseClasses.qhelp

@@ -0,0 +1,59 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+When a class subclasses multiple base classes, attribute lookup is performed from left to right amongst the base classes.
+This form of attribute lookup is called "method resolution order" and is a solution to the 
+<a href="http://en.wikipedia.org/wiki/Multiple_inheritance#The_diamond_problem">diamond inheritance problem</a> where several base classes
+override a method in a shared superclass.
+</p>
+<p>
+Unfortunately, this means that if more than one base class defines the same attribute, the leftmost base class will effectively override
+the attribute of the rightmost base class, even though the leftmost base class is not a subclass of the rightmost base class.
+Unless the methods in question are designed for inheritance, using <code>super</code>, then this implicit overriding may not be the desired behavior. 
+Even if it is the desired behavior it makes the code hard to understand and maintain.
+</p>
+
+</overview>
+<recommendation>
+<p>There are a number of ways that might be used to address this issue:
+</p><ul>
+    <li>Override the attribute in the subclass to implement the correct behavior.</li>
+    <li>Modify the class hierarchy and move equivalent or redundant methods to a common super class.</li>
+    <li>Modify the method hierarchy, breaking up complex methods into constituent parts.</li>
+    <li>Use delegation rather than inheritance.</li>
+</ul>
+
+</recommendation>
+<example>
+
+<p>
+In this example the class <code>ThreadingTCPServer</code> inherits from <code>ThreadingMixIn</code> and from <code>TCPServer</code>.
+However, both these classes implement <code>process_request</code> which means that <code>ThreadingTCPServer</code> will inherit
+<code>process_request</code> from <code>ThreadingMixIn</code>. Consequently, the implementation of <code>process_request</code> in <code>TCPServer</code>
+will be ignored, which may not be the correct behavior.
+</p>
+<sample src="ConflictingAttributesInBaseClasses.py" />
+
+<p>
+This can be fixed either by overriding the method, as shown in class <code>ThreadingTCPServerOverriding</code>
+or by ensuring that the
+functionality provided by the two base classes does not overlap, as shown in class <code>ThreadingTCPServerChangedHierarchy</code>.
+</p>
+<sample src="ConflictingAttributesInBaseClasses_Fixed.py" />
+
+
+</example>
+<references>
+
+    <li>Python Language Reference: <a href="https://docs.python.org/2/reference/datamodel.html">Data model</a>.</li>
+    <li>Python releases: <a href="https://www.python.org/download/releases/2.3/mro/">The Python 2.3 Method Resolution Order</a>.</li>
+    <li>Wikipedia: <a href="https://en.wikipedia.org/wiki/C3_linearization">C3 linearization</a>.</li>
+    <li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Composition_over_inheritance">Composition over inheritance</a>.</li>
+
+
+</references>
+</qhelp>

python/ql/src/Classes/ConflictingAttributesInBaseClasses.ql

@@ -0,0 +1,57 @@
+/**
+ * @name Conflicting attributes in base classes
+ * @description When a class subclasses multiple base classes and more than one base class defines the same attribute, attribute overriding may result in unexpected behavior by instances of this class.
+ * @kind problem
+ * @tags reliability
+ *       maintainability
+ *       modularity
+ * @problem.severity warning
+ * @sub-severity low
+ * @precision high
+ * @id py/conflicting-attributes
+ */
+
+import python
+
+predicate does_nothing(PyFunctionObject f) {
+    not exists(Stmt s | s.getScope() = f.getFunction() |
+        not s instanceof Pass and not ((ExprStmt)s).getValue() = f.getFunction().getDocString()
+    )
+}
+
+/* If a method performs a super() call then it is OK as the 'overridden' method will get called */
+predicate calls_super(FunctionObject f) {
+    exists(Call sup, Call meth, Attribute attr, GlobalVariable v | 
+        meth.getScope() = f.getFunction() and
+        meth.getFunc() = attr and
+        attr.getObject() = sup and
+        attr.getName() = f.getName() and
+        sup.getFunc() = v.getAnAccess() and
+        v.getId() = "super"
+    )
+}
+
+/** Holds if the given name is white-listed for some reason */
+predicate whitelisted(string name) {
+    /* The standard library specifically recommends this :(
+     * See https://docs.python.org/3/library/socketserver.html#asynchronous-mixins */
+    name = "process_request"
+}
+
+from ClassObject c, ClassObject b1, ClassObject b2, string name,
+int i1, int i2, Object o1, Object o2
+where c.getBaseType(i1) = b1 and
+c.getBaseType(i2) = b2 and 
+i1 < i2 and o1 != o2 and
+o1 = b1.lookupAttribute(name) and 
+o2 = b2.lookupAttribute(name) and
+not name.matches("\\_\\_%\\_\\_") and
+not calls_super(o1) and
+not does_nothing(o2) and
+not whitelisted(name) and
+not o1.overrides(o2) and
+not o2.overrides(o1) and
+not c.declaresAttribute(name)
+
+select c, "Base classes have conflicting values for attribute '" + name + "': $@ and $@.", o1, o1.toString(), o2, o2.toString()
+

python/ql/src/Classes/ConflictingAttributesInBaseClasses_Fixed.py

@@ -0,0 +1,24 @@
+   
+#Fixed by overriding. This does not change behavior, but makes it explicit and comprehensible.
+class ThreadingTCPServerOverriding(ThreadingMixIn, TCPServer):
+    
+    def process_request(self, request, client_address):
+        #process_request forwards to do_work, so it is OK to call ThreadingMixIn.process_request directly
+        ThreadingMixIn.process_request(self, request, client_address)
+        
+
+#Fixed by separating threading functionality from request handling.
+class ThreadingMixIn:
+    """Mix-in class to help with threads."""
+
+    def do_job_in_thread(self, job, args):
+        """Start a new thread to do the job"""
+        t = threading.Thread(target = job, args = args)
+        t.start()
+
+class ThreadingTCPServerChangedHierarchy(ThreadingMixIn, TCPServer):
+    
+    def process_request(self, request, client_address):
+        """Start a new thread to process the request."""
+        self.do_job_in_thread(self.do_work,  (request, client_address))
+    

python/ql/src/Classes/DefineEqualsWhenAddingAttributes.py

@@ -0,0 +1,40 @@
+class Point(object):
+
+    def __init__(self, x, y):
+        self._x = x
+        self._y = y
+
+    def __repr__(self):
+        return 'Point(%r, %r)' % (self._x, self._y)
+
+    def __eq__(self, other):
+        if not isinstance(other, Point):
+            return False
+        return self._x == other._x and self._y == other._y
+
+class ColorPoint(Point):
+
+    def __init__(self, x, y, color):
+        Point.__init__(self, x, y)
+        self._color = color
+
+    def __repr__(self):
+        return 'ColorPoint(%r, %r)' % (self._x, self._y, self._color)
+
+#ColorPoint(0, 0, Red) == ColorPoint(0, 0, Green) should be False, but is True.
+
+#Fixed version
+class ColorPoint(Point):
+
+    def __init__(self, x, y, color):
+        Point.__init__(self, x, y)
+        self._color = color
+
+    def __repr__(self):
+        return 'ColorPoint(%r, %r)' % (self._x, self._y, self._color)
+
+    def __eq__(self, other):
+        if not isinstance(other, ColorPoint):
+            return False
+        return Point.__eq__(self, other) and self._color = other._color
+

python/ql/src/Classes/DefineEqualsWhenAddingAttributes.qhelp

@@ -0,0 +1,37 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>A class that defines attributes that are not present in its superclasses
+may need to override the <code>__eq__()</code> method (<code>__ne__()</code>
+should also be defined).</p>
+
+<p>Adding additional attributes without overriding  <code>__eq__()</code> means
+that the additional attributes will not be accounted for in equality tests.</p>
+
+
+</overview>
+<recommendation>
+
+<p>Override the <code>__eq__</code> method.</p>
+
+
+</recommendation>
+<example>
+<p>In the following example the <code>ColorPoint</code>
+class subclasses the <code>Point</code> class and adds a new attribute,
+but does not override the <code>__eq__</code> method.
+</p>
+
+<sample src="DefineEqualsWhenAddingAttributes.py" />
+
+
+</example>
+<references>
+
+<li>Peter Grogono, Philip Santas: <a href="http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.48.5109&amp;rep=rep1&amp;type=pdf">Equality in Object Oriented Languages</a></li>
+
+</references>
+</qhelp>

python/ql/src/Classes/DefineEqualsWhenAddingAttributes.ql

@@ -0,0 +1,52 @@
+/**
+ * @name __eq__ not overridden when adding attributes
+ * @description When adding new attributes to instances of a class, equality for that class needs to be defined.
+ * @kind problem
+ * @tags reliability
+ *       correctness
+ * @problem.severity warning
+ * @sub-severity high
+ * @precision high
+ * @id py/missing-equals
+ */
+
+import python
+import semmle.python.SelfAttribute
+import Equality
+
+predicate class_stores_to_attribute(ClassObject cls, SelfAttributeStore store, string name) {
+    exists(FunctionObject f | f = cls.declaredAttribute(_) and store.getScope() = f.getFunction() and store.getName() = name) and
+    /* Exclude classes used as metaclasses */
+    not cls.getASuperType() = theTypeType()
+}
+
+predicate should_override_eq(ClassObject cls, Object base_eq) {
+    not cls.declaresAttribute("__eq__") and
+    exists(ClassObject sup | sup = cls.getABaseType() and sup.declaredAttribute("__eq__") = base_eq |
+        not exists(GenericEqMethod eq | eq.getScope() = sup.getPyClass()) and
+        not exists(IdentityEqMethod eq | eq.getScope() = sup.getPyClass()) and
+        not base_eq.(FunctionObject).getFunction() instanceof IdentityEqMethod and
+        not base_eq = theObjectType().declaredAttribute("__eq__")
+    )
+}
+
+/** Does the non-overridden __eq__ method access the attribute,
+ * which implies that the  __eq__ method does not need to be overridden.
+ */
+predicate superclassEqExpectsAttribute(ClassObject cls, PyFunctionObject base_eq, string attrname) {
+    not cls.declaresAttribute("__eq__") and
+    exists(ClassObject sup | sup = cls.getABaseType() and sup.declaredAttribute("__eq__") = base_eq |
+        exists(SelfAttributeRead store |
+            store.getName() = attrname |
+            store.getScope() = base_eq.getFunction()
+        )
+    )
+}
+
+from ClassObject cls, SelfAttributeStore store, Object base_eq
+where class_stores_to_attribute(cls, store, _) and should_override_eq(cls, base_eq) and
+/* Don't report overridden unittest.TestCase. -- TestCase overrides __eq__, but subclasses do not really need to. */
+not cls.getASuperType().getName() = "TestCase" and
+not superclassEqExpectsAttribute(cls, base_eq, store.getName())
+
+select cls, "The class '" + cls.getName() + "' does not override $@, but adds the new attribute $@.", base_eq,  "'__eq__'", store, store.getName()