Merge pull request #3362 from RasmusWL/python-keyword-only-args

Python: properly support keyword only arguments

Author: tausbn

python/ql/src/semmle/python/AstGenerated.qll

@@ -1310,13 +1310,13 @@ library class AliasList_ extends @py_alias_list {
 
 /** INTERNAL: See the class `Arguments` for further information. */
 library class Arguments_ extends @py_arguments {
-    /** Gets the keyword default values of this parameters definition. */
+    /** Gets the keyword-only default values of this parameters definition. */
     ExprList getKwDefaults() { py_expr_lists(result, this, 0) }
 
-    /** Gets the nth keyword default value of this parameters definition. */
+    /** Gets the nth keyword-only default value of this parameters definition. */
     Expr getKwDefault(int index) { result = this.getKwDefaults().getItem(index) }
 
-    /** Gets a keyword default value of this parameters definition. */
+    /** Gets a keyword-only default value of this parameters definition. */
     Expr getAKwDefault() { result = this.getKwDefaults().getAnItem() }
 
     /** Gets the default values of this parameters definition. */
@@ -1343,13 +1343,13 @@ library class Arguments_ extends @py_arguments {
     /** Gets the **kwarg annotation of this parameters definition. */
     Expr getKwargannotation() { py_exprs(result, _, this, 4) }
 
-    /** Gets the kw_annotations of this parameters definition. */
+    /** Gets the keyword-only annotations of this parameters definition. */
     ExprList getKwAnnotations() { py_expr_lists(result, this, 5) }
 
-    /** Gets the nth kw_annotation of this parameters definition. */
+    /** Gets the nth keyword-only annotation of this parameters definition. */
     Expr getKwAnnotation(int index) { result = this.getKwAnnotations().getItem(index) }
 
-    /** Gets a kw_annotation of this parameters definition. */
+    /** Gets a keyword-only annotation of this parameters definition. */
     Expr getAKwAnnotation() { result = this.getKwAnnotations().getAnItem() }
 
     ArgumentsParent getParent() { py_arguments(this, result) }

python/ql/src/semmle/python/Function.qll

@@ -49,7 +49,11 @@ class Function extends Function_, Scope, AstNode {
     string getArgName(int index) { result = this.getArg(index).(Name).getId() }
 
     Parameter getArgByName(string name) {
-        result = this.getAnArg() and
+        (
+            result = this.getAnArg()
+            or
+            result = this.getAKeywordOnlyArg()
+        ) and
         result.(Name).getId() = name
     }
 
@@ -90,7 +94,7 @@ class Function extends Function_, Scope, AstNode {
     int getPositionalParameterCount() { result = count(this.getAnArg()) }
 
     /** Gets the number of keyword-only parameters */
-    int getKeywordOnlyParameterCount() { result = count(this.getAKwonlyarg()) }
+    int getKeywordOnlyParameterCount() { result = count(this.getAKeywordOnlyArg()) }
 
     /** Whether this function accepts a variable number of arguments. That is, whether it has a starred (*arg) parameter. */
     predicate hasVarArg() { exists(this.getVararg()) }
@@ -102,6 +106,7 @@ class Function extends Function_, Scope, AstNode {
         result = this.getAStmt() or
         result = this.getAnArg() or
         result = this.getVararg() or
+        result = this.getAKeywordOnlyArg() or
         result = this.getKwarg()
     }
 
@@ -185,6 +190,8 @@ class Parameter extends Parameter_ {
             f.getVararg() = this
             or
             f.getKwarg() = this
+            or
+            f.getAKeywordOnlyArg() = this
         )
     }
 
@@ -202,19 +209,31 @@ class Parameter extends Parameter_ {
 
     /** Gets the expression for the default value of this parameter */
     Expr getDefault() {
-        exists(Function f, int n, int c, int d, Arguments args | args = f.getDefinition().getArgs() |
-            f.getArg(n) = this and
-            c = count(f.getAnArg()) and
-            d = count(args.getADefault()) and
-            result = args.getDefault(d - c + n)
+        exists(Function f, int i, Arguments args | args = f.getDefinition().getArgs() |
+            // positional (normal)
+            f.getArg(i) = this and
+            result = args.getDefault(i)
+        )
+        or
+        exists(Function f, int i, Arguments args | args = f.getDefinition().getArgs() |
+            // keyword-only
+            f.getKeywordOnlyArg(i) = this and
+            result = args.getKwDefault(i)
         )
     }
 
     /** Gets the annotation expression of this parameter */
     Expr getAnnotation() {
-        exists(Function f, int n, Arguments args | args = f.getDefinition().getArgs() |
-            f.getArg(n) = this and
-            result = args.getAnnotation(n)
+        exists(Function f, int i, Arguments args | args = f.getDefinition().getArgs() |
+            // positional (normal)
+            f.getArg(i) = this and
+            result = args.getAnnotation(i)
+        )
+        or
+        exists(Function f, int i, Arguments args | args = f.getDefinition().getArgs() |
+            // keyword-only
+            f.getKeywordOnlyArg(i) = this and
+            result = args.getKwAnnotation(i)
         )
         or
         exists(Function f, Arguments args | args = f.getDefinition().getArgs() |
@@ -228,7 +247,10 @@ class Parameter extends Parameter_ {
 
     Variable getVariable() { result.getAnAccess() = this.asName() }
 
-    /** Gets the position of this parameter */
+    /**
+     * Gets the position of this parameter (if any).
+     * No result if this is a "varargs", "kwargs", or keyword-only parameter.
+     */
     int getPosition() { exists(Function f | f.getArg(result) = this) }
 
     /** Gets the name of this parameter */
@@ -243,13 +265,13 @@ class Parameter extends Parameter_ {
     }
 
     /**
-     * Holds if this parameter is a 'varargs' parameter.
+     * Holds if this parameter is a "varargs" parameter.
      * The `varargs` in `f(a, b, *varargs)`.
      */
     predicate isVarargs() { exists(Function func | func.getVararg() = this) }
 
     /**
-     * Holds if this parameter is a 'kwargs' parameter.
+     * Holds if this parameter is a "kwargs" parameter.
      * The `kwargs` in `f(a, b, **kwargs)`.
      */
     predicate isKwargs() { exists(Function func | func.getKwarg() = this) }
@@ -258,7 +280,8 @@ class Parameter extends Parameter_ {
 /** An expression that generates a callable object, either a function expression or a lambda */
 abstract class CallableExpr extends Expr {
     /**
-     * Gets the parameters of this callable.
+     * Gets The default values and annotations (type-hints) for the arguments of this callable.
+     *
      * This predicate is called getArgs(), rather than getParameters() for compatibility with Python's AST module.
      */
     abstract Arguments getArgs();
@@ -295,7 +318,7 @@ class FunctionExpr extends FunctionExpr_, CallableExpr {
     override Arguments getArgs() { result = FunctionExpr_.super.getArgs() }
 }
 
-/** A lambda expression, such as lambda x:x*x */
+/** A lambda expression, such as `lambda x: x+1` */
 class Lambda extends Lambda_, CallableExpr {
     /** Gets the expression to the right of the colon in this lambda expression */
     Expr getExpression() {
@@ -314,13 +337,36 @@ class Lambda extends Lambda_, CallableExpr {
     override Arguments getArgs() { result = Lambda_.super.getArgs() }
 }
 
-/** The arguments in a function definition */
+/**
+ * The default values and annotations (type hints) for the arguments in a function definition.
+ *
+ * Annotations (PEP 3107) is a general mechanism for providing annotations for a function,
+ * that is generally only used for type hints today (PEP 484).
+ */
 class Arguments extends Arguments_ {
+
     Expr getASubExpression() {
+        result = this.getADefault() or
         result = this.getAKwDefault() or
+        //
         result = this.getAnAnnotation() or
-        result = this.getKwargannotation() or
         result = this.getVarargannotation() or
-        result = this.getADefault()
+        result = this.getAKwAnnotation() or
+        result = this.getKwargannotation()
     }
+
+    // The following 4 methods are overwritten to provide better QLdoc. Since the
+    // Arguments_ is auto-generated, we can't change the poor auto-generated docs there :(
+
+    /** Gets the default value for the `index`'th positional parameter. */
+    override Expr getDefault(int index) { result = super.getDefault(index) }
+
+    /** Gets the default value for the `index`'th keyword-only parameter. */
+    override Expr getKwDefault(int index) { result = super.getKwDefault(index) }
+
+    /** Gets the annotation for the `index`'th positional parameter. */
+    override Expr getAnnotation(int index) { result = super.getAnnotation(index) }
+
+    /** Gets the annotation for the `index`'th keyword-only parameter. */
+    override Expr getKwAnnotation(int index) { result = super.getKwAnnotation(index) }
 }

python/ql/src/semmle/python/objects/ObjectAPI.qll

@@ -438,6 +438,7 @@ class CallableValue extends Value {
             exists(int n |
                 call.getArg(n) = result and
                 this.getParameter(n + offset).getId() = name
+                // TODO: and not positional only argument (Python 3.8+)
             )
             or
             called instanceof BoundMethodObjectInternal and

python/ql/src/semmlecode.python.dbscheme

@@ -5,6 +5,17 @@
  *      by adding rules to dbscheme.template
  */
 
+/* This is a dummy line to alter the dbscheme, so we can make a database upgrade
+ * without actually changing any of the dbscheme predicates. It contains a date
+ * to allow for such updates in the future as well.
+ *
+ * 2020-07-02
+ *
+ * DO NOT remove this comment carelessly, since it can revert the dbscheme back to a
+ * previously seen state (matching a previously seen SHA), which would make the upgrade
+ * mechanism not work properly.
+ */
+
  /*
  * External artifacts
  */
@@ -126,7 +137,7 @@ containerparent(int parent: @container ref,
                 unique int child: @container ref);
 
 @sourceline = @file | @py_Module | @xmllocatable;
- 
+
 numlines(int element_id: @sourceline ref,
          int num_lines: int ref,
          int num_code: int ref,
@@ -922,7 +933,7 @@ ext_rettype(int funcid : @py_object ref,
 ext_proptype(int propid : @py_object ref,
              int typeid : @py_object ref);
 
-ext_argreturn(int funcid : @py_object ref, 
+ext_argreturn(int funcid : @py_object ref,
               int arg : int ref);
 
 py_special_objects(unique int obj : @py_cobject ref,
@@ -935,15 +946,15 @@ py_decorated_object(int object : @py_object ref,
 
 @py_source_element = @py_ast_node | @container;
 
-/* XML Files */ 
+/* XML Files */
 
 xmlEncoding  (unique int id: @file ref, varchar(900) encoding: string ref);
 
 xmlDTDs       (unique int id: @xmldtd,
               varchar(900) root: string ref,
               varchar(900) publicId: string ref,
               varchar(900) systemId: string ref,
-              int fileid: @file ref); 
+              int fileid: @file ref);
 
 xmlElements   (unique int id: @xmlelement,
               varchar(900) name: string ref,
@@ -958,7 +969,7 @@ xmlAttrs      (unique int id: @xmlattribute,
               int idx: int ref,
               int fileid: @file ref);
 
-xmlNs         (int id: @xmlnamespace, 
+xmlNs         (int id: @xmlnamespace,
               varchar(900) prefixName: string ref,
               varchar(900) URI: string ref,
               int fileid: @file ref);
@@ -970,7 +981,7 @@ xmlHasNs      (int elementId: @xmlnamespaceable ref,
 xmlComments   (unique int id: @xmlcomment,
               varchar(3600) text: string ref,
               int parentid: @xmlparent ref,
-              int fileid: @file ref); 
+              int fileid: @file ref);
 
 xmlChars      (unique int id: @xmlcharacters,
               varchar(3600) text: string ref,

python/ql/test/3/library-tests/functions/Function.getAChildNode.expected

@@ -0,0 +1,16 @@
+| test.py:4:1:11:2 | Function func | test.py:5:5:5:12 | pos_only |
+| test.py:4:1:11:2 | Function func | test.py:7:5:7:10 | normal |
+| test.py:4:1:11:2 | Function func | test.py:8:6:8:9 | args |
+| test.py:4:1:11:2 | Function func | test.py:9:5:9:16 | keyword_only |
+| test.py:4:1:11:2 | Function func | test.py:10:7:10:12 | kwargs |
+| test.py:4:1:11:2 | Function func | test.py:12:5:12:41 | ExprStmt |
+| test.py:4:1:11:2 | Function func | test.py:13:5:13:15 | ExprStmt |
+| test.py:4:1:11:2 | Function func | test.py:14:5:14:17 | ExprStmt |
+| test.py:23:1:31:2 | Function func2 | test.py:24:5:24:11 | pos_req |
+| test.py:23:1:31:2 | Function func2 | test.py:25:5:25:17 | pos_w_default |
+| test.py:23:1:31:2 | Function func2 | test.py:26:5:26:18 | pos_w_default2 |
+| test.py:23:1:31:2 | Function func2 | test.py:28:5:28:15 | keyword_req |
+| test.py:23:1:31:2 | Function func2 | test.py:29:5:29:21 | keyword_w_default |
+| test.py:23:1:31:2 | Function func2 | test.py:30:5:30:20 | keyword_also_req |
+| test.py:23:1:31:2 | Function func2 | test.py:32:5:32:18 | ExprStmt |
+| test.py:23:1:31:2 | Function func2 | test.py:33:5:40:5 | ExprStmt |

python/ql/test/3/library-tests/functions/Function.getAChildNode.ql

@@ -0,0 +1,4 @@
+import python
+
+from Function f
+select f, f.getAChildNode()

python/ql/test/3/library-tests/functions/Function.getArg.expected

@@ -0,0 +1,5 @@
+| test.py:4:1:11:2 | Function func | 0 | test.py:5:5:5:12 | Parameter |
+| test.py:4:1:11:2 | Function func | 1 | test.py:7:5:7:10 | Parameter |
+| test.py:23:1:31:2 | Function func2 | 0 | test.py:24:5:24:11 | Parameter |
+| test.py:23:1:31:2 | Function func2 | 1 | test.py:25:5:25:17 | Parameter |
+| test.py:23:1:31:2 | Function func2 | 2 | test.py:26:5:26:18 | Parameter |

python/ql/test/3/library-tests/functions/Function.getArg.ql

@@ -0,0 +1,4 @@
+import python
+
+from Function f, int i
+select f, i, f.getArg(i)

python/ql/test/3/library-tests/functions/Function.getArgByName.expected

@@ -0,0 +1,9 @@
+| test.py:4:1:11:2 | Function func | keyword_only | test.py:9:5:9:16 | Parameter |
+| test.py:4:1:11:2 | Function func | normal | test.py:7:5:7:10 | Parameter |
+| test.py:4:1:11:2 | Function func | pos_only | test.py:5:5:5:12 | Parameter |
+| test.py:23:1:31:2 | Function func2 | keyword_also_req | test.py:30:5:30:20 | Parameter |
+| test.py:23:1:31:2 | Function func2 | keyword_req | test.py:28:5:28:15 | Parameter |
+| test.py:23:1:31:2 | Function func2 | keyword_w_default | test.py:29:5:29:21 | Parameter |
+| test.py:23:1:31:2 | Function func2 | pos_req | test.py:24:5:24:11 | Parameter |
+| test.py:23:1:31:2 | Function func2 | pos_w_default | test.py:25:5:25:17 | Parameter |
+| test.py:23:1:31:2 | Function func2 | pos_w_default2 | test.py:26:5:26:18 | Parameter |

python/ql/test/3/library-tests/functions/Function.getArgByName.ql

@@ -0,0 +1,4 @@
+import python
+
+from Function f, string name
+select f, name, f.getArgByName(name)