improve refcounting loplugin

to find ref-counted classes being managed via other smart pointer
classes.
Hopefully prevent needing fixes like
642ae256
"ChangedUIEventListener is refcounted, mustn't be helt by unique_ptr"

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

compilerplugins/clang/refcounting.cxx

@@ -386,28 +386,67 @@ bool RefCounting::VisitFieldDecl(const FieldDecl * fieldDecl) {
         return true;
     }
 
-    std::string aParentName = fieldDecl->getParent()->getQualifiedNameAsString();
+    // check for dodgy code managing ref-counted stuff with shared_ptr or unique_ptr or similar stuff
+    QualType firstTemplateParamType;
+    if (auto recordType = fieldDecl->getType()->getUnqualifiedDesugaredType()->getAs<RecordType>()) {
+        auto recordDeclName = recordType->getDecl()->getName();
+        if (recordDeclName.find("unique_ptr") != StringRef::npos
+            || recordDeclName.find("shared_ptr") != StringRef::npos
+            || recordDeclName.find("intrusive_ptr") != StringRef::npos) // boost
+        {
+            auto templateDecl = dyn_cast<ClassTemplateSpecializationDecl>(recordType->getDecl());
+            if (templateDecl && templateDecl->getTemplateArgs().size() > 0)
+                firstTemplateParamType = templateDecl->getTemplateArgs()[0].getAsType();
+        }
+    }
 
     if (containsSvRefBaseSubclass(fieldDecl->getType().getTypePtr())) {
         report(
             DiagnosticsEngine::Warning,
-            "SvRefBase subclass being directly heap managed, should be managed via tools::SvRef, "
-            + fieldDecl->getType().getAsString()
-            + ", parent is " + aParentName,
+            "SvRefBase subclass %0 being directly heap managed, should be managed via tools::SvRef, "
+            ", parent is %1",
             fieldDecl->getLocation())
-          << fieldDecl->getSourceRange();
+            << fieldDecl->getType()
+            << fieldDecl->getParent()
+            << fieldDecl->getSourceRange();
+    }
+
+    if (!firstTemplateParamType.isNull() && containsSvRefBaseSubclass(firstTemplateParamType.getTypePtr()))
+    {
+        report(
+            DiagnosticsEngine::Warning,
+            "SvRefBase subclass %0 being managed via smart pointer, should be managed via tools::SvRef, "
+            "parent is %1",
+            fieldDecl->getLocation())
+            << firstTemplateParamType
+            << fieldDecl->getParent()
+            << fieldDecl->getSourceRange();
     }
 
     if (containsSalhelperReferenceObjectSubclass(fieldDecl->getType().getTypePtr())) {
         report(
             DiagnosticsEngine::Warning,
-            "salhelper::SimpleReferenceObject subclass being directly heap managed, should be managed via rtl::Reference, "
-            + fieldDecl->getType().getAsString()
-            + ", parent is " + aParentName,
+            "salhelper::SimpleReferenceObject subclass %0 being directly heap managed, should be managed via rtl::Reference, "
+            "parent is %1",
+            fieldDecl->getLocation())
+            << fieldDecl->getType()
+            << fieldDecl->getParent()
+            << fieldDecl->getSourceRange();
+    }
+
+    if (!firstTemplateParamType.isNull() && containsSalhelperReferenceObjectSubclass(firstTemplateParamType.getTypePtr()))
+    {
+        report(
+            DiagnosticsEngine::Warning,
+            "salhelper::SimpleReferenceObject subclass %0 being managed via smart pointer, should be managed via rtl::Reference, "
+            "parent is %1",
             fieldDecl->getLocation())
-          << fieldDecl->getSourceRange();
+            << firstTemplateParamType
+            << fieldDecl->getParent()
+            << fieldDecl->getSourceRange();
     }
 
