Bug 578145: Evaluations for lambda variables are not working in nested lambda

When the focus frame is a lambda frame, the evaluation
engine will explicitly extract the visible variables from
the target frame.

Currently, it simply looks for the target frame and the
following two frames below and adds their variables to the
visible set. This doesn't work for cases like chained calls
and nested lambda.

An improved approach is to look for more frames in the
same debug file which encloses the target frame, and
append the variables of those enclosing frames to the
visible set as well. This can deal with nested lambda better.

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
Change-Id: I57ab4fb2d0281951a693d05c8800f29d4584393f
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/191713
Tested-by: JDT Bot <jdt-bot@eclipse.org>
Reviewed-by: Gayan Perera <gayanper@gmail.com>
diff --git a/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInAnonymous.java b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInAnonymous.java
new file mode 100644
index 0000000..e7ad11b
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInAnonymous.java
@@ -0,0 +1,21 @@
+import java.util.Arrays;
+import java.util.List;
+
+public class Bug578145LambdaInAnonymous {
+
+	public static void main(String[] args) {
+		int numberInMain = 1;
+
+		new Runnable() {
+			@Override
+			public void run() {
+				List<String> usersInAnonymous = Arrays.asList("Lambda");
+
+				usersInAnonymous.stream().forEach(u -> {
+					int numberInLambda = 10;
+					System.out.println("user name: " + u); // Add a breakpoint here
+				});
+			}
+		}.run();
+	}
+}
diff --git a/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInConstructor.java b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInConstructor.java
new file mode 100644
index 0000000..4d17488
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInConstructor.java
@@ -0,0 +1,20 @@
+import java.util.Arrays;
+import java.util.List;
+
+public class Bug578145LambdaInConstructor {
+	List<String> names;
+
+	public Bug578145LambdaInConstructor(List<String> originalNames) {
+		this.names = originalNames;
+		int localInConstructor = 1;
+
+		this.names.stream().forEach((name) -> {
+			int localInLambda = 10;
+			System.out.println(name); // Add breakpoint here
+		});
+	}
+
+	public static void main(String[] args) {
+		Bug578145LambdaInConstructor instance = new Bug578145LambdaInConstructor(Arrays.asList("Lambda"));
+	}
+}
diff --git a/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInFieldDeclaration.java b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInFieldDeclaration.java
new file mode 100644
index 0000000..313c088
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInFieldDeclaration.java
@@ -0,0 +1,21 @@
+import java.util.function.Consumer;
+
+public class Bug578145LambdaInFieldDeclaration {
+	Runnable runnable = new Runnable() {
+		@Override
+		public void run() {
+			int numberInRunnable = 1;
+
+			Consumer<Integer> myConsumer = (lambdaArg) -> {
+				int numberInLambda = 10;
+				System.out.println("id = " + lambdaArg); // Add breakpoint here
+			};
+			myConsumer.accept(numberInRunnable);
+		}
+	};
+
+	public static void main(String[] args) {
+		Bug578145LambdaInFieldDeclaration instance = new Bug578145LambdaInFieldDeclaration();
+		instance.runnable.run();
+	}
+}
diff --git a/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInStaticInitializer.java b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInStaticInitializer.java
new file mode 100644
index 0000000..1ecdafc
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaInStaticInitializer.java
@@ -0,0 +1,19 @@
+import java.util.Arrays;
+import java.util.List;
+
+public class Bug578145LambdaInStaticInitializer {
+
+	static {
+		int numberInStaticInitializer = 1;
+		List<String> staticList = Arrays.asList("Lambda");
+
+		staticList.stream().forEach((name) -> {
+			int numberInLambda = 10;
+			System.out.println(name); // Add breakpoint here
+		});
+	}
+
+	public static void main(String[] args) {
+		Bug578145LambdaInStaticInitializer instance = new Bug578145LambdaInStaticInitializer();
+	}
+}
diff --git a/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaOnChainCalls.java b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaOnChainCalls.java
new file mode 100644
index 0000000..934f555
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/java8/Bug578145LambdaOnChainCalls.java
@@ -0,0 +1,14 @@
+import java.util.Arrays;
+import java.util.List;
+
+public class Bug578145LambdaOnChainCalls {
+	public static void main(String[] args) {
+		int numberInMain = 1;
+		List<String> users = Arrays.asList("Lambda");
+
+		users.stream().forEach(u -> {
+			int numberInLambda = 10;
+			System.out.println("user name: " + u); // Add a breakpoint here
+		});
+	}
+}
diff --git a/org.eclipse.jdt.debug.tests/java8/Bug578145NestedLambda.java b/org.eclipse.jdt.debug.tests/java8/Bug578145NestedLambda.java
new file mode 100644
index 0000000..9773a81
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/java8/Bug578145NestedLambda.java
@@ -0,0 +1,21 @@
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.Consumer;
+
+public class Bug578145NestedLambda {
+
+	public static void main(String[] args) {
+		int numberInMain = 1;
+
+		Consumer<Integer> myConsumer = (id) -> {
+			int numberInExternalLambda = 10;
+
+			List<String> users = Arrays.asList("Lambda");
+			users.stream().forEach(u -> {
+				int numberInInnerLambda = 100;
+				System.out.println("user name: " + u); // Add a breakpoint here
+			});
+		};
+		myConsumer.accept(numberInMain);
+	}
+}
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
index 599ed29..cbdb61b 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/AbstractDebugTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2021 IBM Corporation and others.
+ * Copyright (c) 2000, 2022 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -501,6 +501,12 @@
 				cfgs.add(createLaunchConfiguration(jp, "Bug571310"));
 				cfgs.add(createLaunchConfiguration(jp, "Bug573547"));
 				cfgs.add(createLaunchConfiguration(jp, "Bug575551"));
