KT-62675 [AA] Handle labeled this expressions in reference shortener

At the moment, there is no good way to meaningfully filter `this`
expressions. The filters for the reference shortener can work only with
symbols, and it does not make a lot of sense to check any particular
symbol when deciding whether to shorten a labeled `this` expression.

We would probably need a better API for the shortener to be able
to filter more precisely (see KT-63555)

^KT-62675 Fixed
This commit is contained in:
Roman Golyshev
2023-11-06 12:17:13 +01:00
committed by teamcity
parent d47557067c
commit 4f5926a88f
23 changed files with 393 additions and 8 deletions
@@ -17,6 +17,7 @@ import org.jetbrains.kotlin.analysis.api.symbols.KtClassLikeSymbol
import org.jetbrains.kotlin.kdoc.psi.impl.KDocName
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtThisExpression
internal class KtFe10ReferenceShortener(
override val analysisSession: KtFe10AnalysisSession,
@@ -41,6 +42,7 @@ internal class KtFe10ReferenceShortener(
override val starImportsToAdd: Set<FqName> get() = emptySet()
override val listOfTypeToShortenInfo: List<TypeToShortenInfo> get() = emptyList()
override val listOfQualifierToShortenInfo: List<QualifierToShortenInfo> get() = emptyList()
override val thisLabelsToShorten: List<ThisLabelToShortenInfo> = emptyList()
override val kDocQualifiersToShorten: List<SmartPsiElementPointer<KDocName>> get() = emptyList()
override val isEmpty: Boolean get() = true
@@ -40,6 +40,7 @@ import org.jetbrains.kotlin.fir.expressions.builder.buildPropertyAccessExpressio
import org.jetbrains.kotlin.fir.references.FirErrorNamedReference
import org.jetbrains.kotlin.fir.references.FirNamedReference
import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference
import org.jetbrains.kotlin.fir.references.FirThisReference
import org.jetbrains.kotlin.fir.references.builder.buildSimpleNamedReference
import org.jetbrains.kotlin.fir.resolve.SessionHolderImpl
import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeAmbiguityError
@@ -91,6 +92,7 @@ internal class KtFirReferenceShortener(
starImportsToAdd = emptySet(),
listOfTypeToShortenInfo = emptyList(),
listOfQualifierToShortenInfo = emptyList(),
thisLabelsToShorten = emptyList(),
kDocQualifiersToShorten = emptyList(),
)
@@ -132,8 +134,8 @@ internal class KtFirReferenceShortener(
additionalImports.simpleImports,
additionalImports.starImports,
collector.typesToShorten.distinctBy { it.element }.map { TypeToShortenInfo(it.element.createSmartPointer(), it.shortenedRef) },
collector.qualifiersToShorten.distinctBy { it.element }
.map { QualifierToShortenInfo(it.element.createSmartPointer(), it.shortenedRef) },
collector.qualifiersToShorten.distinctBy { it.element }.map { QualifierToShortenInfo(it.element.createSmartPointer(), it.shortenedRef) },
collector.labelsToShorten.distinctBy { it.element }.map { ThisLabelToShortenInfo(it.element.createSmartPointer()) },
kDocCollector.kDocQualifiersToShorten.distinctBy { it.element }.map { it.element.createSmartPointer() },
)
}
@@ -399,6 +401,13 @@ private class ShortenQualifier(
override val importAllInParent: Boolean = false
) : ElementToShorten()
private class ShortenThisLabel(
override val element: KtThisExpression,
) : ElementToShorten() {
override val nameToImport: FqName? = null
override val importAllInParent: Boolean = false
}
/**
* N.B. Does not subclass [ElementToShorten] because currently
* there's no reason to do that.
@@ -467,6 +476,12 @@ private class CollectingVisitor(private val collector: ElementsToShortenCollecto
collector.processPropertyAccess(propertyAccessExpression)
}
override fun visitThisReference(thisReference: FirThisReference) {
super.visitThisReference(thisReference)
collector.processThisReference(thisReference)
}
}
private class ElementsToShortenCollector(
@@ -480,6 +495,7 @@ private class ElementsToShortenCollector(
) {
val typesToShorten: MutableList<ShortenType> = mutableListOf()
val qualifiersToShorten: MutableList<ShortenQualifier> = mutableListOf()
val labelsToShorten: MutableList<ShortenThisLabel> = mutableListOf()
fun processTypeRef(resolvedTypeRef: FirResolvedTypeRef) {
val typeElement = resolvedTypeRef.correspondingTypePsi ?: return
@@ -859,6 +875,7 @@ private class ElementsToShortenCollector(
return when (element) {
is KtUserType -> ShortenType(element, shortenedRef, nameToImport, importAllInParent)
is KtDotQualifiedExpression -> ShortenQualifier(element, shortenedRef, nameToImport, importAllInParent)
is KtThisExpression -> ShortenThisLabel(element)
else -> error("Unexpected ${element::class}")
}
}
@@ -1178,10 +1195,14 @@ private class ElementsToShortenCollector(
}
private fun canBePossibleToDropReceiver(qualifiedAccess: FirQualifiedAccessExpression): Boolean {
return when {
qualifiedAccess.explicitReceiver is FirThisReceiverExpression -> shortenOptions.removeThis
return when (val explicitReceiver = qualifiedAccess.explicitReceiver) {
is FirThisReceiverExpression -> {
shortenOptions.removeThis &&
!explicitReceiver.isImplicit &&
explicitReceiver.calleeReference.referencesClosestReceiver()
}
qualifiedAccess.explicitReceiver is FirResolvedQualifier -> qualifiedAccess.extensionReceiver == null
is FirResolvedQualifier -> qualifiedAccess.extensionReceiver == null
else -> false
}
@@ -1219,6 +1240,63 @@ private class ElementsToShortenCollector(
return if (deepestQualifier.hasFakeRootPrefix()) createElementToShorten(deepestQualifier) else null
}
/**
* Checks whether `this` expression references the closest receiver in the current position.
*
* If it is the case, then we can safely remove the label from it (if it exists).
*/
private fun FirThisReference.referencesClosestReceiver(): Boolean {
require(!isImplicit) {
"It doesn't make sense to handle implicit this references"
}
if (labelName == null) return true
val psi = psi as? KtThisExpression ?: return false
val implicitReceivers = towerContextProvider.getClosestAvailableParentContext(psi)?.implicitReceiverStack ?: return false
val closestImplicitReceiver = implicitReceivers.lastOrNull() ?: return false
return boundSymbol == closestImplicitReceiver.boundSymbol
}
private fun canBePossibleToDropLabel(thisReference: FirThisReference): Boolean {
return shortenOptions.removeThisLabels && thisReference.labelName != null
}
/**
* This method intentionally mirrors the appearance
* of the [classShortenStrategy] and [callableShortenStrategy] filters,
* but ATM we don't have a way to properly handle
* [FirThisReference]s through the existing filters.
*
* We need a better way to decide shortening strategy
* for labeled and regular `this` expressions (KT-63555).
*/
private fun thisLabelShortenStrategy(thisReference: FirThisReference): ShortenStrategy {
val referencedSymbol = thisReference.boundSymbol
val strategy = when (referencedSymbol) {
is FirClassLikeSymbol<*> -> classShortenStrategy(referencedSymbol)
is FirCallableSymbol<*> -> callableShortenStrategy(referencedSymbol)
else -> ShortenStrategy.DO_NOT_SHORTEN
}
return strategy
}
fun processThisReference(thisReference: FirThisReference) {
if (!canBePossibleToDropLabel(thisReference)) return
val labeledThisPsi = thisReference.psi as? KtThisExpression ?: return
if (!labeledThisPsi.inSelection) return
if (thisLabelShortenStrategy(thisReference) == ShortenStrategy.DO_NOT_SHORTEN) return
if (thisReference.referencesClosestReceiver()) {
addElementToShorten(createElementToShorten(labeledThisPsi))
}
}
private fun KtElement.isInsideOf(another: KtElement): Boolean = another.textRange.contains(textRange)
/**
@@ -1246,6 +1324,7 @@ private class ElementsToShortenCollector(
when (elementInfoToShorten) {
is ShortenType -> typesToShorten.add(elementInfoToShorten)
is ShortenQualifier -> qualifiersToShorten.add(elementInfoToShorten)
is ShortenThisLabel -> labelsToShorten.add(elementInfoToShorten)
}
}
}
@@ -1284,6 +1363,9 @@ private class ElementsToShortenCollector(
return selectorReference.textRange.intersects(selection)
}
private val KtThisExpression.inSelection: Boolean
get() = textRange.intersects(selection)
private val ClassId.outerClassesWithSelf: Sequence<ClassId>
get() = generateSequence(this) { it.outerClassId }
@@ -1393,6 +1475,7 @@ private class ShortenCommandImpl(
override val starImportsToAdd: Set<FqName>,
override val listOfTypeToShortenInfo: List<TypeToShortenInfo>,
override val listOfQualifierToShortenInfo: List<QualifierToShortenInfo>,
override val thisLabelsToShorten: List<ThisLabelToShortenInfo>,
override val kDocQualifiersToShorten: List<SmartPsiElementPointer<KDocName>>,
) : ShortenCommand
@@ -1408,5 +1491,6 @@ internal fun KtSimpleNameExpression.getDotQualifiedExpressionForSelector(): KtDo
private fun KtElement.getQualifier(): KtElement? = when (this) {
is KtUserType -> qualifier
is KtDotQualifiedExpression -> receiverExpression
is KtThisExpression -> labelQualifier
else -> error("Unexpected ${this::class}")
}
@@ -80,6 +80,16 @@ public class FirIdeNormalAnalysisScriptSourceModuleReferenceShortenerTestGenerat
public void testAllFilesPresentInThisReference() throws Exception {
KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference"), Pattern.compile("^(.+)\\.kts$"), null, true);
}
@Nested
@TestMetadata("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel")
@TestDataPath("$PROJECT_ROOT")
public class WithLabel {
@Test
public void testAllFilesPresentInWithLabel() throws Exception {
KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel"), Pattern.compile("^(.+)\\.kts$"), null, true);
}
}
}
@Nested
@@ -950,6 +950,64 @@ public class FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated ext
public void testThis_safeCall() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/this_safeCall.kt");
}
@Nested
@TestMetadata("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel")
@TestDataPath("$PROJECT_ROOT")
public class WithLabel {
@Test
public void testAllFilesPresentInWithLabel() throws Exception {
KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel"), Pattern.compile("^(.+)\\.kt$"), null, true);
}
@Test
@TestMetadata("extensionReceiver.kt")
public void testExtensionReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/extensionReceiver.kt");
}
@Test
@TestMetadata("extensionReceiver_vs_lambdaReceiver.kt")
public void testExtensionReceiver_vs_lambdaReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/extensionReceiver_vs_lambdaReceiver.kt");
}
@Test
@TestMetadata("extensionReceiver_vs_local.kt")
public void testExtensionReceiver_vs_local() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/extensionReceiver_vs_local.kt");
}
@Test
@TestMetadata("lambdaReceiver.kt")
public void testLambdaReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/lambdaReceiver.kt");
}
@Test
@TestMetadata("lambdaReceiver_vs_lambdaReceiver.kt")
public void testLambdaReceiver_vs_lambdaReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/lambdaReceiver_vs_lambdaReceiver.kt");
}
@Test
@TestMetadata("regularClass.kt")
public void testRegularClass() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/regularClass.kt");
}
@Test
@TestMetadata("regularClass_vs_extensionReceiver.kt")
public void testRegularClass_vs_extensionReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/regularClass_vs_extensionReceiver.kt");
}
@Test
@TestMetadata("regularClass_vs_innerClass.kt")
public void testRegularClass_vs_innerClass() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/regularClass_vs_innerClass.kt");
}
}
}
@Nested
@@ -24,6 +24,11 @@ internal object ShorteningResultsRenderer {
appendLine("[qualifier] $it${shortenedRef?.let { ref -> " -> $ref" } ?: ""}")
}
}
shortening.thisLabelsToShorten.forEach { thisLabel ->
thisLabel.labelToShorten.element?.text?.let {
appendLine("[thisLabel] $it")
}
}
shortening.kDocQualifiersToShorten.forEach { kdoc ->
kdoc.element?.text?.let {
appendLine("[kdoc] $it")
@@ -950,6 +950,64 @@ public class FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenera
public void testThis_safeCall() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/this_safeCall.kt");
}
@Nested
@TestMetadata("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel")
@TestDataPath("$PROJECT_ROOT")
public class WithLabel {
@Test
public void testAllFilesPresentInWithLabel() throws Exception {
KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel"), Pattern.compile("^(.+)\\.kt$"), null, true);
}
@Test
@TestMetadata("extensionReceiver.kt")
public void testExtensionReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/extensionReceiver.kt");
}
@Test
@TestMetadata("extensionReceiver_vs_lambdaReceiver.kt")
public void testExtensionReceiver_vs_lambdaReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/extensionReceiver_vs_lambdaReceiver.kt");
}
@Test
@TestMetadata("extensionReceiver_vs_local.kt")
public void testExtensionReceiver_vs_local() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/extensionReceiver_vs_local.kt");
}
@Test
@TestMetadata("lambdaReceiver.kt")
public void testLambdaReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/lambdaReceiver.kt");
}
@Test
@TestMetadata("lambdaReceiver_vs_lambdaReceiver.kt")
public void testLambdaReceiver_vs_lambdaReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/lambdaReceiver_vs_lambdaReceiver.kt");
}
@Test
@TestMetadata("regularClass.kt")
public void testRegularClass() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/regularClass.kt");
}
@Test
@TestMetadata("regularClass_vs_extensionReceiver.kt")
public void testRegularClass_vs_extensionReceiver() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/regularClass_vs_extensionReceiver.kt");
}
@Test
@TestMetadata("regularClass_vs_innerClass.kt")
public void testRegularClass_vs_innerClass() throws Exception {
runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/thisReference/withLabel/regularClass_vs_innerClass.kt");
}
}
}
@Nested
@@ -18,19 +18,23 @@ import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtThisExpression
import org.jetbrains.kotlin.psi.KtUserType
/**
* @property removeThis If set to `true`, reference shortener will detect redundant `this` qualifiers
* and will collect them to [ShortenCommand.qualifiersToShorten].
* and will collect them to [ShortenCommand.listOfQualifierToShortenInfo].
* @property removeThisLabels If set to `true`, reference shortener will detect redundant labels on `this` expressions,
* and will collect them to [ShortenCommand.thisLabelsToShorten]
*/
public data class ShortenOptions(
public val removeThis: Boolean = false,
public val removeThisLabels: Boolean = false,
) {
public companion object {
public val DEFAULT: ShortenOptions = ShortenOptions()
public val ALL_ENABLED: ShortenOptions = ShortenOptions(removeThis = true)
public val ALL_ENABLED: ShortenOptions = ShortenOptions(removeThis = true, removeThisLabels = true)
}
}
@@ -179,14 +183,26 @@ public data class QualifierToShortenInfo(
val shortenedReference: String?,
)
/**
* A class with a reference to [KtThisExpression] with a label qualifier ([KtThisExpression.labelQualifier]) that can be safely removed
* without changing the semantics of the code.
*/
public data class ThisLabelToShortenInfo(
val labelToShorten: SmartPsiElementPointer<KtThisExpression>,
)
public interface ShortenCommand {
public val targetFile: SmartPsiElementPointer<KtFile>
public val importsToAdd: Set<FqName>
public val starImportsToAdd: Set<FqName>
public val listOfTypeToShortenInfo: List<TypeToShortenInfo>
public val listOfQualifierToShortenInfo: List<QualifierToShortenInfo>
public val thisLabelsToShorten: List<ThisLabelToShortenInfo>
public val kDocQualifiersToShorten: List<SmartPsiElementPointer<KDocName>>
public val isEmpty: Boolean
get() = listOfTypeToShortenInfo.isEmpty() && listOfQualifierToShortenInfo.isEmpty() && kDocQualifiersToShorten.isEmpty()
get() = listOfTypeToShortenInfo.isEmpty() &&
listOfQualifierToShortenInfo.isEmpty() &&
thisLabelsToShorten.isEmpty() &&
kDocQualifiersToShorten.isEmpty()
}
@@ -0,0 +1,9 @@
package test
class Regular {
fun one() {}
}
fun Regular.usage() {
<expr>this@usage.one()</expr>
}
@@ -0,0 +1,11 @@
Before shortening: this@usage.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
[qualifier] this@usage.one()
[thisLabel] this@usage
with SHORTEN_AND_IMPORT:
[qualifier] this@usage.one()
[thisLabel] this@usage
with SHORTEN_AND_STAR_IMPORT:
[qualifier] this@usage.one()
[thisLabel] this@usage
@@ -0,0 +1,13 @@
package test
class Regular {
fun one() {}
}
fun test(action: Regular.() -> Unit) {}
fun Regular.usage() {
test {
<expr>this@usage.one()</expr>
}
}
@@ -0,0 +1,5 @@
Before shortening: this@usage.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
with SHORTEN_AND_IMPORT:
with SHORTEN_AND_STAR_IMPORT:
@@ -0,0 +1,11 @@
package test
class Regular {
fun one() {}
}
fun Regular.usage() {
fun Regular.local() {
<expr>this@usage.one()</expr>
}
}
@@ -0,0 +1,5 @@
Before shortening: this@usage.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
with SHORTEN_AND_IMPORT:
with SHORTEN_AND_STAR_IMPORT:
@@ -0,0 +1,13 @@
package test
class Regular {
fun one() {}
}
fun test(action: Regular.() -> Unit) {}
fun usage() {
test {
<expr>this@test.one()</expr>
}
}
@@ -0,0 +1,11 @@
Before shortening: this@test.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
[qualifier] this@test.one()
[thisLabel] this@test
with SHORTEN_AND_IMPORT:
[qualifier] this@test.one()
[thisLabel] this@test
with SHORTEN_AND_STAR_IMPORT:
[qualifier] this@test.one()
[thisLabel] this@test
@@ -0,0 +1,15 @@
package test
class Regular {
fun one() {}
}
fun test(action: Regular.() -> Unit) {}
fun usage() {
test {
test r2@{
<expr>this@test.one()</expr>
}
}
}
@@ -0,0 +1,5 @@
Before shortening: this@test.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
with SHORTEN_AND_IMPORT:
with SHORTEN_AND_STAR_IMPORT:
@@ -0,0 +1,9 @@
package test
class Regular {
fun one() {}
fun usage() {
<expr>this@Regular.one()</expr>
}
}
@@ -0,0 +1,11 @@
Before shortening: this@Regular.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
[qualifier] this@Regular.one()
[thisLabel] this@Regular
with SHORTEN_AND_IMPORT:
[qualifier] this@Regular.one()
[thisLabel] this@Regular
with SHORTEN_AND_STAR_IMPORT:
[qualifier] this@Regular.one()
[thisLabel] this@Regular
@@ -0,0 +1,13 @@
package test
class Other {
fun one() {}
}
class Regular {
fun one() {}
fun Other.usage() {
<expr>this@Regular.one()</expr>
}
}
@@ -0,0 +1,5 @@
Before shortening: this@Regular.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
with SHORTEN_AND_IMPORT:
with SHORTEN_AND_STAR_IMPORT:
@@ -0,0 +1,11 @@
package test
class Regular {
fun one() {}
inner class Inner {
fun usage() {
<expr>this@Regular.one()</expr>
}
}
}
@@ -0,0 +1,5 @@
Before shortening: this@Regular.one()
with DO_NOT_SHORTEN:
with SHORTEN_IF_ALREADY_IMPORTED:
with SHORTEN_AND_IMPORT:
with SHORTEN_AND_STAR_IMPORT: