loplugin:simplifybool extend to !(a == b) where comparison an overloaded op

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

32 files changed, 201 insertions, 56 deletions

diff --git a/chart2/source/tools/DataSeriesHelper.cxx b/chart2/source/tools/DataSeriesHelper.cxx
index 9a2957a11c37..389a10ededb8 100644
--- a/chart2/source/tools/DataSeriesHelper.cxx
+++ b/chart2/source/tools/DataSeriesHelper.cxx
@@ -625,7 +625,7 @@ bool hasAttributedDataPointDifferentValue( const Reference< chart2::XDataSeries
             if(!xPointProp.is())
                 continue;
             uno::Any aPointValue( xPointProp->getPropertyValue( rPropertyName ) );
-            if( !( rPropertyValue==aPointValue ) )
+            if( rPropertyValue != aPointValue )
                 return true;
         }
     }
diff --git a/chart2/source/view/main/Clipping.cxx b/chart2/source/view/main/Clipping.cxx
index 354871612233..a7c212a91049 100644
--- a/chart2/source/view/main/Clipping.cxx
+++ b/chart2/source/view/main/Clipping.cxx
@@ -260,7 +260,7 @@ void Clipping::clipPolygonAtRectangle( const drawing::PolyPolygonShape3D& rPolyg
                 // compose an Polygon of as many consecutive points as possible
                 if(aFrom == aLast)
                 {
-                    if( !(aTo==aFrom) )
+                    if( aTo != aFrom )
                     {
                         lcl_addPointToPoly( aResult, aTo, nNewPolyIndex, aResultPointCount, nOldPointCount );
                     }
@@ -274,7 +274,7 @@ void Clipping::clipPolygonAtRectangle( const drawing::PolyPolygonShape3D& rPolyg
                             nNewPolyIndex++;
                     }
                     lcl_addPointToPoly( aResult, aFrom, nNewPolyIndex, aResultPointCount, nOldPointCount );
-                    if( !(aTo==aFrom) )
+                    if( aTo != aFrom )
                         lcl_addPointToPoly( aResult, aTo, nNewPolyIndex, aResultPointCount, nOldPointCount );
                 }
                 aLast = aTo;
diff --git a/compilerplugins/clang/simplifybool.cxx b/compilerplugins/clang/simplifybool.cxx
index 7109fcfb96a9..b4752b4108aa 100644
--- a/compilerplugins/clang/simplifybool.cxx
+++ b/compilerplugins/clang/simplifybool.cxx
@@ -53,6 +53,38 @@ Expr const * getSubExprOfLogicalNegation(Expr const * expr) {
         ? nullptr : e->getSubExpr();
 }
 
+bool existsOperator(CompilerInstance& compiler, clang::RecordType const * recordType, BinaryOperator::Opcode opcode) {
+    OverloadedOperatorKind over = BinaryOperator::getOverloadedOperator(opcode);
+    CXXRecordDecl const * recordDecl = dyn_cast<CXXRecordDecl>(recordType->getDecl());
+    if (!recordDecl)
+        return false;
+    // search for member overloads
+    for (auto it = recordDecl->method_begin(); it != recordDecl->method_end(); ++it) {
+        if (it->getOverloadedOperator() == over) {
+            return true;
+        }
+    }
+    // search for free function overloads
+    auto ctx = recordDecl->getDeclContext();
+    if (ctx->getDeclKind() == Decl::LinkageSpec) {
+        ctx = ctx->getParent();
+    }
+    auto declName = compiler.getASTContext().DeclarationNames.getCXXOperatorName(over);
+    auto res = ctx->lookup(declName);
+    for (auto d = res.begin(); d != res.end(); ++d) {
+        FunctionDecl const * f = dyn_cast<FunctionDecl>(*d);
+        if (!f || f->getNumParams() != 2)
+            continue;
+        auto qt = f->getParamDecl(0)->getType();
+        auto lvalue = dyn_cast<LValueReferenceType>(qt.getTypePtr());
+        if (!lvalue)
+            continue;
+        if (lvalue->getPointeeType().getTypePtr() == recordType)
+            return true;
+    }
+    return false;
+}
+
 enum class Value { Unknown, False, True };
 
 Value getValue(Expr const * expr) {
@@ -99,6 +131,13 @@ public:
     bool VisitBinNE(BinaryOperator const * expr);
 
     bool VisitConditionalOperator(ConditionalOperator const * expr);
+
+    bool TraverseFunctionDecl(FunctionDecl *);
+
+    bool TraverseCXXMethodDecl(CXXMethodDecl *);
+
+private:
+    FunctionDecl* m_insideFunctionDecl = nullptr;
 };
 
 void SimplifyBool::run() {
@@ -138,15 +177,50 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
         // triggers.
         if (compat::getBeginLoc(binaryOp).isMacroID())
             return true;
+        if (!binaryOp->isComparisonOp())
+            return true;
         auto t = binaryOp->getLHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
-        // RecordType would require more smarts - we'd need to verify that an inverted operator actually existed
-        if (t->isTemplateTypeParmType() || t->isRecordType() || t->isDependentType())
+        if (t->isTemplateTypeParmType() || t->isDependentType() || t->isRecordType())
             return true;
         // for floating point (with NaN) !(x<y) need not be equivalent to x>=y
         if (t->isFloatingType() ||
             binaryOp->getRHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType()->isFloatingType())
             return true;
-        if (!binaryOp->isComparisonOp())
+        report(
+            DiagnosticsEngine::Warning,
+            ("logical negation of comparison operator, can be simplified by inverting operator"),
+            compat::getBeginLoc(expr))
+            << expr->getSourceRange();
+    }
+    if (auto binaryOp = dyn_cast<CXXOperatorCallExpr>(expr->getSubExpr()->IgnoreParenImpCasts())) {
+        // Ignore macros, otherwise
+        //    OSL_ENSURE(!b, ...);
+        // triggers.
+        if (compat::getBeginLoc(binaryOp).isMacroID())
+            return true;
+        auto op = binaryOp->getOperator();
+        // Negating things like > and >= would probably not be wise, there is no guarantee the negation holds for operator overloaded types.
+        // However, == and != are normally considered ok.
+        if (!(op == OO_EqualEqual || op == OO_ExclaimEqual))
+            return true;
+        BinaryOperator::Opcode negatedOpcode = BinaryOperator::negateComparisonOp(BinaryOperator::getOverloadedOpcode(op));
+        auto t = binaryOp->getArg(0)->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
+        // we need to verify that a negated operator actually existed
+        if (!t->isRecordType())
+            return true;
+        auto recordType = dyn_cast<RecordType>(t);
+        if (!existsOperator(compiler, recordType, negatedOpcode))
+            return true;
+        // if we are inside a similar operator, ignore, eg. operator!= is often defined by calling !operator==
+        if (m_insideFunctionDecl && m_insideFunctionDecl->getNumParams() >= 1) {
+            auto qt = m_insideFunctionDecl->getParamDecl(0)->getType();
+            auto lvalue = dyn_cast<LValueReferenceType>(qt.getTypePtr());
+            if (lvalue && lvalue->getPointeeType()->getUnqualifiedDesugaredType() == recordType)
+                return true;
+        }
+        // QA code
+        StringRef fn(handler.getMainFileName());
+        if (loplugin::isSamePathname(fn, SRCDIR "/testtools/source/bridgetest/bridgetest.cxx"))
             return true;
         report(
             DiagnosticsEngine::Warning,
@@ -1086,6 +1160,22 @@ bool SimplifyBool::VisitConditionalOperator(ConditionalOperator const * expr) {
     return true;
 }
 
+bool SimplifyBool::TraverseFunctionDecl(FunctionDecl * functionDecl) {
+    auto copy = m_insideFunctionDecl;
+    m_insideFunctionDecl = functionDecl;
+    bool ret = RecursiveASTVisitor::TraverseFunctionDecl(functionDecl);
+    m_insideFunctionDecl = copy;
+    return ret;
+}
+
+bool SimplifyBool::TraverseCXXMethodDecl(CXXMethodDecl * functionDecl) {
+    auto copy = m_insideFunctionDecl;
+    m_insideFunctionDecl = functionDecl;
+    bool ret = RecursiveASTVisitor::TraverseCXXMethodDecl(functionDecl);
+    m_insideFunctionDecl = copy;
+    return ret;
+}
+
 loplugin::Plugin::Registration<SimplifyBool> X("simplifybool");
 
 }
diff --git a/compilerplugins/clang/test/simplifybool.cxx b/compilerplugins/clang/test/simplifybool.cxx
index 2cb2e810c110..01549f320ab0 100644
--- a/compilerplugins/clang/test/simplifybool.cxx
+++ b/compilerplugins/clang/test/simplifybool.cxx
@@ -7,6 +7,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include <rtl/ustring.hxx>
+
 void f1(int a, int b)
 {
     if (!(a < b))
@@ -56,4 +58,60 @@ bool f2(E2 e) { return !!(e & E2_1); }
 
 bool f3(E3 e) { return !!(e & E3::E1); }
 
+// record types
+
+struct Record1
+{
+    bool operator==(const Record1&) const;
+};
+
+struct Record2
+{
+    bool operator==(const Record2&) const;
+    bool operator!=(const Record2&) const;
+};
+
+struct Record3
+{
+};
+
+bool operator==(const Record3&, const Record3&);
+bool operator!=(const Record3&, const Record3&);
+
+void testRecord()
+{
+    Record1 a1;
+    Record1 a2;
+    // no warning expected, because a negated operator does not exist
+    bool v = !(a1 == a2);
+    Record2 b1;
+    Record2 b2;
+    v = !(b1 == b2);
+    // expected-error@-1 {{logical negation of comparison operator, can be simplified by inverting operator [loplugin:simplifybool]}}
+    Record3 c1;
+    Record3 c2;
+    v = !(c1 == c2);
+    // expected-error@-1 {{logical negation of comparison operator, can be simplified by inverting operator [loplugin:simplifybool]}}
+    OUString d1;
+    OUString d2;
+    v = !(d1 == d2);
+    // expected-error@-1 {{logical negation of comparison operator, can be simplified by inverting operator [loplugin:simplifybool]}}
+}
+
+struct Record4
+{
+    bool operator==(Record4 const&) const;
+    bool operator!=(Record4 const& other) const
+    {
+        // no warning expected
+        bool v = !operator==(other);
+        v = !(*this == other);
+        OUString c1;
+        OUString c2;
+        v = !(c1 == c2);
+        // expected-error@-1 {{logical negation of comparison operator, can be simplified by inverting operator [loplugin:simplifybool]}}
+        return v;
+    }
+};
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/connectivity/source/drivers/hsqldb/HCatalog.cxx b/connectivity/source/drivers/hsqldb/HCatalog.cxx
index 9824ead35dc8..029e60f9450a 100644
--- a/connectivity/source/drivers/hsqldb/HCatalog.cxx
+++ b/connectivity/source/drivers/hsqldb/HCatalog.cxx
@@ -138,7 +138,7 @@ Sequence< Type > SAL_CALL OHCatalog::getTypes(  )
     const Type* pEnd = pBegin + aTypes.getLength();
     for(;pBegin != pEnd;++pBegin)
     {
-        if ( !(*pBegin == cppu::UnoType<XGroupsSupplier>::get()))
+        if ( *pBegin != cppu::UnoType<XGroupsSupplier>::get())
         {
             aOwnTypes.push_back(*pBegin);
         }
diff --git a/connectivity/source/drivers/mork/MResultSet.cxx b/connectivity/source/drivers/mork/MResultSet.cxx
index 9f3d2e38f33a..880c035dbf6a 100644
--- a/connectivity/source/drivers/mork/MResultSet.cxx
+++ b/connectivity/source/drivers/mork/MResultSet.cxx
@@ -1001,7 +1001,7 @@ static bool matchRow( OValueRow const & row1, OValueRow const & row2 )
         if ( row1Iter->isBound())
         {
             // Compare values, if at anytime there's a mismatch return false
-            if ( !( (*row1Iter) == (*row2Iter) ) )
+            if ( *row1Iter != *row2Iter )
                 return false;
         }
     }
diff --git a/connectivity/source/drivers/mysql_jdbc/YCatalog.cxx b/connectivity/source/drivers/mysql_jdbc/YCatalog.cxx
index 2c84d204f2bc..7255642dadda 100644
--- a/connectivity/source/drivers/mysql_jdbc/YCatalog.cxx
+++ b/connectivity/source/drivers/mysql_jdbc/YCatalog.cxx
@@ -122,7 +122,7 @@ Sequence<Type> SAL_CALL OMySQLCatalog::getTypes()
     const Type* pEnd = pBegin + aTypes.getLength();
     for (; pBegin != pEnd; ++pBegin)
     {
-        if (!(*pBegin == cppu::UnoType<XGroupsSupplier>::get()))
+        if (*pBegin != cppu::UnoType<XGroupsSupplier>::get())
         {
             aOwnTypes.push_back(*pBegin);
         }
diff --git a/dbaccess/source/core/dataaccess/documentdefinition.cxx b/dbaccess/source/core/dataaccess/documentdefinition.cxx
index ad9cca9716ed..c69d42b2418f 100644
--- a/dbaccess/source/core/dataaccess/documentdefinition.cxx
+++ b/dbaccess/source/core/dataaccess/documentdefinition.cxx
@@ -1553,7 +1553,7 @@ void ODocumentDefinition::loadEmbeddedObject( const Reference< XConnection >& i_
                 sDocumentService = GetDocumentServiceFromMediaType( getContentType(), m_aContext, aClassID );
                 // check if we are not a form and
                 // the org.libreoffice.report.pentaho.SOReportJobFactory is not present.
-                if ( !m_bForm && !(sDocumentService == "com.sun.star.text.TextDocument"))
+                if ( !m_bForm && sDocumentService != "com.sun.star.text.TextDocument")
                 {
                     // we seem to be a "new style" report, check if report extension is present.
                     Reference< XContentEnumerationAccess > xEnumAccess( m_aContext->getServiceManager(), UNO_QUERY );
diff --git a/desktop/source/deployment/manager/dp_extensionmanager.cxx b/desktop/source/deployment/manager/dp_extensionmanager.cxx
index d19c127b1e30..b6f8887da08e 100644
--- a/desktop/source/deployment/manager/dp_extensionmanager.cxx
+++ b/desktop/source/deployment/manager/dp_extensionmanager.cxx
@@ -910,7 +910,7 @@ void ExtensionManager::enableExtension(
         if (!extension.is())
             return;
         OUString repository = extension->getRepositoryName();
-        if (!(repository == "user"))
+        if (repository != "user")
             throw lang::IllegalArgumentException(
                 "No valid repository name provided.",
                 static_cast<cppu::OWeakObject*>(this), 0);
@@ -1012,7 +1012,7 @@ void ExtensionManager::disableExtension(
         if (!extension.is())
             return;
         const OUString repository( extension->getRepositoryName());
-        if (! (repository == "user"))
+        if (repository != "user")
             throw lang::IllegalArgumentException(
                 "No valid repository name provided.",
                 static_cast<cppu::OWeakObject*>(this), 0);
diff --git a/desktop/source/deployment/manager/dp_manager.cxx b/desktop/source/deployment/manager/dp_manager.cxx
index 25ca6cdcc2d1..f73eeb2268b2 100644
--- a/desktop/source/deployment/manager/dp_manager.cxx
+++ b/desktop/source/deployment/manager/dp_manager.cxx
@@ -188,7 +188,7 @@ void PackageManagerImpl::initActivationLayer(
         // The data base can always be written because it is always in the user installation
         m_activePackagesDB.reset( new ActivePackages( dbName ) );
 
-        if (! m_readOnly && ! (m_context == "bundled"))
+        if (! m_readOnly && m_context != "bundled")
         {
             // clean up activation layer, scan for zombie temp dirs:
             ActivePackages::Entries id2temp( m_activePackagesDB->getEntries() );
@@ -957,7 +957,7 @@ OUString PackageManagerImpl::getDeployPath( ActivePackages::Data const & data )
     //The bundled extensions are not contained in an additional folder
     //with a unique name. data.temporaryName contains already the
     //UTF8 encoded folder name. See PackageManagerImpl::synchronize
-    if (!(m_context == "bundled"))
+    if (m_context != "bundled")
     {
         buf.append( "_/" );
         buf.append( ::rtl::Uri::encode( data.fileName, rtl_UriCharClassPchar,
@@ -1028,7 +1028,7 @@ PackageManagerImpl::getDeployedPackages_(
     ActivePackages::Entries id2temp( m_activePackagesDB->getEntries() );
     for (auto const& elem : id2temp)
     {
-        if (! (elem.second.failedPrerequisites == "0"))
+        if (elem.second.failedPrerequisites != "0")
             continue;
         try {
             packages.push_back(
diff --git a/desktop/source/deployment/misc/dp_descriptioninfoset.cxx b/desktop/source/deployment/misc/dp_descriptioninfoset.cxx
index 2179d377957b..ecd14d1b1755 100644
--- a/desktop/source/deployment/misc/dp_descriptioninfoset.cxx
+++ b/desktop/source/deployment/misc/dp_descriptioninfoset.cxx
@@ -217,7 +217,7 @@ ExtensionDescription::ExtensionDescription(
                 sDescriptionUri + " contains no root element.", nullptr);
         }
 
-        if ( ! (xRoot->getTagName() == "description"))
+        if ( xRoot->getTagName() != "description")
         {
             throw css::uno::Exception(
                 sDescriptionUri + " does not contain the root element <description>.", nullptr);
@@ -227,7 +227,7 @@ ExtensionDescription::ExtensionDescription(
         OUString nsDescription = xRoot->getNamespaceURI();
 
         //check if this namespace is supported
-        if ( ! (nsDescription == "http://openoffice.org/extensions/description/2006"))
+        if ( nsDescription != "http://openoffice.org/extensions/description/2006")
         {
             throw css::uno::Exception(sDescriptionUri + " contains a root element with an unsupported namespace. ", nullptr);
         }
diff --git a/desktop/source/deployment/registry/executable/dp_executable.cxx b/desktop/source/deployment/registry/executable/dp_executable.cxx
index e312369328dd..95ba4e96e222 100644
--- a/desktop/source/deployment/registry/executable/dp_executable.cxx
+++ b/desktop/source/deployment/registry/executable/dp_executable.cxx
@@ -239,7 +239,7 @@ void BackendImpl::ExecutablePackageImpl::processPackage_(
             else if (getMyBackend()->m_context == "shared")
                 attributes |= (osl_File_Attribute_OwnExe | osl_File_Attribute_GrpExe
                                | osl_File_Attribute_OthExe);
-            else if (!(getMyBackend()->m_context == "bundled"))
+            else if (getMyBackend()->m_context != "bundled")
                 //Bundled extensions are required to be in the properly
                 //installed. That is an executable must have the right flags
                 OSL_ASSERT(false);
diff --git a/desktop/source/pkgchk/unopkg/unopkg_app.cxx b/desktop/source/pkgchk/unopkg/unopkg_app.cxx
index 05ca301dbd0a..f6e4d0c19b7c 100644
--- a/desktop/source/pkgchk/unopkg/unopkg_app.cxx
+++ b/desktop/source/pkgchk/unopkg/unopkg_app.cxx
@@ -393,7 +393,7 @@ extern "C" int unopkg_main()
         //Do not synchronize when command is "reinstall". This could add types and services to UNO and
         //prevent the deletion of the registry data folder
         //syncing is done in XExtensionManager.reinstall
-        if (!subcmd_gui && ! (subCommand == "reinstall")
+        if (!subcmd_gui && subCommand != "reinstall"
             && ! dp_misc::office_is_running())
             dp_misc::syncRepositories(false, xCmdEnv);
 
diff --git a/framework/inc/properties.h b/framework/inc/properties.h
index 86bc5deeb2ee..de3eb86c5694 100644
--- a/framework/inc/properties.h
+++ b/framework/inc/properties.h
@@ -164,7 +164,7 @@ class PropHelper
         aChangedValue.clear();
 
         // if value change ...
-        bChanged = !(aCurrentValue==aNewValue);
+        bChanged = aCurrentValue != aNewValue;
         if (bChanged)
         {
             // ... set information of change.
diff --git a/sc/source/core/data/dpsave.cxx b/sc/source/core/data/dpsave.cxx
index c5c5139ad170..1c2ab97f3a1e 100644
--- a/sc/source/core/data/dpsave.cxx
+++ b/sc/source/core/data/dpsave.cxx
@@ -254,7 +254,7 @@ bool ScDPSaveDimension::operator== ( const ScDPSaveDimension& r ) const
 
     if( pReferenceValue && r.pReferenceValue )
     {
-        if ( !(*pReferenceValue == *r.pReferenceValue) )
+        if ( *pReferenceValue != *r.pReferenceValue )
         {
             return false;
         }
@@ -265,7 +265,7 @@ bool ScDPSaveDimension::operator== ( const ScDPSaveDimension& r ) const
     }
     if( this->pSortInfo && r.pSortInfo )
     {
-        if ( !(*this->pSortInfo == *r.pSortInfo) )
+        if ( *this->pSortInfo != *r.pSortInfo )
         {
             return false;
         }
@@ -276,7 +276,7 @@ bool ScDPSaveDimension::operator== ( const ScDPSaveDimension& r ) const
     }
     if( this->pAutoShowInfo && r.pAutoShowInfo )
     {
-        if ( !(*this->pAutoShowInfo == *r.pAutoShowInfo) )
+        if ( *this->pAutoShowInfo != *r.pAutoShowInfo )
         {
             return false;
         }
diff --git a/sc/source/filter/excel/excrecds.cxx b/sc/source/filter/excel/excrecds.cxx
index 0bfe9cf4a427..f2b11b0bdd1c 100644
--- a/sc/source/filter/excel/excrecds.cxx
+++ b/sc/source/filter/excel/excrecds.cxx
@@ -251,7 +251,7 @@ Exc1904::Exc1904( const ScDocument& rDoc )
 {
     const Date& rDate = rDoc.GetFormatTable()->GetNullDate();
     bVal = (rDate == Date( 1, 1, 1904 ));
-    bDateCompatibility = !(rDate == Date( 30, 12, 1899 ));
+    bDateCompatibility = (rDate != Date( 30, 12, 1899 ));
 }
 
 sal_uInt16 Exc1904::GetNum() const
diff --git a/sc/source/filter/excel/xehelper.cxx b/sc/source/filter/excel/xehelper.cxx
index 7f4850194341..c4e67e5f57c2 100644
--- a/sc/source/filter/excel/xehelper.cxx
+++ b/sc/source/filter/excel/xehelper.cxx
@@ -734,7 +734,7 @@ void XclExpHFConverter::AppendPortion( const EditTextObject* pTextObj, sal_Unico
                 aNewData.maName = XclTools::GetXclFontName( aFont.GetFamilyName() );
                 aNewData.mnWeight = (aFont.GetWeight() > WEIGHT_NORMAL) ? EXC_FONTWGHT_BOLD : EXC_FONTWGHT_NORMAL;
                 aNewData.mbItalic = (aFont.GetItalic() != ITALIC_NONE);
-                bool bNewFont = !(aFontData.maName == aNewData.maName);
+                bool bNewFont = (aFontData.maName != aNewData.maName);
                 bool bNewStyle = (aFontData.mnWeight != aNewData.mnWeight) ||
                                  (aFontData.mbItalic != aNewData.mbItalic);
                 if( bNewFont || (bNewStyle && pFontList) )
diff --git a/sc/source/ui/view/formatsh.cxx b/sc/source/ui/view/formatsh.cxx
index 2cce67118824..f7315debd5de 100644
--- a/sc/source/ui/view/formatsh.cxx
+++ b/sc/source/ui/view/formatsh.cxx
@@ -2081,7 +2081,7 @@ void ScFormatShell::GetAttrState( SfxItemSet& rSet )
                     {
                         if(aCol != aBoxItem.GetBottom()->GetColor() )
                             bColDisable = true;
-                        if(!( aLine == *(aBoxItem.GetBottom())) )
+                        if( aLine != *aBoxItem.GetBottom() )
                             bStyleDisable = true;
                     }
                 }
@@ -2100,7 +2100,7 @@ void ScFormatShell::GetAttrState( SfxItemSet& rSet )
                     {
                         if(aCol != aBoxItem.GetLeft()->GetColor() )
                             bColDisable = true;
-                        if(!( aLine == *(aBoxItem.GetLeft())) )
+                        if( aLine != *aBoxItem.GetLeft() )
                             bStyleDisable = true;
                     }
                 }
@@ -2119,7 +2119,7 @@ void ScFormatShell::GetAttrState( SfxItemSet& rSet )
                     {
                         if(aCol != aBoxItem.GetRight()->GetColor() )
                             bColDisable = true;
-                        if(!( aLine == *(aBoxItem.GetRight())) )
+                        if( aLine != *aBoxItem.GetRight() )
                             bStyleDisable = true;
                     }
                 }
@@ -2138,7 +2138,7 @@ void ScFormatShell::GetAttrState( SfxItemSet& rSet )
                     {
                         if(aCol != aInfoItem.GetVert()->GetColor() )
                             bColDisable = true;
-                        if(!( aLine == *(aInfoItem.GetVert())) )
+                        if( aLine != *aInfoItem.GetVert() )
                             bStyleDisable = true;
                     }
                 }
@@ -2157,7 +2157,7 @@ void ScFormatShell::GetAttrState( SfxItemSet& rSet )
                     {
                         if(aCol != aInfoItem.GetHori()->GetColor() )
                             bColDisable = true;
-                        if(!( aLine == *(aInfoItem.GetHori())) )
+                        if( aLine != *aInfoItem.GetHori() )
                             bStyleDisable = true;
                     }
                 }
diff --git a/sfx2/source/dialog/mgetempl.cxx b/sfx2/source/dialog/mgetempl.cxx
index aff56ac4e28c..aabdb2de55cf 100644
--- a/sfx2/source/dialog/mgetempl.cxx
+++ b/sfx2/source/dialog/mgetempl.cxx
@@ -329,10 +329,7 @@ IMPL_LINK_NOARG(SfxManageStyleSheetPage, EditStyleSelectHdl_Impl, weld::ComboBox
 {
     OUString aTemplName(m_xFollowLb->get_active_text());
     OUString aEditTemplName(m_xName->get_text());
-    if (!( aTemplName == aEditTemplName))
-        m_xEditStyleBtn->set_sensitive(true);
-    else
-        m_xEditStyleBtn->set_sensitive(false);
+    m_xEditStyleBtn->set_sensitive(aTemplName != aEditTemplName);
 }
 
 IMPL_LINK_NOARG(SfxManageStyleSheetPage, EditStyleHdl_Impl, weld::Button&, void)
diff --git a/sfx2/source/doc/SfxDocumentMetaData.cxx b/sfx2/source/doc/SfxDocumentMetaData.cxx
index 549844e18663..fff048706fad 100644
--- a/sfx2/source/doc/SfxDocumentMetaData.cxx
+++ b/sfx2/source/doc/SfxDocumentMetaData.cxx
@@ -1515,7 +1515,7 @@ SfxDocumentMetaData::setTemplateDate(const css::util::DateTime & the_value)
 {
     ::osl::ClearableMutexGuard g(m_aMutex);
     checkInit();
-    if (!(m_TemplateDate == the_value)) {
+    if (m_TemplateDate != the_value) {
         m_TemplateDate = the_value;
         g.clear();
         setModified(true);
diff --git a/svx/source/svdraw/svdedxv.cxx b/svx/source/svdraw/svdedxv.cxx
index be6e6897576a..aa8eb6f687ed 100644
--- a/svx/source/svdraw/svdedxv.cxx
+++ b/svx/source/svdraw/svdedxv.cxx
@@ -2602,7 +2602,7 @@ static SfxItemSet CreatePaintSet( const sal_uInt16 *pRanges, SfxItemPool& rPool,
             const SfxPoolItem* pSourceItem = rSourceSet.GetItem( nWhich );
             const SfxPoolItem* pTargetItem = rTargetSet.GetItem( nWhich );
 
-            if( (pSourceItem && !pTargetItem) || (pSourceItem && pTargetItem && !((*pSourceItem) == (*pTargetItem)) ) )
+            if( (pSourceItem && !pTargetItem) || (pSourceItem && pTargetItem && *pSourceItem != *pTargetItem ) )
             {
                 aPaintSet.Put( *pSourceItem );
             }
diff --git a/sw/source/core/crsr/callnk.cxx b/sw/source/core/crsr/callnk.cxx
index 0a49e268b453..358d0895ed67 100644
--- a/sw/source/core/crsr/callnk.cxx
+++ b/sw/source/core/crsr/callnk.cxx
@@ -133,7 +133,7 @@ SwCallLink::~SwCallLink() COVERITY_NOEXCEPT_FALSE
         // That can be done in one go by the handler.
         rShell.CallChgLnk();
     }
-    else if( !bHasSelection != !(*pCurrentCursor->GetPoint() != *pCurrentCursor->GetMark()) )
+    else if( !bHasSelection != (*pCurrentCursor->GetPoint() == *pCurrentCursor->GetMark()) )
     {
         // always call change link when selection changes
         rShell.CallChgLnk();
diff --git a/sw/source/core/doc/gctable.cxx b/sw/source/core/doc/gctable.cxx
index e6daaaebf136..35fa00f86bd3 100644
--- a/sw/source/core/doc/gctable.cxx
+++ b/sw/source/core/doc/gctable.cxx
@@ -104,7 +104,7 @@ static sal_uInt16 lcl_FindEndPosOfBorder( const SwCollectTableLineBoxes& rCollTL
 
         if( SfxItemState::SET != rBox.GetFrameFormat()->GetItemState(RES_BOX,true, &pItem )
             || nullptr == ( pBrd = GetLineTB( static_cast<const SvxBoxItem*>(pItem), bTop ))
-            || !( *pBrd == rBrdLn ))
+            || *pBrd != rBrdLn )
             break;
         nLastPos = nPos;
     }
diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx
index 158d13864ea3..1cc1d4ba1237 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -563,7 +563,7 @@ bool SwNumRule::operator==( const SwNumRule& rRule ) const
     if( bRet )
     {
         for( sal_uInt8 n = 0; n < MAXLEVEL; ++n )
-            if( !( rRule.Get( n ) == Get( n ) ))
+            if( rRule.Get( n ) != Get( n ) )
             {
                 bRet = false;
                 break;
@@ -577,7 +577,7 @@ void SwNumRule::Set( sal_uInt16 i, const SwNumFormat& rNumFormat )
     OSL_ENSURE( i < MAXLEVEL, "Serious defect" );
     if( i < MAXLEVEL )
     {
-        if( !maFormats[ i ] || !(rNumFormat == Get( i )) )
+        if( !maFormats[ i ] || (rNumFormat != Get( i )) )
         {
             maFormats[ i ].reset(new SwNumFormat( rNumFormat ));
             mbInvalidRuleFlag = true;
diff --git a/sw/source/core/docnode/ndtbl1.cxx b/sw/source/core/docnode/ndtbl1.cxx
index e61840d84f22..daf52e4047a9 100644
--- a/sw/source/core/docnode/ndtbl1.cxx
+++ b/sw/source/core/docnode/ndtbl1.cxx
@@ -993,7 +993,7 @@ void SwDoc::GetTabBorders( const SwCursor& rCursor, SfxItemSet& rSet )
                             aSetBox.SetLine( rBox.GetTop(), SvxBoxItemLine::TOP );
                         }
                         else if ((aSetBox.GetTop() && rBox.GetTop() &&
-                                 !(*aSetBox.GetTop() == *rBox.GetTop())) ||
+                                 (*aSetBox.GetTop() != *rBox.GetTop())) ||
                                  ((!aSetBox.GetTop()) != (!rBox.GetTop()))) // != expression is true, if one and only one of the two pointers is !0
                         {
                             aSetBoxInfo.SetValid(SvxBoxInfoItemValidFlags::TOP, false );
@@ -1012,7 +1012,7 @@ void SwDoc::GetTabBorders( const SwCursor& rCursor, SfxItemSet& rSet )
                             aSetBox.SetLine( rBox.GetLeft(), SvxBoxItemLine::LEFT );
                         }
                         else if ((aSetBox.GetLeft() && rBox.GetLeft() &&
-                                 !(*aSetBox.GetLeft() == *rBox.GetLeft())) ||
+                                 (*aSetBox.GetLeft() != *rBox.GetLeft())) ||
                                  ((!aSetBox.GetLeft()) != (!rBox.GetLeft())))
                         {
                             aSetBoxInfo.SetValid(SvxBoxInfoItemValidFlags::LEFT, false );
@@ -1029,7 +1029,7 @@ void SwDoc::GetTabBorders( const SwCursor& rCursor, SfxItemSet& rSet )
                             aSetBoxInfo.SetLine( rBox.GetLeft(), SvxBoxInfoItemLine::VERT );
                         }
                         else if ((aSetBoxInfo.GetVert() && rBox.GetLeft() &&
-                                 !(*aSetBoxInfo.GetVert() == *rBox.GetLeft())) ||
+                                 (*aSetBoxInfo.GetVert() != *rBox.GetLeft())) ||
                                  ((!aSetBoxInfo.GetVert()) != (!rBox.GetLeft())))
                         {   aSetBoxInfo.SetValid( SvxBoxInfoItemValidFlags::VERT, false );
                             aSetBoxInfo.SetLine( nullptr, SvxBoxInfoItemLine::VERT );
@@ -1045,7 +1045,7 @@ void SwDoc::GetTabBorders( const SwCursor& rCursor, SfxItemSet& rSet )
                         aSetBox.SetLine( rBox.GetRight(), SvxBoxItemLine::RIGHT );
                     }
                     else if ((aSetBox.GetRight() && rBox.GetRight() &&
-                             !(*aSetBox.GetRight() == *rBox.GetRight())) ||
+                             (*aSetBox.GetRight() != *rBox.GetRight())) ||
                              (!aSetBox.GetRight() != !rBox.GetRight()))
                     {   aSetBoxInfo.SetValid( SvxBoxInfoItemValidFlags::RIGHT, false );
                         aSetBox.SetLine( nullptr, SvxBoxItemLine::RIGHT );
@@ -1062,7 +1062,7 @@ void SwDoc::GetTabBorders( const SwCursor& rCursor, SfxItemSet& rSet )
                             aSetBox.SetLine( rBox.GetBottom(), SvxBoxItemLine::BOTTOM );
                         }
                         else if ((aSetBox.GetBottom() && rBox.GetBottom() &&
-                                 !(*aSetBox.GetBottom() == *rBox.GetBottom())) ||
+                                 (*aSetBox.GetBottom() != *rBox.GetBottom())) ||
                                  (!aSetBox.GetBottom() != !rBox.GetBottom()))
                         {   aSetBoxInfo.SetValid( SvxBoxInfoItemValidFlags::BOTTOM, false );
                             aSetBox.SetLine( nullptr, SvxBoxItemLine::BOTTOM );
@@ -1080,7 +1080,7 @@ void SwDoc::GetTabBorders( const SwCursor& rCursor, SfxItemSet& rSet )
                             aSetBoxInfo.SetLine( rBox.GetBottom(), SvxBoxInfoItemLine::HORI );
                         }
                         else if ((aSetBoxInfo.GetHori() && rBox.GetBottom() &&
-                                 !(*aSetBoxInfo.GetHori() == *rBox.GetBottom())) ||
+                                 (*aSetBoxInfo.GetHori() != *rBox.GetBottom())) ||
                                  ((!aSetBoxInfo.GetHori()) != (!rBox.GetBottom())))
                         {
                             aSetBoxInfo.SetValid( SvxBoxInfoItemValidFlags::HORI, false );
diff --git a/sw/source/core/layout/paintfrm.cxx b/sw/source/core/layout/paintfrm.cxx
index 28244dab159f..6c5233a85c95 100644
--- a/sw/source/core/layout/paintfrm.cxx
+++ b/sw/source/core/layout/paintfrm.cxx
@@ -1989,7 +1989,7 @@ void DrawGraphic(
         // to get color of brush, check background color against COL_TRANSPARENT ("no fill"/"auto fill")
         // instead of checking, if transparency is not set.
         const Color aColor( pBrush &&
-                            ( !(pBrush->GetColor() == COL_TRANSPARENT) ||
+                            ( (pBrush->GetColor() != COL_TRANSPARENT) ||
                               gProp.bSFlyMetafile )
                     ? pBrush->GetColor()
                     : aGlobalRetoucheColor );
@@ -3920,7 +3920,7 @@ void SwFlyFrame::PaintSwFrame(vcl::RenderContext& rRenderContext, SwRect const&
                 //     to determine, if background has to be painted, by checking, if
                 //     background color is not COL_TRANSPARENT ("no fill"/"auto fill")
                 //     or a background graphic exists.
-                bPaintCompleteBack = !(aBack.GetColor() == COL_TRANSPARENT) ||
+                bPaintCompleteBack = (aBack.GetColor() != COL_TRANSPARENT) ||
                                      aBack.GetGraphicPos() != GPOS_NONE;
             }
         }
diff --git a/sw/source/filter/html/css1atr.cxx b/sw/source/filter/html/css1atr.cxx
index a2ba5a80c425..a6599a50705d 100644
--- a/sw/source/filter/html/css1atr.cxx
+++ b/sw/source/filter/html/css1atr.cxx
@@ -1220,9 +1220,9 @@ bool SwHTMLWriter::HasScriptDependentItems( const SfxItemSet& rItemSet,
             }
             else
             {
-                if( !( *pItem == *pItemCJK ) ||
-                    !( *pItem == *pItemCTL ) ||
-                    !( *pItemCJK == *pItemCTL ) )
+                if( *pItem != *pItemCJK ||
+                    *pItem != *pItemCTL ||
+                    *pItemCJK != *pItemCTL )
                     return true;
             }
         }
diff --git a/sw/source/filter/ww8/ww8par3.cxx b/sw/source/filter/ww8/ww8par3.cxx
index 863c8cf6b25b..0d4bed31eac3 100644
--- a/sw/source/filter/ww8/ww8par3.cxx
+++ b/sw/source/filter/ww8/ww8par3.cxx
@@ -1521,7 +1521,7 @@ static bool IsEqualFormatting(const SwNumRule &rOne, const SwNumRule &rTwo)
             //The SvxNumberFormat compare, not the SwNumFormat compare
             const SvxNumberFormat &rO = rOne.Get(n);
             const SvxNumberFormat &rT = rTwo.Get(n);
-            if (!(rO == rT))
+            if (rO != rT)
             {
                 bRet = false;
                 break;
diff --git a/sw/source/filter/xml/xmlimp.cxx b/sw/source/filter/xml/xmlimp.cxx
index 695c741f2fcf..abc22321a47d 100644
--- a/sw/source/filter/xml/xmlimp.cxx
+++ b/sw/source/filter/xml/xmlimp.cxx
@@ -1089,7 +1089,7 @@ void SwXMLImport::MergeListsAtDocumentInsertPosition(SwDoc *pDoc)
                 // check style of the each list level
                 for( sal_uInt8 n = 0; n < MAXLEVEL; ++n )
                 {
-                    if( !( pNumRule1->Get( n ) == pNumRule2->Get( n ) ))
+                    if( pNumRule1->Get( n ) != pNumRule2->Get( n ) )
                     {
                         return;
                     }
diff --git a/sw/source/uibase/uiview/view0.cxx b/sw/source/uibase/uiview/view0.cxx
index a51e3c5cbd1c..f902713c9d45 100644
--- a/sw/source/uibase/uiview/view0.cxx
+++ b/sw/source/uibase/uiview/view0.cxx
@@ -568,7 +568,7 @@ void SwView::ExecViewOptions(SfxRequest &rReq)
     SwWrtShell &rSh = GetWrtShell();
     rSh.StartAction();
     SwModule* pModule = SW_MOD();
-    if( !(*rSh.GetViewOptions() == *pOpt ))
+    if( *rSh.GetViewOptions() != *pOpt )
     {
         rSh.ApplyViewOptions( *pOpt );
         if( bBrowseModeChanged )
diff --git a/vcl/source/app/IconThemeSelector.cxx b/vcl/source/app/IconThemeSelector.cxx
index 57eee4d1344b..c4e60fd2d3b1 100644
--- a/vcl/source/app/IconThemeSelector.cxx
+++ b/vcl/source/app/IconThemeSelector.cxx
@@ -156,7 +156,7 @@ IconThemeSelector::operator==(const vcl::IconThemeSelector& other) const
 bool
 IconThemeSelector::operator!=(const vcl::IconThemeSelector& other) const
 {
-    return !((*this) == other);
+    return !(*this == other);
 }
 
 /*static*/ OUString
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 8a3b0d10cfdd..a8d861e271ac 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -2899,7 +2899,7 @@ void DomainMapper::lcl_startParagraphGroup()
     if (!mbIsSplitPara)
         m_pImpl->PushProperties(CONTEXT_PARAGRAPH);
     mbIsSplitPara = false;
-    if (!(m_pImpl->GetTopContextOfType(CONTEXT_PARAGRAPH) == m_pImpl->GetTopContext()))
+    if (m_pImpl->GetTopContextOfType(CONTEXT_PARAGRAPH) != m_pImpl->GetTopContext())
         m_pImpl->PushProperties(CONTEXT_PARAGRAPH);
 
     if (m_pImpl->GetTopContext())