hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #18956 from github/tausbn/python-more-special-method-query-refactoring

Browse Source 
Python: Modernize special method query
This commit is contained in:
Taus
2025-03-28 17:11:24 +01:00
committed by GitHub
parent 342d4a6982 074af6f548
commit 840abbf5b1
4 changed files with 154 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

200
python/ql/src/Functions/SignatureSpecialMethods.ql
View File

@@ -4,6 +4,7 @@
* @kind problem * @kind problem
* @tags reliability * @tags reliability
* correctness * correctness
* quality
* @problem.severity error * @problem.severity error
* @sub-severity low * @sub-severity low
* @precision high * @precision high
@@ -11,12 +12,15 @@
*/ */
import python import python
import semmle.python.dataflow.new.internal.DataFlowDispatch as DD
predicate is_unary_op(string name) { predicate is_unary_op(string name) {
name in [ name in [
"__del__", "__repr__", "__neg__", "__pos__", "__abs__", "__invert__", "__complex__", "__del__", "__repr__", "__neg__", "__pos__", "__abs__", "__invert__", "__complex__",
"__int__", "__float__", "__long__", "__oct__", "__hex__", "__str__", "__index__", "__enter__", "__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,91 +32,138 @@ predicate is_binary_op(string name) {
"__and__", "__xor__", "__or__", "__ne__", "__radd__", "__rsub__", "__rmul__", "__rfloordiv__", "__and__", "__xor__", "__or__", "__ne__", "__radd__", "__rsub__", "__rmul__", "__rfloordiv__",
"__rdiv__", "__rtruediv__", "__rmod__", "__rdivmod__", "__rpow__", "__rlshift__", "__gt__", "__rdiv__", "__rtruediv__", "__rmod__", "__rdivmod__", "__rpow__", "__rlshift__", "__gt__",
"__rrshift__", "__rand__", "__rxor__", "__ror__", "__iadd__", "__isub__", "__imul__", "__rrshift__", "__rand__", "__rxor__", "__ror__", "__iadd__", "__isub__", "__imul__",
"__ifloordiv__", "__idiv__", "__itruediv__", "__ge__", "__imod__", "__idivmod__", "__ipow__", "__ifloordiv__", "__idiv__", "__itruediv__", "__ge__", "__imod__", "__ipow__", "__ilshift__",
"__ilshift__", "__irshift__", "__iand__", "__ixor__", "__ior__", "__coerce__", "__cmp__", "__irshift__", "__iand__", "__ixor__", "__ior__", "__coerce__", "__cmp__", "__rcmp__",
"__rcmp__", "__getattr___", "__getattribute___" "__getattr__", "__getattribute__", "__buffer__", "__release_buffer__", "__matmul__",
"__rmatmul__", "__imatmul__", "__missing__", "__class_getitem__", "__mro_entries__",
"__format__"
] ]
} }
predicate is_ternary_op(string name) { 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(PythonFunctionValue f, string name, ClassValue cls) { int argument_count(string name) {
cls.declaredAttribute(name) = f and is_unary_op(name) and result = 1
( or
is_unary_op(name) and result = 1 is_binary_op(name) and result = 2
or or
is_binary_op(name) and result = 2 is_ternary_op(name) and result = 3
or or
is_ternary_op(name) and result = 3 is_quad_op(name) and result = 4
or }
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( 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, boolean is_unused_default
) { ) {
exists(int required | required = argument_count(func, name, owner) | exists(int required, int correction |
required = argument_count(name) - correction and correction = staticmethod_correction(func)
|
/* actual_non_default <= actual */ /* actual_non_default <= actual */
if required > func.maxParameters() 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 else
if required < func.minParameters() 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 ( else (
func.minParameters() < required and func.getMinPositionalArguments() < required and
not func.getScope().hasVarArg() and not func.hasVarArg() and
message = (required - func.minParameters()) + " default values(s) will never be used" and message =
show_counts = false (required - func.getMinPositionalArguments()) + " default values(s) will never be used" and
show_counts = false and
is_unused_default = true
) )
) )
} }
predicate incorrect_pow(FunctionValue func, string message, boolean show_counts, ClassValue owner) { predicate incorrect_pow(
owner.declaredAttribute("__pow__") = func and Function func, string message, boolean show_counts, boolean is_unused_default
( ) {
func.maxParameters() < 2 and message = "Too few parameters" and show_counts = true 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 or
func.minParameters() > 3 and message = "Too many parameters" and show_counts = true func.getMinPositionalArguments() > 3 - correction and
message = "Too many parameters" and
show_counts = true and
is_unused_default = false
or or
func.minParameters() < 2 and func.getMinPositionalArguments() < 2 - correction and
message = (2 - func.minParameters()) + " default value(s) will never be used" 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 or
func.minParameters() = 3 and func.getMinPositionalArguments() = 3 - correction and
message = "Third parameter to __pow__ should have a default value" 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(FunctionValue func, string message, boolean show_counts, ClassValue owner) { predicate incorrect_round(
owner.declaredAttribute("__get__") = func and Function func, string message, boolean show_counts, boolean is_unused_default
( ) {
func.maxParameters() < 3 and message = "Too few parameters" and show_counts = true 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 or
func.minParameters() > 3 and message = "Too many parameters" and show_counts = true func.getMinPositionalArguments() > 2 - correction and
message = "Too many parameters" and
show_counts = true and
is_unused_default = false
or or
func.minParameters() < 2 and func.getMinPositionalArguments() = 2 - correction and
not func.getScope().hasVarArg() and message = "Second parameter to __round__ should have a default value" and
message = (2 - func.minParameters()) + " default value(s) will never be used" and show_counts = false and
show_counts = false is_unused_default = false
) )
} }
string should_have_parameters(PythonFunctionValue f, string name, ClassValue owner) { predicate incorrect_get(
exists(int i | i = argument_count(f, name, owner) | result = i.toString()) Function func, string message, boolean show_counts, boolean is_unused_default
or ) {
owner.declaredAttribute(name) = f and exists(int correction | correction = staticmethod_correction(func) |
(name = "__get__" or name = "__pow__") and func.getMaxPositionalArguments() < 3 - correction and
result = "2 or 3" message = "Too few parameters" and
show_counts = true and
is_unused_default = false
or
func.getMinPositionalArguments() > 3 - correction and
message = "Too many parameters" and
show_counts = true and
is_unused_default = false
or
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
is_unused_default = true
)
} }
string has_parameters(PythonFunctionValue f) { string should_have_parameters(string name) {
exists(int i | i = f.minParameters() | if name in ["__pow__", "__get__"]
then result = "2 or 3"
else result = argument_count(name).toString()
}
string has_parameters(Function f) {
exists(int i | i = f.getMinPositionalArguments() |
i = 0 and result = "no parameters" i = 0 and result = "no parameters"
or or
i = 1 and result = "1 parameter" i = 1 and result = "1 parameter"
@@ -121,23 +172,44 @@ string has_parameters(PythonFunctionValue f) {
) )
} }
from /** Holds if `f` is likely to be a placeholder, and hence not interesting enough to report. */
PythonFunctionValue f, string message, string sizes, boolean show_counts, string name, predicate isLikelyPlaceholderFunction(Function f) {
ClassValue owner // Body has only a single statement.
where f.getBody().getItem(0) = f.getBody().getLastItem() and
( (
incorrect_special_method_defn(f, message, show_counts, name, owner) // Body is a string literal. This is a common pattern for Zope interfaces.
f.getBody().getLastItem().(ExprStmt).getValue() instanceof StringLiteral
or or
incorrect_pow(f, message, show_counts, owner) and name = "__pow__" // Body just raises an exception.
f.getBody().getLastItem() instanceof Raise
or or
incorrect_get(f, message, show_counts, owner) and name = "__get__" // Body is a pass statement.
f.getBody().getLastItem() instanceof Pass
)
}
from
Function f, string message, string sizes, boolean show_counts, string name, Class owner,
boolean show_unused_defaults
where
owner.getAMethod() = f and
f.getName() = name and
(
incorrect_special_method_defn(f, message, show_counts, name, show_unused_defaults)
or
incorrect_pow(f, message, show_counts, show_unused_defaults) and name = "__pow__"
or
incorrect_get(f, message, show_counts, show_unused_defaults) and name = "__get__"
or
incorrect_round(f, message, show_counts, show_unused_defaults) and
name = "__round__"
) and ) and
not isLikelyPlaceholderFunction(f) and
show_unused_defaults = false and
( (
show_counts = false and sizes = "" show_counts = false and sizes = ""
or or
show_counts = true and show_counts = true and
sizes = sizes = ", which has " + has_parameters(f) + ", but should have " + should_have_parameters(name)
", which has " + has_parameters(f) + ", but should have " +
should_have_parameters(f, name, owner)
) )
select f, message + " for special method " + name + sizes + ", in class $@.", owner, owner.getName() select f, message + " for special method " + name + sizes + ", in class $@.", owner, owner.getName()

5
python/ql/src/change-notes/2025-03-20-modernize-special-method-wrong-signature-query.md Normal file
View File

@@ -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.

15
python/ql/test/query-tests/Functions/general/SignatureSpecialMethods.expected
View File

@@ -1,9 +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: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 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: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 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: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 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: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 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: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: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: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 |
| 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 |

10
python/ql/test/query-tests/Functions/general/om_test.py
View File

@@ -69,11 +69,11 @@ class WrongSpecials(object):
return arg0 == arg1 return arg0 == arg1
def __repr__(): def __repr__():
pass return ""
def __add__(self, other="Unused default"): def __add__(self, other="Unused default"):
pass return 4
@staticmethod @staticmethod
def __abs__(): def __abs__():
return 42 return 42
@@ -105,3 +105,7 @@ class LoggingDict(dict):
class MoreSpecialMethods:
@staticmethod
def __abs__():
return 42