review

Fraunhofer-AISEC · Oct 29, 2024 · b5450ff · b5450ff
1 parent 91f4ea1
commit b5450ff
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 71 deletions.
diff --git a/...uage-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/ExpressionHandler.kt b/...uage-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/ExpressionHandler.kt
@@ -88,18 +88,25 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
      * [`comprehension`](https://docs.python.org/3/library/ast.html#ast.comprehension) into a
      * [ComprehensionExpression].
      *
-     * Connects multiple predicates by "AND".
+     * Connects multiple predicates by `and`.
      */
     private fun handleComprehension(node: Python.AST.comprehension): ComprehensionExpression {
-        return newComprehensionExpression(node).apply {
-            this.variable = handle(node.target)
-            this.iterable = handle(node.iter)
+        return newComprehensionExpression(rawNode = node).apply {
+            variable = handle(node.target)
+            iterable = handle(node.iter)
             val predicates = node.ifs.map { handle(it) }
             if (predicates.size == 1) {
-                this.predicate = predicates.single()
+                predicate = predicates.single()
             } else if (predicates.size > 1) {
-                this.predicate = joinListWithBinOp("and", predicates, node)
+                predicate =
+                    joinListWithBinOp(operatorCode = "and", nodes = predicates, rawNode = node)
             }
+            if (node.is_async != 0L)
+                additionalProblems +=
+                    newProblemExpression(
+                        "Node marked as is_async but we don't support this yet",
+                        rawNode = node
+                    )
         }
     }
 
@@ -109,9 +116,9 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
      * [CollectionComprehension].
      */
     private fun handleGeneratorExp(node: Python.AST.GeneratorExp): CollectionComprehension {
-        return newCollectionComprehension(node).apply {
-            this.statement = handle(node.elt)
-            this.comprehensionExpressions += node.generators.map { handleComprehension(it) }
+        return newCollectionComprehension(rawNode = node).apply {
+            statement = handle(node.elt)
+            comprehensionExpressions += node.generators.map { handleComprehension(it) }
         }
     }
 
@@ -120,10 +127,10 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
      * into a [CollectionComprehension].
      */
     private fun handleListComprehension(node: Python.AST.ListComp): CollectionComprehension {
-        return newCollectionComprehension(node).apply {
-            this.statement = handle(node.elt)
-            this.comprehensionExpressions += node.generators.map { handleComprehension(it) }
-            this.type = objectType("list") // TODO: Replace this once we have dedicated types
+        return newCollectionComprehension(rawNode = node).apply {
+            statement = handle(node.elt)
+            comprehensionExpressions += node.generators.map { handleComprehension(it) }
+            type = objectType("list") // TODO: Replace this once we have dedicated types
         }
     }
 
@@ -132,7 +139,7 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
      * a [CollectionComprehension].
      */
     private fun handleSetComprehension(node: Python.AST.SetComp): CollectionComprehension {
-        return newCollectionComprehension(node).apply {
+        return newCollectionComprehension(rawNode = node).apply {
             this.statement = handle(node.elt)
             this.comprehensionExpressions += node.generators.map { handleComprehension(it) }
             this.type = objectType("set") // TODO: Replace this once we have dedicated types
@@ -144,8 +151,13 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
      * into a [CollectionComprehension].
      */
     private fun handleDictComprehension(node: Python.AST.DictComp): CollectionComprehension {
-        return newCollectionComprehension(node).apply {
-            this.statement = newKeyValueExpression(handle(node.key), handle(node.value), node)
+        return newCollectionComprehension(rawNode = node).apply {
+            this.statement =
+                newKeyValueExpression(
+                    key = handle(node.key),
+                    value = handle(node.value),
+                    rawNode = node
+                )
             this.comprehensionExpressions += node.generators.map { handleComprehension(it) }
             this.type = objectType("dict") // TODO: Replace this once we have dedicated types
         }
@@ -215,10 +227,15 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
         } else if (values.size == 1) {
             values.first()
         } else {
-            joinListWithBinOp("+", values, node)
+            joinListWithBinOp(operatorCode = "+", nodes = values, rawNode = node)
         }
     }
 
+    /**
+     * Joins the [nodes] with a [BinaryOperator] with the [operatorCode]. Nests the whole thing,
+     * where the first element in [nodes] is the lhs of the root of the tree of binary operators.
+     * The last operands are further down the tree.
+     */
     private fun joinListWithBinOp(
         operatorCode: String,
         nodes: List<Expression>,

diff --git a/...-python/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/python/ExpressionHandlerTest.kt b/...-python/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/python/ExpressionHandlerTest.kt
@@ -51,12 +51,12 @@ class ExpressionHandlerTest {
         val listComp = result.functions["listComp"]
         assertNotNull(listComp)
 
-        val body = listComp.body as? Block
-        assertNotNull(body)
-        val singleWithIfAssignment = body.statements[0] as? AssignExpression
-        assertNotNull(singleWithIfAssignment)
-        val singleWithIf = singleWithIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithIf)
+        val body = listComp.body
+        assertIs<Block>(body)
+        val singleWithIfAssignment = body.statements[0]
+        assertIs<AssignExpression>(singleWithIfAssignment)
+        val singleWithIf = singleWithIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithIf)
         assertIs<CallExpression>(singleWithIf.statement)
         assertEquals(1, singleWithIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithIf.comprehensionExpressions[0].variable)
@@ -66,21 +66,21 @@ class ExpressionHandlerTest {
         assertIs<BinaryOperator>(ifPredicate)
         assertEquals("==", ifPredicate.operatorCode)
 
-        val singleWithoutIfAssignment = body.statements[1] as? AssignExpression
-        assertNotNull(singleWithoutIfAssignment)
-        val singleWithoutIf = singleWithoutIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithoutIf)
+        val singleWithoutIfAssignment = body.statements[1]
+        assertIs<AssignExpression>(singleWithoutIfAssignment)
+        val singleWithoutIf = singleWithoutIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithoutIf)
         assertIs<CallExpression>(singleWithoutIf.statement)
         assertEquals(1, singleWithoutIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithoutIf.comprehensionExpressions[0].variable)
         assertIs<Reference>(singleWithoutIf.comprehensionExpressions[0].iterable)
         assertLocalName("x", singleWithoutIf.comprehensionExpressions[0].iterable)
         assertNull(singleWithoutIf.comprehensionExpressions[0].predicate)
 
-        val singleWithDoubleIfAssignment = body.statements[2] as? AssignExpression
-        assertNotNull(singleWithDoubleIfAssignment)
-        val singleWithDoubleIf = singleWithDoubleIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithDoubleIf)
+        val singleWithDoubleIfAssignment = body.statements[2]
+        assertIs<AssignExpression>(singleWithDoubleIfAssignment)
+        val singleWithDoubleIf = singleWithDoubleIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithDoubleIf)
         assertIs<CallExpression>(singleWithDoubleIf.statement)
         assertEquals(1, singleWithDoubleIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithDoubleIf.comprehensionExpressions[0].variable)
@@ -91,7 +91,7 @@ class ExpressionHandlerTest {
         assertEquals("and", doubleIfPredicate.operatorCode)
 
         val doubleAssignment = body.statements[3] as? AssignExpression
-        assertNotNull(doubleAssignment)
+        assertIs<AssignExpression>(doubleAssignment)
         val double = doubleAssignment.rhs[0] as? CollectionComprehension
         assertNotNull(double)
         assertIs<CallExpression>(double.statement)
@@ -112,10 +112,10 @@ class ExpressionHandlerTest {
 
         val body = listComp.body as? Block
         assertNotNull(body)
-        val singleWithIfAssignment = body.statements[0] as? AssignExpression
-        assertNotNull(singleWithIfAssignment)
-        val singleWithIf = singleWithIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithIf)
+        val singleWithIfAssignment = body.statements[0]
+        assertIs<AssignExpression>(singleWithIfAssignment)
+        val singleWithIf = singleWithIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithIf)
         assertIs<CallExpression>(singleWithIf.statement)
         assertEquals(1, singleWithIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithIf.comprehensionExpressions[0].variable)
@@ -125,21 +125,21 @@ class ExpressionHandlerTest {
         assertIs<BinaryOperator>(ifPredicate)
         assertEquals("==", ifPredicate.operatorCode)
 
-        val singleWithoutIfAssignment = body.statements[1] as? AssignExpression
-        assertNotNull(singleWithoutIfAssignment)
-        val singleWithoutIf = singleWithoutIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithoutIf)
+        val singleWithoutIfAssignment = body.statements[1]
+        assertIs<AssignExpression>(singleWithoutIfAssignment)
+        val singleWithoutIf = singleWithoutIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithoutIf)
         assertIs<CallExpression>(singleWithoutIf.statement)
         assertEquals(1, singleWithoutIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithoutIf.comprehensionExpressions[0].variable)
         assertIs<Reference>(singleWithoutIf.comprehensionExpressions[0].iterable)
         assertLocalName("x", singleWithoutIf.comprehensionExpressions[0].iterable)
         assertNull(singleWithoutIf.comprehensionExpressions[0].predicate)
 
-        val singleWithDoubleIfAssignment = body.statements[2] as? AssignExpression
-        assertNotNull(singleWithDoubleIfAssignment)
-        val singleWithDoubleIf = singleWithDoubleIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithDoubleIf)
+        val singleWithDoubleIfAssignment = body.statements[2]
+        assertIs<AssignExpression>(singleWithDoubleIfAssignment)
+        val singleWithDoubleIf = singleWithDoubleIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithDoubleIf)
         assertIs<CallExpression>(singleWithDoubleIf.statement)
         assertEquals(1, singleWithDoubleIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithDoubleIf.comprehensionExpressions[0].variable)
@@ -149,13 +149,12 @@ class ExpressionHandlerTest {
         assertIs<BinaryOperator>(doubleIfPredicate)
         assertEquals("and", doubleIfPredicate.operatorCode)
 
-        val doubleAssignment = body.statements[3] as? AssignExpression
-        assertNotNull(doubleAssignment)
-        val double = doubleAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(double)
+        val doubleAssignment = body.statements[3]
+        assertIs<AssignExpression>(doubleAssignment)
+        val double = doubleAssignment.rhs[0]
+        assertIs<CollectionComprehension>(double)
         assertIs<CallExpression>(double.statement)
         assertEquals(2, double.comprehensionExpressions.size)
-        // TODO: Add tests on the comprehension expressions
     }
 
     @Test
@@ -171,10 +170,10 @@ class ExpressionHandlerTest {
 
         val body = listComp.body as? Block
         assertNotNull(body)
-        val singleWithIfAssignment = body.statements[0] as? AssignExpression
-        assertNotNull(singleWithIfAssignment)
-        val singleWithIf = singleWithIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithIf)
+        val singleWithIfAssignment = body.statements[0]
+        assertIs<AssignExpression>(singleWithIfAssignment)
+        val singleWithIf = singleWithIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithIf)
         var statement = singleWithIf.statement
         assertIs<KeyValueExpression>(statement)
         assertIs<Reference>(statement.key)
