Skip to content

Commit b67ac6b

Browse files
andersfugmannCopilot
andersfugmann
and
Copilot
committed
fix property and accessor locations using PSI
K1 IrProperty.startOffset includes leading modifiers (private, abstract, lateinit, annotations) in the span start; K2 already starts at val/var. Walk the PSI tree from p.startOffset to the enclosing KtProperty, then use valOrVarKeyword.startOffset as the declaration start. This yields a consistent location spanning val/var through the end of the full property declaration (including explicit getter/setter bodies) in both K1 and K2. The PSI-based location is restricted to unspecialised extractions (classTypeArgsIncludingOuterClasses.isNullOrEmpty()). Specialised generic instances (e.g. C<String>.prop) continue to use the binary whole-file location returned by getLocation(p, typeArgs), preserving the existing behaviour that keeps them absent from fromSource() queries. Synthesised accessors (DEFAULT_PROPERTY_ACCESSOR origin) represent the entire property declaration, so they now share the property's location via accessorOverride(). Explicit getter/setter bodies keep their own independently computed location. The visibility merge in extractFunction is extended to accept an overriddenAttributes parameter from the caller; the internal fake-override visibility adjustment (DescriptorVisibilities.PUBLIC for Java binary Object methods) is merged with any caller-supplied attributes so that neither overrides the other silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4f8ea3d commit b67ac6b

4 files changed

Lines changed: 93 additions & 28 deletions

File tree

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

