Skip to content

Commit

Permalink
Resolve review feedback.
Browse files Browse the repository at this point in the history 
Signed-off-by: Yaohai Zheng <[email protected]>
  • Loading branch information
@yaohaizh
yaohaizh committed Jan 19, 2019
1 parent ccd1b9a commit 51428ea
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 63 deletions.
25 changes: 19 additions & 6 deletions ....core/src/org/eclipse/jdt/ls/core/internal/codemanipulation/OverrideMethodsOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
package org.eclipse.jdt.ls.core.internal.codemanipulation;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -52,9 +51,9 @@

public class OverrideMethodsOperation {

public static List<OverridableMethod> getOverridableMethods(IType type) {
public static OverridableMethodsResponse getOverridableMethods(IType type) {
if (type == null || type.getCompilationUnit() == null) {
return Collections.emptyList();
return new OverridableMethodsResponse();
}

List<OverridableMethod> overridables = new ArrayList<>();
Expand All @@ -63,7 +62,7 @@ public static List<OverridableMethod> getOverridableMethods(IType type) {
try {
ITypeBinding typeBinding = ASTNodes.getTypeBinding(astRoot, type);
if (typeBinding == null) {
return overridables;
return new OverridableMethodsResponse();
}

IMethodBinding cloneMethod = null;
Expand All @@ -84,11 +83,11 @@ public static List<OverridableMethod> getOverridableMethods(IType type) {
JavaLanguageServerPlugin.logException("List overridable methods", e);
}

return overridables;
return new OverridableMethodsResponse(type.getElementName(), overridables);
}

public static TextEdit addOverridableMethods(IType type, OverridableMethod[] overridableMethods) {
if (type == null || type.getCompilationUnit() == null) {
if (type == null || type.getCompilationUnit() == null || overridableMethods == null || overridableMethods.length == 0) {
return null;
}

Expand Down Expand Up @@ -185,6 +184,20 @@ private static String[] getMethodParameterTypes(IMethodBinding binding, boolean
return parameterTypes.toArray(new String[0]);
}

public static class OverridableMethodsResponse {
public String type;
public List<OverridableMethod> methods;

public OverridableMethodsResponse() {

}

public OverridableMethodsResponse(String typeName, List<OverridableMethod> methods) {
this.type = typeName;
this.methods = methods;
}
}

public static class OverridableMethod {
public String bindingKey;
public String name;
Expand Down
4 changes: 2 additions & 2 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
import org.eclipse.jdt.ls.core.internal.JobHelpers;
import org.eclipse.jdt.ls.core.internal.LanguageServerWorkingCopyOwner;
import org.eclipse.jdt.ls.core.internal.ServiceStatus;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation.OverridableMethod;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation.OverridableMethodsResponse;
import org.eclipse.jdt.ls.core.internal.handlers.OverrideMethodsHandler.AddOverridableMethodParams;
import org.eclipse.jdt.ls.core.internal.lsp.JavaProtocolExtensions;
import org.eclipse.jdt.ls.core.internal.managers.ContentProviderManager;
Expand Down Expand Up @@ -762,7 +762,7 @@ public CompletableFuture<List<? extends Location>> implementation(TextDocumentPo
}

@Override
public CompletableFuture<List<OverridableMethod>> overridableMethods(CodeActionParams params) {
public CompletableFuture<OverridableMethodsResponse> overridableMethods(CodeActionParams params) {
logInfo(">> java/overridableMethods");
return computeAsync((monitor) -> OverrideMethodsHandler.getOverridableMethods(params));
}
Expand Down
5 changes: 2 additions & 3 deletions ...pse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/OverrideMethodsHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

package org.eclipse.jdt.ls.core.internal.handlers;

import java.util.List;

import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IType;
Expand All @@ -21,6 +19,7 @@
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation.OverridableMethod;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation.OverridableMethodsResponse;
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.text.correction.SourceAssistProcessor;
Expand All @@ -30,7 +29,7 @@

public class OverrideMethodsHandler {

public static List<OverridableMethod> getOverridableMethods(CodeActionParams params) {
public static OverridableMethodsResponse getOverridableMethods(CodeActionParams params) {
IType type = getSelectionType(params);
return OverrideMethodsOperation.getOverridableMethods(type);
}
Expand Down
5 changes: 2 additions & 3 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/JavaProtocolExtensions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.lsp;

import java.util.List;
import java.util.concurrent.CompletableFuture;

import org.eclipse.jdt.ls.core.internal.BuildWorkspaceStatus;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation.OverridableMethod;
import org.eclipse.jdt.ls.core.internal.codemanipulation.OverrideMethodsOperation.OverridableMethodsResponse;
import org.eclipse.jdt.ls.core.internal.handlers.OverrideMethodsHandler.AddOverridableMethodParams;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.lsp4j.TextDocumentIdentifier;
Expand Down Expand Up @@ -46,7 +45,7 @@ public interface JavaProtocolExtensions {
CompletableFuture<BuildWorkspaceStatus> buildWorkspace(boolean forceReBuild);

@JsonRequest
CompletableFuture<List<OverridableMethod>> overridableMethods(CodeActionParams params);
CompletableFuture<OverridableMethodsResponse> overridableMethods(CodeActionParams params);

@JsonRequest
CompletableFuture<WorkspaceEdit> addOverridableMethods(AddOverridableMethodParams params);
Expand Down
4 changes: 4 additions & 0 deletions ...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ public boolean isClassFileContentSupported() {
return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("classFileContentsSupport", "false").toString());
}

public boolean isOverrideMethodsPromptSupported() {
return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("overrideMethodsPromptSupport", "false").toString());
}

public boolean isSupportsCompletionDocumentationMarkdown() {
//@formatter:off
return v3supported && capabilities.getTextDocument().getCompletion() != null
Expand Down
25 changes: 18 additions & 7 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 @@ -15,6 +15,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.OperationCanceledException;
Expand All @@ -28,7 +29,6 @@
import org.eclipse.jdt.core.dom.TypeDeclaration;
import org.eclipse.jdt.core.manipulation.OrganizeImportsOperation;
import org.eclipse.jdt.internal.ui.text.correction.IProblemLocationCore;
import org.eclipse.jdt.ls.core.internal.ChangeUtil;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaCodeActionKind;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
Expand All @@ -47,9 +47,15 @@
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.eclipse.text.edits.TextEdit;

import com.google.common.collect.Sets;

public class SourceAssistProcessor {

private static final Set<String> UNSUPPORTED_RESOURCES = Sets.newHashSet("module-info.java", "package-info.java");

public static final String COMMAND_ID_ACTION_OVERRIDEMETHODSPROMPT = "java.action.overrideMethodsPrompt";


private PreferenceManager preferenceManager;

public SourceAssistProcessor(PreferenceManager preferenceManager) {
Expand All @@ -59,15 +65,19 @@ public SourceAssistProcessor(PreferenceManager preferenceManager) {
public List<Either<Command, CodeAction>> getSourceActionCommands(CodeActionParams params, IInvocationContext context, IProblemLocationCore[] locations) {
List<Either<Command, CodeAction>> $ = new ArrayList<>();

ICompilationUnit cu = context.getCompilationUnit();

// Organize Imports
TextEdit organizeImportsEdit = getOrganizeImportsProposal(context);
Optional<Either<Command, CodeAction>> organizeImports = convertToWorkspaceEditAction(params.getContext(), context.getCompilationUnit(), CorrectionMessages.ReorgCorrectionsSubProcessor_organizeimports_description,
CodeActionKind.SourceOrganizeImports, organizeImportsEdit);
addSourceActionCommand($, params.getContext(), organizeImports);

// Override/Implement Methods
Optional<Either<Command, CodeAction>> overrideMethods = getOverrideMethodsAction(params);
addSourceActionCommand($, params.getContext(), overrideMethods);
if (!UNSUPPORTED_RESOURCES.contains(cu.getResource().getName())) {
// Override/Implement Methods
Optional<Either<Command, CodeAction>> overrideMethods = getOverrideMethodsAction(params);
addSourceActionCommand($, params.getContext(), overrideMethods);
}

// Generate Getter and Setter
TextEdit getterSetterEdit = getGetterSetterProposal(context);
Expand Down Expand Up @@ -109,6 +119,10 @@ private TextEdit getOrganizeImportsProposal(IInvocationContext context) {
}

private Optional<Either<Command, CodeAction>> getOverrideMethodsAction(CodeActionParams params) {
if (!preferenceManager.getClientPreferences().isOverrideMethodsPromptSupported()) {
return Optional.empty();
}

Command command = new Command(ActionMessages.OverrideMethodsAction_label, COMMAND_ID_ACTION_OVERRIDEMETHODSPROMPT, Collections.singletonList(params));
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(JavaCodeActionKind.SOURCE_OVERRIDE_METHODS)) {
CodeAction codeAction = new CodeAction(ActionMessages.OverrideMethodsAction_label);
Expand Down Expand Up @@ -144,9 +158,6 @@ private TextEdit getGetterSetterProposal(IInvocationContext context) {

private Optional<Either<Command, CodeAction>> convertToWorkspaceEditAction(CodeActionContext context, ICompilationUnit cu, String name, String kind, TextEdit edit) {
WorkspaceEdit workspaceEdit = convertToWorkspaceEdit(cu, edit);
if (!ChangeUtil.hasChanges(workspaceEdit)) {
return Optional.empty();
}

Command command = new Command(name, CodeActionHandler.COMMAND_ID_APPLY_EDIT, Collections.singletonList(workspaceEdit));
if (preferenceManager.getClientPreferences().isSupportedCodeActionKind(kind)) {
Expand Down
Loading

0 comments on commit 51428ea

Please sign in to comment.