loplugin:flatten in editeng..extensions

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

13 files changed, 319 insertions, 311 deletions

diff --git a/compilerplugins/clang/flatten.cxx b/compilerplugins/clang/flatten.cxx
index 4e031ef1f3a5..145311144fee 100644
--- a/compilerplugins/clang/flatten.cxx
+++ b/compilerplugins/clang/flatten.cxx
@@ -37,6 +37,7 @@ public:
 private:
     bool rewrite(const IfStmt * );
     SourceRange ignoreMacroExpansions(SourceRange range);
+    SourceRange extendOverComments(SourceRange range);
     std::string getSourceAsString(SourceRange range);
 };
 
@@ -101,8 +102,9 @@ bool Flatten::VisitIfStmt(const IfStmt* ifStmt)
 }
 
 static std::string stripOpenAndCloseBrace(std::string s);
-static std::string deindentThenStmt(std::string const & s);
-static std::vector<std::string> split(std::string const & s);
+static std::string deindent(std::string const & s);
+static std::vector<std::string> split(std::string s);
+static bool startswith(const std::string& rStr, const char* pSubStr);
 
 bool Flatten::rewrite(const IfStmt* ifStmt)
 {
@@ -121,12 +123,22 @@ bool Flatten::rewrite(const IfStmt* ifStmt)
     if (!elseRange.isValid()) {
         return false;
     }
-    auto elseKeywordRange = ifStmt->getElseLoc();
+    SourceRange elseKeywordRange = ifStmt->getElseLoc();
+
+    thenRange = extendOverComments(thenRange);
+    elseRange = extendOverComments(elseRange);
+    elseKeywordRange = extendOverComments(elseKeywordRange);
 
     // in adjusting the formatting I assume that "{" starts on a new line
 
     std::string conditionString = getSourceAsString(conditionRange);
-    conditionString = "!(" + conditionString + ")";
+    auto condExpr = ifStmt->getCond()->IgnoreImpCasts();
+    if (auto exprWithCleanups = dyn_cast<ExprWithCleanups>(condExpr))
+        condExpr = exprWithCleanups->getSubExpr()->IgnoreImpCasts();
+    if (isa<DeclRefExpr>(condExpr) || isa<CallExpr>(condExpr) || isa<MemberExpr>(condExpr))
+        conditionString = "!" + conditionString;
+    else
+        conditionString = "!(" + conditionString + ")";
 
     std::string thenString = getSourceAsString(thenRange);
     bool thenIsCompound = false;
@@ -136,23 +148,14 @@ bool Flatten::rewrite(const IfStmt* ifStmt)
             thenString = stripOpenAndCloseBrace(thenString);
         }
     }
-    thenString = deindentThenStmt(thenString);
+    thenString = deindent(thenString);
 
     std::string elseString = getSourceAsString(elseRange);
-    bool elseIsCompound = false;
-    if (auto compoundStmt = dyn_cast<CompoundStmt>(ifStmt->getElse())) {
-        if (compoundStmt->getLBracLoc().isValid()) {
-            elseIsCompound = true;
-        }
-    }
-    // indent else block if necessary
-    if (thenIsCompound && !elseIsCompound)
-        elseString = "    " + elseString;
 
     if (!replaceText(elseRange, thenString)) {
         return false;
     }
