De-coupling variable declaration of AST placement

Author: oxisto

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

@@ -343,47 +343,11 @@ class ScopeManager : ScopeProvider {
     /**
      * This function MUST be called when a language frontend first handles a [Declaration]. It adds
      * a declaration to the scope manager, taking into account the currently active scope.
-     * Furthermore, it adds the declaration to the [de.fraunhofer.aisec.cpg.graph.DeclarationHolder]
-     * that is associated with the current scope through [ValueDeclarationScope.addValueDeclaration]
-     * and [StructureDeclarationScope.addStructureDeclaration].
-     *
-     * Setting [Scope.astNode] to false is useful, if you want to make sure a certain declaration is
-     * visible within a scope, but is not directly part of the scope's AST. An example is the way
-     * C/C++ handles unscoped enum constants. They are visible in the enclosing scope, e.g., a
-     * translation unit, but they are added to the AST of their enum declaration, not the
-     * translation unit. The enum declaration is then added to the translation unit.
      *
      * @param declaration the declaration to add
-     * @param addToAST specifies, whether the declaration also gets added to the [Scope.astNode] of
-     *   the current scope (if it implements [DeclarationHolder]). Defaults to true.
      */
-    @JvmOverloads
-    fun addDeclaration(declaration: Declaration?, addToAST: Boolean = true) {
-        if (declaration != null) {
-            // New stuff here
-            currentScope?.addSymbol(declaration.symbol, declaration)
-        }
-
-        // Legacy stuff here
-        when (declaration) {
-            is ProblemDeclaration,
-            is IncludeDeclaration -> {
-                // directly add problems and includes to the global scope
-                this.globalScope?.addDeclaration(declaration, addToAST, this)
-            }
-            is ValueDeclaration -> {
-                val scope = this.firstScopeIsInstanceOrNull<ValueDeclarationScope>()
-                scope?.addDeclaration(declaration, addToAST, this)
-            }
-            is ImportDeclaration,
-            is EnumDeclaration,
-            is RecordDeclaration,
-            is NamespaceDeclaration,
-            is TemplateDeclaration -> {
-                val scope = this.firstScopeIsInstanceOrNull<StructureDeclarationScope>()
-                scope?.addDeclaration(declaration, addToAST, this)
-            }
-        }
+    fun addDeclaration(declaration: Declaration) {
+        currentScope?.addSymbol(declaration.symbol, declaration)
     }
 
     /**
@@ -483,11 +447,10 @@ class ScopeManager : ScopeProvider {
     }
 
     /**
-     * Adds typedefs to a [ValueDeclarationScope]. The language frontend needs to decide on the
-     * scope of the typedef. Most likely, typedefs are global. Therefore, the [GlobalScope] is set
-     * as default.
+     * Adds typedefs to a [Scope]. The language frontend needs to decide on the scope of the
+     * typedef. Most likely, typedefs are global. Therefore, the [GlobalScope] is set as default.
      */
-    fun addTypedef(typedef: TypedefDeclaration, scope: ValueDeclarationScope? = globalScope) {
+    fun addTypedef(typedef: TypedefDeclaration, scope: Scope? = globalScope) {
         scope?.addTypedef(typedef)
     }
 
@@ -684,45 +647,43 @@ class ScopeManager : ScopeProvider {
         // We need to build a path from the current scope to the top most one. This ensures us that
         // a local definition overwrites / shadows one that was there on a higher scope.
         while (current != null) {
-            if (current is ValueDeclarationScope) {
-                // This is a little bit of a hack to support partial FQN resolution at least with
-                // typedefs, but it's not really ideal.
-                // And this also should be merged with the scope manager logic when resolving names.
-                //
-                // The better approach would be to harmonize the FQN of all types in one pass before
-                // all this happens.
-                //
-                // This process has several steps:
-                // First, do a quick local lookup, to see if we have a typedef our current scope
-                // (only do this if the name is not qualified)
-                if (!alias.isQualified() && current == scope) {
-                    val decl = current.typedefs[alias]
-                    if (decl != null) {
-                        return decl.type
-                    }
+            // This is a little bit of a hack to support partial FQN resolution at least with
+            // typedefs, but it's not really ideal.
+            // And this also should be merged with the scope manager logic when resolving names.
+            //
+            // The better approach would be to harmonize the FQN of all types in one pass before
+            // all this happens.
+            //
+            // This process has several steps:
+            // First, do a quick local lookup, to see if we have a typedef our current scope
+            // (only do this if the name is not qualified)
+            if (!alias.isQualified() && current == scope) {
+                val decl = current.typedefs[alias]
+                if (decl != null) {
+                    return decl.type
                 }
+            }
 
-                // Next, try to look up the name either by its FQN (if it is qualified) or make it
-                // qualified based on the current namespace
-                val key =
-                    current.typedefs.keys.firstOrNull {
-                        var lookupName = alias
-
-                        // If the lookup name is already a FQN, we can use the name directly
-                        lookupName =
-                            if (lookupName.isQualified()) {
-                                lookupName
-                            } else {
-                                // Otherwise, we want to make an FQN out of it using the current
-                                // namespace
-                                currentNamespace?.fqn(lookupName.localName) ?: lookupName
-                            }
-
-                        it.lastPartsMatch(lookupName)
-                    }
-                if (key != null) {
-                    return current.typedefs[key]?.type
+            // Next, try to look up the name either by its FQN (if it is qualified) or make it
+            // qualified based on the current namespace
+            val key =
+                current.typedefs.keys.firstOrNull {
+                    var lookupName = alias
+
+                    // If the lookup name is already a FQN, we can use the name directly
+                    lookupName =
+                        if (lookupName.isQualified()) {
+                            lookupName
+                        } else {
+                            // Otherwise, we want to make an FQN out of it using the current
+                            // namespace
+                            currentNamespace?.fqn(lookupName.localName) ?: lookupName
+                        }
+
+                    it.lastPartsMatch(lookupName)
                 }
+            if (key != null) {
+                return current.typedefs[key]?.type
             }
 
             current = current.parent

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/DeclarationBuilder.kt

@@ -455,15 +455,10 @@ fun MetadataProvider.newImportDeclaration(
     rawNode: Any? = null,
 ): ImportDeclaration {
     val node = ImportDeclaration()
-    node.applyMetadata(this, "", rawNode)
+    node.applyMetadata(this, alias ?: import, rawNode, doNotPrependNamespace = true)
     node.import = import
     node.alias = alias
     node.style = style
-    if (alias != null) {
-        node.name = alias
-    } else {
-        node.name = import
-    }
 
     log(node)
     return node

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/builder/Fluent.kt

@@ -99,6 +99,8 @@ fun LanguageFrontend<*, *>.namespace(
     init(node)
     scopeManager.leaveScope(node)
     scopeManager.addDeclaration(node)
+    addDeclaration(node)
+
     return node
 }
 
@@ -119,6 +121,7 @@ fun LanguageFrontend<*, *>.record(
     init(node)
     scopeManager.leaveScope(node)
     scopeManager.addDeclaration(node)
+    addDeclaration(node)
 
     return node
 }
@@ -142,6 +145,7 @@ fun LanguageFrontend<*, *>.field(
     }
 
     scopeManager.addDeclaration(node)
+    addDeclaration(node)
 
     return node
 }
@@ -184,6 +188,7 @@ fun LanguageFrontend<*, *>.function(
     scopeManager.leaveScope(node)
 
     scopeManager.addDeclaration(node)
+    addDeclaration(node)
 
     return node
 }
@@ -283,6 +288,7 @@ fun LanguageFrontend<*, *>.param(
     init?.let { it(node) }
 
     scopeManager.addDeclaration(node)
+    this@FunctionDeclaration.parameters += node
 
     return node
 }
@@ -411,8 +417,7 @@ fun LanguageFrontend<*, *>.variable(
     val node = newVariableDeclaration(name, type)
     if (init != null) init(node)
 
-    declarationEdges += node
-
+    declarations += node
     scopeManager.addDeclaration(node)
 
     return node
@@ -432,8 +437,7 @@ fun LanguageFrontend<*, *>.problemDecl(
     val node = newProblemDeclaration(problem = description, problemType = type)
     if (init != null) init(node)
 
-    declarationEdges += node
-
+    declarations += node
     scopeManager.addDeclaration(node)
 
     return node
@@ -667,9 +671,9 @@ fun LanguageFrontend<*, *>.forInitializer(
 
     initializerStatement = node
 
-    if (node.isSingleDeclaration()) {
-
-        scopeManager.addDeclaration(node.singleDeclaration, false)
+    val single = node.singleDeclaration
+    if (single != null) {
+        scopeManager.addDeclaration(single)
     }
 
     return node

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/FunctionScope.kt

@@ -27,4 +27,4 @@ package de.fraunhofer.aisec.cpg.graph.scopes
 
 import de.fraunhofer.aisec.cpg.graph.declarations.FunctionDeclaration
 
-class FunctionScope(astNode: FunctionDeclaration) : ValueDeclarationScope(astNode)
+class FunctionScope(astNode: FunctionDeclaration) : Scope(astNode)

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/GlobalScope.kt

@@ -37,7 +37,7 @@ import de.fraunhofer.aisec.cpg.graph.nodes
  * scope that is restricted to a translation unit, i.e. C++ while still maintaining a unique list of
  * global variables.
  */
-class GlobalScope : StructureDeclarationScope(null) {
+class GlobalScope : Scope(null) {
 
     /**
      * Because the way we currently handle parallel parsing in [TranslationManager.parseParallel],

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/LocalScope.kt

@@ -31,4 +31,4 @@ import de.fraunhofer.aisec.cpg.graph.Node
  * Scope of validity associated to the local statement. Variables declared inside this statement are
  * not visible outside.
  */
-class LocalScope(astNode: Node) : ValueDeclarationScope(astNode) {}
+class LocalScope(astNode: Node) : Scope(astNode) {}

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/NameScope.kt

@@ -33,7 +33,7 @@ import de.fraunhofer.aisec.cpg.graph.Node
  * 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.
  */
-sealed class NameScope(node: Node?) : StructureDeclarationScope(node) {
+sealed class NameScope(node: Node?) : Scope(node) {
 
     init {
         astNode = node

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/NamespaceScope.kt

@@ -25,7 +25,6 @@
  */
 package de.fraunhofer.aisec.cpg.graph.scopes
 
-import de.fraunhofer.aisec.cpg.ScopeManager
 import de.fraunhofer.aisec.cpg.graph.declarations.Declaration
 import de.fraunhofer.aisec.cpg.graph.declarations.ImportDeclaration
 import de.fraunhofer.aisec.cpg.graph.declarations.NamespaceDeclaration
@@ -45,8 +44,8 @@ class NamespaceScope(astNode: NamespaceDeclaration) : NameScope(astNode) {
      * This is the mirror property to [Scope.importedScopeEdges]. It specifies which other [Scope]s
      * are importing this namespace.
      *
-     * This is used in [addDeclaration] to update the [ImportDeclaration.importedSymbols] once we
-     * add a new symbol here, so that is it also visible in the scope of the [ImportDeclaration].
+     * This is used in [addSymbol] to update the [ImportDeclaration.importedSymbols] once we add a
+     * new symbol here, so that is it also visible in the scope of the [ImportDeclaration].
      */
     @Relationship(value = "IMPORTS_SCOPE", direction = Relationship.Direction.INCOMING)
     val importedByEdges: Imports =
@@ -55,18 +54,12 @@ class NamespaceScope(astNode: NamespaceDeclaration) : NameScope(astNode) {
     /** Virtual property for accessing [importedScopeEdges] without property edges. */
     val importedBy: MutableSet<Scope> by unwrappingIncoming(NamespaceScope::importedByEdges)
 
-    override fun addDeclaration(
-        declaration: Declaration,
-        addToAST: Boolean,
-        scopeManager: ScopeManager,
-    ) {
-        val result = super.addDeclaration(declaration, addToAST, scopeManager)
+    override fun addSymbol(symbol: Symbol, declaration: Declaration) {
+        super.addSymbol(symbol, declaration)
 
         // Update imported symbols of dependent scopes
         for (edge in importedByEdges) {
-            edge.declaration?.let { scopeManager.updateImportedSymbols(it) }
+            edge.declaration?.let { ctx?.scopeManager?.updateImportedSymbols(it) }
         }
-
-        return result
     }
 }

cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/scopes/Scope.kt

@@ -29,9 +29,11 @@ import com.fasterxml.jackson.annotation.JsonBackReference
 import de.fraunhofer.aisec.cpg.PopulatedByPass
 import de.fraunhofer.aisec.cpg.frontends.HasImplicitReceiver
 import de.fraunhofer.aisec.cpg.frontends.Language
+import de.fraunhofer.aisec.cpg.graph.Name
 import de.fraunhofer.aisec.cpg.graph.Node
 import de.fraunhofer.aisec.cpg.graph.declarations.Declaration
 import de.fraunhofer.aisec.cpg.graph.declarations.ImportDeclaration
+import de.fraunhofer.aisec.cpg.graph.declarations.TypedefDeclaration
 import de.fraunhofer.aisec.cpg.graph.edges.scopes.Import
 import de.fraunhofer.aisec.cpg.graph.edges.scopes.ImportStyle
 import de.fraunhofer.aisec.cpg.graph.edges.scopes.Imports
@@ -114,8 +116,22 @@ sealed class Scope(
      */
     @Transient var predefinedLookupScopes: MutableMap<Symbol, LookupScopeStatement> = mutableMapOf()
 
+    /**
+     * A map of typedefs keyed by their alias name. This is still needed as a bridge until we
+     * completely redesign the alias / typedef system.
+     */
+    @Transient val typedefs = mutableMapOf<Name, TypedefDeclaration>()
+
+    /**
+     * Adds a [typedef] declaration to the scope. This is used to store typedefs in the scope, so
+     * that they can be resolved later on.
+     */
+    fun addTypedef(typedef: TypedefDeclaration) {
+        typedefs[typedef.alias.name] = typedef
+    }
+
     /** Adds a [declaration] with the defined [symbol]. */
-    fun addSymbol(symbol: Symbol, declaration: Declaration) {
+    open fun addSymbol(symbol: Symbol, declaration: Declaration) {
         if (
             declaration is ImportDeclaration &&
                 declaration.style == ImportStyle.IMPORT_ALL_SYMBOLS_FROM_NAMESPACE
@@ -229,7 +245,7 @@ sealed class Scope(
 
     override fun hashCode(): Int {
         var result = astNode?.hashCode() ?: 0
-        result = 31 * result + (name?.hashCode() ?: 0)
+        result = 31 * result + name.hashCode()
         return result
     }
 
@@ -251,19 +267,12 @@ sealed class Scope(
     override fun toString(): String {
         val builder = ToStringBuilder(this, TO_STRING_STYLE)
 
-        if (name?.isNotEmpty() == true) {
+        if (name.isNotEmpty() == true) {
             builder.append("name", name)
         }
 
         return builder.toString()
     }
-
-    fun addSymbols(other: MutableMap<Symbol, MutableSet<Declaration>>) {
-        for ((key, value) in other.entries) {
-            val list = this.symbols.computeIfAbsent(key) { mutableListOf() }
-            list += value
-        }
-    }
 }
 
 /**