+				cfgs.add(createLaunchConfiguration(jp, "Bug578145NestedLambda"));
+				cfgs.add(createLaunchConfiguration(jp, "Bug578145LambdaInConstructor"));
+				cfgs.add(createLaunchConfiguration(jp, "Bug578145LambdaInFieldDeclaration"));
+				cfgs.add(createLaunchConfiguration(jp, "Bug578145LambdaInStaticInitializer"));
+				cfgs.add(createLaunchConfiguration(jp, "Bug578145LambdaInAnonymous"));
+				cfgs.add(createLaunchConfiguration(jp, "Bug578145LambdaOnChainCalls"));
 	    		loaded18 = true;
 	    		waitForBuild();
 	        }
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/eval/LambdaVariableTest.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/eval/LambdaVariableTest.java
index 39c8b3c..8e8fde2 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/eval/LambdaVariableTest.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/eval/LambdaVariableTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2020 Gayan Perera and others.
+ * Copyright (c) 2020, 2022 Gayan Perera and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -200,6 +200,68 @@
 		assertEquals("wrong result at 1st lambda stack : ", "name", value.getValueString());
 	}
 
+	public void testEvaluate_Bug578145_NestedLambda() throws Exception {
+		debugWithBreakpoint("Bug578145NestedLambda", 16);
+
+		IValue value = doEval(javaThread, "numberInInnerLambda");
+		assertEquals("wrong result : ", "100", value.getValueString());
+
+		value = doEval(javaThread, "numberInExternalLambda");
+		assertEquals("wrong result : ", "10", value.getValueString());
+
+		value = doEval(javaThread, "numberInMain");
+		assertEquals("wrong result : ", "1", value.getValueString());
+	}
+
+	public void testEvaluate_Bug578145_LambdaInConstructor() throws Exception {
+		debugWithBreakpoint("Bug578145LambdaInConstructor", 13);
+		IValue value = doEval(javaThread, "localInLambda");
+		assertEquals("wrong result : ", "10", value.getValueString());
+
+		value = doEval(javaThread, "localInConstructor");
+		assertEquals("wrong result : ", "1", value.getValueString());
+	}
+
+	public void testEvaluate_Bug578145_LambdaInFieldDeclaration() throws Exception {
+		debugWithBreakpoint("Bug578145LambdaInFieldDeclaration", 11);
+
+		IValue value = doEval(javaThread, "numberInLambda");
+		assertEquals("wrong result : ", "10", value.getValueString());
+
+		value = doEval(javaThread, "numberInRunnable");
+		assertEquals("wrong result : ", "1", value.getValueString());
+	}
+
+	public void testEvaluate_Bug578145_LambdaInStaticInitializer() throws Exception {
+		debugWithBreakpoint("Bug578145LambdaInStaticInitializer", 12);
+
+		IValue value = doEval(javaThread, "numberInLambda");
+		assertEquals("wrong result : ", "10", value.getValueString());
+
+		value = doEval(javaThread, "numberInStaticInitializer");
+		assertEquals("wrong result : ", "1", value.getValueString());
+	}
+
+	public void testEvaluate_Bug578145_LambdaInAnonymous() throws Exception {
+		debugWithBreakpoint("Bug578145LambdaInAnonymous", 16);
+
+		IValue value = doEval(javaThread, "numberInLambda");
+		assertEquals("wrong result : ", "10", value.getValueString());
+
+		value = doEval(javaThread, "numberInMain");
+		assertEquals("wrong result : ", "1", value.getValueString());
+	}
+
+	public void testEvaluate_Bug578145_LambdaOnChainCalls() throws Exception {
+		debugWithBreakpoint("Bug578145LambdaOnChainCalls", 11);
+
+		IValue value = doEval(javaThread, "numberInLambda");
+		assertEquals("wrong result : ", "10", value.getValueString());
+
+		value = doEval(javaThread, "numberInMain");
+		assertEquals("wrong result : ", "1", value.getValueString());
+	}
+
 	private void debugWithBreakpoint(String testClass, int lineNumber) throws Exception {
 		createLineBreakpoint(lineNumber, testClass);
 		javaThread = launchToBreakpoint(testClass);
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/LambdaUtils.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/LambdaUtils.java
index aee1c97..b85aa51 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/LambdaUtils.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/LambdaUtils.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2018, 2019 IBM Corporation and others.
+ * Copyright (c) 2018, 2022 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -17,6 +17,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -30,6 +31,8 @@
 import org.eclipse.jdt.internal.debug.core.logicalstructures.JDILambdaVariable;
 import org.eclipse.jdt.internal.debug.eval.ast.engine.IRuntimeContext;
 
+import com.sun.jdi.AbsentInformationException;
+import com.sun.jdi.Location;
 import com.sun.jdi.Method;
 
 /**
@@ -41,11 +44,7 @@
 
 	/**
 	 * Inspects the top stack frame of the context; if that frame is a lambda frame, looks for a variable with the specified name in that frame and
-	 * two frames below that frame.
-	 *
-	 * Inside a lambda expression, variable names are mangled by the compiler. Its therefore necessary to check the outer frame when at a lambda
-	 * frame, in order to find a variable with its name. The lambda expression itself is called by a synthetic static method, which is the first frame
-	 * below the lambda frame. So in total we check 3 stack frames for the variable with the specified name.
+	 * outer frames visible from that frame.
 	 *
 	 * @param context
 	 *            The context in which to check.
@@ -70,12 +69,11 @@
 	}
 
 	/**
-	 * Collects variables visible from a lambda stack frame. I.e. inspects the specified stack frame; if that frame is a lambda frame, collects all
-	 * variables in that frame and two frames below that frame.
+	 * Collects variables visible from a lambda stack frame.
 	 *
-	 * Inside a lambda expression, variable names are mangled by the compiler. Its therefore necessary to check the outer frame when at a lambda
-	 * frame, in order to find a variable with its name. The lambda expression itself is called by a synthetic static method, which is the first frame
-	 * below the lambda frame. So in total we collect variables from 3 stack frames.
+	 * If the debugging class generates all debugging info in its classfile (e.g. line number and source file name), we can use these info to find all
+	 * enclosing frames of the paused line number and collect their variables. Otherwise, collect variables from that lambda frame and two more frames
+	 * below it.
 	 *
 	 * @param frame
 	 *            The lambda frame at which to check.
@@ -85,19 +83,90 @@
 	 *             If accessing the top stack frame or the local variables on stack frames fails, due to failure to communicate with the debug target.
 	 */
 	public static List<IVariable> getLambdaFrameVariables(IStackFrame frame) throws DebugException {
-		List<IVariable> variables = new ArrayList<>();
 		if (LambdaUtils.isLambdaFrame(frame)) {
-			IThread thread = frame.getThread();
-			// look for two frames below the frame which is provided instead starting from first frame.
-			List<IStackFrame> stackFrames = Stream.of(thread.getStackFrames()).dropWhile(f -> f != frame)
-					.limit(3).collect(Collectors.toUnmodifiableList());
-			for (IStackFrame stackFrame : stackFrames) {
-				IVariable[] stackFrameVariables = stackFrame.getVariables();
-				variables.addAll(Arrays.asList(stackFrameVariables));
-				for (IVariable frameVariable : stackFrameVariables) {
-					if (isLambdaObjectVariable(frameVariable)) {
-						variables.addAll(extractVariablesFromLambda(frameVariable));
-					}
+			int lineNumber = frame.getLineNumber();
+			String sourceName = ((IJavaStackFrame) frame).getSourceName();
+			if (lineNumber == -1 || sourceName == null) {
+				return collectVariablesFromLambdaFrame(frame);
+			}
+			return collectVariablesFromEnclosingFrames(frame);
+		}
+		return Collections.emptyList();
+	}
+
+	/**
+	 * Collects variables visible from a lambda stack frame and two frames below that frame.
+	 *
+	 * Inside a lambda expression, variable names are mangled by the compiler. Its therefore necessary to check the outer frame when at a lambda
+	 * frame, in order to find a variable with its name. The lambda expression itself is called by a synthetic static method, which is the first frame
+	 * below the lambda frame. So in total we collect variables from 3 stack frames.
+	 *
+	 * @param frame
+	 *            The lambda frame at which to check.
+	 * @return The variables visible from the stack frame. The variables are ordered top-down, i.e. if shadowing occurs, the more local variable will
+	 *         be first in the resulting list.
+	 * @throws DebugException
+	 *             If accessing the top stack frame or the local variables on stack frames fails, due to failure to communicate with the debug target.
+	 */
+	private static List<IVariable> collectVariablesFromLambdaFrame(IStackFrame frame) throws DebugException {
+		List<IVariable> variables = new ArrayList<>();
+		IThread thread = frame.getThread();
+		// look for two frames below the frame which is provided instead starting from first frame.
+		List<IStackFrame> stackFrames = Stream.of(thread.getStackFrames()).dropWhile(f -> f != frame)
+				.limit(3).collect(Collectors.toUnmodifiableList());
+		for (IStackFrame stackFrame : stackFrames) {
+			IVariable[] stackFrameVariables = stackFrame.getVariables();
+			variables.addAll(Arrays.asList(stackFrameVariables));
+			for (IVariable frameVariable : stackFrameVariables) {
+				if (isLambdaObjectVariable(frameVariable)) {
+					variables.addAll(extractVariablesFromLambda(frameVariable));
+				}
+			}
+		}
+		return Collections.unmodifiableList(variables);
+	}
+
+	/**
+	 * Collect variables from all enclosing frames starting from the provided frame.
+	 */
+	private static List<IVariable> collectVariablesFromEnclosingFrames(IStackFrame frame) throws DebugException {
+		List<IVariable> variables = new ArrayList<>();
+		IThread thread = frame.getThread();
+		List<IStackFrame> stackFrames = Stream.of(thread.getStackFrames()).dropWhile(f -> f != frame)
+				.collect(Collectors.toUnmodifiableList());
+		int pausedLineNumber = frame.getLineNumber();
+		String pausedSourceName = ((IJavaStackFrame) frame).getSourceName();
+		String pausedSourcePath = ((IJavaStackFrame) frame).getSourcePath();
+		boolean isFocusFrame = true;
+		for (IStackFrame stackFrame : stackFrames) {
+			JDIStackFrame jdiFrame = (JDIStackFrame) stackFrame;
+			if (isFocusFrame) {
+				isFocusFrame = false;
+			} else {
+				if (!Objects.equals(pausedSourceName, jdiFrame.getSourceName())
+						|| !Objects.equals(pausedSourcePath, jdiFrame.getSourcePath())) {
+					continue;
+				}
+				List<Location> locations;
+				try {
+					locations = jdiFrame.getUnderlyingMethod().allLineLocations();
+				} catch (AbsentInformationException e) {
+					continue;
+				}
+				if (locations.isEmpty()) {
+					continue;
+				}
+				int methodStartLine = locations.get(0).lineNumber();
+				int methodEndLine = locations.get(locations.size() - 1).lineNumber();
+				if (methodStartLine > pausedLineNumber || methodEndLine < pausedLineNumber) {
+					continue;
+				}
+			}
+			IVariable[] stackFrameVariables = jdiFrame.getVariables();
+			variables.addAll(Arrays.asList(stackFrameVariables));
+			for (IVariable frameVariable : stackFrameVariables) {
+				if (isLambdaObjectVariable(frameVariable)) {
+					variables.addAll(extractVariablesFromLambda(frameVariable));
 				}
 			}
 		}