New loplugin:empty

Change-Id: I8729db064573ac21dfe6b203c5ae244d79ecc4fe
Reviewed-on: https://gerrit.libreoffice.org/66430
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

6 files changed, 195 insertions, 9 deletions

diff --git a/compilerplugins/clang/empty.cxx b/compilerplugins/clang/empty.cxx
new file mode 100644
index 000000000000..74cc7b06e176
--- /dev/null
+++ b/compilerplugins/clang/empty.cxx
@@ -0,0 +1,186 @@
+/* -*- 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 <cassert>
+
+#include "check.hxx"
+#include "plugin.hxx"
+
+// Warn about checks whether a container is empty done via an (expensive) call to obtain the
+// container's size.  For now only handles cases involving strlen.
+
+namespace
+{
+BinaryOperatorKind revert(BinaryOperatorKind op)
+{
+    switch (op)
+    {
+        case BO_LT:
+            return BO_GE;
+        case BO_GT:
+            return BO_LE;
+        case BO_LE:
+            return BO_GT;
+        case BO_GE:
+            return BO_LT;
+        case BO_EQ:
+        case BO_NE:
+            return op;
+        default:
+            assert(false);
+    }
+}
+
+class Empty : public loplugin::FilteringPlugin<Empty>
+{
+public:
+    explicit Empty(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    bool VisitBinLT(BinaryOperator const* expr)
+    {
+        visitComparison(expr);
+        return true;
+    }
+
+    bool VisitBinGT(BinaryOperator const* expr)
+    {
+        visitComparison(expr);
+        return true;
+    }
+
+    bool VisitBinLE(BinaryOperator const* expr)
+    {
+        visitComparison(expr);
+        return true;
+    }
+
+    bool VisitBinGE(BinaryOperator const* expr)
+    {
+        visitComparison(expr);
+        return true;
+    }
+
+    bool VisitBinEQ(BinaryOperator const* expr)
+    {
+        visitComparison(expr);
+        return true;
+    }
+
+    bool VisitBinNE(BinaryOperator const* expr)
+    {
+        visitComparison(expr);
+        return true;
+    }
+
+private:
+    void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+    void visitComparison(BinaryOperator const* expr, CallExpr const* lhs, Expr const* rhs,
+                         BinaryOperatorKind op)
+    {
+        auto const fdecl = lhs->getDirectCallee();
+        if (fdecl == nullptr)
+        {
+            return;
+        }
+        loplugin::DeclCheck dc(fdecl);
+        if (!(dc.Function("strlen").StdNamespace() || dc.Function("strlen").GlobalNamespace()))
+        {
+            return;
+        }
+        APSInt val;
+        if (!rhs->isIntegerConstantExpr(val, compiler.getASTContext()))
+        {
+            return;
+        }
+        switch (op)
+        {
+            case BO_LT:
+                if (val.getExtValue() == 1)
+                {
+                    report(DiagnosticsEngine::Warning,
+                           "replace a comparison like 'strlen(e) < 1' with 'e[0] == '\\0''",
+                           expr->getExprLoc())
+                        << expr->getSourceRange();
+                }
+                break;
+            case BO_GT:
+                if (val.getExtValue() == 0)
+                {
+                    report(DiagnosticsEngine::Warning,
+                           "replace a comparison like 'strlen(e) > 0' with 'e[0] != '\\0''",
+                           expr->getExprLoc())
+                        << expr->getSourceRange();
+                }
+                break;
+            case BO_LE:
+                if (val.getExtValue() == 0)
+                {
+                    report(DiagnosticsEngine::Warning,
+                           "replace a comparison like 'strlen(e) <= 0' with 'e[0] == '\\0''",
+                           expr->getExprLoc())
+                        << expr->getSourceRange();
+                }
+                break;
+            case BO_GE:
+                if (val.getExtValue() == 1)
+                {
+                    report(DiagnosticsEngine::Warning,
+                           "replace a comparison like 'strlen(e) >= 1' with 'e[0] != '\\0''",
+                           expr->getExprLoc())
+                        << expr->getSourceRange();
+                }
+                break;
+            case BO_EQ:
+                if (val.getExtValue() == 0)
+                {
+                    report(DiagnosticsEngine::Warning,
+                           "replace a comparison like 'strlen(e) == 0' with 'e[0] == '\\0''",
+                           expr->getExprLoc())
+                        << expr->getSourceRange();
+                }
+                break;
+            case BO_NE:
+                if (val.getExtValue() == 0)
+                {
+                    report(DiagnosticsEngine::Warning,
+                           "replace a comparison like 'strlen(e) != 0' with 'e[0] != '\\0''",
+                           expr->getExprLoc())
+                        << expr->getSourceRange();
+                }
+                break;
+            default:
+                assert(false);
+        }
+    }
+
+    void visitComparison(BinaryOperator const* expr)
+    {
+        if (ignoreLocation(expr))
+        {
+            return;
+        }
+        if (auto const call = dyn_cast<CallExpr>(expr->getLHS()->IgnoreParenImpCasts()))
+        {
+            visitComparison(expr, call, expr->getRHS(), expr->getOpcode());
+        }
+        else if (auto const call = dyn_cast<CallExpr>(expr->getRHS()->IgnoreParenImpCasts()))
+        {
+            visitComparison(expr, call, expr->getLHS(), revert(expr->getOpcode()));
+        }
+    }
+};
+
+loplugin::Plugin::Registration<Empty> X("empty");
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/connectivity/source/drivers/evoab2/NResultSet.cxx b/connectivity/source/drivers/evoab2/NResultSet.cxx
index 5ae85599ecb2..c48073ed2a41 100644
--- a/connectivity/source/drivers/evoab2/NResultSet.cxx
+++ b/connectivity/source/drivers/evoab2/NResultSet.cxx
@@ -157,7 +157,7 @@ static EContactAddress *
 getDefaultContactAddress( EContact *pContact,int *value )
 {
     EContactAddress *ec = static_cast<EContactAddress *>(e_contact_get(pContact,whichAddress(WORK_ADDR_LINE1)));
-    if ( ec && (strlen(ec->street)>0) )
+    if ( ec && (ec->street[0]!='\0') )
     {
         *value= *value +WORK_ADDR_LINE1 -1;
         return ec;
@@ -165,7 +165,7 @@ getDefaultContactAddress( EContact *pContact,int *value )
     else
         {
             ec = static_cast<EContactAddress *>(e_contact_get(pContact,whichAddress(HOME_ADDR_LINE1)));
-            if ( ec && (strlen(ec->street)>0) )
+            if ( ec && (ec->street[0]!='\0') )
             {
                 *value=*value+HOME_ADDR_LINE1-1;
                 return ec;
diff --git a/sal/osl/unx/pipe.cxx b/sal/osl/unx/pipe.cxx
index c38ab152dada..10cb87770128 100644
--- a/sal/osl/unx/pipe.cxx
+++ b/sal/osl/unx/pipe.cxx
@@ -392,7 +392,7 @@ void SAL_CALL osl_closePipe(oslPipe pPipe)
         SAL_WARN("sal.osl.pipe", "close() failed: " << UnixErrnoString(errno));
 
     /* remove filesystem entry */
