From 09476c126a69812b0b1541ce91da84784ce479a9 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Thu, 19 Jan 2017 03:19:02 -0500 Subject: [PATCH] Bug 461680 - Extend content assist's function parameter hints In addition to showing the parameter list (with the current parameter bolded), the hint now shows the function's name, return type, and whether or not it is virtual. Change-Id: I24be893ee8968fca8d9893230266ec98e2b9ae5a --- .../AbstractContentAssistTest.java | 5 +- .../contentassist2/ParameterHintTests.java | 67 +++++++++++++++++++ .../ui/text/CParameterListValidator.java | 28 ++++++-- .../CProposalContextInformation.java | 29 ++++++++ .../DOMCompletionProposalComputer.java | 48 +++++++++++-- 5 files changed, 167 insertions(+), 10 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java index b055f90177c..9a5a7aac63b 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java @@ -369,7 +369,7 @@ public abstract class AbstractContentAssistTest extends BaseUITestCase { case CONTEXT: if (result instanceof ICompletionProposal) { - result = ((CCompletionProposal) result).getContextInformation(); + result = ((ICompletionProposal) result).getContextInformation(); } if (result instanceof IContextInformation) { strings[i] = ((IContextInformation) result).getContextDisplayString(); @@ -377,6 +377,9 @@ public abstract class AbstractContentAssistTest extends BaseUITestCase { break; case INFORMATION: + if (result instanceof ICompletionProposal) { + result = ((ICompletionProposal) result).getContextInformation(); + } if (result instanceof IContextInformation) { strings[i] = ((IContextInformation) result).getInformationDisplayString(); } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/ParameterHintTests.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/ParameterHintTests.java index 2ea697e792f..115c06bd008 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/ParameterHintTests.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/ParameterHintTests.java @@ -65,6 +65,10 @@ public class ParameterHintTests extends AbstractContentAssistTest { assertContentAssistResults(getBuffer().length() - 1, expected, DEFAULT_FLAGS, CompareType.CONTEXT); } + protected void assertDisplayedParameterHints(String[] expected) throws Exception { + assertContentAssistResults(getBuffer().length() - 1, expected, DEFAULT_FLAGS, CompareType.INFORMATION); + } + //void foo(){aFunc( public void testFunction() throws Exception { assertParameterHints(new String[] { @@ -194,4 +198,67 @@ public class ParameterHintTests extends AbstractContentAssistTest { setCommaAfterFunctionParameter(CCorePlugin.INSERT); assertParameterHints(new String[] { "foo(int i, int j) : void" }); } + + // void foo(int x, int y); + // int main() { foo( + public void testFunction_461680() throws Exception { + assertDisplayedParameterHints(new String[] { + "void foo(int x, int y)" + }); + } + + // struct C { + // void foo(int arg); + // }; + // void caller(C c) { c.foo( + public void testMethod_461680() throws Exception { + assertDisplayedParameterHints(new String[] { + "void C::foo(int arg)" + }); + } + + // struct Base { + // void foo(int arg); + // }; + // struct Derived : Base {}; + // void caller(Derived* d) { d->foo( + public void testNonOverriddenMethod_461680() throws Exception { + assertDisplayedParameterHints(new String[] { + "void Base::foo(int arg)" + }); + } + + // struct Base { + // virtual void foo(int arg); + // }; + // void caller(Base* b) { b->foo( + public void testVirtualMethod_461680() throws Exception { + assertDisplayedParameterHints(new String[] { + "virtual void Base::foo(int arg)" + }); + } + + // struct Base { + // virtual void foo(int arg); + // }; + // struct Derived : Base {}; + // void caller(Derived* d) { d->foo( + public void testNonOverriddenVirtualMethod_461680() throws Exception { + assertDisplayedParameterHints(new String[] { + "virtual void Base::foo(int arg)" + }); + } + + // struct Base { + // virtual void foo(int arg); + // }; + // struct Derived : Base { + // void foo(int arg) override; + // }; + // void caller(Derived* d) { d->foo( + public void testOverriddenVirtualMethod_461680() throws Exception { + assertDisplayedParameterHints(new String[] { + "virtual void Derived::foo(int arg)" + }); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/CParameterListValidator.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/CParameterListValidator.java index 5bae4607ca6..97ee2bc7a09 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/CParameterListValidator.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/CParameterListValidator.java @@ -26,6 +26,8 @@ import org.eclipse.jface.text.contentassist.IContextInformationValidator; import org.eclipse.swt.SWT; import org.eclipse.swt.custom.StyleRange; +import org.eclipse.cdt.internal.ui.text.contentassist.CProposalContextInformation; + /** * This class provides the function parameter parsing for the C/C++ Editor hover. * It is based heavily on the Java class JavaParameterListValidator. @@ -192,9 +194,25 @@ public class CParameterListValidator implements IContextInformationValidator, IC fCurrentParameter= currentParameter; // Don't presume what has been done to the string, rather use as is. - String s= fInformation.getInformationDisplayString(); + String s = fInformation.getInformationDisplayString(); + String params = s; + + // Context information objects of type CProposalContextInformation can have + // an optional prefix before and suffix after the parameter list. + // In such a case, query the indices that bound the parameter list part of + // the string, so we can parse the comma positions accurately. + int paramlistStartIndex = 0; + int paramlistEndIndex = s.length(); + if (fInformation instanceof CProposalContextInformation) { + CProposalContextInformation info = (CProposalContextInformation) fInformation; + if (info.hasPrefixSuffix()) { + paramlistStartIndex = info.getArglistStartIndex(); + paramlistEndIndex = info.getArglistEndIndex(); + params = s.substring(paramlistStartIndex, paramlistEndIndex); + } + } - int[] commas= computeCommaPositions(s); + int[] commas= computeCommaPositions(params); if (commas.length - 2 < fCurrentParameter) { presentation.addStyleRange(new StyleRange(0, s.length(), null, null, SWT.NORMAL)); return true; @@ -203,13 +221,13 @@ public class CParameterListValidator implements IContextInformationValidator, IC int start= commas[fCurrentParameter] + 1; int end= commas[fCurrentParameter + 1]; if (start > 0) - presentation.addStyleRange(new StyleRange(0, start, null, null, SWT.NORMAL)); + presentation.addStyleRange(new StyleRange(paramlistStartIndex, start, null, null, SWT.NORMAL)); if (end > start) - presentation.addStyleRange(new StyleRange(start, end - start, null, null, SWT.BOLD)); + presentation.addStyleRange(new StyleRange(paramlistStartIndex + start, end - start, null, null, SWT.BOLD)); if (end < s.length()) - presentation.addStyleRange(new StyleRange(end, s.length() - end, null, null, SWT.NORMAL)); + presentation.addStyleRange(new StyleRange(paramlistStartIndex + end, params.length() - end, null, null, SWT.NORMAL)); return true; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CProposalContextInformation.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CProposalContextInformation.java index 7e7938a4088..5a610f2332a 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CProposalContextInformation.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CProposalContextInformation.java @@ -27,6 +27,35 @@ public class CProposalContextInformation implements IContextInformation, IContex /** The image to be displayed */ private Image fImage; + /** + * The information display string usually just contains a comma-separated + * list of (function or template) parameters. + * Optionally, it can contain a prefix before and a suffix after the + * parameter list (so that e.g. it displays a function's full signature, + * including name and return value). + * In such a case, fHasPrefixSuffix is true, and fParamlistStartIndex + * and fParamlistEndIndex denote the indices that bound the parameter list + * portion of the information display string. + */ + private boolean fHasPrefixSuffix; + private int fParamlistStartIndex; + private int fParamlistEndIndex; + + public void setHasPrefixSuffix(int paramlistStartIndex, int paramlistEndIndex) { + fHasPrefixSuffix = true; + fParamlistStartIndex = paramlistStartIndex; + fParamlistEndIndex = paramlistEndIndex; + } + public boolean hasPrefixSuffix() { + return fHasPrefixSuffix; + } + public int getArglistStartIndex() { + return fParamlistStartIndex; + } + public int getArglistEndIndex() { + return fParamlistEndIndex; + } + /** * Creates a new context information without an image. * diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/DOMCompletionProposalComputer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/DOMCompletionProposalComputer.java index 56856446a91..b84c1d992ce 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/DOMCompletionProposalComputer.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/DOMCompletionProposalComputer.java @@ -652,7 +652,6 @@ public class DOMCompletionProposalComputer extends ParsingBasedProposalComputer String dispArgString = dispArgs.toString(); String idArgString = idArgs.toString(); - String contextDispargString = hasArgs ? dispArgString : null; StringBuilder dispStringBuff = new StringBuilder(function.getName()); dispStringBuff.append('('); dispStringBuff.append(dispArgString); @@ -669,7 +668,29 @@ public class DOMCompletionProposalComputer extends ParsingBasedProposalComputer idStringBuff.append(')'); String idString = idStringBuff.toString(); - boolean inUsingDeclaration = cContext.isInUsingDirective(); + String contextInfoString = null; + int paramlistStartIndex = 0, paramlistEndIndex = 0; + if (hasArgs) { + StringBuilder contextInfo = new StringBuilder(); + if (function instanceof ICPPMethod && isVirtual((ICPPMethod) function, cContext)) { + contextInfo.append("virtual "); //$NON-NLS-1$ + } + contextInfo.append(returnTypeStr); + contextInfo.append(' '); + if (function instanceof ICPPMethod) { + contextInfo.append(function.getOwner().getName()); + contextInfo.append("::"); //$NON-NLS-1$ + } + contextInfo.append(function.getName()); + contextInfo.append('('); + paramlistStartIndex = contextInfo.length(); + contextInfo.append(dispArgString); + paramlistEndIndex = contextInfo.length(); + contextInfo.append(')'); + contextInfoString = contextInfo.toString(); + } + + boolean inUsingDeclaration = cContext.isInUsingDirective(); if (wantParens && !cContext.isFollowedByOpeningParen()) { // If we might be calling or defining the function in this context, assume we are @@ -693,10 +714,11 @@ public class DOMCompletionProposalComputer extends ParsingBasedProposalComputer proposal.setCursorPosition(cursorPosition); } - if (contextDispargString != null && !inUsingDeclaration) { + if (contextInfoString != null && !inUsingDeclaration) { CProposalContextInformation info = - new CProposalContextInformation(image, dispString, contextDispargString); + new CProposalContextInformation(image, dispString, contextInfoString); info.setContextInformationPosition(cContext.getContextInformationOffset()); + info.setHasPrefixSuffix(paramlistStartIndex, paramlistEndIndex); proposal.setContextInformation(info); } @@ -712,6 +734,24 @@ public class DOMCompletionProposalComputer extends ParsingBasedProposalComputer } } + /** + * Returns true if the given method is virtual, including if it's virtual because + * it overrides a virtual method. + */ + private static boolean isVirtual(ICPPMethod method, CContentAssistInvocationContext context) { + if (method.isVirtual()) { + return true; + } + ICPPMethod[] overridden = ClassTypeHelper.findOverridden(method, + context.getCompletionNode().getTranslationUnit()); + for (ICPPMethod m : overridden) { + if (m.isVirtual()) { + return true; + } + } + return false; + } + /** * Returns true if the invocation is at the function name or before typing any parameters. */