Skip to content

Commit f808eef

Browse files
hopehadfieldhopehadfield
committed
Fix param and exception addition/deletion in method signature refactoring
Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
1 parent 9752381 commit f808eef

File tree

2 files changed

+121
-4
lines changed

2 files changed

+121
-4
lines changed

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureHandler.java

+21-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import java.util.ArrayList;
1616
import java.util.List;
17+
import java.util.stream.Collectors;
1718

1819
import org.eclipse.core.runtime.CoreException;
1920
import org.eclipse.core.runtime.NullProgressMonitor;
@@ -107,12 +108,16 @@ public static Refactoring getChangeSignatureRefactoring(CodeActionParams params,
107108
newParameterInfos.add(ParameterInfo.createInfoForAddedParameter(param.type, param.name, param.defaultValue));
108109
}
109110
}
110-
parameterInfos.clear();
111+
112+
parameterInfos.removeIf(p -> newParameterInfos.contains(p));
113+
parameterInfos.forEach(p -> p.markAsDeleted());
111114
parameterInfos.addAll(newParameterInfos);
115+
112116
List<ExceptionInfo> exceptionInfos = processor.getExceptionInfos();
113117
List<ExceptionInfo> newExceptionInfos = new ArrayList<>();
118+
int index = 0;
114119
for (MethodException exception : exceptions) {
115-
if (exception.typeHandleIdentifier != null) {
120+
if (exception.typeHandleIdentifier != null && (index >= exceptionInfos.size() || exceptionInfos.get(index).getFullyQualifiedName().equals(exception.type))) {
116121
IJavaElement element = JavaCore.create(exception.typeHandleIdentifier);
117122
if (element instanceof IType type) {
118123
newExceptionInfos.add(ExceptionInfo.createInfoForAddedException(type));
@@ -173,9 +178,21 @@ public void acceptTypeNameMatch(TypeNameMatch match) {
173178
newExceptionInfos.add(ExceptionInfo.createInfoForAddedException(type));
174179
}
175180
}
181+
index++;
176182
}
177-
exceptionInfos.clear();
178-
exceptionInfos.addAll(newExceptionInfos);
183+
184+
exceptionInfos.stream().forEach(oldException -> {
185+
if (newExceptionInfos.stream().filter(newException -> newException.getFullyQualifiedName().equals(oldException.getFullyQualifiedName())).collect(Collectors.toList()).isEmpty()) {
186+
oldException.markAsDeleted();
187+
}
188+
});
189+
190+
newExceptionInfos.stream().forEach(newException -> {
191+
if (exceptionInfos.stream().filter(oldException -> oldException.getFullyQualifiedName().equals(newException.getFullyQualifiedName())).collect(Collectors.toList()).isEmpty()) {
192+
exceptionInfos.add(newException);
193+
}
194+
});
195+
179196
Refactoring refactoring = new ProcessorBasedRefactoring(processor);
180197
refactoring.checkInitialConditions(new NullProgressMonitor());
181198
status = refactoring.checkFinalConditions(new NullProgressMonitor());

‎org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureHandlerTest.java

+100
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,104 @@ public void testChangeSignatureRefactoring() throws CoreException {
157157
//@formatter:on
158158
assertEquals(expected, unit.getSource());
159159
}
160+
161+
@Test
162+
public void testChangeSignatureRefactoringJavadocAddDelete() throws CoreException {
163+
//@formatter:off
164+
ICompilationUnit unit = fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
165+
"\r\n" +
166+
"import java.io.IOException;\r\n" +
167+
"\r\n" +
168+
"public class A {\r\n" +
169+
" /**\r\n" +
170+
" * @param input TODO\r\n" +
171+
" * @throws IOException TODO\r\n" +
172+
" *\r\n" +
173+
" */\r\n" +
174+
" public void getName(String input) throws IOException {\r\n" +
175+
" }\r\n" +
176+
"}"
177+
, true, null);
178+
//@formatter:on
179+
CodeActionParams params = CodeActionUtil.constructCodeActionParams(unit, "getName");
180+
List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
181+
Assert.assertNotNull(codeActions);
182+
Either<Command, CodeAction> changeSignatureAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.REFACTOR_CHANGE_SIGNATURE);
183+
Assert.assertNotNull(changeSignatureAction);
184+
Command changeSignatureCommand = CodeActionHandlerTest.getCommand(changeSignatureAction);
185+
Assert.assertNotNull(changeSignatureCommand);
186+
Assert.assertEquals(RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID, changeSignatureCommand.getCommand());
187+
List<Object> arguments = changeSignatureCommand.getArguments();
188+
Assert.assertEquals(3, arguments.size());
189+
Object arg1 = arguments.get(1);
190+
assertEquals(true, arg1 instanceof CodeActionParams);
191+
Object arg2 = arguments.get(2);
192+
assertEquals(true, arg2 instanceof ChangeSignatureInfo);
193+
ChangeSignatureInfo info = (ChangeSignatureInfo) arg2;
194+
IJavaElement element = JavaCore.create(info.methodIdentifier);
195+
assertEquals(true, element instanceof IMethod);
196+
List<MethodParameter> parameters = List.of(info.parameters[0], new MethodParameter("String", "input1", "null", ParameterInfo.INDEX_FOR_ADDED));
197+
List<MethodException> exceptions = List.of(info.exceptions[0], new MethodException("Exception", null));
198+
Refactoring refactoring = ChangeSignatureHandler.getChangeSignatureRefactoring((CodeActionParams) arg1, (IMethod) element, false, "getName1", JdtFlags.VISIBILITY_STRING_PRIVATE, "String", parameters, exceptions);
199+
Change change = refactoring.createChange(new NullProgressMonitor());
200+
change.perform(new NullProgressMonitor());
201+
//@formatter:off
202+
String expected = "package p;\r\n" +
203+
"\r\n" +
204+
"import java.io.IOException;\r\n" +
205+
"\r\n" +
206+
"public class A {\r\n" +
207+
" /**\r\n" +
208+
" * @param input TODO\r\n" +
209+
" * @param input1 TODO\r\n" +
210+
" * @return TODO\r\n" +
211+
" * @throws IOException TODO\r\n" +
212+
" * @throws Exception TODO\r\n" +
213+
" *\r\n" +
214+
" */\r\n" +
215+
" private String getName1(String input, String input1) throws IOException, Exception {\r\n" +
216+
" }\r\n" +
217+
"}";
218+
//@formatter:on
219+
assertEquals(expected, unit.getSource());
220+
221+
ICompilationUnit unitTwo = fPackageP.createCompilationUnit("A.java", expected, true, null);
222+
params = CodeActionUtil.constructCodeActionParams(unit, "getName");
223+
codeActions = server.codeAction(params).join();
224+
Assert.assertNotNull(codeActions);
225+
changeSignatureAction = CodeActionHandlerTest.findAction(codeActions, JavaCodeActionKind.REFACTOR_CHANGE_SIGNATURE);
226+
Assert.assertNotNull(changeSignatureAction);
227+
changeSignatureCommand = CodeActionHandlerTest.getCommand(changeSignatureAction);
228+
Assert.assertNotNull(changeSignatureCommand);
229+
Assert.assertEquals(RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID, changeSignatureCommand.getCommand());
230+
arguments = changeSignatureCommand.getArguments();
231+
Assert.assertEquals(3, arguments.size());
232+
arg1 = arguments.get(1);
233+
assertEquals(true, arg1 instanceof CodeActionParams);
234+
arg2 = arguments.get(2);
235+
assertEquals(true, arg2 instanceof ChangeSignatureInfo);
236+
info = (ChangeSignatureInfo) arg2;
237+
element = JavaCore.create(info.methodIdentifier);
238+
assertEquals(true, element instanceof IMethod);
239+
parameters = List.of(info.parameters[1]);
240+
exceptions = List.of(info.exceptions[1]);
241+
refactoring = ChangeSignatureHandler.getChangeSignatureRefactoring((CodeActionParams) arg1, (IMethod) element, false, "getName1", JdtFlags.VISIBILITY_STRING_PRIVATE, "String", parameters, exceptions);
242+
change = refactoring.createChange(new NullProgressMonitor());
243+
change.perform(new NullProgressMonitor());
244+
//@formatter:off
245+
String expectedTwo = "package p;\r\n" +
246+
"\r\n" +
247+
"public class A {\r\n" +
248+
" /**\r\n" +
249+
" * @param input1 TODO\r\n" +
250+
" * @return TODO\r\n" +
251+
" * @throws Exception TODO\r\n" +
252+
" *\r\n" +
253+
" */\r\n" +
254+
" private String getName1(String input1) throws Exception {\r\n" +
255+
" }\r\n" +
256+
"}";
257+
//@formatter:on
258+
assertEquals(expectedTwo, unitTwo.getSource());
259+
}
160260
}

0 commit comments

Comments
 (0)