Skip to content

Commit 657931a

Browse files
authored
Fix find overridee, add tests (#151)
* Fix find overridee, add tests This CL fixes a few problems in find overridee implementation: a) If the method does not override anything, it would throw instead of returning null, fixed. b) If it overrides a method that overrides another method, it would return null, now it returns the first actual override. c) If it overrides a method from a GrandSuper (where super does not override it), it would return null. Now it returns the method in GrandSuper class.
1 parent 4559f68 commit 657931a

File tree

9 files changed

+387
-10
lines changed

9 files changed

+387
-10
lines changed

api/src/main/kotlin/com/google/devtools/ksp/symbol/KSFunctionDeclaration.kt

+23-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,29 @@ interface KSFunctionDeclaration : KSDeclaration, KSDeclarationContainer {
5757
val parameters: List<KSValueParameter>
5858

5959
/**
60-
* Find the original overridee of this function, if overriding.
60+
* Find the closest overridee of this function, if overriding.
61+
*
62+
* For the following input:
63+
* ```
64+
* abstract class A {
65+
* open fun x() {}
66+
* open fun y() {}
67+
* }
68+
* abstract class B : A() {
69+
* override open fun x() {}
70+
* }
71+
* abstract class C : B() {
72+
* override open fun x() {}
73+
* override open fun y() {}
74+
* }
75+
* ```
76+
* Calling `findOverridee` on `C.x` will return `B.x`.
77+
* Calling `findOverridee` on `C.y` will return `A.y`.
78+
*
79+
* When there are multiple super interfaces implementing the function, the closest declaration
80+
* to the current containing declaration is selected. If they are in the same level, the
81+
* function of the first specified interface (in source) will be returned.
82+
*
6183
* @return [KSFunctionDeclaration] for the original function, if overriding, otherwise null.
6284
* Calling [findOverridee] is expensive and should be avoided if possible.
6385
*/

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processing/impl/ResolverImpl.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ class ResolverImpl(
326326
}
327327
} else {
328328
moduleClassResolver.resolveClass(JavaMethodImpl(psi).containingClass)
329-
?.unsubstitutedMemberScope!!.getDescriptorsFiltered().single { it.findPsi() == psi }
329+
?.unsubstitutedMemberScope!!.getDescriptorsFiltered().singleOrNull { it.findPsi() == psi }
330330
}
331331
}
332332
is PsiField -> moduleClassResolver.resolveClass(JavaFieldImpl(psi).containingClass)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright 2020 Google LLC
3+
* Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package com.google.devtools.ksp.processor
19+
20+
import com.google.devtools.ksp.closestClassDeclaration
21+
import com.google.devtools.ksp.getClassDeclarationByName
22+
import com.google.devtools.ksp.processing.Resolver
23+
import com.google.devtools.ksp.symbol.KSClassDeclaration
24+
import com.google.devtools.ksp.symbol.KSFunctionDeclaration
25+
26+
@Suppress("unused") // used by tests
27+
class OverrideeProcessor: AbstractTestProcessor() {
28+
private val results = mutableListOf<String>()
29+
30+
override fun toResult() = results
31+
32+
override fun process(resolver: Resolver) {
33+
logSubject(resolver, "Subject")
34+
logSubject(resolver, "JavaSubject.Subject")
35+
logSubject(resolver, "lib.Subject")
36+
logSubject(resolver, "ConflictingSubject1")
37+
logSubject(resolver, "ConflictingSubject2")
38+
logSubject(resolver, "ConflictingSubject3")
39+
logSubject(resolver, "ConflictingSubject4")
40+
logSubject(resolver, "OverrideOrder1")
41+
logSubject(resolver, "OverrideOrder2")
42+
}
43+
44+
private fun logSubject(resolver: Resolver, qName:String) {
45+
results.add("$qName:")
46+
val subject = resolver.getClassDeclarationByName(qName)!!
47+
subject.declarations.filterIsInstance<KSClassDeclaration>().forEach {
48+
logClass(it)
49+
}
50+
logClass(subject)
51+
}
52+
53+
private fun logClass(subject: KSClassDeclaration) {
54+
subject.declarations.filterIsInstance<KSFunctionDeclaration>()
55+
.filterNot { it.simpleName.asString() in IGNORED_METHOD_NAMES }
56+
.forEach {
57+
val signature = it.toSignature()
58+
val overrideeSignature = it.findOverridee()?.toSignature()
59+
results.add("$signature -> $overrideeSignature")
60+
}
61+
}
62+
63+
private fun KSFunctionDeclaration.toSignature(): String {
64+
val self = this
65+
return buildString {
66+
append(self.closestClassDeclaration()?.simpleName?.asString())
67+
append(".")
68+
append(self.simpleName.asString())
69+
append(
70+
self.parameters.joinToString(", ", prefix = "(", postfix = ")") {
71+
"${it.name?.asString()}:${it.type.resolve().declaration.simpleName.asString()}"
72+
}
73+
)
74+
}
75+
}
76+
77+
companion object {
78+
// ignore these methods as we receive syntetics of it from compiled code
79+
private val IGNORED_METHOD_NAMES = listOf("equals", "hashCode", "toString")
80+
}
81+
}
82+
83+
interface MyInterface {
84+
fun openFoo(): Int { return 1}
85+
fun absFoo(): Unit
86+
}
87+
88+
interface MyInterface2 {
89+
fun absFoo(): Unit
90+
}
91+
92+
abstract class MyAbstract: MyInterface {
93+
override fun absFoo(): Unit {val a = 1}
94+
override fun openFoo(): Int { return 2 }
95+
}
96+
97+
class Subject2: MyInterface, MyAbstract() {
98+
override fun absFoo(): Unit = TODO()
99+
}

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSFunctionDeclarationDescriptorImpl.kt

