[WIP] Return the uri of the parent class when the class file points to an inner class #3152
Conversation
…ner class - When the class file name containing '$', we try to get its top-level class (remove the '$'' and its inner class name) from the Java model, if the top-level class do exist, we compare the content of the two classes and return the uri of the top-level class if the content is the same.
|
@rgrunber I think this is a worth fixing issue, though I'm not sure the implementation is acceptable or too tricky. WDYT? |
|
One downside I can see is that when the uri is separated for the nested class (without this PR), we can use the uri to have documentSymbols showing only the symbols associated with the nested class. With this change, there's not really a way to do that, particularly because documentSymbols does not take a cursor offset. Just something to be aware of. |
|
I notices another downside, that |
| if (parentClass != null && parentClass instanceof IClassFile parentClassFile) { | ||
| try { | ||
| // they are from the same source file if the contents are the same. | ||
| if (Objects.equals(parentClassFile.getBuffer(), classFile.getBuffer())) { |
There was a problem hiding this comment.
Regarding your comments on getBuffer(), is there ever a case where a nested class would not belong to the same classfile as the part of the element name to the left of the $ ?
There was a problem hiding this comment.
Because $ is a valid character for java class name. Given a class named as Foo$Bar, we cannot simply tell whether it's class Bar nested in Foo, or just a class named as Foo$Bar.
Though this should be a rare case.
There was a problem hiding this comment.
I guess the getBuffer would only ever be called when the $ is present in the name to begin with, so at least it isn't slowing down got-to-definition for all requests.
Does exists() help ?
There was a problem hiding this comment.
I guess the getBuffer would only ever be called when the $ is present in the name to begin with, so at least it isn't slowing down got-to-definition for all requests.
Yes, that's correct.
Does exists() help ?
I think the answer is no. My understanding is that, as long as the object returns from JavaCore.create is not null, that means it's in the Java model, then exists() will always return true.
Considering the original request is not 'hot', maybe hang on this change a little bit? At least there is a solution (though not perfect).
fix microsoft/vscode-java-dependency#833
Before, navigate to a inner class will open a new tab:
before.mp4
After, navigate to a inner class will go to the location in the current file:
after.mp4