Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debugger: Smart Step Into #123879

Open
hediet opened this issue May 14, 2021 · 15 comments
Open

Debugger: Smart Step Into #123879

hediet opened this issue May 14, 2021 · 15 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Milestone

Comments

@hediet
Copy link
Member

hediet commented May 14, 2021

Problem

Given that we are debugging the following code:

console.log(add(f1(), f2())); // This is the currently active line that is about to be run.

function f1() {
    // ...
    return 1;
}

function f2() {
    // ...
    return 2;
}

function add(a, b) {
    return a + b + 1;
}

Next, we want to step into add to fix our bug. However, if we use the "Step Into" command, we step into f1 and f2 first.
This is especially annoying when there are a lot of uninteresting getXYZ functions/getters.

Suggested Solution

It would be nice if VS Code had a way so that the user can specify in which method/function they want to step into.
IntelliJ and PhpStorm have a "smart step into" feature that does this.

If there is only one target to step into, nothing can be done.
However, if there are multiple targets that the debugger can step into, it would be nice if the user could select one.

There a many ways how the selection story can be implemented.

  • By using a quick pick dialog. (like PhpStorm in 2013) If "Step Into" is called and the debug adapter reports multiple targets, show them all in a quick pick dialog (it would list "add", "f1" and "f2"). If the user selects one, the debugger will step into the selected target. This might be confusing if a method appears twice.

  • Source Code Range Highlight + Selection. (similar to IntelliJ) If "Step Into" is called and the debug adapter reports multiple target ranges, highlight all those ranges like this (imagine the line being yellow):
    image
    If the user clicks on a range, the debugger will step into the selected target. With the arrow keys to select a target range and enter to accept it, a keyboard only story can be provided.

  • Modifier Key + Inline Decorators.
    While the user holds Ctrl, inline target decorators are shown before each step-into target in the line that is about to be executed like this (imagine the line being yellow):
    image
    If the user clicks on one of those decorators while Ctrl is pressed, the debugger steps into the selected target. "Step Into" does not need to be invoked before for this scenario and does not need to be changed. This still allows for the "Go To Declaration/Definition" scenario, as separate click targets are shown.
    This could be extended for "Go To Cursor" too (similar to how Chrome does it, except that they don't render additional inline decorators as they don't have the "Go To Definition" feature).
    This approach is very efficient for users who debug with the mouse. Keyboard users could enter this "Show All Step-Into Target Decorators" mode with a specific command and then use the arrow keys to select a target.

JS Debug Adapter Implementation Details

Afaik, the V8 debugger does not directly provide the information/commands required for this.
Some heuristic involving the AST might be needed. I think it is ok if this feature is not enabled in situations where the list of available targets cannot be statically computed. If it is enabled though, it should always work.

The JS debug adapter could always "step in" and use "step out" if the method that V8 stepped into does not match the selected target.

Examples

Here I would like to step into _applySessionResult, skipping _addSelectionToNextFindMatch. I think pressing "ctrl" + clicking on an appearing "jump to/in" decorator would be a really fun experience:

image

@hediet hediet added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues under-discussion Issue is under discussion for relevance, priority, approach labels May 14, 2021
@isidorn
Copy link
Contributor

isidorn commented May 14, 2021

@hediet thanks a lot for creating this issue and for writing down the ideas.
For now assigning also to @connor4312
fyi @weinand

@connor4312
Copy link
Member

Hm, yea. This is tricky to do. An AST heuristic would not necessarily work, since you can step into getter functions which are indistinguishable from property accesses at the AST level. Though we could combine an AST heuristic and "ignore" getter functions, that has potential to work.

For named functions we can step in and step out until we pause at a function matching the name, which would work for many cases but would not handle any more intricate redirection or anonymous functions. I wonder how jetbrains does it.

Another idea would be using the language service's "Go to Implementation" with a Run to Cursor-like mechanism. This would work across languages, but fairly poor reliability.

I'm guessing Jetbrains does the AST approach.

@hediet
Copy link
Member Author

hediet commented May 14, 2021

I don't know if JetBrains does it for JavaScript, they do it for C#, Java and PHP though.
(Also, this feature has two sides: The VS Code UI and the JavaScript debugger - however, having the UI does not make much sense if there is no debug adapter that implements it)

Btw., you can distinguish getter functions from property accesses by using CDB. There is a function for side-effect free evaluation.

I think it would already be enough if it works for simple functions/methods that are not renamed (like, I would say, it is usually the case). In all other cases, this feature could just be not available, so that there will be no "buggy" behavior that users can complain about.

@connor4312
Copy link
Member

connor4312 commented May 14, 2021