@@ -188,10 +187,10 @@ class ExpressionHandlerTest {
         assertIs<BinaryOperator>(ifPredicate)
         assertEquals("==", ifPredicate.operatorCode)
 
-        val singleWithoutIfAssignment = body.statements[1] as? AssignExpression
-        assertNotNull(singleWithoutIfAssignment)
-        val singleWithoutIf = singleWithoutIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithoutIf)
+        val singleWithoutIfAssignment = body.statements[1]
+        assertIs<AssignExpression>(singleWithoutIfAssignment)
+        val singleWithoutIf = singleWithoutIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithoutIf)
         statement = singleWithIf.statement
         assertIs<KeyValueExpression>(statement)
         assertIs<Reference>(statement.key)
@@ -203,10 +202,10 @@ class ExpressionHandlerTest {
         assertLocalName("x", singleWithoutIf.comprehensionExpressions[0].iterable)
         assertNull(singleWithoutIf.comprehensionExpressions[0].predicate)
 
-        val singleWithDoubleIfAssignment = body.statements[2] as? AssignExpression
-        assertNotNull(singleWithDoubleIfAssignment)
-        val singleWithDoubleIf = singleWithDoubleIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithDoubleIf)
+        val singleWithDoubleIfAssignment = body.statements[2]
+        assertIs<AssignExpression>(singleWithDoubleIfAssignment)
+        val singleWithDoubleIf = singleWithDoubleIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithDoubleIf)
         statement = singleWithIf.statement
         assertIs<KeyValueExpression>(statement)
         assertIs<Reference>(statement.key)
