From f0cfd450e5fda979a609deba3e833876c44d3ba4 Mon Sep 17 00:00:00 2001 From: Natalia Ukhorskaya Date: Wed, 6 Apr 2016 17:30:35 +0300 Subject: [PATCH] Debugger: fix step over inside inline function (do not step into function literal argument) --- ChangeLog.md | 4 + .../stepping/DebuggerSteppingHelper.java | 99 ++++++------- .../stepping/KotlinSteppingCommandProvider.kt | 130 ++++++++++++------ .../tinyApp/outs/stepOverIfWithInline.out | 1 - .../tinyApp/outs/stepOverInsideInlineFun.out | 10 ++ .../outs/stepOverTryCatchWithInline.out | 3 + .../stepOver/stepOverInsideInlineFun.kt | 16 +++ .../stepOver/stepOverTryCatchWithInline.kt | 2 +- .../debugger/KotlinSteppingTestGenerated.java | 6 + 9 files changed, 171 insertions(+), 100 deletions(-) create mode 100644 idea/testData/debugger/tinyApp/outs/stepOverInsideInlineFun.out create mode 100644 idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverInsideInlineFun.kt diff --git a/ChangeLog.md b/ChangeLog.md index 8de30ead4ee..34ac51409e1 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -11,6 +11,10 @@ ### IDE +#### Debugger + +- Do not step into inline lambda argument during step over inside inline function body + ## 1.0.2 ### Compiler diff --git a/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/DebuggerSteppingHelper.java b/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/DebuggerSteppingHelper.java index bf385726d48..8112a91089c 100644 --- a/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/DebuggerSteppingHelper.java +++ b/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/DebuggerSteppingHelper.java @@ -20,12 +20,8 @@ import com.intellij.debugger.engine.DebugProcessImpl; import com.intellij.debugger.engine.SuspendContextImpl; import com.intellij.debugger.engine.evaluation.EvaluateException; import com.intellij.debugger.jdi.StackFrameProxyImpl; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.util.Computable; import com.intellij.psi.PsiElement; -import com.intellij.xdebugger.impl.XSourcePositionImpl; -import kotlin.*; -import kotlin.ranges.*; +import kotlin.ranges.IntRange; import org.jetbrains.kotlin.psi.KtFile; import org.jetbrains.kotlin.psi.KtFunction; import org.jetbrains.kotlin.psi.KtFunctionLiteral; @@ -47,36 +43,29 @@ public class DebuggerSteppingHelper { return debugProcess.new ResumeCommand(suspendContext) { @Override public void contextAction() { - final StackFrameProxyImpl frameProxy = suspendContext.getFrameProxy(); - if (frameProxy != null) { - DebugProcessImpl.ResumeCommand runToCursorCommand = ApplicationManager.getApplication().runReadAction(new Computable() { - @Override - public DebugProcessImpl.ResumeCommand compute() { - try { - XSourcePositionImpl position = KotlinSteppingCommandProviderKt.getStepOverPosition( - frameProxy.location(), - file, - linesRange, - inlineArguments, - additionalElementsToSkip - ); - if (position != null) { - return debugProcess.createRunToCursorCommand(suspendContext, position, ignoreBreakpoints); - } - } - catch (EvaluateException ignored) { - } - return null; + try { + StackFrameProxyImpl frameProxy = suspendContext.getFrameProxy(); + if (frameProxy != null) { + Action action = KotlinSteppingCommandProviderKt.getStepOverPosition( + frameProxy.location(), + file, + linesRange, + inlineArguments, + additionalElementsToSkip + ); + + DebugProcessImpl.ResumeCommand command = action.createCommand(debugProcess, suspendContext, ignoreBreakpoints); + + if (command != null) { + command.contextAction(); + return; } - }); - - if (runToCursorCommand != null) { - runToCursorCommand.contextAction(); - return; } - } - debugProcess.createStepOutCommand(suspendContext).contextAction(); + debugProcess.createStepOutCommand(suspendContext).contextAction(); + } + catch (EvaluateException ignored) { + } } }; } @@ -91,35 +80,29 @@ public class DebuggerSteppingHelper { return debugProcess.new ResumeCommand(suspendContext) { @Override public void contextAction() { - final StackFrameProxyImpl frameProxy = suspendContext.getFrameProxy(); - if (frameProxy != null) { - DebugProcessImpl.ResumeCommand runToCursorCommand = ApplicationManager.getApplication().runReadAction(new Computable() { - @Override - public DebugProcessImpl.ResumeCommand compute() { - try { - XSourcePositionImpl position = KotlinSteppingCommandProviderKt.getStepOutPosition( - frameProxy.location(), - suspendContext, - inlineFunctions, - inlineArgument - ); - if (position != null) { - return debugProcess.createRunToCursorCommand(suspendContext, position, ignoreBreakpoints); - } - } - catch (EvaluateException ignored) { - } - return null; + try { + StackFrameProxyImpl frameProxy = suspendContext.getFrameProxy(); + if (frameProxy != null) { + Action action = KotlinSteppingCommandProviderKt.getStepOutPosition( + frameProxy.location(), + suspendContext, + inlineFunctions, + inlineArgument + ); + + DebugProcessImpl.ResumeCommand command = action.createCommand(debugProcess, suspendContext, ignoreBreakpoints); + + if (command != null) { + command.contextAction(); + return; } - }); - - if (runToCursorCommand != null) { - runToCursorCommand.contextAction(); - return; } - } - debugProcess.createStepOverCommand(suspendContext, ignoreBreakpoints).contextAction(); + debugProcess.createStepOverCommand(suspendContext, ignoreBreakpoints).contextAction(); + } + catch (EvaluateException ignored) { + + } } }; } diff --git a/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/KotlinSteppingCommandProvider.kt b/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/KotlinSteppingCommandProvider.kt index 7758ed79429..53aa0702d8d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/KotlinSteppingCommandProvider.kt +++ b/idea/src/org/jetbrains/kotlin/idea/debugger/stepping/KotlinSteppingCommandProvider.kt @@ -36,6 +36,7 @@ import org.jetbrains.kotlin.idea.refactoring.getLineEndOffset import org.jetbrains.kotlin.idea.refactoring.getLineNumber import org.jetbrains.kotlin.idea.refactoring.getLineStartOffset import org.jetbrains.kotlin.idea.util.DebuggerUtils +import org.jetbrains.kotlin.idea.util.application.runReadAction import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.getParentOfType @@ -78,22 +79,31 @@ class KotlinSteppingCommandProvider: JvmSteppingCommandProvider() { val linesRange = startLineNumber + 1..endLineNumber + 1 - val inlineFunctionCalls = getInlineFunctionCallsIfAny(sourcePosition) - if (inlineFunctionCalls.isEmpty()) return null - - val inlineArguments = getInlineArgumentsIfAny(inlineFunctionCalls) - - if (inlineArguments.any { it.shouldNotUseStepOver(sourcePosition.elementAt) }) { - return null - } - - if (inlineArguments.isEmpty() && inlineFunctionCalls.any { it.shouldNotUseStepOver(sourcePosition.elementAt) }) { - return null - } + val inlineArgumentsToSkip = getElementsToSkip(containingFunction as KtNamedFunction, sourcePosition) ?: return null val additionalElementsToSkip = sourcePosition.elementAt.getAdditionalElementsToSkip() - return DebuggerSteppingHelper.createStepOverCommand(suspendContext, ignoreBreakpoints, file, linesRange, inlineArguments, additionalElementsToSkip) + return DebuggerSteppingHelper.createStepOverCommand(suspendContext, ignoreBreakpoints, file, linesRange, inlineArgumentsToSkip, additionalElementsToSkip) + } + + private fun getElementsToSkip(containingFunction: KtNamedFunction, sourcePosition: SourcePosition): List? { + val inlineFunctionCalls = getInlineFunctionCallsIfAny(sourcePosition) + if (!inlineFunctionCalls.isEmpty()) { + val inlineArguments = getInlineArgumentsIfAny(inlineFunctionCalls) + if (inlineArguments.isNotEmpty() && inlineArguments.all { !it.shouldNotUseStepOver(sourcePosition.elementAt) }) { + return inlineArguments + } + + if (inlineArguments.isEmpty() && inlineFunctionCalls.all { !it.shouldNotUseStepOver(sourcePosition.elementAt) }) { + return emptyList() + } + } + + if (InlineUtil.isInline(containingFunction.resolveToDescriptor())) { + return emptyList() + } + + return null } private fun PsiElement.getAdditionalElementsToSkip(): List { @@ -287,14 +297,36 @@ class KotlinSteppingCommandProvider: JvmSteppingCommandProvider() { } } +sealed class Action(val position: XSourcePositionImpl?) { + class STEP_OVER: Action(null) + class STEP_OUT: Action(null) + class RUN_TO_CURSOR(position: XSourcePositionImpl): Action(position) + + fun createCommand( + debugProcess: DebugProcessImpl, + suspendContext: SuspendContextImpl, + ignoreBreakpoints: Boolean + ): DebugProcessImpl.ResumeCommand? { + return when (this) { + is Action.RUN_TO_CURSOR -> { + runReadAction { + debugProcess.createRunToCursorCommand(suspendContext, position!!, ignoreBreakpoints) + } + } + is Action.STEP_OUT -> debugProcess.createStepOutCommand(suspendContext) + is Action.STEP_OVER -> debugProcess.createStepOverCommand(suspendContext, ignoreBreakpoints) + } + } +} + fun getStepOverPosition( location: Location, file: KtFile, range: IntRange, inlinedArguments: List, elementsToSkip: List -): XSourcePositionImpl? { - val computedReferenceType = location.declaringType() ?: return null +): Action { + val computedReferenceType = location.declaringType() ?: return Action.STEP_OVER() fun isLocationSuitable(nextLocation: Location): Boolean { if (nextLocation.method() != location.method() || nextLocation.lineNumber() !in range) { @@ -316,17 +348,28 @@ fun getStepOverPosition( .dropWhile { it.lineNumber() == location.lineNumber() } for (locationAtLine in locations) { - val lineNumber = locationAtLine.lineNumber() - val lineStartOffset = file.getLineStartOffset(lineNumber - 1) ?: continue - if (inlinedArguments.any { it.textRange.contains(lineStartOffset) }) continue - if (elementsToSkip.any { it.textRange.contains(lineStartOffset) }) continue + val xPosition = runReadAction l@ { + val lineNumber = locationAtLine.lineNumber() + val lineStartOffset = file.getLineStartOffset(lineNumber - 1) ?: return@l null + if (inlinedArguments.any { it.textRange.contains(lineStartOffset) }) return@l null + if (elementsToSkip.any { it.textRange.contains(lineStartOffset) }) return@l null - val elementAt = file.findElementAt(lineStartOffset) ?: continue + if (locationAtLine.lineNumber() == location.lineNumber()) return@l null - return XSourcePositionImpl.createByElement(elementAt) ?: return null + val elementAt = file.findElementAt(lineStartOffset) ?: return@l null + XSourcePositionImpl.createByElement(elementAt) + } + + if (xPosition != null) { + return Action.RUN_TO_CURSOR(xPosition) + } } - return null + if (locations.isNotEmpty()) { + return Action.STEP_OUT() + } + + return Action.STEP_OVER() } fun getStepOutPosition( @@ -334,8 +377,8 @@ fun getStepOutPosition( suspendContext: SuspendContextImpl, inlineFunctions: List, inlinedArgument: KtFunctionLiteral? -): XSourcePositionImpl? { - val computedReferenceType = location.declaringType() ?: return null +): Action { + val computedReferenceType = location.declaringType() ?: return Action.STEP_OUT() val locations = computedReferenceType.allLineLocations() val nextLineLocations = locations @@ -345,14 +388,16 @@ fun getStepOutPosition( .dropWhile { it.lineNumber() == location.lineNumber() } if (inlineFunctions.isNotEmpty()) { - return suspendContext.getXPositionForStepOutFromInlineFunction(nextLineLocations, inlineFunctions) ?: return null + val position = suspendContext.getXPositionForStepOutFromInlineFunction(nextLineLocations, inlineFunctions) + return position?.let { Action.RUN_TO_CURSOR(it) } ?: Action.STEP_OVER() } if (inlinedArgument != null) { - return suspendContext.getXPositionForStepOutFromInlinedArgument(nextLineLocations, inlinedArgument) ?: return null + val position = suspendContext.getXPositionForStepOutFromInlinedArgument(nextLineLocations, inlinedArgument) + return position?.let { Action.RUN_TO_CURSOR(it) } ?: Action.STEP_OVER() } - return null + return Action.STEP_OVER() } private fun SuspendContextImpl.getXPositionForStepOutFromInlineFunction( @@ -384,20 +429,25 @@ private fun SuspendContextImpl.getNextPositionWithFilter( skip: (Int, PsiElement) -> Boolean ): XSourcePositionImpl? { for (location in locations) { - val sourcePosition = try { - this.debugProcess.positionManager.getSourcePosition(location) + val position = runReadAction l@ { + val sourcePosition = try { + this.debugProcess.positionManager.getSourcePosition(location) + } + catch(e: NoDataException) { + null + } ?: return@l null + + val file = sourcePosition.file as? KtFile ?: return@l null + val elementAt = sourcePosition.elementAt ?: return@l null + val currentLine = location.lineNumber() - 1 + val lineStartOffset = file.getLineStartOffset(currentLine) ?: return@l null + if (skip(lineStartOffset, elementAt)) return@l null + + XSourcePositionImpl.createByElement(elementAt) + } + if (position != null) { + return position } - catch(e: NoDataException) { - null - } ?: continue - - val file = sourcePosition.file as? KtFile ?: continue - val elementAt = sourcePosition.elementAt ?: continue - val currentLine = location.lineNumber() - 1 - val lineStartOffset = file.getLineStartOffset(currentLine) ?: continue - if (skip(lineStartOffset, elementAt)) continue - - return XSourcePositionImpl.createByElement(elementAt) } return null diff --git a/idea/testData/debugger/tinyApp/outs/stepOverIfWithInline.out b/idea/testData/debugger/tinyApp/outs/stepOverIfWithInline.out index ad7ffbb00be..1cde39be060 100644 --- a/idea/testData/debugger/tinyApp/outs/stepOverIfWithInline.out +++ b/idea/testData/debugger/tinyApp/outs/stepOverIfWithInline.out @@ -17,7 +17,6 @@ stepOverIfWithInline.kt:24 stepOverIfWithInline.kt:28 stepOverIfWithInline.kt:46 stepOverIfWithInline.kt:28 -stepOverIfWithInline.kt:28 stepOverIfWithInline.kt:32 stepOverIfWithInline.kt:46 stepOverIfWithInline.kt:32 diff --git a/idea/testData/debugger/tinyApp/outs/stepOverInsideInlineFun.out b/idea/testData/debugger/tinyApp/outs/stepOverInsideInlineFun.out new file mode 100644 index 00000000000..6f2b45f1156 --- /dev/null +++ b/idea/testData/debugger/tinyApp/outs/stepOverInsideInlineFun.out @@ -0,0 +1,10 @@ +LineBreakpoint created at stepOverInsideInlineFun.kt:11 +!JDK_HOME!\bin\java -agentlib:jdwp=transport=dt_socket,address=!HOST_NAME!:!HOST_PORT!,suspend=y,server=n -Dfile.encoding=!FILE_ENCODING! -classpath !OUTPUT_PATH!;!KOTLIN_RUNTIME!;!CUSTOM_LIBRARY!;!RT_JAR! stepOverInsideInlineFun.StepOverInsideInlineFunKt +Connected to the target VM, address: '!HOST_NAME!:PORT_NAME!', transport: 'socket' +stepOverInsideInlineFun.kt:11 +stepOverInsideInlineFun.kt:12 +stepOverInsideInlineFun.kt:13 +stepOverInsideInlineFun.kt:7 +Disconnected from the target VM, address: '!HOST_NAME!:PORT_NAME!', transport: 'socket' + +Process finished with exit code 0 diff --git a/idea/testData/debugger/tinyApp/outs/stepOverTryCatchWithInline.out b/idea/testData/debugger/tinyApp/outs/stepOverTryCatchWithInline.out index 099b953cdc8..ff8908033d0 100644 --- a/idea/testData/debugger/tinyApp/outs/stepOverTryCatchWithInline.out +++ b/idea/testData/debugger/tinyApp/outs/stepOverTryCatchWithInline.out @@ -17,6 +17,9 @@ stepOverTryCatchWithInline.kt:36 stepOverTryCatchWithInline.kt:38 stepOverTryCatchWithInline.kt:39 stepOverTryCatchWithInline.kt:43 +stepOverTryCatchWithInline.kt:48 +stepOverTryCatchWithInline.kt:43 +stepOverTryCatchWithInline.kt:7 stepOverTryCatchWithInline.kt:8 stepOverTryCatchWithInline.kt:10 Disconnected from the target VM, address: '!HOST_NAME!:PORT_NAME!', transport: 'socket' diff --git a/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverInsideInlineFun.kt b/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverInsideInlineFun.kt new file mode 100644 index 00000000000..3f7b90c60ca --- /dev/null +++ b/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverInsideInlineFun.kt @@ -0,0 +1,16 @@ +package stepOverInsideInlineFun + +fun main(args: Array) { + bar { + val b = 1 + } +} + +inline fun bar(f: (Int) -> Unit) { + //Breakpoint! + val a = 1 + val f = f(1) + val c = 1 +} + +// STEP_OVER: 3 \ No newline at end of file diff --git a/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverTryCatchWithInline.kt b/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverTryCatchWithInline.kt index d50444e5c79..9d5924c5b29 100644 --- a/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverTryCatchWithInline.kt +++ b/idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverTryCatchWithInline.kt @@ -51,4 +51,4 @@ inline fun foo(f: () -> Int): Int { fun test(i: Int) = 1 -// STEP_OVER: 19 \ No newline at end of file +// STEP_OVER: 20 \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java index 5fe17cadd4f..524a4623a6f 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java @@ -415,6 +415,12 @@ public class KotlinSteppingTestGenerated extends AbstractKotlinSteppingTest { doStepOverTest(fileName); } + @TestMetadata("stepOverInsideInlineFun.kt") + public void testStepOverInsideInlineFun() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverInsideInlineFun.kt"); + doStepOverTest(fileName); + } + @TestMetadata("stepOverReifiedParam.kt") public void testStepOverReifiedParam() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/debugger/tinyApp/src/stepping/stepOver/stepOverReifiedParam.kt");