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>

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");

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

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);
 

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: */

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

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

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);
 

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())

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, "");

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();

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();

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();

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

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;

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:

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) )
     {

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) )
     {

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) )
     {

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 ) )
     {

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 ) )
     {

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;