+3-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import com.google.devtools.ksp.isVisibleFrom
2525
import com.google.devtools.ksp.processing.impl.ResolverImpl
2626
import com.google.devtools.ksp.symbol.*
2727
import com.google.devtools.ksp.symbol.impl.KSObjectCache
28+
import com.google.devtools.ksp.symbol.impl.findClosestOverridee
2829
import com.google.devtools.ksp.symbol.impl.toFunctionKSModifiers
2930
import com.google.devtools.ksp.symbol.impl.toKSFunctionDeclaration
3031
import com.google.devtools.ksp.symbol.impl.toKSModifiers
@@ -39,8 +40,8 @@ class KSFunctionDeclarationDescriptorImpl private constructor(val descriptor: Fu
3940
}
4041

4142
override fun findOverridee(): KSFunctionDeclaration? {
42-
return ResolverImpl.instance.resolveFunctionDeclaration(this)?.original?.overriddenDescriptors?.singleOrNull { it.overriddenDescriptors.isEmpty() }
43-
?.toKSFunctionDeclaration()
43+
val descriptor = ResolverImpl.instance.resolveFunctionDeclaration(this)
44+
return descriptor?.findClosestOverridee()?.toKSFunctionDeclaration()
4445
}
4546

4647
override val typeParameters: List<KSTypeParameter> by lazy {

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/java/KSFunctionDeclarationJavaImpl.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ class KSFunctionDeclarationJavaImpl private constructor(val psi: PsiMethod) : KS
5252
}
5353

5454
override fun findOverridee(): KSFunctionDeclaration? {
55-
return ResolverImpl.instance.resolveFunctionDeclaration(this)?.original?.overriddenDescriptors?.single { it.overriddenDescriptors.isEmpty() }
56-
?.toKSFunctionDeclaration()
55+
val descriptor = ResolverImpl.instance.resolveFunctionDeclaration(this)
56+
return descriptor?.findClosestOverridee()?.toKSFunctionDeclaration()
5757
}
5858

