Do not enter local scopes in python (#1982)

* Do not enter local scopes in python * Revert wronge change * Fixed testGlobal * Fixed testVarsAndFields * Fixed testNonLocal --------- Co-authored-by: Christian Banse <[email protected]>
Fraunhofer-AISEC · Jan 29, 2025 · 167a54d · 167a54d
1 parent 54c921a
commit 167a54d
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 33 deletions.
diff --git a/...guage-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt b/...guage-python/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/python/StatementHandler.kt
@@ -33,7 +33,7 @@ import de.fraunhofer.aisec.cpg.frontends.python.PythonLanguage.Companion.MODIFIE
 import de.fraunhofer.aisec.cpg.graph.*
 import de.fraunhofer.aisec.cpg.graph.Annotation
 import de.fraunhofer.aisec.cpg.graph.declarations.*
-import de.fraunhofer.aisec.cpg.graph.scopes.LocalScope
+import de.fraunhofer.aisec.cpg.graph.scopes.FunctionScope
 import de.fraunhofer.aisec.cpg.graph.scopes.NameScope
 import de.fraunhofer.aisec.cpg.graph.scopes.NamespaceScope
 import de.fraunhofer.aisec.cpg.graph.statements.*
@@ -891,11 +891,7 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
         handleArguments(s.args, result, recordDeclaration)
 
         if (s.body.isNotEmpty()) {
-            // Make sure we open a new (block) scope for the function body. This is not a 1:1
-            // mapping to python scopes, since python only has a "function scope", but in the CPG
-            // the function scope only comprises the function arguments, and we need a block scope
-            // to hold all local variables within the function body.
-            result.body = makeBlock(s.body, parentNode = s, enterScope = true)
+            result.body = makeBlock(s.body, parentNode = s)
         }
 
         frontend.scopeManager.leaveScope(result)
@@ -927,11 +923,10 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
      * into a [LookupScopeStatement].
      */
     private fun handleNonLocal(global: Python.AST.Nonlocal): LookupScopeStatement {
-        // We need to find the first outer function scope, or rather the block scope belonging to
-        // the function
+        // We need to find the first outer function scope
         var outerFunctionScope =
             frontend.scopeManager.firstScopeOrNull {
-                it is LocalScope && it != frontend.scopeManager.currentScope
+                it is FunctionScope && it != frontend.scopeManager.currentScope
             }
 
         return newLookupScopeStatement(
@@ -1154,29 +1149,17 @@ class StatementHandler(frontend: PythonLanguageFrontend) :
      * This function "wraps" a list of [Python.AST.BaseStmt] nodes into a [Block]. Since the list
      * itself does not have a code/location, we need to employ [codeAndLocationFromChildren] on the
      * [parentNode].
-     *
-     * Optionally, a new scope will be opened when [enterScope] is specified. This should be done
-     * VERY carefully, as Python has a very limited set of scopes and is most likely only to be used
-     * by [handleFunctionDef].
      */
     private fun makeBlock(
         stmts: List<Python.AST.BaseStmt>,
         parentNode: Python.AST.WithLocation,
-        enterScope: Boolean = false,
     ): Block {
         val result = newBlock()
-        if (enterScope) {
-            frontend.scopeManager.enterScope(result)
-        }
 
         for (stmt in stmts) {
             result.statements += handle(stmt)
         }
 
-        if (enterScope) {
-            frontend.scopeManager.leaveScope(result)
-        }
-
         // Try to retrieve the code and location from the parent node, if it is a base stmt
         val ast = parentNode as? Python.AST.AST
         if (ast != null) {

diff --git a/...age-python/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/python/PythonFrontendTest.kt b/...age-python/src/test/kotlin/de/fraunhofer/aisec/cpg/frontends/python/PythonFrontendTest.kt
@@ -742,13 +742,22 @@ class PythonFrontendTest : BaseTest() {
         assertNotNull(classFieldDeclaredInFunction)
         assertNull(classFieldNoInitializer.initializer)
 
-        val localClassFieldNoInitializer = methBar.variables["classFieldNoInitializer"]
+        val localClassFieldNoInitializer =
+            methBar.variables[
+                    { it.name.localName == "classFieldNoInitializer" && it !is FieldDeclaration }]
         assertNotNull(localClassFieldNoInitializer)
 
-        val localClassFieldWithInit = methBar.variables["classFieldWithInit"]
+        val localClassFieldWithInit =
+            methBar.variables[
+                    { it.name.localName == "classFieldWithInit" && it !is FieldDeclaration }]
         assertNotNull(localClassFieldNoInitializer)
 
-        val localClassFieldDeclaredInFunction = methBar.variables["classFieldDeclaredInFunction"]
+        val localClassFieldDeclaredInFunction =
+            methBar.variables[
+                    {
+                        it.name.localName == "classFieldDeclaredInFunction" &&
+                            it !is FieldDeclaration
+                    }]
         assertNotNull(localClassFieldNoInitializer)
 
         // classFieldNoInitializer = classFieldWithInit
@@ -1763,5 +1772,10 @@ class PythonFrontendTest : BaseTest() {
 
         // There is no field called "b" in the result.
         assertNull(tu.fields["b"])
+
+        val foo = tu.functions["foo"]
+        assertNotNull(foo)
+        val refersTo = foo.refs("fooA").map { it.refersTo }
+        refersTo.forEach { refersTo -> assertIs<ParameterDeclaration>(refersTo) }
     }
 }
diff --git a/.../kotlin/de/fraunhofer/aisec/cpg/frontends/python/statementHandler/StatementHandlerTest.kt b/.../kotlin/de/fraunhofer/aisec/cpg/frontends/python/statementHandler/StatementHandlerTest.kt
@@ -251,16 +251,10 @@ class StatementHandlerTest : BaseTest() {
         var globalC = cVariables.firstOrNull { it.scope == pythonGlobalScope }
         assertNotNull(globalC)
 
-        var localC1 =
-            cVariables.firstOrNull {
-                it.scope?.astNode?.astParent?.name?.localName == "local_write"
-            }
+        var localC1 = cVariables.firstOrNull { it.scope?.astNode?.name?.localName == "local_write" }
         assertNotNull(localC1)
 
-        var localC2 =
-            cVariables.firstOrNull {
-                it.scope?.astNode?.astParent?.name?.localName == "error_write"
-            }
+        var localC2 = cVariables.firstOrNull { it.scope?.astNode?.name?.localName == "error_write" }
         assertNotNull(localC2)
 
         // In global_write, all references should point to global c

diff --git a/cpg-language-python/src/test/resources/python/variable_inference.py b/cpg-language-python/src/test/resources/python/variable_inference.py
@@ -9,4 +9,8 @@ class SomeClass2:
     def static_method(a):
         x = a
         b = x
-        return b
+        return b
+
+def foo(fooA, b):
+    fooA = bar(fooA)
+    return fooA