/** * @name Signature mismatch in overriding method * @description Overriding a method without ensuring that both methods accept the same * number and type of parameters has the potential to cause an error when there is a mismatch. * @kind problem * @problem.severity warning * @tags quality * reliability * correctness * @sub-severity high * @precision very-high * @id py/inheritance/signature-mismatch */ 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 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. */ 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." or sub.getMaxPositionalArguments() < base.getMinPositionalArguments() and msg = "requires " + plural(base.getMinPositionalArguments() - sub.getMaxPositionalArguments(), "fewer positional argument") + " than overridden $@ allows." ) } /** 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 " + plural(sub.getMinPositionalArguments() - base.getMinPositionalArguments(), "more positional argument") + " than some possible calls to overridden $@." or sub.getMaxPositionalArguments() < base.getMaxPositionalArguments() and 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." ) 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 ) } 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() ) ) } module CallOption = LocOption2; 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 ( 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"