From ca6516c2a26745389aed8526b6b4c0149d09fe74 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 21 Mar 2017 18:38:20 +0300 Subject: [PATCH] findDecompiledDeclaration: find builtIns more accurately Use resolveScope of a reference to help searching for builtIn binaries Do not search non-builtIn descriptors in random scopes --- .../forTestCompile/ForTestCompileRuntime.java | 5 ++ .../codeInsight/DescriptorToSourceUtilsIde.kt | 15 +++- .../navigation/findDecompiledDeclaration.kt | 41 +++++---- .../kotlin/idea/references/KtReference.kt | 2 +- .../expected.decompiled | 6 ++ .../expected.sources | 6 ++ .../navigationMultipleRuntimes/src/module.kt | 2 + .../NavigationWithMultipleLibrariesTest.kt | 86 ++++++++++++++----- 8 files changed, 119 insertions(+), 44 deletions(-) create mode 100644 idea/testData/decompiler/navigationMultipleRuntimes/expected.decompiled create mode 100644 idea/testData/decompiler/navigationMultipleRuntimes/expected.sources create mode 100644 idea/testData/decompiler/navigationMultipleRuntimes/src/module.kt diff --git a/compiler/tests-common/org/jetbrains/kotlin/codegen/forTestCompile/ForTestCompileRuntime.java b/compiler/tests-common/org/jetbrains/kotlin/codegen/forTestCompile/ForTestCompileRuntime.java index 331f9d5bf33..25f5f1fe515 100644 --- a/compiler/tests-common/org/jetbrains/kotlin/codegen/forTestCompile/ForTestCompileRuntime.java +++ b/compiler/tests-common/org/jetbrains/kotlin/codegen/forTestCompile/ForTestCompileRuntime.java @@ -56,6 +56,11 @@ public class ForTestCompileRuntime { return assertExists(new File("dist/kotlinc/lib/kotlin-script-runtime.jar")); } + @NotNull + public static File runtimeSourcesJarForTests() { + return assertExists(new File("dist/kotlinc/lib/kotlin-runtime-sources.jar")); + } + // TODO: Do not use these classes, remove them after stdlib tests are merged in the same build as the compiler @NotNull @Deprecated diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/codeInsight/DescriptorToSourceUtilsIde.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/codeInsight/DescriptorToSourceUtilsIde.kt index d770206641f..f67095973cb 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/codeInsight/DescriptorToSourceUtilsIde.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/codeInsight/DescriptorToSourceUtilsIde.kt @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.codeInsight import com.intellij.openapi.project.Project import com.intellij.psi.PsiElement +import com.intellij.psi.search.GlobalSearchScope import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.idea.decompiler.navigation.findDecompiledDeclaration import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils @@ -31,21 +32,27 @@ object DescriptorToSourceUtilsIde { } // Returns all PSI elements for descriptor. It can find declarations in builtins or decompiled code. - fun getAllDeclarations(project: Project, targetDescriptor: DeclarationDescriptor): Collection { - val result = getDeclarationsStream(project, targetDescriptor).toHashSet() + fun getAllDeclarations( + project: Project, + targetDescriptor: DeclarationDescriptor, + builtInsSearchScope: GlobalSearchScope? = null + ): Collection { + val result = getDeclarationsStream(project, targetDescriptor, builtInsSearchScope).toHashSet() // filter out elements which are navigate to some other element of the result // this is needed to avoid duplicated results for references to declaration in same library source file return result.filter { element -> result.none { element != it && it.navigationElement == element } } } - private fun getDeclarationsStream(project: Project, targetDescriptor: DeclarationDescriptor): Sequence { + private fun getDeclarationsStream( + project: Project, targetDescriptor: DeclarationDescriptor, builtInsSearchScope: GlobalSearchScope? = null + ): Sequence { val effectiveReferencedDescriptors = DescriptorToSourceUtils.getEffectiveReferencedDescriptors(targetDescriptor).asSequence() return effectiveReferencedDescriptors.flatMap { effectiveReferenced -> // References in library sources should be resolved to corresponding decompiled declarations, // therefore we put both source declaration and decompiled declaration to stream, and afterwards we filter it in getAllDeclarations sequenceOfLazyValues( { DescriptorToSourceUtils.getSourceFromDescriptor(effectiveReferenced) }, - { findDecompiledDeclaration(project, effectiveReferenced) } + { findDecompiledDeclaration(project, effectiveReferenced, builtInsSearchScope) } ) }.filterNotNull() } diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/navigation/findDecompiledDeclaration.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/navigation/findDecompiledDeclaration.kt index 886014b1ef8..fd24b5b3461 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/navigation/findDecompiledDeclaration.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/navigation/findDecompiledDeclaration.kt @@ -21,6 +21,7 @@ import com.intellij.psi.search.EverythingGlobalScope import com.intellij.psi.search.GlobalSearchScope import org.jetbrains.kotlin.analyzer.ModuleInfo import org.jetbrains.kotlin.builtins.DefaultBuiltIns +import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.impl.TypeAliasConstructorDescriptor import org.jetbrains.kotlin.idea.caches.resolve.BinaryModuleInfo @@ -46,7 +47,9 @@ import java.util.* fun findDecompiledDeclaration( project: Project, - referencedDescriptor: DeclarationDescriptor + referencedDescriptor: DeclarationDescriptor, + // TODO: should not require explicitly specified scope to search for builtIns, use SourceElement to provide such information + builtInsSearchScope: GlobalSearchScope? ): KtDeclaration? { if (ErrorUtils.isError(referencedDescriptor)) return null if (isLocal(referencedDescriptor)) return null @@ -54,11 +57,26 @@ fun findDecompiledDeclaration( val binaryInfo = referencedDescriptor.module.getCapability(ModuleInfo.Capability) as? BinaryModuleInfo - val scope = binaryInfo?.binariesScope() ?: - // TODO: can't return null right now, have to deal with builtins - KotlinSourceFilterScope.libraryClassFiles(EverythingGlobalScope(project), project) + binaryInfo?.binariesScope()?.let { + return findInScope(referencedDescriptor, it) + } + if (KotlinBuiltIns.isBuiltIn(referencedDescriptor)) { + // builtin module does not contain information about it's origin + return builtInsSearchScope?.let { findInScope(referencedDescriptor, it) } + // fallback on searching everywhere since builtIns are accessible from any context + ?: findInScope(referencedDescriptor, GlobalSearchScope.allScope(project)) + ?: findInScope(referencedDescriptor, EverythingGlobalScope(project)) + } + return null +} - val decompiledFiles = findDecompiledFilesForDescriptor(project, referencedDescriptor, scope) +private fun findInScope(referencedDescriptor: DeclarationDescriptor, scope: GlobalSearchScope): KtDeclaration? { + val project = scope.project ?: return null + val decompiledFiles = findCandidateDeclarationsInIndex( + referencedDescriptor, KotlinSourceFilterScope.libraryClassFiles(scope, project), project + ).mapNotNullTo(LinkedHashSet()) { + it?.containingFile as? KtDecompiledFile + } return decompiledFiles.asSequence().mapNotNull { file -> @@ -75,20 +93,11 @@ private fun isLocal(descriptor: DeclarationDescriptor): Boolean { } } -private fun findDecompiledFilesForDescriptor( - project: Project, - referencedDescriptor: DeclarationDescriptor, - scope: GlobalSearchScope -): Collection { - return findCandidateDeclarationsInIndex(project, referencedDescriptor, scope).mapNotNullTo(LinkedHashSet()) { - it?.containingFile as? KtDecompiledFile - } -} private fun findCandidateDeclarationsInIndex( - project: Project, referencedDescriptor: DeclarationDescriptor, - scope: GlobalSearchScope + scope: GlobalSearchScope, + project: Project ): Collection { val containingClass = DescriptorUtils.getParentOfType(referencedDescriptor, ClassDescriptor::class.java, false) if (containingClass != null) { diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/references/KtReference.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/references/KtReference.kt index 9eea144ec58..073c3497b9d 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/references/KtReference.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/references/KtReference.kt @@ -97,7 +97,7 @@ abstract class AbstractKtReference(element: T) return listOfNotNull(psiFacade.findPackage(fqName)) } else { - return DescriptorToSourceUtilsIde.getAllDeclarations(expression.project, targetDescriptor) + return DescriptorToSourceUtilsIde.getAllDeclarations(expression.project, targetDescriptor, expression.resolveScope) } } diff --git a/idea/testData/decompiler/navigationMultipleRuntimes/expected.decompiled b/idea/testData/decompiler/navigationMultipleRuntimes/expected.decompiled new file mode 100644 index 00000000000..4b9e8496f6f --- /dev/null +++ b/idea/testData/decompiler/navigationMultipleRuntimes/expected.decompiled @@ -0,0 +1,6 @@ + Sequence.class +public interface <1>Sequence { + collections.kotlin_builtins +public interface <3>Map { + kotlin.kotlin_builtins +public final class <2><4><5>Int private constructor() : kotlin.Number, kotlin.Comparable { diff --git a/idea/testData/decompiler/navigationMultipleRuntimes/expected.sources b/idea/testData/decompiler/navigationMultipleRuntimes/expected.sources new file mode 100644 index 00000000000..8c1df9c42fb --- /dev/null +++ b/idea/testData/decompiler/navigationMultipleRuntimes/expected.sources @@ -0,0 +1,6 @@ + Collections.kt +public interface <3>Map { + Primitives.kt +public class <2><4><5>Int private constructor() : Number(), Comparable { + Sequence.kt +public interface <1>Sequence { diff --git a/idea/testData/decompiler/navigationMultipleRuntimes/src/module.kt b/idea/testData/decompiler/navigationMultipleRuntimes/src/module.kt new file mode 100644 index 00000000000..16d2305cc37 --- /dev/null +++ b/idea/testData/decompiler/navigationMultipleRuntimes/src/module.kt @@ -0,0 +1,2 @@ +fun main(s: Sequence, m: Map) { +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/decompiler/navigation/NavigationWithMultipleLibrariesTest.kt b/idea/tests/org/jetbrains/kotlin/idea/decompiler/navigation/NavigationWithMultipleLibrariesTest.kt index 0f1f6d78e69..e0048103beb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/decompiler/navigation/NavigationWithMultipleLibrariesTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/decompiler/navigation/NavigationWithMultipleLibrariesTest.kt @@ -29,6 +29,7 @@ import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.psi.PsiManager import com.intellij.testFramework.ModuleTestCase +import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.idea.caches.resolve.LibraryInfo import org.jetbrains.kotlin.idea.caches.resolve.LibrarySourceInfo import org.jetbrains.kotlin.idea.caches.resolve.getNullableModuleInfo @@ -39,9 +40,9 @@ import org.jetbrains.kotlin.test.testFramework.runWriteAction import org.junit.Assert import java.io.File -val testDataPath = PluginTestCaseBase.getTestDataPathBase() + "/decompiler/navigationMultipleLibs/" +class NavigationWithMultipleCustomLibrariesTest : AbstractNavigationWithMultipleLibrariesTest() { -class NavigationWithMultipleLibrariesTest : ModuleTestCase() { + override val testDataPath = PluginTestCaseBase.getTestDataPathBase() + "/decompiler/navigationMultipleLibs/" fun testNavigationToDecompiled() { doTest(false, "expected.decompiled") @@ -51,14 +52,64 @@ class NavigationWithMultipleLibrariesTest : ModuleTestCase() { doTest(true, "expected.sources") } + override fun createProjectLib(libraryName: String, withSources: Boolean): Library { + val librarySrc = testDataPath + "libSrc" + + val libraryJar = MockLibraryUtil.compileLibraryToJar(librarySrc, libraryName, withSources, false, false) + val jarRoot = libraryJar.jarRoot() + return runWriteAction { + val library = ProjectLibraryTable.getInstance(project).createLibrary(libraryName) + val modifiableModel = library.modifiableModel + modifiableModel.addRoot(jarRoot, OrderRootType.CLASSES) + if (withSources) { + modifiableModel.addRoot(jarRoot.findChild("src")!!, OrderRootType.SOURCES) + } + modifiableModel.commit() + library + } + } +} + +class NavigationWithMultipleRuntimesTest : AbstractNavigationWithMultipleLibrariesTest() { + + override val testDataPath = PluginTestCaseBase.getTestDataPathBase() + "/decompiler/navigationMultipleRuntimes/" + + fun testNavigationToDecompiled() { + doTest(false, "expected.decompiled") + } + + fun testNavigationToLibrarySources() { + doTest(true, "expected.sources") + } + + override fun createProjectLib(libraryName: String, withSources: Boolean): Library { + val libraryJar = ForTestCompileRuntime.runtimeJarForTests().copyTo(File(createTempDirectory(), "$libraryName.jar")) + val jarUrl = libraryJar.jarRoot() + return runWriteAction { + val library = ProjectLibraryTable.getInstance(project).createLibrary(libraryName) + val modifiableModel = library.modifiableModel + modifiableModel.addRoot(jarUrl, OrderRootType.CLASSES) + if (withSources) { + val sourcesJar = ForTestCompileRuntime.runtimeSourcesJarForTests().copyTo(File(createTempDirectory(), "$libraryName-sources.jar")) + modifiableModel.addRoot(sourcesJar.jarRoot(), OrderRootType.SOURCES) + } + modifiableModel.commit() + library + } + } +} + +abstract class AbstractNavigationWithMultipleLibrariesTest : ModuleTestCase() { + + abstract val testDataPath: String + fun doTest(withSources: Boolean, expectedFileName: String) { val srcPath = testDataPath + "src" val moduleA = module("moduleA", srcPath) val moduleB = module("moduleB", srcPath) - val librarySrc = testDataPath + "libSrc" - addDependencyOnProjectLibrary(moduleA, "libA", librarySrc, withSources) - addDependencyOnProjectLibrary(moduleB, "libB", librarySrc, withSources) + addDependencyOnProjectLibrary(moduleA, "libA", withSources) + addDependencyOnProjectLibrary(moduleB, "libB", withSources) // navigation code works by providing first matching declaration from indices // that's we need to check references in both modules to guard against possibility of code breaking @@ -76,30 +127,17 @@ class NavigationWithMultipleLibrariesTest : ModuleTestCase() { } private fun findSourceFile(moduleA: Module): PsiFile { - val ioFile = File(moduleA.getModuleDir()).listFiles()[0]; + val ioFile = File(moduleA.getModuleDir()).listFiles().first() val vFile = LocalFileSystem.getInstance().findFileByIoFile(ioFile)!! return PsiManager.getInstance(project).findFile(vFile)!! } - private fun addDependencyOnProjectLibrary(mainModule: Module, libraryName: String, librarySrc: String, withSources: Boolean) { - val library = createProjectLib(libraryName, librarySrc, withSources) + private fun addDependencyOnProjectLibrary(mainModule: Module, libraryName: String, withSources: Boolean) { + val library = createProjectLib(libraryName, withSources) ModuleRootModificationUtil.addDependency(mainModule, library, DependencyScope.COMPILE, false) } - private fun createProjectLib(libName: String, librarySrc: String, withSources: Boolean): Library { - val libraryJar = MockLibraryUtil.compileLibraryToJar(librarySrc, libName, withSources, false, false) - val jarUrl = StandardFileSystems.getJarRootForLocalFile(LocalFileSystem.getInstance().findFileByIoFile(libraryJar)!!)!! - return runWriteAction { - val library = ProjectLibraryTable.getInstance(project).createLibrary(libName) - val modifiableModel = library.modifiableModel - modifiableModel.addRoot(jarUrl, OrderRootType.CLASSES) - if (withSources) { - modifiableModel.addRoot(jarUrl.findChild("src")!!, OrderRootType.SOURCES) - } - modifiableModel.commit() - library - } - } + abstract fun createProjectLib(libraryName: String, withSources: Boolean): Library } @@ -112,4 +150,6 @@ private fun checkLibraryName(referenceTarget: PsiElement, expectedName: String) else -> error("Couldn't get library name") } Assert.assertEquals("Referenced code from unrelated library: ${referenceTarget.text}", expectedName, libraryName) -} \ No newline at end of file +} + +private fun File.jarRoot() = StandardFileSystems.getJarRootForLocalFile(LocalFileSystem.getInstance().findFileByIoFile(this)!!)!!