mirror of
https://github.com/github/codeql.git
synced 2026-07-26 21:44:02 +02:00
Kotlin: locate in-place-update LHS at the variable, not the whole assignment
For a desugared in-place update such as `v += e` the extractor emits an `AssignAddExpr` (etc.) whose left-hand side is a `VarAccess` of `v`. The location of that `VarAccess` was taken from the underlying `IrSetValue` node (`getLocation(e)`). The two frontends record that node's end offset differently: K1 ends it at the left-hand side (so the access spanned just `v`), whereas K2 ends it past the whole assignment (so the access spanned all of `v += e`, redundantly identical to its parent `AssignAddExpr`). The K1 span is the more intuitive and information-preserving one: a variable access should point at the variable reference, not repeat the enclosing assignment's span. Converge K2 onto it. The desugaring represents `v += e` as `v = get(v).op(e)`, so the update's right-hand call already contains an `IrGetValue` receiver that reads `v`. That receiver's source span is exactly the `v` identifier in both frontends, which makes it a frontend-independent anchor for the LHS location. `getUpdateInPlaceReceiver` recovers it (mirroring the existing `getUpdateInPlaceRHS`), and the LHS `VarAccess` is located there. This is fail-closed: when the node is not such an in-place update, or the receiver lacks a usable (non-synthetic, defined) source span, the raw `getLocation(e)` is kept. Under K1 the recovered span equals the previous one, so K1 output is unchanged. Only the five in-place operators in the `test-kotlin2` `exprs` test change, each narrowing the LHS `updated` `VarAccess` from `270:3:270:14` (the whole `updated += 1`) to `270:3:270:9` (the identifier), matching `test-kotlin1` exactly. Prefix/postfix increment/decrement use a different desugaring (with a temporary) and are not in-place updates in this sense; they are left for a separate change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -6292,6 +6292,22 @@ open class KotlinFileExtractor(
|
||||
else -> null
|
||||
}
|
||||
|
||||
/**
|
||||
* For a desugared in-place update such as `v += e` (represented as `v = get(v).op(e)`),
|
||||
* returns the `IrGetValue` receiver that reads `v`. Its source span is the left-hand-side
|
||||
* variable reference (the identifier only) in both the K1 and K2 frontends, so it provides a
|
||||
* frontend-independent location for the update's LHS `VarAccess`. Returns null when [e] is not
|
||||
* such an in-place update, in which case callers keep the raw location.
|
||||
*/
|
||||
private fun getUpdateInPlaceReceiver(e: IrSetValue): IrGetValue? {
|
||||
val op = getStatementOriginOperator(e.origin) ?: return null
|
||||
val rhs = e.value
|
||||
if (rhs !is IrCall || !isNumericFunction(rhs.symbol.owner, op)) return null
|
||||
val receiver = rhs.dispatchReceiver
|
||||
return if (receiver is IrGetValue && receiver.symbol.owner == e.symbol.owner) receiver
|
||||
else null
|
||||
}
|
||||
|
||||
private fun getUpdateInPlaceRHS(
|
||||
origin: IrStatementOrigin?,
|
||||
isExpectedLhs: (IrExpression?) -> Boolean,
|
||||
@@ -7065,7 +7081,22 @@ open class KotlinFileExtractor(
|
||||
extractExprContext(id, locId, callable, exprParent.enclosingStmt)
|
||||
|
||||
val lhsId = tw.getFreshIdLabel<DbVaraccess>()
|
||||
val lhsLocId = tw.getLocation(e)
|
||||
// For a desugared in-place update (`v += e`) the K2 frontend records the set
|
||||
// operation's end offset past the whole assignment, so `getLocation(e)` would
|
||||
// span `v += e` rather than just `v`. Locate the LHS `VarAccess` at the update's
|
||||
// receiver read of `v` instead, whose span is the identifier in both frontends;
|
||||
// fall back to the raw location when this is not such an in-place update or the
|
||||
// receiver lacks a usable source span.
|
||||
val lhsLocId =
|
||||
(e as? IrSetValue)
|
||||
?.let { getUpdateInPlaceReceiver(it) }
|
||||
?.takeIf {
|
||||
it.startOffset != UNDEFINED_OFFSET &&
|
||||
it.endOffset != UNDEFINED_OFFSET &&
|
||||
it.startOffset != SYNTHETIC_OFFSET &&
|
||||
it.endOffset != SYNTHETIC_OFFSET
|
||||
}
|
||||
?.let { tw.getLocation(it) } ?: tw.getLocation(e)
|
||||
extractExprContext(lhsId, lhsLocId, callable, exprParent.enclosingStmt)
|
||||
|
||||
when (e) {
|
||||
|
||||
@@ -1743,20 +1743,20 @@
|
||||
| exprs.kt:267:1:276:1 | Unit | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| exprs.kt:269:3:269:17 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | LocalVariableDeclExpr |
|
||||
| exprs.kt:269:17:269:17 | 0 | exprs.kt:267:1:276:1 | inPlaceOperators | IntegerLiteral |
|
||||
| exprs.kt:270:3:270:9 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:270:3:270:14 | ...+=... | exprs.kt:267:1:276:1 | inPlaceOperators | AssignAddExpr |
|
||||
| exprs.kt:270:3:270:14 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:270:14:270:14 | 1 | exprs.kt:267:1:276:1 | inPlaceOperators | IntegerLiteral |
|
||||
| exprs.kt:271:3:271:9 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:271:3:271:14 | ...-=... | exprs.kt:267:1:276:1 | inPlaceOperators | AssignSubExpr |
|
||||
| exprs.kt:271:3:271:14 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:271:14:271:14 | 1 | exprs.kt:267:1:276:1 | inPlaceOperators | IntegerLiteral |
|
||||
| exprs.kt:272:3:272:9 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:272:3:272:14 | ...*=... | exprs.kt:267:1:276:1 | inPlaceOperators | AssignMulExpr |
|
||||
| exprs.kt:272:3:272:14 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:272:14:272:14 | 1 | exprs.kt:267:1:276:1 | inPlaceOperators | IntegerLiteral |
|
||||
| exprs.kt:273:3:273:9 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:273:3:273:14 | .../=... | exprs.kt:267:1:276:1 | inPlaceOperators | AssignDivExpr |
|
||||
| exprs.kt:273:3:273:14 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:273:14:273:14 | 1 | exprs.kt:267:1:276:1 | inPlaceOperators | IntegerLiteral |
|
||||
| exprs.kt:274:3:274:9 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:274:3:274:14 | ...%=... | exprs.kt:267:1:276:1 | inPlaceOperators | AssignRemExpr |
|
||||
| exprs.kt:274:3:274:14 | updated | exprs.kt:267:1:276:1 | inPlaceOperators | VarAccess |
|
||||
| exprs.kt:274:14:274:14 | 1 | exprs.kt:267:1:276:1 | inPlaceOperators | IntegerLiteral |
|
||||
| exprs.kt:278:8:278:66 | T | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
| exprs.kt:278:8:278:66 | T[] | file://:0:0:0:0 | <none> | TypeAccess |
|
||||
|
||||
Reference in New Issue
Block a user