From 646cdb56e59ac280563b7bdf2ba941235022cb6b Mon Sep 17 00:00:00 2001 From: Roman Golyshev Date: Thu, 11 Jan 2024 12:51:45 +0100 Subject: [PATCH] KTIJ-27841 [AA] Do not import and shorten class constructor if it will alter other references in the file N.B. This implementation does not 100% prevent conflicts or resolve alterations when shortening functions. To guarantee that, we would need to carefully consider all the references in the file, and to check whether they have changed their resolve in the presence of a new import. This is not trivial and will be approached separately under a different task. ^KTIJ-27841 Fixed --- .../fir/components/KtFirReferenceShortener.kt | 30 +++++++++++++++++-- .../constructorCall_vs_constructorCall.txt | 2 -- .../constructorCall_vs_typeRefs.txt | 2 -- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirReferenceShortener.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirReferenceShortener.kt index fa315184895..a6f33cedd63 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirReferenceShortener.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirReferenceShortener.kt @@ -595,7 +595,11 @@ private class ElementsToShortenCollector( findClassifierQualifierToShorten(wholeClassQualifier, wholeQualifierElement)?.let(::addElementToShorten) } - private val FirClassifierSymbol<*>.classIdIfExists: ClassId? get() = (this as? FirClassLikeSymbol<*>)?.classId + private val FirClassifierSymbol<*>.classIdIfExists: ClassId? + get() = (this as? FirClassLikeSymbol<*>)?.classId + + private val FirConstructorSymbol.classIdIfExists: ClassId? + get() = this.containingClassLookupTag()?.classId /** * Returns true if the class symbol has a type parameter that is supposed to be provided for its parent class. @@ -929,6 +933,22 @@ private class ElementsToShortenCollector( return importAffectsUsagesOfClassesWithSameName(classToImport, file, importAllInParent) } + /** + * Same as above, but for more general callable symbols. + * + * Currently only checks constructor calls, assuming `true` for everything else. + */ + private fun importBreaksExistingReferences(callableToImport: FirCallableSymbol<*>, file: KtFile, importAllInParent: Boolean): Boolean { + if (callableToImport is FirConstructorSymbol) { + val classToImport = callableToImport.classIdIfExists + if (classToImport != null) { + return importAffectsUsagesOfClassesWithSameName(classToImport, file, importAllInParent) + } + } + + return false + } + private fun importedClassifierOverwritesAvailableClassifier( availableClassifier: AvailableSymbol>, importAllInParent: Boolean @@ -1221,15 +1241,21 @@ private class ElementsToShortenCollector( when { matchedCallables.isEmpty() -> { if (nameToImport == null || option == ShortenStrategy.SHORTEN_IF_ALREADY_IMPORTED) return null + + val importAllInParent = option == ShortenStrategy.SHORTEN_AND_STAR_IMPORT + if (importBreaksExistingReferences(calledSymbol, qualifiedCallExpression.containingKtFile, importAllInParent)) return null + createElementToShorten( qualifiedCallExpression, nameToImport, - importAllInParent = option == ShortenStrategy.SHORTEN_AND_STAR_IMPORT + importAllInParent, ) } + // Respect caller's request to star import this symbol. matchedCallables.any { it.importKind == ImportKind.EXPLICIT } && option == ShortenStrategy.SHORTEN_AND_STAR_IMPORT -> createElementToShorten(qualifiedCallExpression, nameToImport, importAllInParent = true) + else -> createElementToShorten(qualifiedCallExpression) } } diff --git a/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_constructorCall.txt b/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_constructorCall.txt index 862e0627ace..be7c1eee33b 100644 --- a/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_constructorCall.txt +++ b/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_constructorCall.txt @@ -1,8 +1,6 @@ Before shortening: dependency.MyFoo() with default settings: -[qualifier] dependency.MyFoo() with DO_NOT_SHORTEN: with SHORTEN_IF_ALREADY_IMPORTED: with SHORTEN_AND_IMPORT: -[qualifier] dependency.MyFoo() with SHORTEN_AND_STAR_IMPORT: diff --git a/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_typeRefs.txt b/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_typeRefs.txt index 862e0627ace..be7c1eee33b 100644 --- a/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_typeRefs.txt +++ b/analysis/analysis-api/testData/components/referenceShortener/shortenRange/conflicts/classWithSameName/constructorCall_vs_typeRefs.txt @@ -1,8 +1,6 @@ Before shortening: dependency.MyFoo() with default settings: -[qualifier] dependency.MyFoo() with DO_NOT_SHORTEN: with SHORTEN_IF_ALREADY_IMPORTED: with SHORTEN_AND_IMPORT: -[qualifier] dependency.MyFoo() with SHORTEN_AND_STAR_IMPORT: