Skip to content

Commit

Permalink
KSP: use overrides method instead of findOverridee() (#10272)
Browse files Browse the repository at this point in the history
* KSP: use `overrides` method instead of `findOverridee()`

* Remove package-private tests
  • Loading branch information
dstepanov authored Dec 15, 2023
1 parent 6e7584e commit e3066c4
Show file tree
Hide file tree
Showing 17 changed files with 3 additions and 378 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ internal abstract class AbstractKotlinMethodElement<T : KotlinNativeElement>(
) : AbstractKotlinElement<T>(nativeType, annotationMetadataFactory, visitorContext), MethodElement {

abstract val declaration: KSDeclaration?
abstract val overridee: KSDeclaration?
abstract val internalDeclaringType: ClassElement
abstract val internalDeclaredTypeArguments: Map<String, ClassElement>
abstract val internalReturnType: ClassElement
Expand Down Expand Up @@ -111,20 +110,10 @@ internal abstract class AbstractKotlinMethodElement<T : KotlinNativeElement>(
if (name != overridden.getName() || parameters.size != overridden.parameters.size) {
return false // Fast escape
}
if (nativeType == overridden.nativeType) {
return false // The same method
if (declaration != null && overridden.declaration != null) {
return visitorContext.resolver.overrides(declaration!!, overridden.declaration!!)
}
val thisType = getDeclaringType()
val thatType = overridden.getDeclaringType()
if (thisType.getName() == thatType.getName()) {
// The same type
return false
}
if (!thisType.isAssignable(thatType)) {
// not a parent class
return false
}
return overridee == overridden.declaration
return false
}

override fun hides(memberElement: MemberElement?) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ internal abstract class AbstractKotlinPropertyAccessorMethodElement<T : KotlinNa
accessor.receiver
}

override val overridee: KSDeclaration? by lazy {
accessor.receiver.findOverridee()
}

override val internalDeclaringType: ClassElement by lazy {
resolveDeclaringType(accessor.receiver, owningType)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ internal open class KotlinMethodElement(
visitorContext
)

override val overridee: KSDeclaration? by lazy {
declaration.findOverridee()
}

override val internalDeclaringType: ClassElement by lazy {
resolveDeclaringType(declaration, owningType)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package io.micronaut.kotlin.processing.aop.compile
import io.micronaut.annotation.processing.test.KotlinCompiler
import io.micronaut.aop.Intercepted
import io.micronaut.context.ApplicationContext
import spock.lang.PendingFeature
import spock.lang.Specification

import static io.micronaut.annotation.processing.test.KotlinCompiler.buildContext
Expand Down Expand Up @@ -75,8 +74,6 @@ class StubIntroduction : Interceptor<Any, Any> {
interceptor.invoked == 1
}

@PendingFeature(reason = "KSP can detect ony one overriden method")
// https://github.com/google/ksp/issues/1396
void "test inherited default or abstract methods are not overridden"() {
given:
ApplicationContext context = KotlinCompiler.buildContext('''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2164,7 +2164,6 @@ interface MyInterface {
result
}

@PendingFeature
void "test abstract and interface and overridden methods"() {
when:
def result = buildClassElementMapped('test.MyBean2', '''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,6 @@ open class Convertible : Car {

// mix inheritance + visibility

@Test
fun testPackagePrivateMethodInjectedDifferentPackages() {
assertTrue(spareTire!!.subPackagePrivateMethodInjected)
//Not valid because in Kotlin it is an override
//assertTrue(spareTire.superPackagePrivateMethodInjected)
}

@Test
fun testOverriddenProtectedMethodInjection() {
assertTrue(spareTire!!.subProtectedMethodInjected)
Expand Down Expand Up @@ -358,31 +351,13 @@ open class Convertible : Car {

// necessary injections occur

@Test
fun testPackagePrivateMethodInjectedEvenWhenSimilarMethodLacksAnnotation() {
//Not valid because in Kotlin the method is overridden
//assertTrue(spareTire!!.subPackagePrivateMethodForOverrideInjected)
}


// override or similar method without @Inject

@Test
fun testPrivateMethodNotInjectedWhenSupertypeHasAnnotatedSimilarMethod() {
assertFalse(spareTire!!.superPrivateMethodForOverrideInjected)
}

@Test
fun testPackagePrivateMethodNotInjectedWhenOverrideLacksAnnotation() {
assertFalse(engine!!.subPackagePrivateMethodForOverrideInjected)
assertFalse(engine.superPackagePrivateMethodForOverrideInjected)
}

@Test
fun testPackagePrivateMethodNotInjectedWhenSupertypeHasAnnotatedSimilarMethod() {
assertFalse(spareTire!!.superPackagePrivateMethodForOverrideInjected)
}

@Test
fun testProtectedMethodNotInjectedWhenOverrideNotAnnotated() {
assertFalse(spareTire!!.protectedMethodForOverrideInjected)
Expand All @@ -398,49 +373,8 @@ open class Convertible : Car {
assertFalse(engine!!.overriddenTwiceWithOmissionInSubclassInjected)
}

@Test
fun testOverridingMixedWithPackagePrivate2() {
assertTrue(spareTire!!.spareTirePackagePrivateMethod2Injected)
//Not valid in Kotlin because the method is overridden
//assertTrue((spareTire as Tire).tirePackagePrivateMethod2Injected)
assertFalse((spareTire as RoundThing).roundThingPackagePrivateMethod2Injected)

assertTrue(plainTire!!.tirePackagePrivateMethod2Injected)
//Not valid in Kotlin because the method is overridden
//assertTrue((plainTire as RoundThing).roundThingPackagePrivateMethod2Injected)
}

@Test
fun testOverridingMixedWithPackagePrivate3() {
assertFalse(spareTire!!.spareTirePackagePrivateMethod3Injected)
//Not valid in Kotlin because the method is overridden
//assertTrue((spareTire as Tire).tirePackagePrivateMethod3Injected)
assertFalse((spareTire as RoundThing).roundThingPackagePrivateMethod3Injected)

assertTrue(plainTire!!.tirePackagePrivateMethod3Injected)
//Not valid in Kotlin because the method is overridden
//assertTrue((plainTire as RoundThing).roundThingPackagePrivateMethod3Injected)
}

@Test
fun testOverridingMixedWithPackagePrivate4() {
assertFalse(plainTire!!.tirePackagePrivateMethod4Injected)
//Not the same as Java because package private can be overridden by any subclass in the project
//assertTrue((plainTire as RoundThing).roundThingPackagePrivateMethod4Injected)
}

// inject only once

@Test
fun testOverriddenPackagePrivateMethodInjectedOnlyOnce() {
assertFalse(engine!!.overriddenPackagePrivateMethodInjectedTwice)
}

@Test
fun testSimilarPackagePrivateMethodInjectedOnlyOnce() {
assertFalse(spareTire!!.similarPackagePrivateMethodInjectedTwice)
}

@Test
fun testOverriddenProtectedMethodInjectedOnlyOnce() {
assertFalse(spareTire!!.overriddenProtectedMethodInjectedTwice)
Expand All @@ -465,12 +399,6 @@ open class Convertible : Car {
assertTrue(spareTire.subPrivateMethodInjected)
}

@Test
fun testPackagePrivateMethodInjectedSamePackage() {
assertTrue(engine.subPackagePrivateMethodInjected)
assertFalse(engine.superPackagePrivateMethodInjected)
}

@Test
fun testPrivateMethodInjectedEvenWhenSimilarMethodLacksAnnotation() {
assertTrue(spareTire!!.subPrivateMethodForOverrideInjected)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ import jakarta.inject.Named
abstract class Engine {

var publicNoArgsConstructorInjected: Boolean = false
var subPackagePrivateMethodInjected: Boolean = false
var superPackagePrivateMethodInjected: Boolean = false
var subPackagePrivateMethodForOverrideInjected: Boolean = false
var superPackagePrivateMethodForOverrideInjected: Boolean = false

var overriddenTwiceWithOmissionInMiddleInjected: Boolean = false
var overriddenTwiceWithOmissionInSubclassInjected: Boolean = false
Expand All @@ -36,18 +32,9 @@ abstract class Engine {
protected var tireA: Tire? = null
protected var tireB: Tire? = null

var overriddenPackagePrivateMethodInjectedTwice: Boolean = false
var qualifiersInheritedFromOverriddenMethod: Boolean = false
var overriddenMethodInjected: Boolean = false

@Inject internal open fun injectPackagePrivateMethod() {
superPackagePrivateMethodInjected = true
}

@Inject internal open fun injectPackagePrivateMethodForOverride() {
superPackagePrivateMethodForOverrideInjected = true
}

@Inject
open fun injectQualifiers(@Drivers seatA: Seat, seatB: Seat,
@Named("spare") tireA: Tire, tireB: Tire) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,35 +31,24 @@ constructor(constructorInjection: FuelTank) : RoundThing() {
internal var constructorInjected: Boolean = false

var superPrivateMethodInjected: Boolean = false
var superPackagePrivateMethodInjected: Boolean = false
var superProtectedMethodInjected: Boolean = false
var superPublicMethodInjected: Boolean = false
var subPrivateMethodInjected: Boolean = false
var subPackagePrivateMethodInjected: Boolean = false
var subProtectedMethodInjected: Boolean = false
var subPublicMethodInjected: Boolean = false

var superPrivateMethodForOverrideInjected: Boolean = false
var superPackagePrivateMethodForOverrideInjected: Boolean = false
var subPrivateMethodForOverrideInjected: Boolean = false
var subPackagePrivateMethodForOverrideInjected: Boolean = false
var protectedMethodForOverrideInjected: Boolean = false
var publicMethodForOverrideInjected: Boolean = false

var methodInjectedBeforeFields: Boolean = false
var subtypeFieldInjectedBeforeSupertypeMethods: Boolean = false
var subtypeMethodInjectedBeforeSupertypeMethods: Boolean = false
var similarPrivateMethodInjectedTwice: Boolean = false
var similarPackagePrivateMethodInjectedTwice: Boolean = false
var overriddenProtectedMethodInjectedTwice: Boolean = false
var overriddenPublicMethodInjectedTwice: Boolean = false

var tirePackagePrivateMethod2Injected: Boolean = false

var tirePackagePrivateMethod3Injected: Boolean = false

var tirePackagePrivateMethod4Injected: Boolean = false

init {
this.constructorInjection = constructorInjection
}
Expand All @@ -84,13 +73,6 @@ constructor(constructorInjection: FuelTank) : RoundThing() {
superPrivateMethodInjected = true
}

@Inject internal open fun injectPackagePrivateMethod() {
if (superPackagePrivateMethodInjected) {
similarPackagePrivateMethodInjectedTwice = true
}
superPackagePrivateMethodInjected = true
}

@Inject protected open fun injectProtectedMethod() {
if (superProtectedMethodInjected) {
overriddenProtectedMethodInjectedTwice = true
Expand All @@ -110,10 +92,6 @@ constructor(constructorInjection: FuelTank) : RoundThing() {
subPrivateMethodForOverrideInjected = true
}

@Inject internal open fun injectPackagePrivateMethodForOverride() {
subPackagePrivateMethodForOverrideInjected = true
}

@Inject protected open fun injectProtectedMethodForOverride() {
protectedMethodForOverrideInjected = true
}
Expand All @@ -139,18 +117,6 @@ constructor(constructorInjection: FuelTank) : RoundThing() {
return false
}

@Inject override fun injectPackagePrivateMethod2() {
tirePackagePrivateMethod2Injected = true
}

@Inject override fun injectPackagePrivateMethod3() {
tirePackagePrivateMethod3Injected = true
}

override fun injectPackagePrivateMethod4() {
tirePackagePrivateMethod4Injected = true
}

companion object {

val NEVER_INJECTED = FuelTank()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ class V8Engine : GasEngine() {
publicNoArgsConstructorInjected = true
}

@Inject override fun injectPackagePrivateMethod() {
if (subPackagePrivateMethodInjected) {
overriddenPackagePrivateMethodInjectedTwice = true
}
subPackagePrivateMethodInjected = true
}

/**
* Qualifiers are swapped from how they appear in the superclass.
*/
Expand All @@ -46,10 +39,6 @@ class V8Engine : GasEngine() {
}
}

override fun injectPackagePrivateMethodForOverride() {
subPackagePrivateMethodForOverrideInjected = true
}

@Inject
override fun injectTwiceOverriddenWithOmissionInMiddle() {
overriddenTwiceWithOmissionInMiddleInjected = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,6 @@
*/
package org.atinject.jakartatck.auto.accessories


import jakarta.inject.Inject

open class RoundThing {

var roundThingPackagePrivateMethod2Injected: Boolean = false
private set

var roundThingPackagePrivateMethod3Injected: Boolean = false
private set

var roundThingPackagePrivateMethod4Injected: Boolean = false
private set

@Inject open internal fun injectPackagePrivateMethod2() {
roundThingPackagePrivateMethod2Injected = true
}

@Inject open internal fun injectPackagePrivateMethod3() {
roundThingPackagePrivateMethod3Injected = true
}

@Inject open internal fun injectPackagePrivateMethod4() {
roundThingPackagePrivateMethod4Injected = true
}
}
Loading

0 comments on commit e3066c4

Please sign in to comment.