From a37622892a64be6bc7ead057ba11c705cf052da9 Mon Sep 17 00:00:00 2001 From: Anders Fugmann Date: Sun, 12 Jul 2026 02:17:22 +0200 Subject: [PATCH] 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> --- .../src/main/kotlin/KotlinFileExtractor.kt | 84 ++++++++++++++----- .../library-tests/exprs/PrintAst.expected | 2 +- .../library-tests/exprs/exprs.expected | 2 +- 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt index da46483e7ec..14017a8b6c8 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt @@ -91,6 +91,11 @@ open class KotlinFileExtractor( private var currentIrFile: IrFile? = null + // Set only while extracting the compiler-generated accessor artifacts of a local delegated + // property (declared inside a function body). Used to recognise the synthetic `thisRef` + // argument of that property's forwarding get/set, which under K1 gets a bogus location. + private var currentLocalDelegatedProperty: IrLocalDelegatedProperty? = null + private inline fun with(kind: String, element: IrElement, f: () -> T): T { val name = when (element) { @@ -3187,6 +3192,39 @@ open class KotlinFileExtractor( return tw.getLocation(delegateExpression.startOffset, delegateExpression.endOffset) } + /** + * For the synthetic `thisRef` argument of a *local* delegated property (declared inside a + * function body), returns the location to use, or null to leave the raw location in place. + * + * A local delegated property's forwarding get/set is inlined into the enclosing function + * rather than materialised as a `DELEGATED_PROPERTY_ACCESSOR` (so + * [getDelegatedAccessorSyntheticArgumentLocation] does not apply). Because a local property + * has no dispatch/extension receiver, its synthetic `thisRef` is always a `null` constant + * with no source token. K2 records it at the whole-file location (`file:0:0:0:0`); K1 gives + * it a bogus non-zero location that resolves to an unrelated top-of-file token. + * + * This only fires while [currentLocalDelegatedProperty] is being extracted, and only for a + * `null` constant whose offsets fall outside that property's PSI text range (so a genuine + * source `null` inside the delegate expression, e.g. `by foo(null)`, is never moved). It is a + * no-op under K2, where [getPsi2Ir] is null. + */ + private fun getLocalDelegatedPropertySyntheticNullLocation(e: IrElement): Label? { + val property = currentLocalDelegatedProperty ?: return null + if (e !is CodeQLIrConst<*> || e.value != null) return null + val start = e.startOffset + val end = e.endOffset + if (start < 0 || end < 0) return null + val file = currentIrFile ?: return null + val psiElement = getPsi2Ir()?.findPsiElement(property, file) ?: return null + val ktProperty = generateSequence(psiElement) { it.parent } + .filterIsInstance().firstOrNull() + ?: return null + // A genuine `null` inside the delegate expression lies within the property's range; only + // treat offsets outside the whole property declaration as the synthetic thisRef. + if (start >= ktProperty.startOffset && end <= ktProperty.endOffset) return null + return tw.getWholeFileLocation() + } + /** * For a delegated-property accessor (origin `DELEGATED_PROPERTY_ACCESSOR`), returns the * offset remapping to apply while extracting its body, or null if it cannot be computed. @@ -3402,25 +3440,31 @@ open class KotlinFileExtractor( val delegate: IrVariable? = s.delegate as IrVariable? val propId = tw.getFreshIdLabel() - if (delegate == null) { - // This is not expected to happen, as the plugin hooks into the pipeline before IR lowering. - logger.errorElement("Local delegated property is missing delegate", s) - } else { - extractVariable(delegate, callable, blockId, 0) - tw.writeKtProperties(propId, s.name.asString()) - tw.writeHasLocation(propId, locId) - tw.writeKtPropertyDelegates(propId, useVariable(delegate)) - } - // Getter: - extractStatement(s.getter, callable, blockId, 1) - val getterLabel = getLocallyVisibleFunctionLabels(s.getter).function - tw.writeKtPropertyGetters(propId, getterLabel) + val prevLocalDelegatedProperty = currentLocalDelegatedProperty + currentLocalDelegatedProperty = s + try { + if (delegate == null) { + // This is not expected to happen, as the plugin hooks into the pipeline before IR lowering. + logger.errorElement("Local delegated property is missing delegate", s) + } else { + extractVariable(delegate, callable, blockId, 0) + tw.writeKtProperties(propId, s.name.asString()) + tw.writeHasLocation(propId, locId) + tw.writeKtPropertyDelegates(propId, useVariable(delegate)) + } + // Getter: + extractStatement(s.getter, callable, blockId, 1) + val getterLabel = getLocallyVisibleFunctionLabels(s.getter).function + tw.writeKtPropertyGetters(propId, getterLabel) - val setter = s.setter - if (setter != null) { - extractStatement(setter, callable, blockId, 2) - val setterLabel = getLocallyVisibleFunctionLabels(setter).function - tw.writeKtPropertySetters(propId, setterLabel) + val setter = s.setter + if (setter != null) { + extractStatement(setter, callable, blockId, 2) + val setterLabel = getLocallyVisibleFunctionLabels(setter).function + tw.writeKtPropertySetters(propId, setterLabel) + } + } finally { + currentLocalDelegatedProperty = prevLocalDelegatedProperty } } else -> { @@ -7455,7 +7499,9 @@ open class KotlinFileExtractor( v == null -> { extractNull( e.type, - getDelegatedAccessorSyntheticArgumentLocation(e) ?: tw.getLocation(e), + getDelegatedAccessorSyntheticArgumentLocation(e) + ?: getLocalDelegatedPropertySyntheticNullLocation(e) + ?: tw.getLocation(e), parent, idx, enclosingCallable, diff --git a/java/ql/test-kotlin1/library-tests/exprs/PrintAst.expected b/java/ql/test-kotlin1/library-tests/exprs/PrintAst.expected index 806a4f1ecc8..fa3fcb4f07d 100644 --- a/java/ql/test-kotlin1/library-tests/exprs/PrintAst.expected +++ b/java/ql/test-kotlin1/library-tests/exprs/PrintAst.expected @@ -588,7 +588,7 @@ delegatedProperties.kt: # 39| 0: [MethodCall] provideDelegate(...) # 39| -1: [ClassInstanceExpr] new DelegateProvider(...) # 39| -3: [TypeAccess] DelegateProvider -# 1| 0: [NullLiteral] null +# 0| 0: [NullLiteral] null # 39| 1: [PropertyRefExpr] ...::... # 39| -4: [AnonymousClass] new KProperty0(...) { ... } # 39| 1: [Constructor] diff --git a/java/ql/test-kotlin1/library-tests/exprs/exprs.expected b/java/ql/test-kotlin1/library-tests/exprs/exprs.expected index 0b70fba8896..ddf7e523c5e 100644 --- a/java/ql/test-kotlin1/library-tests/exprs/exprs.expected +++ b/java/ql/test-kotlin1/library-tests/exprs/exprs.expected @@ -1,4 +1,5 @@ | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:6:9:9:9 | | NullLiteral | +| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:18:5:40:5 | fn | NullLiteral | | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:19:9:19:51 | | NullLiteral | | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:19:9:19:51 | | NullLiteral | | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:23:9:23:31 | | NullLiteral | @@ -8,7 +9,6 @@ | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:39:9:39:51 | | NullLiteral | | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:82:9:82:54 | | NullLiteral | | delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:82:9:82:54 | | NullLiteral | -| delegatedProperties.kt:1:9:1:12 | null | delegatedProperties.kt:18:5:40:5 | fn | NullLiteral | | delegatedProperties.kt:5:5:12:5 | Unit | file://:0:0:0:0 | | TypeAccess | | delegatedProperties.kt:6:9:9:9 | int | file://:0:0:0:0 | | TypeAccess | | delegatedProperties.kt:6:9:9:9 | prop1$delegate | delegatedProperties.kt:5:5:12:5 | fn | LocalVariableDeclExpr |