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>

10 files changed, 59 insertions, 21 deletions

diff --git a/basic/source/sbx/sbxcurr.cxx b/basic/source/sbx/sbxcurr.cxx
index e4d525bd794e..70856e7b602c 100644
--- a/basic/source/sbx/sbxcurr.cxx
+++ b/basic/source/sbx/sbxcurr.cxx
@@ -110,7 +110,7 @@ static sal_Int64 ImpStringToCurrency( const OUString &rStr )
     if ( *p == '-'  || *p == '+' )
         sNormalisedNumString.append( *p );
 
-    while ( ( *p >= '0' && *p <= '9' ) )
+    while ( *p >= '0' && *p <= '9' )
     {
         sNormalisedNumString.append( *p++ );
         // #TODO in vba mode set runtime error when a space ( or other )
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);
diff --git a/editeng/source/editeng/impedit4.cxx b/editeng/source/editeng/impedit4.cxx
index 4e284d35e82f..0aae2172e881 100644
--- a/editeng/source/editeng/impedit4.cxx
+++ b/editeng/source/editeng/impedit4.cxx
@@ -2285,7 +2285,7 @@ void ImpEditEngine::DoOnlineSpelling( ContentNode* pThisNodeOnly, bool bSpellAtC
 
             EditPaM aPaM( pNode, nInvStart );
             EditSelection aSel( aPaM, aPaM );
-            while ( ( aSel.Max().GetNode() == pNode ) /* && !bStop */ )
+            while ( aSel.Max().GetNode() == pNode )
             {
                 if ( ( static_cast<size_t>(aSel.Min().GetIndex()) > nInvEnd )
                         || ( ( aSel.Max().GetNode() == pLastNode ) && ( aSel.Max().GetIndex() >= pLastNode->Len() ) ) )
diff --git a/sal/osl/unx/process.cxx b/sal/osl/unx/process.cxx
index 85668f952554..96829d71a96c 100644
--- a/sal/osl/unx/process.cxx
+++ b/sal/osl/unx/process.cxx
@@ -260,7 +260,7 @@ static void ChildStatusProc(void *pData)
 
         if (pid > 0)
         {
-            while (((i = read(channel[0], &status, sizeof(status))) < 0))
+            while ((i = read(channel[0], &status, sizeof(status))) < 0)
             {
                 if (errno != EINTR)
                     break;
diff --git a/sc/source/ui/Accessibility/AccessibleDocument.cxx b/sc/source/ui/Accessibility/AccessibleDocument.cxx
index c514f914c2bd..1d326f5927b9 100644
--- a/sc/source/ui/Accessibility/AccessibleDocument.cxx
+++ b/sc/source/ui/Accessibility/AccessibleDocument.cxx
@@ -969,7 +969,7 @@ bool ScChildrenShapes::FindSelectedShapesChanges(const uno::Reference<drawing::X
     SortedShapes::iterator aDataItr(maZOrderedShapes.begin());
     SortedShapes::const_iterator aDataEndItr(maZOrderedShapes.end());
     SortedShapes::const_iterator aFocusedItr = aDataEndItr;
-    while((aDataItr != aDataEndItr))
+    while(aDataItr != aDataEndItr)
     {
         if (*aDataItr) // is it really a shape or only the sheet
         {
diff --git a/svl/source/numbers/zforfind.cxx b/svl/source/numbers/zforfind.cxx
index c0c7b4816841..760f001f1dbf 100644
--- a/svl/source/numbers/zforfind.cxx
+++ b/svl/source/numbers/zforfind.cxx
@@ -1472,14 +1472,14 @@ DateOrder ImpSvNumberInputScan::GetDateOrder()
         switch ((nOrder & 0xff00) >> 8)
         {
         case 'Y':
-            switch ((nOrder & 0xff))
+            switch (nOrder & 0xff)
             {
             case 'M':
                 return DateOrder::YMD;
             }
             break;
         case 'M':
-            switch ((nOrder & 0xff))
+            switch (nOrder & 0xff)
             {
             case 'Y':
                 return DateOrder::DMY;
@@ -1488,7 +1488,7 @@ DateOrder ImpSvNumberInputScan::GetDateOrder()
             }
             break;
         case 'D':
-            switch ((nOrder & 0xff))
+            switch (nOrder & 0xff)
             {
             case 'Y':
                 return DateOrder::MDY;
@@ -1498,7 +1498,7 @@ DateOrder ImpSvNumberInputScan::GetDateOrder()
             break;
         default:
         case 0:
-            switch ((nOrder & 0xff))
+            switch (nOrder & 0xff)
             {
             case 'Y':
                 return DateOrder::YMD;
diff --git a/sw/source/core/unocore/unofield.cxx b/sw/source/core/unocore/unofield.cxx
index f74834998b65..92258723e11a 100644
--- a/sw/source/core/unocore/unofield.cxx
+++ b/sw/source/core/unocore/unofield.cxx
@@ -215,7 +215,7 @@ static SwServiceType lcl_GetServiceForField( const SwField& rField )
     case SwFieldIds::DocInfo:
         {
             const sal_uInt16 nSubType = rField.GetSubType();
-            switch( (nSubType & 0xff))
+            switch( nSubType & 0xff )
             {
             case DI_CHANGE:
                 nSrvId = ((nSubType&0x300) == DI_SUB_AUTHOR)
diff --git a/sw/source/uibase/app/docstyle.cxx b/sw/source/uibase/app/docstyle.cxx
index 81b3f60bb3b3..fb1a4328b12d 100644
--- a/sw/source/uibase/app/docstyle.cxx
+++ b/sw/source/uibase/app/docstyle.cxx
@@ -2799,7 +2799,7 @@ SfxStyleSheetBase*  SwStyleSheetIterator::First()
             if( !(bIsSearchUsed && bUsed ))
             {
                 const sal_uInt16 nId = pColl->GetPoolFormatId();
-                switch ( (nSMask & ~SFXSTYLEBIT_USED) )
+                switch ( nSMask & ~SFXSTYLEBIT_USED )
                 {
                 case SFXSTYLEBIT_USERDEF:
                     if(!IsPoolUserFormat(nId)) continue;
diff --git a/vcl/source/gdi/pngread.cxx b/vcl/source/gdi/pngread.cxx
index 9bebfb324d85..688d017edca1 100644
--- a/vcl/source/gdi/pngread.cxx
+++ b/vcl/source/gdi/pngread.cxx
@@ -902,7 +902,7 @@ void PNGReaderImpl::ImplReadIDAT()
         mpZCodec.SetBreak( mnChunkLen );
         SvMemoryStream aIStrm( &(*maDataIter), mnChunkLen, StreamMode::READ );
 
-        while ( ( mpZCodec.GetBreak() ) )
+        while ( mpZCodec.GetBreak() )
         {
             // get bytes needed to fill the current scanline
             sal_Int32 nToRead = mnScansize - (mpScanCurrent - mpInflateInBuf);