'Open Call Hierarchy' does not jump to reference where it is invoked at #1824
Conversation
|
test this please |
| List<Range> ranges = toCallRanges(call.getMethodCall().getCallLocations()); | ||
| result.add(new CallHierarchyIncomingCall(symbol, ranges)); | ||
| Collection<CallLocation> callLocations = call.getMethodCall().getCallLocations(); | ||
| List<Range> ranges = toCallRanges(callLocations); |
There was a problem hiding this comment.
not fixed. ranges should only be computed if callLocations != null
There was a problem hiding this comment.
No. still not fixed. List<Range> ranges = toCallRanges(callLocations); needs to go inside the if block.
Collection<CallLocation> callLocations = call.getMethodCall().getCallLocations();
if (callLocations != null) {
List<Range> ranges = toCallRanges(callLocations);
for (CallLocation location : callLocations) {
IOpenable openable = getOpenable(location);
Range callRange = getRange(openable, location);
CallHierarchyItem symbol = toCallHierarchyItem(call.getMember());
symbol.setSelectionRange(callRange);
result.add(new CallHierarchyIncomingCall(symbol, ranges));
}
}
| @@ -181,6 +191,9 @@ private IMember getCallHierarchyElement(String uri, int line, int character, IPr | |||
| return null; | |||
| } | |||
|
|
|||
| if (!wrapper.canHaveChildren()) { | |||
There was a problem hiding this comment.
can be combined with previous if block
| Collection<CallLocation> callLocations = call.getMethodCall().getCallLocations(); | ||
| List<Range> ranges = toCallRanges(callLocations); | ||
| if (callLocations != null) { | ||
| for (CallLocation location : callLocations) { | ||
| IOpenable openable = location.getMember().getCompilationUnit(); | ||
| if (openable == null) { | ||
| openable = location.getMember().getTypeRoot(); | ||
| } | ||
| int[] start = JsonRpcHelpers.toLine(openable, location.getStart()); | ||
| int[] end = JsonRpcHelpers.toLine(openable, location.getEnd()); | ||
| Assert.isNotNull(start, "start"); | ||
| Assert.isNotNull(end, "end"); | ||
| // Assert.isLegal(start[0] == end[0], "Expected equal start and end lines. Start was: " + Arrays.toString(start) + " End was:" + Arrays.toString(end)); | ||
| CallHierarchyItem symbol = toCallHierarchyItem(call.getMember()); | ||
| result.add(new CallHierarchyOutgoingCall(symbol, ranges)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
avoid duplicate code, please refactor L208-L226 to a method call
|
@fbricon I have updated the PR. |
|
@fbricon I have updated the PR. |
|
One small issue I noticed. This seems to work fine for "Call Hierarchy", because it just relies on the selection range when jumping to an item. However, "Peek -> Call Hierarchy" relies on the first fromRange value and those are duplicated across calls that occur in the same method. Are we ok with this ? It seems like it should be possible to adjust this but might make the code a bit more complicated.
|
| @@ -221,13 +249,20 @@ private IMember getCallHierarchyElement(String uri, int line, int character, IPr | |||
|
|
|||
| List<CallHierarchyOutgoingCall> result = new ArrayList<>(); | |||
| for (MethodWrapper call : calls) { | |||
| Collection<CallLocation> callLocations = call.getMethodCall().getCallLocations(); | |||
| List<Range> ranges = toCallRanges(callLocations); | |||
There was a problem hiding this comment.
again, List<Range> ranges = toCallRanges(callLocations); needs to go inside the if block.
There was a problem hiding this comment.
Could you, please, check it again?
Yeah that sucks. If it's easy to fix in this PR, let's do this, else, we'll fix it in a subsequent change. |
snjeza
force-pushed
the
issue-2044
branch
2 times, most recently
from
6649d53
to
38a131c
Compare
| Iterator<CallLocation> iter = callLocations.iterator(); | ||
| while (iter.hasNext()) { | ||
| iter.next(); |
There was a problem hiding this comment.
how is that helping since you're not using the current object from the iterator? Might has well check if callLocations is empty in the if block L253.
Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
|
test this please |
There was a problem hiding this comment.
I'm fine with this. Feel free to merge when ready, and file a bug for #1824 (comment) to track it once this is merged..
|
Yes. This should be included in the next eclipse.jdt.ls (1.4.0) release. You could also try it out with http://download.eclipse.org/jdtls/snapshots/jdt-language-server-latest.tar.gz , once it gets built there. |
|
Thank you @rgrunber, that's exciting to hear. |
Fixes redhat-developer/vscode-java#2044
Signed-off-by: Snjezana Peco snjezana.peco@redhat.com