Restoring Comment Matcher Functionality

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/CommentMatcher.kt

@@ -99,7 +99,7 @@ class CommentMatcher {
                 .filter {
                     artifactLocation == null || artifactLocation == it.location?.artifactLocation
                 }
-                .toMutableList()
+                .toMutableSet()
 
         // Because we sometimes wrap all elements into a NamespaceDeclaration we have to extract the
         // children with a location
@@ -109,6 +109,19 @@ class CommentMatcher {
             }
         )
 
+        // When a child has no location we can not properly consider it for comment matching,
+        // however, it might have
+        // a child with a location that we want to consider, this can overlap with namespaces but
+        // nodes are considered
+        // only once in the set
+        children.addAll(
+            children
+                .filter { node -> node.location == null || node.location?.region == Region() }
+                .flatMap { locationLess ->
+                    SubgraphWalker.getAstChildren(locationLess).filter { it !in children }
+                }
+        )
+
         // Searching for the closest successor to our comment amongst the children of the smallest
         // enclosing nodes
         val successors =

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

@@ -81,12 +81,13 @@ class ExpressionHandler(frontend: PythonLanguageFrontend) :
     private fun handleDict(node: Python.ASTDict): Expression {
         val lst = mutableListOf<Expression>()
         for (i in node.values.indices) { // TODO: keys longer than values possible?
+            // Here we can not use node as raw node as it spans all keys and values
             lst +=
                 newKeyValueExpression(
-                    key = node.keys[i]?.let { handle(it) },
-                    value = handle(node.values[i]),
-                    rawNode = node
-                )
+                        key = node.keys[i]?.let { handle(it) },
+                        value = handle(node.values[i]),
+                    )
+                    .codeAndLocationFromChildren(node)
         }
         val ile = newInitializerListExpression(rawNode = node)
         ile.initializers = lst.toList()

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

