Added preference about macro expansion

5 files changed, 112 insertions, 26 deletions

diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java
index 8c5e99c8ecf..5b46ac54ab1 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java
@@ -19,6 +19,7 @@ public class CheckersMessages extends NLS {
 	private static final String BUNDLE_NAME = "org.eclipse.cdt.codan.internal.checkers.messages"; //$NON-NLS-1$
 	public static String NamingConventionFunctionChecker_LabelNamePattern;
 	public static String ReturnChecker_Param0;
+	public static String StatementHasNoEffectChecker_ParameterMacro;
 	static {
 		// initialize resource bundle
 		NLS.initializeMessages(BUNDLE_NAME, CheckersMessages.class);
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/StatementHasNoEffectChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/StatementHasNoEffectChecker.java
index 5e4d261df31..4b4a6149679 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/StatementHasNoEffectChecker.java
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/StatementHasNoEffectChecker.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.cdt.codan.internal.checkers;
 
+import org.eclipse.cdt.codan.core.cxx.CxxAstUtils;
 import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
 import org.eclipse.cdt.codan.core.model.IProblemWorkingCopy;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
@@ -38,8 +39,8 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTBinaryExpression;
  * 
  */
 public class StatementHasNoEffectChecker extends AbstractIndexAstChecker {
-	private static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.StatementHasNoEffectProblem"; //$NON-NLS-1$
-	private static final String PARAM_MACRO_ID = "macro";
+	public static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.StatementHasNoEffectProblem"; //$NON-NLS-1$
+	public static final String PARAM_MACRO_ID = "macro"; //$NON-NLS-1$
 
 	public void processAst(IASTTranslationUnit ast) {
 		ast.accept(new CheckStmpVisitor());
@@ -52,8 +53,15 @@ public class StatementHasNoEffectChecker extends AbstractIndexAstChecker {
 
 		public int visit(IASTStatement stmt) {
 			if (stmt instanceof IASTExpressionStatement) {
-				if (hasNoEffect(((IASTExpressionStatement) stmt).getExpression())) {
-					reportProblem(ER_ID, stmt, stmt.getRawSignature());
+				IASTExpression expression = ((IASTExpressionStatement) stmt)
+						.getExpression();
+				if (hasNoEffect(expression)) {
+					boolean inMacro = CxxAstUtils.getInstance().isInMacro(
+							expression);
+					boolean shouldReportInMacro = shouldReportInMacro();
+					if (inMacro && !shouldReportInMacro)
+						return PROCESS_SKIP;
+					reportProblem(ER_ID, stmt, expression.getRawSignature());
 				}
 				return PROCESS_SKIP;
 			}
@@ -71,11 +79,13 @@ public class StatementHasNoEffectChecker extends AbstractIndexAstChecker {
 		private boolean hasNoEffect(IASTExpression e) {
 			if (e instanceof IASTBinaryExpression) {
 				IASTBinaryExpression binExpr = (IASTBinaryExpression) e;
-				if (isLValue(binExpr)) return false;
+				if (isLValue(binExpr))
+					return false;
 				switch (binExpr.getOperator()) {
 					case IASTBinaryExpression.op_logicalOr:
 					case IASTBinaryExpression.op_logicalAnd:
-						return hasNoEffect(binExpr.getOperand1()) && hasNoEffect(binExpr.getOperand2());
+						return hasNoEffect(binExpr.getOperand1())
+								&& hasNoEffect(binExpr.getOperand2());
 				}
 				return true;
 			}
@@ -102,7 +112,9 @@ public class StatementHasNoEffectChecker extends AbstractIndexAstChecker {
 					IASTNode parent2 = parent.getParent();
 					if (parent2 instanceof IASTCompoundStatement) {
 						IASTNode parent3 = parent2.getParent();
-						if (parent3 instanceof IGNUASTCompoundStatementExpression) { return false; }
+						if (parent3 instanceof IGNUASTCompoundStatementExpression) {
+							return false;
+						}
 					}
 				}
 				return true;
@@ -112,7 +124,17 @@ public class StatementHasNoEffectChecker extends AbstractIndexAstChecker {
 	}
 
 	public void initPreferences(IProblemWorkingCopy problem) {
-		addPreference(problem, PARAM_MACRO_ID, "Check statements that belong to macro", Boolean.TRUE);
+		addPreference(problem, PARAM_MACRO_ID,
+				CheckersMessages.StatementHasNoEffectChecker_ParameterMacro,
+				Boolean.TRUE);
+	}
+
+	/**
+	 * @return
+	 */
+	public boolean shouldReportInMacro() {
+		return (Boolean) getPreference(getProblemById(ER_ID, getFile()),
+				PARAM_MACRO_ID);
 	}
 
 	@SuppressWarnings("restriction")
@@ -124,8 +146,7 @@ public class StatementHasNoEffectChecker extends AbstractIndexAstChecker {
 			ICPPFunction overload = cppBin.getOverload();
 			if (overload != null)
 				return false;
-			IType expressionType = cppBin.getOperand1()
-					.getExpressionType();
+			IType expressionType = cppBin.getOperand1().getExpressionType();
 			if (!(expressionType instanceof IBasicType)) {
 				return false; // must be overloaded but parser could not
 				// find it
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/messages.properties b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/messages.properties
index f6f1c85a477..35b8467d51e 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/messages.properties
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/messages.properties
@@ -1,2 +1,3 @@
 NamingConventionFunctionChecker_LabelNamePattern=Name Pattern
 ReturnChecker_Param0=Also check functions with implicit return value
+StatementHasNoEffectChecker_ParameterMacro=Report problem in statements that comes from macro expansion
diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/StatementHasNoEffectCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/StatementHasNoEffectCheckerTest.java
index a9f6adb3c51..707420d829a 100644
--- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/StatementHasNoEffectCheckerTest.java
+++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/StatementHasNoEffectCheckerTest.java
@@ -13,7 +13,12 @@ package org.eclipse.cdt.codan.core.internal.checkers;
 import java.io.File;
 import java.io.IOException;
 
+import org.eclipse.cdt.codan.core.CodanRuntime;
+import org.eclipse.cdt.codan.core.model.IProblem;
+import org.eclipse.cdt.codan.core.param.IProblemPreference;
+import org.eclipse.cdt.codan.core.param.MapProblemPreference;
 import org.eclipse.cdt.codan.core.test.CheckerTestCase;
+import org.eclipse.cdt.codan.internal.checkers.StatementHasNoEffectChecker;
 
 /**
  * Test for {@see StatementHasNoEffectChecker} class
@@ -105,7 +110,7 @@ public class StatementHasNoEffectCheckerTest extends CheckerTestCase {
 		checkErrorLine(f1, 3);
 		checkErrorLine(f2, 4);
 	}
-	
+
 	// main() {
 	// 	for (a=b;a;a=a->next);
 	// }
@@ -113,6 +118,7 @@ public class StatementHasNoEffectCheckerTest extends CheckerTestCase {
 		loadCodeAndRun(getAboveComment());
 		checkNoErrors();
 	}
+
 	// void main() {
 	// bool a;
 	// class c {};
@@ -123,7 +129,7 @@ public class StatementHasNoEffectCheckerTest extends CheckerTestCase {
 		loadCodeAndRunCpp(getAboveComment());
 		checkNoErrors();
 	}
-	
+
 	// main() {
 	// A a,b;
 	//
@@ -133,4 +139,48 @@ public class StatementHasNoEffectCheckerTest extends CheckerTestCase {
 		loadCodeAndRun(getAboveComment());
 		checkNoErrors();
 	}
+
+	//#define FUNC(a) a
+	// main() {
+	// int a;
+	//   FUNC(a); // error by default 
+	// }
+	@SuppressWarnings("restriction")
+	public void testInMacro() {
+		IProblemPreference macro = getMapPreference(
+				StatementHasNoEffectChecker.ER_ID,
+				StatementHasNoEffectChecker.PARAM_MACRO_ID);
+		macro.setValue(Boolean.TRUE);
+		loadCodeAndRun(getAboveComment());
+		checkErrorLine(4);
+	}
+
+	//#define FUNC(a) a
+	// main() {
+	// int a;
+	//   FUNC(a); // no error if macro exp turned off
+	// }
+	@SuppressWarnings("restriction")
+	public void testInMacroParamOff() {
+		IProblemPreference macro = getMapPreference(
+				StatementHasNoEffectChecker.ER_ID,
+				StatementHasNoEffectChecker.PARAM_MACRO_ID);
+		macro.setValue(Boolean.FALSE);
+		loadCodeAndRun(getAboveComment());
+		checkNoErrors();
+	}
+
+	/**
+	 * @param problemId
+	 * @param paramId
+	 * @return
+	 */
+	protected IProblemPreference getMapPreference(String problemId,
+			String paramId) {
+		IProblem problem = CodanRuntime.getInstance().getChechersRegistry()
+				.getWorkspaceProfile().findProblem(problemId);
+		IProblemPreference pref = ((MapProblemPreference) problem
+				.getPreference()).getChildDescriptor(paramId);
+		return pref;
+	}
 }
diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java
index 2cd6cbcfc85..cc2f973d34f 100644
--- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java
+++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java
@@ -24,7 +24,6 @@ import org.eclipse.core.runtime.NullProgressMonitor;
  * 
  */
 public class CheckerTestCase extends CodanTestCase {
-
 	private IMarker[] markers;
 
 	public void checkErrorLine(int i) {
@@ -32,19 +31,20 @@ public class CheckerTestCase extends CodanTestCase {
 	}
 
 	/**
-	 * @param i
+	 * @param expectedLine
 	 *            - line
 	 */
-	public void checkErrorLine(File file, int i) {
+	public void checkErrorLine(File file, int expectedLine) {
 		assertTrue(markers != null);
-		assertTrue(markers.length > 0);
+		assertTrue("No problems found but should", markers.length > 0); //$NON-NLS-1$
 		boolean found = false;
+		Integer line = null;
+		String mfile = null;
 		for (int j = 0; j < markers.length; j++) {
 			IMarker m = markers[j];
-			Object line = null;
 			Object pos;
 			try {
-				line = m.getAttribute(IMarker.LINE_NUMBER);
+				line = (Integer) m.getAttribute(IMarker.LINE_NUMBER);
 				if (line == null || line.equals(-1)) {
 					pos = m.getAttribute(IMarker.CHAR_START);
 					line = new Integer(pos2line(((Integer) pos).intValue()));
@@ -54,18 +54,24 @@ public class CheckerTestCase extends CodanTestCase {
 			} catch (IOException e) {
 				fail(e.getMessage());
 			}
-			String mfile = m.getResource().getName();
-			if (line.equals(i)) {
+			mfile = m.getResource().getName();
+			if (line.equals(expectedLine)) {
 				found = true;
-				if (file != null && !file.getName().equals(mfile)) found = false;
-				else break;
+				if (file != null && !file.getName().equals(mfile))
+					found = false;
+				else
+					break;
 			}
 		}
-		assertTrue("Error on line " + i + " not found ", found);
+		assertEquals(Integer.valueOf(expectedLine), line);
+		if (file != null)
+			assertEquals(file.getName(), mfile);
+		assertTrue(found);
 	}
 
 	public void checkNoErrors() {
-		assertTrue("Found errors but should not", markers == null || markers.length == 0);
+		assertTrue("Found errors but should not", markers == null
+				|| markers.length == 0);
 	}
 
 	/**
@@ -94,9 +100,16 @@ public class CheckerTestCase extends CodanTestCase {
 	 * 
 	 */
 	protected void runCodan() {
-		CodanRuntime.getInstance().getBuilder().processResource(cproject.getProject(), new NullProgressMonitor());
+		CodanRuntime
+				.getInstance()
+				.getBuilder()
+				.processResource(cproject.getProject(),
+						new NullProgressMonitor());
 		try {
-			markers = cproject.getProject().findMarkers(IProblemReporter.GENERIC_CODE_ANALYSIS_MARKER_TYPE, true, 1);
+			markers = cproject.getProject()
+					.findMarkers(
+							IProblemReporter.GENERIC_CODE_ANALYSIS_MARKER_TYPE,
+							true, 1);
 		} catch (CoreException e) {
 			fail(e.getMessage());
 		}