Add error handling for rename operation.

Signed-off-by: Yaohai Zheng <[email protected]>
eclipse-jdtls · Nov 26, 2018 · 5142e40 · 5142e40
1 parent a3575c8
commit 5142e40
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 9 deletions.
diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/RenameHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/RenameHandler.java
@@ -27,6 +27,9 @@
 import org.eclipse.lsp4j.RenameOptions;
 import org.eclipse.lsp4j.RenameParams;
 import org.eclipse.lsp4j.WorkspaceEdit;
+import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
+import org.eclipse.lsp4j.jsonrpc.messages.ResponseError;
+import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode;
 import org.eclipse.ltk.core.refactoring.Change;
 import org.eclipse.ltk.core.refactoring.CheckConditionsOperation;
 import org.eclipse.ltk.core.refactoring.CreateChangeOperation;
@@ -81,8 +84,12 @@ public WorkspaceEdit rename(RenameParams params, IProgressMonitor monitor) {
 			}
 			RenameRefactoring renameRefactoring = renameSupport.getRenameRefactoring();
 
-			CreateChangeOperation create = new CreateChangeOperation(new CheckConditionsOperation(renameRefactoring, CheckConditionsOperation.ALL_CONDITIONS), RefactoringStatus.FATAL);
+			CheckConditionsOperation check = new CheckConditionsOperation(renameRefactoring, CheckConditionsOperation.ALL_CONDITIONS);
+			CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
 			create.run(monitor);
+			if (check.getStatus().getSeverity() >= RefactoringStatus.FATAL) {
+				throw new ResponseErrorException(new ResponseError(ResponseErrorCode.InvalidRequest, check.getStatus().getMessageMatchingSeverity(RefactoringStatus.ERROR), null));
+			}
 
 			Change change = create.getChange();
 			ChangeUtil.convertCompositeChange(change, edit);

diff --git a/...eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/RenameHandlerTest.java b/...eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/RenameHandlerTest.java
@@ -19,7 +19,6 @@
 import java.util.LinkedList;
 import java.util.List;
 
-import org.eclipse.core.resources.IFile;
 import org.eclipse.jdt.core.ICompilationUnit;
 import org.eclipse.jdt.core.IJavaProject;
 import org.eclipse.jdt.core.IPackageFragment;
@@ -43,6 +42,7 @@
 import org.eclipse.lsp4j.TextDocumentIdentifier;
 import org.eclipse.lsp4j.TextEdit;
 import org.eclipse.lsp4j.WorkspaceEdit;
+import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
 import org.eclipse.lsp4j.jsonrpc.messages.Either;
 import org.junit.Before;
 import org.junit.Test;
@@ -213,7 +213,7 @@ public void testRenameMethod() throws JavaModelException, BadLocationException {
 	}
 
 	@Test
-	public void testRenameTypeWhithResourceChanges() throws JavaModelException, BadLocationException {
+	public void testRenameTypeWithResourceChanges() throws JavaModelException, BadLocationException {
 		when(clientPreferences.isResourceOperationSupported()).thenReturn(true);
 
 		IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null);
@@ -231,8 +231,6 @@ public void testRenameTypeWhithResourceChanges() throws JavaModelException, BadL
 		Position pos = mergeCode(builder, codes);
 		ICompilationUnit cu = pack1.createCompilationUnit("E.java", builder.toString(), false, null);
 
-		IFile resource = (IFile) cu.getResource();
-
 		WorkspaceEdit edit = getRenameEdit(cu, pos, "Newname");
 		assertNotNull(edit);
 		List<Either<TextDocumentEdit, ResourceOperation>> resourceChanges = edit.getDocumentChanges();
@@ -262,7 +260,42 @@ public void testRenameTypeWhithResourceChanges() throws JavaModelException, BadL
 		assertEquals(expected, TextEditUtil.apply(builder.toString(), testChanges));
 	}
 
-	@Test
+	@Test(expected = ResponseErrorException.class)
+	public void testRenameTypeWithErrors() throws JavaModelException, BadLocationException {
+		when(clientPreferences.isResourceOperationSupported()).thenReturn(true);
+
+		IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null);
+
+		String[] codes = { "package test1;\n",
+				           "public class Newname {\n",
+				           "   }\n",
+				           "}\n" };
+		StringBuilder builder = new StringBuilder();
+		mergeCode(builder, codes);
+		ICompilationUnit cu = pack1.createCompilationUnit("Newname.java", builder.toString(), false, null);
+
+
+		String[] codes1 = { "package test1;\n",
+				           "public class E|* {\n",
+				           "   public E() {\n",
+				           "   }\n",
+				           "   public int bar() {\n", "   }\n",
+				           "   public int foo() {\n",
+				           "		this.bar();\n",
+				           "   }\n",
+				           "}\n" };
+		builder = new StringBuilder();
+		Position pos = mergeCode(builder, codes1);
+		cu = pack1.createCompilationUnit("E.java", builder.toString(), false, null);
+
+		WorkspaceEdit edit = getRenameEdit(cu, pos, "Newname");
+		assertNotNull(edit);
+		List<Either<TextDocumentEdit, ResourceOperation>> resourceChanges = edit.getDocumentChanges();
+
+		assertEquals(resourceChanges.size(), 3);
+	}
+
+	@Test(expected = ResponseErrorException.class)
 	public void testRenameSystemLibrary() throws JavaModelException {
 		IPackageFragment pack1 = sourceFolder.createPackageFragment("test1", false, null);
 
@@ -279,9 +312,7 @@ public void testRenameSystemLibrary() throws JavaModelException {
 		Position pos = mergeCode(builder, codes);
 		ICompilationUnit cu = pack1.createCompilationUnit("E.java", builder.toString(), false, null);
 
-		WorkspaceEdit edit = getRenameEdit(cu, pos, "newname");
-		assertNotNull(edit);
-		assertEquals(edit.getChanges().size(), 0);
+		getRenameEdit(cu, pos, "newname");
 	}
 
 	@Test