From 5ba5007076f3726bebc8a14603d2ebccac78d025 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 6 Aug 2025 12:56:09 +0100
Subject: [PATCH 01/15] Modernize signature mismatch

---
 .../Functions/SignatureOverriddenMethod.ql    | 79 +++++++++++++++----
 1 file changed, 63 insertions(+), 16 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 3e3877bc139..74c5aaa477e 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -13,23 +13,70 @@
  */
 
 import python
-import Expressions.CallArgs
+import semmle.python.dataflow.new.internal.DataFlowDispatch
 
-from FunctionValue base, PythonFunctionValue derived
-where
-  not exists(base.getACall()) and
-  not exists(FunctionValue a_derived |
-    a_derived.overrides(base) and
-    exists(a_derived.getACall())
-  ) and
-  not derived.getScope().isSpecialMethod() and
-  derived.getName() != "__init__" and
-  derived.isNormalMethod() and
-  // call to overrides distributed for efficiency
+predicate overrides(Function base, Function sub) {
+  base.getName() = sub.getName() and
+  base.getScope() = getADirectSuperclass*(sub.getScope())
+}
+
+/** Holds if no way to call `base` would be valid for `sub`. The `msg` applies to the `sub method. */
+predicate strongSignatureMismatch(Function base, Function sub, string msg) {
+  overrides(base, sub) and
   (
-    derived.overrides(base) and derived.minParameters() > base.maxParameters()
+    sub.getMinPositionalArguments() > base.getMaxPositionalArguments() and
+    msg = "requires more positional arguments than overridden $@ allows."
     or
-    derived.overrides(base) and derived.maxParameters() < base.minParameters()
+    sub.getMaxPositionalArguments() < base.getMinPositionalArguments() and
+    msg = "requires fewer positional arguments than overridden $@ allows."
   )
-select derived, "Overriding method '" + derived.getName() + "' has signature mismatch with $@.",
-  base, "overridden method"
+}
+
+/** Holds if there may be some ways to call `base` that would not be valid for `sub`. The `msg` applies to the `sub` method. */
+predicate weakSignatureMismatch(Function base, Function sub, string msg) {
+  overrides(base, sub) and
+  (
+    sub.getMinPositionalArguments() > base.getMinPositionalArguments() and
+    msg = "requires more positional arguments than overridden $@ may accept."
+    or
+    sub.getMaxPositionalArguments() < base.getMaxPositionalArguments() and
+    msg = "requires fewer positional arguments than overridden $@ may accept."
+    or
+    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
+      arg = base.getAnArg().getName() and
+      not arg = sub.getAnArg().getName() and
+      not exists(sub.getKwarg()) and
+      msg = "does not accept keyword argument " + arg + ", which overridden $@ does."
+    )
+    or
+    exists(base.getKwarg()) and
+    not exists(sub.getKwarg()) and
+    msg = "does not accept arbitrary keyword arguments, which overridden $@ does."
+  )
+}
+
+predicate ignore(Function f) {
+  isClassmethod(f)
+  or
+  exists(Function g |
+    g.getScope() = f.getScope() and
+    g.getName() = f.getName() and
+    g != f
+  )
+}
+
+from Function base, Function sub, string msg
+where
+  // not exists(base.getACall()) and
+  // not exists(FunctionValue a_derived |
+  //   a_derived.overrides(base) and
+  //   exists(a_derived.getACall())
+  // ) and
+  not sub.isSpecialMethod() and
+  sub.getName() != "__init__" and
+  not ignore(sub) and
+  not ignore(base) and
+  strongSignatureMismatch(base, sub, msg)
+select sub, "This method " + msg, base, base.getQualifiedName()

From 4212d1b5b65bede97298d5d9e67efe8dd1563971 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 13 Aug 2025 10:18:29 +0100
Subject: [PATCH 02/15] Update alert messages and choose one witness

---
 .../Functions/SignatureOverriddenMethod.ql    | 134 ++++++++++++++++--
 1 file changed, 120 insertions(+), 14 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 74c5aaa477e..3bd8fb6bf31 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -13,11 +13,19 @@
  */
 
 import python
+import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.internal.DataFlowDispatch
 
 predicate overrides(Function base, Function sub) {
   base.getName() = sub.getName() and
-  base.getScope() = getADirectSuperclass*(sub.getScope())
+  base.getScope() = getADirectSuperclass+(sub.getScope())
+}
+
+bindingset[num, str]
+string plural(int num, string str) {
+  num = 1 and result = "1 " + str
+  or
+  num != 1 and result = num.toString() + " " + str + "s"
 }
 
 /** Holds if no way to call `base` would be valid for `sub`. The `msg` applies to the `sub method. */
@@ -25,10 +33,16 @@ predicate strongSignatureMismatch(Function base, Function sub, string msg) {
   overrides(base, sub) and
   (
     sub.getMinPositionalArguments() > base.getMaxPositionalArguments() and
-    msg = "requires more positional arguments than overridden $@ allows."
+    msg =
+      "requires " +
+        plural(sub.getMinPositionalArguments() - base.getMaxPositionalArguments(),
+          "more positional argument") + " than overridden $@ allows."
     or
     sub.getMaxPositionalArguments() < base.getMinPositionalArguments() and
-    msg = "requires fewer positional arguments than overridden $@ allows."
+    msg =
+      "requires " +
+        plural(base.getMinPositionalArguments() - sub.getMaxPositionalArguments(),
+          "fewer positional argument") + " than overridden $@ allows."
   )
 }
 