-    if (!replaceText(elseKeywordRange, "")) {
+    if (!removeText(elseKeywordRange, RewriteOptions(RemoveLineIfEmpty))) {
         return false;
     }
     if (!replaceText(thenRange, elseString)) {
@@ -167,38 +170,44 @@ bool Flatten::rewrite(const IfStmt* ifStmt)
 
 std::string stripOpenAndCloseBrace(std::string s)
 {
-    size_t openBrace = s.find_first_of("{");
-    if (openBrace != std::string::npos) {
-        size_t openLineEnd = s.find_first_of("\n", openBrace + 1);
-        if (openLineEnd != std::string::npos)
-            s = s.substr(openLineEnd + 1);
-        else
-            s = s.substr(openBrace + 1);
+    size_t i = s.find("{");
+    if (i != std::string::npos) {
+        ++i;
+        // strip to line end
+        while (s[i] == ' ')
+            ++i;
+        if (s[i] == '\n')
+            ++i;
+        s = s.substr(i);
     }
-    size_t closeBrace = s.find_last_of("}");
-    if (closeBrace != std::string::npos) {
-        size_t closeLineEnd = s.find_last_of("\n", closeBrace);
-        if (closeLineEnd != std::string::npos)
-            s = s.substr(0, closeLineEnd - 1);
-        else
-            s = s.substr(0, closeBrace - 1);
+    i = s.rfind("}");
+    if (i != std::string::npos) {
+        --i;
+        while (s[i] == ' ')
+            --i;
+        s = s.substr(0,i);
     }
     return s;
 }
 
-std::string deindentThenStmt(std::string const & s)
+std::string deindent(std::string const & s)
 {
     std::vector<std::string> lines = split(s);
     std::string rv;
     for (auto s : lines) {
-        rv += s.length() > 4 ? s.substr(4) : s;
+        if (startswith(s, "    "))
+            rv += s.substr(4);
+        else
+            rv += s;
         rv += "\n";
     }
     return rv;
 }
 
-std::vector<std::string> split(std::string const & s)
+std::vector<std::string> split(std::string s)
 {
+    if (s.back() == '\n')
+        s = s.substr(0, s.size()-1);
     size_t next = -1;
     std::vector<std::string> rv;
     do
@@ -211,6 +220,11 @@ std::vector<std::string> split(std::string const & s)
     return rv;
 }
 
+static bool startswith(const std::string& rStr, const char* pSubStr)
+{
+    return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
+}
+
 SourceRange Flatten::ignoreMacroExpansions(SourceRange range) {
     while (compiler.getSourceManager().isMacroArgExpansion(range.getBegin())) {
         range.setBegin(
@@ -244,6 +258,49 @@ SourceRange Flatten::ignoreMacroExpansions(SourceRange range) {
         ? SourceRange() : range;
 }
 
+/**
+ * Extend the SourceRange to include any leading and trailing whitespace, and any comments.
+ */
+SourceRange Flatten::extendOverComments(SourceRange range)
+{
+    SourceManager& SM = compiler.getSourceManager();
+    SourceLocation startLoc = range.getBegin();
+    SourceLocation endLoc = range.getEnd();
+    const char *p1 = SM.getCharacterData( startLoc );
+    const char *p2 = SM.getCharacterData( endLoc );
+
+    // scan backwards from the beginning to include any spaces on that line
+    while (*(p1-1) == ' ')
+        --p1;
+    startLoc = startLoc.getLocWithOffset(p1 - SM.getCharacterData( startLoc ));
+
+    p2 += Lexer::MeasureTokenLength( endLoc, SM, compiler.getLangOpts());
+    // look for trailing ";"
+    while (*p2 == ';')
+        ++p2;
+    // look for trailing " "
+    while (*p2 == ' ')
+        ++p2;
+    // look for single line comments attached to the end of the statement
+    if (*p2 == '/' && *(p2+1) == '/')
+    {
+        p2 += 2;
+        while (*p2 && *p2 != '\n')
+            ++p2;
+        if (*p2 == '\n')
+            ++p2;
+    }
+    else
+    {
+        // make the source code we extract include any trailing "\n"
+        if (*p2 == '\n')
+            ++p2;
+    }
+    endLoc = endLoc.getLocWithOffset(p2 - SM.getCharacterData( endLoc ));
+
+    return SourceRange(startLoc, endLoc);
+}
+
 std::string Flatten::getSourceAsString(SourceRange range)
 {
     SourceManager& SM = compiler.getSourceManager();
@@ -251,8 +308,8 @@ std::string Flatten::getSourceAsString(SourceRange range)
     SourceLocation endLoc = range.getEnd();
     const char *p1 = SM.getCharacterData( startLoc );
     const char *p2 = SM.getCharacterData( endLoc );
-    unsigned n = Lexer::MeasureTokenLength( endLoc, SM, compiler.getLangOpts());
-    return std::string( p1, p2 - p1 + n);
+    p2 += Lexer::MeasureTokenLength( endLoc, SM, compiler.getLangOpts());
+    return std::string( p1, p2 - p1);
 }
 
 loplugin::Plugin::Registration< Flatten > X("flatten", false);
diff --git a/editeng/source/accessibility/AccessibleEditableTextPara.cxx b/editeng/source/accessibility/AccessibleEditableTextPara.cxx
index c35a2dc05343..04eed5cd5996 100644
--- a/editeng/source/accessibility/AccessibleEditableTextPara.cxx
+++ b/editeng/source/accessibility/AccessibleEditableTextPara.cxx
@@ -434,13 +434,12 @@ namespace accessibility
 
     SvxEditSourceAdapter& AccessibleEditableTextPara::GetEditSource() const
     {
-        if( mpEditSource )
-            return *mpEditSource;
-        else
+        if( !mpEditSource )
             throw uno::RuntimeException("No edit source, object is defunct",
                                         uno::Reference< uno::XInterface >
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleEditableTextPara* > (this) ) ) ); // disambiguate hierarchy
+        return *mpEditSource;
     }
 
     SvxAccessibleTextAdapter& AccessibleEditableTextPara::GetTextForwarder() const
@@ -454,13 +453,12 @@ namespace accessibility
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleEditableTextPara* > (this) ) ) ); // disambiguate hierarchy
 
-        if( pTextForwarder->IsValid() )
-            return *pTextForwarder;
-        else
+        if( !pTextForwarder->IsValid() )
             throw uno::RuntimeException("Text forwarder is invalid, object is defunct",
                                         uno::Reference< uno::XInterface >
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleEditableTextPara* > (this) ) ) ); // disambiguate hierarchy
+        return *pTextForwarder;
     }
 
     SvxViewForwarder& AccessibleEditableTextPara::GetViewForwarder() const
@@ -476,13 +474,12 @@ namespace accessibility
                                           ( const_cast< AccessibleEditableTextPara* > (this) ) ) ); // disambiguate hierarchy
         }
 
-        if( pViewForwarder->IsValid() )
-            return *pViewForwarder;
-        else
+        if( !pViewForwarder->IsValid() )
             throw uno::RuntimeException("View forwarder is invalid, object is defunct",
                                         uno::Reference< uno::XInterface >
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleEditableTextPara* > (this) )  ) );    // disambiguate hierarchy
+        return *pViewForwarder;
     }
 
     SvxAccessibleTextEditViewAdapter& AccessibleEditableTextPara::GetEditViewForwarder( bool bCreate ) const
@@ -2644,10 +2641,9 @@ namespace accessibility
         if (bValidPara)
         {
             // we explicitly allow for the index to point at the character right behind the text
-            if (0 <= nIndex && nIndex <= rCacheTF.GetTextLen( nPara ))
-                nRes = rCacheTF.GetLineNumberAtIndex( nPara, nIndex );
-            else
+            if (0 > nIndex || nIndex > rCacheTF.GetTextLen( nPara ))
                 throw lang::IndexOutOfBoundsException();
+            nRes = rCacheTF.GetLineNumberAtIndex( nPara, nIndex );
         }
         return nRes;
     }
