Skip to content

Commit 05fcbdd

Browse files
authored
Merge pull request #10365 from tamasvajk/kotlin-fix-isUnspecialised-2
Kotlin: Fix `isUnspecialised` to handle generic classes inside generic methods
2 parents 89a331f + b8b0fd8 commit 05fcbdd

14 files changed

+312
-10
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1455,7 +1455,7 @@ open class KotlinFileExtractor(
14551455
extractNewExprForLocalFunction(ids, id, locId, enclosingCallable, enclosingStmt)
14561456
} else {
14571457
val methodId =
1458-
if (extractClassTypeArguments && drType is IrSimpleType && !isUnspecialised(drType)) {
1458+
if (extractClassTypeArguments && drType is IrSimpleType && !isUnspecialised(drType, logger)) {
14591459

14601460
val extractionMethod = if (isFunctionInvoke) {
14611461
// For `kotlin.FunctionX` and `kotlin.reflect.KFunctionX` interfaces, we're making sure that we

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ open class KotlinUsesExtractor(
220220
val extractedTypeArgs = when {
221221
extractClass.symbol.isKFunction() && typeArgs != null && typeArgs.isNotEmpty() -> listOf(typeArgs.last())
222222
extractClass.fqNameWhenAvailable == FqName("kotlin.jvm.functions.FunctionN") && typeArgs != null && typeArgs.isNotEmpty() -> listOf(typeArgs.last())
223-
typeArgs != null && isUnspecialised(c, typeArgs) -> listOf()
223+
typeArgs != null && isUnspecialised(c, typeArgs, logger) -> listOf()
224224
else -> typeArgs
225225
}
226226

@@ -479,7 +479,7 @@ open class KotlinUsesExtractor(
479479
fun useSimpleTypeClass(c: IrClass, args: List<IrTypeArgument>?, hasQuestionMark: Boolean): TypeResults {
480480
if (c.isAnonymousObject) {
481481
args?.let {
482-
if (it.isNotEmpty() && !isUnspecialised(c, it)) {
482+
if (it.isNotEmpty() && !isUnspecialised(c, it, logger)) {
483483
logger.error("Unexpected specialised instance of generic anonymous class")
484484
}
485485
}

java/kotlin-extractor/src/main/kotlin/utils/TypeSubstitution.kt

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

33
import com.github.codeql.KotlinUsesExtractor
4+
import com.github.codeql.Logger
45
import com.github.codeql.getJavaEquivalentClassId
56
import com.github.codeql.utils.versions.codeQlWithHasQuestionMark
67
import com.github.codeql.utils.versions.createImplicitParameterDeclarationWithWrappedDescriptor
@@ -25,6 +26,7 @@ import org.jetbrains.kotlin.ir.types.impl.makeTypeProjection
2526
import org.jetbrains.kotlin.ir.util.classId
2627
import org.jetbrains.kotlin.ir.util.constructedClassType
2728
import org.jetbrains.kotlin.ir.util.constructors
29+
import org.jetbrains.kotlin.ir.util.kotlinFqName
2830
import org.jetbrains.kotlin.name.FqName
2931
import org.jetbrains.kotlin.name.Name
3032
import org.jetbrains.kotlin.types.Variance
@@ -215,27 +217,34 @@ private fun matchingTypeParameters(l: IrTypeParameter?, r: IrTypeParameter): Boo
215217
}
216218

217219
// Returns true if type is C<T1, T2, ...> where C is declared `class C<T1, T2, ...> { ... }`
218-
fun isUnspecialised(paramsContainer: IrTypeParametersContainer, args: List<IrTypeArgument>): Boolean {
220+
fun isUnspecialised(paramsContainer: IrTypeParametersContainer, args: List<IrTypeArgument>, logger: Logger): Boolean {
221+
return isUnspecialised(paramsContainer, args, logger, paramsContainer)
222+
}
223+
224+
private fun isUnspecialised(paramsContainer: IrTypeParametersContainer, args: List<IrTypeArgument>, logger: Logger, origParamsContainer: IrTypeParametersContainer): Boolean {
219225
val unspecialisedHere = paramsContainer.typeParameters.zip(args).all { paramAndArg ->
220226
(paramAndArg.second as? IrTypeProjection)?.let {
221227
// Type arg refers to the class' own type parameter?
222228
it.variance == Variance.INVARIANT &&
223-
matchingTypeParameters(it.type.classifierOrNull?.owner as? IrTypeParameter, paramAndArg.first)
229+
matchingTypeParameters(it.type.classifierOrNull?.owner as? IrTypeParameter, paramAndArg.first)
224230
} ?: false
225231
}
226232
val remainingArgs = args.drop(paramsContainer.typeParameters.size)
227233

228-
val parentClass = paramsContainer.parents.firstIsInstanceOrNull<IrClass>()
234+
val parentTypeContainer = paramsContainer.parents.firstIsInstanceOrNull<IrTypeParametersContainer>()
229235

230236
val parentUnspecialised = when {
231237
remainingArgs.isEmpty() -> true
232-
parentClass == null -> false
233-
else -> isUnspecialised(parentClass, remainingArgs)
238+
parentTypeContainer == null -> {
239+
logger.error("Found more type arguments than parameters: ${origParamsContainer.kotlinFqName.asString()}")
240+
false
241+
}
242+
else -> isUnspecialised(parentTypeContainer, remainingArgs, logger, origParamsContainer)
234243
}
235244
return unspecialisedHere && parentUnspecialised
236245
}
237246

238247
// Returns true if type is C<T1, T2, ...> where C is declared `class C<T1, T2, ...> { ... }`
239-
fun isUnspecialised(type: IrSimpleType) = (type.classifier.owner as? IrClass)?.let {
240-
isUnspecialised(it, type.arguments)
248+
fun isUnspecialised(type: IrSimpleType, logger: Logger) = (type.classifier.owner as? IrClass)?.let {
249+
isUnspecialised(it, type.arguments, logger)
241250
} ?: false

java/ql/test/kotlin/library-tests/classes/PrintAst.expected

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,66 @@ classes.kt:
498498
# 127| 1: [ExprStmt] <Expr>;
499499
# 127| 0: [ClassInstanceExpr] new (...)
500500
# 127| -3: [TypeAccess] Object
501+
# 138| 25: [Class,GenericType,ParameterizedType] Cl0
502+
#-----| -2: (Generic Parameters)
503+
# 138| 0: [TypeVariable] U0
504+
# 138| 1: [Constructor] Cl0
505+
# 138| 5: [BlockStmt] { ... }
506+
# 138| 0: [SuperConstructorInvocationStmt] super(...)
507+
# 138| 1: [BlockStmt] { ... }
508+
# 139| 2: [Method] func1
509+
#-----| 2: (Generic Parameters)
510+
# 139| 0: [TypeVariable] U1
511+
# 139| 3: [TypeAccess] Unit
512+
# 139| 5: [BlockStmt] { ... }
513+
# 140| 0: [LocalTypeDeclStmt] class ...
514+
# 140| 0: [LocalClass]
515+
# 140| 1: [Constructor]
516+
# 140| 5: [BlockStmt] { ... }
517+
# 140| 0: [SuperConstructorInvocationStmt] super(...)
518+
# 140| 2: [Method] func2
519+
#-----| 2: (Generic Parameters)
520+
# 140| 0: [TypeVariable] U2
521+
# 140| 3: [TypeAccess] Unit
522+
# 140| 5: [BlockStmt] { ... }
523+
# 141| 0: [LocalTypeDeclStmt] class ...
524+
# 141| 0: [Class,GenericType,LocalClass,ParameterizedType] Cl1
525+
#-----| -2: (Generic Parameters)
526+
# 141| 0: [TypeVariable] U3
527+
# 141| 1: [Constructor] Cl1
528+
# 141| 5: [BlockStmt] { ... }
529+
# 141| 0: [SuperConstructorInvocationStmt] super(...)
530+
# 141| 1: [BlockStmt] { ... }
531+
# 142| 2: [Method] fn
532+
# 142| 3: [TypeAccess] Unit
533+
# 142| 5: [BlockStmt] { ... }
534+
# 143| 0: [LocalVariableDeclStmt] var ...;
535+
# 143| 1: [LocalVariableDeclExpr] x
536+
# 143| 0: [ClassInstanceExpr] new Cl1(...)
537+
# 143| -3: [TypeAccess] Cl1
538+
# 143| 0: [TypeAccess] U3
539+
# 150| 26: [Class,GenericType,ParameterizedType] Cl00
540+
#-----| -2: (Generic Parameters)
541+
# 150| 0: [TypeVariable] U0
542+
# 150| 1: [Constructor] Cl00
543+
# 150| 5: [BlockStmt] { ... }
544+
# 150| 0: [SuperConstructorInvocationStmt] super(...)
545+
# 150| 1: [BlockStmt] { ... }
546+
# 151| 2: [Class,GenericType,ParameterizedType] Cl01
547+
#-----| -2: (Generic Parameters)
548+
# 151| 0: [TypeVariable] U1
549+
# 151| 1: [Constructor] Cl01
550+
# 151| 5: [BlockStmt] { ... }
551+
# 151| 0: [SuperConstructorInvocationStmt] super(...)
552+
# 151| 1: [BlockStmt] { ... }
553+
# 152| 2: [Method] fn
554+
# 152| 3: [TypeAccess] Unit
555+
# 152| 5: [BlockStmt] { ... }
556+
# 153| 0: [LocalVariableDeclStmt] var ...;
557+
# 153| 1: [LocalVariableDeclExpr] x
558+
# 153| 0: [ClassInstanceExpr] new Cl01(...)
559+
# 153| -3: [TypeAccess] Cl01
560+
# 153| 0: [TypeAccess] U1
501561
generic_anonymous.kt:
502562
# 0| [CompilationUnit] generic_anonymous
503563
# 0| 1: [Class] Generic_anonymousKt
@@ -592,6 +652,114 @@ generic_anonymous.kt:
592652
# 7| 0: [MethodAccess] getMember(...)
593653
# 7| -1: [MethodAccess] getX$private(...)
594654
# 7| -1: [ThisAccess] this
655+
# 15| 3: [Class,GenericType,ParameterizedType] Outer
656+
#-----| -2: (Generic Parameters)
657+
# 15| 0: [TypeVariable] T0
658+
# 15| 6: [Constructor] Outer
659+
# 15| 5: [BlockStmt] { ... }
660+
# 15| 0: [SuperConstructorInvocationStmt] super(...)
661+
# 15| 1: [BlockStmt] { ... }
662+
# 16| 7: [GenericType,Interface,ParameterizedType] C0
663+
#-----| -2: (Generic Parameters)
664+
# 16| 0: [TypeVariable] T0
665+
# 17| 1: [Method] fn0
666+
# 17| 3: [TypeAccess] T0
667+
# 17| 5: [BlockStmt] { ... }
668+
# 17| 0: [ReturnStmt] return ...
669+
# 17| 0: [NullLiteral] null
670+
# 20| 8: [GenericType,Interface,ParameterizedType] C1
671+
#-----| -2: (Generic Parameters)
672+
# 20| 0: [TypeVariable] T1
673+
# 21| 1: [Method] fn1
674+
# 21| 3: [TypeAccess] T1
675+
# 21| 5: [BlockStmt] { ... }
676+
# 21| 0: [ReturnStmt] return ...
677+
# 21| 0: [NullLiteral] null
678+
# 24| 9: [Method] func1
679+
#-----| 2: (Generic Parameters)
680+
# 24| 0: [TypeVariable] U2
681+
# 24| 3: [TypeAccess] Unit
682+
# 24| 5: [BlockStmt] { ... }
683+
# 25| 0: [LocalTypeDeclStmt] class ...
684+
# 25| 0: [LocalClass]
685+
# 25| 1: [Constructor]
686+
# 25| 5: [BlockStmt] { ... }
687+
# 25| 0: [SuperConstructorInvocationStmt] super(...)
688+
# 25| 2: [Method] func2
689+
#-----| 2: (Generic Parameters)
690+
# 25| 0: [TypeVariable] U3
691+
# 25| 3: [TypeAccess] Unit
692+
# 25| 5: [BlockStmt] { ... }
693+
# 26| 0: [ExprStmt] <Expr>;
694+
# 26| 0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
695+
# 26| 0: [TypeAccess] Unit
696+
# 26| 1: [StmtExpr] <Stmt>
697+
# 26| 0: [BlockStmt] { ... }
698+
# 26| 0: [LocalTypeDeclStmt] class ...
699+
# 26| 0: [AnonymousClass,LocalClass] new Object(...) { ... }
700+
# 26| 1: [Constructor]
701+
# 26| 5: [BlockStmt] { ... }
702+
# 26| 0: [SuperConstructorInvocationStmt] super(...)
703+
# 26| 1: [BlockStmt] { ... }
704+
# 26| 1: [ExprStmt] <Expr>;
705+
# 26| 0: [ClassInstanceExpr] new (...)
706+
# 26| -3: [TypeAccess] Object
707+
# 27| 1: [ExprStmt] <Expr>;
708+
# 27| 0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
709+
# 27| 0: [TypeAccess] Unit
710+
# 27| 1: [StmtExpr] <Stmt>
711+
# 27| 0: [BlockStmt] { ... }
712+
# 27| 0: [LocalTypeDeclStmt] class ...
713+
# 27| 0: [AnonymousClass,LocalClass] new Object(...) { ... }
714+
# 27| 1: [Constructor]
715+
# 27| 5: [BlockStmt] { ... }
716+
# 27| 0: [SuperConstructorInvocationStmt] super(...)
717+
# 27| 1: [BlockStmt] { ... }
718+
# 27| 1: [ExprStmt] <Expr>;
719+
# 27| 0: [ClassInstanceExpr] new (...)
720+
# 27| -3: [TypeAccess] Object
721+
# 28| 2: [ExprStmt] <Expr>;
722+
# 28| 0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
723+
# 28| 0: [TypeAccess] Unit
724+
# 28| 1: [StmtExpr] <Stmt>
725+
# 28| 0: [BlockStmt] { ... }
726+
# 28| 0: [LocalTypeDeclStmt] class ...
727+
# 28| 0: [AnonymousClass,LocalClass] new Object(...) { ... }
728+
# 28| 1: [Constructor]
729+
# 28| 5: [BlockStmt] { ... }
730+
# 28| 0: [SuperConstructorInvocationStmt] super(...)
731+
# 28| 1: [BlockStmt] { ... }
732+
# 28| 1: [ExprStmt] <Expr>;
733+
# 28| 0: [ClassInstanceExpr] new (...)
734+
# 28| -3: [TypeAccess] Object
735+
# 29| 3: [ExprStmt] <Expr>;
736+
# 29| 0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
737+
# 29| 0: [TypeAccess] Unit
738+
# 29| 1: [StmtExpr] <Stmt>
739+
# 29| 0: [BlockStmt] { ... }
740+
# 29| 0: [LocalTypeDeclStmt] class ...
741+
# 29| 0: [AnonymousClass,LocalClass] new C0<U2>(...) { ... }
742+
# 29| 1: [Constructor]
743+
# 29| 5: [BlockStmt] { ... }
744+
# 29| 0: [SuperConstructorInvocationStmt] super(...)
745+
# 29| 1: [BlockStmt] { ... }
746+
# 29| 1: [ExprStmt] <Expr>;
747+
# 29| 0: [ClassInstanceExpr] new (...)
748+
# 29| -3: [TypeAccess] C0<U2>
749+
# 30| 4: [ExprStmt] <Expr>;
750+
# 30| 0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
751+
# 30| 0: [TypeAccess] Unit
752+
# 30| 1: [StmtExpr] <Stmt>
753+
# 30| 0: [BlockStmt] { ... }
754+
# 30| 0: [LocalTypeDeclStmt] class ...
755+
# 30| 0: [AnonymousClass,LocalClass] new C0<String>(...) { ... }
756+
# 30| 1: [Constructor]
757+
# 30| 5: [BlockStmt] { ... }
758+
# 30| 0: [SuperConstructorInvocationStmt] super(...)
759+
# 30| 1: [BlockStmt] { ... }
760+
# 30| 1: [ExprStmt] <Expr>;
761+
# 30| 0: [ClassInstanceExpr] new (...)
762+
# 30| -3: [TypeAccess] C0<String>
595763
localClassField.kt:
596764
# 0| [CompilationUnit] localClassField
597765
# 1| 1: [Class] A

java/ql/test/kotlin/library-tests/classes/anonymousClasses.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
| classes.kt:89:16:89:44 | new Interface3<Integer>(...) { ... } | classes.kt:89:16:89:44 | new (...) | | classes.kt:89:16:89:44 | Interface3<Integer> | classes.kt:89:16:89:44 | class ... |
88
| classes.kt:127:16:134:9 | new Object(...) { ... } | classes.kt:127:16:134:9 | new (...) | | classes.kt:127:16:134:9 | Object | classes.kt:127:16:134:9 | class ... |
99
| generic_anonymous.kt:3:19:5:3 | new Object(...) { ... } | generic_anonymous.kt:3:19:5:3 | new (...) | | generic_anonymous.kt:3:19:5:3 | Object | generic_anonymous.kt:3:19:5:3 | class ... |
10+
| generic_anonymous.kt:26:13:26:37 | new Object(...) { ... } | generic_anonymous.kt:26:13:26:37 | new (...) | | generic_anonymous.kt:26:13:26:37 | Object | generic_anonymous.kt:26:13:26:37 | class ... |
11+
| generic_anonymous.kt:27:13:27:37 | new Object(...) { ... } | generic_anonymous.kt:27:13:27:37 | new (...) | | generic_anonymous.kt:27:13:27:37 | Object | generic_anonymous.kt:27:13:27:37 | class ... |
12+
| generic_anonymous.kt:28:13:28:41 | new Object(...) { ... } | generic_anonymous.kt:28:13:28:41 | new (...) | | generic_anonymous.kt:28:13:28:41 | Object | generic_anonymous.kt:28:13:28:41 | class ... |
13+
| generic_anonymous.kt:29:13:29:29 | new C0<U2>(...) { ... } | generic_anonymous.kt:29:13:29:29 | new (...) | | generic_anonymous.kt:29:13:29:29 | C0<U2> | generic_anonymous.kt:29:13:29:29 | class ... |
14+
| generic_anonymous.kt:30:13:30:33 | new C0<String>(...) { ... } | generic_anonymous.kt:30:13:30:33 | new (...) | | generic_anonymous.kt:30:13:30:33 | C0<String> | generic_anonymous.kt:30:13:30:33 | class ... |
1015
| local_anonymous.kt:5:16:7:9 | new Object(...) { ... } | local_anonymous.kt:5:16:7:9 | new (...) | | local_anonymous.kt:5:16:7:9 | Object | local_anonymous.kt:5:16:7:9 | class ... |
1116
| local_anonymous.kt:29:31:35:5 | new Object(...) { ... } | local_anonymous.kt:29:31:35:5 | new (...) | | local_anonymous.kt:29:31:35:5 | Object | local_anonymous.kt:29:31:35:5 | class ... |
1217
| local_anonymous.kt:40:14:44:5 | new Interface2(...) { ... } | local_anonymous.kt:40:14:44:5 | new (...) | | local_anonymous.kt:40:14:44:5 | Interface2 | local_anonymous.kt:40:14:44:5 | class ... |

java/ql/test/kotlin/library-tests/classes/classes.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,21 @@
3434
| classes.kt:119:13:121:13 | Local2 | C1$Local2 | final, private |
3535
| classes.kt:127:16:134:9 | new Object(...) { ... } | <anonymous class> | final, private |
3636
| classes.kt:129:17:131:17 | Local3 | C1$$Local3 | final, private |
37+
| classes.kt:138:1:148:1 | Cl0 | Cl0 | final, public |
38+
| classes.kt:140:9:146:9 | | Cl0$ | final, private |
39+
| classes.kt:141:13:145:13 | Cl1 | Cl0$Cl1 | final, private |
40+
| classes.kt:150:1:156:1 | Cl00 | Cl00 | final, public |
41+
| classes.kt:151:5:155:5 | Cl01 | Cl00$Cl01 | final, public |
3742
| generic_anonymous.kt:0:0:0:0 | Generic_anonymousKt | Generic_anonymousKt | final, public |
3843
| generic_anonymous.kt:1:1:9:1 | Generic | Generic | final, private |
3944
| generic_anonymous.kt:3:19:5:3 | new Object(...) { ... } | <anonymous class> | final, private |
45+
| generic_anonymous.kt:15:1:33:1 | Outer | Outer | final, public |
46+
| generic_anonymous.kt:25:9:31:9 | | Outer$ | final, private |
47+
| generic_anonymous.kt:26:13:26:37 | new Object(...) { ... } | <anonymous class> | final, private |
48+
| generic_anonymous.kt:27:13:27:37 | new Object(...) { ... } | <anonymous class> | final, private |
49+
| generic_anonymous.kt:28:13:28:41 | new Object(...) { ... } | <anonymous class> | final, private |
50+
| generic_anonymous.kt:29:13:29:29 | new C0<U2>(...) { ... } | <anonymous class> | final, private |
51+
| generic_anonymous.kt:30:13:30:33 | new C0<String>(...) { ... } | <anonymous class> | final, private |
4052
| localClassField.kt:1:1:11:1 | A | A | final, public |
4153
| localClassField.kt:3:9:3:19 | L | A$L | final, private |
4254
| localClassField.kt:8:9:8:19 | L | A$L | final, private |

java/ql/test/kotlin/library-tests/classes/classes.kt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,4 +133,24 @@ class C1 {
133133
}
134134
}
135135
}
136+
}
137+
138+
class Cl0<U0> {
139+
fun <U1> func1() {
140+
fun <U2> func2() {
141+
class Cl1<U3>{
142+
fun fn() {
143+
val x = Cl1<U3>()
144+
}
145+
}
146+
}
147+
}
148+
}
149+
150+
class Cl00<U0> {
151+
class Cl01<U1>{
152+
fun fn() {
153+
val x = Cl01<U1>()
154+
}
155+
}
136156
}

java/ql/test/kotlin/library-tests/classes/ctorCalls.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,20 @@ superCall
3434
| classes.kt:119:13:121:13 | super(...) |
3535
| classes.kt:127:16:134:9 | super(...) |
3636
| classes.kt:129:17:131:17 | super(...) |
37+
| classes.kt:138:1:148:1 | super(...) |
38+
| classes.kt:140:9:146:9 | super(...) |
39+
| classes.kt:141:13:145:13 | super(...) |
40+
| classes.kt:150:1:156:1 | super(...) |
41+
| classes.kt:151:5:155:5 | super(...) |
3742
| generic_anonymous.kt:1:1:9:1 | super(...) |
3843
| generic_anonymous.kt:3:19:5:3 | super(...) |
44+
| generic_anonymous.kt:15:1:33:1 | super(...) |
45+
| generic_anonymous.kt:25:9:31:9 | super(...) |
46+
| generic_anonymous.kt:26:13:26:37 | super(...) |
47+
| generic_anonymous.kt:27:13:27:37 | super(...) |
48+
| generic_anonymous.kt:28:13:28:41 | super(...) |
49+
| generic_anonymous.kt:29:13:29:29 | super(...) |
50+
| generic_anonymous.kt:30:13:30:33 | super(...) |
3951
| localClassField.kt:1:1:11:1 | super(...) |
4052
| localClassField.kt:3:9:3:19 | super(...) |
4153
| localClassField.kt:8:9:8:19 | super(...) |

0 commit comments

Comments
 (0)