BoolOp as loop instead of recursion

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

@@ -181,22 +181,47 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
         return subscriptExpression
     }
 
+    /**
+     * Handles a [Python.AST.BoolOp]. This can either be a chain of `and` or `or` expressions. We
+     * model this as [BinaryOperator]s with nesting. Thus, `a and b and c` becomes `a and (b and c)`
+     * (same for `or`).
+     */
     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,
-                rawNode = node
-            )
+        return when {
+            node.values.size <= 1 ->
+                newProblemExpression(
+                    "handleBoolOp: Expected at least two expressions but got " + node.values.size,
+                    rawNode = node
+                )
+            else -> {
+                val result = newBinaryOperator(operatorCode = op, rawNode = node)
+                var idx = 0
+                var currentInnerNode: BinaryOperator = result
+
+                // For all iterations until the last one: set the lhs to handle the current idx and
+                // create a new nested [BinaryOperator] for the rhs. Then slide the window by one.
+                while (idx < node.values.size - 2) {
+                    // handle the current lhs
+                    currentInnerNode.lhs = handle(node.values[idx])
+
+                    // create a new [BinaryOperator] for the current rhs
+                    val nestedBinOp = newBinaryOperator(operatorCode = op, rawNode = node)
+                    currentInnerNode.rhs = nestedBinOp
+                    currentInnerNode = nestedBinOp
+                    idx++
+                }
+
+                // Final run: set lhs and rhs without creating a new nested [BinaryOperator]
+                currentInnerNode.lhs = handle(node.values[idx]) // second last entry
+                currentInnerNode.rhs = handle(node.values[idx + 1]) // last entry
+                result
+            }
         }
-        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.graph.statements.expressions.Literal
+import de.fraunhofer.aisec.cpg.graph.statements.expressions.Reference
+import de.fraunhofer.aisec.cpg.test.analyze
+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)
+        assertEquals("a", (twoBoolOpCondition.lhs as? Reference)?.name?.localName)
+        assertEquals(true, (twoBoolOpCondition.rhs as? Literal<*>)?.value)
+
+        // 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)
+        assertEquals("a", (threeBoolOpCondition.lhs as? Reference)?.name?.localName)
+        val threeBoolOpConditionRhs = threeBoolOpCondition.rhs as? BinaryOperator
+        assertNotNull(threeBoolOpConditionRhs)
+        assertEquals("and", threeBoolOpConditionRhs.operatorCode)
+        assertEquals(true, (threeBoolOpConditionRhs.lhs as? Literal<*>)?.value)
+        assertEquals("b", (threeBoolOpConditionRhs.rhs as? Reference)?.name?.localName)
+
+        val threeBoolOpNoBoolCondition =
+            result.functions["threeBoolOpNoBool"]?.ifs?.singleOrNull()?.condition as? BinaryOperator
+        assertNotNull(threeBoolOpNoBoolCondition)
+        assertEquals("and", threeBoolOpNoBoolCondition.operatorCode)
+        assertEquals("a", (threeBoolOpNoBoolCondition.lhs as? Reference)?.name?.localName)
+        val threeBoolOpNoBoolConditionRhs = threeBoolOpNoBoolCondition.rhs as? BinaryOperator
+        assertNotNull(threeBoolOpNoBoolConditionRhs)
+        assertEquals("and", threeBoolOpNoBoolConditionRhs.operatorCode)
+        assertEquals(true, (threeBoolOpNoBoolConditionRhs.lhs as? Literal<*>)?.value)
+        assertEquals("foo", (threeBoolOpNoBoolConditionRhs.rhs as? Literal<*>)?.value)
+
+        // 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)
+        assertEquals("b", (nestedBoolOpDifferentOp.rhs as? Reference)?.name?.localName)
+        val nestedBoolOpDifferentOpLhs = nestedBoolOpDifferentOp.lhs as? BinaryOperator
+        assertNotNull(nestedBoolOpDifferentOpLhs)
+        assertEquals("and", nestedBoolOpDifferentOpLhs.operatorCode)
+        assertEquals(true, (nestedBoolOpDifferentOpLhs.rhs as? Literal<*>)?.value)
+        assertEquals("a", (nestedBoolOpDifferentOpLhs.lhs as? Reference)?.name?.localName)
+
+        // 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)
+        assertEquals("a", (nestedBoolOpDifferentOp2.lhs as? Reference)?.name?.localName)
+        val nestedBoolOpDifferentOp2Rhs = nestedBoolOpDifferentOp2.rhs as? BinaryOperator
+        assertNotNull(nestedBoolOpDifferentOp2Rhs)
+        assertEquals("and", nestedBoolOpDifferentOp2Rhs.operatorCode)
+        assertEquals(true, (nestedBoolOpDifferentOp2Rhs.lhs as? Literal<*>)?.value)
+        assertEquals("b", (nestedBoolOpDifferentOp2Rhs.rhs as? Reference)?.name?.localName)
+
+        // 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