From f9845bab54006c6dcda6b8169a575295a4dd249b Mon Sep 17 00:00:00 2001 From: Nikolay Krasko Date: Tue, 31 Jul 2018 19:18:06 +0300 Subject: [PATCH] Workaround for the case when some classes are unloaded during ranking (KT-25774) It's important that ranking preserve order of files found in findFilesByNameInPackage() It's only a workaround because it's expected for debugger to have classes unloaded and some valid variants might get bad rank and rejected because of that. #KT-25774 Fixed --- .../idea/debugger/FileRankingCalculator.kt | 21 +++++++--- .../kotlin/idea/debugger/debuggerUtil.kt | 8 ++++ .../parametersWithUnloadedClass.kt | 35 +++++++++++++++++ .../idea/debugger/AbstractFileRankingTest.kt | 12 ++++++ .../debugger/FileRankingTestGenerated.java | 5 +++ .../idea/debugger/LowLevelDebuggerTestBase.kt | 38 +++++++++++++++---- 6 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 idea/testData/debugger/fileRanking/parametersWithUnloadedClass.kt diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/FileRankingCalculator.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/FileRankingCalculator.kt index a1568a9e214..5583c39a94c 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/FileRankingCalculator.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/FileRankingCalculator.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.idea.debugger +import com.intellij.openapi.diagnostic.Logger import com.intellij.psi.PsiElement import com.sun.jdi.* import org.jetbrains.kotlin.builtins.KotlinBuiltIns @@ -15,13 +16,16 @@ import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.diagnostics.DiagnosticUtils import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.LOW -import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.ZERO +import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.MAJOR import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.MINOR import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.NORMAL -import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.MAJOR +import org.jetbrains.kotlin.idea.debugger.FileRankingCalculator.Ranking.Companion.ZERO import org.jetbrains.kotlin.idea.refactoring.getLineStartOffset import org.jetbrains.kotlin.psi.* -import org.jetbrains.kotlin.psi.psiUtil.* +import org.jetbrains.kotlin.psi.psiUtil.getParentOfType +import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypes2 +import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypes3 +import org.jetbrains.kotlin.psi.psiUtil.isAncestor import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.descriptorUtil.varargParameterPosition import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode @@ -166,7 +170,7 @@ abstract class FileRankingCalculator( val jdiPropertyType = when { accessor.isGetter -> method.safeReturnType() - else -> method.safeArguments()?.map { it.type() }?.singleOrNull() + else -> method.safeArguments()?.map { it.safeType() }?.singleOrNull() } return rankingForType(descriptor.returnType, jdiPropertyType, makeTypeMapper(bindingContext)) @@ -219,7 +223,7 @@ abstract class FileRankingCalculator( ): Ranking { return collect( ktParameter.name.asString() == jdiParameter.name(), - rankingForType(ktParameter.type, jdiParameter.type(), typeMapper) + rankingForType(ktParameter.type, jdiParameter.safeType(), typeMapper) ) } @@ -266,6 +270,9 @@ abstract class FileRankingCalculator( private fun fileRankingSafe(file: KtFile, location: Location): Ranking { return try { fileRanking(file, location) + } catch (e: ClassNotLoadedException) { + LOG.error("ClassNotLoadedException should never happen in FileRankingCalculator", e) + ZERO } catch (e: AbsentInformationException) { ZERO } catch (e: InternalException) { @@ -412,6 +419,10 @@ abstract class FileRankingCalculator( private fun makeTypeMapper(bindingContext: BindingContext): KotlinTypeMapper { return KotlinTypeMapper(bindingContext, ClassBuilderMode.LIGHT_CLASSES, IncompatibleClassTracker.DoNothing, "debugger", false) } + + companion object { + val LOG = Logger.getInstance("FileRankingCalculator") + } } private fun String.simpleName() = substringAfterLast('.').substringAfterLast('$') diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/debuggerUtil.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/debuggerUtil.kt index db0ef6d31a7..f4e4ca060a5 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/debuggerUtil.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/debuggerUtil.kt @@ -70,6 +70,14 @@ fun Method.safeReturnType(): Type? { } } +fun LocalVariable.safeType(): Type? { + return try { + return type() + } catch (e: ClassNotLoadedException) { + null + } +} + fun Location.safeSourceName(): String? { return try { sourceName() diff --git a/idea/testData/debugger/fileRanking/parametersWithUnloadedClass.kt b/idea/testData/debugger/fileRanking/parametersWithUnloadedClass.kt new file mode 100644 index 00000000000..2c927ae1e42 --- /dev/null +++ b/idea/testData/debugger/fileRanking/parametersWithUnloadedClass.kt @@ -0,0 +1,35 @@ +//FILE: a/a.kt +package a + +import c.* + +class B : A() { + override fun x(some: SomeImpl) { + foo() + } +} + +// DO_NOT_LOAD: a.SomeImpl +class SomeImpl : Some() + +//FILE: b/a.kt +package b + +import c.* + +class B : A() { + override fun x(some: Some) { + foo() + } +} + +//FILE: c/c.kt +package c + +open class Some + +abstract class A { + abstract fun x(a: T) +} + +fun foo() {} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/debugger/AbstractFileRankingTest.kt b/idea/tests/org/jetbrains/kotlin/idea/debugger/AbstractFileRankingTest.kt index 4b7082db63b..dc7a7370244 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/debugger/AbstractFileRankingTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/debugger/AbstractFileRankingTest.kt @@ -25,10 +25,16 @@ abstract class AbstractFileRankingTest : LowLevelDebuggerTestBase() { val problems = mutableListOf() + val skipClasses = skipLoadingClasses(options) for ((node, origin) in factory.origins) { val classNode = node as? ClassNode ?: continue val expectedFile = origin.element?.containingFile as? KtFile ?: continue val className = classNode.name.replace('/', '.') + + if (className in skipClasses) { + continue + } + val jdiClass = mainThread.virtualMachine().classesByName(className).singleOrNull() ?: error("Class '$className' was not found in the debuggee process class loader") @@ -56,4 +62,10 @@ abstract class AbstractFileRankingTest : LowLevelDebuggerTestBase() { }) } } + + override fun skipLoadingClasses(options: Set): Set { + val skipClasses = options.mapTo(mutableSetOf()) { it.substringAfter("DO_NOT_LOAD:", "").trim() } + skipClasses.remove("") + return skipClasses + } } \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/debugger/FileRankingTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/debugger/FileRankingTestGenerated.java index ece1770bcd8..e2ce68775ad 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/debugger/FileRankingTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/debugger/FileRankingTestGenerated.java @@ -54,6 +54,11 @@ public class FileRankingTestGenerated extends AbstractFileRankingTest { runTest("idea/testData/debugger/fileRanking/multilinePrimaryConstructorWithBody.kt"); } + @TestMetadata("parametersWithUnloadedClass.kt") + public void testParametersWithUnloadedClass() throws Exception { + runTest("idea/testData/debugger/fileRanking/parametersWithUnloadedClass.kt"); + } + @TestMetadata("propertyDelegates.kt") public void testPropertyDelegates() throws Exception { runTest("idea/testData/debugger/fileRanking/propertyDelegates.kt"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/debugger/LowLevelDebuggerTestBase.kt b/idea/tests/org/jetbrains/kotlin/idea/debugger/LowLevelDebuggerTestBase.kt index 8cba4d4a8c4..4650a912940 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/debugger/LowLevelDebuggerTestBase.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/debugger/LowLevelDebuggerTestBase.kt @@ -33,8 +33,13 @@ abstract class LowLevelDebuggerTestBase : CodegenTestCase() { createEnvironmentWithMockJdkAndIdeaAnnotations(ConfigurationKind.ALL, *javaSources) val options = wholeFile.readLines() - .filter { it.matches("^// ?[\\w_]+$".toRegex()) } - .mapTo(mutableSetOf()) { it.drop(2).trim() } + .asSequence() + .filter { it.matches("^// ?[\\w_]+(:.*)$".toRegex()) } + .map { it.drop(2).trim() } + .filter { !it.startsWith("FILE:") } + .toSet() + + val skipLoadingClasses = skipLoadingClasses(options) loadMultiFiles(files) val classBuilderFactory = OriginCollectingClassBuilderFactory(ClassBuilderMode.FULL) @@ -51,14 +56,14 @@ abstract class LowLevelDebuggerTestBase : CodegenTestCase() { } } - val process = startDebuggeeProcess(classesDir) + val process = startDebuggeeProcess(classesDir, skipLoadingClasses) waitUntil { isPortOpen() } val virtualMachine = attachDebugger() try { val mainThread = virtualMachine.allThreads().single { it.name() == "main" } - waitUntil { areCompiledClassesLoaded(mainThread, classBuilderFactory) } + waitUntil { areCompiledClassesLoaded(mainThread, classBuilderFactory, skipLoadingClasses) } doTest(options, mainThread, classBuilderFactory, generationState) } finally { virtualMachine.exit(0) @@ -85,16 +90,33 @@ abstract class LowLevelDebuggerTestBase : CodegenTestCase() { } } - private fun areCompiledClassesLoaded(mainThread: ThreadReference, factory: OriginCollectingClassBuilderFactory): Boolean { + private fun areCompiledClassesLoaded( + mainThread: ThreadReference, + factory: OriginCollectingClassBuilderFactory, + skipLoadingClasses: Set + ): Boolean { for ((node, _) in factory.origins) { val classNode = node as? ClassNode ?: continue - mainThread.virtualMachine().classesByName(classNode.name.replace('/', '.')).firstOrNull() ?: return false + + val fqName = classNode.name.replace('/', '.') + if (fqName in skipLoadingClasses) { + continue + } + + mainThread.virtualMachine().classesByName(fqName).firstOrNull() ?: return false } return true } - private fun startDebuggeeProcess(classesDir: File): Process { - val classesToLoad = this.classFileFactory.getClassFiles().joinToString(",") { it.qualifiedName } + protected open fun skipLoadingClasses(options: Set): Set { + return emptySet() + } + + private fun startDebuggeeProcess(classesDir: File, skipLoadingClasses: Set): Process { + val classesToLoad = this.classFileFactory.getClassFiles() + .map { it.qualifiedName } + .filter { it !in skipLoadingClasses } + .joinToString(",") val classpath = listOf( classesDir.absolutePath,