limit the recursive call

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
eclipse-jdtls · Dec 17, 2019 · e1b20ef · e1b20ef
1 parent 9abc161
commit e1b20ef
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 5 deletions.
diff --git a/...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CallHierarchyHandler.java b/...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CallHierarchyHandler.java
@@ -19,7 +19,9 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
 
 import org.eclipse.core.runtime.Assert;
@@ -55,10 +57,16 @@
 import org.eclipse.lsp4j.SymbolTag;
 
 public class CallHierarchyHandler {
+	private static Map<IJavaElement, MethodWrapper> incomingMethodWrapperCache = new ConcurrentHashMap<>();
+	private static Map<IJavaElement, MethodWrapper> outgoingMethodWrapperCache = new ConcurrentHashMap<>();
 
 	public List<CallHierarchyItem> prepareCallHierarchy(CallHierarchyPrepareParams params, IProgressMonitor monitor) {
 		Assert.isNotNull(params, "params");
 
+		// trigger call hierarchy at a new position, clean the method wrapper cache.
+		incomingMethodWrapperCache.clear();
+		outgoingMethodWrapperCache.clear();
+
 		String uri = params.getTextDocument().getUri();
 		int line = params.getPosition().getLine();
 		int character = params.getPosition().getCharacter();
@@ -163,8 +171,9 @@ private List<CallHierarchyIncomingCall> getIncomingCallItemsAt(String uri, int l
 
 		checkMonitor(monitor);
 
-		MethodWrapper wrapper = toMethodWrapper(candidate, true);
-		if (wrapper == null) {
+		MethodWrapper wrapper = incomingMethodWrapperCache.containsKey(candidate) ?
+			incomingMethodWrapperCache.get(candidate) : getCallRoot(candidate, true);
+		if (wrapper == null || wrapper.isRecursive()) {
 			return null;
 		}
 
@@ -175,6 +184,10 @@ private List<CallHierarchyIncomingCall> getIncomingCallItemsAt(String uri, int l
 
 		List<CallHierarchyIncomingCall> result = new ArrayList<>();
 		for (MethodWrapper call : calls) {
+			IMember member = call.getMember();
+			if (member != null) {
+				incomingMethodWrapperCache.put(member, call);
+			}
 			CallHierarchyItem symbol = toCallHierarchyItem(call.getMember());
 			List<Range> ranges = toCallRanges(call.getMethodCall().getCallLocations());
 			CallHierarchyIncomingCall incomingCall = new CallHierarchyIncomingCall();
@@ -194,8 +207,8 @@ private List<CallHierarchyOutgoingCall> getOutgoingCallItemsAt(String uri, int l
 
 		checkMonitor(monitor);
 
-		MethodWrapper wrapper = toMethodWrapper(candidate, false);
-		if (wrapper == null) {
+		MethodWrapper wrapper = outgoingMethodWrapperCache.containsKey(candidate) ? outgoingMethodWrapperCache.get(candidate) : getCallRoot(candidate, false);
+		if (wrapper == null || wrapper.isRecursive()) {
 			return null;
 		}
 
@@ -206,6 +219,10 @@ private List<CallHierarchyOutgoingCall> getOutgoingCallItemsAt(String uri, int l
 
 		List<CallHierarchyOutgoingCall> result = new ArrayList<>();
 		for (MethodWrapper call : calls) {
+			IMember member = call.getMember();
+			if (member != null) {
+				outgoingMethodWrapperCache.put(member, call);
+			}
 			CallHierarchyItem symbol = toCallHierarchyItem(call.getMember());
 			List<Range> ranges = toCallRanges(call.getMethodCall().getCallLocations());
 			CallHierarchyOutgoingCall outgoingCall = new CallHierarchyOutgoingCall();
@@ -224,7 +241,7 @@ private List<IJavaElement> codeResolve(IJavaElement input, int offset) throws Ja
 		return emptyList();
 	}
 
-	private MethodWrapper toMethodWrapper(IMember member, boolean isIncomingCall) {
+	private MethodWrapper getCallRoot(IMember member, boolean isIncomingCall) {
 		Assert.isNotNull(member, "member");
 
 		IMember[] members = { member };

diff --git a/org.eclipse.jdt.ls.tests/projects/eclipse/hello/src/org/sample/CallHierarchy.java b/org.eclipse.jdt.ls.tests/projects/eclipse/hello/src/org/sample/CallHierarchy.java
@@ -57,4 +57,11 @@ protected void method_2() {
 
   }
 
+  public void recursive1() {
+    recursive2();
+  }
+
+  public void recursive2() {
+    recursive1();
+  }
 }
diff --git a/....jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CallHierarchyHandlerTest.java b/....jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CallHierarchyHandlerTest.java
@@ -170,6 +170,54 @@ public void outgoing_jar() throws Exception {
 		assertTrue(jarUri.contains("WordUtils.class"));
 	}
 
+	@Test
+	public void outgoing_recursive() throws Exception {
+		// Line 60 from `CallHierarchy`
+		//  public void <|>recursive1() {
+		String uri = getUriFromSrcProject("org.sample.CallHierarchy");
+		List<CallHierarchyItem> items = prepareCallHierarchy(uri, 59, 14);
+		assertNotNull(items);
+		assertEquals(1, items.size());
+		assertItem(items.get(0), "recursive1()" + JavaElementLabels.DECL_STRING + "void", Method, "org.sample.CallHierarchy", false, 59);
+
+		List<CallHierarchyOutgoingCall> calls = getOutgoings(items.get(0));
+		assertNotNull(calls);
+		assertEquals(1, calls.size());
+		assertItem(calls.get(0).getTo(), "recursive2()" + JavaElementLabels.DECL_STRING + "void", Method, "org.sample.CallHierarchy", false, 63);
+
+		List<CallHierarchyOutgoingCall> call0Calls = getOutgoings(calls.get(0).getTo());
+		assertNotNull(call0Calls);
+		assertEquals(1, call0Calls.size());
+		assertItem(call0Calls.get(0).getTo(), "recursive1()" + JavaElementLabels.DECL_STRING + "void", Method, "org.sample.CallHierarchy", false, 59);
+
+		List<CallHierarchyOutgoingCall> call1Calls = getOutgoings(call0Calls.get(0).getTo());
+		assertNull(call1Calls);
+	}
+
+	@Test
+	public void incoming_recursive() throws Exception {
+		// Line 60 from `CallHierarchy`
+		//  public void <|>recursive1() {
+		String uri = getUriFromSrcProject("org.sample.CallHierarchy");
+		List<CallHierarchyItem> items = prepareCallHierarchy(uri, 59, 14);
+		assertNotNull(items);
+		assertEquals(1, items.size());
+		assertItem(items.get(0), "recursive1()" + JavaElementLabels.DECL_STRING + "void", Method, "org.sample.CallHierarchy", false, 59);
+
+		List<CallHierarchyIncomingCall> calls = getIncomingCalls(items.get(0));
+		assertNotNull(calls);
+		assertEquals(1, calls.size());
+		assertItem(calls.get(0).getFrom(), "recursive2()" + JavaElementLabels.DECL_STRING + "void", Method, "org.sample.CallHierarchy", false, 63);
+
+		List<CallHierarchyIncomingCall> call0Calls = getIncomingCalls(calls.get(0).getFrom());
+		assertNotNull(call0Calls);
+		assertEquals(1, call0Calls.size());
+		assertItem(call0Calls.get(0).getFrom(), "recursive1()" + JavaElementLabels.DECL_STRING + "void", Method, "org.sample.CallHierarchy", false, 59);
+
+		List<CallHierarchyIncomingCall> call1Calls = getIncomingCalls(call0Calls.get(0).getFrom());
+		assertNull(call1Calls);
+	}
+
 	/**
 	 * @param item
 	 *            to assert