From 24b2eb24c19b259acd201c9e16235c36101bbee5 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 7 Mar 2025 17:23:38 +0000 Subject: [PATCH 1/7] Python: Refactor special method query Moves a bunch of `owner.declaredAttribute(name) = f` instances to the top level, in the process greatly cleaning up the code. The behaviour should be the unchanged. Having done this, there's only one place where we depend on points-to, and that's in the remaining `declaredAttribute` call. This should greatly simplify the move away from points to. --- .../src/Functions/SignatureSpecialMethods.ql | 85 +++++++++---------- 1 file changed, 40 insertions(+), 45 deletions(-) diff --git a/python/ql/src/Functions/SignatureSpecialMethods.ql b/python/ql/src/Functions/SignatureSpecialMethods.ql index feedb6c94b6d..9a1d192611db 100644 --- a/python/ql/src/Functions/SignatureSpecialMethods.ql +++ b/python/ql/src/Functions/SignatureSpecialMethods.ql @@ -40,79 +40,73 @@ predicate is_ternary_op(string name) { predicate is_quad_op(string name) { name = "__setslice__" or name = "__exit__" } -int argument_count(PythonFunctionValue f, string name, ClassValue cls) { - cls.declaredAttribute(name) = f and - ( - is_unary_op(name) and result = 1 - or - is_binary_op(name) and result = 2 - or - is_ternary_op(name) and result = 3 - or - is_quad_op(name) and result = 4 - ) +int argument_count(string name) { + is_unary_op(name) and result = 1 + or + is_binary_op(name) and result = 2 + or + is_ternary_op(name) and result = 3 + or + is_quad_op(name) and result = 4 } predicate incorrect_special_method_defn( - PythonFunctionValue func, string message, boolean show_counts, string name, ClassValue owner + Function func, string message, boolean show_counts, string name ) { - exists(int required | required = argument_count(func, name, owner) | + exists(int required | required = argument_count(name) | /* actual_non_default <= actual */ - if required > func.maxParameters() + if required > func.getMaxPositionalArguments() then message = "Too few parameters" and show_counts = true else - if required < func.minParameters() + if required < func.getMinPositionalArguments() then message = "Too many parameters" and show_counts = true else ( - func.minParameters() < required and - not func.getScope().hasVarArg() and - message = (required - func.minParameters()) + " default values(s) will never be used" and + func.getMinPositionalArguments() < required and + not func.hasVarArg() and + message = + (required - func.getMinPositionalArguments()) + " default values(s) will never be used" and show_counts = false ) ) } -predicate incorrect_pow(FunctionValue func, string message, boolean show_counts, ClassValue owner) { - owner.declaredAttribute("__pow__") = func and +predicate incorrect_pow(Function func, string message, boolean show_counts) { ( - func.maxParameters() < 2 and message = "Too few parameters" and show_counts = true + func.getMaxPositionalArguments() < 2 and message = "Too few parameters" and show_counts = true or - func.minParameters() > 3 and message = "Too many parameters" and show_counts = true + func.getMinPositionalArguments() > 3 and message = "Too many parameters" and show_counts = true or - func.minParameters() < 2 and - message = (2 - func.minParameters()) + " default value(s) will never be used" and + func.getMinPositionalArguments() < 2 and + message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and show_counts = false or - func.minParameters() = 3 and + func.getMinPositionalArguments() = 3 and message = "Third parameter to __pow__ should have a default value" and show_counts = false ) } -predicate incorrect_get(FunctionValue func, string message, boolean show_counts, ClassValue owner) { - owner.declaredAttribute("__get__") = func and +predicate incorrect_get(Function func, string message, boolean show_counts) { ( - func.maxParameters() < 3 and message = "Too few parameters" and show_counts = true + func.getMaxPositionalArguments() < 3 and message = "Too few parameters" and show_counts = true or - func.minParameters() > 3 and message = "Too many parameters" and show_counts = true + func.getMinPositionalArguments() > 3 and message = "Too many parameters" and show_counts = true or - func.minParameters() < 2 and - not func.getScope().hasVarArg() and - message = (2 - func.minParameters()) + " default value(s) will never be used" and + func.getMinPositionalArguments() < 2 and + not func.hasVarArg() and + message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and show_counts = false ) } -string should_have_parameters(PythonFunctionValue f, string name, ClassValue owner) { - exists(int i | i = argument_count(f, name, owner) | result = i.toString()) - or - owner.declaredAttribute(name) = f and - (name = "__get__" or name = "__pow__") and - result = "2 or 3" +string should_have_parameters(string name) { + if name in ["__pow__", "__get__"] + then result = "2 or 3" + else result = argument_count(name).toString() } -string has_parameters(PythonFunctionValue f) { - exists(int i | i = f.minParameters() | +string has_parameters(Function f) { + exists(int i | i = f.getMinPositionalArguments() | i = 0 and result = "no parameters" or i = 1 and result = "1 parameter" @@ -125,19 +119,20 @@ from PythonFunctionValue f, string message, string sizes, boolean show_counts, string name, ClassValue owner where + owner.declaredAttribute(name) = f and ( - incorrect_special_method_defn(f, message, show_counts, name, owner) + incorrect_special_method_defn(f.getScope(), message, show_counts, name) or - incorrect_pow(f, message, show_counts, owner) and name = "__pow__" + incorrect_pow(f.getScope(), message, show_counts) and name = "__pow__" or - incorrect_get(f, message, show_counts, owner) and name = "__get__" + incorrect_get(f.getScope(), message, show_counts) and name = "__get__" ) and ( show_counts = false and sizes = "" or show_counts = true and sizes = - ", which has " + has_parameters(f) + ", but should have " + - should_have_parameters(f, name, owner) + ", which has " + has_parameters(f.getScope()) + ", but should have " + + should_have_parameters(name) ) select f, message + " for special method " + name + sizes + ", in class $@.", owner, owner.getName() From 862b89207df9cac9e2365ae599464d510d4413ed Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 12 Mar 2025 16:41:29 +0000 Subject: [PATCH 2/7] Python: Disable "usused default" logic Adds a new boolean parameter `is_unused_default` that indicates whether the given result is one where a parameter to a special method has a default value (which will never be used when invoked in the normal way). These results are somewhat less useful (because the special method _might_ be invoked directly, in which case the default value would still be relevant), but it seemed like a shame to simply remove the code, so instead I opted to disable it in this way. --- .../src/Functions/SignatureSpecialMethods.ql | 56 +++++++++++++------ 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/python/ql/src/Functions/SignatureSpecialMethods.ql b/python/ql/src/Functions/SignatureSpecialMethods.ql index 9a1d192611db..97c41f4a873e 100644 --- a/python/ql/src/Functions/SignatureSpecialMethods.ql +++ b/python/ql/src/Functions/SignatureSpecialMethods.ql @@ -51,51 +51,71 @@ int argument_count(string name) { } predicate incorrect_special_method_defn( - Function func, string message, boolean show_counts, string name + Function func, string message, boolean show_counts, string name, boolean is_unused_default ) { exists(int required | required = argument_count(name) | /* actual_non_default <= actual */ if required > func.getMaxPositionalArguments() - then message = "Too few parameters" and show_counts = true + then message = "Too few parameters" and show_counts = true and is_unused_default = false else if required < func.getMinPositionalArguments() - then message = "Too many parameters" and show_counts = true + then message = "Too many parameters" and show_counts = true and is_unused_default = false else ( func.getMinPositionalArguments() < required and not func.hasVarArg() and message = (required - func.getMinPositionalArguments()) + " default values(s) will never be used" and - show_counts = false + show_counts = false and + is_unused_default = true ) ) } -predicate incorrect_pow(Function func, string message, boolean show_counts) { +predicate incorrect_pow( + Function func, string message, boolean show_counts, boolean is_unused_default +) { ( - func.getMaxPositionalArguments() < 2 and message = "Too few parameters" and show_counts = true + func.getMaxPositionalArguments() < 2 and + message = "Too few parameters" and + show_counts = true and + is_unused_default = false or - func.getMinPositionalArguments() > 3 and message = "Too many parameters" and show_counts = true + func.getMinPositionalArguments() > 3 and + message = "Too many parameters" and + show_counts = true and + is_unused_default = false or func.getMinPositionalArguments() < 2 and message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and - show_counts = false + show_counts = false and + is_unused_default = true or func.getMinPositionalArguments() = 3 and message = "Third parameter to __pow__ should have a default value" and - show_counts = false + show_counts = false and + is_unused_default = false ) } -predicate incorrect_get(Function func, string message, boolean show_counts) { +predicate incorrect_get( + Function func, string message, boolean show_counts, boolean is_unused_default +) { ( - func.getMaxPositionalArguments() < 3 and message = "Too few parameters" and show_counts = true + func.getMaxPositionalArguments() < 3 and + message = "Too few parameters" and + show_counts = true and + is_unused_default = false or - func.getMinPositionalArguments() > 3 and message = "Too many parameters" and show_counts = true + func.getMinPositionalArguments() > 3 and + message = "Too many parameters" and + show_counts = true and + is_unused_default = false or func.getMinPositionalArguments() < 2 and not func.hasVarArg() and message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and - show_counts = false + show_counts = false and + is_unused_default = true ) } @@ -117,16 +137,18 @@ string has_parameters(Function f) { from PythonFunctionValue f, string message, string sizes, boolean show_counts, string name, - ClassValue owner + ClassValue owner, boolean show_unused_defaults where owner.declaredAttribute(name) = f and ( - incorrect_special_method_defn(f.getScope(), message, show_counts, name) + incorrect_special_method_defn(f.getScope(), message, show_counts, name, show_unused_defaults) + or + incorrect_pow(f.getScope(), message, show_counts, show_unused_defaults) and name = "__pow__" or - incorrect_pow(f.getScope(), message, show_counts) and name = "__pow__" + incorrect_get(f.getScope(), message, show_counts, show_unused_defaults) and name = "__get__" or - incorrect_get(f.getScope(), message, show_counts) and name = "__get__" ) and + show_unused_defaults = false and ( show_counts = false and sizes = "" or From f3353dc3fbbe938a84e592ac2aaedc2a7ae06c94 Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 12 Mar 2025 16:43:53 +0000 Subject: [PATCH 3/7] Python: Ignore special methods with placeholder bodies Instances of this include - Bodies that contain just a docstring (common in Zope interfaces) - Bodies that do nothing but raise an exception. --- python/ql/src/Functions/SignatureSpecialMethods.ql | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/python/ql/src/Functions/SignatureSpecialMethods.ql b/python/ql/src/Functions/SignatureSpecialMethods.ql index 97c41f4a873e..500a384180ae 100644 --- a/python/ql/src/Functions/SignatureSpecialMethods.ql +++ b/python/ql/src/Functions/SignatureSpecialMethods.ql @@ -135,6 +135,19 @@ string has_parameters(Function f) { ) } +/** Holds if `f` is likely to be a placeholder, and hence not interesting enough to report. */ +predicate isLikelyPlaceholderFunction(Function f) { + // Body has only a single statement. + f.getBody().getItem(0) = f.getBody().getLastItem() and + ( + // Body is a string literal. This is a common pattern for Zope interfaces. + f.getBody().getLastItem().(ExprStmt).getValue() instanceof StringLiteral + or + // Body just raises an exception. + f.getBody().getLastItem() instanceof Raise + ) +} + from PythonFunctionValue f, string message, string sizes, boolean show_counts, string name, ClassValue owner, boolean show_unused_defaults @@ -148,6 +161,7 @@ where incorrect_get(f.getScope(), message, show_counts, show_unused_defaults) and name = "__get__" or ) and + not isLikelyPlaceholderFunction(f.getScope()) and show_unused_defaults = false and ( show_counts = false and sizes = "" From bf688b88a9152f5c3f1248b5887ee935e462e832 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 14 Mar 2025 16:29:54 +0000 Subject: [PATCH 4/7] Python: Add missing special methods --- .../src/Functions/SignatureSpecialMethods.ql | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/python/ql/src/Functions/SignatureSpecialMethods.ql b/python/ql/src/Functions/SignatureSpecialMethods.ql index 500a384180ae..b606b3668689 100644 --- a/python/ql/src/Functions/SignatureSpecialMethods.ql +++ b/python/ql/src/Functions/SignatureSpecialMethods.ql @@ -16,7 +16,9 @@ predicate is_unary_op(string name) { name in [ "__del__", "__repr__", "__neg__", "__pos__", "__abs__", "__invert__", "__complex__", "__int__", "__float__", "__long__", "__oct__", "__hex__", "__str__", "__index__", "__enter__", - "__hash__", "__bool__", "__nonzero__", "__unicode__", "__len__", "__iter__", "__reversed__" + "__hash__", "__bool__", "__nonzero__", "__unicode__", "__len__", "__iter__", "__reversed__", + "__aenter__", "__aiter__", "__anext__", "__await__", "__ceil__", "__floor__", "__trunc__", + "__length_hint__", "__dir__", "__bytes__" ] } @@ -28,17 +30,19 @@ predicate is_binary_op(string name) { "__and__", "__xor__", "__or__", "__ne__", "__radd__", "__rsub__", "__rmul__", "__rfloordiv__", "__rdiv__", "__rtruediv__", "__rmod__", "__rdivmod__", "__rpow__", "__rlshift__", "__gt__", "__rrshift__", "__rand__", "__rxor__", "__ror__", "__iadd__", "__isub__", "__imul__", - "__ifloordiv__", "__idiv__", "__itruediv__", "__ge__", "__imod__", "__idivmod__", "__ipow__", - "__ilshift__", "__irshift__", "__iand__", "__ixor__", "__ior__", "__coerce__", "__cmp__", - "__rcmp__", "__getattr___", "__getattribute___" + "__ifloordiv__", "__idiv__", "__itruediv__", "__ge__", "__imod__", "__ipow__", "__ilshift__", + "__irshift__", "__iand__", "__ixor__", "__ior__", "__coerce__", "__cmp__", "__rcmp__", + "__getattr__", "__getattribute__", "__buffer__", "__release_buffer__", "__matmul__", + "__rmatmul__", "__imatmul__", "__missing__", "__class_getitem__", "__mro_entries__", + "__format__" ] } predicate is_ternary_op(string name) { - name in ["__setattr__", "__set__", "__setitem__", "__getslice__", "__delslice__"] + name in ["__setattr__", "__set__", "__setitem__", "__getslice__", "__delslice__", "__set_name__"] } -predicate is_quad_op(string name) { name = "__setslice__" or name = "__exit__" } +predicate is_quad_op(string name) { name in ["__setslice__", "__exit__", "__aexit__"] } int argument_count(string name) { is_unary_op(name) and result = 1 @@ -97,6 +101,27 @@ predicate incorrect_pow( ) } +predicate incorrect_round( + Function func, string message, boolean show_counts, boolean is_unused_default +) { + exists(int correction | correction = staticmethod_correction(func) | + func.getMaxPositionalArguments() < 1 - correction and + message = "Too few parameters" and + show_counts = true and + is_unused_default = false + or + func.getMinPositionalArguments() > 2 - correction and + message = "Too many parameters" and + show_counts = true and + is_unused_default = false + or + func.getMinPositionalArguments() = 2 - correction and + message = "Second parameter to __round__ should have a default value" and + show_counts = false and + is_unused_default = false + ) +} + predicate incorrect_get( Function func, string message, boolean show_counts, boolean is_unused_default ) { @@ -160,6 +185,8 @@ where or incorrect_get(f.getScope(), message, show_counts, show_unused_defaults) and name = "__get__" or + incorrect_round(f.getScope(), message, show_counts, show_unused_defaults) and + name = "__round__" ) and not isLikelyPlaceholderFunction(f.getScope()) and show_unused_defaults = false and From c9e9deb41e08f1fb57ead62e36ae7135138b96e6 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 14 Mar 2025 16:49:33 +0000 Subject: [PATCH 5/7] Python: Adapt to a points-to-less world Technically we still depend on points-to in that we still mention `PythonFunctionValue` and `ClassValue` in the query. However, we immediately move to working with the corresponding `Function` and `Class` AST nodes, and so we're not really using points-to. (The reason for doing things this way is that otherwise the `.toString()` for all of the alerts would change, which would make the diff hard to interpret. This way, it should be fairly simple to see which changes are actually relevant.) We do lose some precision when moving away from points-to, and this is reflected in the changes in the `.expected` file. In particular we no longer do complicated tracking of values, but rather look at the syntactic structure of the classes in question. This causes us to lose out on some results where a special method is defined elsewhere, and causes a single FP where a special method initially has the wrong signature, but is subsequently overwritten with a function with the correct signature. We also lose out on results having to do with default values, as these are now disabled. Finally, it was necessary to add special handling of methods marked with the `staticmethod` decorator, as these expect to receive fewer arguments. This was motivated by a MRVA run, where e.g. sympy showed a lot of examples along the lines of ``` @staticmethod def __abs__(): return ... ``` --- .../src/Functions/SignatureSpecialMethods.ql | 37 +++++++++++++------ .../general/SignatureSpecialMethods.expected | 5 +-- .../query-tests/Functions/general/om_test.py | 10 +++-- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/python/ql/src/Functions/SignatureSpecialMethods.ql b/python/ql/src/Functions/SignatureSpecialMethods.ql index b606b3668689..6c063b3ff9be 100644 --- a/python/ql/src/Functions/SignatureSpecialMethods.ql +++ b/python/ql/src/Functions/SignatureSpecialMethods.ql @@ -11,6 +11,7 @@ */ import python +import semmle.python.dataflow.new.internal.DataFlowDispatch as DD predicate is_unary_op(string name) { name in [ @@ -54,10 +55,20 @@ int argument_count(string name) { is_quad_op(name) and result = 4 } +/** + * Returns 1 if `func` is a static method, and 0 otherwise. This predicate is used to adjust the + * number of expected arguments for a special method accordingly. + */ +int staticmethod_correction(Function func) { + if DD::isStaticmethod(func) then result = 1 else result = 0 +} + predicate incorrect_special_method_defn( Function func, string message, boolean show_counts, string name, boolean is_unused_default ) { - exists(int required | required = argument_count(name) | + exists(int required, int correction | + required = argument_count(name) - correction and correction = staticmethod_correction(func) + | /* actual_non_default <= actual */ if required > func.getMaxPositionalArguments() then message = "Too few parameters" and show_counts = true and is_unused_default = false @@ -78,23 +89,23 @@ predicate incorrect_special_method_defn( predicate incorrect_pow( Function func, string message, boolean show_counts, boolean is_unused_default ) { - ( - func.getMaxPositionalArguments() < 2 and + exists(int correction | correction = staticmethod_correction(func) | + func.getMaxPositionalArguments() < 2 - correction and message = "Too few parameters" and show_counts = true and is_unused_default = false or - func.getMinPositionalArguments() > 3 and + func.getMinPositionalArguments() > 3 - correction and message = "Too many parameters" and show_counts = true and is_unused_default = false or - func.getMinPositionalArguments() < 2 and + func.getMinPositionalArguments() < 2 - correction and message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and show_counts = false and is_unused_default = true or - func.getMinPositionalArguments() = 3 and + func.getMinPositionalArguments() = 3 - correction and message = "Third parameter to __pow__ should have a default value" and show_counts = false and is_unused_default = false @@ -125,18 +136,18 @@ predicate incorrect_round( predicate incorrect_get( Function func, string message, boolean show_counts, boolean is_unused_default ) { - ( - func.getMaxPositionalArguments() < 3 and + exists(int correction | correction = staticmethod_correction(func) | + func.getMaxPositionalArguments() < 3 - correction and message = "Too few parameters" and show_counts = true and is_unused_default = false or - func.getMinPositionalArguments() > 3 and + func.getMinPositionalArguments() > 3 - correction and message = "Too many parameters" and show_counts = true and is_unused_default = false or - func.getMinPositionalArguments() < 2 and + func.getMinPositionalArguments() < 2 - correction and not func.hasVarArg() and message = (2 - func.getMinPositionalArguments()) + " default value(s) will never be used" and show_counts = false and @@ -170,6 +181,9 @@ predicate isLikelyPlaceholderFunction(Function f) { or // Body just raises an exception. f.getBody().getLastItem() instanceof Raise + or + // Body is a pass statement. + f.getBody().getLastItem() instanceof Pass ) } @@ -177,7 +191,8 @@ from PythonFunctionValue f, string message, string sizes, boolean show_counts, string name, ClassValue owner, boolean show_unused_defaults where - owner.declaredAttribute(name) = f and + owner.getScope().getAMethod() = f.getScope() and + f.getScope().getName() = name and ( incorrect_special_method_defn(f.getScope(), message, show_counts, name, show_unused_defaults) or diff --git a/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected b/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected index 94fba173b3b6..5f8cb49c10e9 100644 --- a/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected +++ b/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected @@ -3,7 +3,4 @@ | om_test.py:65:5:65:29 | Function WrongSpecials.__neg__ | Too many parameters for special method __neg__, which has 2 parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | | om_test.py:68:5:68:35 | Function WrongSpecials.__exit__ | Too few parameters for special method __exit__, which has 3 parameters, but should have 4, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | | om_test.py:71:5:71:19 | Function WrongSpecials.__repr__ | Too few parameters for special method __repr__, which has no parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:74:5:74:46 | Function WrongSpecials.__add__ | 1 default values(s) will never be used for special method __add__, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:97:15:97:34 | Function NotOKSpecials.lambda | Too few parameters for special method __sub__, which has 1 parameter, but should have 2, in class $@. | om_test.py:95:1:95:28 | class NotOKSpecials | NotOKSpecials | -| protocols.py:107:1:107:12 | Function f | Too few parameters for special method __add__, which has 1 parameter, but should have 2, in class $@. | protocols.py:110:1:110:29 | class MissingMethods | MissingMethods | -| protocols.py:107:1:107:12 | Function f | Too few parameters for special method __set__, which has 1 parameter, but should have 3, in class $@. | protocols.py:110:1:110:29 | class MissingMethods | MissingMethods | +| om_test.py:83:5:83:18 | Function OKSpecials.__del__ | Too few parameters for special method __del__, which has no parameters, but should have 1, in class $@. | om_test.py:81:1:81:25 | class OKSpecials | OKSpecials | diff --git a/python/ql/test/query-tests/Functions/general/om_test.py b/python/ql/test/query-tests/Functions/general/om_test.py index cbee20625aa8..959ed6bfe348 100644 --- a/python/ql/test/query-tests/Functions/general/om_test.py +++ b/python/ql/test/query-tests/Functions/general/om_test.py @@ -69,11 +69,11 @@ def __exit__(self, arg0, arg1): return arg0 == arg1 def __repr__(): - pass + return "" def __add__(self, other="Unused default"): - pass - + return 4 + @staticmethod def __abs__(): return 42 @@ -105,3 +105,7 @@ def pop(self): +class MoreSpecialMethods: + @staticmethod + def __abs__(): + return 42 From ef9b229023e78fccc259f7a07b35adf0472a27c1 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 14 Mar 2025 16:51:42 +0000 Subject: [PATCH 6/7] Python: Actually get rid of points-to Also adds `quality` to the list of tags for the query. --- .../src/Functions/SignatureSpecialMethods.ql | 23 +++++++++---------- .../general/SignatureSpecialMethods.expected | 12 +++++----- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/python/ql/src/Functions/SignatureSpecialMethods.ql b/python/ql/src/Functions/SignatureSpecialMethods.ql index 6c063b3ff9be..caa6f8c1614b 100644 --- a/python/ql/src/Functions/SignatureSpecialMethods.ql +++ b/python/ql/src/Functions/SignatureSpecialMethods.ql @@ -4,6 +4,7 @@ * @kind problem * @tags reliability * correctness + * quality * @problem.severity error * @sub-severity low * @precision high @@ -188,29 +189,27 @@ predicate isLikelyPlaceholderFunction(Function f) { } from - PythonFunctionValue f, string message, string sizes, boolean show_counts, string name, - ClassValue owner, boolean show_unused_defaults + Function f, string message, string sizes, boolean show_counts, string name, Class owner, + boolean show_unused_defaults where - owner.getScope().getAMethod() = f.getScope() and - f.getScope().getName() = name and + owner.getAMethod() = f and + f.getName() = name and ( - incorrect_special_method_defn(f.getScope(), message, show_counts, name, show_unused_defaults) + incorrect_special_method_defn(f, message, show_counts, name, show_unused_defaults) or - incorrect_pow(f.getScope(), message, show_counts, show_unused_defaults) and name = "__pow__" + incorrect_pow(f, message, show_counts, show_unused_defaults) and name = "__pow__" or - incorrect_get(f.getScope(), message, show_counts, show_unused_defaults) and name = "__get__" + incorrect_get(f, message, show_counts, show_unused_defaults) and name = "__get__" or - incorrect_round(f.getScope(), message, show_counts, show_unused_defaults) and + incorrect_round(f, message, show_counts, show_unused_defaults) and name = "__round__" ) and - not isLikelyPlaceholderFunction(f.getScope()) and + not isLikelyPlaceholderFunction(f) and show_unused_defaults = false and ( show_counts = false and sizes = "" or show_counts = true and - sizes = - ", which has " + has_parameters(f.getScope()) + ", but should have " + - should_have_parameters(name) + sizes = ", which has " + has_parameters(f) + ", but should have " + should_have_parameters(name) ) select f, message + " for special method " + name + sizes + ", in class $@.", owner, owner.getName() diff --git a/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected b/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected index 5f8cb49c10e9..55f1e7381f1b 100644 --- a/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected +++ b/python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected @@ -1,6 +1,6 @@ -| om_test.py:59:5:59:28 | Function WrongSpecials.__div__ | Too many parameters for special method __div__, which has 3 parameters, but should have 2, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:62:5:62:22 | Function WrongSpecials.__mul__ | Too few parameters for special method __mul__, which has 1 parameter, but should have 2, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:65:5:65:29 | Function WrongSpecials.__neg__ | Too many parameters for special method __neg__, which has 2 parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:68:5:68:35 | Function WrongSpecials.__exit__ | Too few parameters for special method __exit__, which has 3 parameters, but should have 4, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:71:5:71:19 | Function WrongSpecials.__repr__ | Too few parameters for special method __repr__, which has no parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | class WrongSpecials | WrongSpecials | -| om_test.py:83:5:83:18 | Function OKSpecials.__del__ | Too few parameters for special method __del__, which has no parameters, but should have 1, in class $@. | om_test.py:81:1:81:25 | class OKSpecials | OKSpecials | +| om_test.py:59:5:59:28 | Function __div__ | Too many parameters for special method __div__, which has 3 parameters, but should have 2, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials | +| om_test.py:62:5:62:22 | Function __mul__ | Too few parameters for special method __mul__, which has 1 parameter, but should have 2, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials | +| om_test.py:65:5:65:29 | Function __neg__ | Too many parameters for special method __neg__, which has 2 parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials | +| om_test.py:68:5:68:35 | Function __exit__ | Too few parameters for special method __exit__, which has 3 parameters, but should have 4, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials | +| om_test.py:71:5:71:19 | Function __repr__ | Too few parameters for special method __repr__, which has no parameters, but should have 1, in class $@. | om_test.py:57:1:57:28 | Class WrongSpecials | WrongSpecials | +| om_test.py:83:5:83:18 | Function __del__ | Too few parameters for special method __del__, which has no parameters, but should have 1, in class $@. | om_test.py:81:1:81:25 | Class OKSpecials | OKSpecials | From 074af6f5481557da8069e88c6d2cc7aed5590b36 Mon Sep 17 00:00:00 2001 From: Taus Date: Thu, 20 Mar 2025 13:57:32 +0000 Subject: [PATCH 7/7] Python: Add change note --- ...5-03-20-modernize-special-method-wrong-signature-query.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 python/ql/src/change-notes/2025-03-20-modernize-special-method-wrong-signature-query.md diff --git a/python/ql/src/change-notes/2025-03-20-modernize-special-method-wrong-signature-query.md b/python/ql/src/change-notes/2025-03-20-modernize-special-method-wrong-signature-query.md new file mode 100644 index 000000000000..e871b7510d9e --- /dev/null +++ b/python/ql/src/change-notes/2025-03-20-modernize-special-method-wrong-signature-query.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- + +- The `py/special-method-wrong-signature` has been modernized and rewritten to no longer rely on outdated APIs. Moreover, the query no longer flags cases where a default value is never used, as these alerts were rarely useful.