hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-07-12 23:15:40 +02:00
Code Issues Packages Projects Releases Wiki Activity

Kotlin: converge local delegated-property synthetic thisRef location onto K2

Browse Source 
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>
This commit is contained in:
Anders Fugmann
2026-07-12 02:17:22 +02:00
parent 7ea8558c25
commit a37622892a
3 changed files with 67 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt
View File

@@ -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 <T> 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<DbLocation>? {
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<KtProperty>().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<DbKt_property>()
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,

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

@@ -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<Integer>(...) { ... }
# 39| 1: [Constructor]

2
java/ql/test-kotlin1/library-tests/exprs/exprs.expected
View File

@@ -1,4 +1,5 @@
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:6:9:9:9 | <get-prop1> | 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 | <get-varResource1> | NullLiteral |
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:19:9:19:51 | <set-varResource1> | NullLiteral |
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:23:9:23:31 | <get-name> | NullLiteral |
@@ -8,7 +9,6 @@
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:39:9:39:51 | <get-varResource2> | NullLiteral |
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:82:9:82:54 | <get-delegatedToMember3> | NullLiteral |
| delegatedProperties.kt:0:0:0:0 | null | delegatedProperties.kt:82:9:82:54 | <set-delegatedToMember3> | 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 | <none> | TypeAccess |
| delegatedProperties.kt:6:9:9:9 | int | file://:0:0:0:0 | <none> | TypeAccess |
| delegatedProperties.kt:6:9:9:9 | prop1$delegate | delegatedProperties.kt:5:5:12:5 | fn | LocalVariableDeclExpr |