Ok, so for JS we could actually do this quite well. The call frame when we step into arguments actually gives us the exact column the function is being called from, so we can remember where functions in that line are and step in and out until we get to the right one. We will just need to do some AST parsing, which might fail for certain TypeScript functions but should work generally well (we use acorn's loose mode for parsing in js-debug; I used to use TypeScript, but including TS in the extension was massive and its parsing is much slower than acorn)

@bpasero
Copy link
Member

bpasero commented May 17, 2021

I had filed #119491 a while ago: I really like the feature where I can Ctrl+click into a method and the debugger stops in the first line of that method. This would also not add any breakpoints it would just stop once. This way you can navigate around the code with the debugger.

@weinand
Copy link
Contributor

weinand commented May 17, 2021

VS Code supports the proposed "Smart Step Into" feature already via DAP's StepInTargets Request.

See https://code.visualstudio.com/updates/v1_46#_step-into-targets

But debug extensions must opt into this (Mock Debug does, js-debug does not yet, I'm not sure about C# and C++)

@weinand
Copy link
Contributor

weinand commented May 17, 2021

@bpasero I like your proposed feature #119491 too, but that is not covered by the mentioned VS Code's "StepInTargets" feature.

@connor4312
Copy link
Member

Oh, neat, I've never seen that! I will track this as a feature request for js-debug and re-tag Isi if I find any missing editor integration.

@connor4312 connor4312 removed the under-discussion Issue is under discussion for relevance, priority, approach label May 17, 2021
@connor4312 connor4312 added this to the On Deck milestone May 17, 2021
@hediet
Copy link
Member Author

hediet commented May 17, 2021

@weinand I guess that is a separate issue and also depends on the success of a possible implementation in js-debug, but what are your thoughts on adding a source code range to StepInTarget? Even if not used today, but that would enable a very nice UI one day. The range would refer to the part of the expression that is about to be evaluated and that would be stepped into. It could be highlighted when selected in the quick select.

@weinand
Copy link
Contributor

weinand commented May 19, 2021

@hediet yes, a range property on the StepInTarget type was already on the radar when we discussed the DAP request initially. But since we (VS Code) were too far away from adopting the StepInTarget feature in VS Code, we decided not to add any properties with unclear UI representation to the spec too early.

Please create a new feature request in the DAP repo for adding a range property.

@hediet
Copy link
Member Author

hediet commented Jul 1, 2021

@isidorn and I did a quick brainstorming session together.

We had a nice idea, however the following gif shows how difficult it is:

recording

Basically, for the bar marked in yellow, chrome dev tools don't allow to set a breakpoint. This means you really have to set the breakpoint in bar itself if you want to step into that particular invocation of bar. This means that some smartness is required to see where a function invocation will jump to.

Otherwise, you could implement "Step into Target" by adding a temporary breakpoint to the target expression and then stepping into it.

function bar() {
   return {
      foo: () => ({
         baz: () => {}
      })
   };
}

function foo() { 
    debugger;
    var obj = {};
    //Object.defineProperty(obj, 'foo', { value: ({ bar }) });
    obj.foo = { bar }; 
    obj[(() => { bar(); return 'foo'; bar(); })()]
        .bar(
          bar())
        .foo().baz();
}
foo();

@connor4312
Copy link
Member

In the approach in #123879 (comment), I suggested doing a repeated step in and step out until we get to where we need to be. As you found, setting breakpoints in complex expressions can be hit and miss, but I think this should work given that line and column numbers of the callers are reported correctly.

@Trass3r
Copy link

Trass3r commented Jul 16, 2021

VS Code supports the proposed "Smart Step Into" feature already via DAP's StepInTargets Request.

But debug extensions must opt into this (Mock Debug does, js-debug does not yet, I'm not sure about C# and C++)

The C++ ticket is microsoft/vscode-cpptools#2488
But it'd require compiler support according to the gdb bug ticket.

@akaroml
Copy link
Member

akaroml commented Oct 19, 2022

Here to upvote for the second solution mentioned by @hediet

When debugging, to step into a target, we now need to right-click on a symbol and pick "Step into target" from the context menu. This works, but the context menu entry has a weakness of discoverability, and the context menu is also a distraction.

With the second solution, the "Step into target" melts into the regular "Step into," while visual hints are presented to the users directly. The benefits are

  1. The capability to step into a target is more easily discovered, and no extra learning is needed
  2. Less steps required
  3. No visual distraction (no context menu involved)

While the implementation of debuggers (Java debugger AFAIK) already reports the target candidates to vscode, it should be feasible now. @testforstephen

@FeldrinH
Copy link

FeldrinH commented Feb 28, 2023

I also like the seconds solution mentioned in the original post.

In my experience there are three things that make the current approach clumsy which the second solution solves:

  1. Having to manullay open a context menu to select the step in target gets quite tedious if you do it a lot.
  2. Having separate step into and step into targets actions makes it possible to accidentally step into the default target without intending to.
  3. A lot of the times the labels provided by the debuggers I use aren't very clear. Having a visual indicator of the location of the step in target would be make it much clearer what I am selecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

8 participants