loplugin:refcounting check for managing OWeakObject with raw pointer

Change-Id: I7471725f1e658940b5e6993361c327be6ccf0d31
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111064
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

8 files changed, 77 insertions, 18 deletions

diff --git a/compilerplugins/clang/refcounting.cxx b/compilerplugins/clang/refcounting.cxx
index f15e423aebd2..7f1d7f38fba8 100644
--- a/compilerplugins/clang/refcounting.cxx
+++ b/compilerplugins/clang/refcounting.cxx
@@ -57,6 +57,7 @@ public:
     bool VisitFunctionDecl(const FunctionDecl *);
     bool VisitTypeLoc(clang::TypeLoc typeLoc);
     bool VisitCXXDeleteExpr(const CXXDeleteExpr *);
+    bool VisitBinaryOperator(const BinaryOperator *);
 
     // Creation of temporaries with one argument are represented by
     // CXXFunctionalCastExpr, while any other number of arguments are
@@ -600,9 +601,8 @@ bool RefCounting::VisitFieldDecl(const FieldDecl * fieldDecl) {
 
 
 bool RefCounting::VisitVarDecl(const VarDecl * varDecl) {
-    if (ignoreLocation(varDecl)) {
+    if (ignoreLocation(varDecl))
         return true;
-    }
 
     checkUnoReference(varDecl->getType(), varDecl, nullptr, "var");
 
@@ -646,6 +646,56 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) {
             varDecl->getLocation())
             << varDecl->getSourceRange();
     }
+
+    if (varDecl->getType()->isPointerType() && varDecl->getInit())
+    {
+        auto newExpr = dyn_cast<CXXNewExpr>(compat::IgnoreImplicit(varDecl->getInit()));
+        if (newExpr)
+        {
+            StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(varDecl)));
+            if (loplugin::isSamePathname(fileName, SRCDIR "/cppuhelper/source/component_context.cxx"))
+                return true;
+            auto pointeeType = varDecl->getType()->getPointeeType();
+            if (containsOWeakObjectSubclass(pointeeType))
+                report(
+                    DiagnosticsEngine::Warning,
+                    "cppu::OWeakObject subclass %0 being managed via raw pointer, should be managed via rtl::Reference",
+                    varDecl->getLocation())
+                    << pointeeType
+                    << varDecl->getSourceRange();
+        }
+    }
+    return true;
+}
+
+bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator)
+{
+    if (ignoreLocation(binaryOperator))
+        return true;
+    if (binaryOperator->getOpcode() != BO_Assign)
+        return true;
+    if (!binaryOperator->getLHS()->getType()->isPointerType())
+        return true;
+
+    // deliberately does not want to keep track at the allocation site
+    StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(binaryOperator)));
+    if (loplugin::isSamePathname(fileName, SRCDIR "/vcl/unx/generic/dtrans/X11_selection.cxx"))
+        return true;
+
+    auto newExpr = dyn_cast<CXXNewExpr>(compat::IgnoreImplicit(binaryOperator->getRHS()));
+    if (newExpr)
+    {
+        auto pointeeType = binaryOperator->getLHS()->getType()->getPointeeType();
+        if (containsOWeakObjectSubclass(pointeeType))
+        {
+            report(
+                DiagnosticsEngine::Warning,
+                "cppu::OWeakObject subclass %0 being managed via raw pointer, should be managed via rtl::Reference",
+                compat::getBeginLoc(binaryOperator))
+                << pointeeType
+                << binaryOperator->getSourceRange();
+        }
+    }
     return true;
 }
 
diff --git a/compilerplugins/clang/test/refcounting.cxx b/compilerplugins/clang/test/refcounting.cxx
index 8a1f277829cc..ca27ac0614a7 100644
--- a/compilerplugins/clang/test/refcounting.cxx
+++ b/compilerplugins/clang/test/refcounting.cxx
@@ -18,6 +18,8 @@ namespace cppu
 {
 class OWeakObject
 {
+    void acquire();
+    void release();
 };
 }
 
@@ -72,4 +74,13 @@ void dummy(Dependent<Dummy>* p1, Dependent<UnoObject>* p2)
     p2->g();
 }
 