+    std::string aParentName = fieldDecl->getParent()->getQualifiedNameAsString();
     if ( aParentName == "com::sun::star::uno::BaseReference"
          || aParentName == "cppu::detail::element_alias"
          // this is playing some kind of game to avoid circular references
@@ -419,11 +458,24 @@ bool RefCounting::VisitFieldDecl(const FieldDecl * fieldDecl) {
     if (containsXInterfaceSubclass(fieldDecl->getType())) {
         report(
             DiagnosticsEngine::Warning,
-            "XInterface subclass being directly heap managed, should be managed via uno::Reference, "
-            + fieldDecl->getType().getAsString()
-            + ", parent is " + aParentName,
+            "XInterface subclass %0 being directly heap managed, should be managed via uno::Reference, "
+            "parent is %1",
+            fieldDecl->getLocation())
+            << fieldDecl->getType()
+            << fieldDecl->getParent()
+            << fieldDecl->getSourceRange();
+    }
+
+    if (!firstTemplateParamType.isNull() && containsXInterfaceSubclass(firstTemplateParamType))
+    {
+        report(
+            DiagnosticsEngine::Warning,
+            "XInterface subclass %0 being managed via smart pointer, should be managed via uno::Reference, "
+            "parent is %1",
             fieldDecl->getLocation())
-          << fieldDecl->getSourceRange();
+            << firstTemplateParamType
+            << fieldDecl->getParent()
+            << fieldDecl->getSourceRange();
     }
 
     checkUnoReference(fieldDecl->getType(), fieldDecl, aParentName, "field");

compilerplugins/clang/test/refcounting.cxx

+/* -*- 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/.
+ */
+
+#include <memory>
+#include <com/sun/star/uno/XInterface.hpp>
+
+struct Foo
+{
+    std::unique_ptr<css::uno::XInterface> m_foo1; // expected-error {{XInterface subclass 'com::sun::star::uno::XInterface' being managed via smart pointer, should be managed via uno::Reference, parent is 'Foo' [loplugin:refcounting]}}
+    std::shared_ptr<css::uno::XInterface> m_foo2; // expected-error {{XInterface subclass 'com::sun::star::uno::XInterface' being managed via smart pointer, should be managed via uno::Reference, parent is 'Foo' [loplugin:refcounting]}}
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

connectivity/source/commontools/TIndex.cxx

@@ -37,7 +37,7 @@ OIndexHelper::OIndexHelper( OTableHelper* _pTable) : connectivity::sdbcx::OIndex
 {
     construct();
     std::vector< OUString> aVector;
-    m_pColumns.reset( new OIndexColumns(this,m_aMutex,aVector) );
+    m_pColumns = new OIndexColumns(this,m_aMutex,aVector);
 }
 
 OIndexHelper::OIndexHelper( OTableHelper* _pTable,
@@ -93,7 +93,7 @@ void OIndexHelper::refreshColumns()
     if(m_pColumns)
         m_pColumns->reFill(aVector);
     else
-        m_pColumns.reset( new OIndexColumns(this,m_aMutex,aVector) );
+        m_pColumns = new OIndexColumns(this,m_aMutex,aVector);
 }
 
 

connectivity/source/commontools/TKey.cxx

@@ -100,7 +100,7 @@ void OTableKeyHelper::refreshColumns()
     if ( m_pColumns )
         m_pColumns->reFill(aVector);
     else
-        m_pColumns.reset( new OKeyColumnsHelper(this,m_aMutex,aVector) );
+        m_pColumns = new OKeyColumnsHelper(this,m_aMutex,aVector);
 }
 
 

connectivity/source/drivers/ado/AGroup.cxx

@@ -79,7 +79,7 @@ void OAdoGroup::refreshUsers()
     if(m_pUsers)
         m_pUsers->reFill(aVector);
     else
-        m_pUsers.reset( new OUsers(m_pCatalog,m_aMutex,aVector,aUsers,isCaseSensitive()) );
+        m_pUsers = new OUsers(m_pCatalog,m_aMutex,aVector,aUsers,isCaseSensitive());
 }
 
 Sequence< sal_Int8 > OAdoGroup::getUnoTunnelImplementationId()

connectivity/source/drivers/ado/AIndex.cxx

@@ -68,7 +68,7 @@ void OAdoIndex::refreshColumns()
     if ( m_pColumns )
         m_pColumns->reFill(aVector);
     else
-        m_pColumns.reset( new OColumns(*this,m_aMutex,aVector,aColumns,isCaseSensitive(),m_pConnection) );
+        m_pColumns = new OColumns(*this,m_aMutex,aVector,aColumns,isCaseSensitive(),m_pConnection);
 }
 
 

