Add support for lambda breakpoints

[gayanper]

The support for method header breakpoints was extended to support
lambda breakpoints using the vscode inline breakpoint feature.

[gayanper]

@testforstephen let me know what you think about this change. May be the name of the breakpoint displayed in UI might need a change. I just named it as lambda breakpoint.

[gayanper]

By the way I had to add a new locator implementation to find LambdaExpressions since the JDT.Core doesn't work out of the box since it works with selection offsets

[testforstephen]

@gayanper Is there any code snippet to demo how this works?

[gayanper]

@gayanper Is there any code snippet to demo how this works?

LambdaDebug.mp4

[testforstephen]

I tried adding an inline breakpoint on a lambda, it works well with current implementation. This is a good start for inline breakpoint support. But I'm not sure whether current approach can be extended to other inline breakpoint case. I'm thinking whether there is a systematic way to handle inline breakpoint.

[gayanper]

I tried adding an inline breakpoint on a lambda, it works well with current implementation. This is a good start for inline breakpoint support. But I'm not sure whether current approach can be extended to other inline breakpoint case. I'm thinking whether there is a systematic way to handle inline breakpoint.

I think we can add support for that as well. The same class we use to search for Lambda expression can be evolve into finding MethodInvocations at AST level at the given column and create the breakpoint for that particular method like we use for method header breakpoints and lambda. I think i can work on it if you think that will add value. I have not come across java IDEs provide such a feature. So may be it will be really good one to have.

[testforstephen]

I think we can add support for that as well. The same class we use to search for Lambda expression can be evolve into finding MethodInvocations at AST level at the given column and create the breakpoint for that particular method like we use for method header breakpoints and lambda. I think i can work on it if you think that will add value. I have not come across java IDEs provide such a feature. So may be it will be really good one to have.

We can release the lambda inline breakpoint first. Currently, inline breakpoints are not so obvious to new users. I believe one day we need to support BreakpointLocationsRequest microsoft/vscode-java-debug#1193, which will hint user the possible inline breakpoints in a line.

[gayanper]

I think we can add support for that as well. The same class we use to search for Lambda expression can be evolve into finding MethodInvocations at AST level at the given column and create the breakpoint for that particular method like we use for method header breakpoints and lambda. I think i can work on it if you think that will add value. I have not come across java IDEs provide such a feature. So may be it will be really good one to have.

We can release the lambda inline breakpoint first. Currently, inline breakpoints are not so obvious to new users. I believe one day we need to support BreakpointLocationsRequest microsoft/vscode-java-debug#1193, which will hint user the possible inline breakpoints in a line.

Thats on my radar as well ;)

[testforstephen]

Found two bugs with the snippet list.stream().map(▮name -> name).forEach((▮item) -> System.out.println(item)):

• If adding 2+ inline breakpoints in the same line, only one is added successfully.
• The second inline breakpoint is always recognized as lambda$1, but it's supposed to be lambda$2.

[testforstephen]

Found two bugs with the snippet list.stream().map(▮name -> name).forEach((▮item) -> System.out.println(item)):

• If adding 2+ inline breakpoints in the same line, only one is added successfully.
• The second inline breakpoint is always recognized as lambda$1, but it's supposed to be lambda$2.

The first issue still exists. The reason is current BreakpointManager uses LineNumber to check if breakpoints are the same. For the inline breakpoints on the same line, the BreakpointManager will add them as the same inline breakpoint.

java-debug/com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/BreakpointManager.java

Lines 80 to 84 in 8c0e87d

	// Compute the breakpoints that are newly added.
	List<IBreakpoint> toAdd = new ArrayList<>();
	List<Integer> visitedLineNumbers = new ArrayList<>();
	for (IBreakpoint breakpoint : breakpoints) {
	IBreakpoint existed = breakpointMap.get(String.valueOf(breakpoint.getLineNumber()));

And the optimized lambda search works for single line lambdas, but does not work for multi-line lambdas. For mult-line lambda, one idea is to limit to the available range from the lambda start position to the body start position.

list.stream().map(🔴name -> name).forEach((🔴item) -> {
    System.out.println(item);
});

[gayanper]

@testforstephen i will look at this in the evening. Did a quick correction in the morning when i saw that i was using wrong column variable. But i think for multi line spanning we need another fix

[gayanper]

@testforstephen i was wondering if we need to send back the column to client as well ?

[testforstephen]

@testforstephen i was wondering if we need to send back the column to client as well ?

Are there any clients that can consume the returned column info? If so, it would be good to return the actual column covered by the breakpoint.

[gayanper]

The protocol has the support and I thought to have the column in the internal breakpoin to make it unique and use line:column as the map key. Now since we have the column we can send it back.

Not sure if vscode will use it for anything or not.

[testforstephen]

this condition only works for expression-style lambda with 2 lines.

[gayanper]

I tested for

        Arrays.asList(1, 2, 3).stream().map(name -> name).forEach(🔴(item) -> {
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
        });

and

        Arrays.asList(1, 2, 3).stream().map(name -> name).forEach(
            (item) -> 🔴{
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
            System.out.println(item + 1 + 2 + 3 + 4 + 5);
        });

Its working, can you give a example snippet ?

[testforstephen]

Case 1:

This will hit lambda entry on line 21.

Case 2:

This will hit lambda entry on line 22.

(startLine != endLine && line >= startLine && line <= endLine
&& (column >= startColumn || column <= endColumn))

It is not accurate to compare the column number for multi-line lambda. It might be better to compare the offset.

[gayanper]

True. But I couldn’t find a method to calculate offset by providing the line and column. But will check more to see if i have missed any such method. JDT should have such methods, not sure if they are part of the API.

[testforstephen]

You can use this.compilationUnit#getPosition(line, column) to get the offset.

[gayanper]

Thanks. I think i missed it. The problem with current condition is I didn’t intend to support blocks. But still if we only do boundry check and ignore middle lines it will work when we have multi lines. But position is much simpler. I will switch to that.

[gayanper]

For the scenario 1 as user what you would expect ?

[testforstephen]

For the scenario 1 as user what you would expect ?

I expect adding a breakpoint on (Line 22, Column 24), and if Column 24 breakpoint is not supported, then fallback to a line breakpoint on Line 22.

If it's inside a block body, I'll treat it as a normal function body and not fall back to the Lambda entry breakpoint.

[testforstephen]

LambdaExpression.getBody() will return either a Block or an Expression, which we can use to determine which lambda it is.

If it's an expression-style lambda, it's ok to enable Lambda breakpoint if the offset is within the whole LambdaExpression range.
If it's a block-style lambda, I would enable Lambda breakpoint only if the offset is in [LambdaStart, LambdaBodyStart).

Just my two cents.

[gayanper]

Now keeping mind the future expansion of inline breakpoints for method invocations, I decided to restrict the lambda breakpoints by

• only support for expressions
• the inline breakpoint should be before the expression

This way we will not break any functionality nor complicate breakpoint logic in future when we introduce method invocation inline breakpoints. And for lambda blocks we can always use line breakpoints which is much clear and straight forward.

WDYT ?

[testforstephen]

I agree with simplifying the condition check of lambda breakpoint. Checking for the offset within [lambdaStart, bodyStart] works well for me.

One minor comment is to remove the unncessary nodeType check. Since LambdaExpression.getNodeType() returns ASTNode.LAMBDA_EXPRESSION, the condition node.getNodeType() != ASTNode.BLOCK is always true and i just remove it from the commit.

Others look good to me.

[testforstephen]

Had a try on the typescript inline breakpoint, it would show an extra cursor on the target column when hitting on inline breakpoint. It's a better experience we can leverage.