5959
override val declarations: List<KSDeclaration> = emptyList()

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSFunctionDeclarationImpl.kt

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import com.google.devtools.ksp.isVisibleFrom
2424
import com.google.devtools.ksp.processing.impl.ResolverImpl
2525
import com.google.devtools.ksp.symbol.*
2626
import com.google.devtools.ksp.symbol.impl.*
27+
import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
2728
import org.jetbrains.kotlin.lexer.KtTokens
2829
import org.jetbrains.kotlin.psi.*
2930
import org.jetbrains.kotlin.resolve.OverridingUtil
@@ -37,8 +38,8 @@ class KSFunctionDeclarationImpl private constructor(val ktFunction: KtFunction)
3738
}
3839

3940
override fun findOverridee(): KSFunctionDeclaration? {
40-
return ResolverImpl.instance.resolveFunctionDeclaration(this)?.original?.overriddenDescriptors?.single { it.overriddenDescriptors.isEmpty() }
41-
?.toKSFunctionDeclaration()
41+
val descriptor = ResolverImpl.instance.resolveFunctionDeclaration(this)
42+
return descriptor?.findClosestOverridee()?.toKSFunctionDeclaration()
4243
}
4344

4445
override val declarations: List<KSDeclaration> by lazy {

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/utils.kt

+30-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import org.jetbrains.kotlin.lexer.KtTokens
3737
import org.jetbrains.kotlin.load.java.JavaVisibilities
3838
import org.jetbrains.kotlin.load.java.descriptors.JavaClassConstructorDescriptor
3939
import org.jetbrains.kotlin.psi.*
40-
import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
4140
import org.jetbrains.kotlin.resolve.source.getPsi
4241
import org.jetbrains.kotlin.types.KotlinType
4342
import org.jetbrains.kotlin.types.StarProjectionImpl
@@ -282,4 +281,33 @@ internal fun DeclarationDescriptor.findPsi(): PsiElement? {
282281
// For synthetic members.
283282
if ((this is CallableMemberDescriptor) && this.kind != CallableMemberDescriptor.Kind.DECLARATION) return null
284283
return (this as? DeclarationDescriptorWithSource)?.source?.getPsi()
285-
}
284+
}
285+
286+
/**
287+
* @see KSFunctionDeclaration.findOverridee for docs.
288+
*/
289+
internal fun FunctionDescriptor.findClosestOverridee(): FunctionDescriptor? {
290+
// When there is an intermediate class between the overridden and our function, we might receive
291+
// a FAKE_OVERRIDE function which is not desired as we are trying to find the actual
292+
// declared method.
293+
294+
// we also want to return the closes function declaration. That is either the closest
295+
// class / interface method OR in case of equal distance (e.g. diamon dinheritance), pick the
296+
// one declared first in the code.
297+
298+
val queue = ArrayDeque<FunctionDescriptor>()
299+
queue.add(this)
300+
301+
while (queue.isNotEmpty()) {
302+
val current = queue.removeFirst()
303+
val overriddenDescriptors = current.original.overriddenDescriptors
304+
overriddenDescriptors.firstOrNull {
305+
it.kind != CallableMemberDescriptor.Kind.FAKE_OVERRIDE
306+
}?.let {
307+
return it.original
308+
}
309+
// if all methods are fake, add them to the queue
310+
queue.addAll(overriddenDescriptors)
311+
}
312+
return null
313+
}

compiler-plugin/src/test/java/com/google/devtools/ksp/test/KotlinKSPTestGenerated.java

+5
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,11 @@ public void testMultipleModules() throws Exception {
157157
runTest("testData/api/multipleModules.kt");
158158
}
159159

160+
@TestMetadata(("overridee.kt"))
161+
public void testOverridee() throws Exception {
162+
runTest("testData/api/overridee.kt");
163+
}
164+
160165
@TestMetadata("parameterTypes.kt")
161166
public void testParameterTypes() throws Exception {
162167
runTest("testData/api/parameterTypes.kt");

0 commit comments

Comments
 (0)