connectivity/source/drivers/ado/AKey.cxx

@@ -64,7 +64,7 @@ void OAdoKey::refreshColumns()
     if(m_pColumns)
         m_pColumns->reFill(aVector);
     else
-        m_pColumns.reset( new OColumns(*this,m_aMutex,aVector,aColumns,isCaseSensitive(),m_pConnection) );
+        m_pColumns = new OColumns(*this,m_aMutex,aVector,aColumns,isCaseSensitive(),m_pConnection);
 }
 
 Sequence< sal_Int8 > OAdoKey::getUnoTunnelImplementationId()

connectivity/source/drivers/ado/AUser.cxx

@@ -63,7 +63,7 @@ void OAdoUser::refreshGroups()
     if(m_pGroups)
         m_pGroups->reFill(aVector);
     else
-        m_pGroups.reset( new OGroups(m_pCatalog,m_aMutex,aVector,aGroups,isCaseSensitive()) );
+        m_pGroups = new OGroups(m_pCatalog,m_aMutex,aVector,aGroups,isCaseSensitive());
 }
 
 Sequence< sal_Int8 > OAdoUser::getUnoTunnelImplementationId()

connectivity/source/drivers/dbase/DIndex.cxx

@@ -103,7 +103,7 @@ void ODbaseIndex::refreshColumns()
     if(m_pColumns)
         m_pColumns->reFill(aVector);
     else
-        m_pColumns.reset( new ODbaseIndexColumns(this,m_aMutex,aVector) );
+        m_pColumns = new ODbaseIndexColumns(this,m_aMutex,aVector);
 }
 
 Sequence< sal_Int8 > ODbaseIndex::getUnoTunnelImplementationId()

dbaccess/source/core/api/TableDeco.cxx

@@ -566,7 +566,7 @@ void ODBTableDecorator::refreshColumns()
         OContainerMediator* pMediator = new OContainerMediator( pCol, m_xColumnDefinitions );
         m_xColumnMediator = pMediator;
         pCol->setMediator( pMediator );
-        m_pColumns.reset( pCol );
+        m_pColumns = pCol;
     }
     else
         m_pColumns->reFill(aVector);

dbaccess/source/core/inc/TableDeco.hxx

@@ -74,7 +74,7 @@ namespace dbaccess
     // <properties>
         mutable sal_Int32                                         m_nPrivileges;
     // </properties>
-        std::unique_ptr<::connectivity::sdbcx::OCollection>       m_pColumns;
+        rtl::Reference<::connectivity::sdbcx::OCollection>       m_pColumns;
 
         // IColumnFactory
         virtual OColumn*    createColumn(const OUString& _rName) const override;

i18npool/inc/defaultnumberingprovider.hxx

@@ -23,9 +23,10 @@
 #include <com/sun/star/text/XNumberingFormatter.hpp>
 #include <com/sun/star/text/XNumberingTypeInfo.hpp>
 #include <com/sun/star/lang/XServiceInfo.hpp>
-#include <cppuhelper/implbase.hxx>
 #include <com/sun/star/i18n/XTransliteration.hpp>
 #include <com/sun/star/container/XHierarchicalNameAccess.hpp>
+#include <cppuhelper/implbase.hxx>
+#include <rtl/ref.hxx>
 
 #include <transliterationImpl.hxx>
 
@@ -74,7 +75,7 @@ public:
 private:
     css::uno::Reference < css::uno::XComponentContext > m_xContext;
     css::uno::Reference < css::container::XHierarchicalNameAccess > xHierarchicalNameAccess;
