Fixes oclint#16 for SwitchCase has multiple sub statements

sprohaszka · Mar 14, 2013 · 4d07049 · 4d07049
1 parent 7dc83d1
commit 4d07049
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 6 deletions.
diff --git a/oclint-rules/rules/convention/MissingBreakInSwitchStatementRule.cpp b/oclint-rules/rules/convention/MissingBreakInSwitchStatementRule.cpp
@@ -56,18 +56,48 @@ class MissingBreakInSwitchStatementRule :
         return 3;
     }
 
+    bool isSwitchCase(Stmt *stmt)
+    {
+        return stmt && isa<SwitchCase>(stmt);
+    }
+
+    bool isBreakingPoint(Stmt *stmt)
+    {
+        return stmt && (isa<BreakStmt>(stmt) ||
+                isa<ReturnStmt>(stmt) || isa<CXXThrowExpr>(stmt) || isa<ObjCAtThrowStmt>(stmt));
+    }
+
     bool VisitSwitchStmt(SwitchStmt *switchStmt)
     {
-        SwitchCase *currentSwitchCase = switchStmt->getSwitchCaseList();
-        while (currentSwitchCase)
+        CompoundStmt *compoundStmt = dyn_cast<CompoundStmt>(switchStmt->getBody());
+        if (compoundStmt)
         {
-            FindingBreak findingBreak;
-            if (!findingBreak.findBreak(currentSwitchCase))
+            bool breakFound = true;
+            for (CompoundStmt::body_iterator body = compoundStmt->body_begin(),
+                bodyEnd = compoundStmt->body_end(); body != bodyEnd; body++)
+            {
+                Stmt *bodyStmt = dyn_cast<Stmt>(*body);
+                if (isBreakingPoint(bodyStmt))
+                {
+                    breakFound = true;
+                    continue;
+                }
+                if (isSwitchCase(bodyStmt))
+                {
+                    if (!breakFound)
+                    {
+                        addViolation(switchStmt, this);
+                        break;
+                    }
+
+                    FindingBreak findingBreak;
+                    breakFound = findingBreak.findBreak(dyn_cast<SwitchCase>(bodyStmt));
+                }
+            }
+            if (!breakFound)
             {
                 addViolation(switchStmt, this);
-                break;
             }
-            currentSwitchCase = currentSwitchCase->getNextSwitchCase();
         }
 
         return true;

diff --git a/oclint-rules/test/convention/MissingBreakInSwitchStatementRuleTest.cpp b/oclint-rules/test/convention/MissingBreakInSwitchStatementRuleTest.cpp
@@ -23,6 +23,16 @@ case 4:     \n\
 } }");
 }
 
+TEST(MissingBreakInSwitchStatementRuleTest, OneCaseWithBreak)
+{
+    testRuleOnCode(new MissingBreakInSwitchStatementRule(), "void m(); void aMethod(int a) { \n\
+switch(a){  \n\
+case 1:     \n\
+\tm();      \n\
+\tbreak;    \n\
+} }");
+}
+
 TEST(MissingBreakInSwitchStatementRuleTest, OneCaseHasNoBreak)
 {
     testRuleOnCode(new MissingBreakInSwitchStatementRule(), "void m(); void aMethod(int a) { switch(a){\n\
@@ -116,6 +126,22 @@ default:      \n\
 } }");
 }
 
+/*
+ Tests for the false positive found by Stephan Esch
+ Details at https://github.com/oclint/oclint/issues/16
+*/
+
+TEST(MissingBreakInSwitchStatementRuleTest, CasesHaveAssignmentAndBreak)
+{
+    testRuleOnCode(new MissingBreakInSwitchStatementRule(), "void aMethod(int a) { \n\
+int i;      \n\
+switch(a){  \n\
+case 1:     \n\
+\ti = 1;    \n\
+\tbreak;    \n\
+} }");
+}
+
 int main(int argc, char **argv)
 {
     ::testing::InitGoogleMock(&argc, argv);