Bug 216098 - [breakpoints] Can't set a watchpoint on a final fieldI20141029-0800I20141028-2000I20141028-0800I20141027-2000I20141027-0800I20141026-2100

Change-Id: I331d9ebd0102c1125194e61efe0ac2bafb75b1f0
Signed-off-by: Sarika Sinha <sarika.sinha@in.ibm.com>

7 files changed, 100 insertions, 64 deletions

diff --git a/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java b/org.eclipse.jdt.debug.tests/testprograms/org/eclipse/debug/tests/targets/BreakpointsLocationBug344984.java
index 4c702f09c..7b99c50a0 100644
--- a/org.eclipse.jdt.debug.tests/testprograms/BreakpointsLocationBug344984.java
+++ b/org.eclipse.jdt.debug.tests/testprograms/org/eclipse/debug/tests/targets/BreakpointsLocationBug344984.java
@@ -1,17 +1,21 @@
-/*******************************************************************************

- * Copyright (c) 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

- * http://www.eclipse.org/legal/epl-v10.html

- *

- * Contributors:

- *     IBM Corporation - initial API and implementation

- *******************************************************************************/

-public class BreakpointsLocationBug344984 {

-    private final String fWorkingValues; // Breakpoint here 

-    BreakpointsLocationBug344984() {

-        fWorkingValues= null;

-        System.out.println(fWorkingValues);

-    }

+/*******************************************************************************
+ * Copyright (c) 2014 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.debug.tests.targets;
+public class BreakpointsLocationBug344984 {
+    private final String fWorkingValues; // Breakpoint here 
+    BreakpointsLocationBug344984() {
+        fWorkingValues= null;
+        System.out.println(fWorkingValues);
+    }
+    public static void main(String[] args) {
+		new BreakpointsLocationBug344984();
+	}
 }
\ No newline at end of file
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 729d2e3a6..05f49d329 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
@@ -182,7 +182,8 @@ public abstract class AbstractDebugTest extends TestCase implements  IEvaluation
 	public static final String BOUND_JRE_PROJECT_NAME = "BoundJRE";
 
 	final String[] LAUNCH_CONFIG_NAMES_1_4 = {"LargeSourceFile", "LotsOfFields", "Breakpoints", "InstanceVariablesTests", "LocalVariablesTests", "LocalVariableTests2", "StaticVariablesTests",
-			"DropTests", "ThrowsNPE", "ThrowsException", "org.eclipse.debug.tests.targets.Watchpoint", "org.eclipse.debug.tests.targets.CallLoop", "A",
+ "DropTests", "ThrowsNPE", "ThrowsException", "org.eclipse.debug.tests.targets.Watchpoint",
+			"org.eclipse.debug.tests.targets.BreakpointsLocationBug344984", "org.eclipse.debug.tests.targets.CallLoop", "A",
 			"HitCountLooper", "CompileError", "MultiThreadedLoop", "HitCountException", "MultiThreadedException", "MultiThreadedList", "MethodLoop", "StepFilterOne",
 			"StepFilterFour", "EvalArrayTests", "EvalSimpleTests", "EvalTypeTests", "EvalNestedTypeTests", "EvalTypeHierarchyTests", "WorkingDirectoryTest", 
 			"OneToTen", "OneToTenPrint", "FloodConsole", "ConditionalStepReturn", "VariableChanges", "DefPkgReturnType", "InstanceFilterObject", "org.eclipse.debug.tests.targets.CallStack", 
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 1413bcc4e..6aa2be25a 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, 2012 IBM Corporation and others.
+ *  Copyright (c) 2000, 2014 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
@@ -110,27 +110,6 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest {
 	}
 	
 	/**
-	 * Tests setting a line breakpoint on a final field with an Expression as an initializer
-	 * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=376354
-	 * 
-	 * @throws Exception
-	 */
-	public void testFinalFieldWithTypeDecl2() throws Exception {
-		testLocation(15, 15, "FinalBreakpointLocations");
-	}
-	
-	/**
-	 * Tests setting a line breakpoint on a final field with an Expression as an initializer looking
-	 * for best match
-	 * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=376354
-	 * 
-	 * @throws Exception
-	 */
-	public void testFinalFieldWithTypeDecl2a() throws Exception {
-		testLocation(15, 15, "FinalBreakpointLocations", "FinalBreakpointLocations", true);
-	}
-	
-	/**
 	 * Tests setting a line breakpoint on an inner type member for the initializer of 
 	 * a final local field variable
 	 * @see https://bugs.eclipse.org/bugs/show_bug.cgi?id=376354
@@ -150,7 +129,7 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest {
 	 * @throws Exception
 	 */
 	public void testFinalFieldWithTypeDecl3a() throws Exception {
-		testLocation(17, 17, "FinalBreakpointLocations");
+		testLocation(17, 17, "FinalBreakpointLocations", "FinalBreakpointLocations", true);
 	}
 	
 	/**
@@ -252,7 +231,7 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest {
 	 * @throws Exception
 	 */
 	public void testFieldLocationOnFinalField() throws Exception {
-		testLocation(12, 14, "BreakpointsLocationBug344984");
+		testLocation(13, 13, "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984");
 	}
 	
 	/**
@@ -261,7 +240,7 @@ public class BreakpointLocationVerificationTests extends AbstractDebugTest {
 	 * @throws Exception
 	 */
 	public void testFieldLocationOnFinalFielda() throws Exception {
-		testLocation(12, 14, "BreakpointsLocationBug344984", "BreakpointsLocationBug344984", true);
+		testLocation(13, 13, "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984", "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984", true);
 	}
 	
 	/**
diff --git a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java
index 1f6ae2ebf..abea61a1f 100644
--- a/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java
+++ b/org.eclipse.jdt.debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/WatchpointTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- *  Copyright (c) 2000, 2011 IBM Corporation and others.
+ *  Copyright (c) 2000, 2014 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
@@ -297,4 +297,39 @@ public class WatchpointTests extends AbstractDebugTest {
 			getBreakpointManager().setEnabled(true);
 		}		
 	}	
+
+	/**
+	 * Tests that a watchpoint set to be skipped is indeed skipped
+	 * 
+	 * @throws Exception
+	 */
+	public void testFinalWatchpoint() throws Exception {
+		String typeName = "org.eclipse.debug.tests.targets.BreakpointsLocationBug344984";
+
+		IJavaWatchpoint wp = createWatchpoint(typeName, "fWorkingValues", true, true);
+
+		IJavaThread thread = null;
+		try {
+			thread = launchToBreakpoint(typeName);
+			assertNotNull("Breakpoint not hit within timeout period", thread);
+
+			IBreakpoint hit = getBreakpoint(thread);
+			IStackFrame frame = thread.getTopStackFrame();
+			assertNotNull("No watchpoint", hit);
+
+			// should be modification
+			assertTrue("First hit should be modification", !wp.isAccessSuspend(thread.getDebugTarget()));
+			// line 27
+			assertEquals("Should be on line 15", 15, frame.getLineNumber());
+
+			getBreakpointManager().setEnabled(false);
+			resumeAndExit(thread);
+
+		}
+		finally {
+			terminateAndRemove(thread);
+			removeAllBreakpoints();
+			getBreakpointManager().setEnabled(true);
+		}
+	}
 }
