Bug 344984 - BadLocationException from ToggleBreakpointAdapter whenv20120302-2319

trying to toggle watchpoint

4 files changed, 105 insertions, 68 deletions

diff --git a/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java b/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java
new file mode 100644
index 000000000..666b9f71e
--- /dev/null
+++ b/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java
@@ -0,0 +1,7 @@
+public class BreakpointsLocationBug344984 {

+    private final String fWorkingValues; // Breakpoint here 

+    BreakpointsLocationBug344984() {

+        fWorkingValues= null;

+        System.out.println(fWorkingValues);

+    }

+}
\ No newline at end of file
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java
index e1cb4834f..efd72a892 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/BreakpointLocationVerificationTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- *  Copyright (c) 2000, 2011 IBM Corporation and others.
+ *  Copyright (c) 2000, 2012 IBM Corporation and others.
  *  All rights reserved. This program and the accompanying materials
  *  are made available under the terms of the Eclipse Public License v1.0
  *  which accompanies this distribution, and is available at
@@ -138,6 +138,14 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest {
 	}
     
 	/**
+	 * Tests that a breakpoint is not set on a final field
+	 * @throws Exception
+	 */
+	public void testFieldLocationOnFinalField() throws Exception {
+		testLocation(2, 4, "BreakpointsLocationBug344984");
+	}
+	
+	/**
 	 * Tests a specific breakpoint location
 	 * @throws Exception
 	 */
diff --git a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java
index bce1e16ed..ae0528d91 100644
--- a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java
+++ b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ToggleBreakpointAdapter.java
@@ -205,71 +205,7 @@ public class ToggleBreakpointAdapter implements IToggleBreakpointsTargetExtensio
         Job job = new Job("Toggle Line Breakpoint") { //$NON-NLS-1$
             @Override
 			protected IStatus run(IProgressMonitor monitor) {
-            	ITextEditor editor = getTextEditor(part);
-                if (editor != null && selection instanceof ITextSelection) {
-                    if (monitor.isCanceled()) {
-                        return Status.CANCEL_STATUS;
-                    }
-                    try {
-	                    report(null, part);
-	                    ISelection sel = selection;
-	                	if(!(selection instanceof IStructuredSelection)) {
-	                		sel = translateToMembers(part, selection);
-	                	}
-	                	if(isInterface(sel, part)) {
-	                		report(ActionMessages.ToggleBreakpointAdapter_6, part);
-	                    	return Status.OK_STATUS;
-	                	}
-	                    if(sel instanceof IStructuredSelection) {
-	                    	IMember member = (IMember) ((IStructuredSelection)sel).getFirstElement();
-	                    	IType type = null;
-	                    	if(member.getElementType() == IJavaElement.TYPE) {
-	                    		type = (IType) member;
-	                    	}
-	                    	else {
-	                    		type = member.getDeclaringType();
-	                    	}
-	                    	String tname = null;
-	                    	IJavaProject project = type.getJavaProject();
-	                    	if (locator == null || (project != null && !project.isOnClasspath(type))) {
-	                    		tname = createQualifiedTypeName(type);
-	                    	} else {
-	                    		tname = locator.getFullyQualifiedTypeName();
-	                    	}
-	                    	IResource resource = BreakpointUtils.getBreakpointResource(type);
-							int lnumber = locator == null ? ((ITextSelection) selection).getStartLine() + 1 : locator.getLineLocation();
-							IJavaLineBreakpoint existingBreakpoint = JDIDebugModel.lineBreakpointExists(resource, tname, lnumber);
-							if (existingBreakpoint != null) {
-								deleteBreakpoint(existingBreakpoint, editor, monitor);
-								return Status.OK_STATUS;
-							}
-							Map<String, Object> attributes = new HashMap<String, Object>(10);
-							IDocumentProvider documentProvider = editor.getDocumentProvider();
-							if (documentProvider == null) {
-							    return Status.CANCEL_STATUS;
-							}
-							IDocument document = documentProvider.getDocument(editor.getEditorInput());
-							int charstart = -1, charend = -1;
-							try {
-								IRegion line = document.getLineInformation(lnumber - 1);
-								charstart = line.getOffset();
-								charend = charstart + line.getLength();
-							} 	
-							catch (BadLocationException ble) {JDIDebugUIPlugin.log(ble);}
-							BreakpointUtils.addJavaBreakpointAttributes(attributes, type);
-							IJavaLineBreakpoint breakpoint = JDIDebugModel.createLineBreakpoint(resource, tname, lnumber, charstart, charend, 0, true, attributes);
-							if(locator == null) {
-								new BreakpointLocationVerifierJob(document, parseCompilationUnit(type.getTypeRoot()), breakpoint, lnumber, tname, type, editor, bestMatch).schedule();
-							}
-	                    }
-	                    else {
-	                    	report(ActionMessages.ToggleBreakpointAdapter_3, part);
-	                    	return Status.OK_STATUS;
-	                    }
-                    } 
-                    catch (CoreException ce) {return ce.getStatus();}
-                }
-                return Status.OK_STATUS;
+            	return doLineBreakpointToggle(selection, part, locator, bestMatch, monitor);
             }
         };
         job.setPriority(Job.INTERACTIVE);