@@ -2663,27 +2659,24 @@ namespace accessibility
         DBG_ASSERT( bValidPara, "getTextAtLineNumber: current paragraph index out of range" );
         if (bValidPara)
         {
-            if (0 <= nLineNo && nLineNo < rCacheTF.GetLineCount( nPara ))
+            if (0 > nLineNo || nLineNo >= rCacheTF.GetLineCount( nPara ))
+                throw lang::IndexOutOfBoundsException();
+            sal_Int32 nStart = 0, nEnd = 0;
+            rCacheTF.GetLineBoundaries( nStart, nEnd, nPara, nLineNo );
+            if (nStart >= 0 && nEnd >=  0)
             {
-                sal_Int32 nStart = 0, nEnd = 0;
-                rCacheTF.GetLineBoundaries( nStart, nEnd, nPara, nLineNo );
-                if (nStart >= 0 && nEnd >=  0)
+                try
                 {
-                    try
-                    {
-                        aResult.SegmentText     = getTextRange( nStart, nEnd );
-                        aResult.SegmentStart    = nStart;
-                        aResult.SegmentEnd      = nEnd;
-                    }
-                    catch (const lang::IndexOutOfBoundsException&)
-                    {
-                        // this is not the exception that should be raised in this function ...
-                        DBG_UNHANDLED_EXCEPTION();
-                    }
+                    aResult.SegmentText     = getTextRange( nStart, nEnd );
+                    aResult.SegmentStart    = nStart;
+                    aResult.SegmentEnd      = nEnd;
+                }
+                catch (const lang::IndexOutOfBoundsException&)
+                {
+                    // this is not the exception that should be raised in this function ...
+                    DBG_UNHANDLED_EXCEPTION();
                 }
             }
-            else
-                throw lang::IndexOutOfBoundsException();
         }
         return aResult;
     }
diff --git a/editeng/source/accessibility/AccessibleImageBullet.cxx b/editeng/source/accessibility/AccessibleImageBullet.cxx
index fe88afb6f37d..58cc35859957 100644
--- a/editeng/source/accessibility/AccessibleImageBullet.cxx
+++ b/editeng/source/accessibility/AccessibleImageBullet.cxx
@@ -484,13 +484,12 @@ namespace accessibility
     SvxEditSource& AccessibleImageBullet::GetEditSource() const
     {
 
-        if( mpEditSource )
-            return *mpEditSource;
-        else
+        if( !mpEditSource )
             throw uno::RuntimeException("No edit source, object is defunct",
                                         uno::Reference< uno::XInterface >
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleImageBullet* > (this) ) ) );  // disambiguate hierarchy
+        return *mpEditSource;
     }
 
     SvxTextForwarder& AccessibleImageBullet::GetTextForwarder() const
@@ -505,13 +504,12 @@ namespace accessibility
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleImageBullet* > (this) ) ) );  // disambiguate hierarchy
 
-        if( pTextForwarder->IsValid() )
-            return *pTextForwarder;
-        else
+        if( !pTextForwarder->IsValid() )
             throw uno::RuntimeException("Text forwarder is invalid, object is defunct",
                                         uno::Reference< uno::XInterface >
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleImageBullet* > (this) ) ) );  // disambiguate hierarchy
+        return *pTextForwarder;
     }
 
     SvxViewForwarder& AccessibleImageBullet::GetViewForwarder() const
@@ -528,13 +526,12 @@ namespace accessibility
                                           ( const_cast< AccessibleImageBullet* > (this) ) ) );  // disambiguate hierarchy
         }
 
-        if( pViewForwarder->IsValid() )
-            return *pViewForwarder;
-        else
+        if( !pViewForwarder->IsValid() )
             throw uno::RuntimeException("View forwarder is invalid, object is defunct",
                                         uno::Reference< uno::XInterface >
                                         ( static_cast< ::cppu::OWeakObject* >
                                           ( const_cast< AccessibleImageBullet* > (this) )  ) ); // disambiguate hierarchy
+        return *pViewForwarder;
     }
 
 