@@ -37,18 +51,26 @@ predicate weakSignatureMismatch(Function base, Function sub, string msg) {
   overrides(base, sub) and
   (
     sub.getMinPositionalArguments() > base.getMinPositionalArguments() and
-    msg = "requires more positional arguments than overridden $@ may accept."
+    msg =
+      "requires " +
+        plural(sub.getMinPositionalArguments() - base.getMinPositionalArguments(),
+          "more positional argument") + "than some possible calls to overridden $@."
     or
     sub.getMaxPositionalArguments() < base.getMaxPositionalArguments() and
-    msg = "requires fewer positional arguments than overridden $@ may accept."
+    msg =
+      "requires " +
+        plural(base.getMaxPositionalArguments() - sub.getMaxPositionalArguments(),
+          "fewer positional argument") + " than some possible calls to overridden $@."
     or
+    sub.getMinPositionalArguments() <= base.getMinPositionalArguments() and
+    sub.getMaxPositionalArguments() >= base.getMaxPositionalArguments() and
     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
       arg = base.getAnArg().getName() and
       not arg = sub.getAnArg().getName() and
       not exists(sub.getKwarg()) and
-      msg = "does not accept keyword argument " + arg + ", which overridden $@ does."
+      msg = "does not accept keyword argument `" + arg + "`, which overridden $@ does."
     )
     or
     exists(base.getKwarg()) and
@@ -67,16 +89,100 @@ predicate ignore(Function f) {
   )
 }
 
-from Function base, Function sub, string msg
+Function resolveCall(Call call) {
+  exists(DataFlowCall dfc | call = dfc.getNode().(CallNode).getNode() |
+    result = viableCallable(dfc).(DataFlowFunction).getScope()
+  )
+}
+
+predicate callViableForEither(Function base, Function sub, Call call) {
+  overrides(base, sub) and
+  base = resolveCall(call) and
+  sub = resolveCall(call)
+}
+
+predicate matchingStatic(Function base, Function sub) {
+  overrides(base, sub) and
+  (
+    isStaticmethod(base) and
+    isStaticmethod(sub)
+    or
+    not isStaticmethod(base) and
+    not isStaticmethod(sub)
+  )
+}
+
+int extraSelfArg(Function func) { if isStaticmethod(func) then result = 0 else result = 1 }
+
+predicate callMatchesSignature(Function func, Call call) {
+  (
+    call.getPositionalArgumentCount() + extraSelfArg(func) >= func.getMinPositionalArguments()
+    or
+    exists(call.getStarArg())
+    or
+    exists(call.getKwargs())
+  ) and
+  call.getPositionalArgumentCount() + extraSelfArg(func) <= func.getMaxPositionalArguments() and
+  (
+    exists(func.getKwarg())
+    or
+    forall(string name | name = call.getANamedArgumentName() | exists(func.getArgByName(name)))
+  )
+}
+
+Call getASignatureMismatchWitness(Function base, Function sub) {
+  callViableForEither(base, sub, result) and
+  callMatchesSignature(base, result) and
+  not callMatchesSignature(sub, result)
+}
+
+Call chooseASignatureMismatchWitnessInFile(Function base, Function sub, File file) {
+  result =
+    min(Call c |
+      c = getASignatureMismatchWitness(base, sub) and
+      c.getLocation().getFile() = file
+    |
+      c order by c.getLocation().getStartLine(), c.getLocation().getStartColumn()
+    )
+}
+
+Call chooseASignatureMismatchWitness(Function base, Function sub) {
+  exists(getASignatureMismatchWitness(base, sub)) and
+  (
+    result = chooseASignatureMismatchWitnessInFile(base, sub, base.getLocation().getFile())
+    or
+    not exists(Call c |
+      c = getASignatureMismatchWitness(base, sub) and
+      c.getLocation().getFile() = base.getLocation().getFile()
+    ) and
+    result = chooseASignatureMismatchWitnessInFile(base, sub, base.getLocation().getFile())
+    or
+    not exists(Call c |
+      c = getASignatureMismatchWitness(base, sub) and
+      c.getLocation().getFile() = [base, sub].getLocation().getFile()
+    ) and
+    result =
+      min(Call c |
+        c = getASignatureMismatchWitness(base, sub)
+      |
+        c
+        order by
+          c.getLocation().getFile().getAbsolutePath(), c.getLocation().getStartLine(),
+          c.getLocation().getStartColumn()
+      )
+  )
+}
+
+from Function base, Function sub, string msg, string extraMsg, Call call
 where
-  // not exists(base.getACall()) and
-  // not exists(FunctionValue a_derived |
-  //   a_derived.overrides(base) and
-  //   exists(a_derived.getACall())
-  // ) and
   not sub.isSpecialMethod() and
   sub.getName() != "__init__" and
   not ignore(sub) and
   not ignore(base) and
-  strongSignatureMismatch(base, sub, msg)
-select sub, "This method " + msg, base, base.getQualifiedName()
+  matchingStatic(base, sub) and
+  weakSignatureMismatch(base, sub, msg) and
+  //msg = " has a different signature to $@." and
+  call = chooseASignatureMismatchWitness(base, sub) and
+  extraMsg =
+    " $@ correctly calls the base method, but does not match the signature of the overriding method."
+select sub, "This method " + msg + extraMsg, base, base.getQualifiedName(), call, "This call"

From 067c98d3ee00af0e8608f2ac89aad54125586bdb Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 13 Aug 2025 11:55:39 +0100
Subject: [PATCH 03/15] Include conditional alert messages for various cases

---
 .../Functions/SignatureOverriddenMethod.ql    | 27 ++++++++++++++-----
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 3bd8fb6bf31..cb97873b32d 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -15,6 +15,7 @@
 import python
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.internal.DataFlowDispatch
+import codeql.util.Option
 
 predicate overrides(Function base, Function sub) {
   base.getName() = sub.getName() and
@@ -54,7 +55,7 @@ predicate weakSignatureMismatch(Function base, Function sub, string msg) {
     msg =
       "requires " +
         plural(sub.getMinPositionalArguments() - base.getMinPositionalArguments(),
-          "more positional argument") + "than some possible calls to overridden $@."
+          "more positional argument") + " than some possible calls to overridden $@."
     or
     sub.getMaxPositionalArguments() < base.getMaxPositionalArguments() and
     msg =
@@ -173,16 +174,28 @@ Call chooseASignatureMismatchWitness(Function base, Function sub) {
   )
 }
 