@@ -375,6 +311,88 @@ public class ToggleBreakpointAdapter implements IToggleBreakpointsTargetExtensio
     }
     
     /**
+     * Performs the actual toggling of the line breakpoint
+     * @param selection the current selection (from the editor or view)
+     * @param part the active part
+     * @param locator the locator, may be <code>null</code>
+     * @param bestMatch if we should consider the best match rather than an exact match
+     * @param monitor progress reporting
+     * @return the status of the toggle
+     * @since 3.8
+     */
+    IStatus doLineBreakpointToggle(ISelection selection, IWorkbenchPart part, ValidBreakpointLocationLocator locator, boolean bestMatch, IProgressMonitor monitor) {
+    	ITextEditor editor = getTextEditor(part);
+        if (editor != null && selection instanceof ITextSelection) {
+            if (monitor.isCanceled()) {
+                return Status.CANCEL_STATUS;
+            }
+            ITextSelection tsel = (ITextSelection) selection;
+            if(tsel.getStartLine() < 0) {
+            	return Status.CANCEL_STATUS;
+            }
+            try {
+                report(null, part);
+                ISelection sel = selection;
+            	if(!(selection instanceof IStructuredSelection)) {
+            		sel = translateToMembers(part, selection);
+            	}
+            	if(isInterface(sel, part)) {
+            		report(ActionMessages.ToggleBreakpointAdapter_6, part);
+                	return Status.OK_STATUS;
+            	}
+                if(sel instanceof IStructuredSelection) {
+                	IMember member = (IMember) ((IStructuredSelection)sel).getFirstElement();
+                	IType type = null;
+                	if(member.getElementType() == IJavaElement.TYPE) {
+                		type = (IType) member;
+                	}
+                	else {
+                		type = member.getDeclaringType();
+                	}
+                	String tname = null;
+                	IJavaProject project = type.getJavaProject();
+                	if (locator == null || (project != null && !project.isOnClasspath(type))) {
+                		tname = createQualifiedTypeName(type);
+                	} else {
+                		tname = locator.getFullyQualifiedTypeName();
+                	}
+                	IResource resource = BreakpointUtils.getBreakpointResource(type);
+					int lnumber = locator == null ? tsel.getStartLine() + 1 : locator.getLineLocation();
+					IJavaLineBreakpoint existingBreakpoint = JDIDebugModel.lineBreakpointExists(resource, tname, lnumber);
+					if (existingBreakpoint != null) {
+						deleteBreakpoint(existingBreakpoint, editor, monitor);
+						return Status.OK_STATUS;
+					}
+					Map<String, Object> attributes = new HashMap<String, Object>(10);
+					IDocumentProvider documentProvider = editor.getDocumentProvider();
+					if (documentProvider == null) {
+					    return Status.CANCEL_STATUS;
+					}
+					IDocument document = documentProvider.getDocument(editor.getEditorInput());
+					int charstart = -1, charend = -1;
+					try {
+						IRegion line = document.getLineInformation(lnumber - 1);
+						charstart = line.getOffset();
+						charend = charstart + line.getLength();
+					} 	
+					catch (BadLocationException ble) {JDIDebugUIPlugin.log(ble);}
+					BreakpointUtils.addJavaBreakpointAttributes(attributes, type);
+					IJavaLineBreakpoint breakpoint = JDIDebugModel.createLineBreakpoint(resource, tname, lnumber, charstart, charend, 0, true, attributes);
+					if(locator == null) {
+						new BreakpointLocationVerifierJob(document, parseCompilationUnit(type.getTypeRoot()), breakpoint, lnumber, tname, type, editor, bestMatch).schedule();
+					}
+                }
+                else {
+                	report(ActionMessages.ToggleBreakpointAdapter_3, part);
+                	return Status.OK_STATUS;
+                }
+            } 
+            catch (CoreException ce) {return ce.getStatus();}
+        }
+        return Status.OK_STATUS;
+    }
+    
+    /**
      * Toggles a class load breakpoint
      * @param part the part
      * @param selection the current selection
@@ -867,8 +885,7 @@ public class ToggleBreakpointAdapter implements IToggleBreakpointsTargetExtensio
 	                        breakpoint = getWatchpoint(typeName, fieldName);
 	                        if (breakpoint == null) {
 	                        	if(!allowed) {
-	                        		toggleLineBreakpoints(part, finalSelection);
-	                        		return Status.OK_STATUS;
+	                        		return doLineBreakpointToggle(finalSelection, part, null, true, monitor);
 	                        	}
 	                        	int start = -1;
 	                            int end = -1;
diff --git a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java
index 89edac815..d90c3a144 100644
--- a/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java
+++ b/org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/breakpoints/ValidBreakpointLocationLocator.java
@@ -14,6 +14,7 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import org.eclipse.jdt.core.Flags;
 import org.eclipse.jdt.core.dom.ASTNode;
 import org.eclipse.jdt.core.dom.ASTVisitor;
 import org.eclipse.jdt.core.dom.AbstractTypeDeclaration;
@@ -719,6 +720,10 @@ public class ValidBreakpointLocationLocator extends ASTVisitor {
 	 */
 	@Override
 	public boolean visit(FieldDeclaration node) {
+		int mods = node.getModifiers();
+		if(Flags.isFinal(mods)) {
+			return false;
+		}
 		if (visit(node, false)) {
 			if (fBestMatch) {
 				// check if the line contains a single field declaration.