Modify MoveInstanceMethodProcessor to check for possible null comparison

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2025 IBM Corporation and others.
+ * Copyright (c) 2000, 2026 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -1405,6 +1405,8 @@ public final class RefactoringCoreMessages extends NLS {
 
 	public static String MoveInstanceMethodProcessor_target_name_already_used;
 
+	public static String MoveInstanceMethodProcessor_target_null_comparison;
+
 	public static String MoveInstanceMethodProcessor_this_reference;
 
 	public static String MoveInstanceMethodProcessor_uses_super;
@@ -2550,6 +2552,7 @@ public final class RefactoringCoreMessages extends NLS {
 	public static String ConvertToRecordRefactoring_member_types_not_supported;
 
 
+
 	static {
 		NLS.initializeMessages(BUNDLE_NAME, RefactoringCoreMessages.class);
 	}

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties

@@ -934,6 +934,7 @@ MoveInstanceMethodProcessor_this_reference=A reference to 'this' has been found
 MoveInstanceMethodProcessor_no_resolved_target=The target of the method could not be resolved.
 MoveInstanceMethodProcessor_no_null_argument=The method invocation ''{0}'' cannot be updated, since it uses null as argument.
 MoveInstanceMethodProcessor_target_name_already_used=The name of the target conflicts with the method parameter ''{0}''.
+MoveInstanceMethodProcessor_target_null_comparison=The move will cause ''{0}'' being replaced by ''this'' and compared to null which is invalid logic and may imply an NPE can occur.
 MoveInstanceMethodProcessor_target_element_pattern=Target element: ''{0}''
 MoveInstanceMethodProcessor_present_type_parameter_warning=The type parameter ''{0}'' is already present in the target type ''{1}'' and will be removed from the method.
 MoveInstanceMethodProcessor_remove_original_method=Remove method declaration

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/MoveInstanceMethodProcessor.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2025 IBM Corporation and others.
+ * Copyright (c) 2000, 2026 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -97,6 +97,8 @@
 import org.eclipse.jdt.core.dom.IMethodBinding;
 import org.eclipse.jdt.core.dom.ITypeBinding;
 import org.eclipse.jdt.core.dom.IVariableBinding;
+import org.eclipse.jdt.core.dom.InfixExpression;
+import org.eclipse.jdt.core.dom.InfixExpression.Operator;
 import org.eclipse.jdt.core.dom.Javadoc;
 import org.eclipse.jdt.core.dom.MethodDeclaration;
 import org.eclipse.jdt.core.dom.MethodInvocation;
@@ -1713,7 +1715,7 @@ public RefactoringStatus checkFinalConditions(final IProgressMonitor monitor, fi
 		final RefactoringStatus status= new RefactoringStatus();
 		fChangeManager= new TextChangeManager();
 		try {
-			monitor.beginTask("", 6); //$NON-NLS-1$
+			monitor.beginTask("", 7); //$NON-NLS-1$
 			monitor.setTaskName(RefactoringCoreMessages.MoveInstanceMethodProcessor_checking);
 			status.merge(Checks.checkIfCuBroken(fMethod));
 			if (!status.hasError()) {
@@ -1732,6 +1734,7 @@ public RefactoringStatus checkFinalConditions(final IProgressMonitor monitor, fi
 								checkConflictingTarget(Progress.subMonitor(monitor, 1), status);
 								checkConflictingMethod(Progress.subMonitor(monitor, 1), status);
 								checkOverrideOuterMethod(Progress.subMonitor(monitor, 1), status);
+								checkTargetNullCheck(Progress.subMonitor(monitor,  1), status);
 								checkFinalMethod(status);
 
 								Checks.addModifiedFilesToChecker(computeModifiedFiles(fMethod.getCompilationUnit(), type.getCompilationUnit()), context);
@@ -1751,6 +1754,44 @@ public RefactoringStatus checkFinalConditions(final IProgressMonitor monitor, fi
 		return status;
 	}
 
+	private void checkTargetNullCheck(IProgressMonitor monitor, RefactoringStatus status) throws JavaModelException {
+		Assert.isNotNull(monitor);
+		Assert.isNotNull(status);
+		try {
+			monitor.beginTask("", 1); //$NON-NLS-1$
+			monitor.setTaskName(RefactoringCoreMessages.MoveInstanceMethodProcessor_checking);
+			ASTVisitor visitor= new ASTVisitor() {
+				@Override
+				public boolean visit(InfixExpression node) {
+					if (node.getOperator() == Operator.EQUALS || node.getOperator() == Operator.NOT_EQUALS) {
+						Expression rightOperand= node.getRightOperand();
+						if (rightOperand instanceof NullLiteral) {
+							Expression leftOperand= node.getLeftOperand();
+							if (leftOperand instanceof Name name) {
+								IBinding nameBinding= name.resolveBinding();
+								if (nameBinding.isEqualTo(fTarget)) {
+									throw new AbortSearchException();
+								}
+							}
+						}
+					}
+					return true;
+				}
+			};
+			try {
+				final MethodDeclaration declaration= ASTNodeSearchUtil.getMethodDeclarationNode(fMethod, fSourceRewrite.getRoot());
+				if (declaration != null) {
+					declaration.accept(visitor);
+				}
+			} catch (AbortSearchException e) {
+				status.merge(RefactoringStatus.createErrorStatus(
+						Messages.format(RefactoringCoreMessages.MoveInstanceMethodProcessor_target_null_comparison, new String[] {fTarget.getName()}), JavaStatusContext.create(fMethod)));
+			}
+		} finally {
+			monitor.done();
+		}
+	}
+
 	/**
 	 * Checks whether the target is a type variable or a generic type.
 	 *

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/cannotMove/testFail21/in/A.java

@@ -0,0 +1,10 @@
+package p1;
+
+class A {
+    B b;
+
+    int m() {
+        if (b != null) return 2;
+        return 0;
+    }
+}

org.eclipse.jdt.ui.tests.refactoring/resources/MoveInstanceMethod/cannotMove/testFail21/in/B.java

@@ -0,0 +1,5 @@
+package p1;
+
+public class B {
+
+}

org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/MoveInstanceMethodTests.java

@@ -789,6 +789,48 @@ public void testFail1() throws Exception {
 		failHelper1("p1.A", 5, 26, 5, 29, PARAMETER, "b", true, true);
 	}
 
+	// Cannot move static method
+	@Test
+	public void testFail2() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 23, 6, 24, PARAMETER, "b", true, true);
+	}
+
+	// Cannot move native method
+	@Test
+	public void testFail3() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 23, 6, 24, PARAMETER, "b", true, true);
+	}
+
+	// Cannot move method that references "super"
+	@Test
+	public void testFail4() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 11, 20, 11, 21, PARAMETER, "b", true, true);
+	}
+
+	// Cannot move method that references an enclosing instance
+	@Test
+	public void testFail5() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 8, 21, 8, 21, PARAMETER, "b", true, true);
+	}
+
+	// Cannot move potentially directly recursive method
+	@Test
+	public void testFail6() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 16, 6, 17, PARAMETER, "b", true, true);
+	}
+
+	// Cannot move synchronized method
+	@Test
+	public void testFail8() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 29, 6, 29, PARAMETER, "b", true, true);
+	}
+
+	// Cannot move method if there's no new potential receiver
+	@Test
+	public void testFail9() throws Exception {
+		failHelper1(new String[] { "p1.A", "p2.B", "p3.C"}, "p1.A", 7, 17, 7, 20, PARAMETER, "b", true, true);
+	}
+
 	// Cannot move method if there's no new potential receiver
 	@Test
 	public void testFail10() throws Exception {
@@ -857,45 +899,9 @@ public void testFail20() throws Exception {
 		failHelper1(new String[] { "p1.A" }, "p1.A", 6, 17, 6, 23, FIELD, "b", true, true);
 	}
 
-	// Cannot move static method
-	@Test
-	public void testFail2() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 23, 6, 24, PARAMETER, "b", true, true);
-	}
-
-	// Cannot move native method
-	@Test
-	public void testFail3() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 23, 6, 24, PARAMETER, "b", true, true);
-	}
-
-	// Cannot move method that references "super"
-	@Test
-	public void testFail4() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 11, 20, 11, 21, PARAMETER, "b", true, true);
-	}
-
-	// Cannot move method that references an enclosing instance
-	@Test
-	public void testFail5() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 8, 21, 8, 21, PARAMETER, "b", true, true);
-	}
-
-	// Cannot move potentially directly recursive method
-	@Test
-	public void testFail6() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 16, 6, 17, PARAMETER, "b", true, true);
-	}
-
-	// Cannot move synchronized method
-	@Test
-	public void testFail8() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B"}, "p1.A", 6, 29, 6, 29, PARAMETER, "b", true, true);
-	}
-
-	// Cannot move method if there's no new potential receiver
+	// Issue 3035
 	@Test
-	public void testFail9() throws Exception {
-		failHelper1(new String[] { "p1.A", "p2.B", "p3.C"}, "p1.A", 7, 17, 7, 20, PARAMETER, "b", true, true);
+	public void testFail21() throws Exception {
+		failHelper1(new String[] { "p1.A", "p1.B" }, "p1.A", 6, 9, 6, 10, FIELD, "b", true, true);
 	}
 }