Fix naming strategy for 'assign all params to new fields' action

Signed-off-by: Fred Bricon <[email protected]>

Author: fbricon

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/proposals/AssignToVariableAssistProposal.java

@@ -85,16 +85,17 @@ public class AssignToVariableAssistProposal extends LinkedCorrectionProposal {
 	private final int  fVariableKind;
 	private final List<ASTNode> fNodesToAssign; // ExpressionStatement or SingleVariableDeclaration(s)
 	private final ITypeBinding fTypeBinding;
+	private final List<String> fParamNames;
 
 	private VariableDeclarationFragment fExistingFragment;
 
 	public AssignToVariableAssistProposal(ICompilationUnit cu, int variableKind, ExpressionStatement node, ITypeBinding typeBinding, int relevance) {
 		super("", JavaCodeActionKind.QUICK_ASSIST, cu, null, relevance); //$NON-NLS-1$
 
 		fVariableKind= variableKind;
+		fParamNames = null;
 		fNodesToAssign= new ArrayList<>();
 		fNodesToAssign.add(node);
-
 		fTypeBinding= Bindings.normalizeForDeclarationUse(typeBinding, node.getAST());
 		if (variableKind == LOCAL) {
 			setDisplayName(CorrectionMessages.AssignToVariableAssistProposal_assigntolocal_description);
@@ -108,6 +109,7 @@ public AssignToVariableAssistProposal(ICompilationUnit cu, String kind, int vari
 		super("", kind, cu, null, relevance); //$NON-NLS-1$
 
 		fVariableKind = variableKind;
+		fParamNames = null;
 		fNodesToAssign = new ArrayList<>();
 		fNodesToAssign.add(node);
 
@@ -128,7 +130,7 @@ public AssignToVariableAssistProposal(ICompilationUnit cu, SingleVariableDeclara
 		fNodesToAssign.add(parameter);
 		fTypeBinding= typeBinding;
 		fExistingFragment= existingFragment;
-
+		fParamNames = null;
 		if (existingFragment == null) {
 			setDisplayName(CorrectionMessages.AssignToVariableAssistProposal_assignparamtofield_description);
 		} else {
@@ -143,6 +145,8 @@ public AssignToVariableAssistProposal(ICompilationUnit cu, List<SingleVariableDe
 		fNodesToAssign= new ArrayList<>();
 		fNodesToAssign.addAll(parameters);
 		fTypeBinding= null;
+		fParamNames = new ArrayList<>();
+		populateNames(parameters);
 
 		setDisplayName(CorrectionMessages.AssignToVariableAssistProposal_assignallparamstofields_description);
 	}
@@ -161,6 +165,16 @@ protected ASTRewrite getRewrite() throws CoreException {
 		}
 	}
 
+	private void populateNames(List<SingleVariableDeclaration> parameters) {
+		if (parameters != null && parameters.size() > 0) {
+			for (SingleVariableDeclaration param : parameters) {
+				if (param.getName() != null) {
+					fParamNames.add(param.getName().getIdentifier());
+				}
+			}
+		}
+	}
+
 	private ASTRewrite doAddLocal() {
 		ASTNode nodeToAssign= fNodesToAssign.get(0);
 		Expression expression= ((ExpressionStatement) nodeToAssign).getExpression();
@@ -373,7 +387,28 @@ private String[] suggestFieldNames(ITypeBinding binding, Expression expression,
 	}
 
 	private Collection<String> getUsedVariableNames(ASTNode nodeToAssign) {
-		return Arrays.asList(ASTResolving.getUsedVariableNames(nodeToAssign));
+		Collection<String> usedVarNames = Arrays.asList(ASTResolving.getUsedVariableNames(nodeToAssign));
+		Collection<String> additionalVarNames = getRemainingParamNamed(nodeToAssign);
+		if (additionalVarNames != null) {
+			usedVarNames = new ArrayList<>(Arrays.asList(ASTResolving.getUsedVariableNames(nodeToAssign)));
+			usedVarNames.addAll(additionalVarNames);
+		}
+		return usedVarNames;
+	}
+
+	private ArrayList<String> getRemainingParamNamed(ASTNode nodeToAssign) {
+		ArrayList<String> paramNames = null;
+		if (fParamNames != null) {
+			paramNames = new ArrayList<>();
+			paramNames.addAll(fParamNames);
+			if (nodeToAssign instanceof SingleVariableDeclaration && ((SingleVariableDeclaration) nodeToAssign).getName() != null) {
+				int index = fNodesToAssign.indexOf(nodeToAssign);
+				if (index >= 0 && index < paramNames.size()) {
+					paramNames.remove(index);
+				}
+			}
+		}
+		return paramNames;
 	}
 
 	private int findAssignmentInsertIndex(List<Statement> statements, ASTNode nodeToAssign) {

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

@@ -262,6 +262,54 @@ public void testResolveCodeAction_QuickAssists() throws Exception {
 		Assert.assertEquals(buf.toString(), actual);
 	}
 
+	@Test
+	public void testAssignAllParamsToFields() throws Exception {
+		when(preferenceManager.getClientPreferences().isResolveCodeActionSupported()).thenReturn(true);
+
+		StringBuilder buf = new StringBuilder();
+		buf.append("public class App {\n");
+		buf.append("    private String s;\n");
+		buf.append("\n");
+		buf.append("    public App(String s, String s3, String s2) {\n");
+		buf.append("    }\n");
+		buf.append("}\n");
+		ICompilationUnit unit = defaultPackage.createCompilationUnit("App.java", buf.toString(), false, null);
+		CodeActionParams params = new CodeActionParams();
+		params.setTextDocument(new TextDocumentIdentifier(JDTUtils.toURI(unit)));
+		Range range = CodeActionUtil.getRange(unit, "s3");
+		params.setRange(range);
+		CodeActionContext context = new CodeActionContext(Collections.emptyList(), Collections.singletonList(JavaCodeActionKind.QUICK_ASSIST));
+		params.setContext(context);
+
+		List<Either<Command, CodeAction>> codeActions = server.codeAction(params).join();
+		Assert.assertNotNull(codeActions);
+		Assert.assertFalse("No quickassist actions were found", codeActions.isEmpty());
+
+		Optional<Either<Command, CodeAction>> assignAllToNewFieldsResponse = codeActions.stream().filter(codeAction -> {
+			return "Assign all parameters to new fields".equals(codeAction.getRight().getTitle());
+		}).findFirst();
+		Assert.assertTrue("Should return the quick assist 'Assign all parameters to new fields'", assignAllToNewFieldsResponse.isPresent());
+		CodeAction unresolvedCodeAction = assignAllToNewFieldsResponse.get().getRight();
+
+		CodeAction resolvedCodeAction = server.resolveCodeAction(unresolvedCodeAction).join();
+		Assert.assertNotNull("Should resolve the edit property in the resolveCodeAction request", resolvedCodeAction.getEdit());
+		String actual = AbstractQuickFixTest.evaluateWorkspaceEdit(resolvedCodeAction.getEdit());
+		buf = new StringBuilder();
+		buf.append("public class App {\n");
+		buf.append("    private String s;\n");
+		buf.append("    private String s4;\n");
+		buf.append("    private String s3;\n");
+		buf.append("    private String s2;\n");
+		buf.append("\n");
+		buf.append("    public App(String s, String s3, String s2) {\n");
+		buf.append("        s4 = s;\n");
+		buf.append("        this.s3 = s3;\n");
+		buf.append("        this.s2 = s2;\n");
+		buf.append("    }\n");
+		buf.append("}\n");
+		Assert.assertEquals(buf.toString(), actual);
+	}
+
 	@Test
 	public void testResolveCodeAction_SourceActions() throws Exception {
 		when(preferenceManager.getClientPreferences().isResolveCodeActionSupported()).thenReturn(true);