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()