@@ -60,7 +60,7 @@ object JepSingleton {
         val virtualEnvName = System.getenv("CPG_PYTHON_VIRTUALENV") ?: "cpg"
         val virtualEnvPath =
             Paths.get(System.getProperty("user.home"), ".virtualenvs", virtualEnvName)
-        val pythonVersions = listOf("3.9", "3.10", "3.11", "3.12", "3.13")
+        val pythonVersions = listOf("3.8", "3.9", "3.10", "3.11", "3.12", "3.13")
         val wellKnownPaths = mutableListOf<Path>()
         pythonVersions.forEach { version ->
             // Linux

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

@@ -1197,7 +1197,7 @@ interface Python {
      *  |  arg(identifier arg, expr? annotation, string? type_comment)
      * ```
      */
-    class ASTarg(pyObject: PyObject) : AST(pyObject) {
+    class ASTarg(pyObject: PyObject) : AST(pyObject), WithPythonLocation {
         val arg: String by lazy { "arg" of pyObject }
         val annotation: ASTBASEexpr? by lazy { "annotation" of pyObject }
         val type_comment: String? by lazy { "type_comment" of pyObject }

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

@@ -77,7 +77,7 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
     private fun handleWhile(node: Python.ASTWhile): Statement {
         val ret = newWhileStatement(rawNode = node)
         ret.condition = frontend.expressionHandler.handle(node.test)
-        ret.statement = makeBlock(node.body)
+        ret.statement = makeBlock(node.body).codeAndLocationFromChildren(node)
         node.orelse.firstOrNull()?.let { TODO("Not supported") }
         return ret
     }
@@ -86,7 +86,7 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
         val ret = newForEachStatement(rawNode = node)
         ret.iterable = frontend.expressionHandler.handle(node.iter)
         ret.variable = frontend.expressionHandler.handle(node.target)
-        ret.statement = makeBlock(node.body)
+        ret.statement = makeBlock(node.body).codeAndLocationFromChildren(node)
         node.orelse.firstOrNull()?.let { TODO("Not supported") }
         return ret
     }
@@ -114,13 +114,13 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
         ret.condition = frontend.expressionHandler.handle(node.test)
         ret.thenStatement =
             if (node.body.isNotEmpty()) {
-                makeBlock(node.body)
+                makeBlock(node.body).codeAndLocationFromChildren(node)
             } else {
                 null
             }
         ret.elseStatement =
             if (node.orelse.isNotEmpty()) {
-                makeBlock(node.orelse)
+                makeBlock(node.orelse).codeAndLocationFromChildren(node)
             } else {
                 null
             }
@@ -330,7 +330,7 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
         // END HANDLE ARGUMENTS
 
         if (s.body.isNotEmpty()) {
-            result.body = makeBlock(s.body)
+            result.body = makeBlock(s.body).codeAndLocationFromChildren(s)
         }
 
         frontend.scopeManager.leaveScope(result)

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

@@ -726,13 +726,13 @@ class PythonFrontendTest : BaseTest() {
         val p = tu.namespaces["literal"]
         assertNotNull(p)
 
-        assertEquals(Region(1, 0, 1, 8), (p.statements[0]).location?.region)
-        assertEquals(Region(1, 4, 1, 8), (p.variables["b"])?.firstAssignment?.location?.region)
-        assertEquals(Region(2, 0, 2, 6), (p.statements[1]).location?.region)
-        assertEquals(Region(3, 0, 3, 7), (p.statements[2]).location?.region)
-        assertEquals(Region(4, 0, 4, 10), (p.statements[3]).location?.region)
-        assertEquals(Region(5, 0, 5, 11), (p.statements[4]).location?.region)
-        assertEquals(Region(6, 0, 6, 8), (p.statements[5]).location?.region)
+        assertEquals(Region(1, 1, 1, 9), (p.statements[0]).location?.region)
+        assertEquals(Region(1, 5, 1, 9), (p.variables["b"])?.firstAssignment?.location?.region)
+        assertEquals(Region(2, 1, 2, 7), (p.statements[1]).location?.region)
+        assertEquals(Region(3, 1, 3, 8), (p.statements[2]).location?.region)
+        assertEquals(Region(4, 1, 4, 11), (p.statements[3]).location?.region)
+        assertEquals(Region(5, 1, 5, 12), (p.statements[4]).location?.region)
+        assertEquals(Region(6, 1, 6, 9), (p.statements[5]).location?.region)
     }
 
     @Test
@@ -935,7 +935,6 @@ class PythonFrontendTest : BaseTest() {
     }
 
     @Test
-    @Ignore // TODO
     fun testCommentMatching() {
         val topLevel = Path.of("src", "test", "resources", "python")
         val tu =
@@ -950,7 +949,7 @@ class PythonFrontendTest : BaseTest() {
 
         val commentedNodes = SubgraphWalker.flattenAST(tu).filter { it.comment != null }
 
-        assertEquals(10, commentedNodes.size)
+        assertEquals(9, commentedNodes.size)
 
         val functions = commentedNodes.filterIsInstance<FunctionDeclaration>()
         assertEquals(1, functions.size)
@@ -968,9 +967,10 @@ class PythonFrontendTest : BaseTest() {
         assertEquals("# a parameter", params.first { it.name.localName == "i" }.comment)
         assertEquals("# another parameter", params.first { it.name.localName == "j" }.comment)
 
-        val variable = commentedNodes.filterIsInstance<VariableDeclaration>()
-        assertEquals(1, variable.size)
-        assertEquals("# A comment", variable.first().comment)
+        val assignment = commentedNodes.filterIsInstance<AssignExpression>()
+        assertEquals(2, assignment.size)
+        assertEquals("# A comment# a number", assignment.first().comment)
+        assertEquals("# comment end", assignment.last().comment)
 
         val block = commentedNodes.filterIsInstance<Block>()
         assertEquals(1, block.size)
@@ -980,14 +980,6 @@ class PythonFrontendTest : BaseTest() {
         assertEquals(2, kvs.size)
         assertEquals("# a entry", kvs.first { it.code?.contains("a") ?: false }.comment)
         assertEquals("# b entry", kvs.first { it.code?.contains("b") ?: false }.comment)
-
-        val declStmts = commentedNodes.filterIsInstance<DeclarationStatement>()
-        assertEquals(2, declStmts.size)
-        assertEquals("# a number", declStmts.first { it.location?.region?.startLine == 3 }.comment)
-        assertEquals(
-            "# comment end",
-            declStmts.first { it.location?.region?.startLine == 18 }.comment
-        )
     }
 
     @Test