+void foo4()
+{
+    // expected-error@+1 {{cppu::OWeakObject subclass 'UnoObject' being managed via raw pointer, should be managed via rtl::Reference [loplugin:refcounting]}}
+    UnoObject* p = new UnoObject;
+    (void)p;
+    // expected-error@+1 {{cppu::OWeakObject subclass 'UnoObject' being managed via raw pointer, should be managed via rtl::Reference [loplugin:refcounting]}}
+    p = new UnoObject;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/cpputools/source/unoexe/unoexe.cxx b/cpputools/source/unoexe/unoexe.cxx
index bc1df676a38a..b5be9ff175ff 100644
--- a/cpputools/source/unoexe/unoexe.cxx
+++ b/cpputools/source/unoexe/unoexe.cxx
@@ -30,6 +30,7 @@
 #include <rtl/string.h>
 #include <rtl/strbuf.hxx>
 #include <rtl/ustrbuf.hxx>
+#include <rtl/ref.hxx>
 
 #include <cppuhelper/bootstrap.hxx>
 #include <cppuhelper/implbase.hxx>
@@ -344,11 +345,10 @@ void ODisposingListener::disposing( const EventObject & )
 
 void ODisposingListener::waitFor( const Reference< XComponent > & xComp )
 {
-    ODisposingListener * pListener = new ODisposingListener;
-    Reference< XEventListener > xListener( pListener );
+    rtl::Reference<ODisposingListener> xListener = new ODisposingListener;
 
     xComp->addEventListener( xListener );
-    pListener->cDisposed.wait();
+    xListener->cDisposed.wait();
 }
 
 } // namespace unoexe
