teach unnecessaryparen plugin about other kinds of statements

i.e. do / while / switch

Change-Id: Id0985015cc425557f9984734701d56466f8a6088
Reviewed-on: https://gerrit.libreoffice.org/39601
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 48 insertions, 10 deletions

diff --git a/compilerplugins/clang/test/unnecessaryparen.cxx b/compilerplugins/clang/test/unnecessaryparen.cxx
index 62b4b69d3271..201032a703ae 100644
--- a/compilerplugins/clang/test/unnecessaryparen.cxx
+++ b/compilerplugins/clang/test/unnecessaryparen.cxx
@@ -14,7 +14,7 @@ int main()
     int x = 1;
     x = ((2)); // expected-error {{parentheses around parentheses [loplugin:unnecessaryparen]}}
 
-    if ((foo(1))) foo(2); // expected-error {{parentheses immediately inside if [loplugin:unnecessaryparen]}}
+    if ((foo(1))) foo(2); // expected-error {{parentheses immediately inside if statement [loplugin:unnecessaryparen]}}
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unnecessaryparen.cxx b/compilerplugins/clang/unnecessaryparen.cxx
index cf65ecbabacf..e04468fdf096 100644
--- a/compilerplugins/clang/unnecessaryparen.cxx
+++ b/compilerplugins/clang/unnecessaryparen.cxx
@@ -38,11 +38,22 @@ public:
              return;
         if (loplugin::hasPathnamePrefix(fn, WORKDIR "/YaccTarget/idlc/source/parser.cxx"))
              return;
+        // TODO yuck, comma operator at work
+        if (loplugin::hasPathnamePrefix(fn, SRCDIR "/writerfilter/source/rtftok/rtftokenizer.cxx"))
+             return;
+        if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/source/filter/html/htmltab.cxx"))
+             return;
+
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
     }
 
     bool VisitParenExpr(const ParenExpr *);
     bool VisitIfStmt(const IfStmt *);
+    bool VisitDoStmt(const DoStmt *);
+    bool VisitWhileStmt(const WhileStmt *);
+    bool VisitSwitchStmt(const SwitchStmt *);
+private:
+    void VisitSomeStmt(const Stmt *parent, const Expr* cond, StringRef stmtName);
 };
 
 bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
@@ -66,22 +77,49 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
 
 bool UnnecessaryParen::VisitIfStmt(const IfStmt* ifStmt)
 {
-    if (ignoreLocation(ifStmt))
-        return true;
+    VisitSomeStmt(ifStmt, ifStmt->getCond(), "if");
+    return true;
+}
 
-    if (auto parenExpr = dyn_cast<ParenExpr>(ifStmt->getCond()->IgnoreImpCasts())) {
+bool UnnecessaryParen::VisitDoStmt(const DoStmt* doStmt)
+{
+    VisitSomeStmt(doStmt, doStmt->getCond(), "do");
+    return true;
+}
+
+bool UnnecessaryParen::VisitWhileStmt(const WhileStmt* whileStmt)
+{
+    VisitSomeStmt(whileStmt, whileStmt->getCond(), "while");
+    return true;
+}
+
+bool UnnecessaryParen::VisitSwitchStmt(const SwitchStmt* switchStmt)
+{
+    VisitSomeStmt(switchStmt, switchStmt->getCond(), "switch");
+    return true;
+}
+
+void UnnecessaryParen::VisitSomeStmt(const Stmt *parent, const Expr* cond, StringRef stmtName)
+{
+    if (ignoreLocation(parent))
+        return;
+    if (parent->getLocStart().isMacroID())
+        return;
+
+    auto parenExpr = dyn_cast<ParenExpr>(cond->IgnoreImpCasts());
+    if (parenExpr) {
         if (parenExpr->getLocStart().isMacroID())
-            return true;
+            return;
         // assignments need extra parentheses or they generate a compiler warning
         auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr());
         if (binaryOp && binaryOp->getOpcode() == BO_Assign)
-            return true;
+            return;
         report(
-            DiagnosticsEngine::Warning, "parentheses immediately inside if",
-            ifStmt->getLocStart())
-            << ifStmt->getSourceRange();
+            DiagnosticsEngine::Warning, "parentheses immediately inside %0 statement",
+            parenExpr->getLocStart())
+            << stmtName
+            << parenExpr->getSourceRange();
     }
-    return true;
 }
 
 loplugin::Plugin::Registration< UnnecessaryParen > X("unnecessaryparen", true);