Lines changed: 73 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,7 +1748,8 @@ open class KotlinFileExtractor(
17481748
extractMethodAndParameterTypeAccesses: Boolean,
17491749
extractAnnotations: Boolean,
17501750
typeSubstitution: TypeSubstitution?,
1751-
classTypeArgsIncludingOuterClasses: List<IrTypeArgument>?
1751+
classTypeArgsIncludingOuterClasses: List<IrTypeArgument>?,
1752+
overriddenAttributes: OverriddenFunctionAttributes? = null
17521753
) =
17531754
if (isFake(f)) {
17541755
if (needsInterfaceForwarder(f))
@@ -1766,8 +1767,18 @@ open class KotlinFileExtractor(
17661767
// specifically in interfaces loaded from Java classes show up like fake overrides.
17671768
val overriddenVisibility =
17681769
if (f.isFakeOverride && isJavaBinaryObjectMethodRedeclaration(f))
1769-
OverriddenFunctionAttributes(visibility = DescriptorVisibilities.PUBLIC)
1770+
DescriptorVisibilities.PUBLIC
17701771
else null
1772+
// Merge caller-supplied overrides with the internal visibility adjustment.
1773+
val mergedAttributes = when {
1774+
overriddenAttributes == null && overriddenVisibility == null -> null
1775+
overriddenAttributes == null -> OverriddenFunctionAttributes(visibility = overriddenVisibility)
1776+
overriddenVisibility == null -> overriddenAttributes
1777+
else -> overriddenAttributes.copy(
1778+
visibility = overriddenVisibility.takeUnless { overriddenAttributes.visibility != null }
1779+
?: overriddenAttributes.visibility
1780+
)
1781+
}
17711782
forceExtractFunction(
17721783
f,
17731784
parentId,
@@ -1776,7 +1787,7 @@ open class KotlinFileExtractor(
17761787
extractAnnotations,
17771788
typeSubstitution,
17781789
classTypeArgsIncludingOuterClasses,
1779-
overriddenAttributes = overriddenVisibility
1790+
overriddenAttributes = mergedAttributes
17801791
)
17811792
.also {
17821793
// The defaults-forwarder function is a static utility, not a member, so we only
@@ -2659,16 +2670,28 @@ open class KotlinFileExtractor(
26592670

26602671
DeclarationStackAdjuster(p).use {
26612672
val id = useProperty(p, parentId, classTypeArgsIncludingOuterClasses)
2673+
// PSI location is only used for the primary (unspecialised) extraction. Specialised
2674+
// generic instances defer to getLocation so that the backing binary-file location is
2675+
// preserved, keeping specialised properties absent from fromSource() queries.
26622676
val locId =
2663-
if (usesK2) getPsiBasedLocation(p) ?: getLocation(p, classTypeArgsIncludingOuterClasses)
2664-
else getLocation(p, classTypeArgsIncludingOuterClasses)
2677+
if (classTypeArgsIncludingOuterClasses.isNullOrEmpty())
2678+
getPsiBasedLocation(p) ?: getLocation(p, classTypeArgsIncludingOuterClasses)
2679+
else
2680+
getLocation(p, classTypeArgsIncludingOuterClasses)
26652681
tw.writeKtProperties(id, p.name.asString())
26662682
tw.writeHasLocation(id, locId)
26672683

26682684
val bf = p.backingField
26692685
val getter = p.getter
26702686
val setter = p.setter
26712687

2688+
// Synthesised (DEFAULT_PROPERTY_ACCESSOR) getter and setter should point
2689+
// at the property head, not the initialiser or custom accessor body.
2690+
fun accessorOverride(f: IrFunction) =
2691+
if (f.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR)
2692+
getPsiBasedAccessorLocation(p)?.let { OverriddenFunctionAttributes(sourceLoc = it) }
2693+
else null
2694+
26722695
if (getter == null) {
26732696
if (!isExternalDeclaration(p)) {
26742697
logger.warnElement("IrProperty without a getter", p)
@@ -2682,7 +2705,8 @@ open class KotlinFileExtractor(
26822705
extractMethodAndParameterTypeAccesses = extractFunctionBodies,
26832706
extractAnnotations = extractAnnotations,
26842707
typeSubstitution,
2685-
classTypeArgsIncludingOuterClasses
2708+
classTypeArgsIncludingOuterClasses,
2709+
overriddenAttributes = accessorOverride(getter)
26862710
)
26872711
?.cast<DbMethod>()
26882712
if (getterId != null) {
@@ -2712,7 +2736,8 @@ open class KotlinFileExtractor(
27122736
extractMethodAndParameterTypeAccesses = extractFunctionBodies,
27132737
extractAnnotations = extractAnnotations,
27142738
typeSubstitution,
2715-
classTypeArgsIncludingOuterClasses
2739+
classTypeArgsIncludingOuterClasses,
2740+
overriddenAttributes = accessorOverride(setter)
27162741
)
27172742
?.cast<DbMethod>()
27182743
if (setterId != null) {
@@ -2931,6 +2956,46 @@ open class KotlinFileExtractor(
29312956
return tw.getLocation(declStart, declaration.endOffset)
29322957
}
29332958

2959+
/**
2960+
* Returns the PSI-based location for a property declaration, spanning from
2961+
* the `val`/`var` keyword to the end of the full property declaration (including
2962+
* any explicit getter/setter body).
2963+
*
2964+
* IR offsets are inconsistent across K1 and K2:
2965+
* - K1 [IrProperty.startOffset] includes leading modifiers (private, abstract,
2966+
* lateinit, @Annotation) in the span start. K2 starts at `val`/`var`.
2967+
* - K1 [IrProperty.endOffset] stops at the end of the declaration line and
2968+
* does not include an explicit getter/setter body on a subsequent line. K2
2969+
* includes the getter/setter body.
2970+
*
2971+
* Both are resolved by walking the PSI tree: the [KtProperty] node covers the
2972+
* full declaration including getter/setter, and [KtProperty.valOrVarKeyword]
2973+
* gives the correct start, excluding modifiers.
2974+
*/
2975+
private fun getPsiBasedLocation(p: IrProperty): Label<DbLocation>? {
2976+
if (p.startOffset < 0 || p.endOffset < 0) return null
2977+
val file = currentIrFile ?: return null
2978+
val ktFile = getPsi2Ir()?.getKtFile(file) ?: return null
2979+
val ktProperty = ktFile.findElementAt(p.startOffset)
2980+
?.let { leaf -> generateSequence(leaf) { it.parent }.filterIsInstance<KtProperty>().firstOrNull() }
2981+
?: return null
2982+
val declStart = ktProperty.valOrVarKeyword.startOffset
2983+
val declEnd = ktProperty.endOffset
2984+
return tw.getLocation(declStart, declEnd)
2985+
}
2986+
2987+
private fun getPsiBasedAccessorLocation(p: IrProperty): Label<DbLocation>? {
2988+
if (p.startOffset < 0 || p.endOffset < 0) return null
2989+
val file = currentIrFile ?: return null
2990+
val ktFile = getPsi2Ir()?.getKtFile(file) ?: return null
2991+
val ktProperty = ktFile.findElementAt(p.startOffset)
2992+
?.let { leaf -> generateSequence(leaf) { it.parent }.filterIsInstance<KtProperty>().firstOrNull() }
2993+
?: return null
2994+
val declStart = ktProperty.valOrVarKeyword.startOffset
2995+
val declEnd = ktProperty.nameIdentifier?.endOffset ?: ktProperty.valOrVarKeyword.endOffset
2996+
return tw.getLocation(declStart, declEnd)
2997+
}
2998+
29342999
private fun extractVariable(
29353000
v: IrVariable,
29363001
callable: Label<out DbCallable>,
@@ -2957,7 +3022,7 @@ open class KotlinFileExtractor(
29573022
with("variable expr", v) {
29583023
val varId = useVariable(v)
29593024
val exprId = tw.getFreshIdLabel<DbLocalvariabledeclexpr>()
2960-
val locId = getPsiBasedLocation(v) ?: tw.getLocation(getVariableLocationProvider(v))
3025+
val locId = getPsiBasedLocation(v as IrElement) ?: tw.getLocation(getVariableLocationProvider(v))
29613026
val type = useType(v.type)
29623027
tw.writeLocalvars(varId, v.name.asString(), type.javaResult.id, exprId)
29633028
tw.writeLocalvarsKotlinType(varId, type.kotlinResult.id)

‎java/ql/test-kotlin1/library-tests/comments/comments.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ comments
1919
commentOwners
2020
| comments.kt:4:1:11:3 | /**\n * A group of *members*.\n *\n * This class has no useful logic; it's just a documentation example.\n *\n * @property name the name of this group.\n * @constructor Creates an empty group.\n */ | comments.kt:12:1:31:1 | Group |
2121
| comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:5:17:46 | members |
22-
| comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:5:17:46 | members |
2322
| comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:13:17:46 | getMembers$private |
23+
| comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:13:17:46 | members |
2424
| comments.kt:19:5:22:7 | /**\n * Adds a [member] to this group.\n * @return the new size of the group.\n */ | comments.kt:23:5:26:5 | add |
2525
| comments.kt:35:5:35:34 | /** Medium is in the middle */ | comments.kt:36:5:36:14 | Medium |
2626
| comments.kt:37:5:37:23 | /** This is high */ | comments.kt:38:5:38:11 | High |

‎java/ql/test-kotlin1/library-tests/modifiers/modifiers.expected‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@
22
| modifiers.kt:1:6:29:1 | X | Constructor | public |
33
| modifiers.kt:2:5:2:21 | a | Field | final |
44
| modifiers.kt:2:5:2:21 | a | Field | private |
5-
| modifiers.kt:2:5:2:21 | a | Property | private |
5+
| modifiers.kt:2:13:2:21 | a | Property | private |
66
| modifiers.kt:2:13:2:21 | getA$private | Method | final |
77
| modifiers.kt:2:13:2:21 | getA$private | Method | private |
88
| modifiers.kt:3:5:3:23 | b | Field | final |
99
| modifiers.kt:3:5:3:23 | b | Field | private |
10-
| modifiers.kt:3:5:3:23 | b | Property | protected |
10+
| modifiers.kt:3:15:3:23 | b | Property | protected |
1111
| modifiers.kt:3:15:3:23 | getB | Method | final |
1212
| modifiers.kt:3:15:3:23 | getB | Method | protected |
1313
| modifiers.kt:4:5:4:22 | c | Field | final |
1414
| modifiers.kt:4:5:4:22 | c | Field | private |
15-
| modifiers.kt:4:5:4:22 | c | Property | internal |
15+
| modifiers.kt:4:14:4:22 | c | Property | internal |
1616
| modifiers.kt:4:14:4:22 | getC$main | Method | final |
1717
| modifiers.kt:4:14:4:22 | getC$main | Method | internal |
1818
| modifiers.kt:5:5:5:34 | d | Field | final |
@@ -25,7 +25,7 @@
2525
| modifiers.kt:7:15:9:5 | Nested | Constructor | public |
2626
| modifiers.kt:8:9:8:29 | e | Field | final |
2727
| modifiers.kt:8:9:8:29 | e | Field | private |
28-
| modifiers.kt:8:9:8:29 | e | Property | public |
28+
| modifiers.kt:8:16:8:29 | e | Property | public |
2929
| modifiers.kt:8:16:8:29 | getE | Method | final |
3030
| modifiers.kt:8:16:8:29 | getE | Method | public |
3131
| modifiers.kt:11:5:15:5 | fn1 | Method | final |
@@ -76,12 +76,12 @@
7676
| modifiers.kt:35:1:41:1 | LateInit | Class | public |
7777
| modifiers.kt:35:8:41:1 | LateInit | Constructor | public |
7878
| modifiers.kt:36:5:36:40 | test0 | Field | private |
79-
| modifiers.kt:36:5:36:40 | test0 | Property | lateinit |
80-
| modifiers.kt:36:5:36:40 | test0 | Property | private |
8179
| modifiers.kt:36:22:36:40 | getTest0$private | Method | final |
8280
| modifiers.kt:36:22:36:40 | getTest0$private | Method | private |
8381
| modifiers.kt:36:22:36:40 | setTest0$private | Method | final |
8482
| modifiers.kt:36:22:36:40 | setTest0$private | Method | private |
83+
| modifiers.kt:36:22:36:40 | test0 | Property | lateinit |
84+
| modifiers.kt:36:22:36:40 | test0 | Property | private |
8585
| modifiers.kt:38:5:40:5 | fn | Method | final |
8686
| modifiers.kt:38:5:40:5 | fn | Method | public |
8787
| modifiers.kt:39:18:39:36 | LateInit test1 | LocalVariableDecl | lateinit |

‎java/ql/test-kotlin1/library-tests/properties/properties.expected‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,30 @@
44
| properties.kt:3:5:3:25 | modifiableInt | properties.kt:3:5:3:25 | getModifiableInt | properties.kt:3:5:3:25 | setModifiableInt | properties.kt:3:5:3:25 | modifiableInt | public |
55
| properties.kt:4:5:4:24 | immutableInt | properties.kt:4:5:4:24 | getImmutableInt | file://:0:0:0:0 | <none> | properties.kt:4:5:4:24 | immutableInt | public |
66
| properties.kt:5:5:5:26 | typedProp | properties.kt:5:5:5:26 | getTypedProp | file://:0:0:0:0 | <none> | properties.kt:5:5:5:26 | typedProp | public |
7-
| properties.kt:6:5:6:38 | abstractTypeProp | properties.kt:6:14:6:38 | getAbstractTypeProp | file://:0:0:0:0 | <none> | file://:0:0:0:0 | <none> | public |
7+
| properties.kt:6:14:6:38 | abstractTypeProp | properties.kt:6:14:6:38 | getAbstractTypeProp | file://:0:0:0:0 | <none> | file://:0:0:0:0 | <none> | public |
88
| properties.kt:7:5:7:30 | initialisedInInit | properties.kt:7:5:7:30 | getInitialisedInInit | file://:0:0:0:0 | <none> | properties.kt:7:5:7:30 | initialisedInInit | public |
99
| properties.kt:11:5:11:40 | useConstructorArg | properties.kt:11:5:11:40 | getUseConstructorArg | file://:0:0:0:0 | <none> | properties.kt:11:5:11:40 | useConstructorArg | public |
1010
| properties.kt:12:5:13:21 | five | properties.kt:13:13:13:21 | getFive | file://:0:0:0:0 | <none> | file://:0:0:0:0 | <none> | public |
1111
| properties.kt:14:5:15:21 | six | properties.kt:15:13:15:21 | getSix | file://:0:0:0:0 | <none> | file://:0:0:0:0 | <none> | public |
1212
| properties.kt:16:5:18:40 | getSet | properties.kt:17:13:17:33 | getGetSet | properties.kt:18:13:18:40 | setGetSet | file://:0:0:0:0 | <none> | public |
13-
| properties.kt:19:5:20:15 | defaultGetter | properties.kt:20:13:20:15 | getDefaultGetter | file://:0:0:0:0 | <none> | properties.kt:19:5:20:15 | defaultGetter | public |
14-
| properties.kt:21:5:22:15 | varDefaultGetter | properties.kt:22:13:22:15 | getVarDefaultGetter | properties.kt:21:5:22:15 | setVarDefaultGetter | properties.kt:21:5:22:15 | varDefaultGetter | public |
15-
| properties.kt:23:5:24:15 | varDefaultSetter | properties.kt:23:5:24:15 | getVarDefaultSetter | properties.kt:24:13:24:15 | setVarDefaultSetter | properties.kt:23:5:24:15 | varDefaultSetter | public |
16-
| properties.kt:25:5:27:15 | varDefaultGetterSetter | properties.kt:26:13:26:15 | getVarDefaultGetterSetter | properties.kt:27:13:27:15 | setVarDefaultGetterSetter | properties.kt:25:5:27:15 | varDefaultGetterSetter | public |
13+
| properties.kt:19:5:20:15 | defaultGetter | properties.kt:19:5:20:15 | getDefaultGetter | file://:0:0:0:0 | <none> | properties.kt:19:5:20:15 | defaultGetter | public |
14+
| properties.kt:21:5:22:15 | varDefaultGetter | properties.kt:21:5:22:15 | getVarDefaultGetter | properties.kt:21:5:22:15 | setVarDefaultGetter | properties.kt:21:5:22:15 | varDefaultGetter | public |
15+
| properties.kt:23:5:24:15 | varDefaultSetter | properties.kt:23:5:24:15 | getVarDefaultSetter | properties.kt:23:5:24:15 | setVarDefaultSetter | properties.kt:23:5:24:15 | varDefaultSetter | public |
16+
| properties.kt:25:5:27:15 | varDefaultGetterSetter | properties.kt:25:5:27:15 | getVarDefaultGetterSetter | properties.kt:25:5:27:15 | setVarDefaultGetterSetter | properties.kt:25:5:27:15 | varDefaultGetterSetter | public |
1717
| properties.kt:28:5:29:22 | overrideGetter | properties.kt:29:13:29:22 | getOverrideGetter | properties.kt:28:5:29:22 | setOverrideGetter | properties.kt:28:5:29:22 | overrideGetter | public |
1818
| properties.kt:30:5:31:29 | overrideGetterUseField | properties.kt:31:13:31:29 | getOverrideGetterUseField | properties.kt:30:5:31:29 | setOverrideGetterUseField | properties.kt:30:5:31:29 | overrideGetterUseField | public |
1919
| properties.kt:32:5:33:29 | useField | properties.kt:33:13:33:29 | getUseField | file://:0:0:0:0 | <none> | properties.kt:32:5:33:29 | useField | public |
20-
| properties.kt:34:5:34:36 | lateInitVar | properties.kt:34:14:34:36 | getLateInitVar | properties.kt:34:14:34:36 | setLateInitVar | properties.kt:34:5:34:36 | lateInitVar | lateinit, public |
21-
| properties.kt:35:5:35:32 | privateProp | properties.kt:35:13:35:32 | getPrivateProp$private | file://:0:0:0:0 | <none> | properties.kt:35:5:35:32 | privateProp | private |
22-
| properties.kt:36:5:36:36 | protectedProp | properties.kt:36:15:36:36 | getProtectedProp | file://:0:0:0:0 | <none> | properties.kt:36:5:36:36 | protectedProp | protected |
23-
| properties.kt:37:5:37:30 | publicProp | properties.kt:37:12:37:30 | getPublicProp | file://:0:0:0:0 | <none> | properties.kt:37:5:37:30 | publicProp | public |
24-
| properties.kt:38:5:38:34 | internalProp | properties.kt:38:14:38:34 | getInternalProp$main | file://:0:0:0:0 | <none> | properties.kt:38:5:38:34 | internalProp | internal |
25-
| properties.kt:67:1:67:23 | constVal | properties.kt:67:7:67:23 | getConstVal | file://:0:0:0:0 | <none> | properties.kt:67:1:67:23 | constVal | public |
20+
| properties.kt:34:14:34:36 | lateInitVar | properties.kt:34:14:34:36 | getLateInitVar | properties.kt:34:14:34:36 | setLateInitVar | properties.kt:34:5:34:36 | lateInitVar | lateinit, public |
21+
| properties.kt:35:13:35:32 | privateProp | properties.kt:35:13:35:32 | getPrivateProp$private | file://:0:0:0:0 | <none> | properties.kt:35:5:35:32 | privateProp | private |
22+
| properties.kt:36:15:36:36 | protectedProp | properties.kt:36:15:36:36 | getProtectedProp | file://:0:0:0:0 | <none> | properties.kt:36:5:36:36 | protectedProp | protected |
23+
| properties.kt:37:12:37:30 | publicProp | properties.kt:37:12:37:30 | getPublicProp | file://:0:0:0:0 | <none> | properties.kt:37:5:37:30 | publicProp | public |
24+
| properties.kt:38:14:38:34 | internalProp | properties.kt:38:14:38:34 | getInternalProp$main | file://:0:0:0:0 | <none> | properties.kt:38:5:38:34 | internalProp | internal |
25+
| properties.kt:67:7:67:23 | constVal | properties.kt:67:7:67:23 | getConstVal | file://:0:0:0:0 | <none> | properties.kt:67:1:67:23 | constVal | public |
2626
| properties.kt:70:5:70:16 | prop | properties.kt:70:5:70:16 | getProp | file://:0:0:0:0 | <none> | properties.kt:70:5:70:16 | prop | public |
2727
| properties.kt:78:1:79:13 | x | properties.kt:79:5:79:13 | getX | file://:0:0:0:0 | <none> | file://:0:0:0:0 | <none> | public |
2828
| properties.kt:80:1:81:13 | x | properties.kt:81:5:81:13 | getX | file://:0:0:0:0 | <none> | file://:0:0:0:0 | <none> | public |
29-
| properties.kt:84:5:84:29 | data | properties.kt:84:13:84:29 | getData$private | properties.kt:84:13:84:29 | setData$private | properties.kt:84:5:84:29 | data | private |
30-
| properties.kt:92:5:93:18 | data | properties.kt:93:9:93:18 | getData | properties.kt:92:13:93:18 | setData$private | properties.kt:92:5:93:18 | data | private |
29+
| properties.kt:84:13:84:29 | data | properties.kt:84:13:84:29 | getData$private | properties.kt:84:13:84:29 | setData$private | properties.kt:84:5:84:29 | data | private |
30+
| properties.kt:92:13:93:18 | data | properties.kt:93:9:93:18 | getData | properties.kt:92:13:93:18 | setData$private | properties.kt:92:5:93:18 | data | private |
3131
fieldDeclarations
3232
| properties.kt:2:27:2:50 | int constructorProp; | properties.kt:2:27:2:50 | constructorProp | 0 |
3333
| properties.kt:2:53:2:83 | int mutableConstructorProp; | properties.kt:2:53:2:83 | mutableConstructorProp | 0 |

0 commit comments

Comments
 (0)