Kotlin extractor: fix K2 regressions without expected churn

CODEOWNERS

@@ -28,7 +28,6 @@
 /swift/extractor/ @github/codeql-swift @github/code-scanning-language-coverage
 /misc/codegen/ @github/codeql-swift
 /java/kotlin-extractor/ @github/codeql-kotlin @github/code-scanning-language-coverage
-/java/ql/test-kotlin1/ @github/codeql-kotlin
 /java/ql/test-kotlin2/ @github/codeql-kotlin
 
 # Experimental CodeQL cryptography

java/kotlin-extractor/dev/wrapper.py

@@ -75,6 +75,9 @@ def get_version():
 
 
 def install(version: str, quiet: bool):
+    if install_dir.exists():
+        return
+
     if quiet:
         info_out = subprocess.DEVNULL
         info = lambda *args: None
@@ -83,8 +86,6 @@ def install(version: str, quiet: bool):
         info = lambda *args: print(*args, file=sys.stderr)
     file = file_template.format(version=version)
     url = url_template.format(version=version)
-    if install_dir.exists():
-        shutil.rmtree(install_dir)
     install_dir.mkdir()
     zips_dir.mkdir(exist_ok=True)
     zip = zips_dir / file
@@ -156,8 +157,11 @@ def main(opts, forwarded_opts):
         selected_version = current_version or DEFAULT_VERSION
     if selected_version != current_version:
         # don't print information about install procedure unless explicitly using --select
-        install(selected_version, quiet=opts.select is None)
+        if install_dir.exists():
+            shutil.rmtree(install_dir)
         version_file.write_text(selected_version)
+    # don't print information about install procedure unless explicitly using --select
+    install(selected_version, quiet=opts.select is None)
     if opts.select and not forwarded_opts and not opts.version:
         print(f"selected {selected_version}")
         return

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

@@ -6,6 +6,8 @@ import com.github.codeql.utils.*
 import com.github.codeql.utils.versions.*
 import com.semmle.extractor.java.OdasaOutput
 import java.io.Closeable
+import java.nio.file.Files
+import java.nio.file.Path
 import java.util.*
 import kotlin.collections.ArrayList
 import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext
@@ -50,6 +52,7 @@ import org.jetbrains.kotlin.load.java.structure.JavaMethod
 import org.jetbrains.kotlin.load.java.structure.JavaTypeParameter
 import org.jetbrains.kotlin.load.java.structure.JavaTypeParameterListOwner
 import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass
+import org.jetbrains.kotlin.fir.java.VirtualFileBasedSourceElement
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.types.Variance
 import org.jetbrains.kotlin.util.OperatorNameConventions
@@ -161,23 +164,100 @@ open class KotlinFileExtractor(
         }
     }
 
