Skip to content

Commit

Permalink
Fix for "completion overwrites" and "toggle insert" (aka Ctrl+Enter)
Browse files Browse the repository at this point in the history 
#471
  • Loading branch information
@eric-milles
eric-milles committed Nov 27, 2018
1 parent a3c04d3 commit b32cee2
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 46 deletions.
36 changes: 29 additions & 7 deletions ...n.test/src/org/codehaus/groovy/eclipse/codeassist/tests/ConstructorCompletionTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.codehaus.groovy.eclipse.codeassist.tests

import static org.eclipse.jdt.internal.ui.text.java.AbstractJavaCompletionProposal.MODIFIER_TOGGLE_COMPLETION_MODE

import groovy.transform.NotYetImplemented

import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist
Expand Down Expand Up @@ -67,30 +69,50 @@ final class ConstructorCompletionTests extends CompletionTestSuite {
checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY')
}

@Test @NotYetImplemented // do this with Ctrl triggering of constructor proposal
@Test
void testConstructorCompletion3() {
String contents = 'class YYY { YYY() {} }\nnew YY()\nkkk' // trailing parens
String expected = 'class YYY { YYY() {} }\nnew YYY()\nkkk'
String contents = 'class YYY { YYY(x) {} }\nnew YY\nkkk'
String expected = 'class YYY { YYY(x) {} }\nnew YYY(x)\nkkk'
setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false')
checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY')
}

@Test
void testConstructorCompletion4() {
String contents = 'class YYY { YYY(x) {} }\nnew YY\nkkk'
String expected = 'class YYY { YYY(x) {} }\nnew YYY(x)\nkkk'
String contents = 'class YYY { YYY(x, y) {} }\nnew YY\nkkk'
String expected = 'class YYY { YYY(x, y) {} }\nnew YYY(x, y)\nkkk'
setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false')
checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY')
}

@Test
void testConstructorCompletion5() {
String contents = 'class YYY { YYY(x, y) {} }\nnew YY\nkkk'
String expected = 'class YYY { YYY(x, y) {} }\nnew YYY(x, y)\nkkk'
String contents = 'class YYY { YYY() {} }\nnew YY\nkkk'
String expected = 'class YYY { YYY() {} }\nnew YYY()\nkkk'
setJavaPreference(PreferenceConstants.CODEASSIST_INSERT_COMPLETION, 'false')
setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false')
checkProposalApplicationNonType(contents, expected, getIndexOf(contents, 'new YY'), 'YYY')
}

@Test
void testConstructorCompletion6() {
String contents = 'class YYY { YYY() {} }\nnew YY()\nkkk'
String expected = 'class YYY { YYY() {} }\nnew YYY()\nkkk'
setJavaPreference(PreferenceConstants.CODEASSIST_INSERT_COMPLETION, 'false')
setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false')
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getIndexOf(contents, 'new YY'))
applyProposalAndCheck(findFirstProposal(proposals, 'YYY', false), expected) // completion overwrites
}

@Test // https://github.com/groovy/groovy-eclipse/issues/471
void testConstructorCompletion7() {
String contents = 'class YYY { YYY() {} }\nnew YY()\nkkk'
String expected = 'class YYY { YYY() {} }\nnew YYY()\nkkk'
setJavaPreference(PreferenceConstants.CODEASSIST_GUESS_METHOD_ARGUMENTS, 'false')
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getIndexOf(contents, 'new YY'))
applyProposalAndCheck(findFirstProposal(proposals, 'YYY', false), expected, 0 as char, MODIFIER_TOGGLE_COMPLETION_MODE)
}

