Skip to content

Commit a376228

Browse files
Kotlin: converge local delegated-property synthetic thisRef location onto K2
Follow-up to the member/top-level delegated-accessor convergence, covering the remaining case: a LOCAL delegated property (declared inside a function body, e.g. `val x: Int by DelegateProvider()`). Unlike member/top-level delegated properties, a local delegated property's forwarding get/set is inlined into the enclosing function rather than materialised as a `DELEGATED_PROPERTY_ACCESSOR`, so the previous fix (getDelegatedAccessorSyntheticArgumentLocation, gated on that origin) did not apply. Because a local property has no dispatch/extension receiver, its synthetic `thisRef` argument (passed to the delegate's `provideDelegate`/`getValue`/`setValue`) is always a `null` constant with no source token. Under K1 this synthetic `null` was given a bogus location `1:9:1:12`: its IR offsets (8..11) are not real source offsets and `findPsiElement` resolves them to the file's line-1 `import`. K2 records it at the whole-file location `0:0:0:0`, which is the honest representation for an argument with no source token, and is the canonical target for this unification. This change adds `getLocalDelegatedPropertySyntheticNullLocation`, gated on: - a scoped context (`currentLocalDelegatedProperty`) that is set only while extracting a specific `IrLocalDelegatedProperty`'s generated artifacts (its delegate initializer and forwarding get/set), - the element being a `null` `CodeQLIrConst`, and - the element offsets lying outside the property's `KtProperty` text range (so a genuine source `null` inside the delegate expression, e.g. `by foo(null)`, is never relocated). It returns the whole-file location, mirroring K2. Per the design review, this is deliberately a distinct, narrowly-scoped path anchored on the local delegated property being extracted, NOT a relaxation of the accessor-origin gate to arbitrary enclosing functions (which would risk moving genuine source `null`/`this` literals). It relies on PSI, so it is a no-op under K2. Verified via full dual-suite relearn (CI-faithful: 2.3.20/lang-1.9 for tk1, default/2.4.0 for tk2, database consistency checks): all 3333 tests pass. Only two tk1 files change (exprs.expected and its PrintAst), each a single row relocating the synthetic `null` from `1:9:1:12` to `0:0:0:0`; the NullLiteral rows now match test-kotlin2 exactly, no `1:9:1:12` bogus locations remain anywhere in test-kotlin1, and test-kotlin2 is byte-for-byte unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7ea8558 commit a376228

3 files changed

Lines changed: 68 additions & 22 deletions