-    private fun javaBinaryDeclaresMethod(c: IrClass, name: String) =
-        ((c.source as? JavaSourceElement)?.javaElement as? BinaryJavaClass)?.methods?.any {
-            it.name.asString() == name
+    private fun javaBinaryDeclaresMethod(c: IrClass, name: String): Boolean? {
+        // K1 path: source is JavaSourceElement wrapping a BinaryJavaClass - inspect class metadata
+        val binaryJavaClass = (c.source as? JavaSourceElement)?.javaElement as? BinaryJavaClass
+        if (binaryJavaClass != null) {
+            return binaryJavaClass.methods.any { it.name.asString() == name }
+        }
+
+        // K2 path: binary Java classes use VirtualFileBasedSourceElement instead of
+        // JavaSourceElement. The BinaryJavaClass is not stored in the source element, so we parse
+        // the class bytes directly using ASM to check if the method is explicitly declared.
+        if (c.source is VirtualFileBasedSourceElement) {
+            val virtualFile = (c.source as VirtualFileBasedSourceElement).virtualFile
+            if (!virtualFile.name.endsWith(".class")) return null
+            return try {
+                val bytes = virtualFile.contentsToByteArray()
+                var found = false
+                var hasKotlinMetadata = false
+                val reader = org.jetbrains.org.objectweb.asm.ClassReader(bytes)
+                reader.accept(
+                    object : org.jetbrains.org.objectweb.asm.ClassVisitor(
+                        org.jetbrains.org.objectweb.asm.Opcodes.ASM9
+                    ) {
+                        override fun visitAnnotation(
+                            descriptor: String,
+                            visible: Boolean
+                        ): org.jetbrains.org.objectweb.asm.AnnotationVisitor? {
+                            if (descriptor == "Lkotlin/Metadata;") hasKotlinMetadata = true
+                            return null
+                        }
+
+                        override fun visitMethod(
+                            access: Int,
+                            methodName: String,
+                            descriptor: String,
+                            signature: String?,
+                            exceptions: Array<String>?
+                        ): org.jetbrains.org.objectweb.asm.MethodVisitor? {
+                            if (methodName == name) found = true
+                            return null
+                        }
+                    },
+                    org.jetbrains.org.objectweb.asm.ClassReader.SKIP_CODE or
+                        org.jetbrains.org.objectweb.asm.ClassReader.SKIP_DEBUG or
+                        org.jetbrains.org.objectweb.asm.ClassReader.SKIP_FRAMES
+                )
+                if (hasKotlinMetadata) false else found
+            } catch (e: Exception) {
+                logger.warn("Failed to check binary class methods for ${c.fqNameWhenAvailable}: $e")
+                null
+            }
         }
+        return null
+    }
 
     private fun isJavaBinaryDeclaration(f: IrFunction) =
         f.parentClassOrNull?.let { javaBinaryDeclaresMethod(it, f.name.asString()) } ?: false
 
+    private fun hasConcreteSiblingObjectMethod(f: IrFunction): Boolean {
+        val parentClass = f.parentClassOrNull ?: return false
+        return parentClass.declarations
+            .asSequence()
+            .filterIsInstance<IrFunction>()
+            .filter { sibling ->
+                sibling !== f &&
+                    sibling.name == f.name &&
+                    sibling.codeQlValueParameters.size == f.codeQlValueParameters.size
+            }
+            .any { sibling ->
+                val hasInvisibleFakeVisibility =
+                    sibling.visibility.let {
+                        it is DelegatedDescriptorVisibility && it.delegate == Visibilities.InvisibleFake
+                    }
+                !sibling.isFakeOverride && !hasInvisibleFakeVisibility
+            }
+    }
+
     private fun isJavaBinaryObjectMethodRedeclaration(d: IrDeclaration) =
         when (d) {
             is IrFunction ->
+                d.parentClassOrNull?.typeParameters?.isEmpty() == true &&
                 when (d.name.asString()) {
                     "toString" -> d.codeQlValueParameters.isEmpty()
                     "hashCode" -> d.codeQlValueParameters.isEmpty()
-                    "equals" -> d.codeQlValueParameters.singleOrNull()?.type?.isNullableAny() ?: false
+                    // Under K2 (language version 2.0+), the Object.equals(Object) parameter is
+                    // typed as Any (non-nullable) rather than Any? (nullable). Accept both.
+                    "equals" ->
+                        d.codeQlValueParameters
+                            .singleOrNull()
+                            ?.type
+                            ?.let { it.isNullableAny() || it.isAny() } ?: false
                     else -> false
-                } && isJavaBinaryDeclaration(d)
+                } &&
+                    !hasConcreteSiblingObjectMethod(d) &&
+                    isJavaBinaryDeclaration(d)
             else -> false
         }
 
