Handle python BoolOp for multiple arguments (#1698)

cpg-language-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/ExpressionHandler.kt

@@ -181,22 +181,42 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
         return subscriptExpression
     }
 
+    /**
+     * This method handles the python
+     * [`BoolOp` expression](https://docs.python.org/3/library/ast.html#ast.BoolOp).
+     *
+     * Generates a (potentially nested) [BinaryOperator] from a `BoolOp`. Less than two operands in
+     * [Python.AST.BoolOp.values] don't make sense and will generate a [ProblemExpression]. If only
+     * two operands exist, a simple [BinaryOperator] will be generated. More than two operands will
+     * lead to a nested [BinaryOperator]. E.g., if [Python.AST.BoolOp.values] contains the operators
+     * `[a, b, c]`, the result will be `a OP (b OP c)`.
+     */
     private fun handleBoolOp(node: Python.AST.BoolOp): Expression {
         val op =
             when (node.op) {
                 is Python.AST.And -> "and"
                 is Python.AST.Or -> "or"
             }
-        val ret = newBinaryOperator(operatorCode = op, rawNode = node)
-        if (node.values.size != 2) {
-            return newProblemExpression(
-                "Expected exactly two expressions but got " + node.values.size,
+
+        return if (node.values.size <= 1) {
+            newProblemExpression(
+                "Expected exactly two expressions but got ${node.values.size}",
                 rawNode = node
             )
+        } else {
+            // Start with the last two operands, then keep prepending the previous ones until the
+            // list is finished.
+            val lastTwo = newBinaryOperator(op, rawNode = node)
+            lastTwo.rhs = handle(node.values.last())
+            lastTwo.lhs = handle(node.values[node.values.size - 2])
+            return node.values.subList(0, node.values.size - 2).foldRight(lastTwo) { newVal, start
+                ->
+                val nextValue = newBinaryOperator(op, rawNode = node)
+                nextValue.rhs = start
+                nextValue.lhs = handle(newVal)
+                nextValue
+            }
         }
-        ret.lhs = handle(node.values[0])
-        ret.rhs = handle(node.values[1])
-        return ret
     }
 
     private fun handleList(node: Python.AST.List): Expression {

cpg-language-python/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/python/ExpressionHandlerTest.kt

@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2024, Fraunhofer AISEC. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ *                    $$$$$$\  $$$$$$$\   $$$$$$\
+ *                   $$  __$$\ $$  __$$\ $$  __$$\
+ *                   $$ /  \__|$$ |  $$ |$$ /  \__|
+ *                   $$ |      $$$$$$$  |$$ |$$$$\
+ *                   $$ |      $$  ____/ $$ |\_$$ |
+ *                   $$ |  $$\ $$ |      $$ |  $$ |
+ *                   \$$$$$   |$$ |      \$$$$$   |
+ *                    \______/ \__|       \______/
+ *
+ */
+package de.fraunhofer.aisec.cpg.frontends.python
+
+import de.fraunhofer.aisec.cpg.graph.*
+import de.fraunhofer.aisec.cpg.graph.statements.expressions.BinaryOperator
+import de.fraunhofer.aisec.cpg.test.analyze
+import de.fraunhofer.aisec.cpg.test.assertLiteralValue
+import de.fraunhofer.aisec.cpg.test.assertLocalName
+import java.nio.file.Path
+import kotlin.test.*
+
+class ExpressionHandlerTest {
+    @Test
+    fun testBoolOps() {
+        val topLevel = Path.of("src", "test", "resources", "python")
+        val result =
+            analyze(listOf(topLevel.resolve("boolop.py").toFile()), topLevel, true) {
+                it.registerLanguage<PythonLanguage>()
+            }
+        assertNotNull(result)
+
+        val twoBoolOpCondition =
+            result.functions["twoBoolOp"]?.ifs?.singleOrNull()?.condition as? BinaryOperator
+        assertNotNull(twoBoolOpCondition)
+        assertEquals("and", twoBoolOpCondition.operatorCode)
+        assertLocalName("a", twoBoolOpCondition.lhs)
+        assertLiteralValue(true, twoBoolOpCondition.rhs)
+
+        // We expect that lhs comes first in the EOG and then the rhs.
+        assertContains(twoBoolOpCondition.lhs.nextEOG, twoBoolOpCondition.rhs)
+
+        val threeBoolOpCondition =
+            result.functions["threeBoolOp"]?.ifs?.singleOrNull()?.condition as? BinaryOperator
+        assertNotNull(threeBoolOpCondition)
+        assertEquals("and", threeBoolOpCondition.operatorCode)
+        assertLocalName("a", threeBoolOpCondition.lhs)
+        val threeBoolOpConditionRhs = threeBoolOpCondition.rhs as? BinaryOperator
+        assertNotNull(threeBoolOpConditionRhs)
+        assertEquals("and", threeBoolOpConditionRhs.operatorCode)
+        assertLiteralValue(true, threeBoolOpConditionRhs.lhs)
+        assertLocalName("b", threeBoolOpConditionRhs.rhs)
+
+        val threeBoolOpNoBoolCondition =
+            result.functions["threeBoolOpNoBool"]?.ifs?.singleOrNull()?.condition as? BinaryOperator
+        assertNotNull(threeBoolOpNoBoolCondition)
+        assertEquals("and", threeBoolOpNoBoolCondition.operatorCode)
+        assertLocalName("a", threeBoolOpNoBoolCondition.lhs)
+        val threeBoolOpNoBoolConditionRhs = threeBoolOpNoBoolCondition.rhs as? BinaryOperator
+        assertNotNull(threeBoolOpNoBoolConditionRhs)
+        assertEquals("and", threeBoolOpNoBoolConditionRhs.operatorCode)
+        assertLiteralValue(true, threeBoolOpNoBoolConditionRhs.lhs)
+        assertLiteralValue("foo", threeBoolOpNoBoolConditionRhs.rhs)
+
+        // We expect that lhs comes first in the EOG and then the lhs of the rhs and last the rhs of
+        // the rhs.
+        assertContains(threeBoolOpNoBoolCondition.lhs.nextEOG, threeBoolOpNoBoolConditionRhs.lhs)
+        assertContains(threeBoolOpNoBoolConditionRhs.lhs.nextEOG, threeBoolOpNoBoolConditionRhs.rhs)
+
+        val nestedBoolOpDifferentOp =
+            result.functions["nestedBoolOpDifferentOp"]?.ifs?.singleOrNull()?.condition
+                as? BinaryOperator
+        assertNotNull(nestedBoolOpDifferentOp)
+        assertEquals("or", nestedBoolOpDifferentOp.operatorCode)
+        assertLocalName("b", nestedBoolOpDifferentOp.rhs)
+        val nestedBoolOpDifferentOpLhs = nestedBoolOpDifferentOp.lhs as? BinaryOperator
+        assertNotNull(nestedBoolOpDifferentOpLhs)
+        assertEquals("and", nestedBoolOpDifferentOpLhs.operatorCode)
+        assertLiteralValue(true, nestedBoolOpDifferentOpLhs.rhs)
+        assertLocalName("a", nestedBoolOpDifferentOpLhs.lhs)
+
+        // We expect that lhs of the "and" comes first in the EOG and then the rhs of the "and",
+        // then we evaluate the whole "and" and last the rhs of the "or".
+        assertContains(nestedBoolOpDifferentOpLhs.lhs.nextEOG, nestedBoolOpDifferentOpLhs.rhs)
+        assertContains(nestedBoolOpDifferentOpLhs.rhs.nextEOG, nestedBoolOpDifferentOpLhs)
+        assertContains(nestedBoolOpDifferentOpLhs.nextEOG, nestedBoolOpDifferentOp.rhs)
+
+        val nestedBoolOpDifferentOp2 =
+            result.functions["nestedBoolOpDifferentOp2"]?.ifs?.singleOrNull()?.condition
+                as? BinaryOperator
+        assertNotNull(nestedBoolOpDifferentOp2)
+        assertEquals("or", nestedBoolOpDifferentOp2.operatorCode)
+        assertLocalName("a", nestedBoolOpDifferentOp2.lhs)
+        val nestedBoolOpDifferentOp2Rhs = nestedBoolOpDifferentOp2.rhs as? BinaryOperator
+        assertNotNull(nestedBoolOpDifferentOp2Rhs)
+        assertEquals("and", nestedBoolOpDifferentOp2Rhs.operatorCode)
+        assertLiteralValue(true, nestedBoolOpDifferentOp2Rhs.lhs)
+        assertLocalName("b", nestedBoolOpDifferentOp2Rhs.rhs)
+
+        // We expect that lhs comes first in the EOG and then the lhs of the rhs and last the rhs of
+        // the rhs.
+        assertContains(nestedBoolOpDifferentOp2.lhs.nextEOG, nestedBoolOpDifferentOp2Rhs.lhs)
+        assertContains(nestedBoolOpDifferentOp2Rhs.lhs.nextEOG, nestedBoolOpDifferentOp2Rhs.rhs)
+    }
+}

cpg-language-python/src/test/resources/python/boolop.py

@@ -0,0 +1,24 @@
+def twoBoolOp(a):
+    if a and True:
+        print(a)
+    return a
+
+def threeBoolOp(a, b):
+    if a and True and b:
+        print(a)
+    return a
+
+def nestedBoolOpDifferentOp(a, b):
+    if a and True or b:
+        print(a)
+    return a
+
+def nestedBoolOpDifferentOp2(a, b):
+    if a or True and b:
+        print(a)
+    return a
+
+def threeBoolOpNoBool(a):
+    if a and True and "foo":
+        print(a)
+    return a