-    std::unique_ptr<TransliterationImpl> translit;
+    rtl::Reference<TransliterationImpl> translit;
     /// @throws css::uno::RuntimeException
     OUString SAL_CALL makeNumberingIdentifier( sal_Int16 index );
     /// @throws css::uno::RuntimeException

i18npool/inc/indexentrysupplier_common.hxx

@@ -23,6 +23,7 @@
 #include <com/sun/star/i18n/XExtendedIndexEntrySupplier.hpp>
 #include <cppuhelper/implbase.hxx>
 #include <com/sun/star/lang/XServiceInfo.hpp>
+#include <rtl/ref.hxx>
 #include <collatorImpl.hxx>
 
 #include <memory>
@@ -80,7 +81,7 @@ public:
 protected:
     const sal_Char *   implementationName;
     bool               usePhonetic;
-    std::unique_ptr<CollatorImpl>
+    rtl::Reference<CollatorImpl>
                        collator;
     css::lang::Locale  aLocale;
     OUString           aAlgorithm;

i18npool/inc/indexentrysupplier_default.hxx

@@ -97,7 +97,7 @@ public:
     sal_Int16 mkeys[MAX_KEYS];
     sal_Int16 mkey_count;
     OUString skipping_chars;
-    std::unique_ptr<CollatorImpl> collator;
+    rtl::Reference<CollatorImpl> collator;
     sal_Int16 compare(sal_Unicode c1, sal_Unicode c2);
 };
 

i18npool/source/defaultnumberingprovider/defaultnumberingprovider.cxx

