mirror of
https://github.com/github/codeql.git
synced 2026-07-26 21:44:02 +02:00
Kotlin: span assignment-desugared setter calls through the assigned value
An assignment to a property, delegated property or `@JvmStatic` property
(`lhs = rhs`) desugars to a setter call. The two frontends anchor that
synthesised call differently:
- K1 records the call (and its synthetic implicit-`this` receiver and any
synthetic reflection arguments) at the assignment's left-hand side only
(`varResource0 = 3` -> `37:9:37:20`, `curValue = value` -> `29:17:29:24`).
- K2 spans the whole assignment, through the assigned value
(`37:9:37:24`, `29:17:29:32`).
The setter call *is* the desugaring of the whole assignment, so K2's
whole-assignment span is the more intuitive, information-preserving one: it keeps
the call on the source construct it represents rather than dropping the
right-hand side. This matches the existing array-`set` EQ-origin widening
precedent, which already widens K1 onto the whole-assignment span.
We converge K1 onto the K2 span with a scoped offset remap set only while
extracting the setter call, keyed on the call's exact offset pair. Because the
compiler gives the synthetic implicit-`this` receiver and the synthetic
reflection arguments the same offsets as the call, the single remap widens them
along with the `MethodCall` node, while real source arguments (the assigned
value itself, e.g. the `2` / `3` / `value`) keep their own offsets and are
unaffected. The remap fires only for an `IrStatementOrigin.EQ` call whose last
value argument (the assigned value) ends past the call's own end offset, so it is
a no-op under K2 (where the call already spans the assigned value) and for every
non-assignment call.
Tradeoff: the direction is fixed (K1 -> K2) because K1 cannot, from its raw IR,
recover the whole-assignment span for these synthetic children; K2 produces it
natively. That is acceptable here because the whole-assignment span is the more
correct one on the merits and is already the established convention for array
`set`.
Expected updates (K1 only; K2 already emitted these):
- library-tests/exprs/exprs.expected (delegated + direct property setters)
- library-tests/generic-instance-methods/test.expected (setStored)
- library-tests/jvmstatic-annotation/test.expected (@JvmStatic setters)
Both suites relearned; all tests pass. Divergence 798 -> 758 rows.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 86bfc022-3ecc-4746-ba23-3b76c4e4c3e4
This commit is contained in:
Anders Fugmann
@@ -4986,6 +4986,40 @@ open class KotlinFileExtractor(
|
||||
else -> false
|
||||
}
|
||||
|
||||
/**
|
||||
* For a property/delegated-property setter [IrCall] desugared from an assignment
|
||||
* `lhs = rhs` (origin [IrStatementOrigin.EQ]), returns the end offset of the assigned
|
||||
* value (the call's last value argument) when it extends past the call's own end offset,
|
||||
* or null otherwise.
|
||||
*
|
||||
* The K1 frontend records such a setter call (and its synthetic implicit-`this` receiver
|
||||
* and synthetic reflection arguments) at the assignment's left-hand side only
|
||||
* (`varResource0 = 3` -> `37:9:37:20`), whereas K2 spans the whole assignment through the
|
||||
* assigned value (`37:9:37:24`). The setter call *is* the desugaring of the whole
|
||||
* assignment, so K2's span is the more intuitive one; we converge K1 onto it. Returns null
|
||||
* under K2 (where the call already spans the assigned value, so no widening is needed) and
|
||||
* for any non-assignment call.
|
||||
*/
|
||||
private fun getSetterCallAssignedValueEndOffset(c: IrCall): Int? {
|
||||
if (c.origin != IrStatementOrigin.EQ) return null
|
||||
val n = c.codeQlValueArgumentsCount
|
||||
if (n == 0) return null
|
||||
val assignedValue = c.codeQlGetValueArgument(n - 1) ?: return null
|
||||
val valueEnd = assignedValue.endOffset
|
||||
if (
|
||||
valueEnd == UNDEFINED_OFFSET ||
|
||||
valueEnd == SYNTHETIC_OFFSET ||
|
||||
c.startOffset == UNDEFINED_OFFSET ||
|
||||
c.startOffset == SYNTHETIC_OFFSET ||
|
||||
c.endOffset == UNDEFINED_OFFSET ||
|
||||
c.endOffset == SYNTHETIC_OFFSET
|
||||
) {
|
||||
return null
|
||||
}
|
||||
if (valueEnd <= c.endOffset) return null
|
||||
return valueEnd
|
||||
}
|
||||
|
||||
private fun extractCall(
|
||||
c: IrCall,
|
||||
callable: Label<out DbCallable>,
|
||||
@@ -5977,7 +6011,29 @@ open class KotlinFileExtractor(
|
||||
}
|
||||
}
|
||||
else -> {
|
||||
extractMethodAccess(target, true, true)
|
||||
// A property/delegated-property setter call desugared from an assignment
|
||||
// `lhs = rhs` is anchored by K1 at the left-hand side only, while K2 spans the
|
||||
// whole assignment through the assigned value. Widen the K1 span to match by
|
||||
// remapping the call's exact offset pair (which its synthetic implicit-`this`
|
||||
// receiver and synthetic reflection arguments also carry) onto one that runs
|
||||
// through the assigned value. Real source arguments have their own offsets and
|
||||
// are unaffected; the remap is a no-op under K2 (helper returns null).
|
||||
val assignedValueEnd = getSetterCallAssignedValueEndOffset(c)
|
||||
if (assignedValueEnd != null) {
|
||||
val previousRemap = tw.scopedOffsetRemap
|
||||
tw.scopedOffsetRemap =
|
||||
Pair(
|
||||
Pair(c.startOffset, c.endOffset),
|
||||
Pair(c.startOffset, assignedValueEnd)
|
||||
)
|
||||
try {
|
||||
extractMethodAccess(target, true, true)
|
||||
} finally {
|
||||
tw.scopedOffsetRemap = previousRemap
|
||||
}
|
||||
} else {
|
||||
extractMethodAccess(target, true, true)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,9 +90,9 @@
|
||||
| delegatedProperties.kt:20:17:20:28 | <get-varResource1>(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:20:17:20:28 | Object | delegatedProperties.kt:18:5:40:5 | fn | TypeAccess |
|
||||
| delegatedProperties.kt:20:17:20:28 | new (...) | delegatedProperties.kt:18:5:40:5 | fn | ClassInstanceExpr |
|
||||
| delegatedProperties.kt:21:9:21:20 | <set-varResource1>(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:21:9:21:20 | Object | delegatedProperties.kt:18:5:40:5 | fn | TypeAccess |
|
||||
| delegatedProperties.kt:21:9:21:20 | new (...) | delegatedProperties.kt:18:5:40:5 | fn | ClassInstanceExpr |
|
||||
| delegatedProperties.kt:21:9:21:24 | <set-varResource1>(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:21:9:21:24 | Object | delegatedProperties.kt:18:5:40:5 | fn | TypeAccess |
|
||||
| delegatedProperties.kt:21:9:21:24 | new (...) | delegatedProperties.kt:18:5:40:5 | fn | ClassInstanceExpr |
|
||||
| delegatedProperties.kt:21:24:21:24 | 2 | delegatedProperties.kt:18:5:40:5 | fn | IntegerLiteral |
|
||||
| delegatedProperties.kt:23:9:23:31 | String | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| delegatedProperties.kt:23:9:23:31 | name$delegate | delegatedProperties.kt:18:5:40:5 | fn | LocalVariableDeclExpr |
|
||||
@@ -140,8 +140,8 @@
|
||||
| delegatedProperties.kt:28:50:28:71 | ? ... | file://:0:0:0:0 | <none> | WildcardTypeAccess |
|
||||
| delegatedProperties.kt:28:50:28:71 | KProperty<?> | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| delegatedProperties.kt:28:74:28:83 | int | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| delegatedProperties.kt:29:17:29:24 | setCurValue(...) | delegatedProperties.kt:28:22:30:13 | setValue | MethodCall |
|
||||
| delegatedProperties.kt:29:17:29:24 | this | delegatedProperties.kt:28:22:30:13 | setValue | ThisAccess |
|
||||
| delegatedProperties.kt:29:17:29:32 | setCurValue(...) | delegatedProperties.kt:28:22:30:13 | setValue | MethodCall |
|
||||
| delegatedProperties.kt:29:17:29:32 | this | delegatedProperties.kt:28:22:30:13 | setValue | ThisAccess |
|
||||
| delegatedProperties.kt:29:28:29:32 | value | delegatedProperties.kt:28:22:30:13 | setValue | VarAccess |
|
||||
| delegatedProperties.kt:33:9:33:76 | int | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| delegatedProperties.kt:33:9:33:76 | readOnly$delegate | delegatedProperties.kt:18:5:40:5 | fn | LocalVariableDeclExpr |
|
||||
@@ -198,8 +198,8 @@
|
||||
| delegatedProperties.kt:36:9:36:29 | println(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:36:17:36:28 | getVarResource0(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:36:17:36:28 | this | delegatedProperties.kt:18:5:40:5 | fn | ThisAccess |
|
||||
| delegatedProperties.kt:37:9:37:20 | setVarResource0(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:37:9:37:20 | this | delegatedProperties.kt:18:5:40:5 | fn | ThisAccess |
|
||||
| delegatedProperties.kt:37:9:37:24 | setVarResource0(...) | delegatedProperties.kt:18:5:40:5 | fn | MethodCall |
|
||||
| delegatedProperties.kt:37:9:37:24 | this | delegatedProperties.kt:18:5:40:5 | fn | ThisAccess |
|
||||
| delegatedProperties.kt:37:24:37:24 | 3 | delegatedProperties.kt:18:5:40:5 | fn | IntegerLiteral |
|
||||
| delegatedProperties.kt:39:9:39:51 | int | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| delegatedProperties.kt:39:9:39:51 | varResource2$delegate | delegatedProperties.kt:18:5:40:5 | fn | LocalVariableDeclExpr |
|
||||
|
||||
@@ -7,7 +7,7 @@ calls
|
||||
| Test.java:27:5:27:24 | getter(...) | Test.java:16:22:16:25 | user | Test.java:14:14:14:17 | Test | Generic2.class:0:0:0:0 | getter | Generic2.class:0:0:0:0 | Generic2<? super String> |
|
||||
| test.kt:5:32:5:46 | identity(...) | test.kt:5:3:5:46 | identity2 | test.kt:1:1:13:1 | Generic | test.kt:6:3:6:35 | identity | test.kt:1:1:13:1 | Generic |
|
||||
| test.kt:7:21:7:26 | getStored(...) | test.kt:7:3:7:26 | getter | test.kt:1:1:13:1 | Generic | test.kt:3:3:3:12 | getStored | test.kt:1:1:13:1 | Generic |
|
||||
| test.kt:8:26:8:31 | setStored(...) | test.kt:8:3:8:41 | setter | test.kt:1:1:13:1 | Generic | test.kt:3:3:3:12 | setStored | test.kt:1:1:13:1 | Generic |
|
||||
| test.kt:8:26:8:39 | setStored(...) | test.kt:8:3:8:41 | setter | test.kt:1:1:13:1 | Generic | test.kt:3:3:3:12 | setStored | test.kt:1:1:13:1 | Generic |
|
||||
| test.kt:11:44:11:70 | privateid(...) | test.kt:11:3:11:70 | callPrivateId | test.kt:1:1:13:1 | Generic | file:///!unknown-binary-location/Generic.class:0:0:0:0 | privateid | file:///!unknown-binary-location/Generic.class:0:0:0:0 | Generic<String> |
|
||||
| test.kt:18:3:18:35 | identity(...) | test.kt:15:1:28:1 | user | test.kt:0:0:0:0 | TestKt | file:///!unknown-binary-location/Generic.class:0:0:0:0 | identity | file:///!unknown-binary-location/Generic.class:0:0:0:0 | Generic<String> |
|
||||
| test.kt:19:3:19:36 | identity2(...) | test.kt:15:1:28:1 | user | test.kt:0:0:0:0 | TestKt | file:///!unknown-binary-location/Generic.class:0:0:0:0 | identity2 | file:///!unknown-binary-location/Generic.class:0:0:0:0 | Generic<String> |
|
||||
|
||||
@@ -32,19 +32,19 @@ staticMembers
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:16:16:16:43 | setStaticProp(...) | test.kt:16:16:16:43 | HasCompanion.Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:20:18:20:45 | getPropWithStaticGetter(...) | test.kt:20:18:20:45 | HasCompanion.Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:20:26:20:45 | getPropWithStaticSetter(...) | test.kt:20:26:20:45 | this | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:21:24:21:43 | setPropWithStaticSetter(...) | test.kt:21:24:21:43 | this | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:21:24:21:47 | setPropWithStaticSetter(...) | test.kt:21:24:21:47 | this | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:24:15:24:34 | getPropWithStaticGetter(...) | test.kt:24:15:24:34 | this | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:25:18:25:60 | setPropWithStaticSetter(...) | test.kt:25:18:25:60 | HasCompanion.Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:25:35:25:54 | setPropWithStaticGetter(...) | test.kt:25:35:25:54 | this | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:25:35:25:58 | setPropWithStaticGetter(...) | test.kt:25:35:25:58 | this | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:52:3:52:32 | staticMethod(...) | test.kt:52:3:52:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:53:3:53:35 | nonStaticMethod(...) | test.kt:53:3:53:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:54:3:54:25 | setStaticProp(...) | test.kt:54:3:54:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:54:3:54:54 | setStaticProp(...) | test.kt:54:3:54:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:54:29:54:54 | getNonStaticProp(...) | test.kt:54:29:54:40 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:55:3:55:28 | setNonStaticProp(...) | test.kt:55:3:55:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:55:3:55:54 | setNonStaticProp(...) | test.kt:55:3:55:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:55:32:55:54 | getStaticProp(...) | test.kt:55:32:55:43 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:56:3:56:35 | setPropWithStaticGetter(...) | test.kt:56:3:56:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:56:3:56:71 | setPropWithStaticGetter(...) | test.kt:56:3:56:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:56:39:56:71 | getPropWithStaticSetter(...) | test.kt:56:39:56:50 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:57:3:57:35 | setPropWithStaticSetter(...) | test.kt:57:3:57:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:57:3:57:71 | setPropWithStaticSetter(...) | test.kt:57:3:57:14 | Companion | instance |
|
||||
| test.kt:11:3:27:3 | Companion | test.kt:57:39:57:71 | getPropWithStaticGetter(...) | test.kt:57:39:57:50 | Companion | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | JavaUser.java:13:5:13:34 | staticMethod(...) | JavaUser.java:13:5:13:16 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | JavaUser.java:14:5:14:46 | nonStaticMethod(...) | JavaUser.java:14:5:14:25 | NonCompanion.INSTANCE | instance |
|
||||
@@ -59,16 +59,16 @@ staticMembers
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:33:52:33:69 | nonStaticMethod(...) | test.kt:33:52:33:69 | NonCompanion.INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:34:44:34:58 | staticMethod(...) | test.kt:34:44:34:58 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:40:24:40:43 | getPropWithStaticSetter(...) | test.kt:40:24:40:43 | NonCompanion.INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:41:22:41:41 | setPropWithStaticSetter(...) | test.kt:41:22:41:41 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:41:22:41:45 | setPropWithStaticSetter(...) | test.kt:41:22:41:45 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:44:13:44:32 | getPropWithStaticGetter(...) | test.kt:44:13:44:32 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:45:33:45:52 | setPropWithStaticGetter(...) | test.kt:45:33:45:52 | NonCompanion.INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:45:33:45:56 | setPropWithStaticGetter(...) | test.kt:45:33:45:56 | NonCompanion.INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:60:3:60:32 | staticMethod(...) | test.kt:60:3:60:32 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:61:3:61:35 | nonStaticMethod(...) | test.kt:61:3:61:14 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:62:3:62:25 | setStaticProp(...) | test.kt:62:3:62:25 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:62:3:62:54 | setStaticProp(...) | test.kt:62:3:62:54 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:62:29:62:54 | getNonStaticProp(...) | test.kt:62:29:62:40 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:63:3:63:28 | setNonStaticProp(...) | test.kt:63:3:63:14 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:63:3:63:54 | setNonStaticProp(...) | test.kt:63:3:63:14 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:63:32:63:54 | getStaticProp(...) | test.kt:63:32:63:54 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:64:3:64:35 | setPropWithStaticGetter(...) | test.kt:64:3:64:14 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:64:3:64:71 | setPropWithStaticGetter(...) | test.kt:64:3:64:14 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:64:39:64:71 | getPropWithStaticSetter(...) | test.kt:64:39:64:50 | INSTANCE | instance |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:65:3:65:35 | setPropWithStaticSetter(...) | test.kt:65:3:65:35 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:65:3:65:71 | setPropWithStaticSetter(...) | test.kt:65:3:65:71 | NonCompanion | static |
|
||||
| test.kt:31:1:47:1 | NonCompanion | test.kt:65:39:65:71 | getPropWithStaticGetter(...) | test.kt:65:39:65:71 | NonCompanion | static |
|
||||
|
||||
Reference in New Issue
Block a user