From 26918193cff28d121bd55a24cc09bd8f4eff540f Mon Sep 17 00:00:00 2001 From: Alena Laskavaia Date: Wed, 24 Mar 2010 03:31:35 +0000 Subject: [PATCH] - fixed f.p and added test --- .../checkers/SuggestedParenthesisChecker.java | 76 +++++++------------ .../StatementHasNoEffectCheckerTest.java | 2 +- .../SuggestedParenthesisCheckerTest.java | 43 ++++++++++- .../core/test/AutomatedIntegrationSuite.java | 4 +- 4 files changed, 72 insertions(+), 53 deletions(-) rename codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/{checkers/sample => internal/checkers}/StatementHasNoEffectCheckerTest.java (97%) rename codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/{checkers/sample => internal/checkers}/SuggestedParenthesisCheckerTest.java (65%) diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SuggestedParenthesisChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SuggestedParenthesisChecker.java index b19e0e440f0..487a1f3645a 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/SuggestedParenthesisChecker.java +++ b/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); - if (precedence == 2) { // unary not - if (isUsedAsOperand(expression)) { - reportProblem(ER_ID, expression, - "Suggested parenthesis around expression"); - return PROCESS_SKIP; - } - } - if (precedence >= 0) { - synchronized (svis) { // since we use only one instance of this - // visitor sync just in case - svis.init(expression); - expression.accept(svis); - if (svis.suspicious == true) { - reportProblem(ER_ID, svis.other, + 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) { + int pp = getPrecedence(parentExpr); + if (pp == -1 || pp == precedence) + return PROCESS_CONTINUE; + reportProblem(ER_ID, expression, + "Suggested parenthesis around expression"); } } 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; - IASTExpression other; - boolean suspicious = false; - - void init(IASTExpression e) { - parent = e; - suspicious = false; - } - - 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; - } + /** + * @param parent + * @return + */ + private boolean isInParentesis(IASTExpression node) { + IASTNode parent = node.getParent(); + if (parent instanceof IASTUnaryExpression) { + IASTUnaryExpression br = (IASTUnaryExpression) parent; + if (br.getOperator() == IASTUnaryExpression.op_bracketedPrimary) { + return true; } - if (getPrecedence(expression) < 0) // not considered operator - return PROCESS_CONTINUE; - if (getPrecedence(expression) == getPrecedence(parent)) { - return PROCESS_SKIP; - } - suspicious = true; - other = expression; - return PROCESS_ABORT; } + return false; } } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/checkers/sample/StatementHasNoEffectCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/StatementHasNoEffectCheckerTest.java similarity index 97% rename from codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/checkers/sample/StatementHasNoEffectCheckerTest.java rename to codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/StatementHasNoEffectCheckerTest.java index 48b87b94157..0511ee75357 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/checkers/sample/StatementHasNoEffectCheckerTest.java +++ b/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; diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/checkers/sample/SuggestedParenthesisCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/SuggestedParenthesisCheckerTest.java similarity index 65% rename from codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/checkers/sample/SuggestedParenthesisCheckerTest.java rename to codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/SuggestedParenthesisCheckerTest.java index f3435826592..386ddb5e88c 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/checkers/sample/SuggestedParenthesisCheckerTest.java +++ b/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(); } + /*- + + main() { + int a=1,b=3; + if (a && !b) b=4; // no error here on line 3 + } + + */ + public void test_lastnot() { + load("test4.c"); + runOnFile(); + checkNoErrors(); + } + + /*- + + main() { + int a=1,b=3; + if ((!a) && 10) b=4; // no error here on line 3 + } + + */ + public void test_fixed() { + load("test5.c"); + runOnFile(); + checkNoErrors(); + } + + /*- + + main() { + int a=1,b=3; + if (a && b & a) b=4; // error here on line 3 + } + + */ + public void test_mixedbin() { + load("test6.c"); + runOnFile(); + checkErrorLine(3); + } } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java index 41df6b7bd78..2a18383b911 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java +++ b/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.checkers.sample.SuggestedParenthesisCheckerTest; +import org.eclipse.cdt.codan.core.internal.checkers.StatementHasNoEffectCheckerTest; +import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerTest; public class AutomatedIntegrationSuite extends TestSuite { public AutomatedIntegrationSuite() {