Recognize Q_SIGNAL and Q_SLOT on single functions

Qt allows signals and slots to be marked directly on the function, e.g.,

class T
{
    Q_SIGNAL void some_signal();
    Q_SLOT   void some_slot();
};

This change modifies the Qt signal/slot tagger to look for these tags in
addition to the previously implemented search for the visibility label.

Change-Id: Ibf43df8d80d4ca9f8b62776e7a35a4fc067a289e
Reviewed-on: https://git.eclipse.org/r/10701
Reviewed-by: Doug Schaefer <dschaefer@qnx.com>
IP-Clean: Doug Schaefer <dschaefer@qnx.com>
Tested-by: Doug Schaefer <dschaefer@qnx.com>

qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/qt/core/QtKeywords.java

@@ -11,16 +11,13 @@ package org.eclipse.cdt.qt.core;
 /**
  * Declares constants related to tokens that are special in Qt applications.
  */
-public class QtKeywords
+public class QtKeywords {
-{
+	public static final String CONNECT = "connect";
+	public static final String Q_SIGNAL = "Q_SIGNAL";
 	public static final String Q_SIGNALS = "Q_SIGNALS";
+	public static final String Q_SLOT = "Q_SLOT";
 	public static final String Q_SLOTS = "Q_SLOTS";
+	public static final String QOBJECT = "QObject";
 	public static final String SIGNALS = "signals";
 	public static final String SLOTS = "slots";
-
-    public static final String SIGNAL = "SIGNAL";
-    public static final String SLOT = "SLOT";
-
-    public static final String QOBJECT = "QObject";
-    public static final String CONNECT = "connect";
 }

qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/qt/internal/core/QtSignalSlotTagger.java

@@ -9,11 +9,14 @@
 package org.eclipse.cdt.qt.internal.core;
 
 import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
+import org.eclipse.cdt.core.dom.ast.IASTFileLocation;
 import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation;
 import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
 import org.eclipse.cdt.core.dom.ast.IASTNodeLocation;
 import org.eclipse.cdt.core.dom.ast.IASTPreprocessorMacroExpansion;
+import org.eclipse.cdt.core.dom.ast.IASTProblemHolder;
+import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration;
 import org.eclipse.cdt.core.dom.ast.IBinding;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTVisibilityLabel;
@@ -25,19 +28,36 @@ import org.eclipse.cdt.core.dom.ast.tag.IWritableTag;
 import org.eclipse.cdt.qt.core.QtKeywords;
 import org.eclipse.cdt.qt.core.QtPlugin;
 
-public class QtSignalSlotTagger implements IBindingTagger
+/**
-{
+ * Finds all functions that are marked as Qt signals or slots and tags them in
-    private static ICPPASTVisibilityLabel findVisibilityLabel( ICPPMethod method, IASTNode ast )
+ * the index. There are two ways that Qt understands for marking a function as a
-    {
+ * signal or slot: 1) With a macro in the function's visibility label 2) With a
+ * macro before the function itself E.g., both of these cases are valid:
+ * 
+ * <pre>
+ * class T
+ * {
+ * private:
+ *     Q_SLOT void some_slot();
+ * 
+ * signals:
+ *     void some_signal();
+ * };
+ * </pre>
+ * 
+ * The 6 applicable macros are signals, Q_SIGNALS, Q_SIGNAL, slots, Q_SLOTS, and
+ * Q_SLOT.
+ */
+public class QtSignalSlotTagger implements IBindingTagger {
+	private static ICPPASTVisibilityLabel findVisibilityLabel(
+			ICPPMethod method, IASTNode ast) {
 		// the visibility cannot be found without an ast
 		if (ast == null)
 			return null;
 
 		IASTNode methodDecl = ast;
 		ICPPASTCompositeTypeSpecifier classType = null;
-        while( methodDecl != null
+		while (methodDecl != null && classType == null) {
-            && classType == null )
-        {
 			IASTNode parent = methodDecl.getParent();
 			if (parent instanceof ICPPASTCompositeTypeSpecifier)
 				classType = (ICPPASTCompositeTypeSpecifier) parent;
@@ -45,13 +65,11 @@ public class QtSignalSlotTagger implements IBindingTagger
 				methodDecl = parent;
 		}
 
-        if( methodDecl == null
+		if (methodDecl == null || classType == null)
-         || classType == null )
 			return null;
 
 		ICPPASTVisibilityLabel lastLabel = null;
-        for( IASTDeclaration decl : classType.getMembers() )
+		for (IASTDeclaration decl : classType.getMembers()) {
-        {
 			if (decl instanceof ICPPASTVisibilityLabel)
 				lastLabel = (ICPPASTVisibilityLabel) decl;
 			else if (decl == methodDecl)
@@ -61,38 +79,149 @@ public class QtSignalSlotTagger implements IBindingTagger
 		return null;
 	}
 
+	private static byte getBitset(IASTNodeLocation... locations) {
+		for (IASTNodeLocation location : locations)
+			if (location instanceof IASTMacroExpansionLocation) {
+				IASTMacroExpansionLocation macroExpansion = (IASTMacroExpansionLocation) location;
+				IASTPreprocessorMacroExpansion exp = macroExpansion
+						.getExpansion();
+				String macro = exp.getMacroReference().toString();
+
+				if (QtKeywords.Q_SIGNAL.equals(macro)
+						|| QtKeywords.Q_SIGNALS.equals(macro)
+						|| QtKeywords.SIGNALS.equals(macro))
+					return QtPlugin.SignalSlot_Mask_signal;
+				if (QtKeywords.Q_SLOT.equals(macro)
+						|| QtKeywords.Q_SLOTS.equals(macro)
+						|| QtKeywords.SLOTS.equals(macro))
+					return QtPlugin.SignalSlot_Mask_slot;
+			}
+
+		return 0;
+	}
+
+	private static byte getBitset(IASTNode... nodes) {
+		byte bitset = 0;
+		for (IASTNode node : nodes)
+			if (node != null)
+				for (IASTNodeLocation loc : node.getNodeLocations())
+					bitset |= getBitset(loc);
+
+		return bitset;
+	}
+
+	private static IASTNode getSimpleDecl(IASTNode node) {
+		while (node != null && !(node instanceof IASTSimpleDeclaration))
+			node = node.getParent();
+		return node;
+	}
+
+	private byte getQtMarkers(ICPPMethod method, IASTName ast) {
+		byte bitset = 0;
+		if (ast == null)
+			return bitset;
+
+		// Look for macros on the previous visibility label.
+		bitset |= getBitset(findVisibilityLabel(method, ast));
+
+		// Look for macros on this function. See Bug 401696 for a better
+		// description of why it needs
+		// to work this why. Briefly, the parser does not associate empty macros
+		// with the function when
+		// they are the first thing in the declaration. E.g.,
+		// #define X
+		// void func1() {}
+		// X void func2() {}
+		// Could also look like:
+		// void func1() {} X
+		// void func2() {}
+		//
+		// The following code instead looks at the parents and children to find
+		// all node locations between
+		// the declarators.
+		//
+		// We first look at parents to find the closest SimpleDeclaration. We
+		// then look at that node's parent
+		// to find the node that is right before the target. Then we look at all
+		// node locations between the
+		// end of that previous node and the end of the target node.
+
+		// find the closest containing SimpleDecl
+		IASTNode simpleDecl = getSimpleDecl(ast);
+		IASTNode parent = simpleDecl == null ? null : simpleDecl.getParent();
+		if (parent == null)
+			return bitset;
+
+		// find the declaration before the target
+		IASTNode previous = null;
+		IASTNode[] children = parent.getChildren();
+		if (children.length > 1)
+			for (int i = 1; i < children.length; ++i) {
+				if (children[i] == simpleDecl) {
+					// if we haven't found a SimpleDecl, then find the nearest
+					// previous non-problem node
+					for (int j = i - 1; previous == null && j >= 0; --j)
+						if (!(children[j] instanceof IASTProblemHolder))
+							previous = children[j];
+					break;
+				}
+				if (children[i] instanceof IASTSimpleDeclaration)
+					previous = children[i];
+			}
+
+		// Signals/slots can only be declared inside of classes, so all cases we
+		// care about have a
+		// previous child, even if it is only the Base-class specifier.
+		if (previous == null)
+			return bitset;
+
+		IASTFileLocation prevLocation = previous.getFileLocation();
+		int prev_off = prevLocation.getNodeOffset();
+		int prev_end = prevLocation.getNodeOffset()
+				+ prevLocation.getNodeLength();
+
+		// Figure out where the target node ends.
+		int end = ast.getFileLocation().getNodeOffset()
+				+ ast.getFileLocation().getNodeLength();
+
+		// Examine all locations that appear after the previous node and before
+		// the target node.
+		boolean found_previous = false;
+		for (IASTNodeLocation loc : parent.getNodeLocations()) {
+			int o = loc.getNodeOffset();
+			int e = loc.getNodeOffset() + loc.getNodeLength();
+
+			// if the previous node has already been found, process this one
+			if (found_previous)
+				bitset |= getBitset(loc);
+
+			// otherwise see if this is the previous node
+			else if (o <= prev_off && e >= prev_end)
+				found_previous = true;
+
+			// stop processing when we're processed to the end of the target
+			if (e >= end)
+				break;
+		}
+
+		return bitset;
+	}
+
 	@Override
-    public ITag process( ITagWriter tagWriter, IBinding binding, IASTName ast )
+	public ITag process(ITagWriter tagWriter, IBinding binding, IASTName ast) {
-    {
 		// only methods a be signals or slots
 		if (!(binding instanceof ICPPMethod))
 			return null;
 
-        // a visibility label is required in order to decide whether the method is a signal/slot
+		// Find all qt marker macros associated with this node.
 		ICPPMethod method = (ICPPMethod) binding;
-        ICPPASTVisibilityLabel v = findVisibilityLabel( method, ast );
+		byte bitset = getQtMarkers(method, ast);
-        if( v == null )
-            return null;
 
-        byte bitset = 0;
+		// create and store the bitset if needed
-        for( IASTNodeLocation loc : v.getNodeLocations() )
+		if (bitset != 0) {
-            if( loc instanceof IASTMacroExpansionLocation )
+			IWritableTag tag = tagWriter.createTag(
-            {
+					QtPlugin.SIGNAL_SLOT_TAGGER_ID, 1);
-                IASTMacroExpansionLocation macroExpansion = (IASTMacroExpansionLocation)loc;
+			if (tag != null && tag.putByte(0, bitset))
-                IASTPreprocessorMacroExpansion exp = macroExpansion.getExpansion();
-                String macro = exp.getMacroReference().toString();
-
-                if( QtKeywords.SIGNALS.equals( macro ) || QtKeywords.Q_SIGNALS.equals( macro ) )
-                    bitset |= QtPlugin.SignalSlot_Mask_signal;
-                else if( QtKeywords.SLOTS.equals( macro ) || QtKeywords.Q_SLOTS.equals( macro ) )
-                    bitset |= QtPlugin.SignalSlot_Mask_slot;
-            }
-
-        if( bitset != 0 )
-        {
-            IWritableTag tag = tagWriter.createTag( QtPlugin.SIGNAL_SLOT_TAGGER_ID, 1 );
-            if( tag != null
-             && tag.putByte( 0, bitset ) )
 				return tag;
 		}
 

qt/org.eclipse.cdt.qt.ui/src/org/eclipse/cdt/qt/internal/ui/QtCompletionProposalComputer.java

@@ -46,16 +46,15 @@ import org.eclipse.cdt.qt.core.QtKeywords;
 import org.eclipse.cdt.qt.core.QtNature;
 import org.eclipse.cdt.qt.core.QtPlugin;
 import org.eclipse.cdt.ui.CUIPlugin;
+import org.eclipse.cdt.ui.text.contentassist.ICEditorContentAssistInvocationContext;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.contentassist.ICompletionProposal;
 
 @SuppressWarnings("restriction")
-public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
+public class QtCompletionProposalComputer extends ParsingBasedProposalComputer {
-{
+	private boolean isApplicable(ICEditorContentAssistInvocationContext context) {
-    private boolean isApplicable( CContentAssistInvocationContext context )
-    {
 		ITranslationUnit tu = context.getTranslationUnit();
 		if (tu == null)
 			return false;
@@ -68,86 +67,83 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 		if (project == null)
 			return false;
 
-        try
+		try {
-        {
 			return project.hasNature(QtNature.ID);
-        }
+		} catch (CoreException e) {
-        catch( CoreException e )
-        {
 			CUIPlugin.log(e);
 			return false;
 		}
 	}
 
-    private static boolean is_QObject_connect( CContentAssistInvocationContext context, IASTCompletionContext astContext, IASTName name )
+	private static boolean is_QObject_connect(
-    {
+			ICEditorContentAssistInvocationContext context,
+			IASTCompletionContext astContext, IASTName name) {
 		IASTName connectName = name.getLastName();
 		if (!QtKeywords.CONNECT.equals(new String(connectName.getSimpleID())))
 			return false;
 
-        IBinding[] funcBindings = astContext.findBindings( connectName, ! context.isContextInformationStyle() );
+		IBinding[] funcBindings = astContext.findBindings(connectName,
+				!context.isContextInformationStyle());
 		for (IBinding funcBinding : funcBindings)
-            if( funcBinding instanceof ICPPFunction )
+			if (funcBinding instanceof ICPPFunction) {
-            {
 				IBinding ownerBinding = ((ICPPFunction) funcBinding).getOwner();
-                if( ownerBinding != null && QtKeywords.QOBJECT.equals( ownerBinding.getName() ) )
+				if (ownerBinding != null
+						&& QtKeywords.QOBJECT.equals(ownerBinding.getName()))
 					return true;
 			}
 
 		return false;
 	}
 
-    private static class Completion
+	private static class Completion {
-    {
 		private final String replacement;
 		private final String display;
 		private final int cursorOffset;
 
-        public static final Completion SIGNAL = new Completion( "SIGNAL()", "SIGNAL(a)", -1 );
+		public static final Completion SIGNAL = new Completion("SIGNAL()",
-        public static final Completion SLOT = new Completion( "SLOT()", "SLOT(a)", -1 );
+				"SIGNAL(a)", -1);
+		public static final Completion SLOT = new Completion("SLOT()",
+				"SLOT(a)", -1);
 
-        public Completion( String replacement )
+		public Completion(String replacement) {
-        {
 			this(replacement, replacement, 0);
 		}
 
-        public Completion( String replacement, String display, int cursorOffset )
+		public Completion(String replacement, String display, int cursorOffset) {
-        {
 			this.replacement = replacement;
 			this.display = display;
 			this.cursorOffset = cursorOffset;
 		}
 
-        public ICompletionProposal createProposal( CContentAssistInvocationContext context )
+		public ICompletionProposal createProposal(
-        {
+				ICEditorContentAssistInvocationContext context) {
 			int repLength = replacement.length();
 			int repOffset = context.getInvocationOffset();
-            CCompletionProposal p = new CCompletionProposal( replacement, repOffset, repLength, null, display, RelevanceConstants.DEFAULT_TYPE_RELEVANCE, context.getViewer() );
+			CCompletionProposal p = new CCompletionProposal(replacement,
+					repOffset, repLength, null, display,
+					RelevanceConstants.DEFAULT_TYPE_RELEVANCE,
+					context.getViewer());
 			p.setCursorPosition(repLength + cursorOffset);
 			return p;
 		}
 
 		@Override
-        public String toString()
+		public String toString() {
-        {
 			if (replacement == null)
 				return super.toString();
 			return replacement + '@' + cursorOffset;
 		}
 	}
 
-    private static interface MethodFilter
+	private static interface MethodFilter {
-    {
 		public boolean keep(ICPPMethod method);
 
-        public static class Qt
+		public static class Qt {
-        {
+			public static final MethodFilter Signal = new MethodFilter() {
-            public static final MethodFilter Signal = new MethodFilter()
-            {
 				@Override
-                public boolean keep( ICPPMethod method )
+				public boolean keep(ICPPMethod method) {
-                {
+					ITagReader tagReader = CCorePlugin.getTagService()
-                    ITagReader tagReader = CCorePlugin.getTagService().findTagReader( method );
+							.findTagReader(method);
 					if (tagReader == null)
 						return false;
 
@@ -161,12 +157,11 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 				}
 			};
 
-            public static final MethodFilter Slot = new MethodFilter()
+			public static final MethodFilter Slot = new MethodFilter() {
-            {
 				@Override
-                public boolean keep( ICPPMethod method )
+				public boolean keep(ICPPMethod method) {
-                {
+					ITagReader tagReader = CCorePlugin.getTagService()
-                    ITagReader tagReader = CCorePlugin.getTagService().findTagReader( method );
+							.findTagReader(method);
 					if (tagReader == null)
 						return false;
 
@@ -182,43 +177,43 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 		}
 	}
 
-    private static Iterable<ICPPMethod> filterMethods( final ICPPClassType cls, final MethodFilter filter )
+	private static Iterable<ICPPMethod> filterMethods(final ICPPClassType cls,
-    {
+			final MethodFilter filter) {
-        return new Iterable<ICPPMethod>()
+		return new Iterable<ICPPMethod>() {
-        {
 			@Override
-            public Iterator<ICPPMethod> iterator()
+			public Iterator<ICPPMethod> iterator() {
-            {
+				return new Iterator<ICPPMethod>() {
-                return new Iterator<ICPPMethod>()
-                {
 					private int index = 0;
 					private final ICPPMethod[] methods = cls.getMethods();
 
 					@Override
-                    public boolean hasNext()
+					public boolean hasNext() {
-                    {
 						for (; index < methods.length; ++index)
 							if (filter.keep(methods[index]))
 								return true;
 						return false;
 					}
 
-                    @Override public ICPPMethod next() { return methods[index++]; }
+					@Override
-                    @Override public void remove() { }
+					public ICPPMethod next() {
+						return methods[index++];
+					}
+
+					@Override
+					public void remove() {
+					}
 				};
 			}
 		};
 	}
 
-    private static String getSignature( ICPPMethod method )
+	private static String getSignature(ICPPMethod method) {
-    {
 		StringBuilder signature = new StringBuilder();
 
 		signature.append(method.getName());
 		signature.append('(');
 		boolean first = true;
-        for( ICPPParameter param : method.getParameters() )
+		for (ICPPParameter param : method.getParameters()) {
-        {
 			if (first)
 				first = false;
 			else
@@ -230,8 +225,8 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 		return signature.toString();
 	}
 
-    private static void addCompletionsFor( Collection<Completion> completions, IASTInitializerClause init, MethodFilter filter )
+	private static void addCompletionsFor(Collection<Completion> completions,
-    {
+			IASTInitializerClause init, MethodFilter filter) {
 		if (!(init instanceof ICPPASTInitializerClause))
 			return;
 
@@ -249,7 +244,8 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 	}
 
 	// Copied from org.eclipse.cdt.internal.ui.text.CParameterListValidator
-    private static int indexOfClosingPeer(String code, char left, char right, int pos) {
+	private static int indexOfClosingPeer(String code, char left, char right,
+			int pos) {
 		int level = 0;
 		final int length = code.length();
 		while (pos < length) {
@@ -301,15 +297,16 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 		return fields;
 	}
 
-
+	private void addConnectParameterCompletions(
-    private void addConnectParameterCompletions( List<ICompletionProposal> proposals, CContentAssistInvocationContext context, IASTCompletionNode completionNode, String prefix )
+			List<ICompletionProposal> proposals,
-    {
+			ICEditorContentAssistInvocationContext context,
+			IASTCompletionNode completionNode, String prefix) {
 		IASTName[] names = completionNode.getNames();
 		List<Completion> completions = new LinkedList<Completion>();
 
-        for( IASTName name : names )
+		for (IASTName name : names) {
-        {
+			// The node isn't properly hooked up, must have backtracked out of
-            // The node isn't properly hooked up, must have backtracked out of this node
+			// this node
 			if (name.getTranslationUnit() == null)
 				continue;
 
@@ -318,21 +315,23 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 				continue;
 			IASTNode astNode = (IASTNode) astContext;
 
-            if( is_QObject_connect( context, astContext, name ) )
+			if (is_QObject_connect(context, astContext, name)) {
-            {
 				int parseOffset = context.getParseOffset();
 				int invocationOffset = context.getInvocationOffset();
 
 				String unparsed = "";
-                try { unparsed = context.getDocument().get( parseOffset, invocationOffset - parseOffset ); }
+				try {
-                catch( BadLocationException e ) { CCorePlugin.log( e ); }
+					unparsed = context.getDocument().get(parseOffset,
+							invocationOffset - parseOffset);
+				} catch (BadLocationException e) {
+					CCorePlugin.log(e);
+				}
 
 				if (unparsed.length() > 0 && unparsed.charAt(0) == '(')
 					unparsed = unparsed.substring(1);
 
 				int[] commas = computeCommaPositions(unparsed);
-                switch( commas.length )
+				switch (commas.length) {
-                {
 				case 3:
 					completions.add(Completion.SIGNAL);
 					break;
@@ -340,9 +339,7 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 					completions.add(Completion.SLOT);
 					break;
 				}
-            }
+			} else if (astNode.getPropertyInParent() == IASTFunctionCallExpression.ARGUMENT) {
-            else if( astNode.getPropertyInParent() == IASTFunctionCallExpression.ARGUMENT )
-            {
 				IASTNode parent = astNode.getParent();
 				if (!(parent instanceof IASTFunctionCallExpression))
 					continue;
@@ -357,22 +354,20 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 					continue;
 
 				IASTInitializerClause[] args = call.getArguments();
-                switch( args.length )
+				switch (args.length) {
-                {
 				case 2:
-                    //if( QtKeywords.SIGNAL.equals( prefix ) )
+					addCompletionsFor(completions, args[0],
-                        addCompletionsFor( completions, args[0], MethodFilter.Qt.Signal );
+							MethodFilter.Qt.Signal);
 					break;
 				case 4:
-                    if( QtKeywords.SLOT.equals( prefix ) )
+					addCompletionsFor(completions, args[2],
-                        addCompletionsFor( completions, args[2], MethodFilter.Qt.Slot );
+							MethodFilter.Qt.Slot);
 					break;
 				}
 			}
 		}
 
-        for( Completion completion : completions )
+		for (Completion completion : completions) {
-        {
 			ICompletionProposal proposal = completion.createProposal(context);
 			if (proposal != null)
 				proposals.add(proposal);
@@ -380,13 +375,16 @@ public class QtCompletionProposalComputer extends ParsingBasedProposalComputer
 	}
 
 	@Override
-    protected List<ICompletionProposal> computeCompletionProposals( CContentAssistInvocationContext context, IASTCompletionNode completionNode, String prefix ) throws CoreException
+	protected List<ICompletionProposal> computeCompletionProposals(
-    {
+			CContentAssistInvocationContext context,
+			IASTCompletionNode completionNode, String prefix)
+			throws CoreException {
 		if (!isApplicable(context))
 			return Collections.emptyList();
 
 		List<ICompletionProposal> proposals = new ArrayList<ICompletionProposal>();
-        addConnectParameterCompletions( proposals, context, completionNode, prefix );
+		addConnectParameterCompletions(proposals, context, completionNode,
+				prefix);
 		return proposals;
 	}
 }