compilerplugins/clang/collapseif.cxx | 129 +++++++++++++++++++++++++++
compilerplugins/clang/test/collapseif.cxx | 55 +++++++++++
sc/source/filter/xml/xmlexprt.cxx | 110 ++++++++++-------------
solenv/CompilerTest_compilerplugins_clang.mk | 1
4 files changed, 236 insertions(+), 59 deletions(-)
New commits:
commit 4b363760b9f196e139ee367d54252c4d6cbe25f3
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Mon Nov 5 14:05:28 2018 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Tue Nov 6 07:45:56 2018 +0100
don't leave empty slots in array in ScXMLExport::GetViewSettings
the code was creating 4 entries in the array, but potentially not
putting anything there
Change-Id: Ia44aea42b3939842f921d30996c5f10e595fd768
Reviewed-on: https://gerrit.libreoffice.org/62894
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/sc/source/filter/xml/xmlexprt.cxx
b/sc/source/filter/xml/xmlexprt.cxx
index 4532b06ee897..7c1649727880 100644
--- a/sc/source/filter/xml/xmlexprt.cxx
+++ b/sc/source/filter/xml/xmlexprt.cxx
@@ -5116,73 +5116,65 @@ void
ScXMLExport::GetChangeTrackViewSettings(uno::Sequence<beans::PropertyValue>
sal_Int32 nChangePos(rProps.getLength());
rProps.realloc(nChangePos + 1);
beans::PropertyValue* pProps(rProps.getArray());
- if (pProps)
- {
- uno::Sequence<beans::PropertyValue>
aChangeProps(SC_VIEWCHANGES_COUNT);
- beans::PropertyValue* pChangeProps(aChangeProps.getArray());
- if (pChangeProps)
- {
- pChangeProps[SC_SHOW_CHANGES].Name = "ShowChanges";
- pChangeProps[SC_SHOW_CHANGES].Value <<=
pViewSettings->ShowChanges();
- pChangeProps[SC_SHOW_ACCEPTED_CHANGES].Name =
"ShowAcceptedChanges";
- pChangeProps[SC_SHOW_ACCEPTED_CHANGES].Value <<=
pViewSettings->IsShowAccepted();
- pChangeProps[SC_SHOW_REJECTED_CHANGES].Name =
"ShowRejectedChanges";
- pChangeProps[SC_SHOW_REJECTED_CHANGES].Value <<=
pViewSettings->IsShowRejected();
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME].Name =
"ShowChangesByDatetime";
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME].Value <<=
pViewSettings->HasDate();
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_MODE].Name =
"ShowChangesByDatetimeMode";
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_MODE].Value <<=
static_cast<sal_Int16>(pViewSettings->GetTheDateMode());
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_FIRST_DATETIME].Name
= "ShowChangesByDatetimeFirstDatetime";
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_FIRST_DATETIME].Value
<<= pViewSettings->GetTheFirstDateTime().GetUNODateTime();
- pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_SECOND_DATETIME].Name
= "ShowChangesByDatetimeSecondDatetime";
-
pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_SECOND_DATETIME].Value <<=
pViewSettings->GetTheLastDateTime().GetUNODateTime();
- pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR].Name =
"ShowChangesByAuthor";
- pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR].Value <<=
pViewSettings->HasAuthor();
- pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR_NAME].Name =
"ShowChangesByAuthorName";
- pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR_NAME].Value <<=
pViewSettings->GetTheAuthorToShow();
- pChangeProps[SC_SHOW_CHANGES_BY_COMMENT].Name =
"ShowChangesByComment";
- pChangeProps[SC_SHOW_CHANGES_BY_COMMENT].Value <<=
pViewSettings->HasComment();
- pChangeProps[SC_SHOW_CHANGES_BY_COMMENT_TEXT].Name =
"ShowChangesByCommentText";
- pChangeProps[SC_SHOW_CHANGES_BY_COMMENT_TEXT].Value <<=
pViewSettings->GetTheComment();
- pChangeProps[SC_SHOW_CHANGES_BY_RANGES].Name =
"ShowChangesByRanges";
- pChangeProps[SC_SHOW_CHANGES_BY_RANGES].Value <<=
pViewSettings->HasRange();
- OUString sRangeList;
- ScRangeStringConverter::GetStringFromRangeList(sRangeList,
&(pViewSettings->GetTheRangeList()), GetDocument(), FormulaGrammar::CONV_OOO);
- pChangeProps[SC_SHOW_CHANGES_BY_RANGES_LIST].Name =
"ShowChangesByRangesList";
- pChangeProps[SC_SHOW_CHANGES_BY_RANGES_LIST].Value <<=
sRangeList;
-
- pProps[nChangePos].Name = "TrackedChangesViewSettings";
- pProps[nChangePos].Value <<= aChangeProps;
- }
- }
+
+ uno::Sequence<beans::PropertyValue> aChangeProps(SC_VIEWCHANGES_COUNT);
+ beans::PropertyValue* pChangeProps(aChangeProps.getArray());
+ pChangeProps[SC_SHOW_CHANGES].Name = "ShowChanges";
+ pChangeProps[SC_SHOW_CHANGES].Value <<= pViewSettings->ShowChanges();
+ pChangeProps[SC_SHOW_ACCEPTED_CHANGES].Name = "ShowAcceptedChanges";
+ pChangeProps[SC_SHOW_ACCEPTED_CHANGES].Value <<=
pViewSettings->IsShowAccepted();
+ pChangeProps[SC_SHOW_REJECTED_CHANGES].Name = "ShowRejectedChanges";
+ pChangeProps[SC_SHOW_REJECTED_CHANGES].Value <<=
pViewSettings->IsShowRejected();
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME].Name =
"ShowChangesByDatetime";
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME].Value <<=
pViewSettings->HasDate();
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_MODE].Name =
"ShowChangesByDatetimeMode";
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_MODE].Value <<=
static_cast<sal_Int16>(pViewSettings->GetTheDateMode());
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_FIRST_DATETIME].Name =
"ShowChangesByDatetimeFirstDatetime";
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_FIRST_DATETIME].Value <<=
pViewSettings->GetTheFirstDateTime().GetUNODateTime();
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_SECOND_DATETIME].Name =
"ShowChangesByDatetimeSecondDatetime";
+ pChangeProps[SC_SHOW_CHANGES_BY_DATETIME_SECOND_DATETIME].Value <<=
pViewSettings->GetTheLastDateTime().GetUNODateTime();
+ pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR].Name = "ShowChangesByAuthor";
+ pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR].Value <<=
pViewSettings->HasAuthor();
+ pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR_NAME].Name =
"ShowChangesByAuthorName";
+ pChangeProps[SC_SHOW_CHANGES_BY_AUTHOR_NAME].Value <<=
pViewSettings->GetTheAuthorToShow();
+ pChangeProps[SC_SHOW_CHANGES_BY_COMMENT].Name = "ShowChangesByComment";
+ pChangeProps[SC_SHOW_CHANGES_BY_COMMENT].Value <<=
pViewSettings->HasComment();
+ pChangeProps[SC_SHOW_CHANGES_BY_COMMENT_TEXT].Name =
"ShowChangesByCommentText";
+ pChangeProps[SC_SHOW_CHANGES_BY_COMMENT_TEXT].Value <<=
pViewSettings->GetTheComment();
+ pChangeProps[SC_SHOW_CHANGES_BY_RANGES].Name = "ShowChangesByRanges";
+ pChangeProps[SC_SHOW_CHANGES_BY_RANGES].Value <<=
pViewSettings->HasRange();
+ OUString sRangeList;
+ ScRangeStringConverter::GetStringFromRangeList(sRangeList,
&(pViewSettings->GetTheRangeList()), GetDocument(), FormulaGrammar::CONV_OOO);
+ pChangeProps[SC_SHOW_CHANGES_BY_RANGES_LIST].Name =
"ShowChangesByRangesList";
+ pChangeProps[SC_SHOW_CHANGES_BY_RANGES_LIST].Value <<= sRangeList;
+
+ pProps[nChangePos].Name = "TrackedChangesViewSettings";
+ pProps[nChangePos].Value <<= aChangeProps;
}
}
void ScXMLExport::GetViewSettings(uno::Sequence<beans::PropertyValue>& rProps)
{
- rProps.realloc(4);
- beans::PropertyValue* pProps(rProps.getArray());
- if(pProps)
+ if (GetModel().is())
{
- if (GetModel().is())
+ rProps.realloc(4);
+ beans::PropertyValue* pProps(rProps.getArray());
+ ScModelObj* pDocObj(ScModelObj::getImplementation( GetModel() ));
+ if (pDocObj)
{
- ScModelObj* pDocObj(ScModelObj::getImplementation( GetModel() ));
- if (pDocObj)
+ SfxObjectShell* pEmbeddedObj = pDocObj->GetEmbeddedObject();
+ if (pEmbeddedObj)
{
- SfxObjectShell* pEmbeddedObj = pDocObj->GetEmbeddedObject();
- if (pEmbeddedObj)
- {
- tools::Rectangle aRect(pEmbeddedObj->GetVisArea());
- sal_uInt16 i(0);
- pProps[i].Name = "VisibleAreaTop";
- pProps[i].Value <<= static_cast<sal_Int32>(aRect.getY());
- pProps[++i].Name = "VisibleAreaLeft";
- pProps[i].Value <<= static_cast<sal_Int32>(aRect.getX());
- pProps[++i].Name = "VisibleAreaWidth";
- pProps[i].Value <<=
static_cast<sal_Int32>(aRect.getWidth());
- pProps[++i].Name = "VisibleAreaHeight";
- pProps[i].Value <<=
static_cast<sal_Int32>(aRect.getHeight());
- }
+ tools::Rectangle aRect(pEmbeddedObj->GetVisArea());
+ sal_uInt16 i(0);
+ pProps[i].Name = "VisibleAreaTop";
+ pProps[i].Value <<= static_cast<sal_Int32>(aRect.getY());
+ pProps[++i].Name = "VisibleAreaLeft";
+ pProps[i].Value <<= static_cast<sal_Int32>(aRect.getX());
+ pProps[++i].Name = "VisibleAreaWidth";
+ pProps[i].Value <<= static_cast<sal_Int32>(aRect.getWidth());
+ pProps[++i].Name = "VisibleAreaHeight";
+ pProps[i].Value <<= static_cast<sal_Int32>(aRect.getHeight());
}
}
}
commit 347f3475805f8c16d36133e8c857c88e9cc6cc14
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Mon Nov 5 15:12:18 2018 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Tue Nov 6 07:45:44 2018 +0100
new loplugin collapseif
Look for nested if statements with relatively small conditions, where
they can be collapsed into one if statement.
Change-Id: I7d5d4e418d0ce928991a3308fc88969c00c0d0f2
Reviewed-on: https://gerrit.libreoffice.org/62898
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/compilerplugins/clang/collapseif.cxx
b/compilerplugins/clang/collapseif.cxx
new file mode 100644
index 000000000000..b3f192c2ce1d
--- /dev/null
+++ b/compilerplugins/clang/collapseif.cxx
@@ -0,0 +1,129 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <unordered_map>
+#include <unordered_set>
+
+#include "plugin.hxx"
+#include "check.hxx"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/StmtVisitor.h"
+
+/**
+ Look for nested if statements with relatively small conditions, where they
can be collapsed into
+ one if statement.
+*/
+namespace
+{
+class CollapseIf : public loplugin::FilteringPlugin<CollapseIf>
+{
+public:
+ explicit CollapseIf(loplugin::InstantiationData const& data)
+ : FilteringPlugin(data)
+ {
+ }
+
+ virtual void run() override {
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+ bool VisitIfStmt(IfStmt const*);
+
+private:
+ int getNoCharsInSourceCodeOfExpr(IfStmt const*);
+ bool containsComment(Stmt const* stmt);
+};
+
+bool CollapseIf::VisitIfStmt(IfStmt const* ifStmt)
+{
+ if (ignoreLocation(ifStmt))
+ return true;
+ if (ifStmt->getElse())
+ return true;
+
+ IfStmt const* secondIfStmt = nullptr;
+ if (auto compoundStmt = dyn_cast<CompoundStmt>(ifStmt->getThen()))
+ {
+ if (compoundStmt->size() != 1)
+ return true;
+ secondIfStmt = dyn_cast<IfStmt>(*compoundStmt->body_begin());
+ if (!secondIfStmt)
+ return true;
+ if (secondIfStmt->getElse())
+ return true;
+ }
+ else
+ {
+ secondIfStmt = dyn_cast<IfStmt>(ifStmt->getThen());
+ if (!secondIfStmt)
+ return true;
+ }
+
+ int noChars1 = getNoCharsInSourceCodeOfExpr(ifStmt);
+ int noChars2 = getNoCharsInSourceCodeOfExpr(secondIfStmt);
+ if (noChars1 + noChars2 > 40)
+ return true;
+
+ // Sometimes there is a comment between the first and second if, so
+ // merging them would make the comment more awkward to write.
+ if (containsComment(ifStmt))
+ return true;
+
+ report(DiagnosticsEngine::Warning, "nested if should be collapsed into one
statement %0 %1",
+ compat::getBeginLoc(ifStmt))
+ << noChars1 << noChars2 << ifStmt->getSourceRange();
+ return true;
+}
+
+int CollapseIf::getNoCharsInSourceCodeOfExpr(IfStmt const* ifStmt)
+{
+ // Measure the space between the "if" the beginning of the "then" block
because
+ // measuring the size of the condition expression is unreliable, because
clang
+ // does not report the location of the last token accurately.
+ SourceManager& SM = compiler.getSourceManager();
+ SourceLocation startLoc = compat::getBeginLoc(ifStmt);
+ SourceLocation endLoc = compat::getBeginLoc(ifStmt->getThen());
+ char const* p1 = SM.getCharacterData(startLoc);
+ char const* p2 = SM.getCharacterData(endLoc);
+
+ int count = 0;
+ for (auto p = p1; p < p2; ++p)
+ if (*p != ' ')
+ ++count;
+
+ return count;
+}
+
+bool CollapseIf::containsComment(Stmt const* stmt)
+{
+ SourceManager& SM = compiler.getSourceManager();
+ auto range = stmt->getSourceRange();
+ SourceLocation startLoc = range.getBegin();
+ SourceLocation endLoc = range.getEnd();
+ char const* p1 = SM.getCharacterData(startLoc);
+ char const* p2 = SM.getCharacterData(endLoc);
+ p2 += Lexer::MeasureTokenLength(endLoc, SM, compiler.getLangOpts());
+
+ // check for comments
+ constexpr char const comment1[] = "/*";
+ constexpr char const comment2[] = "//";
+ if (std::search(p1, p2, comment1, comment1 + strlen(comment1)) != p2)
+ return true;
+ if (std::search(p1, p2, comment2, comment2 + strlen(comment2)) != p2)
+ return true;
+
+ return false;
+}
+
+/** Off by default because some places are a judgement call if it should be
collapsed or not. */
+loplugin::Plugin::Registration<CollapseIf> X("collapseif", false);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/collapseif.cxx
b/compilerplugins/clang/test/collapseif.cxx
new file mode 100644
index 000000000000..6f8658473918
--- /dev/null
+++ b/compilerplugins/clang/test/collapseif.cxx
@@ -0,0 +1,55 @@
+/* -*- 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 <sal/config.h>
+
+int f1(int x)
+{
+ // expected-error@+1 {{nested if should be collapsed into one statement 9
9 [loplugin:collapseif]}}
+ if (x == 1)
+ if (x == 2)
+ return 1;
+
+ // expected-error@+1 {{nested if should be collapsed into one statement 9
9 [loplugin:collapseif]}}
+ if (x == 1)
+ {
+ if (x == 2)
+ return 1;
+ }
+
+ // no warning expected
+ if (x == 1)
+ {
+ // comment here prevents warning
+ if (x == 2)
+ return 1;
+ }
+
+ // no warning expected
+ if (x == 1)
+ {
+ if (x == 2)
+ return 1;
+ }
+ else
+ return 3;
+
+ // no warning expected
+ if (x == 1)
+ {
+ if (x == 2)
+ return 1;
+ else
+ return 3;
+ }
+
+ return 2;
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk
b/solenv/CompilerTest_compilerplugins_clang.mk
index 0c086df0f7a4..a4d46ced20a8 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -13,6 +13,7 @@ $(eval $(call
gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/badstatics \
compilerplugins/clang/test/blockblock \
compilerplugins/clang/test/casttovoid \
+ compilerplugins/clang/test/collapseif \
compilerplugins/clang/test/commaoperator \
$(if $(filter-out WNT,$(OS)),compilerplugins/clang/test/constfields) \
compilerplugins/clang/test/constparams \
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