-    if (strlen(pPipe->m_Name) > 0)
+    if (pPipe->m_Name[0] != '\0')
         unlink(pPipe->m_Name);
 
     pPipe->m_bClosed = true;
@@ -407,7 +407,7 @@ oslPipe SAL_CALL osl_acceptPipe(oslPipe pPipe)
     if (!pPipe)
         return nullptr;
 
-    assert(strlen(pPipe->m_Name) > 0);  // you cannot have an empty pipe name
+    assert(pPipe->m_Name[0] != '\0');  // you cannot have an empty pipe name
 
 #if defined(CLOSESOCKET_DOESNT_WAKE_UP_ACCEPT)
     pPipe->m_bIsAccepting = true;
diff --git a/sal/osl/unx/security.cxx b/sal/osl/unx/security.cxx
index 29897d8e75de..982b7c940c2f 100644
--- a/sal/osl/unx/security.cxx
+++ b/sal/osl/unx/security.cxx
@@ -320,7 +320,7 @@ static bool osl_psz_getHomeDir(oslSecurity Security, OString* pszDirectory)
         pStr = getenv("HOME");
 #endif
 
-        if (pStr != nullptr && strlen(pStr) > 0 && access(pStr, 0) == 0)
+        if (pStr != nullptr && pStr[0] != '\0' && access(pStr, 0) == 0)
         {
             auto const len = std::strlen(pStr);
             if (len > sal_uInt32(std::numeric_limits<sal_Int32>::max())) {
@@ -389,7 +389,7 @@ static bool osl_psz_getConfigDir(oslSecurity Security, OString* pszDirectory)
 
     sal_Char *pStr = getenv("XDG_CONFIG_HOME");
 
-    if (pStr == nullptr || strlen(pStr) == 0 || access(pStr, 0) != 0)
+    if (pStr == nullptr || pStr[0] == '\0' || access(pStr, 0) != 0)
     {
         // a default equal to $HOME/.config should be used.
         OString home;
diff --git a/sal/osl/unx/socket.cxx b/sal/osl/unx/socket.cxx
index dd84afc0ccc4..0765daaef2e7 100644
--- a/sal/osl/unx/socket.cxx
+++ b/sal/osl/unx/socket.cxx
@@ -903,7 +903,7 @@ oslSocketResult osl_psz_getLocalHostname (
 {
     static sal_Char LocalHostname[256] = "";
 
-    if (strlen(LocalHostname) == 0)
+    if (LocalHostname[0] == '\0')
     {
 
 #ifdef SYSV
@@ -940,7 +940,7 @@ oslSocketResult osl_psz_getLocalHostname (
         }
     }
 
-    if (strlen(LocalHostname) > 0)
+    if (LocalHostname[0] != '\0')
     {
         strncpy(pBuffer, LocalHostname, nBufLen);
         pBuffer[nBufLen - 1] = '\0';
diff --git a/xmloff/qa/unit/uxmloff.cxx b/xmloff/qa/unit/uxmloff.cxx
index 4cba6e41fc16..a5e11528fe81 100644
--- a/xmloff/qa/unit/uxmloff.cxx
+++ b/xmloff/qa/unit/uxmloff.cxx
@@ -210,7 +210,7 @@ void Test::testMetaGenerator()
 
         SvXMLMetaDocumentContext::setBuildId(
                 OUString::createFromAscii(tests[i].generator), xInfoSet);
-        if (std::strlen(tests[i].buildId) != 0)
+        if (tests[i].buildId[0] != '\0')
         {
             CPPUNIT_ASSERT_EQUAL(OUString::createFromAscii(tests[i].buildId),
                     xInfoSet->getPropertyValue("BuildId").get<OUString>());