Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interface/class snippets should not be available inside method bodies #689

Merged
merged 1 commit into from
Jun 12, 2018
Merged

interface/class snippets should not be available inside method bodies #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ....core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalRequestor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public final class CompletionProposalRequestor extends CompletionRequestor {
private CompletionProposalDescriptionProvider descriptionProvider;
private CompletionResponse response;
private boolean fIsTestCodeExcluded;
private CompletionContext context;

// Update SUPPORTED_KINDS when mapKind changes
// @formatter:off
Expand Down Expand Up @@ -130,6 +131,7 @@ public CompletionItem toCompletionItem(CompletionProposal proposal, int index) {
@Override
public void acceptContext(CompletionContext context) {
super.acceptContext(context);
this.context = context;
response.setContext(context);
this.descriptionProvider = new CompletionProposalDescriptionProvider(context);
}
Expand Down Expand Up @@ -218,4 +220,8 @@ public boolean isTestCodeExcluded() {
return fIsTestCodeExcluded;
}

public CompletionContext getContext() {
return context;
}

}
168 changes: 157 additions & 11 deletions ...ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/SnippetCompletionProposal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,26 @@
import java.util.Set;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.CompletionContext;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IMethod;
import org.eclipse.jdt.core.compiler.ITerminalSymbols;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.ExpressionStatement;
import org.eclipse.jdt.core.dom.Initializer;
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
import org.eclipse.jdt.internal.codeassist.InternalCompletionContext;
import org.eclipse.jdt.internal.codeassist.complete.CompletionOnFieldType;
import org.eclipse.jdt.internal.codeassist.complete.CompletionOnKeyword2;
import org.eclipse.jdt.internal.codeassist.complete.CompletionOnSingleNameReference;
import org.eclipse.jdt.internal.compiler.ast.ASTNode;
import org.eclipse.jdt.internal.corext.dom.TokenScanner;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.corext.codemanipulation.StubUtility;
import org.eclipse.jdt.ls.core.internal.corext.refactoring.structure.ASTNodeSearchUtil;
import org.eclipse.jdt.ls.core.internal.handlers.CompletionResolveHandler;
import org.eclipse.jdt.ls.core.internal.preferences.CodeGenerationTemplate;
import org.eclipse.lsp4j.CompletionItem;
Expand All @@ -33,36 +49,156 @@
public class SnippetCompletionProposal {
private static final String CLASS_SNIPPET_LABEL = "class";
private static final String INTERFACE_SNIPPET_LABEL = "interface";
private static final String CLASS_KEYWORD = "class";
private static final String INTERFACE_KEYWORD = "interface";
private static final Set<String> UNSUPPORTED_RESOURCES = Sets.newHashSet("module-info.java", "package-info.java");

public static List<CompletionItem> getSnippets(ICompilationUnit cu) {
public static List<CompletionItem> getSnippets(ICompilationUnit cu, CompletionContext completionContext, IProgressMonitor monitor) {
if (cu == null) {
throw new IllegalArgumentException("Compilation unit must not be null"); //$NON-NLS-1$
}
char[] completionToken = completionContext.getToken();
boolean isInterfacePrefix = true;
boolean isClassPrefix = true;
if (completionToken != null && completionToken.length > 0) {
String prefix = new String(completionToken);
isInterfacePrefix = INTERFACE_KEYWORD.startsWith(prefix);
isClassPrefix = CLASS_KEYWORD.startsWith(prefix);
}
if (!isInterfacePrefix && !isClassPrefix) {
return Collections.emptyList();
}
if (monitor == null) {
monitor = new NullProgressMonitor();
}
//This check might need to be pushed back to the different get*Snippet() methods, depending on future features
if (UNSUPPORTED_RESOURCES.contains(cu.getResource().getName())) {
return Collections.emptyList();
}
boolean needsPublic = needsPublic(cu, completionContext, monitor);
if (monitor.isCanceled()) {
return Collections.emptyList();
}
List<CompletionItem> res = new ArrayList<>(2);
CompletionItem classSnippet = getClassSnippet(cu);
if (classSnippet != null) {
res.add(classSnippet);
if (isClassPrefix) {
CompletionItem classSnippet = getClassSnippet(cu, completionContext, needsPublic, monitor);
if (classSnippet != null) {
res.add(classSnippet);
}
}
CompletionItem interfaceSnippet = getInterfaceSnippet(cu);
if (interfaceSnippet != null) {
res.add(interfaceSnippet);
if (isInterfacePrefix) {
CompletionItem interfaceSnippet = getInterfaceSnippet(cu, completionContext, needsPublic, monitor);
if (interfaceSnippet != null) {
res.add(interfaceSnippet);
}
}
return res;
}

private static CompletionItem getClassSnippet(ICompilationUnit cu) {
private static boolean accept(ICompilationUnit cu, CompletionContext completionContext, boolean acceptClass) {
if (completionContext != null && completionContext.isExtended()) {
if (completionContext.isInJavadoc()) {
return false;
}
if (completionContext instanceof InternalCompletionContext) {
InternalCompletionContext internalCompletionContext = (InternalCompletionContext) completionContext;
ASTNode node = internalCompletionContext.getCompletionNode();
if (node instanceof CompletionOnKeyword2) {
return true;
}
if (node instanceof CompletionOnFieldType) {
return true;
}
if (acceptClass && node instanceof CompletionOnSingleNameReference) {
if (completionContext.getEnclosingElement() instanceof IMethod) {
CompilationUnit ast = CoreASTProvider.getInstance().getAST(cu, CoreASTProvider.WAIT_YES, null);
org.eclipse.jdt.core.dom.ASTNode astNode = ASTNodeSearchUtil.getAstNode(ast, completionContext.getTokenStart(), completionContext.getTokenEnd() - completionContext.getTokenStart() + 1);
return (astNode == null || (astNode.getParent() instanceof ExpressionStatement));
}
return true;
}
}
}
return false;
}

private static boolean needsPublic(ICompilationUnit cu, CompletionContext completionContext, IProgressMonitor monitor) {
if (completionContext != null && completionContext.isExtended()) {
if (completionContext.isInJavadoc()) {
return false;
}
if (completionContext instanceof InternalCompletionContext) {
InternalCompletionContext internalCompletionContext = (InternalCompletionContext) completionContext;
ASTNode node = internalCompletionContext.getCompletionNode();
if (node instanceof CompletionOnKeyword2 || node instanceof CompletionOnFieldType || node instanceof CompletionOnSingleNameReference) {
if (completionContext.getEnclosingElement() instanceof IMethod) {
return false;
}
try {
TokenScanner scanner = new TokenScanner(cu);
int curr = scanner.readNext(0, true);
int previous = curr;
while (scanner.getCurrentEndOffset() < completionContext.getTokenStart()) {
previous = curr;
if (monitor.isCanceled()) {
return false;
}
if (curr == ITerminalSymbols.TokenNameEOF) {
break;
}
try {
curr = scanner.readNext(true);
} catch (CoreException e) {
// ignore
}
}
if (scanner.isModifier(previous)) {
return false;
}
} catch (CoreException e) {
JavaLanguageServerPlugin.logException(e.getMessage(), e);
}
if (node instanceof CompletionOnSingleNameReference) {
CompilationUnit ast = CoreASTProvider.getInstance().getAST(cu, CoreASTProvider.WAIT_YES, null);
if (monitor.isCanceled()) {
return false;
}
org.eclipse.jdt.core.dom.ASTNode astNode = ASTNodeSearchUtil.getAstNode(ast, completionContext.getOffset(), 1);
if (astNode == null) {
return false;
}
while (astNode != null) {
if (astNode instanceof Initializer) {
return false;
}
astNode = astNode.getParent();
}
}
return true;
}
}
}
return false;
}

private static CompletionItem getClassSnippet(ICompilationUnit cu, CompletionContext completionContext, boolean needsPublic, IProgressMonitor monitor) {
if (!accept(cu, completionContext, true)) {
return null;
}
if (monitor.isCanceled()) {
return null;
}
final CompletionItem classSnippetItem = new CompletionItem();
classSnippetItem.setLabel(CLASS_SNIPPET_LABEL);
classSnippetItem.setFilterText(CLASS_SNIPPET_LABEL);
classSnippetItem.setSortText(SortTextHelper.convertRelevance(1));

try {
classSnippetItem.setInsertText(StubUtility.getSnippetContent(cu, CodeGenerationTemplate.CLASSSNIPPET, cu.findRecommendedLineSeparator(), isSnippetStringSupported()));
if (needsPublic) {
classSnippetItem.setInsertText(StubUtility.getSnippetContent(cu, CodeGenerationTemplate.CLASSSNIPPET_PUBLIC, cu.findRecommendedLineSeparator(), isSnippetStringSupported()));
} else {
classSnippetItem.setInsertText(StubUtility.getSnippetContent(cu, CodeGenerationTemplate.CLASSSNIPPET_DEFAULT, cu.findRecommendedLineSeparator(), isSnippetStringSupported()));
}
setFields(classSnippetItem, cu);
} catch (CoreException e) {
JavaLanguageServerPlugin.log(e.getStatus());
Expand All @@ -71,14 +207,24 @@ private static CompletionItem getClassSnippet(ICompilationUnit cu) {
return classSnippetItem;
}

private static CompletionItem getInterfaceSnippet(ICompilationUnit cu) {
private static CompletionItem getInterfaceSnippet(ICompilationUnit cu, CompletionContext completionContext, boolean needsPublic, IProgressMonitor monitor) {
if (!accept(cu, completionContext, false)) {
return null;
}
if (monitor.isCanceled()) {
return null;
}
final CompletionItem interfaceSnippetItem = new CompletionItem();
interfaceSnippetItem.setFilterText(INTERFACE_SNIPPET_LABEL);
interfaceSnippetItem.setLabel(INTERFACE_SNIPPET_LABEL);
interfaceSnippetItem.setSortText(SortTextHelper.convertRelevance(0));

try {
interfaceSnippetItem.setInsertText(StubUtility.getSnippetContent(cu, CodeGenerationTemplate.INTERFACESNIPPET, cu.findRecommendedLineSeparator(), isSnippetStringSupported()));
if (needsPublic) {
interfaceSnippetItem.setInsertText(StubUtility.getSnippetContent(cu, CodeGenerationTemplate.INTERFACESNIPPET_PUBLIC, cu.findRecommendedLineSeparator(), isSnippetStringSupported()));
} else {
interfaceSnippetItem.setInsertText(StubUtility.getSnippetContent(cu, CodeGenerationTemplate.INTERFACESNIPPET_DEFAULT, cu.findRecommendedLineSeparator(), isSnippetStringSupported()));
}
setFields(interfaceSnippetItem, cu);
} catch (CoreException e) {
JavaLanguageServerPlugin.log(e.getStatus());
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ Either<List<CompletionItem>, CompletionList> completion(CompletionParams positio
completionItems = this.computeContentAssist(unit,
position.getPosition().getLine(),
position.getPosition().getCharacter(), monitor);
completionItems.addAll(SnippetCompletionProposal.getSnippets(unit));
} catch (OperationCanceledException ignorable) {
// No need to pollute logs when query is cancelled
monitor.setCanceled(true);
Expand Down Expand Up @@ -109,6 +108,7 @@ public boolean isCanceled() {
try {
unit.codeComplete(offset, collector, subMonitor);
proposals.addAll(collector.getCompletionItems());
proposals.addAll(SnippetCompletionProposal.getSnippets(unit, collector.getContext(), subMonitor));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem here is the proposals ignore the currently typed text, i.e if you type m in the class body, you'd still get the class and interface snippets proposals. While vscode luckily filters them out, other clients won't necessarily do it.

} catch (OperationCanceledException e) {
monitor.setCanceled(true);
}
Expand Down
33 changes: 28 additions & 5 deletions ....jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/CodeGenerationTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,23 +109,38 @@ public enum CodeGenerationTemplate {
CodeTemplateContextType.METHODBODY_CONTEXTTYPE,
CodeTemplatePreferences.CODETEMPLATE_METHODBODY_DEFAULT),

/**
* Snippet `public class` content template
*/
CLASSSNIPPET_PUBLIC(
CodeTemplatePreferences.CODETEMPLATE_CODESNIPPET,
CodeTemplateContextType.CLASSSNIPPET_CONTEXTTYPE,
CodeTemplatePreferences.CODETEMPLATE_CLASSSNIPPET_PUBLIC),

/**
* Snippet `class` content template
*/
CLASSSNIPPET(
CLASSSNIPPET_DEFAULT(
CodeTemplatePreferences.CODETEMPLATE_CODESNIPPET,
CodeTemplateContextType.CLASSSNIPPET_CONTEXTTYPE,
CodeTemplatePreferences.CODETEMPLATE_CLASSSNIPPET_DEFAULT),

/**
* Snippet `public interface` content template
*/
INTERFACESNIPPET_PUBLIC(
CodeTemplatePreferences.CODETEMPLATE_CODESNIPPET,
CodeTemplateContextType.INTERFACESNIPPET_CONTEXTTYPE,
CodeTemplatePreferences.CODETEMPLATE_INTERFACESNIPPET_PUBLIC),

/**
* Snippet `interface` content template
*/
INTERFACESNIPPET(
INTERFACESNIPPET_DEFAULT(
CodeTemplatePreferences.CODETEMPLATE_CODESNIPPET,
CodeTemplateContextType.INTERFACESNIPPET_CONTEXTTYPE,
CodeTemplatePreferences.CODETEMPLATE_INTERFACESNIPPET_DEFAULT);


private final String preferenceId;
private final String contextType;
private final String defaultContent;
Expand Down Expand Up @@ -288,10 +303,18 @@ class CodeTemplatePreferences {
/**
* Default value for class snippet body content
*/
public static final String CODETEMPLATE_CLASSSNIPPET_DEFAULT = "${package_header}/**\n * ${type_name}\n */\npublic class ${type_name} {\n\n\t${cursor}\n}";
public static final String CODETEMPLATE_CLASSSNIPPET_DEFAULT = "${package_header}class ${type_name} {\n\n\t${cursor}\n}";

/**
* Default value for public class snippet body content
*/
public static final String CODETEMPLATE_CLASSSNIPPET_PUBLIC = "${package_header}/**\n * ${type_name}\n */\npublic class ${type_name} {\n\n\t${cursor}\n}";
/**
* Default value for interface snippet body content
*/
public static final String CODETEMPLATE_INTERFACESNIPPET_DEFAULT = "${package_header}/**\n * ${type_name}\n */\npublic interface ${type_name} {\n\n\t${cursor}\n}";
public static final String CODETEMPLATE_INTERFACESNIPPET_DEFAULT = "${package_header}interface ${type_name} {\n\n\t${cursor}\n}";
/**
* Default value for public interface snippet body content
*/
public static final String CODETEMPLATE_INTERFACESNIPPET_PUBLIC = "${package_header}/**\n * ${type_name}\n */\npublic interface ${type_name} {\n\n\t${cursor}\n}";
}
Loading