Bug 424824: Codan checker for Qt

This implements a Codan checker for QObject::connect and disconnect
function calls.  There are several overloads for each function, but the
basic call looks like:

    Q * q;
    QObject::connect( q, SIGNAL( sign() ), q, SLOT( slot() ) );

This function calls requires that Q have a Qt signal called sign and a
Qt slot called slot, e.g.,

    class Q : public QObject
    {
    Q_OBJECT
    Q_SIGNAL void sign();
    Q_SLOT   void slot();
    };

The Qt checker raises a warning if either condition is not true.

It also raises a warning for SIGNAL or SLOT expansions without a
parameter.

Change-Id: If68a5bcdabb3f118801675e46ae926e6a250378a
Signed-off-by: Andrew Eidsness <eclipse@jfront.com>
Reviewed-on: https://git.eclipse.org/r/20231
Tested-by: Hudson CI
Reviewed-by: Doug Schaefer <dschaefer@qnx.com>
IP-Clean: Doug Schaefer <dschaefer@qnx.com>

qt/org.eclipse.cdt.qt.core/META-INF/MANIFEST.MF

@@ -8,7 +8,9 @@ Bundle-Vendor: %providerName
 Require-Bundle: org.eclipse.core.runtime,
  org.eclipse.core.resources,
  org.eclipse.core.expressions;bundle-version="[3.2.0,4.0.0)",
- org.eclipse.cdt.core
+ org.eclipse.cdt.core,
+ org.eclipse.cdt.codan.core,
+ org.eclipse.cdt.codan.core.cxx
 Bundle-RequiredExecutionEnvironment: JavaSE-1.6
 Bundle-ActivationPolicy: lazy
 Bundle-Localization: plugin

qt/org.eclipse.cdt.qt.core/plugin.xml

@@ -100,4 +100,33 @@
             </with>
         </definition>
     </extension>
+    <extension
+        point="org.eclipse.cdt.codan.core.checkers"
+        id="org.eclipse.cdt.core.qt.SyntaxCheckers">
+        <category
+              id="org.eclipse.cdt.qt.core.qtproblemcategory"
+              name="Qt Problems">
+        </category>
+        <checker
+            id="org.eclipse.cdt.core.qt.connectfunctioncallchecker"
+            name="Qt Connect Function Call Checker"
+            class="org.eclipse.cdt.internal.qt.core.codan.QtSyntaxChecker">
+            <problem
+                category="org.eclipse.cdt.qt.core.qtproblemcategory"
+                defaultEnabled="true"
+                defaultSeverity="Warning"
+                id="org.eclipse.cdt.qt.core.qtproblem"
+                markerType="org.eclipse.cdt.qt.core.qtproblem"
+                messagePattern="{0}"
+                name="Qt Syntax Problem">
+            </problem>
+        </checker>
+    </extension>
+    <extension
+        point="org.eclipse.core.resources.markers"
+        id="qtproblem"
+        name="Qt C/C++ Problem">
+        <super type="org.eclipse.cdt.codan.core.codanProblem"/> 
+        <persistent value="true"/>
+    </extension>
 </plugin>

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

@@ -7,13 +7,12 @@
  */
 package org.eclipse.cdt.internal.qt.core;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 
 import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
 import org.eclipse.cdt.core.dom.ast.IASTInitializerClause;
-import org.eclipse.cdt.core.dom.ast.IASTName;
 import org.eclipse.cdt.core.dom.ast.IASTNode;
 import org.eclipse.cdt.core.dom.ast.IBasicType;
 import org.eclipse.cdt.core.dom.ast.IEnumeration;