diff --git a/editeng/source/uno/unotext.cxx b/editeng/source/uno/unotext.cxx
index 6b209a6f2121..ebc6ee10fba9 100644
--- a/editeng/source/uno/unotext.cxx
+++ b/editeng/source/uno/unotext.cxx
@@ -2009,63 +2009,60 @@ void SvxPropertyValuesToItemSet(
     for (sal_Int32 i = 0;  i < nProps;  ++i)
     {
         const SfxItemPropertySimpleEntry *pEntry = pPropSet->getPropertyMap().getByName( pProps[i].Name );
-        if (pEntry)
-        {
-            // Note: there is no need to take special care of the properties
-            //      TextField (EE_FEATURE_FIELD) and
-            //      TextPortionType (WID_PORTIONTYPE)
-            //  since they are read-only and thus are already taken care of below.
+        if (!pEntry)
+            throw beans::UnknownPropertyException( "Unknown property: " + pProps[i].Name, static_cast < cppu::OWeakObject * > ( nullptr ) );
+        // Note: there is no need to take special care of the properties
+        //      TextField (EE_FEATURE_FIELD) and
+        //      TextPortionType (WID_PORTIONTYPE)
+        //  since they are read-only and thus are already taken care of below.
 
-            if (pEntry->nFlags & beans::PropertyAttribute::READONLY)
-                // should be PropertyVetoException which is not yet defined for the new import API's functions
-                throw uno::RuntimeException("Property is read-only: " + pProps[i].Name, static_cast < cppu::OWeakObject * > ( nullptr ) );
-                //throw PropertyVetoException ("Property is read-only: " + pProps[i].Name, static_cast < cppu::OWeakObject * > ( 0 ) );
+        if (pEntry->nFlags & beans::PropertyAttribute::READONLY)
+            // should be PropertyVetoException which is not yet defined for the new import API's functions
+            throw uno::RuntimeException("Property is read-only: " + pProps[i].Name, static_cast < cppu::OWeakObject * > ( nullptr ) );
+            //throw PropertyVetoException ("Property is read-only: " + pProps[i].Name, static_cast < cppu::OWeakObject * > ( 0 ) );
 
-            if (pEntry->nWID == WID_FONTDESC)
-            {
-                awt::FontDescriptor aDesc;
-                if (pProps[i].Value >>= aDesc)
-                    SvxUnoFontDescriptor::FillItemSet( aDesc, rItemSet );
-            }
-            else if (pEntry->nWID == WID_NUMLEVEL)
+        if (pEntry->nWID == WID_FONTDESC)
+        {
+            awt::FontDescriptor aDesc;
+            if (pProps[i].Value >>= aDesc)
+                SvxUnoFontDescriptor::FillItemSet( aDesc, rItemSet );
+        }
+        else if (pEntry->nWID == WID_NUMLEVEL)
+        {
+            if (pForwarder)
             {
-                if (pForwarder)
-                {
-                    sal_Int16 nLevel = -1;
-                    pProps[i].Value >>= nLevel;
+                sal_Int16 nLevel = -1;
+                pProps[i].Value >>= nLevel;
 
-                    // #101004# Call interface method instead of unsafe cast
-                    if (!pForwarder->SetDepth( nPara, nLevel ))
-                        throw lang::IllegalArgumentException();
-                }
+                // #101004# Call interface method instead of unsafe cast
+                if (!pForwarder->SetDepth( nPara, nLevel ))
+                    throw lang::IllegalArgumentException();
             }
-            else if (pEntry->nWID == WID_NUMBERINGSTARTVALUE )
+        }
+        else if (pEntry->nWID == WID_NUMBERINGSTARTVALUE )
+        {
+            if( pForwarder )
             {
-                if( pForwarder )
-                {
-                    sal_Int16 nStartValue = -1;
-                    if( !(pProps[i].Value >>= nStartValue) )
-                        throw lang::IllegalArgumentException();
+                sal_Int16 nStartValue = -1;
+                if( !(pProps[i].Value >>= nStartValue) )
+                    throw lang::IllegalArgumentException();
 
-                    pForwarder->SetNumberingStartValue( nPara, nStartValue );
-                }
+                pForwarder->SetNumberingStartValue( nPara, nStartValue );
             }
-            else if (pEntry->nWID == WID_PARAISNUMBERINGRESTART )
+        }
+        else if (pEntry->nWID == WID_PARAISNUMBERINGRESTART )
+        {
+            if( pForwarder )
             {
-                if( pForwarder )
-                {
-                    bool bParaIsNumberingRestart = false;
-                    if( !(pProps[i].Value >>= bParaIsNumberingRestart) )
-                        throw lang::IllegalArgumentException();
+                bool bParaIsNumberingRestart = false;
+                if( !(pProps[i].Value >>= bParaIsNumberingRestart) )
+                    throw lang::IllegalArgumentException();
 
-                    pForwarder->SetParaIsNumberingRestart( nPara, bParaIsNumberingRestart );
-                }
+                pForwarder->SetParaIsNumberingRestart( nPara, bParaIsNumberingRestart );
             }
-            else
-                pPropSet->setPropertyValue( pProps[i].Name, pProps[i].Value, rItemSet );
         }
         else
-            throw beans::UnknownPropertyException( "Unknown property: " + pProps[i].Name, static_cast < cppu::OWeakObject * > ( nullptr ) );
+            pPropSet->setPropertyValue( pProps[i].Name, pProps[i].Value, rItemSet );
     }
 }
 
diff --git a/embeddedobj/source/commonembedding/embedobj.cxx b/embeddedobj/source/commonembedding/embedobj.cxx
index f85ce88c133e..d24533a63146 100644
--- a/embeddedobj/source/commonembedding/embedobj.cxx
+++ b/embeddedobj/source/commonembedding/embedobj.cxx
@@ -230,33 +230,30 @@ void OCommonEmbeddedObject::SwitchStateTo_Impl( sal_Int32 nNextState )
                     throw embed::WrongStateException( "client site not set, yet", *this );
 
                 uno::Reference< embed::XInplaceClient > xInplaceClient( m_xClientSite, uno::UNO_QUERY );
-                if ( xInplaceClient.is() && xInplaceClient->canInplaceActivate() )
-                {
-                    xInplaceClient->activatingInplace();
+                if ( !xInplaceClient.is() || !xInplaceClient->canInplaceActivate() )
+                    throw embed::WrongStateException(); //TODO: can't activate inplace
+                xInplaceClient->activatingInplace();
 
-                    uno::Reference< embed::XWindowSupplier > xClientWindowSupplier( xInplaceClient, uno::UNO_QUERY_THROW );
+                uno::Reference< embed::XWindowSupplier > xClientWindowSupplier( xInplaceClient, uno::UNO_QUERY_THROW );
 
-                    m_xClientWindow = xClientWindowSupplier->getWindow();
-                    m_aOwnRectangle = xInplaceClient->getPlacement();
-                    m_aClipRectangle = xInplaceClient->getClipRectangle();
-                    awt::Rectangle aRectangleToShow = GetRectangleInterception( m_aOwnRectangle, m_aClipRectangle );
+                m_xClientWindow = xClientWindowSupplier->getWindow();
+                m_aOwnRectangle = xInplaceClient->getPlacement();
+                m_aClipRectangle = xInplaceClient->getClipRectangle();
+                awt::Rectangle aRectangleToShow = GetRectangleInterception( m_aOwnRectangle, m_aClipRectangle );
 
-                    // create own window based on the client window
-                    // place and resize the window according to the rectangles
-                    uno::Reference< awt::XWindowPeer > xClientWindowPeer( m_xClientWindow, uno::UNO_QUERY_THROW );
+                // create own window based on the client window
+                // place and resize the window according to the rectangles
+                uno::Reference< awt::XWindowPeer > xClientWindowPeer( m_xClientWindow, uno::UNO_QUERY_THROW );
 
-                    // dispatch provider may not be provided
-                    uno::Reference< frame::XDispatchProvider > xContainerDP = xInplaceClient->getInplaceDispatchProvider();
-                    bool bOk = m_xDocHolder->ShowInplace( xClientWindowPeer, aRectangleToShow, xContainerDP );
-                    m_nObjectState = nNextState;
-                    if ( !bOk )
-                    {
-                        SwitchStateTo_Impl( embed::EmbedStates::RUNNING );
-                        throw embed::WrongStateException(); //TODO: can't activate inplace
-                    }
-                }
-                else
+                // dispatch provider may not be provided
+                uno::Reference< frame::XDispatchProvider > xContainerDP = xInplaceClient->getInplaceDispatchProvider();
+                bool bOk = m_xDocHolder->ShowInplace( xClientWindowPeer, aRectangleToShow, xContainerDP );
+                m_nObjectState = nNextState;
+                if ( !bOk )
+                {
+                    SwitchStateTo_Impl( embed::EmbedStates::RUNNING );
                     throw embed::WrongStateException(); //TODO: can't activate inplace
+                }
             }
             else if ( nNextState == embed::EmbedStates::ACTIVE )
             {
@@ -296,45 +293,42 @@ void OCommonEmbeddedObject::SwitchStateTo_Impl( sal_Int32 nNextState )
                 // TODO:
                 uno::Reference< css::frame::XLayoutManager > xContainerLM =
                             xInplaceClient->getLayoutManager();
-                if ( xContainerLM.is() )
+                if ( !xContainerLM.is() )
+                    throw embed::WrongStateException(); //TODO: can't activate UI
+                // dispatch provider may not be provided
+                uno::Reference< frame::XDispatchProvider > xContainerDP = xInplaceClient->getInplaceDispatchProvider();
+
+                // get the container module name
+                OUString aModuleName;
+                try
                 {
-                    // dispatch provider may not be provided
-                    uno::Reference< frame::XDispatchProvider > xContainerDP = xInplaceClient->getInplaceDispatchProvider();
-
-                    // get the container module name
-                    OUString aModuleName;
-                    try
-                    {
-                        uno::Reference< embed::XComponentSupplier > xCompSupl( m_xClientSite, uno::UNO_QUERY_THROW );
-                        uno::Reference< uno::XInterface > xContDoc( xCompSupl->getComponent(), uno::UNO_QUERY_THROW );
-
-                        uno::Reference< frame::XModuleManager2 > xManager( frame::ModuleManager::create( m_xContext ) );
-
-                        aModuleName = xManager->identify( xContDoc );
-                    }
-                    catch( const uno::Exception& )
-                    {}
-
-                    // if currently another object is UIactive it will be deactivated; usually this will activate the LM of
-                    // the container. Locking the LM will prevent flicker.
-                    xContainerLM->lock();
-                    xInplaceClient->activatingUI();
-                    bool bOk = m_xDocHolder->ShowUI( xContainerLM, xContainerDP, aModuleName );
-                    xContainerLM->unlock();
-
-                    if ( bOk )
-                    {
-                        m_nObjectState = nNextState;
-                        m_xDocHolder->ResizeHatchWindow();
-                    }
-                    else
-                    {
-                        xInplaceClient->deactivatedUI();
-                        throw embed::WrongStateException(); //TODO: can't activate UI
-                    }
+                    uno::Reference< embed::XComponentSupplier > xCompSupl( m_xClientSite, uno::UNO_QUERY_THROW );
+                    uno::Reference< uno::XInterface > xContDoc( xCompSupl->getComponent(), uno::UNO_QUERY_THROW );
+
+                    uno::Reference< frame::XModuleManager2 > xManager( frame::ModuleManager::create( m_xContext ) );
+
+                    aModuleName = xManager->identify( xContDoc );
+                }
+                catch( const uno::Exception& )
+                {}
+
+                // if currently another object is UIactive it will be deactivated; usually this will activate the LM of
+                // the container. Locking the LM will prevent flicker.
+                xContainerLM->lock();
+                xInplaceClient->activatingUI();
+                bool bOk = m_xDocHolder->ShowUI( xContainerLM, xContainerDP, aModuleName );
+                xContainerLM->unlock();
+
+                if ( bOk )
+                {
+                    m_nObjectState = nNextState;
+                    m_xDocHolder->ResizeHatchWindow();
                 }
                 else
+                {
+                    xInplaceClient->deactivatedUI();
                     throw embed::WrongStateException(); //TODO: can't activate UI
+                }
             }
         }
         else
