new loplugin:mapindex

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

compilerplugins/clang/mapindex.cxx

@@ -0,0 +1,105 @@
+/* -*- 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 <memory>
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+
+#include "check.hxx"
+#include "plugin.hxx"
+
+/**
+  Look for places we are using map[idx] in a bool context, which allocates an entry in the map, which is sometimes a side-effect we don't want.
+*/
+
+namespace
+{
+class MapIndex : public loplugin::FilteringPlugin<MapIndex>
+{
+public:
+    explicit MapIndex(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual void run() override
+    {
+        if (preRun())
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool VisitImplicitCastExpr(const ImplicitCastExpr*);
+    bool VisitMemberExpr(const MemberExpr*);
+};
+
+bool MapIndex::VisitMemberExpr(const MemberExpr* memberExpr)
+{
+    if (ignoreLocation(memberExpr))
+        return true;
+    // operator bool conversion
+    auto conversionDecl = dyn_cast<CXXConversionDecl>(memberExpr->getMemberDecl());
+    if (!conversionDecl || !conversionDecl->getConversionType()->isBooleanType())
+        return true;
+    auto operatorCall = dyn_cast<CXXOperatorCallExpr>(memberExpr->getBase()->IgnoreCasts());
+    if (!operatorCall)
+        return true;
+    if (operatorCall->getOperator() != OverloadedOperatorKind::OO_Subscript)
+        return true;
+    auto tc = loplugin::TypeCheck(operatorCall->getArg(0)->getType());
+    if (!tc.Class("map") && !tc.Class("unordered_map"))
+        return true;
+    report(DiagnosticsEngine::Warning,
+           "will create an empty entry in the map, you sure about that, rather use count()1",
+           operatorCall->getExprLoc());
+    return true;
+}
+
+bool MapIndex::VisitImplicitCastExpr(const ImplicitCastExpr* implicitCastExpr)
+{
+    if (ignoreLocation(implicitCastExpr))
+    {
+        return true;
+    }
+
+    // first cast is some kind of "ToBoolean" cast
+    auto ck = implicitCastExpr->getCastKind();
+    if (ck != CK_MemberPointerToBoolean && ck != CK_PointerToBoolean && ck != CK_IntegralToBoolean
+        && ck != CK_FloatingToBoolean && ck != CK_FloatingComplexToBoolean
+        && ck != CK_IntegralComplexToBoolean)
+        return true;
+
+    // second cast is LValueToRValue
+    implicitCastExpr = dyn_cast<ImplicitCastExpr>(implicitCastExpr->getSubExpr());
+    if (!implicitCastExpr)
+        return true;
+
+    if (implicitCastExpr->getCastKind() != CK_LValueToRValue)
+        return true;
+    auto operatorCall = dyn_cast<CXXOperatorCallExpr>(implicitCastExpr->getSubExpr());
+    if (!operatorCall)
+        return true;
+    if (operatorCall->getOperator() != OverloadedOperatorKind::OO_Subscript)
+        return true;
+    auto tc = loplugin::TypeCheck(operatorCall->getArg(0)->getType());
+    if (!tc.Class("map") && !tc.Class("unordered_map"))
+        return true;
+    report(DiagnosticsEngine::Warning,
+           "will create an empty entry in the map, you sure about that, rather use count()2",
+           implicitCastExpr->getExprLoc());
+    return true;
+}
+
+loplugin::Plugin::Registration<MapIndex> X("mapindex");
+
+} // namespace
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */

compilerplugins/clang/test/mapindex.cxx

@@ -0,0 +1,42 @@
+/* -*- 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 <map>
+#include <memory>
+
+struct CallbackFlushHandler
+{
+};
+
+struct LibLODocument_Impl
+{
+    std::map<size_t, std::shared_ptr<CallbackFlushHandler>> mpCallbackFlushHandlers;
+};
+
+void foo(LibLODocument_Impl* pDoc)
+{
+    std::map<int, int> aMap;
+    if (aMap[0]) // expected-error {{will create an empty entry in the map, you sure about that, rather use count()2 [loplugin:mapindex]}}
+        ;
+
+    // expected-error@+1 {{will create an empty entry in the map, you sure about that, rather use count()1 [loplugin:mapindex]}}
+    if (pDoc->mpCallbackFlushHandlers[0])
+        ;
+}
+
+void no_warning_expected(const std::string& payload)
+{
+    for (size_t numberPos = 0; numberPos < payload.length(); ++numberPos)
+    {
+        if (payload[numberPos] == ',')
+            break;
+    }
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

desktop/source/lib/init.cxx

@@ -3261,7 +3261,7 @@ static void doc_postUnoCommand(LibreOfficeKitDocument* pThis, const char* pComma
     }
 
     bool bResult = false;
-    if (bNotifyWhenFinished && pDocument->mpCallbackFlushHandlers[nView])
+    if (bNotifyWhenFinished && pDocument->mpCallbackFlushHandlers.count(nView))
     {
         bResult = comphelper::dispatchCommand(aCommand, comphelper::containerToSequence(aPropertyValuesVector),
                 new DispatchResultListener(pCommand, pDocument->mpCallbackFlushHandlers[nView]));

hwpfilter/source/hpara.cxx

@@ -165,9 +165,10 @@ bool HWPPara::Read(HWPFile & hwpf, unsigned char flag)
     ii = 0;
     while (ii < nch)
     {
-        hhstr[ii] = readHBox(hwpf);
-        if (!hhstr[ii])
+        auto hBox = readHBox(hwpf);
+        if (!hBox)
             return false;
+        hhstr[ii] = std::move(hBox);
         if (hhstr[ii]->hh == CH_END_PARA)
             break;
         if( hhstr[ii]->hh < CH_END_PARA )

include/comphelper/IdPropArrayHelper.hxx

@@ -100,9 +100,10 @@ namespace comphelper
         OSL_ENSURE(s_nRefCount, "OIdPropertyArrayUsageHelper::getArrayHelper : suspicious call : have a refcount of 0 !");
         ::osl::MutexGuard aGuard(OIdPropertyArrayUsageHelperMutex<TYPE>::get());
         // do we have the array already?
-        if (! (*s_pMap)[nId] )
+        auto& rEntry = (*s_pMap)[nId];
+        if (!rEntry)
         {
-            (*s_pMap)[nId] = createArrayHelper(nId);
+            rEntry = createArrayHelper(nId);
             OSL_ENSURE((*s_pMap)[nId], "OIdPropertyArrayUsageHelper::getArrayHelper : createArrayHelper returned nonsense !");
         }
         return (*s_pMap)[nId];

l10ntools/source/xmlparse.cxx

@@ -376,7 +376,7 @@ void XMLFile::InsertL10NElement( XMLElement* pElement )
     else        // Already there
     {
         pElem=pos->second;
-        if ( (*pElem)[ sLanguage ] )
+        if ( pElem->count(sLanguage) )
         {
             fprintf(stdout,"Error: Duplicated entry. ID = %s  LANG = %s in File %s\n", sId.getStr(), sLanguage.getStr(), m_sFileName.getStr() );
             exit( -1 );

solenv/CompilerTest_compilerplugins_clang.mk

@@ -36,6 +36,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/indentation \
     compilerplugins/clang/test/logexceptionnicely \
     compilerplugins/clang/test/loopvartoosmall \
+    compilerplugins/clang/test/mapindex \
     compilerplugins/clang/test/oncevar \
     compilerplugins/clang/test/oslendian-1 \
     compilerplugins/clang/test/oslendian-2 \