First part of scope cleanup (#1856)

* First part of scope cleanup * Renamed `BlockScope` to `LocalScope` * Removed `LoopScope` * Removed `TryScope` * Removed `SwitchScope` * Added new NamespaceScope * Formatting * Cleanup of previous block functions * Fixed Go tests * More squashing --------- Co-authored-by: KuechA <[email protected]>
Fraunhofer-AISEC · Dec 2, 2024 · aad282e · aad282e
1 parent d40f4eb
commit aad282e
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 234 deletions.
diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt
@@ -79,9 +79,6 @@ class ScopeManager : ScopeProvider {
      */
     private val symbolTable = mutableMapOf<ReferenceTag, Pair<Reference, ValueDeclaration>>()
 
-    /** True, if the scope manager is currently in a [BlockScope]. */
-    val isInBlock: Boolean
-        get() = this.firstScopeOrNull { it is BlockScope } != null
     /** True, if the scope manager is currently in a [FunctionScope]. */
     val isInFunction: Boolean
         get() = this.firstScopeOrNull { it is FunctionScope } != null
@@ -92,13 +89,16 @@ class ScopeManager : ScopeProvider {
     val globalScope: GlobalScope?
         get() = scopeMap[null] as? GlobalScope
 
-    /** The current block, according to the scope that is currently active. */
-    val currentBlock: Block?
-        get() = this.firstScopeIsInstanceOrNull<BlockScope>()?.astNode as? Block
     /** The current function, according to the scope that is currently active. */
     val currentFunction: FunctionDeclaration?
         get() = this.firstScopeIsInstanceOrNull<FunctionScope>()?.astNode as? FunctionDeclaration
 
+    /** The current block, according to the scope that is currently active. */
+    val currentBlock: Block?
+        get() =
+            currentScope?.astNode as? Block
+                ?: currentScope?.astNode?.firstParentOrNull { it is Block } as? Block
+
     /**
      * The current method in the active scope tree, this ensures that 'this' keywords are mapped
      * correctly if a method contains a lambda or other types of function declarations
@@ -218,11 +218,11 @@ class ScopeManager : ScopeProvider {
      * new scope, this function needs to be called. Appropriate scopes will then be created
      * on-the-fly, if they do not exist.
      *
-     * The scope manager has an internal association between the type of scope, e.g. a [BlockScope]
+     * The scope manager has an internal association between the type of scope, e.g. a [LocalScope]
      * and the CPG node it represents, e.g. a [Block].
      *
-     * Afterwards, all calls to [addDeclaration] will be distributed to the
-     * [de.fraunhofer.aisec.cpg.graph.DeclarationHolder] that is currently in-scope.
+     * Afterward, all calls to [addDeclaration] will be distributed to the [DeclarationHolder] that
+     * is currently in-scope.
      */
     fun enterScope(nodeToScope: Node) {
         var newScope: Scope? = null
@@ -231,21 +231,21 @@ class ScopeManager : ScopeProvider {
         if (!scopeMap.containsKey(nodeToScope)) {
             newScope =
                 when (nodeToScope) {
-                    is Block -> BlockScope(nodeToScope)
                     is WhileStatement,
                     is DoStatement,
-                    is AssertStatement -> LoopScope(nodeToScope)
+                    is AssertStatement,
                     is ForStatement,
-                    is ForEachStatement -> LoopScope(nodeToScope as Statement)
-                    is SwitchStatement -> SwitchScope(nodeToScope)
+                    is ForEachStatement,
+                    is SwitchStatement,
+                    is TryStatement,
+                    is IfStatement,
+                    is CatchClause,
+                    is Block -> LocalScope(nodeToScope)
                     is FunctionDeclaration -> FunctionScope(nodeToScope)
-                    is IfStatement -> ValueDeclarationScope(nodeToScope)
-                    is CatchClause -> ValueDeclarationScope(nodeToScope)
                     is RecordDeclaration -> RecordScope(nodeToScope)
                     is TemplateDeclaration -> TemplateScope(nodeToScope)
-                    is TryStatement -> TryScope(nodeToScope)
                     is TranslationUnitDeclaration -> FileScope(nodeToScope)
-                    is NamespaceDeclaration -> newNameScopeIfNecessary(nodeToScope)
+                    is NamespaceDeclaration -> newNamespaceIfNecessary(nodeToScope)
                     else -> {
                         LOGGER.error(
                             "No known scope for AST node of type {}",
@@ -266,23 +266,23 @@ class ScopeManager : ScopeProvider {
     }
 
     /**
-     * A small internal helper function used by [enterScope] to create a [NameScope].
+     * A small internal helper function used by [enterScope] to create a [NamespaceScope].
      *
      * The issue with name scopes, such as a namespace, is that it can exist across several files,
      * i.e. translation units, represented by different [NamespaceDeclaration] nodes. But, in order
      * to make namespace resolution work across files, only one [NameScope] must exist that holds
      * all declarations, such as classes, independently of the translation units. Therefore, we need
      * to check, whether such as node already exists. If it does already exist:
      * - we update the scope map so that the current [NamespaceDeclaration] points to the existing
-     *   [NameScope]
+     *   [NamespaceScope]
      * - we return null, indicating to [enterScope], that no new scope needs to be pushed by
      *   [enterScope].
      *
-     * Otherwise, we return a new name scope.
+     * Otherwise, we return a new namespace scope.
      */
-    private fun newNameScopeIfNecessary(nodeToScope: NamespaceDeclaration): NameScope? {
+    private fun newNamespaceIfNecessary(nodeToScope: NamespaceDeclaration): NamespaceScope? {
         val existingScope =
-            filterScopes { it is NameScope && it.name == nodeToScope.name }.firstOrNull()
+            filterScopes { it is NamespaceScope && it.name == nodeToScope.name }.firstOrNull()
 
         return if (existingScope != null) {
             // update the AST node to this namespace declaration
@@ -296,25 +296,7 @@ class ScopeManager : ScopeProvider {
             // does not need to push a new scope
             null
         } else {
-            NameScope(nodeToScope)
-        }
-    }
-
-    /**
-     * Similar to [enterScope], but does so in a "read-only" mode, e.g. it does not modify the scope
-     * tree and does not create new scopes on the fly, as [enterScope] does.
-     */
-    fun enterScopeIfExists(nodeToScope: Node?) {
-        if (scopeMap.containsKey(nodeToScope)) {
-            val scope = scopeMap[nodeToScope]
-
-            // we need a special handling of name spaces, because
-            // they are associated to more than one AST node
-            if (scope is NameScope) {
-                // update AST (see enterScope for an explanation)
-                scope.astNode = nodeToScope
-            }
-            currentScope = scope
+            NamespaceScope(nodeToScope)
         }
     }
 

diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/BlockScope.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/BlockScope.kt
diff --git a/...nhofer/aisec/cpg/graph/scopes/TryScope.kt → ...ofer/aisec/cpg/graph/scopes/LocalScope.kt b/...nhofer/aisec/cpg/graph/scopes/TryScope.kt → ...ofer/aisec/cpg/graph/scopes/LocalScope.kt
@@ -26,16 +26,9 @@
 package de.fraunhofer.aisec.cpg.graph.scopes
 
 import de.fraunhofer.aisec.cpg.graph.Node
-import de.fraunhofer.aisec.cpg.graph.statements.BreakStatement
 
-class TryScope(astNode: Node?) : ValueDeclarationScope(astNode), Breakable {
-
-    private val breaks = mutableListOf<BreakStatement>()
-
-    override fun addBreakStatement(breakStatement: BreakStatement) {
-        breaks.add(breakStatement)
-    }
-
-    override val breakStatements: List<BreakStatement>
-        get() = breaks
-}
+/**
+ * Scope of validity associated to the local statement. Variables declared inside this statement are
+ * not visible outside.
+ */
+class LocalScope(astNode: Node) : ValueDeclarationScope(astNode) {}
diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/LoopScope.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/LoopScope.kt
diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/NameScope.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/NameScope.kt
@@ -29,10 +29,10 @@ import de.fraunhofer.aisec.cpg.graph.Node
 
 /**
  * A scope which acts as a namespace with a certain name, which is prefixed to all local names
- * declared in it. This could be a package or other structural elements, like a class. In the latter
- * case, the derived [RecordScope] should be used.
+ * declared in it. This could be a package or other structural elements, like a class. In the first
+ * case, the derived [NamespaceScope], in the latter case, the derived [RecordScope] should be used.
  */
-open class NameScope(node: Node?) : StructureDeclarationScope(node) {
+sealed class NameScope(node: Node?) : StructureDeclarationScope(node) {
 
     init {
         astNode = node

diff --git a/...fer/aisec/cpg/graph/scopes/SwitchScope.kt → .../aisec/cpg/graph/scopes/NamespaceScope.kt b/...fer/aisec/cpg/graph/scopes/SwitchScope.kt → .../aisec/cpg/graph/scopes/NamespaceScope.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Fraunhofer AISEC. All rights reserved.
+ * 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.
@@ -25,17 +25,11 @@
  */
 package de.fraunhofer.aisec.cpg.graph.scopes
 
-import de.fraunhofer.aisec.cpg.graph.statements.BreakStatement
-import de.fraunhofer.aisec.cpg.graph.statements.SwitchStatement
+import de.fraunhofer.aisec.cpg.graph.declarations.NamespaceDeclaration
 
-class SwitchScope(switchStatement: SwitchStatement) :
-    ValueDeclarationScope(switchStatement), Breakable {
-    private val breaks = mutableListOf<BreakStatement>()
-
-    override fun addBreakStatement(breakStatement: BreakStatement) {
-        breaks.add(breakStatement)
-    }
-
-    override val breakStatements: List<BreakStatement>
-        get() = breaks
-}
+/**
+ * This scope is opened up by a [NamespaceDeclaration] and represents the scope of the whole
+ * namespace. This scope is special in a way that it will only exist once (per [GlobalScope]) and
+ * contains all symbols declared in this namespace, even if they are spread across multiple files.
+ */
+class NamespaceScope(astNode: NamespaceDeclaration) : NameScope(astNode)
diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/Scope.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/Scope.kt
@@ -56,7 +56,7 @@ typealias SymbolMap = MutableMap<Symbol, MutableList<Declaration>>
  * restriction and can act as namespaces to avoid name collisions.
  */
 @NodeEntity
-abstract class Scope(
+sealed class Scope(
     @Relationship(value = "SCOPE", direction = Relationship.Direction.INCOMING)
     @JsonBackReference
     open var astNode: Node?
@@ -191,14 +191,6 @@ abstract class Scope(
         labelStatement.label?.let { labelStatements[it] = labelStatement }
     }
 
-    fun isBreakable(): Boolean {
-        return this is LoopScope || this is SwitchScope
-    }
-
-    fun isContinuable(): Boolean {
-        return this is LoopScope
-    }
-
     override fun equals(other: Any?): Boolean {
         if (this === other) return true
         if (javaClass != other?.javaClass) return false

diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/ScopeTraits.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/ScopeTraits.kt
diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/StructureDeclarationScope.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/StructureDeclarationScope.kt
@@ -25,12 +25,18 @@
  */
 package de.fraunhofer.aisec.cpg.graph.scopes
 
+import de.fraunhofer.aisec.cpg.ScopeManager
 import de.fraunhofer.aisec.cpg.graph.DeclarationHolder
 import de.fraunhofer.aisec.cpg.graph.Node
 import de.fraunhofer.aisec.cpg.graph.declarations.*
 
-open class StructureDeclarationScope(final override var astNode: Node?) :
-    ValueDeclarationScope(astNode) {
+/**
+ * This sealed (and abstract) class represents a [Scope] that in addition to declare variables also
+ * defines structures, such as classes, namespaces, etc.
+ *
+ * This is actually only needed because of the legacy [ScopeManager.resolve] function.
+ */
+sealed class StructureDeclarationScope(astNode: Node?) : ValueDeclarationScope(astNode) {
     val structureDeclarations: List<Declaration>
         get() {
             return symbols

diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/ValueDeclarationScope.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/ValueDeclarationScope.kt
@@ -39,7 +39,7 @@ import org.slf4j.LoggerFactory
  * Is a scope where local variables can be declared and independent of specific language constructs.
  * Works for if, for, and extends to the block scope
  */
-open class ValueDeclarationScope(override var astNode: Node?) : Scope(astNode) {
+sealed class ValueDeclarationScope(astNode: Node?) : Scope(astNode) {
     val valueDeclarations: List<ValueDeclaration>
         get() {
             return symbols.flatMap { it.value }.filterIsInstance<ValueDeclaration>()