@@ -368,14 +362,11 @@ void OCommonEmbeddedObject::SwitchStateTo_Impl( sal_Int32 nNextState )
             if ( xContainerLM.is() )
                 bOk = m_xDocHolder->HideUI( xContainerLM );
 
-            if ( bOk )
-            {
-                m_nObjectState = nNextState;
-                m_xDocHolder->ResizeHatchWindow();
-                xInplaceClient->deactivatedUI();
-            }
-            else
+            if ( !bOk )
                 throw embed::WrongStateException(); //TODO: can't activate UI
+            m_nObjectState = nNextState;
+            m_xDocHolder->ResizeHatchWindow();
+            xInplaceClient->deactivatedUI();
         }
     }
     else
diff --git a/embeddedobj/source/commonembedding/persistence.cxx b/embeddedobj/source/commonembedding/persistence.cxx
index ba5a3fde2c6e..2a5d09bfeb94 100644
--- a/embeddedobj/source/commonembedding/persistence.cxx
+++ b/embeddedobj/source/commonembedding/persistence.cxx
@@ -953,26 +953,23 @@ void SAL_CALL OCommonEmbeddedObject::setPersistentEntry(
 
     if ( m_bWaitSaveCompleted )
     {
-        if ( nEntryConnectionMode == embed::EntryInitModes::NO_INIT )
-        {
-            // saveCompleted is expected, handle it accordingly
-            if ( m_xNewParentStorage == xStorage && m_aNewEntryName == sEntName )
-            {
-                saveCompleted( true );
-                return;
-            }
-
-            // if a completely different entry is provided, switch first back to the old persistence in saveCompleted
-            // and then switch to the target persistence
-            bool bSwitchFurther = ( m_xParentStorage != xStorage || m_aEntryName != sEntName );
-            saveCompleted( false );
-            if ( !bSwitchFurther )
-                return;
-        }
-        else
+        if ( nEntryConnectionMode != embed::EntryInitModes::NO_INIT )
             throw embed::WrongStateException(
                         "The object waits for saveCompleted() call!",
                         static_cast< ::cppu::OWeakObject* >(this) );
+        // saveCompleted is expected, handle it accordingly
+        if ( m_xNewParentStorage == xStorage && m_aNewEntryName == sEntName )
+        {
+            saveCompleted( true );
+            return;
+        }
+
+        // if a completely different entry is provided, switch first back to the old persistence in saveCompleted
+        // and then switch to the target persistence
+        bool bSwitchFurther = ( m_xParentStorage != xStorage || m_aEntryName != sEntName );
+        saveCompleted( false );
+        if ( !bSwitchFurther )
+            return;
     }
 
     // for now support of this interface is required to allow breaking of links and converting them to normal embedded
diff --git a/embeddedobj/source/commonembedding/specialobject.cxx b/embeddedobj/source/commonembedding/specialobject.cxx
index e6f04fdf5b06..24100fa7465d 100644
--- a/embeddedobj/source/commonembedding/specialobject.cxx
+++ b/embeddedobj/source/commonembedding/specialobject.cxx
@@ -163,10 +163,9 @@ void SAL_CALL OSpecialEmbeddedObject::doVerb( sal_Int32 nVerbID )
     {
 
         uno::Reference < ui::dialogs::XExecutableDialog > xDlg( m_xDocHolder->GetComponent(), uno::UNO_QUERY );
-        if ( xDlg.is() )
-            xDlg->execute();
-        else
+        if ( !xDlg.is() )
             throw embed::UnreachableStateException();
+        xDlg->execute();
     }
     else
         OCommonEmbeddedObject::doVerb( nVerbID );
diff --git a/embeddedobj/source/commonembedding/visobj.cxx b/embeddedobj/source/commonembedding/visobj.cxx
index 7ed1bdcc029e..431f871fe492 100644
--- a/embeddedobj/source/commonembedding/visobj.cxx
+++ b/embeddedobj/source/commonembedding/visobj.cxx
@@ -193,13 +193,10 @@ embed::VisualRepresentation SAL_CALL OCommonEmbeddedObject::getPreferredVisualRe
                 "GDIMetaFile",
                 cppu::UnoType<uno::Sequence< sal_Int8 >>::get() );
 
