From 1766c68e9ca21840d6071585639363aef35461f7 Mon Sep 17 00:00:00 2001 From: Roman Golyshev Date: Wed, 30 Aug 2023 18:26:59 +0200 Subject: [PATCH] [FIR][AA] Rely more on `isInBestCandidates` flag `KtFirReferenceShortener` After the KT-61568 has been fixed, `isInBestCandidates` correctly works in more cases, and can be more reliably used in `KtFirReferenceShortener` ^KTIJ-26808 Fixed ^KTIJ-26840 Fixed --- .../fir/components/KtFirReferenceShortener.kt | 27 +++---------------- ...ModuleReferenceShortenerTestGenerated.java | 12 +++++++++ ...ModuleReferenceShortenerTestGenerated.java | 12 +++++++++ .../memberVsCompanionObjectMemberConflict.kt | 15 +++++++++++ .../memberVsCompanionObjectMemberConflict.txt | 5 ++++ .../staticMethodFromBaseClassConflict.kt | 26 ++++++++++++++++++ .../staticMethodFromBaseClassConflict.txt | 11 ++++++++ 7 files changed, 84 insertions(+), 24 deletions(-) create mode 100644 analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.kt create mode 100644 analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.txt create mode 100644 analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.kt create mode 100644 analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.txt 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 079620d02b4..1af10a36c9d 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 @@ -977,31 +977,10 @@ private class ElementsToShortenCollector( if (candidates.mapNotNull { it.candidate.originScope } .hasScopeCloserThan(scopeForQualifiedAccess, expressionInScope)) return false val candidatesWithinSamePriorityScopes = candidates.filter { it.candidate.originScope == scopeForQualifiedAccess } - if (candidatesWithinSamePriorityScopes.isEmpty() || candidatesWithinSamePriorityScopes.size == 1) return true - /** - * This is a conservative decision to avoid false positives. - * - * TODO: Figure out the priorities among `candidatesWithinSamePriorityScopes` and determine if [firQualifiedAccess] matches the - * one with the highest priority. At this moment, we have some counter examples that [OverloadCandidate.isInBestCandidates] is true - * and its symbol matches [firQualifiedAccess], but we cannot shorten it. - * - * For example: - * package foo - * class Foo { - * fun test() { - * // It references FIRST. Removing `foo` lets it reference SECOND. However, the one has true for - * // [OverloadCandidate.isInBestCandidates] is FIRST. Therefore, making a decision based on `isInBestCandidates` can - * // cause false positives i.e., shortening changes the referenced symbol. - * foo.myRun { - * 42 - * } - * } - * } - * inline fun myRun(block: () -> R): R = block() // FIRST - * inline fun T.myRun(block: T.() -> R): R = block() // SECOND - */ - return false + // TODO isInBestCandidates should probably be used more actively to filter candidates + return candidatesWithinSamePriorityScopes.isEmpty() || + candidatesWithinSamePriorityScopes.singleOrNull()?.isInBestCandidates == true } private fun processPropertyAccess(firPropertyAccess: FirPropertyAccessExpression) { diff --git a/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/references/FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java b/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/references/FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java index d4b44eb370f..ffcd580d270 100644 --- a/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/references/FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java +++ b/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/references/FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java @@ -298,6 +298,12 @@ public class FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated ext runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/kdocUnresolved.kt"); } + @Test + @TestMetadata("memberVsCompanionObjectMemberConflict.kt") + public void testMemberVsCompanionObjectMemberConflict() throws Exception { + runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.kt"); + } + @Test @TestMetadata("multipleImport.kt") public void testMultipleImport() throws Exception { @@ -436,6 +442,12 @@ public class FirIdeNormalAnalysisSourceModuleReferenceShortenerTestGenerated ext runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/shortenAlreadyImportedFunction4.kt"); } + @Test + @TestMetadata("staticMethodFromBaseClassConflict.kt") + public void testStaticMethodFromBaseClassConflict() throws Exception { + runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.kt"); + } + @Test @TestMetadata("superClass.kt") public void testSuperClass() throws Exception { diff --git a/analysis/analysis-api-standalone/tests-gen/org/jetbrains/kotlin/analysis/api/standalone/fir/test/cases/generated/cases/references/FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java b/analysis/analysis-api-standalone/tests-gen/org/jetbrains/kotlin/analysis/api/standalone/fir/test/cases/generated/cases/references/FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java index 482e0222cff..084e606a68c 100644 --- a/analysis/analysis-api-standalone/tests-gen/org/jetbrains/kotlin/analysis/api/standalone/fir/test/cases/generated/cases/references/FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java +++ b/analysis/analysis-api-standalone/tests-gen/org/jetbrains/kotlin/analysis/api/standalone/fir/test/cases/generated/cases/references/FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenerated.java @@ -298,6 +298,12 @@ public class FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenera runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/kdocUnresolved.kt"); } + @Test + @TestMetadata("memberVsCompanionObjectMemberConflict.kt") + public void testMemberVsCompanionObjectMemberConflict() throws Exception { + runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.kt"); + } + @Test @TestMetadata("multipleImport.kt") public void testMultipleImport() throws Exception { @@ -436,6 +442,12 @@ public class FirStandaloneNormalAnalysisSourceModuleReferenceShortenerTestGenera runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/shortenAlreadyImportedFunction4.kt"); } + @Test + @TestMetadata("staticMethodFromBaseClassConflict.kt") + public void testStaticMethodFromBaseClassConflict() throws Exception { + runTest("analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.kt"); + } + @Test @TestMetadata("superClass.kt") public void testSuperClass() throws Exception { diff --git a/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.kt b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.kt new file mode 100644 index 00000000000..ad507b5326e --- /dev/null +++ b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.kt @@ -0,0 +1,15 @@ +package test + +class MyClass { + companion object { + val prop = "" + } + + object Other { + val prop = "" + + fun usage() { + println(MyClass.prop) + } + } +} \ No newline at end of file diff --git a/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.txt b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.txt new file mode 100644 index 00000000000..a28388a427e --- /dev/null +++ b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/memberVsCompanionObjectMemberConflict.txt @@ -0,0 +1,5 @@ +Before shortening: MyClass.prop +with DO_NOT_SHORTEN: +with SHORTEN_IF_ALREADY_IMPORTED: +with SHORTEN_AND_IMPORT: +with SHORTEN_AND_STAR_IMPORT: diff --git a/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.kt b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.kt new file mode 100644 index 00000000000..945861d4d82 --- /dev/null +++ b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.kt @@ -0,0 +1,26 @@ +// FILE: main.kt +package test + +import dependency.Middle +import dependency.Base + +class MyClass : Middle { + fun usage() { + Middle.test() + Base.test() + } +} + +// FILE: dependency/Middle.java +package dependency; + +public class Middle extends Base { + public static void test() {} +} + +// FILE: dependency/Base.java +package dependency; + +public class Base { + public static void test() {} +} diff --git a/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.txt b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.txt new file mode 100644 index 00000000000..9ab993a083f --- /dev/null +++ b/analysis/analysis-api/testData/components/referenceShortener/referenceShortener/staticMethodFromBaseClassConflict.txt @@ -0,0 +1,11 @@ +Before shortening: fun usage() { + Middle.test() + Base.test() + } +with DO_NOT_SHORTEN: +with SHORTEN_IF_ALREADY_IMPORTED: +[qualifier] Middle.test() +with SHORTEN_AND_IMPORT: +[qualifier] Middle.test() +with SHORTEN_AND_STAR_IMPORT: +[qualifier] Middle.test()