@@ -1312,27 +1392,28 @@ open class KotlinFileExtractor(
     ): TypeResults {
         with("value parameter", vp) {
             val location = locOverride ?: getLocation(vp, classTypeArgsIncludingOuterClasses)
+            val parentFunction = vp.parent as? IrFunction
+            val javaCallable = parentFunction?.let { getJavaCallable(it) }
             val maybeAlteredType =
-                (vp.parent as? IrFunction)?.let {
+                parentFunction?.let {
                     if (overridesCollectionsMethodWithAlteredParameterTypes(it))
                         eraseCollectionsMethodParameterType(vp.type, it.name.asString(), idx)
                     else if (
-                        (vp.parent as? IrConstructor)?.parentClassOrNull?.kind ==
+                        (parentFunction as? IrConstructor)?.parentClassOrNull?.kind ==
                             ClassKind.ANNOTATION_CLASS
                     )
                         kClassToJavaClass(vp.type)
                     else null
                 } ?: vp.type
-            val javaType =
-                (vp.parent as? IrFunction)?.let {
-                    getJavaCallable(it)?.let { jCallable ->
-                        getJavaValueParameterType(jCallable, idx)
-                    }
-                }
+            val javaType = javaCallable?.let { jCallable -> getJavaValueParameterType(jCallable, idx) }
+            val addParameterWildcardsByDefault =
+                !getInnermostWildcardSupppressionAnnotation(vp) &&
+                    !(javaCallable == null &&
+                        parentFunction?.origin == IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB)
             val typeWithWildcards =
                 addJavaLoweringWildcards(
                     maybeAlteredType,
-                    !getInnermostWildcardSupppressionAnnotation(vp),
+                    addParameterWildcardsByDefault,
                     javaType
                 )
             val substitutedType =
@@ -1346,9 +1427,9 @@ open class KotlinFileExtractor(
                 vp.origin == IrDeclarationOrigin.UNDERSCORE_PARAMETER ||
                     ((vp.parent as? IrFunction)?.let { hasSynthesizedParameterNames(it) } ?: true)
             val javaParameter =
-                when (val callable = (vp.parent as? IrFunction)?.let { getJavaCallable(it) }) {
-                    is JavaConstructor -> callable.valueParameters.getOrNull(idx)
-                    is JavaMethod -> callable.valueParameters.getOrNull(idx)
+                when (javaCallable) {
+                    is JavaConstructor -> javaCallable.valueParameters.getOrNull(idx)
+                    is JavaMethod -> javaCallable.valueParameters.getOrNull(idx)
                     else -> null
                 }
             val extraAnnotations =
@@ -2874,6 +2955,45 @@ open class KotlinFileExtractor(
         return v
     }
 
+    private val sourceTextCache = mutableMapOf<String, String?>()
+
+    private fun getCurrentFileSourceText() =
+        sourceTextCache.getOrPut(filePath) {
+            runCatching { Files.readString(Path.of(filePath)) }.getOrNull()
+        }
+
+    private fun getVariableNameLocation(v: IrVariable): Label<DbLocation>? {
+        if (v.startOffset < 0 || v.endOffset < v.startOffset) return null
+
+        val source = getCurrentFileSourceText() ?: return null
+        if (v.startOffset >= source.length) return null
+
+        val name = v.name.asString()
+        if (name.isEmpty()) return null
+
+        val endExclusive = minOf(v.endOffset + 1, source.length)
+        val declarationText = source.substring(v.startOffset, endExclusive)
+        val nameOffsetInDeclaration = declarationText.indexOf(name)
+        if (nameOffsetInDeclaration < 0) return null
+
+        val nameStartOffset = v.startOffset + nameOffsetInDeclaration
+        val nameEndOffset = nameStartOffset + name.length - 1
+        return tw.getLocation(nameStartOffset, nameEndOffset)
+    }
+
+    private fun shouldUseVariableNameLocation(v: IrVariable): Boolean {
+        val initializer = v.initializer
+        return initializer is IrTypeOperatorCall && initializer.operator == IrTypeOperator.IMPLICIT_NOTNULL
+    }
+
+    private fun getVariableLocation(v: IrVariable): Label<DbLocation> {
+        if (shouldUseVariableNameLocation(v)) {
+            val nameLocation = getVariableNameLocation(v)
+            if (nameLocation != null) return nameLocation
+        }
+        return tw.getLocation(getVariableLocationProvider(v))
+    }
+
     private fun extractVariable(
         v: IrVariable,
         callable: Label<out DbCallable>,
@@ -2882,7 +3002,7 @@ open class KotlinFileExtractor(
     ) {
         with("variable", v) {
             val stmtId = tw.getFreshIdLabel<DbLocalvariabledeclstmt>()
-            val locId = tw.getLocation(getVariableLocationProvider(v))
+            val locId = getVariableLocation(v)
             tw.writeStmts_localvariabledeclstmt(stmtId, parent, idx, callable)
             tw.writeHasLocation(stmtId, locId)
             extractVariableExpr(v, callable, stmtId, 1, stmtId)
@@ -2900,7 +3020,7 @@ open class KotlinFileExtractor(
         with("variable expr", v) {
             val varId = useVariable(v)
             val exprId = tw.getFreshIdLabel<DbLocalvariabledeclexpr>()
-            val locId = tw.getLocation(getVariableLocationProvider(v))
+            val locId = getVariableLocation(v)
             val type = useType(v.type)
             tw.writeLocalvars(varId, v.name.asString(), type.javaResult.id, exprId)
             tw.writeLocalvarsKotlinType(varId, type.kotlinResult.id)
@@ -4066,6 +4186,28 @@ open class KotlinFileExtractor(
             else -> false
         }
 
+    private fun getCallResultType(c: IrCall, syntacticCallTarget: IrFunction): IrType {
+        if (syntacticCallTarget.origin != IrDeclarationOrigin.IR_EXTERNAL_JAVA_DECLARATION_STUB) {
+            return c.type
+        }
+
+        val primitiveInfo =
+            (c.type as? IrSimpleType)?.let { primitiveTypeMapping.getPrimitiveInfo(it) } ?: return c.type
+        val parentClass = syntacticCallTarget.parentClassOrNull ?: return c.type
+        val returnIsClassifier =
+            javaBinaryMethodReturnIsClassifierType(
+                parentClass,
+                getFunctionShortName(syntacticCallTarget).nameInDB,
+                syntacticCallTarget.codeQlValueParameters.size,
+                syntacticCallTarget is IrConstructor
+            )
+        return if (returnIsClassifier == true) {
+            primitiveInfo.javaClass.symbol.typeWith()
+        } else {
+            c.type
+        }
+    }
+
     private fun isGenericArrayType(typeName: String) =
         when (typeName) {
             "Array" -> true
@@ -4111,7 +4253,7 @@ open class KotlinFileExtractor(
                 extractRawMethodAccess(
                     syntacticCallTarget,
                     c,
-                    c.type,
+                    getCallResultType(c, syntacticCallTarget),
                     callable,
                     parent,
                     idx,