-        if( xTransferable->isDataFlavorSupported( aDataFlavor ))
-        {
-            aVisualRepresentation.Data = xTransferable->getTransferData( aDataFlavor );
-            aVisualRepresentation.Flavor = aDataFlavor;
-        }
-        else
+        if( !xTransferable->isDataFlavorSupported( aDataFlavor ))
             throw uno::RuntimeException();
+        aVisualRepresentation.Data = xTransferable->getTransferData( aDataFlavor );
+        aVisualRepresentation.Flavor = aDataFlavor;
     }
 
     if ( bBackToLoaded )
diff --git a/embeddedobj/source/commonembedding/xfactory.cxx b/embeddedobj/source/commonembedding/xfactory.cxx
index a361190c580f..caf009f80a27 100644
--- a/embeddedobj/source/commonembedding/xfactory.cxx
+++ b/embeddedobj/source/commonembedding/xfactory.cxx
@@ -78,52 +78,49 @@ uno::Reference< uno::XInterface > SAL_CALL OOoEmbeddedObjectFactory::createInsta
         throw container::NoSuchElementException();
 
     uno::Reference< uno::XInterface > xResult;
-    if ( xStorage->isStorageElement( sEntName ) )
-    {
-        // the object must be based on storage
-        uno::Reference< embed::XStorage > xSubStorage =
-                xStorage->openStorageElement( sEntName, embed::ElementModes::READ );
-
-        uno::Reference< beans::XPropertySet > xPropSet( xSubStorage, uno::UNO_QUERY_THROW );
-
-        OUString aMediaType;
-        try {
-            uno::Any aAny = xPropSet->getPropertyValue("MediaType");
-            aAny >>= aMediaType;
-        }
-        catch ( const uno::Exception& )
-        {
-        }
-
-        try {
-            uno::Reference< lang::XComponent > xComp( xSubStorage, uno::UNO_QUERY );
-            if ( xComp.is() )
-                xComp->dispose();
-        }
-        catch ( const uno::Exception& )
-        {
-        }
-        xSubStorage.clear();
-
-        uno::Sequence< beans::NamedValue > aObject = m_aConfigHelper.GetObjectPropsByMediaType( aMediaType );
-
-        // If the sequence is empty, fall back to the FileFormatVersion=6200 filter, Base only has that.
-        if (!aObject.hasElements() && aMediaType == MIMETYPE_OASIS_OPENDOCUMENT_DATABASE_ASCII)
-            aObject = m_aConfigHelper.GetObjectPropsByMediaType(MIMETYPE_VND_SUN_XML_BASE_ASCII);
-
-        if ( !aObject.getLength() )
-            throw io::IOException(); // unexpected mimetype of the storage
-
-        xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
-                                                m_xContext,
-                                                aObject ) ),
-                    uno::UNO_QUERY );
-    }
-    else
+    if ( !xStorage->isStorageElement( sEntName ) )
     {
         // the object must be OOo embedded object, if it is not an exception must be thrown
         throw io::IOException(); // TODO:
     }
