Improve support for method breakpoints

[gayanper]

Now method breakpoints can added by adding a line breakpoint at method
header.

[gayanper]

@testforstephen Please review this and let me know what you think ?

[testforstephen]

Had a try and found two issues:

• If I add a breakpoint to method header first and then add an usual line breakpoint to the method body, all breakpoints are skipped during debugging.
  
• Add a breakpoint to the method header, I see it applies to all overloaded methods and they all might be hit on method entry.
  

I'm thinking a different approach to support the breakpoint added through method header. I think it might be easier to handle it as a line breakpoint instead of method breakpoint.

1. Reuse JdtSourceLookUpProvider.getFullyQualifiedName(...) to return FQN#methodSignature if the line points to method header.
2. In the line breakpoint class Breakpoint.collectLocations(...), we can find the JDI method using method signature first and then get the target breakpoint location using com.sun.jdi.Method#location().

[gayanper]

Had a try and found two issues:

• If I add a breakpoint to method header first and then add an usual line breakpoint to the method body, all breakpoints are skipped during debugging.
  
• Add a breakpoint to the method header, I see it applies to all overloaded methods and they all might be hit on method entry.
  

I'm thinking a different approach to support the breakpoint added through method header. I think it might be easier to handle it as a line breakpoint instead of method breakpoint.

1. Reuse JdtSourceLookUpProvider.getFullyQualifiedName(...) to return FQN#methodSignature if the line points to method header.
2. In the line breakpoint class Breakpoint.collectLocations(...), we can find the JDI method using method signature first and then get the target breakpoint location using com.sun.jdi.Method#location().

Yes it might be more simpler and efficient than the method entry request. But will it work for decompiled classes since there is no source location available ? And i wonder if we could use the same for lambda and BreakpointLocations implementation so that it will be more user friendly to add breakpoint to step into just selected method invocation in a expression with many method invocations. WDYT ?

[gayanper]

@testforstephen I will try to change the fix with your suggestion, I think we can support method breakpoints better with your suggestion, specially overcoming the overloaded problem with less effort. About lambda I think we might be able to do the same if we find the correct method invocation signature as the FQN. Thanks for the suggestion, it makes the solution more extendable.

[testforstephen]

But will it work for decompiled classes since there is no source location available ?

If the classfile doesn't include the source location info, current breakpoint policy cannot work.

And i wonder if we could use the same for lambda and BreakpointLocations implementation so that it will be more user friendly to add breakpoint to step into just selected method invocation in a expression with many method invocations.

VS Code 1.69 added a new capability to step into targets, we can adopt it in the scenario of multiple chain calls in the same line.

[gayanper]

@testforstephen I have reworked the solution, please check and let me know.

[gayanper]

@testforstephen Do you think if we need to comparison of generic method signature ?

[testforstephen]

The latest commit works pretty well.

Another improvement we can optimize is the breakpoint event reason. Currently the method header breakpoint stops with the reason "Paused on Breakpoint", it would be better if it can displayed as "Paused on Function Breakpoint". One approach we can use is to store some additional property via BreakpointRequest#putProperty(), and use that to distinguish the BreakpointEvent reason.

[gayanper]

Will improve this on Monday

[testforstephen]

@testforstephen Do you think if we need to comparison of generic method signature ?

I think checking the method signature is enough. I see current implementation is reusing binding#getKey(), which considers both generic signature and signature. if it works, that's ok.

[gayanper]

@testforstephen Do you think if we need to comparison of generic method signature ?

I think checking the method signature is enough. I see current implementation is reusing binding#getKey(), which considers both generic signature and signature. if it works, that's ok.

Then let’s improve this once we have the JDT API i tagged you in.

[testforstephen]

@gayanper It works well for me. Thanks for contribution.