Skip to content

Commit 4fc7d84

Browse files
Revert more precise version of call matches signature predicate for performance.
1 parent 71dec0b commit 4fc7d84

File tree

3 files changed

+8
-20
lines changed

3 files changed

+8
-20
lines changed

python/ql/src/Functions/SignatureOverriddenMethod.ql

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -171,24 +171,14 @@ int extraSelfArg(Function func) { if isStaticmethod(func) then result = 0 else r
171171

172172
/** Holds if the call `call` matches the signature for `func`. */
173173
predicate callMatchesSignature(Function func, Call call) {
174-
func = resolveCall(call) and
174+
// TODO: This is not fully precise.
175+
// For example, it does not detect that a method `def foo(self,x,y)` is matched by a call `obj.foo(1,y=2)`
176+
// since y is passed in the call as a keyword argument, but still counts toward a positional argument of the method.
177+
// A more precise version was attempted, but reverted due to performance concerns.
175178
(
176-
// Each parameter of the function is accounted for in the call
177-
forall(Parameter param, int i | param = func.getArg(i) |
178-
// self arg
179-
i = 0 and not isStaticmethod(func)
180-
or
181-
// positional arg
182-
i - extraSelfArg(func) < call.getPositionalArgumentCount()
183-
or
184-
// has default
185-
exists(param.getDefault())
186-
or
187-
// keyword arg
188-
call.getANamedArgumentName() = param.getName()
189-
)
179+
// Enough parameters
180+
call.getPositionalArgumentCount() + extraSelfArg(func) >= func.getMinPositionalArguments()
190181
or
191-
// arbitrary varargs or kwargs
192182
exists(call.getStarArg())
193183
or
194184
exists(call.getKwargs())

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,4 @@
88
| 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 |
99
| 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 |
1010
| 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 |
11-
| 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 |
1211
| 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 |
13-
| 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 |

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ def meth6(self, x): pass # $Alert[py/inheritance/signature-mismatch] # weak mism
136136

137137
def meth7(self, x, *ys): pass # $Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with 1 arg only)
138138

139-
def meth8(self, x, z): pass # $Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with arg named y)
139+
def meth8(self, x, z): pass # $MISSING:Alert[py/inheritance/signature-mismatch] # weak mismatch (base may be called with arg named y). The witnessing call to meth8 is not detected as matching the signature of Base2.meth8.
140140

141141
def meth9(self, x, z): pass # No alert (never called with wrong keyword arg)
142142

@@ -146,4 +146,4 @@ def meth11(self, x, z, **kwargs): pass # $MISSING:Alert[py/inheritance/signature
146146

147147
def meth12(self): pass # $Alert[py/inheritance/signature-mismatch] # call including extra kwarg invalid
148148

149-
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.
149+
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.

0 commit comments

Comments
 (0)