Null check handling for Stack trace ambiguity #543

[SougandhS]

What it does

Fix for #543

How to test

Author checklist

• I have thoroughly tested my changes
• The change is following the coding conventions
• I have signed the Eclipse Contributor Agreement (ECA)

[iloveeclipse]

1. Commit message is wrong and doesn't explain the fix, see https://github.com/eclipse-jdt/.github/blob/main/CONTRIBUTING.md#commit-message-recommendations

2. With this fix, instead of an error I get this dialog:
   
   and of course this makes no sense, I don't want to create any brekpoints!
   This was before using method name to find unqualified Stacktraces in Java Stack Trace #498:
   

3. Clocking on ForkJoinTask in at java.base/java.util.concurrent.ForkJoinTask.reportExecutionException(ForkJoinTask.java:605) shows empty and meaningless error about opening something from clipboard (???):

while it was type selection dialog before :

[iloveeclipse]

Original code is unreadable, this makes it even more unreadable.

Ideally I would revert entire #498 PR as it adds non maintainable code, I simply can't read this huge nested method.

Also while playing with the PR I've noticed it makes use of OpenFromClipboardAction code for the cases where it obviously generated absolutely wrong class names like bin.org.eclipse.core.runtime.internal.adaptor - note the bin at beginning.

Java Model Exception: Error in Java Model (code 969): bin.org.eclipse.core.runtime.internal.adaptor [in /data/git/equinox/bundles/org.eclipse.osgi/bin] does not exist
	at org.eclipse.jdt.internal.core.JavaElement.newJavaModelException(JavaElement.java:556)
	at org.eclipse.jdt.internal.core.Openable.generateInfos(Openable.java:234)
	at org.eclipse.jdt.internal.core.Openable.openAncestors(Openable.java:508)
	at org.eclipse.jdt.internal.core.Openable.generateInfos(Openable.java:228)
	at org.eclipse.jdt.internal.core.JavaElement.openWhenClosed(JavaElement.java:569)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:292)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:278)
	at org.eclipse.jdt.internal.core.BinaryType.getClassFileInfo(BinaryType.java:232)
	at org.eclipse.jdt.internal.core.BinaryType.getChildren(BinaryType.java:195)
	at org.eclipse.jdt.internal.core.JavaElement.getChildrenOfType(JavaElement.java:247)
	at org.eclipse.jdt.internal.core.BinaryType.getMethods(BinaryType.java:494)
	at org.eclipse.jdt.internal.debug.ui.console.JavaStackTraceHyperlink$3.runInUIThread(JavaStackTraceHyperlink.java:247)

1. Please amend your commit and update commit message to match recommended style.

[SougandhS]

Sure @iloveeclipse 👍

[iloveeclipse]

Looking on the number of issues I've found above, I will now revert #498 PR as

1. the code has multiple regressions
2. The code in org.eclipse.jdt.internal.debug.ui.console.JavaStackTraceHyperlink.searchCompleted(Object, String, int, IStatus) is simply not maintainable.

I would recommend to rewrite #498.

[SougandhS]

Looking on the number of issues I've found above, I will now revert #498 PR as

1. the code has multiple regressions
2. The code in org.eclipse.jdt.internal.debug.ui.console.JavaStackTraceHyperlink.searchCompleted(Object, String, int, IStatus) is simply not maintainable.

I would recommend to rewrite #498.

Will re-do this like you suggested.

[SougandhS]

1. Commit message is wrong and doesn't explain the fix, see https://github.com/eclipse-jdt/.github/blob/main/CONTRIBUTING.md#commit-message-recommendations
2. With this fix, instead of an error I get this dialog:
   
   and of course this makes no sense, I don't want to create any brekpoints!
   This was before using method name to find unqualified Stacktraces in Java Stack Trace #498:
   
3. Clocking on ForkJoinTask in at java.base/java.util.concurrent.ForkJoinTask.reportExecutionException(ForkJoinTask.java:605) shows empty and meaningless error about opening something from clipboard (???):

while it was type selection dialog before :

Hi @iloveeclipse

Regarding 2.

It seems to be the default behaviour -> In master I get the same behaviour now too. If the generated source is not list and have only 1 binaryType and is of exception type then this implementation is called ->

org.eclipse.jdt.internal.debug.ui.console.JavaExceptionHyperLink.processSearchResult() instead of org.eclipse.jdt.internal.debug.ui.console.JavaStackTraceHyperlink.processSearchResult()

I reproduced the same now.
If I have only 1 BinaryType then control goes here

then ->

But If I have multiple BinaryType then openType is triggered

Shall I raise this as a new issue ?

[iloveeclipse]

Shall I raise this as a new issue ?

I don't have IDE right now, but any bug you see in a plain SDK without any changes is a bug and should be reported. If you see "add breakpoint" dalog for clicking on "regular" stack trace line, instead of navigation to the right code, it is a bug.

[SougandhS]

Shall I raise this as a new issue ?

I don't have IDE right now, but any bug you see in a plain SDK without any changes is a bug and should be reported. If you see "add breakpoint" dalog for clicking on "regular" stack trace line, instead of navigation to the right code, it is a bug.

on regular stacktrace it is not going to any breakpoint creation.. its only for Exception traces . I see its a feature, but like you reported I dont feel its a good approach. can we provide some right click option for creating breakpoint option rather than directly showing this ?