new loplugin:redundantpointerops

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

13 files changed, 179 insertions, 11 deletions

diff --git a/chart2/source/model/main/UndoManager.cxx b/chart2/source/model/main/UndoManager.cxx
index 7e5894b012cb..6d6e9ef915da 100644
--- a/chart2/source/model/main/UndoManager.cxx
+++ b/chart2/source/model/main/UndoManager.cxx
@@ -319,7 +319,7 @@ namespace chart
     Reference< XInterface > SAL_CALL UndoManager::getParent(  )
     {
         UndoManagerMethodGuard aGuard( *m_pImpl );
-        return *&m_pImpl->getParent();
+        return m_pImpl->getParent();
     }
 
     void SAL_CALL UndoManager::setParent( const Reference< XInterface >& )
diff --git a/compilerplugins/clang/redundantpointerops.cxx b/compilerplugins/clang/redundantpointerops.cxx
new file mode 100644
index 000000000000..6a88cdb13d70
--- /dev/null
+++ b/compilerplugins/clang/redundantpointerops.cxx
@@ -0,0 +1,129 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+
+#include <clang/AST/CXXInheritance.h>
+#include "compat.hxx"
+#include "plugin.hxx"
+
+/**
+ * Look for:
+ *     (&x)->y
+ * which can be tranformed to:
+ *      x.y
+ * And
+ *    &*x
+ * which can be:
+ *    x
+ *
+ * @TODO
+ *    (*x).y
+ *  which can be:
+ *    x->y
+ */
+
+namespace {
+
+class RedundantPointerOps:
+    public RecursiveASTVisitor<RedundantPointerOps>, public loplugin::Plugin
+{
+public:
+    explicit RedundantPointerOps(InstantiationData const & data): Plugin(data) {}
+
+    virtual void run() override
+    {
+        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool VisitFunctionDecl(FunctionDecl const *);
+    bool VisitMemberExpr(MemberExpr const *);
+    bool VisitUnaryOperator(UnaryOperator const *);
+};
+
+bool RedundantPointerOps::VisitFunctionDecl(FunctionDecl const * functionDecl)
+{
+    if (ignoreLocation(functionDecl))
+        return true;
+    //functionDecl->dump();
+    return true;
+}
+
+bool RedundantPointerOps::VisitMemberExpr(MemberExpr const * memberExpr)
+{
+    if (ignoreLocation(memberExpr))
+        return true;
+    if (memberExpr->getLocStart().isMacroID())
+        return true;
+    auto base = memberExpr->getBase()->IgnoreParenImpCasts();
+            //parentStmt(parentStmt(memberExpr))->dump();
+    if (memberExpr->isArrow())
+    {
+        if (auto unaryOp = dyn_cast<UnaryOperator>(base))
+        {
+            if (unaryOp->getOpcode() == UO_AddrOf)
+                report(
+                    DiagnosticsEngine::Warning, "'&' followed by '->', rather use '.'",
+                    memberExpr->getLocStart())
+                    << memberExpr->getSourceRange();
+
+        }
+        else if (auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(base))
+        {
+            if (operatorCallExpr->getOperator() == OO_Amp)
+                report(
+                    DiagnosticsEngine::Warning, "'&' followed by '->', rather use '.'",
+                    memberExpr->getLocStart())
+                    << memberExpr->getSourceRange();
+
+        }
+    }
+//    else
+//    {
+//        if (auto unaryOp = dyn_cast<UnaryOperator>(base))
+//        {
+//            if (unaryOp->getOpcode() == UO_Deref)
+//                report(
+//                    DiagnosticsEngine::Warning, "'*' followed by '.', rather use '->'",
+//                    memberExpr->getLocStart())
+//                    << memberExpr->getSourceRange();
+//
+//        }
+//    }
+    return true;
+}
+
+bool RedundantPointerOps::VisitUnaryOperator(UnaryOperator const * unaryOperator)
+{
+    if (ignoreLocation(unaryOperator))
+        return true;
+    if (unaryOperator->getLocStart().isMacroID())
+        return true;
+    if (unaryOperator->getOpcode() != UO_Deref)
+        return true;
+    auto innerOp = dyn_cast<UnaryOperator>(unaryOperator->getSubExpr()->IgnoreParenImpCasts());
+    if (!innerOp || innerOp->getOpcode() != UO_AddrOf)
+        return true;
+
+    report(
+        DiagnosticsEngine::Warning, "'&' followed by '*', rather use '.'",
+        unaryOperator->getLocStart())
+        << unaryOperator->getSourceRange();
+    return true;
+}
+
+loplugin::Plugin::Registration< RedundantPointerOps > X("redundantpointerops");
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/redundantpointerops.cxx b/compilerplugins/clang/test/redundantpointerops.cxx
new file mode 100644
index 000000000000..c218c089caba
--- /dev/null
+++ b/compilerplugins/clang/test/redundantpointerops.cxx
@@ -0,0 +1,38 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+struct Struct1 {
+    int x;
+};
+
+void function1(Struct1& s)
+{
+    (&s)->x = 1; // expected-error {{'&' followed by '->', rather use '.' [loplugin:redundantpointerops]}}
+};
+
+struct Struct2 {
+    int x;
+    Struct2* operator&() { return this; }
+};
+
+void function2(Struct2 s)
+{
+    (&s)->x = 1; // expected-error {{'&' followed by '->', rather use '.' [loplugin:redundantpointerops]}}
+};
+
+void function3(Struct1& s)
+{
+    (*(&s)).x = 1; // expected-error {{'&' followed by '*', rather use '.' [loplugin:redundantpointerops]}}
+};
+
+//void function4(Struct1* s)
+//{
+//    (*s).x = 1; // xxexpected-error {{'*' followed by '.', rather use '->' [loplugin:redundantpointerops]}}
+//};
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/cui/source/tabpages/tpgradnt.cxx b/cui/source/tabpages/tpgradnt.cxx
index 2e38b9db1db0..ba3a550cb2af 100644
--- a/cui/source/tabpages/tpgradnt.cxx
+++ b/cui/source/tabpages/tpgradnt.cxx
@@ -202,7 +202,7 @@ void SvxGradientTabPage::ActivatePage( const SfxItemSet& rSet )
         else
             aString += aURL.getBase();
 
-        sal_Int32 nPos = SearchGradientList( ( &static_cast<const XFillGradientItem&>( rSet.Get(XATTR_FILLGRADIENT) ) )->GetName() );
+        sal_Int32 nPos = SearchGradientList( static_cast<const XFillGradientItem&>( rSet.Get(XATTR_FILLGRADIENT) ).GetName() );
         if ( nPos != LISTBOX_ENTRY_NOTFOUND )
         {
             sal_uInt16 nId = m_pGradientLB->GetItemId( static_cast<size_t>( nPos ) );
diff --git a/cui/source/tabpages/tphatch.cxx b/cui/source/tabpages/tphatch.cxx
index ca9c2abb777e..d9978103e212 100644
--- a/cui/source/tabpages/tphatch.cxx
+++ b/cui/source/tabpages/tphatch.cxx
@@ -189,7 +189,7 @@ void SvxHatchTabPage::ActivatePage( const SfxItemSet& rSet )
         else
             aString += aURL.getBase();
 
-        sal_Int32 nPos = SearchHatchList( ( &static_cast<const XFillHatchItem&>( rSet.Get(XATTR_FILLHATCH)) )->GetName() );
+        sal_Int32 nPos = SearchHatchList( static_cast<const XFillHatchItem&>( rSet.Get(XATTR_FILLHATCH) ).GetName() );
         if( nPos != LISTBOX_ENTRY_NOTFOUND )
         {
             sal_uInt16 nId = m_pHatchLB->GetItemId( static_cast<size_t>( nPos ) );
diff --git a/cui/source/tabpages/tppattern.cxx b/cui/source/tabpages/tppattern.cxx
index 7b6cf2c63649..95746d9ca8d1 100644
--- a/cui/source/tabpages/tppattern.cxx
+++ b/cui/source/tabpages/tppattern.cxx
@@ -188,7 +188,7 @@ void SvxPatternTabPage::ActivatePage( const SfxItemSet& rSet )
         else
             aString += aURL.getBase();
 
-        sal_Int32 nPos = SearchPatternList( ( &static_cast<const XFillBitmapItem&>( rSet.Get(XATTR_FILLBITMAP)) )->GetName() );
+        sal_Int32 nPos = SearchPatternList( static_cast<const XFillBitmapItem&>( rSet.Get(XATTR_FILLBITMAP)).GetName() );
         if( nPos != LISTBOX_ENTRY_NOTFOUND )
         {
             sal_uInt16 nId = m_pPatternLB->GetItemId( static_cast<size_t>( nPos ) );
diff --git a/dbaccess/source/ui/misc/dbaundomanager.cxx b/dbaccess/source/ui/misc/dbaundomanager.cxx
index aaa961254fd1..2b8c5f06e1f1 100644
--- a/dbaccess/source/ui/misc/dbaundomanager.cxx
+++ b/dbaccess/source/ui/misc/dbaundomanager.cxx
@@ -309,7 +309,7 @@ namespace dbaui
     Reference< XInterface > SAL_CALL UndoManager::getParent(  )
     {
         UndoManagerMethodGuard aGuard( *m_xImpl );
-        return *&m_xImpl->rParent;
+        return m_xImpl->rParent;
     }
 
     void SAL_CALL UndoManager::setParent( const Reference< XInterface >& )
diff --git a/extensions/source/propctrlr/propertycomposer.hxx b/extensions/source/propctrlr/propertycomposer.hxx
index a5902adeb19f..8f2f1fc102a5 100644
--- a/extensions/source/propctrlr/propertycomposer.hxx
+++ b/extensions/source/propctrlr/propertycomposer.hxx
@@ -135,7 +135,7 @@ namespace pcr
                 : ::osl::MutexGuard( _rInstance.m_aMutex )
             {
                 if ( _rInstance.m_aSlaveHandlers.empty() )
-                    throw css::lang::DisposedException( OUString(), *(&_rInstance) );
+                    throw css::lang::DisposedException( OUString(), _rInstance );
             }
         };
     };
diff --git a/sfx2/source/doc/docundomanager.cxx b/sfx2/source/doc/docundomanager.cxx
index 143dffb1e3e3..2036a1f16bec 100644
--- a/sfx2/source/doc/docundomanager.cxx
+++ b/sfx2/source/doc/docundomanager.cxx
@@ -95,7 +95,7 @@ namespace sfx2
             if ( pObjectShell != nullptr )
                 pUndoManager = pObjectShell->GetUndoManager();
             if ( !pUndoManager )
-                throw NotInitializedException( OUString(), *&i_baseModel );
+                throw NotInitializedException( OUString(), i_baseModel );
             return pUndoManager;
         }
     };
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index aec4fc09aecc..1a6001eb6a9f 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -29,6 +29,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/redundantcast \
     compilerplugins/clang/test/redundantcopy \
     compilerplugins/clang/test/redundantinline \
+    compilerplugins/clang/test/redundantpointerops \
     compilerplugins/clang/test/refcounting \
     compilerplugins/clang/test/salbool \
     compilerplugins/clang/test/salunicodeliteral \
diff --git a/sw/source/core/doc/CntntIdxStore.cxx b/sw/source/core/doc/CntntIdxStore.cxx
index f164af01569f..4c824c2198da 100644
--- a/sw/source/core/doc/CntntIdxStore.cxx
+++ b/sw/source/core/doc/CntntIdxStore.cxx
@@ -402,7 +402,7 @@ void ContentIdxStoreImpl::SaveUnoCursors(SwDoc* pDoc, sal_uLong nNode, sal_Int32
         const SwUnoTableCursor* pUnoTableCursor = dynamic_cast<const SwUnoTableCursor*>(pUnoCursor.get());
         if( pUnoTableCursor )
         {
-            for(SwPaM& rPaM : (&(const_cast<SwUnoTableCursor*>(pUnoTableCursor))->GetSelRing())->GetRingContainer())
+            for(SwPaM& rPaM : const_cast<SwUnoTableCursor*>(pUnoTableCursor)->GetSelRing().GetRingContainer())
             {
                 lcl_ChkUnoCrsrPaMBoth(m_aUnoCursorEntries, nNode, nContent, rPaM);
             }
diff --git a/sw/source/core/doc/doccorr.cxx b/sw/source/core/doc/doccorr.cxx
index 2012dfc06faf..2e73dd1d850e 100644
--- a/sw/source/core/doc/doccorr.cxx
+++ b/sw/source/core/doc/doccorr.cxx
@@ -147,7 +147,7 @@ void PaMCorrAbs( const SwPaM& rRange,
             dynamic_cast<SwUnoTableCursor *>(pUnoCursor.get());
         if( pUnoTableCursor )
         {
-            for(SwPaM& rPaM : (&pUnoTableCursor->GetSelRing())->GetRingContainer())
+            for(SwPaM& rPaM : pUnoTableCursor->GetSelRing().GetRingContainer())
             {
                 bChange |=
                     lcl_PaMCorrAbs( rPaM, aStart, aEnd, aNewPos );
@@ -289,7 +289,7 @@ void PaMCorrRel( const SwNodeIndex &rOldNode,
             dynamic_cast<SwUnoTableCursor*>(pUnoCursor.get());
         if( pUnoTableCursor )
         {
-            for(SwPaM& rPaM : (&pUnoTableCursor->GetSelRing())->GetRingContainer())
+            for(SwPaM& rPaM : pUnoTableCursor->GetSelRing().GetRingContainer())
             {
                 lcl_PaMCorrRel1( &rPaM, pOldNode, aNewPos, nCntIdx );
             }
diff --git a/toolkit/source/controls/grid/sortablegriddatamodel.cxx b/toolkit/source/controls/grid/sortablegriddatamodel.cxx
index af96671af26f..ad1cc2611a51 100644
--- a/toolkit/source/controls/grid/sortablegriddatamodel.cxx
+++ b/toolkit/source/controls/grid/sortablegriddatamodel.cxx
@@ -205,7 +205,7 @@ public:
         :comphelper::ComponentGuard( i_component, i_broadcastHelper )
     {
         if ( !i_component.isInitialized() )
-            throw css::lang::NotInitializedException( OUString(), *&i_component );
+            throw css::lang::NotInitializedException( OUString(), i_component );
     }
 };