Breakpoints for Java lambdas.

[lahodaj]

This is something I was looking at long time ago, but was not very happy about the UI. So I was trying to improve the UI, but I didn't make progress on that, and this got stashed. So, now I decided to try to recover this, even with the not-so-nice UI.

I would consider this patch to be a fairly advanced prototype.

Currently, having code like:

        List.of("a", "b", "c")
            .stream()
            .map(s -> s).filter(s -> !s.isEmpty())
            .forEach(s -> System.err.println(s));
    }

putting a breakpoint at the line with map will mean the debugger will stop at the line when map is called, and then each time any of the lambdas is called. This is rarely what one wants. It would be better to be able to say that I want the debugger to stop at e.g. the first lambda.

This is what this PR is trying to do. There's an ability to specify the breakpoint applies to n-th lambda on the given line. Special values are -1, which means to stop at the line outside of any lambda, and Integer.MIN_VALUE, which means stop each time this line is reached, regardless of any lambdas. This is the way it works on the backend.

Inside the NetBeans UI, it works like this:

• when a line breakpoint is added on a line with lambdas, it is considered to be on the line only, and there's a disable lambda breakpoint added for each lambda automatically. These can then be enabled/disabled independently.
• when the line breakpoint is removed, all the lambda breakpoints are removed as well.

The ability is also used in the DAP Java server, for use in DAP clients (e.g. in VS Code). The UI in VS Code is better, but also has some sharp edges.

Inside NB, it can look like this:

---

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

• are all your own work, and you have the right to contribute them.
• are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

1. Was this PR correctly labeled, did the right tests run? When did they run?
2. Is this PR squashed?
3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

[matthiasblaesing]

• I think the "don't break in lambda by default" is not a good approach. My reasoning: The feature is hard to discover, so without knowledge of this PR I would come to the conclusion "NetBeans can't break in lambdas". Breaking on the line outside the lambda (i.e. before execution of the chain) is IMHO the least interesting breakpoint as the intermediate steps are what I would be interested in.

• The don't break by default setting is also harmful for people using multiple lines to make the lambda readable. Today they can set a break point and it will work as expected. With this the breakpoint you can set without hassle is basicly useless. I mean this formatting:

• The lambda breakpoint enablement is unstable. Consider this starting point (nothing is running right now):

When I invoke "Debug file", the activation of the lamdba breakpoint is lost (see lambda index 2):

What I wonder in summary is: Isn't this "just" another "condition"? The ability to break in lambda is already there, at least I can set a break point for the example in the row with multiple lambdas and get multiple stops, that match what I would expect for the lambda execution. What is missing from that POV is "just" a filter to select the right lambda(s).

The drawback I see is, that the multiple breakpoints would allow more flexibility (independent counts for conditionally enabled breakpoints, conditions, log output).

[lahodaj]

@matthiasblaesing, thanks for trying. I guess what you are saying is that:

• by default, the breakpoints should stop everywhere (i.e. the lambda index should be Integer.MIN_VALUE in the current internal meaning)
• the automatically added lambda breakpoints should be dropped.

That way, anyone could go to the settings and specify the lambda index. (+the UI maybe could be slightly better e.g. a combobox showing "Everywhere"/"Main Line Only"/"Lambda 1"/"Lambda 2"/...). Note the breakpoint properties already allow to set the lambda index in this prototype.

I can't say I disagree with that. The automagically setup breakpoints where a bit preparation for a (much) better UI (similar to what VS Code is doing), but I don't have the UI, and I don't know if I ever will. So maybe this is simply too complex. And it can be resurrected if we have the better UI.

[matthiasblaesing]

Yes you understood my point correctly. I'm not opposed to multiple breakpoints in general, but from a UI/UX perspective I think making the options available in the line break point might be more realistic as the UI impact would be smaller.

I think it would be good if the "lamdba" "condition" would allow to select multiple lambdas, for this example (in reality I expect lambdas with more interesting content in it 😄):

public class Test3 {

    public static void main(String[] args) {
        List.of("a", "b", "c")
                .stream()
                .map(s -> s.trim()).filter(s -> !s.isEmpty()).forEach(s -> System.err.println(s));
    }
}

it might make sense to break on the first and last lamdba. That way you can observe what goes into the stream pipeline and what reaches the end.

[lahodaj]

FWIW, the lambda index is now a list, and the weird UI is removed. So, e.g. -1, 1 is a valid list of lambda indices to stop on, and means "stop outside of any lambda, and on the second lambda on the line".

Also, there's a test added, which should (hopefully) ensure the lambda breakpoints work as intended over DAP.

I think this is ready for review.

[matthiasblaesing]

A manual test with NetBeans IDE was successful for me and showed the expected behavior:

• setting a breakpoint without further config causes breaks outside the lambdas and at each lambda
• setting a lambda index filter causes breaks at the expected places
• it was possible to create multiple breakpoints on the same line, so that I could breakpoints with different behavior (thread stopping, only logging)

I eyeballed the IDE side implementation and in general that made sense to me. I added a few inline comments, but nothing major.

One thing I'm not sure about is 0 vs. 1 based indexes. I.e. entering 0 to get breaks on the first lambda gave me a slight dissonance. On the other hand are java developers accustomed to 0 == first index.

I skipped the LSP parts.

[matthiasblaesing]

I suggest to add a minimal explanation/usage instruction:

Suggested change

	L_Line_Breakpoint_Lambda_Index=Lambda &Index\:
	ACSD_L_Line_Breakpoint_Lambda_Index=Lambda Index
	ACSD_TF_Line_Breakpoint_Lambda_Index=Lambda Index
	TTT_TF_Line_Breakpoint_Lambda_Index=Lambda Index to stop at
	L_Line_Breakpoint_Lambda_Index=Lambda &Indexes\:
	ACSD_L_Line_Breakpoint_Lambda_Index=Lambda Indexes
	ACSD_TF_Line_Breakpoint_Lambda_Index=Lambda Indexes
	TTT_TF_Line_Breakpoint_Lambda_Index=Comma seperated lambda indexes to stop at\nempty: all locations\n-1: stop outside of lambda\n0-based index: break in that lambda expression

[matthiasblaesing]

Would it make sense to use correct plural form? I.e. getLambdaIndexes?

[matthiasblaesing]

Should this be localized?

[matthiasblaesing]

I think we should be prepared for bogus values, so I would suggest this:

Suggested change

	breakpoint.setLambdaIndex(Arrays.stream(lambdaIndexText.split(", *")).mapToInt(v -> Integer.parseInt(v)).toArray());
	int[] indexes = Arrays.stream(lambdaIndexText.split(", *"))
	.map(indexString -> {
	try {
	return Integer.parseInt(indexString);
	} catch (NumberFormatException ex) {
	// Ignore invalid values
	return null;
	}
	})
	.filter(value -> value != null)
	.mapToInt(i -> i)
	.toArray();
	breakpoint.setLambdaIndex(indexes);

[matthiasblaesing]

I assume this is done to get the raw ints boxed?

[matthiasblaesing]

Should here be a comment, that this relies on the lambda naming convention and might break? I only suspect, that this detail is not described in the JLS?