-from Function base, Function sub, string msg, string extraMsg, Call call
+module CallOption = LocOption2<Location, Call>;
+
+from Function base, Function sub, string msg, string extraMsg, CallOption::Option call
 where
   not sub.isSpecialMethod() and
   sub.getName() != "__init__" and
   not ignore(sub) and
   not ignore(base) and
   matchingStatic(base, sub) and
-  weakSignatureMismatch(base, sub, msg) and
-  //msg = " has a different signature to $@." and
-  call = chooseASignatureMismatchWitness(base, sub) and
-  extraMsg =
-    " $@ correctly calls the base method, but does not match the signature of the overriding method."
+  (
+    call.asSome() = chooseASignatureMismatchWitness(base, sub) and
+    extraMsg =
+      " $@ correctly calls the base method, but does not match the signature of the overriding method." and
+    (
+      strongSignatureMismatch(base, sub, msg)
+      or
+      not strongSignatureMismatch(base, sub, _) and
+      weakSignatureMismatch(base, sub, msg)
+    )
+    or
+    not exists(getASignatureMismatchWitness(base, sub)) and
+    strongSignatureMismatch(base, sub, msg) and
+    extraMsg = ""
+  )
 select sub, "This method " + msg + extraMsg, base, base.getQualifiedName(), call, "This call"

From f429b9038c238e431cdbded3c9764b3116dccef2 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 15 Aug 2025 14:22:34 +0100
Subject: [PATCH 04/15] Update tests, update alert messages

---
 .../Functions/IncorrectlyOverriddenMethod.ql  |  1 +
 .../Functions/SignatureOverriddenMethod.ql    | 73 +++++++++++++++----
 .../SignatureOverriddenMethod.expected        |  6 +-
 .../SignatureOverriddenMethod.qlref           |  3 +-
 .../query-tests/Functions/overriding/test.py  | 10 +--
 5 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/python/ql/src/Functions/IncorrectlyOverriddenMethod.ql b/python/ql/src/Functions/IncorrectlyOverriddenMethod.ql
index a46a2370c0e..a4e3bd6e0ce 100644
--- a/python/ql/src/Functions/IncorrectlyOverriddenMethod.ql
+++ b/python/ql/src/Functions/IncorrectlyOverriddenMethod.ql
@@ -1,4 +1,5 @@
 /**
+ * @deprecated
  * @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
diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index cb97873b32d..bf4e16306dd 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -29,21 +29,63 @@ string plural(int num, string str) {
   num != 1 and result = num.toString() + " " + str + "s"
 }
 
+string describeMin(Function func) {
+  exists(string descr | descr = plural(func.getMinPositionalArguments(), "positional argument") |
+    if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
+    then result = descr
+    else result = "at least " + descr
+  )
+}
+
+string describeMax(Function func) {
+  if func.hasVarArg()
+  then result = "arbitrarily many positional arguments"
+  else
+    exists(string descr | descr = plural(func.getMaxPositionalArguments(), "positional argument") |
+      if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
+      then result = descr
+      else result = "at most " + descr
+    )
+}
+
+string describeMinShort(Function func) {
+  exists(string descr | descr = func.getMinPositionalArguments().toString() |
+    if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
+    then result = descr
+    else result = "at least " + descr
+  )
+}
+
+string describeMaxShort(Function func) {
+  if func.hasVarArg()
+  then result = "arbitrarily many"
+  else
+    exists(string descr | descr = func.getMaxPositionalArguments().toString() |
+      if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
+      then result = descr
+      else result = "at most " + descr
+    )
+}
+
+string describeMaxBound(Function func) {
+  if func.hasVarArg()
+  then result = "arbitrarily many"
+  else result = func.getMaxPositionalArguments().toString()
+}
+
 /** Holds if no way to call `base` would be valid for `sub`. The `msg` applies to the `sub method. */
 predicate strongSignatureMismatch(Function base, Function sub, string msg) {
   overrides(base, sub) and
   (
     sub.getMinPositionalArguments() > base.getMaxPositionalArguments() and
     msg =
-      "requires " +
-        plural(sub.getMinPositionalArguments() - base.getMaxPositionalArguments(),
-          "more positional argument") + " than overridden $@ allows."
+      "requires " + describeMin(sub) + ", whereas overridden $@ requires " + describeMaxShort(base) +
+        "."
     or
     sub.getMaxPositionalArguments() < base.getMinPositionalArguments() and
     msg =
-      "requires " +
-        plural(base.getMinPositionalArguments() - sub.getMaxPositionalArguments(),
-          "fewer positional argument") + " than overridden $@ allows."
+      "requires " + describeMax(sub) + ", whereas overridden $@ requires " + describeMinShort(base) +
+        "."
   )
 }
 