+    // the object must be based on storage
+    uno::Reference< embed::XStorage > xSubStorage =
+            xStorage->openStorageElement( sEntName, embed::ElementModes::READ );
+
+    uno::Reference< beans::XPropertySet > xPropSet( xSubStorage, uno::UNO_QUERY_THROW );
+
+    OUString aMediaType;
+    try {
+        uno::Any aAny = xPropSet->getPropertyValue("MediaType");
+        aAny >>= aMediaType;
+    }
+    catch ( const uno::Exception& )
+    {
+    }
+
+    try {
+        uno::Reference< lang::XComponent > xComp( xSubStorage, uno::UNO_QUERY );
+        if ( xComp.is() )
+            xComp->dispose();
+    }
+    catch ( const uno::Exception& )
+    {
+    }
+    xSubStorage.clear();
+
+    uno::Sequence< beans::NamedValue > aObject = m_aConfigHelper.GetObjectPropsByMediaType( aMediaType );
+
+    // If the sequence is empty, fall back to the FileFormatVersion=6200 filter, Base only has that.
+    if (!aObject.hasElements() && aMediaType == MIMETYPE_OASIS_OPENDOCUMENT_DATABASE_ASCII)
+        aObject = m_aConfigHelper.GetObjectPropsByMediaType(MIMETYPE_VND_SUN_XML_BASE_ASCII);
+
+    if ( !aObject.getLength() )
+        throw io::IOException(); // unexpected mimetype of the storage
+
+    xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
+                                            m_xContext,
+                                            aObject ) ),
+                uno::UNO_QUERY );
 
     uno::Reference< embed::XEmbedPersist > xPersist( xResult, uno::UNO_QUERY_THROW );
 
@@ -160,23 +157,20 @@ uno::Reference< uno::XInterface > SAL_CALL OOoEmbeddedObjectFactory::createInsta
     uno::Reference< uno::XInterface > xResult;
 
     // find document service name
-    if ( !aFilterName.isEmpty() )
-    {
-        uno::Sequence< beans::NamedValue > aObject = m_aConfigHelper.GetObjectPropsByFilter( aFilterName );
-        if ( !aObject.getLength() )
-            throw io::IOException(); // unexpected mimetype of the storage
-
-
-        xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
-                                            m_xContext,
-                                            aObject ) ),
-                    uno::UNO_QUERY );
-    }
-    else
+    if ( aFilterName.isEmpty() )
     {
         // the object must be OOo embedded object, if it is not an exception must be thrown
         throw io::IOException(); // TODO:
     }
+    uno::Sequence< beans::NamedValue > aObject = m_aConfigHelper.GetObjectPropsByFilter( aFilterName );
+    if ( !aObject.getLength() )
+        throw io::IOException(); // unexpected mimetype of the storage
+
+
+    xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
+                                        m_xContext,
+                                        aObject ) ),
+                uno::UNO_QUERY );
 
     uno::Reference< embed::XEmbedPersist > xPersist( xResult, uno::UNO_QUERY_THROW );
 
@@ -301,25 +295,22 @@ uno::Reference< uno::XInterface > SAL_CALL OOoEmbeddedObjectFactory::createInsta
 
     OUString aFilterName = m_aConfigHelper.UpdateMediaDescriptorWithFilterName( aTempMedDescr, false );
 
-    if ( !aFilterName.isEmpty() )
-    {
-        uno::Sequence< beans::NamedValue > aObject = m_aConfigHelper.GetObjectPropsByFilter( aFilterName );
-        if ( !aObject.getLength() )
-            throw io::IOException(); // unexpected mimetype of the storage
-
-
-        xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
-                                            m_xContext,
-                                            aObject,
-                                            aTempMedDescr,
-                                            lObjArgs ) ),
-                    uno::UNO_QUERY );
-    }
-    else
+    if ( aFilterName.isEmpty() )
     {
         // the object must be OOo embedded object, if it is not an exception must be thrown
         throw io::IOException(); // TODO:
     }
+    uno::Sequence< beans::NamedValue > aObject = m_aConfigHelper.GetObjectPropsByFilter( aFilterName );
+    if ( !aObject.getLength() )
+        throw io::IOException(); // unexpected mimetype of the storage
+
+
+    xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
+                                        m_xContext,
+                                        aObject,
+                                        aTempMedDescr,
+                                        lObjArgs ) ),
+                uno::UNO_QUERY );
 
     return xResult;
 }