@@ -647,7 +647,7 @@ DefaultNumberingProvider::makeNumberingString( const Sequence<beans::PropertyVal
                     OUString transliteration;
                     getPropertyByName(aProperties, "Transliteration", true) >>= transliteration;
                     if ( !translit )
-                        translit.reset( new TransliterationImpl(m_xContext) );
+                        translit = new TransliterationImpl(m_xContext);
                     translit->loadModuleByImplName(transliteration, aLocale);
                     result += translit->transliterateString2String(tmp, 0, tmp.getLength());
                } catch (Exception& ) {

i18npool/source/indexentry/indexentrysupplier_common.cxx

@@ -30,7 +30,7 @@ namespace com { namespace sun { namespace star { namespace i18n {
 IndexEntrySupplier_Common::IndexEntrySupplier_Common(const Reference < uno::XComponentContext >& rxContext)
 {
     implementationName = "com.sun.star.i18n.IndexEntrySupplier_Common";
-    collator.reset( new CollatorImpl(rxContext) );
+    collator = new CollatorImpl(rxContext);
     usePhonetic = false;
 }
 

include/connectivity/sdbcx/VGroup.hxx

@@ -56,7 +56,7 @@ namespace connectivity
                         public ODescriptor
         {
         protected:
-            std::unique_ptr<OUsers>    m_pUsers;
+            rtl::Reference<OUsers>    m_pUsers;
 
             using OGroup_BASE::rBHelper;
 

include/connectivity/sdbcx/VIndex.hxx

@@ -57,7 +57,7 @@ namespace connectivity
             bool            m_IsPrimaryKeyIndex;
             bool            m_IsClustered;
 
-            std::unique_ptr<OCollection>    m_pColumns;
+            rtl::Reference<OCollection>    m_pColumns;
 
             using ODescriptor_BASE::rBHelper;
             virtual void refreshColumns() override;

include/connectivity/sdbcx/VKey.hxx

@@ -69,7 +69,7 @@ namespace connectivity
         {
         protected:
             std::shared_ptr<KeyProperties>   m_aProps;
-            std::unique_ptr<OCollection>     m_pColumns;
+            rtl::Reference<OCollection>      m_pColumns;
 
             using ODescriptor_BASE::rBHelper;
             // OPropertyArrayUsageHelper

include/connectivity/sdbcx/VUser.hxx

@@ -53,7 +53,7 @@ namespace connectivity
                         public ODescriptor
         {
         protected:
-            std::unique_ptr<OGroups>  m_pGroups;
+            rtl::Reference<OGroups>  m_pGroups;
 
             using OUser_BASE::rBHelper;
 

sc/source/filter/xml/XMLTrackedChangesContext.cxx

@@ -177,7 +177,7 @@ class ScXMLChangeTextPContext : public ScXMLImportContext
     OUString                    sLName;
     OUStringBuffer              sText;
     ScXMLChangeCellContext*     pChangeCellContext;
-    std::unique_ptr<SvXMLImportContext>
+    rtl::Reference<SvXMLImportContext>
                                 pTextPContext;
     sal_uInt16                  nPrefix;
 
@@ -876,8 +876,8 @@ SvXMLImportContext *ScXMLChangeTextPContext::CreateChildContext( sal_uInt16 nTem
         if (!pTextPContext)
         {
             bWasContext = false;
-            pTextPContext.reset( GetScImport().GetTextImport()->CreateTextChildContext(
-                                    GetScImport(), nPrefix, sLName, xAttrList) );
+            pTextPContext= GetScImport().GetTextImport()->CreateTextChildContext(
+                                    GetScImport(), nPrefix, sLName, xAttrList);
         }
         if (pTextPContext)
         {

solenv/CompilerTest_compilerplugins_clang.mk

@@ -23,6 +23,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/refcounting \
     compilerplugins/clang/test/salbool \
     compilerplugins/clang/test/salunicodeliteral \
     compilerplugins/clang/test/stringconstant \

sw/source/uibase/inc/dbtree.hxx

@@ -34,7 +34,7 @@ class SW_DLLPUBLIC SwDBTreeList : public SvTreeListBox
     bool            bInitialized;
     bool            bShowColumns;
 
-    std::unique_ptr<SwDBTreeList_Impl> pImpl;
+    rtl::Reference<SwDBTreeList_Impl> pImpl;
 
     DECL_DLLPRIVATE_LINK( DBCompare, const SvSortData&, sal_Int32 );
 

xmloff/source/draw/XMLNumberStyles.cxx

@@ -484,7 +484,7 @@ private:
     bool mbTextual;
     bool mbDecimal02;
     OUString maText;
-    std::shared_ptr< SvXMLImportContext > mpSlaveContext;
+    rtl::Reference< SvXMLImportContext > mxSlaveContext;
 
 public:
 
@@ -511,7 +511,7 @@ SdXMLNumberFormatMemberImportContext::SdXMLNumberFormatMemberImportContext( SvXM
 :   SvXMLImportContext(rImport, nPrfx, rLocalName),
     mpParent( pParent ),
     maNumberStyle( rLocalName ),
-    mpSlaveContext( pSlaveContext )
+    mxSlaveContext( pSlaveContext )
 {
     mbLong = false;
     mbTextual = false;
@@ -548,17 +548,17 @@ SvXMLImportContext *SdXMLNumberFormatMemberImportContext::CreateChildContext( sa
                            const OUString& rLocalName,
                            const css::uno::Reference< css::xml::sax::XAttributeList >& xAttrList )
 {
-    return mpSlaveContext->CreateChildContext( nPrefix, rLocalName, xAttrList );
+    return mxSlaveContext->CreateChildContext( nPrefix, rLocalName, xAttrList );
 }
 
 void SdXMLNumberFormatMemberImportContext::StartElement( const css::uno::Reference< css::xml::sax::XAttributeList >& xAttrList )
 {
-    mpSlaveContext->StartElement( xAttrList );
+    mxSlaveContext->StartElement( xAttrList );
 }
 
 void SdXMLNumberFormatMemberImportContext::EndElement()
 {
-    mpSlaveContext->EndElement();
+    mxSlaveContext->EndElement();
 
     if( mpParent )
         mpParent->add( maNumberStyle, mbLong, mbTextual, mbDecimal02, maText );
@@ -566,7 +566,7 @@ void SdXMLNumberFormatMemberImportContext::EndElement()
 
 void SdXMLNumberFormatMemberImportContext::Characters( const OUString& rChars )
 {
-    mpSlaveContext->Characters( rChars );
+    mxSlaveContext->Characters( rChars );
     maText += rChars;
 }