@Test
void testContructorCompletionWithClosure1() {
String contents = '''\
Expand Down
23 changes: 8 additions & 15 deletions ...rg/codehaus/groovy/eclipse/codeassist/completions/GroovyJavaMethodCompletionProposal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,29 +190,16 @@ protected String computeReplacementString() {
return "";
}

char[] proposalName = fProposal.getName();
boolean hasWhitespace = ProposalUtils.hasWhitespace(proposalName);

if (fProposal.getKind() == CompletionProposal.METHOD_NAME_REFERENCE) {
// complete the name only for a method pointer or static import expression
return String.valueOf(!hasWhitespace ? proposalName : CharOperation.concat('"', proposalName, '"'));
}

// if no whitespace in the method name and no arguments, there is nothing groovy to do
if (!hasWhitespace && (!hasParameters() || !hasArgumentList())) {
if (!hasArgumentList() || !hasParameters()) {
String replacementString = super.computeReplacementString();
if (replacementString.endsWith(RPAREN + SEMICOLON)) {
while (replacementString.endsWith(SEMICOLON)) {
replacementString = replacementString.substring(0, replacementString.length() - 1);
}
return replacementString;
}

//
StringBuffer buffer = new StringBuffer();

fProposal.setName(!hasWhitespace ? proposalName : CharOperation.concat('"', proposalName, '"'));
appendMethodNameReplacement(buffer);
fProposal.setName(proposalName);

if (!hasParameters()) {
while (Character.isWhitespace(buffer.charAt(buffer.length() - 1))) {
Expand Down Expand Up @@ -463,6 +450,12 @@ public int getContextInformationPosition() {
return fContextInformationPosition;
}

@Override // called by isPrefix
protected int getPatternMatchRule(String pattern, String string) {
if (string.charAt(0) == '"') string = string.substring(1);
return super.getPatternMatchRule(pattern, string);
}

@Override
public Point getSelection(IDocument document) {
if (fSelectedRegion != null) {
Expand Down
9 changes: 7 additions & 2 deletions .../org/codehaus/groovy/eclipse/codeassist/processors/GroovyProposalTypeSearchRequestor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
public class GroovyProposalTypeSearchRequestor implements ISearchRequestor {

private static final char[] NO_TYPE_NAME = {'.'};
private static final char[] EMPTY_PARENS = {'(', ')'};
private static final char[] _AS_ = {' ', 'a', 's', ' '};

private static final int CHECK_CANCEL_FREQUENCY = 50;
Expand Down Expand Up @@ -603,7 +604,11 @@ private ICompletionProposal proposeConstructor(AcceptedCtor ctor) {
proposal.setCompletion(CharOperation.NO_CHAR);
proposal.setReplaceRange(context.completionLocation, context.completionLocation);
} else {
proposal.setCompletion(new char[] {'(', ')'});
if (context.isParenAfter(javaContext.getDocument())) {
proposal.setCompletion(CharOperation.NO_CHAR);
} else {
proposal.setCompletion(EMPTY_PARENS);
}
proposal.setTokenRange(offset, context.completionEnd);
proposal.setReplaceRange(context.completionEnd, context.completionEnd);

Expand Down Expand Up @@ -652,7 +657,7 @@ private ICompletionProposal proposeConstructor(AcceptedCtor ctor) {
populateParameterInfo(proposal, ctor.parameterCount, ctor.parameterNames, ctor.parameterTypes, ctor.signature);

// TODO: Leverage IRelevanceRule for this?
float relevanceMultiplier = (ctor.accessibility == IAccessRule.K_ACCESSIBLE) ? 3 : 0;
float relevanceMultiplier = (ctor.accessibility == IAccessRule.K_ACCESSIBLE ? 3 : 0);
relevanceMultiplier += computeRelevanceForCaseMatching(completionExpressionChars, ctor.simpleTypeName);
proposal.setRelevance(Relevance.MEDIUM_HIGH.getRelevance(relevanceMultiplier));

Expand Down
40 changes: 18 additions & 22 deletions ...completion/src/org/codehaus/groovy/eclipse/codeassist/proposals/GroovyMethodProposal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist;
import org.codehaus.groovy.eclipse.codeassist.ProposalUtils;
import org.codehaus.groovy.eclipse.codeassist.completions.GroovyJavaMethodCompletionProposal;
import org.codehaus.groovy.eclipse.codeassist.completions.NamedArgsMethodNode;
Expand All @@ -37,11 +36,11 @@
import org.eclipse.jdt.groovy.core.util.GroovyUtils;
import org.eclipse.jdt.internal.codeassist.CompletionEngine;
import org.eclipse.jdt.internal.codeassist.impl.AssistOptions;
import org.eclipse.jdt.internal.core.nd.util.CharArrayUtils;
import org.eclipse.jdt.internal.ui.text.java.AnnotationAtttributeProposalInfo;
import org.eclipse.jdt.internal.ui.text.java.LazyJavaCompletionProposal;
import org.eclipse.jdt.ui.text.java.IJavaCompletionProposal;
import org.eclipse.jdt.ui.text.java.JavaContentAssistInvocationContext;
import org.eclipse.jface.text.BadLocationException;

public class GroovyMethodProposal extends AbstractGroovyProposal {

Expand Down Expand Up @@ -113,13 +112,21 @@ public IJavaCompletionProposal createJavaProposal(CompletionEngine engine, Conte
if (context.location == ContentAssistLocation.METHOD_CONTEXT) {
proposal.setCompletion(CharOperation.NO_CHAR);
proposal.setReplaceRange(context.completionLocation, context.completionLocation);
} else { // this is a normal method proposal
boolean parens = (kind == CompletionProposal.ANNOTATION_ATTRIBUTE_REF ? false : !isParens(context, javaContext));
} else {
boolean parens;
switch (kind) {
case CompletionProposal.ANNOTATION_ATTRIBUTE_REF:
case CompletionProposal.METHOD_NAME_REFERENCE:
parens = false;
break;
default:
parens = !context.isParenAfter(javaContext.getDocument());
}
proposal.setCompletion(completionName(parens));
proposal.setReplaceRange(context.completionLocation - context.completionExpression.length(), context.completionEnd);
}
proposal.setDeclarationSignature(ProposalUtils.createTypeSignature(method.getDeclaringClass()));
proposal.setName(method.getName().toCharArray());
proposal.setName(completionName(false));
if (method instanceof NamedArgsMethodNode) {
fillInExtraParameters((NamedArgsMethodNode) method, proposal);
} else {
Expand Down Expand Up @@ -198,27 +205,16 @@ private void fillInExtraParameters(NamedArgsMethodNode namedArgsMethod, GroovyCo
proposal.setOptionalParameterTypeNames(getParameterTypeNames(namedArgsMethod.getOptionalParams()));
}

private boolean isParens(ContentAssistContext context, JavaContentAssistInvocationContext javaContext) {
if (javaContext.getDocument().getLength() > context.completionEnd) {
try {
return javaContext.getDocument().getChar(context.completionEnd) == '(';
} catch (BadLocationException e) {
GroovyContentAssist.logError("Exception during content assist", e);
}
}
return false;
}

protected char[] completionName(boolean includeParens) {
String name = method.getName();
char[] nameArr = name.toCharArray();
if (ProposalUtils.hasWhitespace(nameArr)) {
name = '"' + name + '"';
StringBuilder name = new StringBuilder(method.getName());
if (name.chars().anyMatch(Character::isWhitespace)) {
name.insert(0, '"');
name.append('"');
}
if (includeParens) {
name += "()";
name.append("()");
}
return name.toCharArray();
return CharArrayUtils.extractChars(name);
}

protected int getModifiers() {
Expand Down
16 changes: 16 additions & 0 deletions ...completion/src/org/codehaus/groovy/eclipse/codeassist/requestor/ContentAssistContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.eclipse.jdt.groovy.search.VariableScope;
import org.eclipse.jdt.internal.codeassist.InternalCompletionContext;
import org.eclipse.jdt.ui.PreferenceConstants;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.IDocument;

public class ContentAssistContext {

Expand Down Expand Up @@ -219,4 +221,18 @@ public VariableScope getPerceivedCompletionScope() {
}
return currentScope;
}

/**
* Determines if right paren comes after completion range.
*/
public boolean isParenAfter(IDocument document) {
if (document != null && document.getLength() > completionEnd) {
try {
return ('(' == document.getChar(completionEnd));
} catch (BadLocationException e) {
GroovyContentAssist.logError("Exception during content assist", e);
}
}
return false;
}
}

0 comments on commit b32cee2

Please sign in to comment.