diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt index ca67e28abcd..a26a6526f95 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt @@ -1748,7 +1748,8 @@ open class KotlinFileExtractor( extractMethodAndParameterTypeAccesses: Boolean, extractAnnotations: Boolean, typeSubstitution: TypeSubstitution?, - classTypeArgsIncludingOuterClasses: List? + classTypeArgsIncludingOuterClasses: List?, + overriddenAttributes: OverriddenFunctionAttributes? = null ) = if (isFake(f)) { if (needsInterfaceForwarder(f)) @@ -1766,8 +1767,18 @@ open class KotlinFileExtractor( // specifically in interfaces loaded from Java classes show up like fake overrides. val overriddenVisibility = if (f.isFakeOverride && isJavaBinaryObjectMethodRedeclaration(f)) - OverriddenFunctionAttributes(visibility = DescriptorVisibilities.PUBLIC) + DescriptorVisibilities.PUBLIC else null + // Merge caller-supplied overrides with the internal visibility adjustment. + val mergedAttributes = when { + overriddenAttributes == null && overriddenVisibility == null -> null + overriddenAttributes == null -> OverriddenFunctionAttributes(visibility = overriddenVisibility) + overriddenVisibility == null -> overriddenAttributes + else -> overriddenAttributes.copy( + visibility = overriddenVisibility.takeUnless { overriddenAttributes.visibility != null } + ?: overriddenAttributes.visibility + ) + } forceExtractFunction( f, parentId, @@ -1776,7 +1787,7 @@ open class KotlinFileExtractor( extractAnnotations, typeSubstitution, classTypeArgsIncludingOuterClasses, - overriddenAttributes = overriddenVisibility + overriddenAttributes = mergedAttributes ) .also { // The defaults-forwarder function is a static utility, not a member, so we only @@ -2659,9 +2670,14 @@ open class KotlinFileExtractor( DeclarationStackAdjuster(p).use { val id = useProperty(p, parentId, classTypeArgsIncludingOuterClasses) + // PSI location is only used for the primary (unspecialised) extraction. Specialised + // generic instances defer to getLocation so that the backing binary-file location is + // preserved, keeping specialised properties absent from fromSource() queries. val locId = - if (usesK2) getPsiBasedLocation(p) ?: getLocation(p, classTypeArgsIncludingOuterClasses) - else getLocation(p, classTypeArgsIncludingOuterClasses) + if (classTypeArgsIncludingOuterClasses.isNullOrEmpty()) + getPsiBasedLocation(p) ?: getLocation(p, classTypeArgsIncludingOuterClasses) + else + getLocation(p, classTypeArgsIncludingOuterClasses) tw.writeKtProperties(id, p.name.asString()) tw.writeHasLocation(id, locId) @@ -2669,6 +2685,13 @@ open class KotlinFileExtractor( val getter = p.getter val setter = p.setter + // Synthesised (DEFAULT_PROPERTY_ACCESSOR) getter and setter should point + // at the property head, not the initialiser or custom accessor body. + fun accessorOverride(f: IrFunction) = + if (f.origin == IrDeclarationOrigin.DEFAULT_PROPERTY_ACCESSOR) + getPsiBasedAccessorLocation(p)?.let { OverriddenFunctionAttributes(sourceLoc = it) } + else null + if (getter == null) { if (!isExternalDeclaration(p)) { logger.warnElement("IrProperty without a getter", p) @@ -2682,7 +2705,8 @@ open class KotlinFileExtractor( extractMethodAndParameterTypeAccesses = extractFunctionBodies, extractAnnotations = extractAnnotations, typeSubstitution, - classTypeArgsIncludingOuterClasses + classTypeArgsIncludingOuterClasses, + overriddenAttributes = accessorOverride(getter) ) ?.cast() if (getterId != null) { @@ -2712,7 +2736,8 @@ open class KotlinFileExtractor( extractMethodAndParameterTypeAccesses = extractFunctionBodies, extractAnnotations = extractAnnotations, typeSubstitution, - classTypeArgsIncludingOuterClasses + classTypeArgsIncludingOuterClasses, + overriddenAttributes = accessorOverride(setter) ) ?.cast() if (setterId != null) { @@ -2931,6 +2956,46 @@ open class KotlinFileExtractor( return tw.getLocation(declStart, declaration.endOffset) } + /** + * Returns the PSI-based location for a property declaration, spanning from + * the `val`/`var` keyword to the end of the full property declaration (including + * any explicit getter/setter body). + * + * IR offsets are inconsistent across K1 and K2: + * - K1 [IrProperty.startOffset] includes leading modifiers (private, abstract, + * lateinit, @Annotation) in the span start. K2 starts at `val`/`var`. + * - K1 [IrProperty.endOffset] stops at the end of the declaration line and + * does not include an explicit getter/setter body on a subsequent line. K2 + * includes the getter/setter body. + * + * Both are resolved by walking the PSI tree: the [KtProperty] node covers the + * full declaration including getter/setter, and [KtProperty.valOrVarKeyword] + * gives the correct start, excluding modifiers. + */ + private fun getPsiBasedLocation(p: IrProperty): Label? { + if (p.startOffset < 0 || p.endOffset < 0) return null + val file = currentIrFile ?: return null + val ktFile = getPsi2Ir()?.getKtFile(file) ?: return null + val ktProperty = ktFile.findElementAt(p.startOffset) + ?.let { leaf -> generateSequence(leaf) { it.parent }.filterIsInstance().firstOrNull() } + ?: return null + val declStart = ktProperty.valOrVarKeyword.startOffset + val declEnd = ktProperty.endOffset + return tw.getLocation(declStart, declEnd) + } + + private fun getPsiBasedAccessorLocation(p: IrProperty): Label? { + if (p.startOffset < 0 || p.endOffset < 0) return null + val file = currentIrFile ?: return null + val ktFile = getPsi2Ir()?.getKtFile(file) ?: return null + val ktProperty = ktFile.findElementAt(p.startOffset) + ?.let { leaf -> generateSequence(leaf) { it.parent }.filterIsInstance().firstOrNull() } + ?: return null + val declStart = ktProperty.valOrVarKeyword.startOffset + val declEnd = ktProperty.nameIdentifier?.endOffset ?: ktProperty.valOrVarKeyword.endOffset + return tw.getLocation(declStart, declEnd) + } + private fun extractVariable( v: IrVariable, callable: Label, @@ -2957,7 +3022,7 @@ open class KotlinFileExtractor( with("variable expr", v) { val varId = useVariable(v) val exprId = tw.getFreshIdLabel() - val locId = getPsiBasedLocation(v) ?: tw.getLocation(getVariableLocationProvider(v)) + val locId = getPsiBasedLocation(v as IrElement) ?: tw.getLocation(getVariableLocationProvider(v)) val type = useType(v.type) tw.writeLocalvars(varId, v.name.asString(), type.javaResult.id, exprId) tw.writeLocalvarsKotlinType(varId, type.kotlinResult.id) diff --git a/java/ql/test-kotlin1/library-tests/comments/comments.expected b/java/ql/test-kotlin1/library-tests/comments/comments.expected index 0b0d16dc346..bcda3ec0f74 100644 --- a/java/ql/test-kotlin1/library-tests/comments/comments.expected +++ b/java/ql/test-kotlin1/library-tests/comments/comments.expected @@ -19,8 +19,8 @@ comments commentOwners | 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 | | comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:5:17:46 | members | -| comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:5:17:46 | members | | comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:13:17:46 | getMembers$private | +| comments.kt:14:5:16:7 | /**\n * Members of this group.\n */ | comments.kt:17:13:17:46 | members | | 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 | | comments.kt:35:5:35:34 | /** Medium is in the middle */ | comments.kt:36:5:36:14 | Medium | | comments.kt:37:5:37:23 | /** This is high */ | comments.kt:38:5:38:11 | High | diff --git a/java/ql/test-kotlin1/library-tests/compilation-units/cus.expected b/java/ql/test-kotlin1/library-tests/compilation-units/cus.expected index 97b688e8795..6a09d9908c3 100644 --- a/java/ql/test-kotlin1/library-tests/compilation-units/cus.expected +++ b/java/ql/test-kotlin1/library-tests/compilation-units/cus.expected @@ -2,6 +2,8 @@ | AbstractList$RandomAccessSpliterator | .../AbstractList$RandomAccessSpliterator.class:0:0:0:0 | | ArrayList | .../ArrayList.class:0:0:0:0 | | ArrayList$ArrayListSpliterator | .../ArrayList$ArrayListSpliterator.class:0:0:0:0 | +| CleanerImpl$CleanableList | .../CleanerImpl$CleanableList.class:0:0:0:0 | +| CleanerImpl$CleanableList$Node | .../CleanerImpl$CleanableList$Node.class:0:0:0:0 | | List | .../List.class:0:0:0:0 | | ListIterator | .../ListIterator.class:0:0:0:0 | | MemorySessionImpl$ResourceList | .../MemorySessionImpl$ResourceList.class:0:0:0:0 | diff --git a/java/ql/test-kotlin1/library-tests/modifiers/modifiers.expected b/java/ql/test-kotlin1/library-tests/modifiers/modifiers.expected index bc7c6207642..7badb98a44b 100644 --- a/java/ql/test-kotlin1/library-tests/modifiers/modifiers.expected +++ b/java/ql/test-kotlin1/library-tests/modifiers/modifiers.expected @@ -2,17 +2,17 @@ | modifiers.kt:1:6:29:1 | X | Constructor | public | | modifiers.kt:2:5:2:21 | a | Field | final | | modifiers.kt:2:5:2:21 | a | Field | private | -| modifiers.kt:2:5:2:21 | a | Property | private | +| modifiers.kt:2:13:2:21 | a | Property | private | | modifiers.kt:2:13:2:21 | getA$private | Method | final | | modifiers.kt:2:13:2:21 | getA$private | Method | private | | modifiers.kt:3:5:3:23 | b | Field | final | | modifiers.kt:3:5:3:23 | b | Field | private | -| modifiers.kt:3:5:3:23 | b | Property | protected | +| modifiers.kt:3:15:3:23 | b | Property | protected | | modifiers.kt:3:15:3:23 | getB | Method | final | | modifiers.kt:3:15:3:23 | getB | Method | protected | | modifiers.kt:4:5:4:22 | c | Field | final | | modifiers.kt:4:5:4:22 | c | Field | private | -| modifiers.kt:4:5:4:22 | c | Property | internal | +| modifiers.kt:4:14:4:22 | c | Property | internal | | modifiers.kt:4:14:4:22 | getC$main | Method | final | | modifiers.kt:4:14:4:22 | getC$main | Method | internal | | modifiers.kt:5:5:5:34 | d | Field | final | @@ -25,7 +25,7 @@ | modifiers.kt:7:15:9:5 | Nested | Constructor | public | | modifiers.kt:8:9:8:29 | e | Field | final | | modifiers.kt:8:9:8:29 | e | Field | private | -| modifiers.kt:8:9:8:29 | e | Property | public | +| modifiers.kt:8:16:8:29 | e | Property | public | | modifiers.kt:8:16:8:29 | getE | Method | final | | modifiers.kt:8:16:8:29 | getE | Method | public | | modifiers.kt:11:5:15:5 | fn1 | Method | final | @@ -76,12 +76,12 @@ | modifiers.kt:35:1:41:1 | LateInit | Class | public | | modifiers.kt:35:8:41:1 | LateInit | Constructor | public | | modifiers.kt:36:5:36:40 | test0 | Field | private | -| modifiers.kt:36:5:36:40 | test0 | Property | lateinit | -| modifiers.kt:36:5:36:40 | test0 | Property | private | | modifiers.kt:36:22:36:40 | getTest0$private | Method | final | | modifiers.kt:36:22:36:40 | getTest0$private | Method | private | | modifiers.kt:36:22:36:40 | setTest0$private | Method | final | | modifiers.kt:36:22:36:40 | setTest0$private | Method | private | +| modifiers.kt:36:22:36:40 | test0 | Property | lateinit | +| modifiers.kt:36:22:36:40 | test0 | Property | private | | modifiers.kt:38:5:40:5 | fn | Method | final | | modifiers.kt:38:5:40:5 | fn | Method | public | | modifiers.kt:39:18:39:36 | LateInit test1 | LocalVariableDecl | lateinit | diff --git a/java/ql/test-kotlin1/library-tests/properties/properties.expected b/java/ql/test-kotlin1/library-tests/properties/properties.expected index 1f60e9054b2..8cc2702ab5a 100644 --- a/java/ql/test-kotlin1/library-tests/properties/properties.expected +++ b/java/ql/test-kotlin1/library-tests/properties/properties.expected @@ -4,30 +4,30 @@ | 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 | | properties.kt:4:5:4:24 | immutableInt | properties.kt:4:5:4:24 | getImmutableInt | file://:0:0:0:0 | | properties.kt:4:5:4:24 | immutableInt | public | | properties.kt:5:5:5:26 | typedProp | properties.kt:5:5:5:26 | getTypedProp | file://:0:0:0:0 | | properties.kt:5:5:5:26 | typedProp | public | -| properties.kt:6:5:6:38 | abstractTypeProp | properties.kt:6:14:6:38 | getAbstractTypeProp | file://:0:0:0:0 | | file://:0:0:0:0 | | public | +| properties.kt:6:14:6:38 | abstractTypeProp | properties.kt:6:14:6:38 | getAbstractTypeProp | file://:0:0:0:0 | | file://:0:0:0:0 | | public | | properties.kt:7:5:7:30 | initialisedInInit | properties.kt:7:5:7:30 | getInitialisedInInit | file://:0:0:0:0 | | properties.kt:7:5:7:30 | initialisedInInit | public | | properties.kt:11:5:11:40 | useConstructorArg | properties.kt:11:5:11:40 | getUseConstructorArg | file://:0:0:0:0 | | properties.kt:11:5:11:40 | useConstructorArg | public | | properties.kt:12:5:13:21 | five | properties.kt:13:13:13:21 | getFive | file://:0:0:0:0 | | file://:0:0:0:0 | | public | | properties.kt:14:5:15:21 | six | properties.kt:15:13:15:21 | getSix | file://:0:0:0:0 | | file://:0:0:0:0 | | public | | 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 | | public | -| properties.kt:19:5:20:15 | defaultGetter | properties.kt:20:13:20:15 | getDefaultGetter | file://:0:0:0:0 | | properties.kt:19:5:20:15 | defaultGetter | public | -| 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 | -| 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 | -| 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 | +| properties.kt:19:5:20:15 | defaultGetter | properties.kt:19:5:20:15 | getDefaultGetter | file://:0:0:0:0 | | properties.kt:19:5:20:15 | defaultGetter | public | +| 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 | +| 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 | +| 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 | | 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 | | 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 | | properties.kt:32:5:33:29 | useField | properties.kt:33:13:33:29 | getUseField | file://:0:0:0:0 | | properties.kt:32:5:33:29 | useField | public | -| 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 | -| properties.kt:35:5:35:32 | privateProp | properties.kt:35:13:35:32 | getPrivateProp$private | file://:0:0:0:0 | | properties.kt:35:5:35:32 | privateProp | private | -| properties.kt:36:5:36:36 | protectedProp | properties.kt:36:15:36:36 | getProtectedProp | file://:0:0:0:0 | | properties.kt:36:5:36:36 | protectedProp | protected | -| properties.kt:37:5:37:30 | publicProp | properties.kt:37:12:37:30 | getPublicProp | file://:0:0:0:0 | | properties.kt:37:5:37:30 | publicProp | public | -| properties.kt:38:5:38:34 | internalProp | properties.kt:38:14:38:34 | getInternalProp$main | file://:0:0:0:0 | | properties.kt:38:5:38:34 | internalProp | internal | -| properties.kt:67:1:67:23 | constVal | properties.kt:67:7:67:23 | getConstVal | file://:0:0:0:0 | | properties.kt:67:1:67:23 | constVal | public | +| 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 | +| properties.kt:35:13:35:32 | privateProp | properties.kt:35:13:35:32 | getPrivateProp$private | file://:0:0:0:0 | | properties.kt:35:5:35:32 | privateProp | private | +| properties.kt:36:15:36:36 | protectedProp | properties.kt:36:15:36:36 | getProtectedProp | file://:0:0:0:0 | | properties.kt:36:5:36:36 | protectedProp | protected | +| properties.kt:37:12:37:30 | publicProp | properties.kt:37:12:37:30 | getPublicProp | file://:0:0:0:0 | | properties.kt:37:5:37:30 | publicProp | public | +| properties.kt:38:14:38:34 | internalProp | properties.kt:38:14:38:34 | getInternalProp$main | file://:0:0:0:0 | | properties.kt:38:5:38:34 | internalProp | internal | +| properties.kt:67:7:67:23 | constVal | properties.kt:67:7:67:23 | getConstVal | file://:0:0:0:0 | | properties.kt:67:1:67:23 | constVal | public | | properties.kt:70:5:70:16 | prop | properties.kt:70:5:70:16 | getProp | file://:0:0:0:0 | | properties.kt:70:5:70:16 | prop | public | | properties.kt:78:1:79:13 | x | properties.kt:79:5:79:13 | getX | file://:0:0:0:0 | | file://:0:0:0:0 | | public | | properties.kt:80:1:81:13 | x | properties.kt:81:5:81:13 | getX | file://:0:0:0:0 | | file://:0:0:0:0 | | public | -| 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 | -| 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 | +| 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 | +| 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 | fieldDeclarations | properties.kt:2:27:2:50 | int constructorProp; | properties.kt:2:27:2:50 | constructorProp | 0 | | properties.kt:2:53:2:83 | int mutableConstructorProp; | properties.kt:2:53:2:83 | mutableConstructorProp | 0 |