@@ -221,7 +220,7 @@ class ExpressionHandlerTest {
         assertEquals("and", doubleIfPredicate.operatorCode)
 
         val doubleAssignment = body.statements[3] as? AssignExpression
-        assertNotNull(doubleAssignment)
+        assertIs<AssignExpression>(doubleAssignment)
         val double = doubleAssignment.rhs[0] as? CollectionComprehension
         assertNotNull(double)
         statement = singleWithIf.statement
@@ -230,7 +229,6 @@ class ExpressionHandlerTest {
         assertLocalName("i", statement.key)
         assertIs<CallExpression>(statement.value)
         assertEquals(2, double.comprehensionExpressions.size)
-        // TODO: Add tests on the comprehension expressions
     }
 
     @Test
@@ -246,10 +244,10 @@ class ExpressionHandlerTest {
 
         val body = listComp.body as? Block
         assertNotNull(body)
-        val singleWithIfAssignment = body.statements[0] as? AssignExpression
-        assertNotNull(singleWithIfAssignment)
-        val singleWithIf = singleWithIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithIf)
+        val singleWithIfAssignment = body.statements[0]
+        assertIs<AssignExpression>(singleWithIfAssignment)
+        val singleWithIf = singleWithIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithIf)
         assertIs<BinaryOperator>(singleWithIf.statement)
         assertEquals(1, singleWithIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithIf.comprehensionExpressions[0].variable)
@@ -259,10 +257,10 @@ class ExpressionHandlerTest {
         assertIs<BinaryOperator>(ifPredicate)
         assertEquals("==", ifPredicate.operatorCode)
 
-        val singleWithoutIfAssignment = body.statements[1] as? AssignExpression
-        assertNotNull(singleWithoutIfAssignment)
-        val singleWithoutIf = singleWithoutIfAssignment.rhs[0] as? CollectionComprehension
-        assertNotNull(singleWithoutIf)
+        val singleWithoutIfAssignment = body.statements[1]
+        assertIs<AssignExpression>(singleWithoutIfAssignment)
+        val singleWithoutIf = singleWithoutIfAssignment.rhs[0]
+        assertIs<CollectionComprehension>(singleWithoutIf)
         assertIs<BinaryOperator>(singleWithoutIf.statement)
         assertEquals(1, singleWithoutIf.comprehensionExpressions.size)
         assertLocalName("i", singleWithoutIf.comprehensionExpressions[0].variable)