Add additional test cases

python/ql/src/Functions/SignatureOverriddenMethod.ql

@@ -108,6 +108,7 @@ predicate weakSignatureMismatch(Function base, Function sub, string msg) {
     exists(string arg |
       // TODO: positional-only args not considered
       // e.g. `def foo(x, y, /, z):` has x,y as positional only args, should not be considered as possible kw args
+      // However, this likely does not create FPs, as we require a 'witness' call to generate an alert.
       arg = base.getAnArg().getName() and
       not arg = sub.getAnArg().getName() and
       not exists(sub.getKwarg()) and
@@ -159,6 +160,9 @@ int extraSelfArg(Function func) { if isStaticmethod(func) then result = 0 else r
 
 predicate callMatchesSignature(Function func, Call call) {
   (
+    // TODO: This is not fully precise.
+    // For example, it does not detect that a method `def foo(self,x,y)` is matched by a call `obj.foo(1,y=2)`
+    // since y is passed in the call as a keyword argument, but still counts toward a positional argument of the method.
     call.getPositionalArgumentCount() + extraSelfArg(func) >= func.getMinPositionalArguments()
     or
     exists(call.getStarArg())

python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected

@@ -1,5 +1,11 @@
 | test.py:24:5:24:26 | Function meth1 | This method requires 2 positional arguments, whereas overridden $@ requires 1. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:5:5:5:20 | Function meth1 | Base.meth1 | test.py:15:9:15:20 | Attribute() | This call |
-| test.py:27:5:27:20 | Function meth2 | This method requires 1 positional argument, whereas overridden $@ requires 2 positional arguments. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:8:5:8:26 | Function meth2 | Base.meth2 | test.py:18:9:18:21 | Attribute() | This call |
+| test.py:27:5:27:20 | Function meth2 | This method requires 1 positional argument, whereas overridden $@ requires 2. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:8:5:8:26 | Function meth2 | Base.meth2 | test.py:18:9:18:21 | Attribute() | This call |
 | test.py:30:5:30:26 | Function meth3 | This method requires 2 positional arguments, whereas overridden $@ requires 1. | test.py:11:5:11:20 | Function meth3 | Base.meth3 | file://:0:0:0:0 | (none) | This call |
 | test.py:69:5:69:24 | Function meth | This method requires 2 positional arguments, whereas overridden $@ requires 1. | test.py:64:5:64:19 | Function meth | BlameBase.meth | file://:0:0:0:0 | (none) | This call |
 | test.py:74:5:74:24 | Function meth | This method requires 2 positional arguments, whereas overridden $@ requires 1. | test.py:64:5:64:19 | Function meth | BlameBase.meth | file://:0:0:0:0 | (none) | This call |
+| test.py:125:5:125:20 | Function meth1 | This method requires 1 positional argument, whereas overridden $@ may be called with 2. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:82:5:82:25 | Function meth1 | Base2.meth1 | test.py:110:9:110:23 | Attribute() | This call |
+| test.py:131:5:131:31 | Function meth4 | This method requires at least 3 positional arguments, whereas overridden $@ requires at most 2. | test.py:88:5:88:25 | Function meth4 | Base2.meth4 | file://:0:0:0:0 | (none) | This call |
+| test.py:133:5:133:28 | Function meth5 | This method requires at most 3 positional arguments, whereas overridden $@ requires at least 4. | test.py:90:5:90:34 | Function meth5 | Base2.meth5 | file://:0:0:0:0 | (none) | This call |
+| test.py:135:5:135:23 | Function meth6 | This method requires 2 positional arguments, whereas overridden $@ may be called with arbitrarily many. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:92:5:92:28 | Function meth6 | Base2.meth6 | test.py:113:9:113:27 | Attribute() | This call |
+| test.py:137:5:137:28 | Function meth7 | This method requires at least 2 positional arguments, whereas overridden $@ may be called with 1. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:94:5:94:25 | Function meth7 | Base2.meth7 | test.py:114:9:114:20 | Attribute() | This call |
+| test.py:147:5:147:21 | Function meth12 | This method does not accept arbitrary keyword arguments, which overridden $@ does. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:104:5:104:31 | Function meth12 | Base2.meth12 | test.py:119:9:119:24 | Attribute() | This call |

python/ql/test/query-tests/Functions/overriding/test.py

@@ -66,7 +66,7 @@ class BlameBase(object):
 
 class Correct1(BlameBase):
 
-    def meth(self, arg): # $Alert[py/inheritance/signature-mismatch] # Has 1 more arg. The incorrect-overriden-method query would alert for the base method in this case.
+    def meth(self, arg): # $Alert[py/inheritance/signature-mismatch] # Has 1 more arg. The incorrect-overridden-method query would alert for the base method in this case.
         pass
 
 class Correct2(BlameBase):
@@ -76,3 +76,74 @@ class Correct2(BlameBase):
 
 c = Correct2()
 c.meth("hi")
+
+class Base2:
+
+    def meth1(self, x=1): pass
+
+    def meth2(self, x=1): pass
+
+    def meth3(self): pass
+
+    def meth4(self, x=1): pass
+
+    def meth5(self, x, y, z, w=1): pass
+
+    def meth6(self, x, *ys): pass
+
+    def meth7(self, *ys): pass
+
+    def meth8(self, x, y): pass
+
+    def meth9(self, x, y): pass
+
+    def meth10(self, x, *, y=3): pass
+
+    def meth11(self, x, y): pass
+
+    def meth12(self, **kwargs): pass
+
+    def meth13(self, /, x): pass
+
+    def call_some(self):
+        self.meth1()
+        self.meth1(x=2)
+        self.meth3()
+        self.meth3(x=2)
+        self.meth6(2, 3, 4)
+        self.meth7()
+        self.meth8(1,y=3)
+        self.meth9(1,2)
+        self.meth10(1,y=3)
+        self.meth11(1,y=3)
+        self.meth12(x=2)
+        self.meth13(x=2)
+
+
+class Derrived2(Base2):
+
+    def meth1(self): pass # $Alert[py/inheritance/signature-mismatch] # Weak mismatch (base may be called with 2 args. only alert if mismatching call exists)
+
+    def meth2(self): pass # No alert (weak mismatch, but not called)
+
+    def meth3(self, x=1): pass # No alert (no mismatch - all base calls are valid for sub)
+
+    def meth4(self, x, y, z=1): pass # $Alert[py/inheritance/signature-mismatch] # sub min > base max (strong mismatch)
+
+    def meth5(self, x, y=1): pass # $Alert[py/inheritance/signature-mismatch]
+
+    def meth6(self, x): pass # $Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with 3+ args)
+
+    def meth7(self, x, *ys): pass # $Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with 1 arg only)
+
+    def meth8(self, x, z): pass # $MISSING:Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with arg named y), however the call to meth8 that witnesses this is not detected as a valid call to Base2.meth8.
+
+    def meth9(self, x, z): pass # No alert (never called with wrong keyword arg)
+
+    def meth10(self, x, **kwargs): pass # No alert (y is kw-only arg in base, calls that use it are valid for sub)
+
+    def meth11(self, x, z, **kwargs): pass # $MISSING:Alert[py/inheritance/signature-mismatch] # call using y kw-arg is invalid due to not specifying z, but this is not detected. Likely a fairly niche situation.
+
+    def meth12(self): pass # $Alert[py/inheritance/signature-mismatch] # call including extra kwarg invalid
+
+    def meth13(self, /, y): pass # $MISSING:Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with arg named x), however meth13 is incorrectly detected as having 2 minimum positional arguments, whereas x is kw-only; resulting in the witness call not being detected as a valid call to Base2.meth13.