diff --git a/hwpfilter/source/hwpreader.cxx b/hwpfilter/source/hwpreader.cxx
index d6c34fcb2354..7bc22fa370c7 100644
--- a/hwpfilter/source/hwpreader.cxx
+++ b/hwpfilter/source/hwpreader.cxx
@@ -4856,13 +4856,12 @@ HwpImportFilter::HwpImportFilter(const Reference< XComponentContext >& rxContext
     try {
         Reference< XDocumentHandler > xHandler( rxContext->getServiceManager()->createInstanceWithContext( WRITER_IMPORTER_NAME, rxContext ), UNO_QUERY );
 
-        HwpReader *p = new HwpReader;
+        rtl::Reference<HwpReader> p = new HwpReader;
         p->setDocumentHandler( xHandler );
 
         Reference< XImporter > xImporter( xHandler, UNO_QUERY );
         rImporter = xImporter;
-        Reference< XFilter > xFilter( p );
-        rFilter = xFilter;
+        rFilter = p;
     }
     catch( Exception & )
     {
diff --git a/lingucomponent/source/thesaurus/libnth/nthesimp.cxx b/lingucomponent/source/thesaurus/libnth/nthesimp.cxx
index 79bd0b51ae4a..4df96c3dff05 100644
--- a/lingucomponent/source/thesaurus/libnth/nthesimp.cxx
+++ b/lingucomponent/source/thesaurus/libnth/nthesimp.cxx
@@ -386,7 +386,7 @@ Sequence < Reference < css::linguistic2::XMeaning > > SAL_CALL Thesaurus::queryM
                     OUString aAlt( cTerm + catst);
                     pStr[i] = aAlt;
                 }
-                Meaning * pMn = new Meaning(aRTerm);
+                rtl::Reference<Meaning> pMn = new Meaning(aRTerm);
                 OUString dTerm(pe->defn,strlen(pe->defn),eEnc );
                 pMn->SetMeaning(dTerm);
                 pMn->SetSynonyms(aStr);
diff --git a/linguistic/source/spelldta.cxx b/linguistic/source/spelldta.cxx
index 7ae3d3d7f42d..b3f57e9b870f 100644
--- a/linguistic/source/spelldta.cxx
+++ b/linguistic/source/spelldta.cxx
@@ -28,6 +28,7 @@
 
 #include <linguistic/misc.hxx>
 #include <linguistic/spelldta.hxx>
+#include <rtl/ref.hxx>
 
 
 using namespace osl;
@@ -255,11 +256,11 @@ void SpellAlternatives::SetAlternatives( const Sequence< OUString > &rAlt )
 css::uno::Reference < css::linguistic2::XSpellAlternatives > SpellAlternatives::CreateSpellAlternatives(
         const OUString &rWord, LanguageType nLang, sal_Int16 nTypeP, const css::uno::Sequence< OUString > &rAlt )
 {
-    SpellAlternatives* pAlt = new SpellAlternatives;
+    rtl::Reference<SpellAlternatives> pAlt = new SpellAlternatives;
     pAlt->SetWordLanguage( rWord, nLang );
     pAlt->SetFailureType( nTypeP );
     pAlt->SetAlternatives( rAlt );
-    return Reference < XSpellAlternatives >(pAlt);
+    return pAlt;
 }
 
 
diff --git a/slideshow/source/engine/shapes/gdimtftools.cxx b/slideshow/source/engine/shapes/gdimtftools.cxx
index f5a41c48a561..e3d22e5033f2 100644
--- a/slideshow/source/engine/shapes/gdimtftools.cxx
+++ b/slideshow/source/engine/shapes/gdimtftools.cxx
@@ -168,8 +168,7 @@ GDIMetaFileSharedPtr getMetaFile( const uno::Reference< lang::XComponent >&
     // TODO(P3): Move creation of DummyRenderer out of the
     // loop! Either by making it static, or transforming
     // the whole thing here into a class.
-    DummyRenderer*                              pRenderer( new DummyRenderer() );
-    uno::Reference< graphic::XGraphicRenderer > xRenderer( pRenderer );
+    rtl::Reference<DummyRenderer> xRenderer( new DummyRenderer() );
 
     // creating the graphic exporter
     uno::Reference< drawing::XGraphicExportFilter > xExporter =
@@ -180,7 +179,7 @@ GDIMetaFileSharedPtr getMetaFile( const uno::Reference< lang::XComponent >&
     aProps[0].Value <<= OUString("SVM");
 
     aProps[1].Name = "GraphicRenderer";
-    aProps[1].Value <<= xRenderer;
+    aProps[1].Value <<= uno::Reference< graphic::XGraphicRenderer >(xRenderer);
 
     uno::Sequence< beans::PropertyValue > aFilterData(4);
     aFilterData[0].Name = "ScrollText";
@@ -203,7 +202,7 @@ GDIMetaFileSharedPtr getMetaFile( const uno::Reference< lang::XComponent >&
     if( !xExporter->filter( aProps ) )
         return GDIMetaFileSharedPtr();
 
-    GDIMetaFileSharedPtr xMtf = pRenderer->getMtf( (mtfLoadFlags & MTF_LOAD_FOREIGN_SOURCE) != 0 );
+    GDIMetaFileSharedPtr xMtf = xRenderer->getMtf( (mtfLoadFlags & MTF_LOAD_FOREIGN_SOURCE) != 0 );
 
     // pRenderer is automatically destroyed when xRenderer
     // goes out of scope
diff --git a/writerperfect/source/common/DocumentHandler.cxx b/writerperfect/source/common/DocumentHandler.cxx
index 181415033909..6b5ffe58ad9e 100644
--- a/writerperfect/source/common/DocumentHandler.cxx
+++ b/writerperfect/source/common/DocumentHandler.cxx
@@ -126,8 +126,7 @@ void DocumentHandler::endDocument() { mxHandler->endDocument(); }
 void DocumentHandler::startElement(const char* psName,
                                    const librevenge::RVNGPropertyList& xPropList)
 {
-    SvXMLAttributeList* pAttrList = new SvXMLAttributeList();
-    Reference<XAttributeList> xAttrList(pAttrList);
+    rtl::Reference<SvXMLAttributeList> pAttrList = new SvXMLAttributeList();
     librevenge::RVNGPropertyList::Iter i(xPropList);
     for (i.rewind(); i.next();)
     {
@@ -163,7 +162,7 @@ void DocumentHandler::startElement(const char* psName,
     }
 
     OUString sElementName(psName, strlen(psName), RTL_TEXTENCODING_UTF8);
-    mxHandler->startElement(sElementName, xAttrList);
+    mxHandler->startElement(sElementName, pAttrList);
 }
 
 void DocumentHandler::endElement(const char* psName)