Skip to content

Commit

Permalink
auto-select field when generating constructors (#2125)
Browse files Browse the repository at this point in the history 
Signed-off-by: Shi Chen <[email protected]>
  • Loading branch information
@CsCherrYY
CsCherrYY authored Jun 28, 2022
1 parent 7eafab9 commit b7b3190
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 10 deletions.
14 changes: 10 additions & 4 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import org.eclipse.lsp4j.Command;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.messages.Either;

Expand Down Expand Up @@ -129,10 +130,7 @@ public List<Either<Command, CodeAction>> getCodeActionCommands(CodeActionParams
return Collections.emptyList();
}

int start = DiagnosticsHelper.getStartOffset(unit, params.getRange());
int end = DiagnosticsHelper.getEndOffset(unit, params.getRange());
InnovationContext context = new InnovationContext(unit, start, end - start);
context.setASTRoot(astRoot);
InnovationContext context = getContext(unit, astRoot, params.getRange());
List<Diagnostic> diagnostics = params.getContext().getDiagnostics().stream().filter((d) -> {
return JavaLanguageServerPlugin.SERVER_SOURCE_ID.equals(d.getSource());
}).collect(Collectors.toList());
Expand Down Expand Up @@ -335,6 +333,14 @@ public static CompilationUnit getASTRoot(ICompilationUnit unit, IProgressMonitor
return CoreASTProvider.getInstance().getAST(unit, CoreASTProvider.WAIT_YES, monitor);
}

public static InnovationContext getContext(ICompilationUnit unit, CompilationUnit astRoot, Range range) {
int start = DiagnosticsHelper.getStartOffset(unit, range);
int end = DiagnosticsHelper.getEndOffset(unit, range);
InnovationContext context = new InnovationContext(unit, start, end - start);
context.setASTRoot(astRoot);
return context;
}

private static class ChangeCorrectionProposalComparator implements Comparator<ChangeCorrectionProposal> {

@Override
Expand Down
52 changes: 49 additions & 3 deletions ...dt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateConstructorsHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -23,13 +24,15 @@
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IField;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IType;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.FieldDeclaration;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
Expand All @@ -44,9 +47,11 @@
import org.eclipse.jdt.internal.corext.dom.Bindings;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.corrections.DiagnosticsHelper;
import org.eclipse.jdt.ls.core.internal.corrections.InnovationContext;
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspMethodBinding;
import org.eclipse.jdt.ls.core.internal.handlers.JdtDomModels.LspVariableBinding;
import org.eclipse.jdt.ls.core.internal.preferences.Preferences;
import org.eclipse.jdt.ls.core.internal.text.correction.QuickAssistProcessor;
import org.eclipse.jdt.ls.core.internal.text.correction.SourceAssistProcessor;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.Range;
Expand All @@ -62,15 +67,20 @@ public static CheckConstructorsResponse checkConstructorsStatus(CodeActionParams

public static CheckConstructorsResponse checkConstructorsStatus(CodeActionParams params, IProgressMonitor monitor) {
IType type = SourceAssistProcessor.getSelectionType(params, monitor);
return checkConstructorStatus(type, monitor);
return checkConstructorStatus(type, params.getRange(), monitor);
}

public static CheckConstructorsResponse checkConstructorStatus(IType type, IProgressMonitor monitor) {
public static CheckConstructorsResponse checkConstructorStatus(IType type, Range range, IProgressMonitor monitor) {
if (type == null || type.getCompilationUnit() == null) {
return new CheckConstructorsResponse();
}

try {
ICompilationUnit compilationUnit = type.getCompilationUnit();
if (compilationUnit == null) {
return new CheckConstructorsResponse();
}

CompilationUnit astRoot = CoreASTProvider.getInstance().getAST(type.getCompilationUnit(), CoreASTProvider.WAIT_YES, monitor);
if (astRoot == null) {
return new CheckConstructorsResponse();
Expand Down Expand Up @@ -108,10 +118,11 @@ public static CheckConstructorsResponse checkConstructorStatus(IType type, IProg
}
}

List<String> fieldNames = getFieldNames(compilationUnit, astRoot, range);
//@formatter:off
return new CheckConstructorsResponse(
Arrays.stream(superConstructors).map(binding -> new LspMethodBinding(binding)).toArray(LspMethodBinding[]::new),
fields.stream().map(binding -> new LspVariableBinding(binding)).toArray(LspVariableBinding[]::new)
fields.stream().map(binding -> fieldNames.contains(binding.getName()) ? new LspVariableBinding(binding, true) : new LspVariableBinding(binding)).toArray(LspVariableBinding[]::new)
);
//@formatter:on
} catch (JavaModelException e) {
Expand Down Expand Up @@ -210,6 +221,41 @@ private static boolean compareConstructor(IMethodBinding binding, LspMethodBindi
return Arrays.equals(parameters, lspBinding.parameters);
}

private static List<String> getFieldNames(ICompilationUnit unit, CompilationUnit astRoot, Range range) {
if (range == null) {
return Collections.emptyList();
}
InnovationContext context = CodeActionHandler.getContext(unit, astRoot, range);
ArrayList<ASTNode> coveredNodes = QuickAssistProcessor.getFullyCoveredNodes(context, context.getCoveringNode());
List<String> names = new ArrayList<>();
// add covered names
coveredNodes.forEach(coveredNode -> {
if (coveredNode instanceof FieldDeclaration) {
String name = SourceAssistProcessor.getFieldName((FieldDeclaration) coveredNode, coveredNode);
if (name != null) {
names.add(name);
}
} else if (coveredNode instanceof VariableDeclarationFragment) {
String name = SourceAssistProcessor.getFieldName((VariableDeclarationFragment) coveredNode);
if (name != null) {
names.add(name);
}
}
});
// add covering name
ASTNode coveringNode = context.getCoveringNode();
if (coveringNode != null) {
FieldDeclaration fieldDeclaration = SourceAssistProcessor.getFieldDeclarationNode(coveringNode);
if (fieldDeclaration != null) {
String name = SourceAssistProcessor.getFieldName(fieldDeclaration, coveringNode);
if (name != null) {
names.add(name);
}
}
}
return names;
}

public static class CheckConstructorsResponse {
public LspMethodBinding[] constructors = new LspMethodBinding[0];
public LspVariableBinding[] fields = new LspVariableBinding[0];
Expand Down
10 changes: 10 additions & 0 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JdtDomModels.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,22 @@ public static class LspVariableBinding {
public String name;
public String type;
public boolean isField;
public boolean isSelected;

public LspVariableBinding(IVariableBinding binding) {
this.bindingKey = binding.getKey();
this.name = binding.getName();
this.type = binding.getType().getName();
this.isField = binding.isField();
this.isSelected = false;
}

public LspVariableBinding(IVariableBinding binding, boolean isSelected) {
this.bindingKey = binding.getKey();
this.name = binding.getName();
this.type = binding.getType().getName();
this.isField = binding.isField();
this.isSelected = isSelected;
}
}

Expand Down
13 changes: 10 additions & 3 deletions ...t.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/SourceAssistProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParam
return $;
}

private String getFieldName(FieldDeclaration fieldDeclaration, ASTNode node) {
public static String getFieldName(FieldDeclaration fieldDeclaration, ASTNode node) {
if (fieldDeclaration == null || node == null) {
return null;
}
Expand All @@ -253,6 +253,13 @@ private String getFieldName(FieldDeclaration fieldDeclaration, ASTNode node) {
return null;
}

public static String getFieldName(VariableDeclarationFragment fragment) {
if (fragment == null) {
return null;
}
return fragment.getName().getIdentifier();
}

private void addGenerateAccessorsSourceActionCommand(CodeActionParams params, IInvocationContext context, List<Either<Command, CodeAction>> $, IType type, String fieldName, boolean isInTypeDeclaration) throws JavaModelException {
AccessorField[] accessors = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.BOTH);
AccessorField[] getters = GenerateGetterSetterOperation.getUnimplementedAccessors(type, AccessorKind.GETTER);
Expand Down Expand Up @@ -502,7 +509,7 @@ private Optional<Either<Command, CodeAction>> getGenerateConstructorsAction(Code
}

if (preferenceManager.getClientPreferences().isGenerateConstructorsPromptSupported()) {
CheckConstructorsResponse status = GenerateConstructorsHandler.checkConstructorStatus(type, monitor);
CheckConstructorsResponse status = GenerateConstructorsHandler.checkConstructorStatus(type, params.getRange(), monitor);
if (status.constructors.length == 0) {
return Optional.empty();
}
Expand Down Expand Up @@ -723,7 +730,7 @@ private static ASTNode getImportDeclarationNode(ASTNode node) {
return node instanceof ImportDeclaration ? node : null;
}

private static FieldDeclaration getFieldDeclarationNode(ASTNode node) {
public static FieldDeclaration getFieldDeclarationNode(ASTNode node) {
if (node == null) {
return null;
}
Expand Down
2 changes: 2 additions & 0 deletions ....tests/src/org/eclipse/jdt/ls/core/internal/handlers/GenerateConstructorsHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ public void testCheckConstructorStatus() throws JavaModelException {
assertEquals(2, response.fields.length);
assertEquals("instance", response.fields[0].name);
assertEquals("String", response.fields[0].type);
assertEquals(false, response.fields[0].isSelected);
assertEquals("address", response.fields[1].name);
assertEquals("String", response.fields[1].type);
assertEquals(true, response.fields[1].isSelected);
}

@Test
Expand Down

0 comments on commit b7b3190

Please sign in to comment.