@@ -53,15 +95,13 @@ predicate weakSignatureMismatch(Function base, Function sub, string msg) {
   (
     sub.getMinPositionalArguments() > base.getMinPositionalArguments() and
     msg =
-      "requires " +
-        plural(sub.getMinPositionalArguments() - base.getMinPositionalArguments(),
-          "more positional argument") + " than some possible calls to overridden $@."
+      "requires " + describeMin(sub) + ", whereas overridden $@ may be called with " +
+        base.getMinPositionalArguments().toString() + "."
     or
     sub.getMaxPositionalArguments() < base.getMaxPositionalArguments() and
     msg =
-      "requires " +
-        plural(base.getMaxPositionalArguments() - sub.getMaxPositionalArguments(),
-          "fewer positional argument") + " than some possible calls to overridden $@."
+      "requires " + describeMax(sub) + ", whereas overridden $@ may be called with " +
+        describeMaxBound(base) + "."
     or
     sub.getMinPositionalArguments() <= base.getMinPositionalArguments() and
     sub.getMaxPositionalArguments() >= base.getMaxPositionalArguments() and
@@ -83,7 +123,9 @@ predicate weakSignatureMismatch(Function base, Function sub, string msg) {
 predicate ignore(Function f) {
   isClassmethod(f)
   or
-  exists(Function g |
+  exists(
+    Function g // other functions with the same name, e.g. @property getters/setters.
+  |
     g.getScope() = f.getScope() and
     g.getName() = f.getName() and
     g != f
@@ -96,7 +138,7 @@ Function resolveCall(Call call) {
   )
 }
 
-predicate callViableForEither(Function base, Function sub, Call call) {
+predicate callViableForEitherOverride(Function base, Function sub, Call call) {
   overrides(base, sub) and
   base = resolveCall(call) and
   sub = resolveCall(call)
@@ -132,7 +174,7 @@ predicate callMatchesSignature(Function func, Call call) {
 }
 
 Call getASignatureMismatchWitness(Function base, Function sub) {
-  callViableForEither(base, sub, result) and
+  callViableForEitherOverride(base, sub, result) and
   callMatchesSignature(base, result) and
   not callMatchesSignature(sub, result)
 }
@@ -195,6 +237,7 @@ where
     )
     or
     not exists(getASignatureMismatchWitness(base, sub)) and
+    call.isNone() and
     strongSignatureMismatch(base, sub, msg) and
     extraMsg = ""
   )
diff --git a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
index bcb9363a55a..bc8accf6f30 100644
--- a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
+++ b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
@@ -1 +1,5 @@
-| test.py:30:5:30:26 | Function Derived.meth3 | Overriding method 'meth3' has signature mismatch with $@. | test.py:11:5:11:20 | Function Base.meth3 | overridden method |
+| 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: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 |
diff --git a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.qlref b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.qlref
index a306477b3b4..5470a05e0e4 100644
--- a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.qlref
+++ b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.qlref
@@ -1 +1,2 @@
-Functions/SignatureOverriddenMethod.ql
+query: Functions/SignatureOverriddenMethod.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql
diff --git a/python/ql/test/query-tests/Functions/overriding/test.py b/python/ql/test/query-tests/Functions/overriding/test.py
index c4c7caaa1aa..a289bfc79c4 100644
--- a/python/ql/test/query-tests/Functions/overriding/test.py
+++ b/python/ql/test/query-tests/Functions/overriding/test.py
@@ -21,13 +21,13 @@ class Base(object):
 
 class Derived(Base):
 
-    def meth1(self, spam):
+    def meth1(self, spam): # $Alert[py/inheritance/signature-mismatch] # Has 1 more arg, base called in Base.foo
         pass
 
-    def meth2(self):
+    def meth2(self): # $Alert[py/inheritance/signature-mismatch] # Has 1 fewer arg, base called in Base.foo
         pass
 
-    def meth3(self, eggs): #Incorrectly overridden and not called. 
+    def meth3(self, eggs): # $Alert[py/inheritance/signature-mismatch] # Has 1 more arg. Method is not called.
         pass
 
     def bar(self):
@@ -66,12 +66,12 @@ class BlameBase(object):
 
 class Correct1(BlameBase):
 
-    def meth(self, arg):
+    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.
         pass
 
 class Correct2(BlameBase):
 
-    def meth(self, arg):
+    def meth(self, arg): # $Alert[py/inheritance/signature-mismatch] # Has 1 more arg
         pass
 
 c = Correct2()

From 2bbf24b3ea6279938c7d98225d170618b63a7949 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 18 Aug 2025 16:01:39 +0100
Subject: [PATCH 05/15] Add additional test cases

---
 .../Functions/SignatureOverriddenMethod.ql    |  4 +
 .../SignatureOverriddenMethod.expected        |  8 +-
 .../query-tests/Functions/overriding/test.py  | 73 ++++++++++++++++++-
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index bf4e16306dd..4ac515f935a 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/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())
diff --git a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
index bc8accf6f30..255894ca6fc 100644
--- a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
+++ b/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 |
diff --git a/python/ql/test/query-tests/Functions/overriding/test.py b/python/ql/test/query-tests/Functions/overriding/test.py
index a289bfc79c4..cc70da66822 100644
--- a/python/ql/test/query-tests/Functions/overriding/test.py
+++ b/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. 

From 502ea82c9157981f675fd8f8c1a611bb53482f34 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 19 Aug 2025 09:39:14 +0100
Subject: [PATCH 06/15] Updae other test output

---
 .../Functions/general/SignatureOverriddenMethod.expected      | 4 ++--
 .../Functions/overriding/WrongNameForArgumentInCall.expected  | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/ql/test/query-tests/Functions/general/SignatureOverriddenMethod.expected b/python/ql/test/query-tests/Functions/general/SignatureOverriddenMethod.expected
index 81c45ab8abe..21b11212075 100644
--- a/python/ql/test/query-tests/Functions/general/SignatureOverriddenMethod.expected
+++ b/python/ql/test/query-tests/Functions/general/SignatureOverriddenMethod.expected
@@ -1,2 +1,2 @@
-| om_test.py:32:5:32:35 | Function Derived.grossly_wrong1 | Overriding method 'grossly_wrong1' has signature mismatch with $@. | om_test.py:12:5:12:41 | Function Base.grossly_wrong1 | overridden method |
-| om_test.py:35:5:35:47 | Function Derived.grossly_wrong2 | Overriding method 'grossly_wrong2' has signature mismatch with $@. | om_test.py:15:5:15:41 | Function Base.grossly_wrong2 | overridden method |
+| om_test.py:32:5:32:35 | Function grossly_wrong1 | This method requires 2 positional arguments, whereas overridden $@ requires 3. | om_test.py:12:5:12:41 | Function grossly_wrong1 | Base.grossly_wrong1 | file://:0:0:0:0 | (none) | This call |
+| om_test.py:35:5:35:47 | Function grossly_wrong2 | This method requires 4 positional arguments, whereas overridden $@ requires 3. | om_test.py:15:5:15:41 | Function grossly_wrong2 | Base.grossly_wrong2 | file://:0:0:0:0 | (none) | This call |
diff --git a/python/ql/test/query-tests/Functions/overriding/WrongNameForArgumentInCall.expected b/python/ql/test/query-tests/Functions/overriding/WrongNameForArgumentInCall.expected
index d2fc2ef6784..0cadf5e5fbf 100644
--- a/python/ql/test/query-tests/Functions/overriding/WrongNameForArgumentInCall.expected
+++ b/python/ql/test/query-tests/Functions/overriding/WrongNameForArgumentInCall.expected
@@ -1 +1,2 @@
 | test.py:19:9:19:31 | Attribute() | Keyword argument 'spam' is not a supported parameter name of $@. | test.py:5:5:5:20 | Function meth1 | method Base.meth1 |
+| test.py:112:9:112:23 | Attribute() | Keyword argument 'x' is not a supported parameter name of $@. | test.py:86:5:86:20 | Function meth3 | method Base2.meth3 |

From 900a5cd9d7e1f4fd0a1cbbd2a8245370876ac5b8 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 19 Aug 2025 12:31:45 +0100
Subject: [PATCH 07/15] Update documentation

---
 .../Functions/SignatureOverriddenMethod.py    | 15 +++++++-----
 .../Functions/SignatureOverriddenMethod.qhelp | 23 +++++++------------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.py b/python/ql/src/Functions/SignatureOverriddenMethod.py
index 7beddcb9e95..e3522351fc0 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.py
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.py
@@ -1,9 +1,12 @@
 
-# Base class method
-def runsource(self, source, filename="<input>", symbol="single"):
-    ... # Definition
+class Base:
+    def runsource(self, source, filename="<input>"):
+        ...
     
     
-# Extend base class method
-def runsource(self, source):
-    ... # Definition
\ No newline at end of file
+class Sub(Base):
+    def runsource(self, source): # BAD: Does not match the signature of overridden method.
+        ... 
+
+def run(obj: Base):
+    obj.runsource("source", filename="foo.txt")
\ No newline at end of file
diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.qhelp b/python/ql/src/Functions/SignatureOverriddenMethod.qhelp
index b7da2678e3d..69fd13ed90f 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.qhelp
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.qhelp
@@ -5,32 +5,25 @@
 
 
 <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> When the signature of a method of a base class and a method of a subclass that overrides it don't match, a call to the base class method 
+may not be a valid call to the subclass method, and thus raise an exception if an instance of the subclass is passed instead. 
+If following the Liskov Substitution Principle, in which an instance of a subclass should be usable in every context as though it were a an 
+instance of the base class, this behavior breaks the principle.
 </p>
 
 </overview>
 <recommendation>
 
-<p>Ensure that the overriding method accepts all the parameters that are legal for
-overridden method.</p>
+<p>Ensure that the overriding method in the subclass accepts the same parameters as the base 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>
+<p>In the following example, <code>Base.runsource</code> takes an optional <code>filename</code> argument. However, the overriding method 
+<code>Sub.runsource</code> does not. This means the <code>run</code> function will fail if passed an instance of <code>Sub</code>.
+</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>
 

From 0a83c11f42dd272878a18b17efc2a2fa6da9ec79 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 19 Aug 2025 13:06:41 +0100
Subject: [PATCH 08/15] Add changenote.+ fix typo

---
 python/ql/src/Functions/SignatureOverriddenMethod.qhelp     | 2 +-
 python/ql/src/change-notes/2025-08-19-signature-mismatch.md | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 python/ql/src/change-notes/2025-08-19-signature-mismatch.md

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.qhelp b/python/ql/src/Functions/SignatureOverriddenMethod.qhelp
index 69fd13ed90f..0575d967d3e 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.qhelp
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.qhelp
@@ -7,7 +7,7 @@
 <overview>
 <p> When the signature of a method of a base class and a method of a subclass that overrides it don't match, a call to the base class method 
 may not be a valid call to the subclass method, and thus raise an exception if an instance of the subclass is passed instead. 
-If following the Liskov Substitution Principle, in which an instance of a subclass should be usable in every context as though it were a an 
+If following the Liskov Substitution Principle, in which an instance of a subclass should be usable in every context as though it were an 
 instance of the base class, this behavior breaks the principle.
 </p>
 
diff --git a/python/ql/src/change-notes/2025-08-19-signature-mismatch.md b/python/ql/src/change-notes/2025-08-19-signature-mismatch.md
new file mode 100644
index 00000000000..60c3efa32eb
--- /dev/null
+++ b/python/ql/src/change-notes/2025-08-19-signature-mismatch.md
@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* The `py/inheritance/signature-mismatch` query has been modernized. It produces more precise results and more descriptive alert messages. 
+* The `py/inheritance/incorrect-overriding-signature` query has been deprecated. Its results have been consolidated into the `py/inheritance/signature-mismatch` query.
\ No newline at end of file

From 6587ad435ec460517c359b016babb62c5f4699fd Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 19 Aug 2025 15:18:30 +0100
Subject: [PATCH 09/15] Update
 python/ql/src/Functions/SignatureOverriddenMethod.ql

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---
 python/ql/src/Functions/SignatureOverriddenMethod.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 4ac515f935a..63da1ec35e4 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -202,7 +202,7 @@ Call chooseASignatureMismatchWitness(Function base, Function sub) {
       c = getASignatureMismatchWitness(base, sub) and
       c.getLocation().getFile() = base.getLocation().getFile()
     ) and
-    result = chooseASignatureMismatchWitnessInFile(base, sub, base.getLocation().getFile())
+    result = chooseASignatureMismatchWitnessInFile(base, sub, sub.getLocation().getFile())
     or
     not exists(Call c |
       c = getASignatureMismatchWitness(base, sub) and

From 125c6534b76a053b26ca2f9e9491d2f47e6c7923 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 1 Sep 2025 23:41:28 +0100
Subject: [PATCH 10/15] Use new option name

---
 python/ql/src/Functions/SignatureOverriddenMethod.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 63da1ec35e4..b76611596f6 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -220,7 +220,7 @@ Call chooseASignatureMismatchWitness(Function base, Function sub) {
   )
 }
 
-module CallOption = LocOption2<Location, Call>;
+module CallOption = LocatableOption<Location, Call>;
 
 from Function base, Function sub, string msg, string extraMsg, CallOption::Option call
 where

From 318d1cd392efb6ab1e8aa46c6ddfc9e6f633e7a9 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 2 Sep 2025 12:02:08 +0100
Subject: [PATCH 11/15] Increase precision in detecting call matches signature

---
 .../Functions/SignatureOverriddenMethod.ql    | 20 +++++++++++++++----
 .../SignatureOverriddenMethod.expected        |  2 ++
 .../query-tests/Functions/overriding/test.py  |  4 ++--
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index b76611596f6..623521cc9c5 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -160,15 +160,27 @@ 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()
+    // Each parameter of the function is accounted for in the call
+    forall(Parameter param, int i | param = func.getArg(i) |
+      // self arg
+      i = 0 and not isStaticmethod(func)
+      or
+      // positional arg
+      i - extraSelfArg(func) < call.getPositionalArgumentCount()
+      or
+      // has default
+      exists(param.getDefault())
+      or
+      // keyword arg
+      call.getANamedArgumentName() = param.getName()
+    )
     or
+    // arbitrary varargs or kwargs
     exists(call.getStarArg())
     or
     exists(call.getKwargs())
   ) and
+  // No excess parameters
   call.getPositionalArgumentCount() + extraSelfArg(func) <= func.getMaxPositionalArguments() and
   (
     exists(func.getKwarg())
diff --git a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
index 255894ca6fc..07756ee4600 100644
--- a/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
+++ b/python/ql/test/query-tests/Functions/overriding/SignatureOverriddenMethod.expected
@@ -8,4 +8,6 @@
 | 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:139:5:139:26 | Function meth8 | This method does not accept keyword argument `y`, which overridden $@ does. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:96:5:96:26 | Function meth8 | Base2.meth8 | test.py:115:9:115:25 | 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 |
+| test.py:149:5:149:27 | Function meth13 | This method does not accept keyword argument `x`, which overridden $@ does. $@ correctly calls the base method, but does not match the signature of the overriding method. | test.py:106:5:106:27 | Function meth13 | Base2.meth13 | test.py:120:9:120:24 | Attribute() | This call |
diff --git a/python/ql/test/query-tests/Functions/overriding/test.py b/python/ql/test/query-tests/Functions/overriding/test.py
index cc70da66822..63ee50e820c 100644
--- a/python/ql/test/query-tests/Functions/overriding/test.py
+++ b/python/ql/test/query-tests/Functions/overriding/test.py
@@ -136,7 +136,7 @@ class Derrived2(Base2):
 
     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 meth8(self, x, z): pass # $Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with arg named y)
 
     def meth9(self, x, z): pass # No alert (never called with wrong keyword arg)
 
@@ -146,4 +146,4 @@ class Derrived2(Base2):
 
     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. 
+    def meth13(self, /, y): pass # $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. 

From 9fa630faf597b68d3c27fcfe65c17ebf2a43ac7d Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 3 Sep 2025 11:00:59 +0100
Subject: [PATCH 12/15] Add comments documenting helper predicates, and add
 call resolve condition to callMatchesSignature to avoid cartesian product

---
 .../src/Functions/SignatureOverriddenMethod.ql | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 623521cc9c5..fa1caedb2df 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -17,11 +17,13 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.internal.DataFlowDispatch
 import codeql.util.Option
 
+/** Holds if `base` is overriden by `sub` */
 predicate overrides(Function base, Function sub) {
   base.getName() = sub.getName() and
   base.getScope() = getADirectSuperclass+(sub.getScope())
 }
 
+/** Constructs a string to pluralize `str` depending on `num`. */
 bindingset[num, str]
 string plural(int num, string str) {
   num = 1 and result = "1 " + str
@@ -29,6 +31,7 @@ string plural(int num, string str) {
   num != 1 and result = num.toString() + " " + str + "s"
 }
 
+/** Describes the minimum number of arguments `func` can accept, using "at least" if it may accept more. */
 string describeMin(Function func) {
   exists(string descr | descr = plural(func.getMinPositionalArguments(), "positional argument") |
     if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
@@ -37,6 +40,7 @@ string describeMin(Function func) {
   )
 }
 
+/** Described the maximum number of arguments `func` can accept, using "at most" if it may accept fewer, and "arbitrarily many" if it has a vararg. */
 string describeMax(Function func) {
   if func.hasVarArg()
   then result = "arbitrarily many positional arguments"
@@ -48,6 +52,7 @@ string describeMax(Function func) {
     )
 }
 
+/** Describes the minumum number of arguments `func` can accept, without repeating "positional arguments". */
 string describeMinShort(Function func) {
   exists(string descr | descr = func.getMinPositionalArguments().toString() |
     if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
@@ -56,6 +61,7 @@ string describeMinShort(Function func) {
   )
 }
 
+/** Describes the maximum number of arguments `func` can accept, without repeating "positional arguments". */
 string describeMaxShort(Function func) {
   if func.hasVarArg()
   then result = "arbitrarily many"
@@ -67,6 +73,7 @@ string describeMaxShort(Function func) {
     )
 }
 
+/** Describe an upper bound on the number of arguments `func` may accept, without specifying "at most". */
 string describeMaxBound(Function func) {
   if func.hasVarArg()
   then result = "arbitrarily many"
@@ -121,6 +128,7 @@ predicate weakSignatureMismatch(Function base, Function sub, string msg) {
   )
 }
 
+/** Holds if `f` should be ignored for considering signature mismatches. */
 predicate ignore(Function f) {
   isClassmethod(f)
   or
@@ -133,18 +141,21 @@ predicate ignore(Function f) {
   )
 }
 
+/** Gets a function that `call` may resolve to. */
 Function resolveCall(Call call) {
   exists(DataFlowCall dfc | call = dfc.getNode().(CallNode).getNode() |
     result = viableCallable(dfc).(DataFlowFunction).getScope()
   )
 }
 
+/** Holds if `call` may resolve to either `base` or `sub`, and `base` is overiden by `sub`. */
 predicate callViableForEitherOverride(Function base, Function sub, Call call) {
   overrides(base, sub) and
   base = resolveCall(call) and
   sub = resolveCall(call)
 }
 
+/** Holds if either both `base` and `sub` are static methods, or both are not static methods, and `base` is overriden by `sub`. */
 predicate matchingStatic(Function base, Function sub) {
   overrides(base, sub) and
   (
@@ -158,7 +169,9 @@ predicate matchingStatic(Function base, Function sub) {
 
 int extraSelfArg(Function func) { if isStaticmethod(func) then result = 0 else result = 1 }
 
+/** Holds if the call `call` matches the signature for `func`. */
 predicate callMatchesSignature(Function func, Call call) {
+  func = resolveCall(call) and
   (
     // Each parameter of the function is accounted for in the call
     forall(Parameter param, int i | param = func.getArg(i) |
@@ -189,12 +202,14 @@ predicate callMatchesSignature(Function func, Call call) {
   )
 }
 
+/** Gets a call which matches the signature of `base`, but not of overriden `sub`. */
 Call getASignatureMismatchWitness(Function base, Function sub) {
   callViableForEitherOverride(base, sub, result) and
   callMatchesSignature(base, result) and
   not callMatchesSignature(sub, result)
 }
 
+/** Choose a 'witnessing' call that matches the signature of `base` but not of overridden `sub`, and is in the file `file`. */
 Call chooseASignatureMismatchWitnessInFile(Function base, Function sub, File file) {
   result =
     min(Call c |
@@ -205,6 +220,7 @@ Call chooseASignatureMismatchWitnessInFile(Function base, Function sub, File fil
     )
 }
 
+/** Choose a 'witnessing' call that matches the signature of `base` but not of overridden `sub`. */
 Call chooseASignatureMismatchWitness(Function base, Function sub) {
   exists(getASignatureMismatchWitness(base, sub)) and
   (
@@ -242,6 +258,7 @@ where
   not ignore(base) and
   matchingStatic(base, sub) and
   (
+    // If we have a witness, alert for a 'weak' mismatch, but prefer the message for a 'strong' mismatch if that holds.
     call.asSome() = chooseASignatureMismatchWitness(base, sub) and
     extraMsg =
       " $@ correctly calls the base method, but does not match the signature of the overriding method." and
@@ -252,6 +269,7 @@ where
       weakSignatureMismatch(base, sub, msg)
     )
     or
+    // With no witness, only alert for 'strong' mismatches.
     not exists(getASignatureMismatchWitness(base, sub)) and
     call.isNone() and
     strongSignatureMismatch(base, sub, msg) and

From 71dec0b23e01bc24c3238aaa3a28f77c1b0f6057 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 3 Sep 2025 11:06:21 +0100
Subject: [PATCH 13/15] Fix typos

---
 python/ql/src/Functions/SignatureOverriddenMethod.ql | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index fa1caedb2df..54c5e3243b0 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -17,7 +17,7 @@ import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.internal.DataFlowDispatch
 import codeql.util.Option
 
-/** Holds if `base` is overriden by `sub` */
+/** Holds if `base` is overridden by `sub` */
 predicate overrides(Function base, Function sub) {
   base.getName() = sub.getName() and
   base.getScope() = getADirectSuperclass+(sub.getScope())
@@ -52,7 +52,7 @@ string describeMax(Function func) {
     )
 }
 
-/** Describes the minumum number of arguments `func` can accept, without repeating "positional arguments". */
+/** Describes the minimum number of arguments `func` can accept, without repeating "positional arguments". */
 string describeMinShort(Function func) {
   exists(string descr | descr = func.getMinPositionalArguments().toString() |
     if func.getMinPositionalArguments() = func.getMaxPositionalArguments()
@@ -148,14 +148,14 @@ Function resolveCall(Call call) {
   )
 }
 
-/** Holds if `call` may resolve to either `base` or `sub`, and `base` is overiden by `sub`. */
+/** Holds if `call` may resolve to either `base` or `sub`, and `base` is overridden by `sub`. */
 predicate callViableForEitherOverride(Function base, Function sub, Call call) {
   overrides(base, sub) and
   base = resolveCall(call) and
   sub = resolveCall(call)
 }
 
-/** Holds if either both `base` and `sub` are static methods, or both are not static methods, and `base` is overriden by `sub`. */
+/** Holds if either both `base` and `sub` are static methods, or both are not static methods, and `base` is overridden by `sub`. */
 predicate matchingStatic(Function base, Function sub) {
   overrides(base, sub) and
   (
@@ -202,7 +202,7 @@ predicate callMatchesSignature(Function func, Call call) {
   )
 }
 
-/** Gets a call which matches the signature of `base`, but not of overriden `sub`. */
+/** Gets a call which matches the signature of `base`, but not of overridden `sub`. */
 Call getASignatureMismatchWitness(Function base, Function sub) {
   callViableForEitherOverride(base, sub, result) and
   callMatchesSignature(base, result) and

From eb246f6f71118d0aa7f4a19d315c76e46ea4676d Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 8 Sep 2025 09:43:22 +0100
Subject: [PATCH 14/15] Performance experiment - add getFunctionFIle for better
 join order

---
 python/ql/src/Functions/SignatureOverriddenMethod.ql | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index 54c5e3243b0..feb69f5b13c 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -202,6 +202,9 @@ predicate callMatchesSignature(Function func, Call call) {
   )
 }
 
+pragma[nomagic]
+private File getFunctionFile(Function f) { result = f.getLocation().getFile() }
+
 /** Gets a call which matches the signature of `base`, but not of overridden `sub`. */
 Call getASignatureMismatchWitness(Function base, Function sub) {
   callViableForEitherOverride(base, sub, result) and
@@ -224,17 +227,17 @@ Call chooseASignatureMismatchWitnessInFile(Function base, Function sub, File fil
 Call chooseASignatureMismatchWitness(Function base, Function sub) {
   exists(getASignatureMismatchWitness(base, sub)) and
   (
-    result = chooseASignatureMismatchWitnessInFile(base, sub, base.getLocation().getFile())
+    result = chooseASignatureMismatchWitnessInFile(base, sub, getFunctionFile(base))
     or
     not exists(Call c |
       c = getASignatureMismatchWitness(base, sub) and
       c.getLocation().getFile() = base.getLocation().getFile()
     ) and
-    result = chooseASignatureMismatchWitnessInFile(base, sub, sub.getLocation().getFile())
+    result = chooseASignatureMismatchWitnessInFile(base, sub, getFunctionFile(sub))
     or
     not exists(Call c |
       c = getASignatureMismatchWitness(base, sub) and
-      c.getLocation().getFile() = [base, sub].getLocation().getFile()
+      c.getLocation().getFile() = getFunctionFile([base, sub])
     ) and
     result =
       min(Call c |

From f9e094de619b2d37342d717d266cb101ac8afd7b Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 9 Sep 2025 17:25:48 +0100
Subject: [PATCH 15/15] Simplify choosaASignatureMismatchWitness for improved
 performance

---
 .../Functions/SignatureOverriddenMethod.ql    | 49 +++++++------------
 1 file changed, 17 insertions(+), 32 deletions(-)

diff --git a/python/ql/src/Functions/SignatureOverriddenMethod.ql b/python/ql/src/Functions/SignatureOverriddenMethod.ql
index feb69f5b13c..15b3fa70640 100644
--- a/python/ql/src/Functions/SignatureOverriddenMethod.ql
+++ b/python/ql/src/Functions/SignatureOverriddenMethod.ql
@@ -212,43 +212,28 @@ Call getASignatureMismatchWitness(Function base, Function sub) {
   not callMatchesSignature(sub, result)
 }
 
-/** Choose a 'witnessing' call that matches the signature of `base` but not of overridden `sub`, and is in the file `file`. */
-Call chooseASignatureMismatchWitnessInFile(Function base, Function sub, File file) {
-  result =
-    min(Call c |
-      c = getASignatureMismatchWitness(base, sub) and
-      c.getLocation().getFile() = file
-    |
-      c order by c.getLocation().getStartLine(), c.getLocation().getStartColumn()
-    )
+pragma[inline]
+string preferredFile(File callFile, Function base, Function sub) {
+  if callFile = getFunctionFile(base)
+  then result = " A"
+  else
+    if callFile = getFunctionFile(sub)
+    then result = " B"
+    else result = callFile.getAbsolutePath()
 }
 
 /** Choose a 'witnessing' call that matches the signature of `base` but not of overridden `sub`. */
 Call chooseASignatureMismatchWitness(Function base, Function sub) {
   exists(getASignatureMismatchWitness(base, sub)) and
-  (
-    result = chooseASignatureMismatchWitnessInFile(base, sub, getFunctionFile(base))
-    or
-    not exists(Call c |
-      c = getASignatureMismatchWitness(base, sub) and
-      c.getLocation().getFile() = base.getLocation().getFile()
-    ) and
-    result = chooseASignatureMismatchWitnessInFile(base, sub, getFunctionFile(sub))
-    or
-    not exists(Call c |
-      c = getASignatureMismatchWitness(base, sub) and
-      c.getLocation().getFile() = getFunctionFile([base, sub])
-    ) and
-    result =
-      min(Call c |
-        c = getASignatureMismatchWitness(base, sub)
-      |
-        c
-        order by
-          c.getLocation().getFile().getAbsolutePath(), c.getLocation().getStartLine(),
-          c.getLocation().getStartColumn()
-      )
-  )
+  result =
+    min(Call c |
+      c = getASignatureMismatchWitness(base, sub)
+    |
+      c
+      order by
+        preferredFile(c.getLocation().getFile(), base, sub), c.getLocation().getStartLine(),
+        c.getLocation().getStartColumn()
+    )
 }
 
 module CallOption = LocatableOption<Location, Call>;