Update tests

Author: joefarebrother

python/ql/src/Classes/InitCallsSubclass/InitCallsSubclassMethod.ql

@@ -15,31 +15,21 @@ import python
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.internal.DataFlowDispatch
 
-predicate initSelfCall(Function init, DataFlow::MethodCallNode call) {
-  init.isInitMethod() and
-  call.getScope() = init and
-  exists(DataFlow::Node self, DataFlow::ParameterNode selfArg |
-    call.calls(self, _) and
-    selfArg.getParameter() = init.getArg(0) and
-    DataFlow::localFlow(selfArg, self)
-  )
-}
-
 predicate initSelfCallOverridden(
-  Function init, DataFlow::MethodCallNode call, Function target, Function override
+  Function init, DataFlow::Node self, DataFlow::MethodCallNode call, Function target,
+  Function override
 ) {
   init.isInitMethod() and
   call.getScope() = init and
-  exists(Class superclass, Class subclass, DataFlow::Node self, DataFlow::ParameterNode selfArg |
+  exists(Class superclass, Class subclass, DataFlow::ParameterNode selfArg |
     superclass = init.getScope() and
     subclass = override.getScope() and
     subclass = getADirectSubclass+(superclass) and
     selfArg.getParameter() = init.getArg(0) and
     DataFlow::localFlow(selfArg, self) and
     call.calls(self, override.getName()) and
     target = superclass.getAMethod() and
-    target.getName() = override.getName() and
-    not lastUse(self)
+    target.getName() = override.getName()
   )
 }
 
@@ -58,10 +48,14 @@ predicate readsFromSelf(Function method) {
   )
 }
 
-from Function init, DataFlow::MethodCallNode call, Function target, Function override
+from
+  Function init, DataFlow::Node self, DataFlow::MethodCallNode call, Function target,
+  Function override
 where
-  initSelfCallOverridden(init, call, target, override) and
+  initSelfCallOverridden(init, self, call, target, override) and
   readsFromSelf(override) and
-  not isClassmethod(override)
+  not isClassmethod(override) and
+  not lastUse(self) and
+  not target.getName().matches("\\_%")
 select call, "This call to $@ in an initialization method is overridden by $@.", target,
   target.getQualifiedName(), override, override.getQualifiedName()

python/ql/test/query-tests/Classes/init-calls-subclass-method/InitCallsSubclassMethod.expected

@@ -1 +1 @@
-| init_calls_subclass.py:7:9:7:24 | Attribute() | Call to self.$@ in __init__ method, which is overridden by $@. | init_calls_subclass.py:10:5:10:26 | Function set_up | set_up | init_calls_subclass.py:19:5:19:26 | Function set_up | method Sub.set_up |
+| init_calls_subclass.py:8:13:8:28 | ControlFlowNode for Attribute() | This call to $@ in an initialization method is overridden by $@. | init_calls_subclass.py:11:9:11:30 | Function set_up | bad1.Super.set_up | init_calls_subclass.py:20:9:20:30 | Function set_up | bad1.Sub.set_up |

python/ql/test/query-tests/Classes/init-calls-subclass-method/InitCallsSubclassMethod.qlref

@@ -1 +1 @@
-Classes/InitCallsSubclassMethod.ql
+Classes/InitCallsSubclass/InitCallsSubclassMethod.ql

python/ql/test/query-tests/Classes/init-calls-subclass-method/init_calls_subclass.py

@@ -1,22 +1,69 @@
 #Superclass __init__ calls subclass method
 
-class Super(object):
+def bad1():
+    class Super:
 
-    def __init__(self, arg):
-        self._state = "Not OK"
-        self.set_up(arg)
-        self._state = "OK"
+        def __init__(self, arg):
+            self._state = "Not OK"
+            self.set_up(arg) # BAD: set_up is overriden.
+            self._state = "OK"
 
-    def set_up(self, arg):
-        "Do some set up"
+        def set_up(self, arg):
+            "Do some set up"
 
-class Sub(Super):
+    class Sub(Super):
 
-    def __init__(self, arg):
-        Super.__init__(self, arg)
-        self.important_state = "OK"
+        def __init__(self, arg):
+            super().__init__(arg)
+            self.important_state = "OK"
 
-    def set_up(self, arg):
-        Super.set_up(self, arg)
-        "Do some more set up" # Dangerous as self._state is "Not OK" and
-                              # self.important_state is uninitialized
+        def set_up(self, arg):
+            super().set_up(arg)
+            "Do some more set up" # `self` is partially initialized
+            if self.important_state == "OK":
+                pass
+
+def good2():
+    class Super:
+        def __init__(self, arg):
+            self.a = arg 
+            self.postproc() # OK: Here `postproc` is called after initialisation. 
+
+        def postproc(self):
+            if self.a == 1:
+                pass
+
+    class Sub(Super):
+        def postproc(self):
+            if self.a == 2:
+                pass
+
+def good3():
+    class Super:
+        def __init__(self, arg):
+            self.a = arg 
+            self.set_b() # OK: Here `set_b` is used for initialisation, but does not read the partialy initialized state of `self`. 
+            self.c = 1
+
+        def set_b(self):
+            self.b = 3
+
+    class Sub(Super):
+        def set_b(self):
+            self.b = 4
+
+def good4():
+    class Super:
+        def __init__(self, arg):
+            self.a = arg 
+            # OK: Here `_set_b` is likely an internal method (as indicated by the _ prefix). 
+            # We assume thus that regular consumers of the library will not override it, and classes that do are internal and account for `self`'s partially initialised state.
+            self._set_b() 
+            self.c = 1 
+
+        def _set_b(self):
+            self.b = 3
+
+    class Sub(Super):
+        def _set_b(self): 
+            self.b = self.a+1