Do not report suspicious semicolons produced by macro expansion.

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SuspiciousSemicolonChecker.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010 Marc-Andre Laperle and others.
+ * Copyright (c) 2010, 2011 Marc-Andre Laperle 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
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     Marc-Andre Laperle - Initial API and implementation
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.cdt.codan.internal.checkers;
 
@@ -38,15 +39,15 @@ public class SuspiciousSemicolonChecker extends AbstractIndexAstChecker {
 				if (statement instanceof IASTIfStatement) {
 					IASTStatement thenStmt = ((IASTIfStatement) statement).getThenClause();
 					IASTStatement elseStmt = ((IASTIfStatement) statement).getElseClause();
-					if (elseStmt instanceof IASTNullStatement && noMacroInvolved(elseStmt) && doReportAfterElse()) {
+					if (elseStmt instanceof IASTNullStatement && doReportAfterElse() &&
+							!macroInvolved(elseStmt)) {
 						reportProblem(ER_ID, elseStmt);
 					}
-					if (elseStmt != null && doNotReportIfElse() == true)
+					if (elseStmt != null && doNotReportIfElse())
 						return PROCESS_CONTINUE;
-					if (thenStmt instanceof IASTNullStatement && noMacroInvolved(thenStmt)) {
+					if (thenStmt instanceof IASTNullStatement && !macroInvolved(thenStmt)) {
 						reportProblem(ER_ID, thenStmt);
 					}
-
 				}
 				return PROCESS_CONTINUE;
 			}
@@ -57,16 +58,20 @@ public class SuspiciousSemicolonChecker extends AbstractIndexAstChecker {
 		final IProblem pt = getProblemById(ER_ID, getFile());
 		return (Boolean) getPreference(pt, PARAM_ELSE);
 	}
+
 	protected boolean doReportAfterElse() {
 		final IProblem pt = getProblemById(ER_ID, getFile());
 		return (Boolean) getPreference(pt, PARAM_ALFTER_ELSE);
 	}
 
-	protected boolean noMacroInvolved(IASTStatement node) {
+	protected boolean macroInvolved(IASTStatement node) {
+		if (includesMacroExpansion(node)) {
+			return true;
+		}
 		IASTNodeSelector nodeSelector = node.getTranslationUnit().getNodeSelector(node.getTranslationUnit().getFilePath());
 		IASTFileLocation fileLocation = node.getFileLocation();
 		IASTPreprocessorMacroExpansion macro = nodeSelector.findEnclosingMacroExpansion(fileLocation.getNodeOffset() - 1, 1);
-		return macro == null;
+		return macro != null;
 	}
 
 	public void initPreferences(IProblemWorkingCopy problem) {

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/AbstractIndexAstChecker.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2010 Alena Laskavaia 
+ * Copyright (c) 2009, 2011 Alena Laskavaia 
  * 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
@@ -23,6 +23,7 @@ import org.eclipse.cdt.core.dom.ast.IASTImageLocation;
 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.IASTTranslationUnit;
 import org.eclipse.cdt.core.model.CoreModel;
 import org.eclipse.cdt.core.model.ICElement;
@@ -132,7 +133,7 @@ public abstract class AbstractIndexAstChecker extends AbstractCheckerWithProblem
 	private IProblemLocation getProblemLocation(IASTNode astNode, IASTFileLocation astLocation) {
 		int line = astLocation.getStartingLineNumber();
 		IProblemLocationFactory locFactory = getRuntime().getProblemLocationFactory();
-		if (hasMacroLocation(astNode) && astNode instanceof IASTName) {
+		if (enclosedInMacroExpansion(astNode) && astNode instanceof IASTName) {
 			IASTImageLocation imageLocation = ((IASTName) astNode).getImageLocation();
 			if (imageLocation != null) {
 				int start = imageLocation.getNodeOffset();
@@ -147,9 +148,17 @@ public abstract class AbstractIndexAstChecker extends AbstractCheckerWithProblem
 		return locFactory.createProblemLocation(getFile(), line);
 	}
 
-	private boolean hasMacroLocation(IASTNode astNode) {
+	protected static boolean enclosedInMacroExpansion(IASTNode node) {
-		return astNode.getNodeLocations().length == 1 &&
+		IASTNodeLocation[] nodeLocations = node.getNodeLocations();
-				astNode.getNodeLocations()[0] instanceof IASTMacroExpansionLocation;
+		return nodeLocations.length == 1 && nodeLocations[0] instanceof IASTMacroExpansionLocation;
+	}
+
+	protected static boolean includesMacroExpansion(IASTNode node) {
+		for (IASTNodeLocation nodeLocation : node.getNodeLocations()) {
+			if (nodeLocation instanceof IASTMacroExpansionLocation)
+				return true;
+		}
+		return false;
 	}
 
 	protected IFile getFile() {

codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/SuspiciousSemicolonCheckerTest.java

@@ -152,6 +152,15 @@ public class SuspiciousSemicolonCheckerTest extends CheckerTestCase {
 		checkNoErrors();
 	}
 
+	// #define MACRO(cond) if (cond) ;
+	// void foo() {
+	//   MACRO(true);
+	// }
+	public void testMacroExpansion() {
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
 	// main() {
 	//	   if (false)
 	//	        ; // only this one is reported