@@ -363,22 +354,19 @@ uno::Reference< uno::XInterface > SAL_CALL OOoEmbeddedObjectFactory::createInsta
 
     OUString aFilterName = m_aConfigHelper.UpdateMediaDescriptorWithFilterName( aTempMedDescr, aObject );
 
-    if ( !aFilterName.isEmpty() )
-    {
-
-        xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
-                                            m_xContext,
-                                            aObject,
-                                            aTempMedDescr,
-                                            lObjArgs ) ),
-                    uno::UNO_QUERY );
-    }
-    else
+    if ( aFilterName.isEmpty() )
     {
         // the object must be OOo embedded object, if it is not an exception must be thrown
         throw io::IOException(); // TODO:
     }
 
+    xResult.set(static_cast< ::cppu::OWeakObject* > ( new OCommonEmbeddedObject(
+                                        m_xContext,
+                                        aObject,
+                                        aTempMedDescr,
+                                        lObjArgs ) ),
+                uno::UNO_QUERY );
+
     return xResult;
 }
 
diff --git a/embeddedobj/source/msole/olevisual.cxx b/embeddedobj/source/msole/olevisual.cxx
index ace3ee94aa0f..49e41275c93b 100644
--- a/embeddedobj/source/msole/olevisual.cxx
+++ b/embeddedobj/source/msole/olevisual.cxx
@@ -296,17 +296,14 @@ awt::Size SAL_CALL OleEmbeddedObject::getVisualAreaSize( sal_Int64 nAspect )
 #endif
     {
         // return cached value
-        if ( m_bHasCachedSize )
-        {
-            SAL_WARN_IF( nAspect != m_nCachedAspect, "embeddedobj.ole", "Unexpected aspect is requested!" );
-            aResult = m_aCachedSize;
-        }
-        else
+        if ( !m_bHasCachedSize )
         {
             throw embed::NoVisualAreaSizeException(
                             "No size available!",
                             static_cast< ::cppu::OWeakObject* >(this) );
         }
+        SAL_WARN_IF( nAspect != m_nCachedAspect, "embeddedobj.ole", "Unexpected aspect is requested!" );
+        aResult = m_aCachedSize;
     }
 
     return aResult;
diff --git a/eventattacher/source/eventattacher.cxx b/eventattacher/source/eventattacher.cxx
index 654cce006837..4862049ea14b 100644
--- a/eventattacher/source/eventattacher.cxx
+++ b/eventattacher/source/eventattacher.cxx
@@ -482,10 +482,9 @@ void FilterAllListenerImpl::convertToEventReturn( Any & rRet, const Type & rRetT
     else if( !rRet.getValueType().equals( rRetType ) )
     {
         Reference< XTypeConverter > xConverter = m_pEA->getConverter();
-        if( xConverter.is() )
-            rRet = xConverter->convertTo( rRet, rRetType );
-        else
+        if( !xConverter.is() )
             throw CannotConvertException(); // TODO TypeConversionException
+        rRet = xConverter->convertTo( rRet, rRetType );
     }
 }
 
diff --git a/extensions/source/bibliography/bibload.cxx b/extensions/source/bibliography/bibload.cxx
index eeb13f2fc7b7..22cf6b8275b0 100644
--- a/extensions/source/bibliography/bibload.cxx
+++ b/extensions/source/bibliography/bibload.cxx
@@ -621,20 +621,17 @@ Any BibliographyLoader::getPropertyValue(const OUString& rPropertyName)
         CUSTOM5_POS                , // BibliographyDataField_CUSTOM5
         ISBN_POS                    //BibliographyDataField_ISBN
     };
-    if(rPropertyName == "BibliographyDataFieldNames")
+    if(rPropertyName != "BibliographyDataFieldNames")
+        throw UnknownPropertyException();
+    Sequence<PropertyValue> aSeq(COLUMN_COUNT);
+    PropertyValue* pArray = aSeq.getArray();
+    BibConfig* pConfig = BibModul::GetConfig();
+    for(sal_uInt16 i = 0; i <= text::BibliographyDataField::ISBN ; i++)
     {
-        Sequence<PropertyValue> aSeq(COLUMN_COUNT);
-        PropertyValue* pArray = aSeq.getArray();
-        BibConfig* pConfig = BibModul::GetConfig();
-        for(sal_uInt16 i = 0; i <= text::BibliographyDataField::ISBN ; i++)
-        {
-            pArray[i].Name = pConfig->GetDefColumnName(aInternalMapping[i]);
-            pArray[i].Value <<= (sal_Int16) i;
-        }
-        aRet <<= aSeq;
+        pArray[i].Name = pConfig->GetDefColumnName(aInternalMapping[i]);
+        pArray[i].Value <<= (sal_Int16) i;
     }
-    else
-        throw UnknownPropertyException();
+    aRet <<= aSeq;
     return aRet;
 }
 
diff --git a/extensions/source/update/ui/updatecheckui.cxx b/extensions/source/update/ui/updatecheckui.cxx
index 2e334fc2c361..b7e512dd537c 100644
--- a/extensions/source/update/ui/updatecheckui.cxx
+++ b/extensions/source/update/ui/updatecheckui.cxx
@@ -390,10 +390,9 @@ void UpdateCheckUI::setPropertyValue(const OUString& rPropertyName,
     else if( rPropertyName == PROPERTY_CLICK_HDL ) {
         uno::Reference< task::XJob > aJob;
         rValue >>= aJob;
-        if ( aJob.is() )
-            mrJob = aJob;
-        else
+        if ( !aJob.is() )
             throw lang::IllegalArgumentException();
+        mrJob = aJob;
     }
     else if (rPropertyName == PROPERTY_SHOW_MENUICON ) {
         bool bShowMenuIcon = false;