Review comments addressed

Author: iloveeclipse

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2006, 2025 IBM Corporation and others.
+ * Copyright (c) 2006, 2021 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java

@@ -81,6 +81,7 @@
 import org.eclipse.jdt.debug.core.IJavaType;
 import org.eclipse.jdt.debug.core.IJavaWatchpoint;
 import org.eclipse.jdt.debug.core.JDIDebugModel;
+import org.eclipse.jdt.internal.core.LambdaMethod;
 import org.eclipse.jdt.internal.corext.template.java.CompilationUnitContext;
 import org.eclipse.jdt.internal.corext.template.java.CompilationUnitContextType;
 import org.eclipse.jdt.internal.corext.template.java.JavaContextType;
@@ -286,7 +287,7 @@ protected IStatus run(IProgressMonitor monitor) {
 		job.schedule();
 	}
 
-	public void toggleLambdaEntryMethodBreakpoints(final IWorkbenchPart part, final ISelection finalSelection, final String lambdaMethodName, final String lambdaMethodSignature, final LambdaBreakpoint lambdaProperties) {
+	static void toggleLambdaEntryMethodBreakpoints(final IWorkbenchPart part, final ISelection finalSelection, final String lambdaMethodName, final String lambdaMethodSignature, final LambdaProperties lambdaProperties) {
 		Job job = new Job("Toggle Lambda Entry Method Breakpoints") { //$NON-NLS-1$
 			@Override
 			protected IStatus run(IProgressMonitor monitor) {
@@ -307,7 +308,7 @@ protected IStatus run(IProgressMonitor monitor) {
 		job.schedule();
 	}
 
-	static IStatus doToggleLambdaEntryMethodBreakpoints(IWorkbenchPart part, ISelection selection, String lambdaMethodName, String lambdaMethodSignature, LambdaBreakpoint lambdaProperties, IProgressMonitor monitor) throws CoreException {
+	static IStatus doToggleLambdaEntryMethodBreakpoints(IWorkbenchPart part, ISelection selection, String lambdaMethodName, String lambdaMethodSignature, LambdaProperties lambdaProperties, IProgressMonitor monitor) throws CoreException {
 		ITextEditor textEditor = getTextEditor(part);
 		if (textEditor == null || !(selection instanceof ITextSelection)) {
 			return Status.OK_STATUS;
@@ -412,13 +413,13 @@ private static void doToggleMethodBreakpoint(IMethod member, IWorkbenchPart part
 		doToggleMethodBreakpoint(member, null, null, null, part, finalSelection, monitor);
 	}
 
-	private static void doToggleMethodBreakpoint(IMethod member, String lambdaMethodName, String lambdaMethodSignature, LambdaBreakpoint lambdaProperties, IWorkbenchPart part, ISelection finalSelection, IProgressMonitor monitor) throws CoreException {
+	private static void doToggleMethodBreakpoint(IMethod member, String lambdaMethodName, String lambdaMethodSignature, LambdaProperties lambdaProperties, IWorkbenchPart part, ISelection finalSelection, IProgressMonitor monitor) throws CoreException {
 		int lambdaPosition = 0;
 		if (lambdaProperties != null) {
 			lambdaPosition = lambdaProperties.lambdaPosition();
 		}
 
-		IJavaBreakpoint breakpoint = getMethodBreakpoint(member, lambdaMethodName, lambdaMethodSignature, lambdaPosition);
+		IJavaBreakpoint breakpoint = getMethodBreakpoint(member, lambdaPosition);
 		if (breakpoint != null) {
 			if (BreakpointToggleUtils.isToggleTracepoint()) {
 				deleteTracepoint(breakpoint, part, monitor);
@@ -470,14 +471,11 @@ private static void doToggleMethodBreakpoint(IMethod member, String lambdaMethod
 			}
 			BreakpointToggleUtils.setUnsetTracepoint(false);
 		}
-		if (lambdaMethodName != null || lambdaMethodSignature != null) {
+		if (lambdaProperties != null) {
 			methodBreakpoint.setLambdaBreakpoint(true);
 			methodBreakpoint.setInlineLambdaPosition(lambdaPosition);
-			String lambdaName = lambdaProperties.getLambdaName();
 			if (lambdaProperties.isInline()) {
-				methodBreakpoint.setLambdaName(lambdaName);
-			} else {
-				methodBreakpoint.setLambdaName(null);
+				methodBreakpoint.setLambdaName(lambdaProperties.getLambdaName());
 			}
 		}
 		if (ValidBreakpointLocationLocator.LOCATION_METHOD_CLOSE) {
@@ -1372,60 +1370,51 @@ protected static IMethod getMethodHandle(IEditorPart editorPart, String typeName
 	 *
 	 * @param element
 	 *            the element to get the breakpoint from
-	 * @param lambdaMethodSignature
-	 * @param lambdaMethodName
 	 * @param lambdaPosition
+	 *            the position of the lambda expression in the line (if any)
 	 * @return the current breakpoint from the element or <code>null</code>
 	 */
 	@SuppressWarnings("restriction")
-	protected static IJavaBreakpoint getMethodBreakpoint(IMember element, String lambdaMethodName, String lambdaMethodSignature, int lambdaPosition) {
+	protected static IJavaBreakpoint getMethodBreakpoint(IMember element, int lambdaPosition) {
+		if (!(element instanceof IMethod method)) {
+			return null;
+		}
         IBreakpointManager breakpointManager = DebugPlugin.getDefault().getBreakpointManager();
         IBreakpoint[] breakpoints = breakpointManager.getBreakpoints(JDIDebugModel.getPluginIdentifier());
-		if (!(element instanceof IMethod)) {
-			return null;
-        }
-		IMethod method = (IMethod) element;
 		for (IBreakpoint breakpoint : breakpoints) {
-			if (!(breakpoint instanceof IJavaMethodBreakpoint)) {
+			if (!(breakpoint instanceof IJavaMethodBreakpoint methodBreakpoint)) {
 				continue;
 			}
-			IJavaMethodBreakpoint methodBreakpoint = (IJavaMethodBreakpoint) breakpoint;
-			IMember container = null;
+			IMember container;
 			try {
 				container = BreakpointUtils.getMember(methodBreakpoint);
-			} catch (CoreException e) {
-				JDIDebugUIPlugin.log(e);
-				return null;
-			}
-			if (container == null) {
-				try {
+				if (container == null) {
 					if (method.getDeclaringType().getFullyQualifiedName().equals(methodBreakpoint.getTypeName())
 							&& method.getElementName().equals(methodBreakpoint.getMethodName())
 							&& methodBreakpoint.getMethodSignature().equals(resolveMethodSignature(method))) {
 						return methodBreakpoint;
 					}
-				} catch (CoreException e) {
-					JDIDebugUIPlugin.log(e);
+					continue;
 				}
-			} else {
-				if (container instanceof IMethod) {
-					if (method instanceof org.eclipse.jdt.internal.core.LambdaMethod) {
-						try {
-							if (methodBreakpoint.getInlineLambdaPosition() == lambdaPosition
-									&& methodBreakpoint.getCharStart() == element.getSourceRange().getOffset()) {
-								return methodBreakpoint;
-							}
-						} catch (CoreException e) {
-							JDIDebugUIPlugin.log(e);
-							return null;
-						}
-
-					} else if (method.getDeclaringType().getFullyQualifiedName().equals(container.getDeclaringType().getFullyQualifiedName())) {
-						if (method.isSimilar((IMethod) container)) {
-							return methodBreakpoint;
-						}
+				if (!(container instanceof IMethod)) {
+					continue;
+				}
+				if (method instanceof LambdaMethod) {
+					ISourceRange sourceRange = element.getSourceRange();
+					if (sourceRange != null && methodBreakpoint.getInlineLambdaPosition() == lambdaPosition
+							&& methodBreakpoint.getCharStart() == sourceRange.getOffset()) {
+						return methodBreakpoint;
 					}
+					continue;
 				}
+				if (method.getDeclaringType().getFullyQualifiedName().equals(container.getDeclaringType().getFullyQualifiedName())) {
+					if (method.isSimilar((IMethod) container)) {
+						return methodBreakpoint;
+					}
+				}
+			} catch (CoreException e) {
+				JDIDebugUIPlugin.log(e);
+				continue;
 			}
 		}
         return null;
@@ -1697,53 +1686,64 @@ private void toggleFieldOrMethodBreakpoints(IWorkbenchPart part, ISelection sele
 			toggleWatchpoints(part, ts);
 		} else if (loc.getLocationType() == ValidBreakpointLocationLocator.LOCATION_LINE) {
 			if (lambdaEntryBP) {
-				IEditorInput editorInput = editor.getEditorInput();
-				IDocumentProvider documentProvider = editor.getDocumentProvider();
-				if (documentProvider == null) {
-					BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
-					throw new CoreException(Status.CANCEL_STATUS);
-				}
-				IDocument document = documentProvider.getDocument(editorInput);
-				try {
-					IRegion region = document.getLineInformation(ts.getStartLine());
-					List<LambdaExpression> lambdaExpresions = findLambdaExpressions(editor, region);
-					if (lambdaExpresions.isEmpty()) {
-						return;
-					}
-					int lambdaPosition;
-					if (lambdaExpresions.size() == 1) {
-						lambdaPosition = 0;
-					} else {
-						lambdaPosition = selectLambda(lambdaExpresions);
-						if (lambdaPosition == -1) {
-							return;
-						}
-					}
-					LambdaExpression selectedLambda = lambdaExpresions.get(lambdaPosition);
-					IMethodBinding methodBinding = selectedLambda.resolveMethodBinding();
-					if (methodBinding == null) {
-						BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
-						return;
-					}
-					String lambdaMethodName = LambdaLocationLocatorHelper.toMethodName(methodBinding);
-					String lambdaMethodSignature = LambdaLocationLocatorHelper.toMethodSignature(methodBinding);
-					ITextSelection textSelection = new TextSelection(document, selectedLambda.getStartPosition(), selectedLambda.getLength());
-					boolean inline = lambdaExpresions.size() > 1 ? true : false;
-					LambdaBreakpoint lambdaBp = new LambdaBreakpoint(lambdaExpresions.get(lambdaPosition).toString(), lambdaPosition, inline);
-					toggleLambdaEntryMethodBreakpoints(part, textSelection, lambdaMethodName, lambdaMethodSignature, lambdaBp);
-				} catch (BadLocationException e) {
-					BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
-				}
-
+				toggleLambdaBreakpoint(part, ts, editor);
 			} else {
 				toggleLineBreakpoints(part, ts, false, loc);
 			}
 		}
 	}
 
+	private static void toggleLambdaBreakpoint(IWorkbenchPart part, ITextSelection ts, ITextEditor editor) throws CoreException {
+		IDocumentProvider documentProvider = editor.getDocumentProvider();
+		if (documentProvider == null) {
+			BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
+			throw new CoreException(Status.CANCEL_STATUS);
+		}
+		IEditorInput editorInput = editor.getEditorInput();
+		IDocument document = documentProvider.getDocument(editorInput);
+		if (document == null) {
+			BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
+			throw new CoreException(Status.CANCEL_STATUS);
+		}
+		try {
+			IRegion region = document.getLineInformation(ts.getStartLine());
+			List<LambdaExpression> lambdaExpresions = findLambdaExpressions(editor, region);
+			if (lambdaExpresions.isEmpty()) {
+				return;
+			}
+			int lambdaPosition;
+			if (lambdaExpresions.size() == 1) {
+				lambdaPosition = 0;
+			} else {
+				lambdaPosition = selectLambda(lambdaExpresions);
+				if (lambdaPosition == -1) {
+					return;
+				}
+			}
+			LambdaExpression selectedLambda = lambdaExpresions.get(lambdaPosition);
+			IMethodBinding methodBinding = selectedLambda.resolveMethodBinding();
+			if (methodBinding == null) {
+				BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
+				return;
+			}
+			String lambdaMethodName = LambdaLocationLocatorHelper.toMethodName(methodBinding);
+			String lambdaMethodSignature = LambdaLocationLocatorHelper.toMethodSignature(methodBinding);
+			ITextSelection textSelection = new TextSelection(document, selectedLambda.getStartPosition(), selectedLambda.getLength());
+			boolean inline = lambdaExpresions.size() > 1;
+			LambdaProperties lambdaProperties = new LambdaProperties(lambdaExpresions.get(lambdaPosition).toString(), lambdaPosition, inline);
+			toggleLambdaEntryMethodBreakpoints(part, textSelection, lambdaMethodName, lambdaMethodSignature, lambdaProperties);
+		} catch (BadLocationException e) {
+			BreakpointToggleUtils.report(ActionMessages.LambdaEntryBreakpointToggleAction_Unavailable, part);
+		}
+	}
+
 	private static List<LambdaExpression> findLambdaExpressions(ITextEditor editor, IRegion region) {
 		LambdaCollector lambdas = new LambdaCollector(region.getOffset(), region.getOffset() + region.getLength());
 		CompilationUnit unitForLambdas = parseCompilationUnit(editor);
+		if (unitForLambdas == null) {
+			JDIDebugUIPlugin.log("Failed to parse CU for: " + editor.getTitle(), new IllegalStateException()); //$NON-NLS-1$
+			return List.of();
+		}
 		unitForLambdas.accept(lambdas);
 		return lambdas.getLambdaExpressions();
 	}
@@ -1962,10 +1962,11 @@ private static ICompilationUnit getCompilationUnit(ITextEditor editor) {
 		return JavaCore.createCompilationUnitFrom(file);
 	}
 
-	public int selectLambda(List<LambdaExpression> lambdaExps) {
+	static int selectLambda(List<LambdaExpression> lambdaExps) {
 		Map<String, Integer> seen = new HashMap<>();
-		// Generate unique names for lambda expressions. Note that two identical
-		// lambda expressions in different locations should still have different names.
+		// Generate unique labels used also as keys for lambda expressions. Note that two identical
+		// lambda expressions in different locations should still have different names
+		// otherwise the selection dialog cannot distinguish them.
 		List<String> lambdaNames = lambdaExps.stream().map(expr -> shortenLambdaExpression(expr.toString())).map(name -> {
 			int count = seen.getOrDefault(name, 0);
 			seen.put(name, count + 1);
@@ -2033,7 +2034,7 @@ protected void setShellStyle(int newShellStyle) {
 		}
 	}
 
-	private static final record LambdaBreakpoint(String lambdaName, int lambdaPosition, boolean isInline) {
+	private static final record LambdaProperties(String lambdaName, int lambdaPosition, boolean isInline) {
 		public String getLambdaName() {
 			return shortenLambdaExpression(lambdaName);
 		}

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/LambdaCollector.java

@@ -26,16 +26,16 @@
 public class LambdaCollector extends ASTVisitor {
 	private final int fLineOffset;
 	private final int fLineEndPosition;
-	private List<LambdaExpression> lambdaExpressions;
+	private final List<LambdaExpression> lambdaExpressions;
 
 	public LambdaCollector(int lineOffset, int lineEndPosition) {
 		fLineOffset = lineOffset;
 		fLineEndPosition = lineEndPosition;
 		lambdaExpressions = new ArrayList<>();
 	}
+
 	@Override
 	public boolean visit(LambdaExpression node) {
-
 		if (node.getStartPosition() < fLineOffset || node.getStartPosition() > fLineEndPosition) {
 			return false;
 		}