loplugin:unusedvariablecheck improve

to find unused smart pointer variables

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

21 files changed, 63 insertions, 24 deletions

diff --git a/comphelper/qa/unit/test_traceevent.cxx b/comphelper/qa/unit/test_traceevent.cxx
index 46c1cef5be25..34d10f519d2b 100644
--- a/comphelper/qa/unit/test_traceevent.cxx
+++ b/comphelper/qa/unit/test_traceevent.cxx
@@ -31,8 +31,6 @@ namespace
 {
 void trace_event_test()
 {
-    std::shared_ptr<comphelper::AsyncEvent> pAsync7Locked;
-
     {
         // When we start recording is off and this will not generate any 'X' event when we leave the scope
         comphelper::ProfileZone aZone0("test0");
diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 552f676735d3..57fdf83b79ff 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -818,6 +818,42 @@ bool hasExternalLinkage(VarDecl const * decl) {
     return true;
 }
 
+bool isSmartPointerType(QualType qt)
+{
+    // First check whether the object type as written is, or is derived from, std::unique_ptr or
+    // std::shared_ptr, in case the get member function is declared at a base class of that std
+    // type:
+    if (loplugin::isDerivedFrom(
+            qt->getAsCXXRecordDecl(),
+            [](Decl const * decl) {
+                auto const dc = loplugin::DeclCheck(decl);
+                return dc.ClassOrStruct("unique_ptr").StdNamespace()
+                    || dc.ClassOrStruct("shared_ptr").StdNamespace();
+            }))
+        return true;
+
+    // Then check the object type coerced to the type of the get member function, in
+    // case the type-as-written is derived from one of these types (tools::SvRef is
+    // final, but the rest are not):
+    auto const tc2 = loplugin::TypeCheck(qt);
+    if (tc2.ClassOrStruct("unique_ptr").StdNamespace()
+           || tc2.ClassOrStruct("shared_ptr").StdNamespace()
+           || tc2.Class("Reference").Namespace("uno").Namespace("star")
+                .Namespace("sun").Namespace("com").GlobalNamespace()
+           || tc2.Class("Reference").Namespace("rtl").GlobalNamespace()
+           || tc2.Class("SvRef").Namespace("tools").GlobalNamespace()
+           || tc2.Class("WeakReference").Namespace("tools").GlobalNamespace()
+           || tc2.Class("ScopedReadAccess").Namespace("Bitmap").GlobalNamespace()
+           || tc2.Class("ScopedVclPtrInstance").GlobalNamespace()
+           || tc2.Class("VclPtr").GlobalNamespace()
+           || tc2.Class("ScopedVclPtr").GlobalNamespace()
+           || tc2.Class("intrusive_ptr").Namespace("boost").GlobalNamespace())
+    {
+        return true;
+    }
+    return false;
+}
+
 bool isSmartPointerType(const Expr* e)
 {
     // First check whether the object type as written is, or is derived from, std::unique_ptr or
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 156955da4b87..540352df9868 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -306,6 +306,7 @@ int derivedFromCount(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* ba
 bool hasExternalLinkage(VarDecl const * decl);
 
 bool isSmartPointerType(const Expr*);
+bool isSmartPointerType(clang::QualType);
 
 const Decl* getFunctionDeclContext(ASTContext& context, const Stmt* stmt);
 
diff --git a/compilerplugins/clang/test/unusedvariablecheck.cxx b/compilerplugins/clang/test/unusedvariablecheck.cxx
index c5b2a04d89fe..e53da1be40a8 100644
--- a/compilerplugins/clang/test/unusedvariablecheck.cxx
+++ b/compilerplugins/clang/test/unusedvariablecheck.cxx
@@ -12,6 +12,7 @@
 #include <list>
 #include <string>
 #include <vector>
+#include <memory>
 
 namespace
 {
@@ -23,6 +24,8 @@ int main()
     std::list<int> v1; // expected-error {{unused variable 'v1' [loplugin:unusedvariablecheck]}}
     std::string v2; // expected-error {{unused variable 'v2' [loplugin:unusedvariablecheck]}}
     Vec<int> v3; // expected-error {{unused variable 'v3' [loplugin:unusedvariablecheck]}}
+    std::unique_ptr<int>
+        v4; // expected-error {{unused variable 'v4' [loplugin:unusedvariablecheck]}}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unusedvariablecheck.cxx b/compilerplugins/clang/unusedvariablecheck.cxx
index 1e09645733e0..73529e118bf4 100644
--- a/compilerplugins/clang/unusedvariablecheck.cxx
+++ b/compilerplugins/clang/unusedvariablecheck.cxx
@@ -16,6 +16,7 @@
 #include "compat.hxx"
 #include "check.hxx"
 #include "unusedvariablecheck.hxx"
+#include "plugin.hxx"
 
 namespace loplugin
 {
@@ -57,6 +58,10 @@ bool UnusedVariableCheck::VisitVarDecl( const VarDecl* var )
     auto type = var->getType();
     bool check = loplugin::isExtraWarnUnusedType(type);
 
+    if (!check)
+        check = isUnusedSmartPointer(var);
+
+
     // this chunk of logic generates false+, which is why we don't leave it on
 /*
     if (!check && type->isRecordType())
@@ -88,6 +93,22 @@ bool UnusedVariableCheck::VisitVarDecl( const VarDecl* var )
     return true;
     }
 
+bool UnusedVariableCheck::isUnusedSmartPointer( const VarDecl* var )
+    {
+        // if we have a var of smart-pointer type, and that var is both uninitialised and
+        // not referenced, then we can remove it
+        if (!isSmartPointerType(var->getType()))
+            return false;
+
+        if (!var->hasInit())
+            return true;
+
+        auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(var->getInit());
+        if (!cxxConstructExpr)
+            return false;
+        return cxxConstructExpr->getConstructor()->isDefaultConstructor();
+    }
+
 static Plugin::Registration< UnusedVariableCheck > unusedvariablecheck( "unusedvariablecheck" );
 
 } // namespace
diff --git a/compilerplugins/clang/unusedvariablecheck.hxx b/compilerplugins/clang/unusedvariablecheck.hxx
index f22162e108a6..5f5edbbbaf96 100644
--- a/compilerplugins/clang/unusedvariablecheck.hxx
+++ b/compilerplugins/clang/unusedvariablecheck.hxx
@@ -23,6 +23,8 @@ class UnusedVariableCheck
         explicit UnusedVariableCheck( const InstantiationData& data );
         virtual void run() override;
         bool VisitVarDecl( const VarDecl* var );
+    private:
+        bool isUnusedSmartPointer( const VarDecl* var );
     };
 
 } // namespace
diff --git a/connectivity/source/drivers/mysqlc/mysqlc_driver.cxx b/connectivity/source/drivers/mysqlc/mysqlc_driver.cxx
index 5e5b83625e90..d38cfe24eb44 100644
--- a/connectivity/source/drivers/mysqlc/mysqlc_driver.cxx
+++ b/connectivity/source/drivers/mysqlc/mysqlc_driver.cxx
@@ -84,7 +84,6 @@ Reference<XConnection> SAL_CALL MysqlCDriver::connect(const OUString& url,
         return nullptr;
     }
 
-    Reference<XConnection> xConn;
     // create a new connection with the given properties and append it to our vector
     rtl::Reference<OConnection> pCon = new OConnection(*this);
 
diff --git a/dbaccess/source/ui/app/AppController.cxx b/dbaccess/source/ui/app/AppController.cxx
index 378dceb9cf3c..003b295e3178 100644
--- a/dbaccess/source/ui/app/AppController.cxx
+++ b/dbaccess/source/ui/app/AppController.cxx
@@ -2287,7 +2287,6 @@ bool OApplicationController::requestQuickHelp(const void* /*pUserData*/, OUStrin
 bool OApplicationController::requestDrag(const weld::TreeIter& /*rEntry*/)
 {
     bool bSuccess = false;
-    rtl::Reference<TransferableHelper> pTransfer;
 
     OApplicationView* pContainer = getContainer();
     if (pContainer && pContainer->getSelectionCount())
diff --git a/sfx2/source/devtools/ObjectInspectorTreeHandler.cxx b/sfx2/source/devtools/ObjectInspectorTreeHandler.cxx
index 253932bb2632..7228f48e13ca 100644
--- a/sfx2/source/devtools/ObjectInspectorTreeHandler.cxx
+++ b/sfx2/source/devtools/ObjectInspectorTreeHandler.cxx
@@ -640,7 +640,6 @@ public:
         for (int i = 0; i < nLength; i++)
         {
             uno::Any aArrayValue = mxIdlArray->get(maAny, i);
-            uno::Reference<uno::XInterface> xCurrent;
 
             auto* pObjectInspectorNode
                 = createNodeObjectForAny(OUString::number(i), aArrayValue, "");
diff --git a/sw/qa/extras/globalfilter/globalfilter.cxx b/sw/qa/extras/globalfilter/globalfilter.cxx
index 04d24a825f27..96989a473d30 100644
--- a/sw/qa/extras/globalfilter/globalfilter.cxx
+++ b/sw/qa/extras/globalfilter/globalfilter.cxx
@@ -404,7 +404,6 @@ std::vector<uno::Reference<graphic::XGraphic>>
     lcl_getGraphics(const uno::Reference<lang::XComponent>& xComponent)
 {
     std::vector<uno::Reference<graphic::XGraphic>> aGraphics;
-    uno::Reference<drawing::XShape> xShape;
 
     uno::Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(xComponent, uno::UNO_QUERY);
     uno::Reference<drawing::XDrawPage> xDrawPage = xDrawPageSupplier->getDrawPage();
diff --git a/sw/source/core/unocore/unodraw.cxx b/sw/source/core/unocore/unodraw.cxx
index 5c5bd75576c7..66deb6769321 100644
--- a/sw/source/core/unocore/unodraw.cxx
+++ b/sw/source/core/unocore/unodraw.cxx
@@ -1016,7 +1016,6 @@ uno::Reference< beans::XPropertySetInfo >  SwXShape::getPropertySetInfo()
     SolarMutexGuard aGuard;
     if (!mxPropertySetInfo)
     {
-        uno::Reference< beans::XPropertySetInfo >  aRet;
         if(m_xShapeAgg.is())
         {
             const uno::Type& rPropSetType = cppu::UnoType<beans::XPropertySet>::get();
diff --git a/sw/source/core/unocore/unoframe.cxx b/sw/source/core/unocore/unoframe.cxx
index cefcd9ae3666..da51bdc3455f 100644
--- a/sw/source/core/unocore/unoframe.cxx
+++ b/sw/source/core/unocore/unoframe.cxx
@@ -3262,7 +3262,6 @@ SwXTextFrame::CreateCursor()
 uno::Reference< text::XTextCursor >  SwXTextFrame::createTextCursor()
 {
     SolarMutexGuard aGuard;
-    uno::Reference< text::XTextCursor >  aRef;
     SwFrameFormat* pFormat = GetFrameFormat();
     if(!pFormat)
         throw uno::RuntimeException();
diff --git a/ucb/source/ucp/webdav-curl/DAVResourceAccess.cxx b/ucb/source/ucp/webdav-curl/DAVResourceAccess.cxx
index 1c3e9a5320e0..d1adcdd81e1a 100644
--- a/ucb/source/ucp/webdav-curl/DAVResourceAccess.cxx
+++ b/ucb/source/ucp/webdav-curl/DAVResourceAccess.cxx
@@ -484,7 +484,6 @@ void DAVResourceAccess::GET0(
 {
     initialize();
 
-    uno::Reference< io::XInputStream > xStream;
     int errorCount = 0;
     bool bRetry;
     do
diff --git a/ucb/source/ucp/webdav-curl/webdavcontent.cxx b/ucb/source/ucp/webdav-curl/webdavcontent.cxx
index 3beb87b9593f..30f141368e3a 100644
--- a/ucb/source/ucp/webdav-curl/webdavcontent.cxx
+++ b/ucb/source/ucp/webdav-curl/webdavcontent.cxx
@@ -2994,7 +2994,6 @@ Content::ResourceType Content::resourceTypeForLocks(
     {
         osl::MutexGuard g(m_aMutex);
         //check if cache contains what we need, usually the first PROPFIND on the URI has supported lock
-        std::unique_ptr< ContentProperties > xProps;
         if (m_xCachedProps)
         {
             uno::Sequence< ucb::LockEntry > aSupportedLocks;
diff --git a/xmloff/source/chart/XMLChartPropertyContext.cxx b/xmloff/source/chart/XMLChartPropertyContext.cxx
index d39b36bfc3ad..a06cad4780f4 100644
--- a/xmloff/source/chart/XMLChartPropertyContext.cxx
+++ b/xmloff/source/chart/XMLChartPropertyContext.cxx
@@ -47,8 +47,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLChartPropertyContex
     ::std::vector< XMLPropertyState > &rProperties,
     const XMLPropertyState& rProp )
 {
-    SvXMLImportContextRef xContext;
-
     switch( mxMapper->getPropertySetMapper()->GetEntryContextId( rProp.mnIndex ) )
     {
         case XML_SCH_CONTEXT_SPECIAL_SYMBOL_IMAGE:
diff --git a/xmloff/source/chart/XMLChartStyleContext.cxx b/xmloff/source/chart/XMLChartStyleContext.cxx
index 1f8fa14eee27..b17bf5d82711 100644
--- a/xmloff/source/chart/XMLChartStyleContext.cxx
+++ b/xmloff/source/chart/XMLChartStyleContext.cxx
@@ -109,8 +109,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLChartStyleContext::
     sal_Int32 nElement,
     const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttrList )
 {
-    SvXMLImportContextRef xContext;
-
     if( IsTokenInNamespace(nElement, XML_NAMESPACE_STYLE) ||
         IsTokenInNamespace(nElement, XML_NAMESPACE_LO_EXT) )
     {
diff --git a/xmloff/source/draw/XMLGraphicsDefaultStyle.cxx b/xmloff/source/draw/XMLGraphicsDefaultStyle.cxx
index 2224b236c7aa..7bb493731f68 100644
--- a/xmloff/source/draw/XMLGraphicsDefaultStyle.cxx
+++ b/xmloff/source/draw/XMLGraphicsDefaultStyle.cxx
@@ -62,8 +62,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLGraphicsDefaultStyl
     sal_Int32 nElement,
     const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttrList )
 {
-    SvXMLImportContextRef xContext;
-
     if( IsTokenInNamespace(nElement, XML_NAMESPACE_STYLE) ||
         IsTokenInNamespace(nElement, XML_NAMESPACE_LO_EXT) )
     {
diff --git a/xmloff/source/draw/XMLShapeStyleContext.cxx b/xmloff/source/draw/XMLShapeStyleContext.cxx
index 98191b71c8cd..da4341731b0f 100644
--- a/xmloff/source/draw/XMLShapeStyleContext.cxx
+++ b/xmloff/source/draw/XMLShapeStyleContext.cxx
@@ -88,8 +88,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLShapeStyleContext::
     sal_Int32 nElement,
     const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttrList )
 {
-    SvXMLImportContextRef xContext;
-
     if( IsTokenInNamespace(nElement, XML_NAMESPACE_STYLE) ||
         IsTokenInNamespace(nElement, XML_NAMESPACE_LO_EXT) )
     {
diff --git a/xmloff/source/text/XMLTextPropertySetContext.cxx b/xmloff/source/text/XMLTextPropertySetContext.cxx
index 2dc3af0eaa3f..b2db201cac2e 100644
--- a/xmloff/source/text/XMLTextPropertySetContext.cxx
+++ b/xmloff/source/text/XMLTextPropertySetContext.cxx
@@ -55,8 +55,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLTextPropertySetCont
     ::std::vector< XMLPropertyState > &rProperties,
     const XMLPropertyState& rProp )
 {
-    SvXMLImportContextRef xContext;
-
     switch( mxMapper->getPropertySetMapper()
                     ->GetEntryContextId( rProp.mnIndex ) )
     {
diff --git a/xmloff/source/text/XMLTextShapeStyleContext.cxx b/xmloff/source/text/XMLTextShapeStyleContext.cxx
index 7bc12c2f9773..81df6cf85d6b 100644
--- a/xmloff/source/text/XMLTextShapeStyleContext.cxx
+++ b/xmloff/source/text/XMLTextShapeStyleContext.cxx
@@ -80,8 +80,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLTextShapePropertySe
     ::std::vector< XMLPropertyState > &rProperties,
     const XMLPropertyState& rProp )
 {
-    SvXMLImportContextRef xContext;
-
     switch( mxMapper->getPropertySetMapper()
                     ->GetEntryContextId( rProp.mnIndex ) )
     {
diff --git a/xmloff/source/text/txtvfldi.cxx b/xmloff/source/text/txtvfldi.cxx
index 35dcd65b5528..8725435a67b5 100644
--- a/xmloff/source/text/txtvfldi.cxx
+++ b/xmloff/source/text/txtvfldi.cxx
@@ -644,8 +644,6 @@ css::uno::Reference< css::xml::sax::XFastContextHandler > XMLVariableDeclsImport
         sal_Int32 nElement,
         const css::uno::Reference< css::xml::sax::XFastAttributeList >& xAttrList )
 {
-    SvXMLImportContextRef xImportContext;
-
     if( IsTokenInNamespace(nElement, XML_NAMESPACE_TEXT) )
     {
         enum XMLTokenEnum eElementName;