Skip to content

Commit 7f5dcfa

Browse files
authored
Merge pull request github#9379 from tamasvajk/kotlin-android-specific-return-types
Kotlin: Change return type of Android specific `ConcurrentHashMap.keySet`
2 parents 9dd20f1 + 89ffefd commit 7f5dcfa

File tree

5 files changed

+63
-4
lines changed

5 files changed

+63
-4
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -704,16 +704,17 @@ open class KotlinFileExtractor(
704704

705705
val paramsSignature = allParamTypes.joinToString(separator = ",", prefix = "(", postfix = ")") { it.javaResult.signature!! }
706706

707-
val substReturnType = typeSubstitution?.let { it(f.returnType, TypeContext.RETURN, pluginContext) } ?: f.returnType
707+
val adjustedReturnType = getAdjustedReturnType(f)
708+
val substReturnType = typeSubstitution?.let { it(adjustedReturnType, TypeContext.RETURN, pluginContext) } ?: adjustedReturnType
708709

709710
val locId = locOverride ?: getLocation(f, classTypeArgsIncludingOuterClasses)
710711

711712
if (f.symbol is IrConstructorSymbol) {
712713
val unitType = useType(pluginContext.irBuiltIns.unitType, TypeContext.RETURN)
713714
val shortName = when {
714-
f.returnType.isAnonymous -> ""
715+
adjustedReturnType.isAnonymous -> ""
715716
typeSubstitution != null -> useType(substReturnType).javaResult.shortName
716-
else -> f.returnType.classFqName?.shortName()?.asString() ?: f.name.asString()
717+
else -> adjustedReturnType.classFqName?.shortName()?.asString() ?: f.name.asString()
717718
}
718719
val constrId = id.cast<DbConstructor>()
719720
tw.writeConstrs(constrId, shortName, "$shortName$paramsSignature", unitType.javaResult.id, parentId, sourceDeclaration.cast<DbConstructor>())

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.github.codeql
22

33
import com.github.codeql.utils.*
4+
import com.github.codeql.utils.versions.codeQlWithHasQuestionMark
45
import com.github.codeql.utils.versions.isRawType
56
import com.semmle.extractor.java.OdasaOutput
67
import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext
@@ -861,6 +862,27 @@ open class KotlinUsesExtractor(
861862
return getFunctionLabel(f, null, classTypeArgsIncludingOuterClasses)
862863
}
863864

865+
fun getAdjustedReturnType(f: IrFunction) : IrType {
866+
// The return type of `java.util.concurrent.ConcurrentHashMap<K,V>.keySet/0` is defined as `Set<K>` in the stubs inside the Android SDK.
867+
// This does not match the Java SDK return type: `ConcurrentHashMap.KeySetView<K,V>`, so it's adjusted here.
868+
// This is a deliberate change in the Android SDK: https://github.com/AndroidSDKSources/android-sdk-sources-for-api-level-31/blob/2c56b25f619575bea12f9c5520ed2259620084ac/java/util/concurrent/ConcurrentHashMap.java#L1244-L1249
869+
// The annotation on the source is not visible in the android.jar, so we can't make the change based on that.
870+
// TODO: there are other instances of `dalvik.annotation.codegen.CovariantReturnType` in the Android SDK, we should handle those too if they cause DB inconsistencies
871+
val parentClass = f.parentClassOrNull
872+
if (parentClass == null ||
873+
parentClass.fqNameWhenAvailable?.asString() != "java.util.concurrent.ConcurrentHashMap" ||
874+
getFunctionShortName(f).nameInDB != "keySet" ||
875+
f.valueParameters.isNotEmpty() ||
876+
f.returnType.classFqName?.asString() != "kotlin.collections.MutableSet") {
877+
return f.returnType
878+
}
879+
880+
val otherKeySet = parentClass.declarations.filterIsInstance<IrFunction>().find { it.name.asString() == "keySet" && it.valueParameters.size == 1 }
881+
?: return f.returnType
882+
883+
return otherKeySet.returnType.codeQlWithHasQuestionMark(false)
884+
}
885+
864886
/*
865887
* There are some pairs of classes (e.g. `kotlin.Throwable` and
866888
* `java.lang.Throwable`) which are really just 2 different names
@@ -877,7 +899,7 @@ open class KotlinUsesExtractor(
877899
maybeParentId,
878900
getFunctionShortName(f).nameInDB,
879901
f.valueParameters,
880-
f.returnType,
902+
getAdjustedReturnType(f),
881903
f.extensionReceiverParameter,
882904
getFunctionTypeParameters(f),
883905
classTypeArgsIncludingOuterClasses,
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| test.kt:4:5:6:5 | keySet | 0 | java.util.concurrent.ConcurrentHashMap$KeySetView<K,V> |
2+
| test.kt:8:5:10:5 | keySet | 1 | java.util.concurrent.ConcurrentHashMap$KeySetView<K,V> |
3+
| test.kt:17:5:19:5 | keySet | 0 | java.util.Set<K> |
4+
| test.kt:21:5:23:5 | keySet | 1 | java.util.concurrent.OtherConcurrentHashMap$KeySetView<K,V> |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import java
2+
3+
from Method m
4+
where m.fromSource()
5+
select m, m.getNumberOfParameters(), m.getReturnType().(RefType).getQualifiedName()
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package java.util.concurrent
2+
3+
class ConcurrentHashMap<K,V> {
4+
fun keySet(): MutableSet<K> {
5+
return null!!
6+
}
7+
8+
fun keySet(p: V): KeySetView<K,V>? {
9+
return null
10+
}
11+
12+
class KeySetView<K,V> {
13+
}
14+
}
15+
16+
class OtherConcurrentHashMap<K,V> {
17+
fun keySet(): MutableSet<K> {
18+
return null!!
19+
}
20+
21+
fun keySet(p: V): KeySetView<K,V>? {
22+
return null
23+
}
24+
25+
class KeySetView<K,V> {
26+
}
27+
}

0 commit comments

Comments
 (0)