- fixed f.p and added test

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

@@ -15,6 +15,7 @@ import org.eclipse.cdt.core.dom.ast.ASTNodeProperty;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
 import org.eclipse.cdt.core.dom.ast.IASTExpression;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
 
@@ -37,32 +38,29 @@ public class SuggestedParenthesisChecker extends AbstractIndexAstChecker {
 	}
 
 	class ExpressionVisitor extends ASTVisitor {
-		private SuspiciousExpressionVisitor svis;
-
 		ExpressionVisitor() {
 			shouldVisitExpressions = true;
-			svis = new SuspiciousExpressionVisitor();
 		}
 
 		public int visit(IASTExpression expression) {
 			int precedence = getPrecedence(expression);
+			IASTNode parent = expression.getParent();
+			if (parent instanceof IASTExpression) {
+				IASTExpression parentExpr = (IASTExpression) parent;
+				if (isInParentesis(expression))
+					return PROCESS_CONTINUE;
 				if (precedence == 2) { // unary not
 					if (isUsedAsOperand(expression)) {
 						reportProblem(ER_ID, expression,
 								"Suggested parenthesis around expression");
 						return PROCESS_SKIP;
 					}
-			}
+				} else if (precedence >= 0) {
-			if (precedence >= 0) {
+					int pp = getPrecedence(parentExpr);
-				synchronized (svis) { // since we use only one instance of this
+					if (pp == -1 || pp == precedence)
-										// visitor sync just in case
+						return PROCESS_CONTINUE;
-					svis.init(expression);
+					reportProblem(ER_ID, expression,
-					expression.accept(svis);
-					if (svis.suspicious == true) {
-						reportProblem(ER_ID, svis.other,
 							"Suggested parenthesis around expression");
-						return PROCESS_SKIP;
-					}
 				}
 			}
 			return PROCESS_CONTINUE;
@@ -71,7 +69,7 @@ public class SuggestedParenthesisChecker extends AbstractIndexAstChecker {
 		private boolean isUsedAsOperand(IASTExpression expression) {
 			ASTNodeProperty prop = expression.getPropertyInParent();
 			if (prop == IASTBinaryExpression.OPERAND_ONE
-					|| prop == IASTBinaryExpression.OPERAND_TWO
+			// || prop == IASTBinaryExpression.OPERAND_TWO
 					|| prop == IASTUnaryExpression.OPERAND)
 				return true;
 			return false;
@@ -104,38 +102,18 @@ public class SuggestedParenthesisChecker extends AbstractIndexAstChecker {
 		return -1;
 	}
 
-	class SuspiciousExpressionVisitor extends ASTVisitor {
+	/**
-		IASTExpression parent;
+	 * @param parent
-		IASTExpression other;
+	 * @return
-		boolean suspicious = false;
+	 */
-
+	private boolean isInParentesis(IASTExpression node) {
-		void init(IASTExpression e) {
+		IASTNode parent = node.getParent();
-			parent = e;
+		if (parent instanceof IASTUnaryExpression) {
-			suspicious = false;
+			IASTUnaryExpression br = (IASTUnaryExpression) parent;
-		}
+			if (br.getOperator() == IASTUnaryExpression.op_bracketedPrimary) {
-
+				return true;
-		SuspiciousExpressionVisitor() {
-			shouldVisitExpressions = true;
-		}
-
-		public int visit(IASTExpression expression) {
-			if (expression == parent)
-				return PROCESS_CONTINUE;
-			if (expression instanceof IASTUnaryExpression) {
-				IASTUnaryExpression uExpr = (IASTUnaryExpression) expression;
-				int operator = uExpr.getOperator();
-				if (operator == IASTUnaryExpression.op_bracketedPrimary) {
-					return PROCESS_SKIP;
 			}
 		}
-			if (getPrecedence(expression) < 0) // not considered operator
+		return false;
-				return PROCESS_CONTINUE;
-			if (getPrecedence(expression) == getPrecedence(parent)) {
-				return PROCESS_SKIP;
-			}
-			suspicious = true;
-			other = expression;
-			return PROCESS_ABORT;
-		}
 	}
 }

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

@@ -8,7 +8,7 @@
  * Contributors:
  *    Alena Laskavaia  - initial API and implementation
  *******************************************************************************/
-package org.eclipse.cdt.codan.core.checkers.sample;
+package org.eclipse.cdt.codan.core.internal.checkers;
 
 import org.eclipse.cdt.codan.core.test.CheckerTestCase;
 

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

@@ -8,7 +8,7 @@
  * Contributors:
  *    Alena Laskavaia  - initial API and implementation
  *******************************************************************************/
-package org.eclipse.cdt.codan.core.checkers.sample;
+package org.eclipse.cdt.codan.core.internal.checkers;
 
 import org.eclipse.cdt.codan.core.test.CheckerTestCase;
 
@@ -59,4 +59,45 @@ public class SuggestedParenthesisCheckerTest extends CheckerTestCase {
 		runOnFile();
 		checkNoErrors();
 	}
+	/*-
+	 <code file="test4.c">
+	 main() {
+      int a=1,b=3;
+	   if (a && !b) b=4; // no error here on line 3
+	 }
+	 </code>
+	 */
+	public void test_lastnot() {
+		load("test4.c");
+		runOnFile();
+		checkNoErrors();
+	}
+	
+	/*-
+	 <code file="test5.c">
+	 main() {
+       int a=1,b=3;
+	   if ((!a) && 10) b=4; // no error here on line 3
+	 }
+	 </code>
+	 */
+	public void test_fixed() {
+		load("test5.c");
+		runOnFile();
+		checkNoErrors();
+	}
+	
+	/*-
+	 <code file="test6.c">
+	 main() {
+      int a=1,b=3;
+	   if (a && b & a) b=4; //  error here on line 3
+	 }
+	 </code>
+	 */
+	public void test_mixedbin() {
+		load("test6.c");
+		runOnFile();
+		checkErrorLine(3);
+	}
 }

codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java

@@ -14,8 +14,8 @@ package org.eclipse.cdt.codan.core.test;
 import junit.framework.Test;
 import junit.framework.TestSuite;
 
-import org.eclipse.cdt.codan.core.checkers.sample.StatementHasNoEffectCheckerTest;
+import org.eclipse.cdt.codan.core.internal.checkers.StatementHasNoEffectCheckerTest;
-import org.eclipse.cdt.codan.core.checkers.sample.SuggestedParenthesisCheckerTest;
+import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerTest;
 
 public class AutomatedIntegrationSuite extends TestSuite {
 	public AutomatedIntegrationSuite() {