diff --git a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java
index b5c09023f..b0277cecf 100644
--- a/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java
+++ b/org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/BreakpointMarkerUpdater.java
@@ -23,6 +23,7 @@ import org.eclipse.jdt.core.dom.CompilationUnit;
 import org.eclipse.jdt.debug.core.IJavaLineBreakpoint;
 import org.eclipse.jdt.debug.core.IJavaPatternBreakpoint;
 import org.eclipse.jdt.debug.core.IJavaStratumLineBreakpoint;
+import org.eclipse.jdt.debug.core.IJavaWatchpoint;
 import org.eclipse.jdt.internal.debug.core.JDIDebugPlugin;
 import org.eclipse.jdt.internal.debug.core.JavaDebugUtils;
 import org.eclipse.jdt.internal.debug.core.breakpoints.JavaLineBreakpoint;
@@ -111,6 +112,10 @@ public class BreakpointMarkerUpdater implements IMarkerUpdater {
 			if(loc.getLocationType() == ValidBreakpointLocationLocator.LOCATION_NOT_FOUND) {
 				return false;
 			}
+			// Remove the watch point if it is not a valid watch point now
+			if (loc.getLocationType() != ValidBreakpointLocationLocator.LOCATION_FIELD && breakpoint instanceof IJavaWatchpoint) {
+				return false;
+			}
 			int line = loc.getLineLocation();
 			//if the line number is already good, perform no marker updating
 			if(MarkerUtilities.getLineNumber(marker) == line) {
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 94288c85c..bb810b458 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
@@ -897,12 +897,18 @@ public class ToggleBreakpointAdapter implements IToggleBreakpointsTargetExtensio
 								fieldName = javaField.getElementName();
 								int f = javaField.getFlags();
 								boolean fin = Flags.isFinal(f);
+								if (fin) {
+									fin = javaField.getConstant() != null; // watch point is allowed if no constant value
+								}
 								allowed = !(fin) & !(Flags.isStatic(f) & fin);
 							} else if (element instanceof IJavaFieldVariable) {
 								IJavaFieldVariable var = (IJavaFieldVariable) element;
 								typeName = var.getDeclaringType().getName();
 								fieldName = var.getName();
 								boolean fin = var.isFinal();
+								if (fin) {
+									fin = javaField.getConstant() != null; // watch point is allowed if no constant value
+								}
 								allowed = !(fin) & !(var.isStatic() & fin);
 							}
 	                        breakpoint = getWatchpoint(typeName, fieldName);
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 3b74f5ccb..877f2ce3d 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
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2012 IBM Corporation and others.
+ * Copyright (c) 2003, 2014 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
@@ -185,14 +185,13 @@ public class ValidBreakpointLocationLocator extends ASTVisitor {
 	}
 
 	/**
-	 * Return the line number of the computed valid location, if the location
-	 * type is LOCATION_LINE.
+	 * Return the line number of the computed valid location
 	 */
 	public int getLineLocation() {
-		if (fLocationType == LOCATION_LINE) {
-			return fLineLocation;
+		if (fLocationType == LOCATION_NOT_FOUND) {
+			return -1;
 		}
-		return -1;
+		return fLineLocation;
 	}
 
 	/**
@@ -731,17 +730,28 @@ public class ValidBreakpointLocationLocator extends ASTVisitor {
 					int line = lineNumber(offset);
 					if(Flags.isFinal(node.getModifiers())) {
 						if(init != null) {
-							if (line == fLineNumber && isReplacedByConstantValue(init)) {
+							if (line == fLineNumber) {
+								if (isReplacedByConstantValue(init)) {
+									fLocationType = LOCATION_LINE;
+								} else {
+									fLocationType = LOCATION_FIELD;
+								}
 								fMemberOffset = offset;
 								fLineLocation = line;
-								fLocationType = LOCATION_LINE;
 								fLocationFound = true;
 								fTypeName = computeTypeName(node);
 								return false;
 							}
 						}
 						else {
-							//if it is an uninitialized final field, try to find the next executable line
+							if (line == fLineNumber) {
+								fMemberOffset = offset;
+								fLineLocation = line;
+								fLocationType = LOCATION_FIELD;
+								fLocationFound = true;
+								fTypeName = computeTypeName(node);
+								return false;
+							}
 							return false;
 						}
 					}
@@ -750,6 +760,7 @@ public class ValidBreakpointLocationLocator extends ASTVisitor {
 						// contains the name of the field
 						if (line == fLineNumber) {
 							fMemberOffset = offset;
+							fLineLocation = line;
 							fLocationType = LOCATION_FIELD;
 							fLocationFound = true;
 							return false;
@@ -1369,10 +1380,11 @@ public class ValidBreakpointLocationLocator extends ASTVisitor {
 	public boolean visit(VariableDeclarationFragment node) {
 		Expression initializer = node.getInitializer();
 		if (visit(node, false)) {
-			int startLine = lineNumber(node.getName().getStartPosition());
+			int offset = node.getName().getStartPosition();
+			int line = lineNumber(offset);
 			if (initializer != null) {
-				if (fLineNumber == startLine) {
-						fLineLocation = startLine;
+				if (fLineNumber == line) {
+					fLineLocation = line;
 						fLocationFound = true;
 						fLocationType = LOCATION_LINE;
 						fTypeName = computeTypeName(node);
@@ -1381,19 +1393,13 @@ public class ValidBreakpointLocationLocator extends ASTVisitor {
 				initializer.accept(this);
 			} else {
 				// the variable has no initializer
-				int offset = node.getName().getStartPosition();
 				// check if the breakpoint is to be set on the line which
 				// contains the name of the field
-				ASTNode parent = node.getParent();
-				if(parent.getNodeType() == ASTNode.FIELD_DECLARATION) {
-					//if the parent field is final and we are not initializing, find the next executable line
-					if(Flags.isFinal(((FieldDeclaration)parent).getModifiers())) {
-						return false;
-					}
-				}
-				if (lineNumber(offset) == fLineNumber) {
+				if (line == fLineNumber) {
 					fMemberOffset = offset;
+					fLineLocation = line;
 					fLocationType = LOCATION_FIELD;
+					fTypeName = computeTypeName(node);
 					fLocationFound = true;
 					return false;
 				}