@@ -37,15 +36,11 @@ public class QtFunctionCall {
 	private QtFunctionCall() {
 	}
 
-	protected static IASTNode safeArgsAt(IASTNode[] args, int index) {
-		return args.length > index ? args[index] : null;
-	}
-
 	/**
 	 * Returns a collection of all Qt method references within the given function call.  Returns
 	 * null if there are no Qt method references.
 	 */
-	public static Collection<IASTName> getReferences(IASTFunctionCallExpression call) {
+	public static Collection<QtMethodReference> getReferences(IASTFunctionCallExpression call) {
 		ICPPFunction function = ASTUtil.resolveFunctionBinding(ICPPFunction.class, call);
 		if (function == null)
 			return null;
@@ -57,7 +52,7 @@ public class QtFunctionCall {
 		return null;
 	}
 
-	private static Collection<IASTName> getReferencesInConnect(ICPPFunction function, IASTFunctionCallExpression call) {
+	private static Collection<QtMethodReference> getReferencesInConnect(ICPPFunction function, IASTFunctionCallExpression call) {
 		if (function == null)
 			return null;
 
@@ -83,8 +78,8 @@ public class QtFunctionCall {
 		}
 
 		// Otherwise find the signal and member parameters based on the overload.
-		IASTName signal = null;
-		IASTName member = null;
+		QtMethodReference signal = null;
+		QtMethodReference member = null;
 
 		// static bool connect( const QObject *sender, const char *signal,
 		//                      const QObject *receiver, const char *member,
@@ -102,20 +97,10 @@ public class QtFunctionCall {
 			member = QtMethodReference.parse(call, ASTUtil.getReceiverType(call), safeArgsAt(args, 2));
 		}
 
-		// Merge non-null signal and slot to return a list of 0, 1, or 2 elements.
-		if (signal == null) {
-			if (member == null)
-				return null;
-			return Collections.singletonList(member);
-		}
-
-		if (member == null)
-			return Collections.singletonList(signal);
-
-		return Arrays.asList(signal, member);
+		return mergeNonNull(signal, member);
 	}
 
-	private static Collection<IASTName> getReferencesInDisconnect(ICPPFunction function, IASTFunctionCallExpression call) {
+	private static Collection<QtMethodReference> getReferencesInDisconnect(ICPPFunction function, IASTFunctionCallExpression call) {
 		if (function == null)
 			return null;
 
@@ -140,8 +125,8 @@ public class QtFunctionCall {
 		}
 
 		// Otherwise find the signal and member parameters based on the overload.
-		IASTName signal = null;
-		IASTName member = null;
+		QtMethodReference signal = null;
+		QtMethodReference member = null;
 
 		if (type1 instanceof IBasicType && ( (IBasicType)type1 ).getKind() == IBasicType.Kind.eChar ) {
 			switch(params.length) {
@@ -167,16 +152,34 @@ public class QtFunctionCall {
 			member = QtMethodReference.parse(call, ASTUtil.getBaseType(safeArgsAt(args, 1)), safeArgsAt(args, 2));
 		}
 
-		// Merge non-null signal and slot to return a list of 0, 1, or 2 elements.
-		if (signal == null) {
-			if (member == null)
-				return null;
-			return Collections.singletonList(member);
+		return mergeNonNull(signal, member);
+	}
+
+	private static IASTNode safeArgsAt(IASTNode[] args, int index) {
+		return args.length > index ? args[index] : null;
+	}
+
+	private static <T> Collection<T> mergeNonNull(T...withNulls) {
+		T firstNonNull = null;
+		ArrayList<T> list = null;
+		for(T t : withNulls) {
+			if (t == null)
+				continue;
+			else if(list != null)
+				list.add(t);
+			else if(firstNonNull == null)
+				firstNonNull = t;
+			else {
+				list = new ArrayList<T>(withNulls.length);
+				list.add(firstNonNull);
+				list.add(t);
+			}
 		}
 
-		if (member == null)
-			return Collections.singletonList(signal);
-
-		return Arrays.asList(signal, member);
+		if (list != null)
+			return list;
+		if (firstNonNull != null)
+			return Collections.singletonList(firstNonNull);
+		return null;
 	}
 }

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

@@ -81,6 +81,13 @@ public class QtMethodReference extends ASTNameReference {
 		this.expansionParam = expansionParam;
 	}
 
+	/**
+	 * Return the C++ class that defines the Qt method that is being referenced.
+	 */
+	public ICPPClassType getContainingType() {
+		return cls;
+	}
+
 	/**
 	 * Look for SIGNAL or SLOT macro expansions at the location of the given node.  Return the
 	 * QMethod reference is an expansion is found and null otherwise.

qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/codan/Messages.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2014 QNX Software Systems and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.cdt.internal.qt.core.codan;
+
+import org.eclipse.osgi.util.NLS;
+
+public class Messages extends NLS {
+	private static final String BUNDLE_NAME = "org.eclipse.cdt.internal.qt.core.codan.messages"; //$NON-NLS-1$
+	public static String Function_Not_Resolved_Msg;
+	public static String Parameter_Not_Resolved_Msg;
+	public static String Missing_Parameter_Msg;
+	public static String SignalSlot_Not_Defined_Msg;
+
+	public static String QtConnect_macro_without_method_1;
+	public static String QtConnect_macro_method_not_found_3;
+
+	static {
+		// initialize resource bundle
+		NLS.initializeMessages(BUNDLE_NAME, Messages.class);
+	}
+
+	private Messages() {
+	}
+}

qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/codan/QtSyntaxChecker.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) 2014 QNX Software Systems and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.cdt.internal.qt.core.codan;
+
+import java.util.Collection;
+
+import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
+import org.eclipse.cdt.codan.core.model.CheckerLaunchMode;
+import org.eclipse.cdt.codan.core.model.ICheckerWithPreferences;
+import org.eclipse.cdt.codan.core.model.IProblemWorkingCopy;
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTExpression;
+import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
+import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.internal.qt.core.ASTUtil;
+import org.eclipse.cdt.internal.qt.core.QtFunctionCall;
+import org.eclipse.cdt.internal.qt.core.QtMethodReference;
+import org.eclipse.cdt.qt.core.QtNature;
+import org.eclipse.cdt.qt.core.QtPlugin;
+import org.eclipse.osgi.util.NLS;
+
+/**
+ * A Codan checker for QObject::connect and QObject::disconnect function calls.  The checker
+ * confirms that SIGNAL and SLOT macro expansions reference a valid Qt signal or slot.
+ */
+public class QtSyntaxChecker extends AbstractIndexAstChecker implements ICheckerWithPreferences {
+	private final Checker checker = new Checker();
+
+	@Override
+	public boolean runInEditor() {
+		return true;
+	}
+
+	@Override
+	public void initPreferences(IProblemWorkingCopy problem) {
+		// don't run on full or incremental builds
+		getTopLevelPreference(problem);
+		getLaunchModePreference(problem).enableInLaunchModes(
+				CheckerLaunchMode.RUN_ON_FILE_OPEN,
+        		CheckerLaunchMode.RUN_AS_YOU_TYPE,
+                CheckerLaunchMode.RUN_ON_DEMAND );
+	}
+
+	@Override
+	public void processAst(IASTTranslationUnit ast) {
+		// Run the checker only on Qt-enabled projects.
+		if (QtNature.hasNature(ASTUtil.getProject(ast)))
+			ast.accept(checker);
+	}
+
+	private class Checker extends ASTVisitor {
+		public Checker() {
+			shouldVisitExpressions = true;
+		}
+
+		@Override
+		public int visit(IASTExpression expr) {
+			if (!(expr instanceof IASTFunctionCallExpression))
+				return PROCESS_CONTINUE;
+			IASTFunctionCallExpression fncall = (IASTFunctionCallExpression) expr;
+
+			Collection<QtMethodReference> refs = QtFunctionCall.getReferences(fncall);
+			if (refs != null)
+				for(QtMethodReference ref : refs) {
+					IBinding binding = ref.resolveBinding();
+					if (binding != null)
+						continue;
+
+					// Either the macro expansion didn't have an argument, or the argument was not a valid method.
+					if (ref.getRawSignature().isEmpty())
+						report(ref, Messages.QtConnect_macro_without_method_1, ref.getType().macroName);
+					else
+						report(ref, Messages.QtConnect_macro_method_not_found_3, ref.getType().paramName, ref.getContainingType().getName(), ref.getRawSignature());
+				}
+
+			return PROCESS_CONTINUE;
+		}
+
+		private void report(IASTNode node, String message, Object... args) {
+			if (args.length <= 0)
+				reportProblem(QtPlugin.QT_SYNTAX_ERR_ID, node, message);
+			else
+				reportProblem(QtPlugin.QT_SYNTAX_ERR_ID, node, NLS.bind(message, args));
+		}
+	};
+}

qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/codan/messages.properties

@@ -0,0 +1,13 @@
+# Copyright (c) 2014 QNX Software Systems and others.
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+
+Function_Not_Resolved_Msg=Function "{0}" can not be resolved
+Parameter_Not_Resolved_Msg="{0}" is not a valid parameter
+Missing_Parameter_Msg=Missing parameter
+SignalSlot_Not_Defined_Msg=Signal/Slot "{0}" is not defined
+
+QtConnect_macro_without_method_1    = The {0} parameter does not specify a method
+QtConnect_macro_method_not_found_3  = {1}::{2} has not been tagged as a Qt {0}; make sure all parameter types are fully qualified

qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/pdom/QtASTVisitor.java

@@ -38,6 +38,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPInternalBinding;
 import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics;
 import org.eclipse.cdt.internal.core.parser.scanner.LocationMap;
 import org.eclipse.cdt.internal.qt.core.QtFunctionCall;
+import org.eclipse.cdt.internal.qt.core.QtMethodReference;
 import org.eclipse.cdt.internal.qt.core.QtMethodUtil;
 import org.eclipse.cdt.internal.qt.core.index.QProperty;
 import org.eclipse.cdt.qt.core.QtKeywords;
@@ -121,7 +122,7 @@ public class QtASTVisitor extends ASTVisitor {
 	@Override
 	public int visit(IASTExpression expr) {
 		if (expr instanceof IASTFunctionCallExpression) {
-			Collection<IASTName> refs = QtFunctionCall.getReferences((IASTFunctionCallExpression) expr);
+			Collection<QtMethodReference> refs = QtFunctionCall.getReferences((IASTFunctionCallExpression) expr);
 			if (refs != null)
 				for (IASTName ref : refs) {
 					IASTFileLocation nameLoc = ref.getFileLocation();

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

@@ -25,6 +25,8 @@ public class QtPlugin extends Plugin {
 	public static final String QMAKE_ENV_PROVIDER_EXT_POINT_NAME = "qmakeEnvProvider"; //$NON-NLS-1$
 	public static final String QMAKE_ENV_PROVIDER_ID = ID + "." + QMAKE_ENV_PROVIDER_EXT_POINT_NAME; //$NON-NLS-1$
 
+    public static final String QT_SYNTAX_ERR_ID = "org.eclipse.cdt.qt.core.qtproblem"; //$NON-NLS-1$
+
     /**
      * Instances of QtIndex are cached within the session properties of the project from
      * which they are created.  This name is used to store the property.