mirror of
https://github.com/github/codeql.git
synced 2026-04-26 09:15:12 +02:00
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 ... ```
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 |
|
||||
|
||||
@@ -69,11 +69,11 @@ class WrongSpecials(object):
|
||||
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 @@ class LoggingDict(dict):
|
||||
|
||||
|
||||
|
||||
class MoreSpecialMethods:
|
||||
@staticmethod
|
||||
def __abs__():
|
||||
return 42
|
||||
|
||||
Reference in New Issue
Block a user