Merge pull request github#2827 from BekaValentine/objectapi-to-valueapi-expectedmappingforformatstring

Python: ObjectAPI to ValueAPI: ExpectedMappingForFormatString

Author: RasmusWL

python/ql/src/Expressions/ExpectedMappingForFormatString.ql

@@ -13,7 +13,14 @@
 import python
 import semmle.python.strings
 
-from Expr e, ClassObject t
-where exists(BinaryExpr b | b.getOp() instanceof Mod and format_string(b.getLeft()) and e = b.getRight() and
-mapping_format(b.getLeft()) and e.refersTo(_, t, _) and not t.isMapping())
+from Expr e, ClassValue t
+where
+    exists(BinaryExpr b |
+        b.getOp() instanceof Mod and
+        format_string(b.getLeft()) and
+        e = b.getRight() and
+        mapping_format(b.getLeft()) and
+        e.pointsTo().getClass() = t and
+        not t.isMapping()
+    )
 select e, "Right hand side of a % operator must be a mapping, not class $@.", t, t.getName()

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

@@ -456,6 +456,45 @@ class ClassValue extends Value {
         or
         this.hasAttribute("__getitem__")
     }
+    
+    /** Holds if this class is a container(). That is, does it have a __getitem__ method.*/
+    predicate isContainer() {
+        exists(this.lookup("__getitem__"))
+    }
+    
+    /** Holds if this class is probably a sequence. */
+    predicate isSequence() {
+        /* To determine whether something is a sequence or a mapping is not entirely clear,
+         * so we need to guess a bit.
+         */
+        this.getASuperType() = ClassValue::tuple()
+        or
+        this.getASuperType() = ClassValue::list()
+        or
+        this.getASuperType() = ClassValue::range()
+        or
+        this.getASuperType() = ClassValue::bytes()
+        or
+        this.getASuperType() = ClassValue::unicode()
+        or
+        major_version() = 2 and this.getASuperType() = Value::named("collections.Sequence")
+        or
+        major_version() = 3 and this.getASuperType() = Value::named("collections.abc.Sequence")
+        or
+        /* Does it have an index or __reversed__ method? */
+        this.isContainer() and
+        (
+            this.hasAttribute("index") or
+            this.hasAttribute("__reversed__")
+        )
+    }
+    
+    /** Holds if this class is a mapping. */
+    predicate isMapping() {
+        this.hasAttribute("__getitem__")
+        and
+        not this.isSequence()
+    }
 
     /** Holds if this class is a descriptor. */
     predicate isDescriptorType() {
@@ -859,7 +898,7 @@ module ClassValue {
     ClassValue complex() {
         result = TBuiltinClassObject(Builtin::special("complex"))
     }
-
+    
     /** Get the `ClassValue` for the `bytes` class (also called `str` in Python 2). */
     ClassValue bytes() {
         result = TBuiltinClassObject(Builtin::special("bytes"))

python/ql/test/query-tests/Expressions/general/ExpectedMappingForFormatString.expected

@@ -1 +1 @@
-| str_fmt_test.py:5:26:5:26 | x | Right hand side of a % operator must be a mapping, not class $@. | file://:Compiled Code:0:0:0:0 | builtin-class list | list |
+| str_fmt_test.py:5:26:5:26 | x | Right hand side of a % operator must be a mapping, not class $@. | file://:0:0:0:0 | builtin-class list | list |