File tree

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

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ open class KotlinFileExtractor(
9191

9292
private var currentIrFile: IrFile? = null
9393

94+
// Set only while extracting the compiler-generated accessor artifacts of a local delegated
95+
// property (declared inside a function body). Used to recognise the synthetic `thisRef`
96+
// argument of that property's forwarding get/set, which under K1 gets a bogus location.
97+
private var currentLocalDelegatedProperty: IrLocalDelegatedProperty? = null
98+
9499
private inline fun <T> with(kind: String, element: IrElement, f: () -> T): T {
95100
val name =
96101
when (element) {
@@ -3187,6 +3192,39 @@ open class KotlinFileExtractor(
31873192
return tw.getLocation(delegateExpression.startOffset, delegateExpression.endOffset)
31883193
}
31893194

3195+
/**
3196+
* For the synthetic `thisRef` argument of a *local* delegated property (declared inside a
3197+
* function body), returns the location to use, or null to leave the raw location in place.
3198+
*
3199+
* A local delegated property's forwarding get/set is inlined into the enclosing function
3200+
* rather than materialised as a `DELEGATED_PROPERTY_ACCESSOR` (so
3201+
* [getDelegatedAccessorSyntheticArgumentLocation] does not apply). Because a local property
3202+
* has no dispatch/extension receiver, its synthetic `thisRef` is always a `null` constant
3203+
* with no source token. K2 records it at the whole-file location (`file:0:0:0:0`); K1 gives
3204+
* it a bogus non-zero location that resolves to an unrelated top-of-file token.
3205+
*
3206+
* This only fires while [currentLocalDelegatedProperty] is being extracted, and only for a
3207+
* `null` constant whose offsets fall outside that property's PSI text range (so a genuine
3208+
* source `null` inside the delegate expression, e.g. `by foo(null)`, is never moved). It is a
3209+
* no-op under K2, where [getPsi2Ir] is null.
3210+
*/
3211+
private fun getLocalDelegatedPropertySyntheticNullLocation(e: IrElement): Label<DbLocation>? {
3212+
val property = currentLocalDelegatedProperty ?: return null
3213+
if (e !is CodeQLIrConst<*> || e.value != null) return null
3214+
val start = e.startOffset
3215+
val end = e.endOffset
3216+
if (start < 0 || end < 0) return null
3217+
val file = currentIrFile ?: return null
3218+
val psiElement = getPsi2Ir()?.findPsiElement(property, file) ?: return null
3219+
val ktProperty = generateSequence(psiElement) { it.parent }
3220+
.filterIsInstance<KtProperty>().firstOrNull()
3221+
?: return null
3222+
// A genuine `null` inside the delegate expression lies within the property's range; only
3223+
// treat offsets outside the whole property declaration as the synthetic thisRef.
3224+
if (start >= ktProperty.startOffset && end <= ktProperty.endOffset) return null
3225+
return tw.getWholeFileLocation()
3226+
}
3227+
31903228
/**
31913229
* For a delegated-property accessor (origin `DELEGATED_PROPERTY_ACCESSOR`), returns the
31923230
* offset remapping to apply while extracting its body, or null if it cannot be computed.
@@ -3402,25 +3440,31 @@ open class KotlinFileExtractor(
34023440
val delegate: IrVariable? = s.delegate as IrVariable?
34033441
val propId = tw.getFreshIdLabel<DbKt_property>()
34043442

3405-
if (delegate == null) {
3406-
// This is not expected to happen, as the plugin hooks into the pipeline before IR lowering.
3407-
logger.errorElement("Local delegated property is missing delegate", s)
3408-
} else {
3409-
extractVariable(delegate, callable, blockId, 0)
3410-
tw.writeKtProperties(propId, s.name.asString())
3411-
tw.writeHasLocation(propId, locId)
3412-
tw.writeKtPropertyDelegates(propId, useVariable(delegate))
3413-
}
3414-
// Getter:
3415-
extractStatement(s.getter, callable, blockId, 1)
3416-
val getterLabel = getLocallyVisibleFunctionLabels(s.getter).function
3417-
tw.writeKtPropertyGetters(propId, getterLabel)
3418-
3419-
val setter = s.setter
3420-
if (setter != null) {
3421-
extractStatement(setter, callable, blockId, 2)
3422-
val setterLabel = getLocallyVisibleFunctionLabels(setter).function
3423-
tw.writeKtPropertySetters(propId, setterLabel)
3443+
val prevLocalDelegatedProperty = currentLocalDelegatedProperty
3444+
currentLocalDelegatedProperty = s
3445+
try {
3446+
if (delegate == null) {
3447+
// This is not expected to happen, as the plugin hooks into the pipeline before IR lowering.
3448+
logger.errorElement("Local delegated property is missing delegate", s)
3449+
} else {
3450+
extractVariable(delegate, callable, blockId, 0)
3451+
tw.writeKtProperties(propId, s.name.asString())
3452+
tw.writeHasLocation(propId, locId)
3453+
tw.writeKtPropertyDelegates(propId, useVariable(delegate))
3454+
}
3455+
// Getter:
3456+
extractStatement(s.getter, callable, blockId, 1)
3457+
val getterLabel = getLocallyVisibleFunctionLabels(s.getter).function
3458+
tw.writeKtPropertyGetters(propId, getterLabel)
3459+
3460+
val setter = s.setter
3461+
if (setter != null) {
3462+
extractStatement(setter, callable, blockId, 2)
3463+
val setterLabel = getLocallyVisibleFunctionLabels(setter).function
3464+
tw.writeKtPropertySetters(propId, setterLabel)
3465+
}
3466+
} finally {
3467+
currentLocalDelegatedProperty = prevLocalDelegatedProperty
34243468
}
34253469
}
34263470
else -> {
@@ -7455,7 +7499,9 @@ open class KotlinFileExtractor(
74557499
v == null -> {
74567500
extractNull(
74577501
e.type,
7458-
getDelegatedAccessorSyntheticArgumentLocation(e) ?: tw.getLocation(e),
7502+
getDelegatedAccessorSyntheticArgumentLocation(e)
7503+
?: getLocalDelegatedPropertySyntheticNullLocation(e)
7504+
?: tw.getLocation(e),
74597505
parent,
74607506
idx,
74617507
enclosingCallable,

java/ql/test-kotlin1/library-tests/exprs/PrintAst.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ delegatedProperties.kt:
588588
# 39| 0: [MethodCall] provideDelegate(...)
589589
# 39| -1: [ClassInstanceExpr] new DelegateProvider(...)
590590
# 39| -3: [TypeAccess] DelegateProvider
591-
# 1| 0: [NullLiteral] null
591+
# 0| 0: [NullLiteral] null
592592
# 39| 1: [PropertyRefExpr] ...::...
593593
# 39| -4: [AnonymousClass] new KProperty0<Integer>(...) { ... }
594594
# 39| 1: [Constructor]

java/ql/test-kotlin1/library-tests/exprs/exprs.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:6:9:9:9 | <get-prop1> | NullLiteral |
2+
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:18:5:40:5 | fn | NullLiteral |
23
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:19:9:19:51 | <get-varResource1> | NullLiteral |
34
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:19:9:19:51 | <set-varResource1> | NullLiteral |
45
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:23:9:23:31 | <get-name> | NullLiteral |
@@ -8,7 +9,6 @@
89
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:39:9:39:51 | <get-varResource2> | NullLiteral |
910
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:82:9:82:54 | <get-delegatedToMember3> | NullLiteral |
1011
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:82:9:82:54 | <set-delegatedToMember3> | NullLiteral |
11-
| delegatedProperties.kt:1:9:1:12 | null | delegatedProperties.kt:18:5:40:5 | fn | NullLiteral |
1212
| delegatedProperties.kt:5:5:12:5 | Unit | file://:0:0:0:0 | <none> | TypeAccess |
1313
| delegatedProperties.kt:6:9:9:9 | int | file://:0:0:0:0 | <none> | TypeAccess |
1414
| delegatedProperties.kt:6:9:9:9 | prop1$delegate | delegatedProperties.kt:5:5:12:5 | fn | LocalVariableDeclExpr |

0 commit comments

Comments
 (0)