Merge branch 'main' into cleanup-scopes-part1

.github/workflows/build.yml

@@ -68,11 +68,19 @@ jobs:
         id: build
         env:
           VERSION: ${{ env.version }}
+      - name: Prepare report.xml for Codecov
+        run: |
+          # this is needed because codecov incorrectly reports lines that have no coverage information (good or bad) as a miss
+          # See https://github.com/codecov/feedback/issues/564 and https://github.com/Kotlin/kotlinx-kover/issues/699.
+          # Actually these lines should just not exist in the coverage XML file, since they are only structural elements, such
+          # as brackets.
+          cat cpg-all/build/reports/kover/report.xml | grep -v 'mi="0" ci="0" mb="0" cb="0"' > cpg-all/build/reports/kover/report-codecov.xml
+          rm cpg-all/build/reports/kover/report.xml
       - name: Upload Code Coverage
         uses: codecov/codecov-action@v5
         with:
           fail_ci_if_error: true
-          files: ./cpg-all/build/reports/kover/report.xml
+          files: ./cpg-all/build/reports/kover/report-codecov.xml
           token: ${{ secrets.CODECOV_TOKEN }}
           verbose: true
       - name: Prepare test and coverage reports

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

@@ -795,8 +795,10 @@ class ScopeManager : ScopeProvider {
      *
      * @return the declaration, or null if it does not exist
      */
-    fun getRecordForName(name: Name): RecordDeclaration? {
-        return lookupSymbolByName(name).filterIsInstance<RecordDeclaration>().singleOrNull()
+    fun getRecordForName(name: Name, language: Language<*>?): RecordDeclaration? {
+        return lookupSymbolByName(name, language)
+            .filterIsInstance<RecordDeclaration>()
+            .singleOrNull()
     }
 
     fun typedefFor(alias: Name, scope: Scope? = currentScope): Type? {
@@ -856,6 +858,18 @@ class ScopeManager : ScopeProvider {
     override val scope: Scope?
         get() = currentScope
 
+    /**
+     * A convenience function to call [lookupSymbolByName] with the properties of [node]. The
+     * arguments [scope] and [predicate] are forwarded.
+     */
+    fun lookupSymbolByNameOfNode(
+        node: Node,
+        scope: Scope? = node.scope,
+        predicate: ((Declaration) -> Boolean)? = null,
+    ): List<Declaration> {
+        return lookupSymbolByName(node.name, node.language, node.location, scope, predicate)
+    }
+
     /**
      * This function tries to convert a [Node.name] into a [Symbol] and then performs a lookup of
      * this symbol. This can either be an "unqualified lookup" if [name] is not qualified or a
@@ -867,12 +881,13 @@ class ScopeManager : ScopeProvider {
      * function overloading. But it will only return list of declarations within the same scope; the
      * list cannot be spread across different scopes.
      *
-     * This means that as soon one or more declarations for the symbol are found in a "local" scope,
-     * these shadow all other occurrences of the same / symbol in a "higher" scope and only the ones
-     * from the lower ones will be returned.
+     * This means that as soon one or more declarations (of the matching [language]) for the symbol
+     * are found in a "local" scope, these shadow all other occurrences of the same / symbol in a
+     * "higher" scope and only the ones from the lower ones will be returned.
      */
     fun lookupSymbolByName(
         name: Name,
+        language: Language<*>?,
         location: PhysicalLocation? = null,
         startScope: Scope? = currentScope,
         predicate: ((Declaration) -> Boolean)? = null,
@@ -882,14 +897,25 @@ class ScopeManager : ScopeProvider {
         // We need to differentiate between a qualified and unqualified lookup. We have a qualified
         // lookup, if the scope is not null. In this case we need to stay within the specified scope
         val list =
-            if (scope != null) {
-                    scope.lookupSymbol(n.localName, thisScopeOnly = true, predicate = predicate)
-                } else {
+            when {
+                scope != null -> {
+                    scope
+                        .lookupSymbol(
+                            n.localName,
+                            languageOnly = language,
+                            thisScopeOnly = true,
+                            predicate = predicate
+                        )
+                        .toMutableList()
+                }
+                else -> {
                     // Otherwise, we can look up the symbol alone (without any FQN) starting from
                     // the startScope
-                    startScope?.lookupSymbol(n.localName, predicate = predicate)
+                    startScope
+                        ?.lookupSymbol(n.localName, languageOnly = language, predicate = predicate)
+                        ?.toMutableList() ?: mutableListOf()
                 }
-                ?.toMutableList() ?: return listOf()
+            }
 
         // If we have both the definition and the declaration of a function declaration in our list,
         // we chose only the definition
@@ -914,9 +940,15 @@ class ScopeManager : ScopeProvider {
      * It is important to know that the lookup needs to be unique, so if multiple declarations match
      * this symbol, a warning is triggered and null is returned.
      */
-    fun lookupUniqueTypeSymbolByName(name: Name, startScope: Scope?): DeclaresType? {
+    fun lookupUniqueTypeSymbolByName(
+        name: Name,
+        language: Language<*>?,
+        startScope: Scope?
+    ): DeclaresType? {
         var symbols =
-            lookupSymbolByName(name = name, startScope = startScope) { it is DeclaresType }
+            lookupSymbolByName(name = name, language = language, startScope = startScope) {
+                    it is DeclaresType
+                }
                 .filterIsInstance<DeclaresType>()
 
         // We need to have a single match, otherwise we have an ambiguous type, and we cannot

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

@@ -57,8 +57,8 @@ class TypeManager {
         MutableMap<TemplateDeclaration, MutableList<ParameterizedType>> =
         ConcurrentHashMap()
 
-    val firstOrderTypes: MutableSet<Type> = ConcurrentHashMap.newKeySet()
-    val secondOrderTypes: MutableSet<Type> = ConcurrentHashMap.newKeySet()
+    val firstOrderTypes = mutableListOf<Type>()
+    val secondOrderTypes = mutableListOf<Type>()
 
     /**
      * @param recordDeclaration that is instantiated by a template containing parameterizedtypes
@@ -200,26 +200,9 @@ class TypeManager {
         }
 
         if (t.isFirstOrderType) {
-            // Make sure we only ever return one unique object per type
-            if (!firstOrderTypes.add(t)) {
-                return firstOrderTypes.first { it == t && it is T } as T
-            } else {
-                log.trace(
-                    "Registering unique first order type {}{}",
-                    t.name,
-                    if ((t as? ObjectType)?.generics?.isNotEmpty() == true) {
-                        " with generics ${t.generics.joinToString(",", "[", "]") { it.name.toString() }}"
-                    } else {
-                        ""
-                    }
-                )
-            }
+            synchronized(firstOrderTypes) { firstOrderTypes.add(t) }
         } else if (t is SecondOrderType) {
-            if (!secondOrderTypes.add(t)) {
-                return secondOrderTypes.first { it == t && it is T } as T
-            } else {
-                log.trace("Registering unique second order type {}", t.name)
-            }
+            synchronized(secondOrderTypes) { secondOrderTypes.add(t) }
         }
 
         return t
@@ -380,5 +363,5 @@ fun Reference.nameIsType(): Type? {
     }
 
     // Lastly, check if the reference contains a symbol that points to type (declaration)
-    return scopeManager.lookupUniqueTypeSymbolByName(name, scope)?.declaredType
+    return scopeManager.lookupUniqueTypeSymbolByName(name, language, scope)?.declaredType
 }

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

@@ -299,8 +299,8 @@ fun <T : Node, AstNode> T.codeAndLocationFromOtherRawNode(rawNode: AstNode?): T
  * are between the child nodes.
  *
  * @param parentNode Used to extract the code for this node.
- * @param newLineType The char(s) used to describe a new line, usually either "\n" or "\r\n". This
- *   is needed because the location block spanning the children usually comprises more than one
+ * @param lineBreakSequence The char(s) used to describe a new line, usually either "\n" or "\r\n".
+ *   This is needed because the location block spanning the children usually comprises more than one
  *   line.
  */
 context(CodeAndLocationProvider<AstNode>)

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

@@ -113,34 +113,16 @@ fun LanguageProvider.objectType(
                 "Could not create type: translation context not available"
             )
 
-    val scope = c.scopeManager.currentScope
-
-    synchronized(c.typeManager.firstOrderTypes) {
-        // We can try to look up the type by its name and return it, if it already exists.
-        var type =
-            c.typeManager.firstOrderTypes.firstOrNull {
-                it is ObjectType &&
-                    it.name == name &&
-                    it.scope == scope &&
-                    it.generics == generics &&
-                    it.language == language
-            }
-        if (type != null) {
-            return type
-        }
-
-        // Otherwise, we either need to create the type because of the generics or because we do not
-        // know the type yet.
-        type = ObjectType(name, generics, false, language)
-        // Apply our usual metadata, such as scope, code, location, if we have any. Make sure only
-        // to refer by the local name because we will treat types as sort of references when
-        // creating them and resolve them later.
-        type.applyMetadata(this, name, rawNode = rawNode, localNameOnly = true)
-
-        // Piping it through register type will ensure that in any case we return the one unique
-        // type object (per scope) for it.
-        return c.typeManager.registerType(type)
-    }
+    // Otherwise, we either need to create the type because of the generics or because we do not
+    // know the type yet.
+    var type = ObjectType(name, generics, false, language)
+    // Apply our usual metadata, such as scope, code, location, if we have any. Make sure only
+    // to refer by the local name because we will treat types as sort of references when
+    // creating them and resolve them later.
+    type.applyMetadata(this, name, rawNode = rawNode, localNameOnly = true)
+
+    // Piping it through register type will ensure that we know the type and can resolve it later
+    return c.typeManager.registerType(type)
 }
 
 /**

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

@@ -26,6 +26,7 @@
 package de.fraunhofer.aisec.cpg.graph.scopes
 
 import com.fasterxml.jackson.annotation.JsonBackReference
+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.Node.Companion.TO_STRING_STYLE
@@ -131,6 +132,7 @@ sealed class Scope(
      */
     fun lookupSymbol(
         symbol: Symbol,
+        languageOnly: Language<*>? = null,
         thisScopeOnly: Boolean = false,
         replaceImports: Boolean = true,
         predicate: ((Declaration) -> Boolean)? = null
@@ -158,9 +160,14 @@ sealed class Scope(
                 list.replaceImports(symbol)
             }
 
+            // Filter according to the language
+            if (languageOnly != null) {
+                list.removeIf { it.language != languageOnly }
+            }
+
             // Filter the list according to the predicate, if we have any
             if (predicate != null) {
-                list = list.filter(predicate).toMutableList()
+                list.removeIf { !predicate.invoke(it) }
             }
 
             // If we have a hit, we can break the loop

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

@@ -222,7 +222,7 @@ class ImportResolver(ctx: TranslationContext) : ComponentPass(ctx) {
         for (part in parts) {
             var namespaces =
                 scopeManager
-                    .lookupSymbolByName(part, import.location, import.scope)
+                    .lookupSymbolByName(part, import.language, import.location, import.scope)
                     .filterIsInstance<NamespaceDeclaration>()
 
             // We are only interested in "leaf" namespace declarations, meaning that they do not
@@ -271,7 +271,13 @@ class ImportResolver(ctx: TranslationContext) : ComponentPass(ctx) {
 
         // Let's do some importing. We need to import either a wildcard
         if (import.wildcardImport) {
-            val list = scopeManager.lookupSymbolByName(import.import, import.location, scope)
+            val list =
+                scopeManager.lookupSymbolByName(
+                    import.import,
+                    import.language,
+                    import.location,
+                    scope
+                )
             val symbol = list.singleOrNull()
             if (symbol != null) {
                 // In this case, the symbol must point to a name scope
@@ -284,7 +290,7 @@ class ImportResolver(ctx: TranslationContext) : ComponentPass(ctx) {
             // or a symbol directly
             val list =
                 scopeManager
-                    .lookupSymbolByName(import.import, import.location, scope)
+                    .lookupSymbolByName(import.import, import.language, import.location, scope)
                     .toMutableList()
             import.importedSymbols = mutableMapOf(import.symbol to list)
         }

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

@@ -204,7 +204,7 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
 
         // Find a list of candidate symbols. Currently, this is only used the in the "next-gen" call
         // resolution, but in future this will also be used in resolving regular references.
-        ref.candidates = scopeManager.lookupSymbolByName(ref.name, ref.location).toSet()
+        ref.candidates = scopeManager.lookupSymbolByNameOfNode(ref).toSet()
 
         // Preparation for a future without legacy call resolving. Taking the first candidate is not
         // ideal since we are running into an issue with function pointers here (see workaround
@@ -590,7 +590,9 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
         var candidates = mutableSetOf<Declaration>()
         val records = possibleContainingTypes.mapNotNull { it.root.recordDeclaration }.toSet()
         for (record in records) {
-            candidates.addAll(ctx.scopeManager.lookupSymbolByName(record.name.fqn(symbol)))
+            candidates.addAll(
+                ctx.scopeManager.lookupSymbolByName(record.name.fqn(symbol), record.language)
+            )
         }
 
         // Find invokes by supertypes
@@ -700,7 +702,11 @@ open class SymbolResolver(ctx: TranslationContext) : ComponentPass(ctx) {
             listOf()
         } else {
             val firstLevelCandidates =
-                possibleTypes.map { scopeManager.lookupSymbolByName(it.name.fqn(name)) }.flatten()
+                possibleTypes
+                    .map { record ->
+                        scopeManager.lookupSymbolByName(record.name.fqn(name), record.language)
+                    }
+                    .flatten()
 
             // C++ does not allow overloading at different hierarchy levels. If we find a
             // FunctionDeclaration with the same name as the function in the CallExpression we have

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

@@ -104,7 +104,8 @@ open class TypeResolver(ctx: TranslationContext) : ComponentPass(ctx) {
         // filter for nodes that implement DeclaresType, because otherwise we will get a lot of
         // constructor declarations and such with the same name. It seems this is ok since most
         // languages will prefer structs/classes over functions when resolving types.
-        var declares = scopeManager.lookupUniqueTypeSymbolByName(type.name, type.scope)
+        var declares =
+            scopeManager.lookupUniqueTypeSymbolByName(type.name, type.language, type.scope)
 
         // If we did not find any declaration, we can try to infer a record declaration for it
         if (declares == null) {