More clang-tidy fixes

dantti · dantti · commit 1c7e6cae9528 · 2024-12-30T16:56:11.000-03:00
diff --git a/.clang-tidy b/.clang-tidy
@@ -0,0 +1,11 @@
+# This is a generated file do not edit but instead edit .clang-tidy.checks and .clang-tidy.settings
+
+Checks: '*,-abseil*,-altera-*,-android-*,-boost-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-hicpp-*,-llvmlibc-*,-linuxkernel-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,-bugprone-unchecked-optional-access,-bugprone-narrowing-conversions,-bugprone-easily-swappable-parameters,-bugprone-branch-clone,-bugprone-implicit-widening-of-multiplication-result,-bugprone-suspicious-include,-bugprone-integer-division,-bugprone-suspicious-enum-usage,-cert-err58-cpp,-cert-dcl21-cpp,-cert-dcl50-cpp,-clang-analyzer-alpha*,-clang-analyzer-osx.*,-clang-analyzer-webkit.*,-clang-analyzer-cplusplus.NewDeleteLeaks,-clang-analyzer-core.CallAndMessage,-clang-analyzer-alpha.core.CastToStruct,-clang-analyzer-alpha.deadcode.UnreachableCode,-clang-analyzer-core.NonNullParamChecker,-concurrency-*,-clang-diagnostic-pragma-once-outside-header,-google-*,google-explicit-constructor,google-readability-namespace-comments,-llvm-include-order,-llvm-namespace-comment,-llvm-qualified-auto,-llvm-else-after-return,-llvm-header-guard,-misc-no-recursion,-misc-non-private-member-variables-in-classes,-misc-const-correctness,-misc-include-cleaner,-modernize-use-override,-modernize-use-bool-literals,-modernize-pass-by-value,-modernize-raw-string-literal,-modernize-use-using,-modernize-loop-convert,-modernize-return-braced-init-list,-modernize-use-equals-default,-modernize-use-trailing-return-type,-modernize-use-nodiscard,-modernize-avoid-bind,-modernize-avoid-c-arrays,-modernize-type-traits,-performance-unnecessary-copy-initialization,-performance-no-automatic-move,-performance-no-int-to-ptr,-performance-enum-size,-readability-else-after-return,-readability-named-parameter,-readability-implicit-bool-conversion,-readability-namespace-comments,-readability-qualified-auto,-readability-magic-numbers,-readability-braces-around-statements,-readability-function-cognitive-complexity,-readability-const-return-type,-readability-inconsistent-declaration-parameter-name,-readability-simplify-boolean-expr,-readability-redundant-access-specifiers,-readability-identifier-length,-readability-misleading-indentation,-readability-make-member-function-const,-readability-suspicious-call-argument,-readability-avoid-nested-conditional-operator,-clang-analyzer-security.insecureAPI.*,-performance-unnecessary-value-param,-readability-convert-member-functions-to-static,-bugprone-misplaced-widening-cast,'
+
+CheckOptions:
+    - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
+      value: 1
+    - key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted
+      value: 1
+    - key: performance-move-const-arg.CheckTriviallyCopyableMove
+      value: 0
diff --git a/.clang-tidy.checks b/.clang-tidy.checks
@@ -0,0 +1,105 @@
+# I want all checks by  default. That way I'll have to black list new checks rather than whitelist them
+*
+
+-abseil* # Try
+-altera-*
+-android-*
+-boost-*
+-cppcoreguidelines-* # Try
+-darwin-*
+-fuchsia-*
+-hicpp-* # Tryu
+-llvmlibc-*
+-linuxkernel-*
+-mpi-*
+-objc-*
+-openmp-*
+-zircon-*
+
+-bugprone-unchecked-optional-access                   # https://github.com/llvm/llvm-project/issues/55530
+-bugprone-narrowing-conversions
+-bugprone-easily-swappable-parameters
+-bugprone-branch-clone
+-bugprone-implicit-widening-of-multiplication-result
+-bugprone-suspicious-include                          # This one doesn't like us importing moc_XXX.cpp files
+-bugprone-integer-division
+-bugprone-suspicious-enum-usage                       # in Qt5 this complains about Qt::CTRL | Qt::Key_a which is the right way to do it in Qt6
+
+-cert-err58-cpp
+-cert-dcl21-cpp
+
+# I only use this when I ignore the parameters altogether (See ContactsPage.cpp e.g)
+-cert-dcl50-cpp
+
+-clang-analyzer-alpha*
+-clang-analyzer-osx.*
+-clang-analyzer-webkit.*
+-clang-analyzer-cplusplus.NewDeleteLeaks
+-clang-analyzer-core.CallAndMessage
+-clang-analyzer-alpha.core.CastToStruct
+-clang-analyzer-alpha.deadcode.UnreachableCode
+-clang-analyzer-core.NonNullParamChecker
+
+-concurrency-*
+
+# This gives false possitive in all header files.
+-clang-diagnostic-pragma-once-outside-header
+
+-google-* # Try
+google-explicit-constructor
+google-readability-namespace-comments
+
+-llvm-include-order
+-llvm-namespace-comment
+-llvm-qualified-auto
+-llvm-else-after-return
+-llvm-header-guard                               # complains even though we have #pragma once
+
+-misc-no-recursion
+-misc-non-private-member-variables-in-classes    # I agree instance variables should be private, but often I do this by design - too noisy.
+-misc-const-correctness # Lots of hits, lets ignore it for now.
+-misc-include-cleaner   # Completely broken as of Oct 2023
+
+-modernize-use-override
+-modernize-use-bool-literals
+-modernize-pass-by-value
+-modernize-raw-string-literal
+-modernize-use-using
+-modernize-loop-convert
+-modernize-return-braced-init-list
+-modernize-use-equals-default
+-modernize-use-trailing-return-type
+-modernize-use-nodiscard
+-modernize-avoid-bind
+-modernize-avoid-c-arrays
+-modernize-type-traits # Lots of errors from QTestLib
+
+-performance-unnecessary-copy-initialization
+-performance-no-automatic-move                  # often doesn't do much of a difference in Qt apps, due to their ref counting.
+-performance-no-int-to-ptr
+-performance-enum-size                          # int is faster than int8_t
+
+-readability-else-after-return
+-readability-named-parameter
+-readability-implicit-bool-conversion           # Lots of warnings about if (myPointer) -> if (myPointer != nullptr)
+-readability-namespace-comments
+-readability-qualified-auto
+-readability-magic-numbers
+-readability-braces-around-statements
+-readability-function-cognitive-complexity
+-readability-const-return-type
+-readability-inconsistent-declaration-parameter-name
+-readability-simplify-boolean-expr
+-readability-redundant-access-specifiers
+-readability-identifier-length
+-readability-misleading-indentation
+-readability-make-member-function-const                   # complains about functions updating pointers too
+-readability-suspicious-call-argument
+-readability-avoid-nested-conditional-operator  # I simply do not agree that this is hard to read, esp with clang-format formatting it nicely
+
+-clang-analyzer-security.insecureAPI.*
+
+# Try all the rest in this file
+-performance-unnecessary-value-param
+-readability-convert-member-functions-to-static
+-bugprone-misplaced-widening-cast
diff --git a/.clang-tidy.settings b/.clang-tidy.settings
@@ -0,0 +1,7 @@
+CheckOptions:
+    - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
+      value: 1
+    - key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted
+      value: 1
+    - key: performance-move-const-arg.CheckTriviallyCopyableMove
+      value: 0
diff --git a/QXlsx/header/xlsxdocument.h b/QXlsx/header/xlsxdocument.h
@@ -130,7 +130,7 @@ class QXLSX_EXPORT Document : public QObject
     bool autosizeColumnWidth(const CellRange &range);
     bool autosizeColumnWidth(int column);
     bool autosizeColumnWidth(int colFirst, int colLast);
-    bool autosizeColumnWidth(void);
+    bool autosizeColumnWidth();
 
 private:
     QMap<int, int> getMaximalColumnWidth(int firstRow = 1, int lastRow = INT_MAX);
diff --git a/QXlsx/header/xlsxworksheet.h b/QXlsx/header/xlsxworksheet.h
@@ -186,7 +186,7 @@ class QXLSX_EXPORT Worksheet : public AbstractSheet
     void setWhiteSpaceVisible(bool visible);
     bool setStartPage(int spagen); // add by liufeijin20181028
 
-    QVector<CellLocation> getFullCells(int *maxRow, int *maxCol);
+    QVector<CellLocation> getFullCells(int *maxRow, int *maxCol) const;
 
 private:
     void saveToXmlFile(QIODevice *device) const override;
diff --git a/QXlsx/source/xlsxcell.cpp b/QXlsx/source/xlsxcell.cpp
@@ -84,8 +84,7 @@ Cell::Cell(const Cell *const cell)
  */
 Cell::~Cell()
 {
-    if (nullptr != d_ptr)
-        delete d_ptr;
+    delete d_ptr;
 }
 
 /*!
diff --git a/QXlsx/source/xlsxcellformula.cpp b/QXlsx/source/xlsxcellformula.cpp
@@ -104,7 +104,10 @@ CellFormula::CellFormula(const CellFormula &other)
  */
 CellFormula &CellFormula::operator=(const CellFormula &other)
 {
-    d = other.d;
+    if (this != &other) // Self-assignment check [cert-oop54-cpp]
+    {
+        this->d = other.d;
+    }
     return *this;
 }
 
diff --git a/QXlsx/source/xlsxchart.cpp b/QXlsx/source/xlsxchart.cpp
@@ -1168,25 +1168,6 @@ void ChartPrivate::saveXmlSer(QXmlStreamWriter &writer, XlsxSeries *ser, int id)
         writer.writeEndElement();
     }
 
-#if 0
-    if (!ser->axDataSource_numRef.isEmpty())
-    {
-        if (chartType == Chart::CT_ScatterChart || chartType == Chart::CT_BubbleChart)
-        {
-            writer.writeStartElement(QStringLiteral("c:xVal"));
-        }
-        else
-        {
-            writer.writeStartElement(QStringLiteral("c:cat"));
-        }
-
-        writer.writeStartElement(QStringLiteral("c:numRef"));
-        writer.writeTextElement(QStringLiteral("c:f"), ser->axDataSource_numRef);
-        writer.writeEndElement();//c:numRef
-        writer.writeEndElement();//c:cat or c:xVal
-    }
-#endif
-
     if (!ser->numberDataSource_numRef.isEmpty()) {
         if (chartType == Chart::CT_ScatterChart || chartType == Chart::CT_BubbleChart)
             writer.writeStartElement(QStringLiteral("c:yVal"));
diff --git a/QXlsx/source/xlsxchartsheet.cpp b/QXlsx/source/xlsxchartsheet.cpp
@@ -42,8 +42,7 @@ Chartsheet::Chartsheet(const QString &name, int id, Workbook *workbook, CreateFl
     if (flag == Chartsheet::F_NewFromScratch) {
         d_func()->drawing = std::make_shared<Drawing>(this, flag);
 
-        DrawingAbsoluteAnchor *anchor =
-            new DrawingAbsoluteAnchor(drawing(), DrawingAnchor::Picture);
+        auto anchor = new DrawingAbsoluteAnchor(drawing(), DrawingAnchor::Picture);
 
         anchor->pos = QPoint(0, 0);
         anchor->ext = QSize(9293679, 6068786);
diff --git a/QXlsx/source/xlsxconditionalformatting.cpp b/QXlsx/source/xlsxconditionalformatting.cpp
@@ -147,7 +147,10 @@ ConditionalFormatting::ConditionalFormatting(const ConditionalFormatting &other)
  */
 ConditionalFormatting &ConditionalFormatting::operator=(const ConditionalFormatting &other)
 {
-    this->d = other.d;
+    if (this != &other) // Self-assignment check [cert-oop54-cpp]
+    {
+        this->d = other.d;
+    }
     return *this;
 }
 
@@ -786,9 +789,11 @@ bool ConditionalFormatting::saveToXml(QXmlStreamWriter &writer) const
                 writer.writeTextElement(QStringLiteral("formula"),
                                         it.value().toString().arg(startCell));
             }
-        } else if ((it = rule->attrs.constFind(XlsxCfRuleData::A_formula1)) !=
-                   rule->attrs.constEnd()) {
-            writer.writeTextElement(QStringLiteral("formula"), it.value().toString());
+        } else {
+            it = rule->attrs.constFind(XlsxCfRuleData::A_formula1);
+            if (it != rule->attrs.constEnd()) {
+                writer.writeTextElement(QStringLiteral("formula"), it.value().toString());
+            }
         }
 
         it = rule->attrs.constFind(XlsxCfRuleData::A_formula2);
diff --git a/QXlsx/source/xlsxdatavalidation.cpp b/QXlsx/source/xlsxdatavalidation.cpp
@@ -137,7 +137,10 @@ DataValidation::DataValidation(const DataValidation &other)
  */
 DataValidation &DataValidation::operator=(const DataValidation &other)
 {
-    this->d = other.d;
+    if (this != &other) // Self-assignment check [cert-oop54-cpp]
+    {
+        this->d = other.d;
+    }
     return *this;
 }
 
diff --git a/QXlsx/source/xlsxdocument.cpp b/QXlsx/source/xlsxdocument.cpp
@@ -72,8 +72,8 @@ std::string copyTag(const std::string &sFrom, const std::string &sTo, const std:
     const std::string tagEnd         = "</" + tag + ">";
 
     // search all occurrences of tag in 'sFrom'
-    std::string sFromData = "";
-    size_t startIndex     = 0;
+    std::string sFromData;
+    size_t startIndex = 0;
     while (true) {
         std::size_t startPos = sFrom.find(tagToFindStart, startIndex);
         if (startPos != std::string::npos) {
@@ -491,7 +491,7 @@ bool DocumentPrivate::saveCsv(QString mainCSVFileName) const
 
         currentSheet->workbook()->setActiveSheet(sheetIndexNumber);
 
-        Worksheet *wsheet = static_cast<Worksheet *>(currentSheet->workbook()->activeSheet());
+        auto wsheet = static_cast<Worksheet *>(currentSheet->workbook()->activeSheet());
         if (wsheet == nullptr) {
             continue;
         }
@@ -572,7 +572,7 @@ bool DocumentPrivate::copyStyle(const QString &from, const QString &to)
     ZipReader zipReader(from);
     QStringList filePaths = zipReader.filePaths();
 
-    std::shared_ptr<ZipReader> toReader = std::shared_ptr<ZipReader>(new ZipReader(to));
+    auto toReader = std::make_shared<ZipReader>(to);
 
     QStringList toFilePaths = toReader->filePaths();
 
@@ -1539,7 +1539,7 @@ bool Document::autosizeColumnWidth(int colFirst, int colLast)
   Auto sets width in characters for all columns.
   Returns true on success.
  */
-bool Document::autosizeColumnWidth(void)
+bool Document::autosizeColumnWidth()
 {
     bool erg = false;
 
diff --git a/QXlsx/source/xlsxnumformatparser.cpp b/QXlsx/source/xlsxnumformatparser.cpp
@@ -29,8 +29,12 @@ bool NumFormatParser::isDateTime(const QString &formatCode)
 
         // quoted plain text block: don't care, ignore
         case '"':
-            while (i < formatCode.length() - 1 && formatCode[++i] != QLatin1Char('"'))
-                ;
+            while (i < formatCode.length() - 1) {
+                ++i;
+                if (formatCode[i] == QLatin1Char('"')) {
+                    break;
+                }
+            }
             break;
 
         // escaped char: don't care, ignore
diff --git a/QXlsx/source/xlsxstyles.cpp b/QXlsx/source/xlsxstyles.cpp
@@ -148,24 +148,26 @@ void Styles::fixNumFmt(const Format &format)
 
     const auto &str = format.numberFormat();
     if (!str.isEmpty()) {
-        QHash<QString, std::shared_ptr<XlsxFormatNumberData>>::ConstIterator cIt;
         // Assign proper number format index
         const auto &it = m_builtinNumFmtsHash.constFind(str);
         if (it != m_builtinNumFmtsHash.constEnd()) {
             const_cast<Format *>(&format)->fixNumberFormat(it.value(), str);
-        } else if ((cIt = m_customNumFmtsHash.constFind(str)) != m_customNumFmtsHash.constEnd()) {
-            const_cast<Format *>(&format)->fixNumberFormat(cIt.value()->formatIndex, str);
         } else {
-            // Assign a new fmt Id.
-            const_cast<Format *>(&format)->fixNumberFormat(m_nextCustomNumFmtId, str);
-
-            auto fmt          = std::make_shared<XlsxFormatNumberData>();
-            fmt->formatIndex  = m_nextCustomNumFmtId;
-            fmt->formatString = str;
-            m_customNumFmtIdMap.insert(m_nextCustomNumFmtId, fmt);
-            m_customNumFmtsHash.insert(str, fmt);
-
-            m_nextCustomNumFmtId += 1;
+            auto cIt = m_customNumFmtsHash.constFind(str);
+            if (cIt != m_customNumFmtsHash.constEnd()) {
+                const_cast<Format *>(&format)->fixNumberFormat(cIt.value()->formatIndex, str);
+            } else {
+                // Assign a new fmt Id.
+                const_cast<Format *>(&format)->fixNumberFormat(m_nextCustomNumFmtId, str);
+
+                auto fmt          = std::make_shared<XlsxFormatNumberData>();
+                fmt->formatIndex  = m_nextCustomNumFmtId;
+                fmt->formatString = str;
+                m_customNumFmtIdMap.insert(m_nextCustomNumFmtId, fmt);
+                m_customNumFmtsHash.insert(str, fmt);
+
+                m_nextCustomNumFmtId += 1;
+            }
         }
     } else {
         const auto id = format.numberFormatIndex();
@@ -350,7 +352,7 @@ void Styles::saveToXmlFile(QIODevice *device) const
 
 void Styles::writeNumFmts(QXmlStreamWriter &writer) const
 {
-    if (m_customNumFmtIdMap.size() == 0)
+    if (m_customNumFmtIdMap.isEmpty())
         return;
 
     writer.writeStartElement(QStringLiteral("numFmts"));
@@ -433,7 +435,7 @@ void Styles::writeFont(QXmlStreamWriter &writer, const Format &format, bool isDx
     }
 
     if (format.hasProperty(FormatPrivate::P_Font_Color)) {
-        XlsxColor color = format.property(FormatPrivate::P_Font_Color).value<XlsxColor>();
+        auto color = format.property(FormatPrivate::P_Font_Color).value<XlsxColor>();
         color.saveToXml(writer);
     }
 
diff --git a/QXlsx/source/xlsxutility.cpp b/QXlsx/source/xlsxutility.cpp
@@ -90,7 +90,7 @@ QVariant datetimeFromNumber(double num, bool is1904)
         num = num - 1;
     }
 
-    qint64 msecs = static_cast<qint64>(num * 1000 * 60 * 60 * 24.0 + 0.5);
+    auto msecs = static_cast<qint64>(std::lround(num * 1000 * 60 * 60 * 24.0));
 
     if (is1904)
         msecs += msecs1904;
diff --git a/QXlsx/source/xlsxworksheet.cpp b/QXlsx/source/xlsxworksheet.cpp

Original file line number	Diff line number	Diff line change
`@@ -84,8 +84,7 @@ Cell::Cell(const Cell *const cell)`
`84`	`84`	`*/`
`85`	`85`	`Cell::~Cell()`
`86`	`86`	`{`
`87`		`- if (nullptr != d_ptr)`
`88`		`- delete d_ptr;`
	`87`	`+ delete d_ptr;`
`89`	`88`	`}`
`90`	`89`
`91`	`90`	`/*!`
Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,10 @@ CellFormula::CellFormula(const CellFormula &other)`
`104`	`104`	`*/`
`105`	`105`	`CellFormula &CellFormula::operator=(const CellFormula &other)`
`106`	`106`	`{`
`107`		`- d = other.d;`
	`107`	`+ if (this != &other) // Self-assignment check [cert-oop54-cpp]`
	`108`	`+ {`
	`109`	`+ this->d = other.d;`
	`110`	`+ }`
`108`	`111`	`return *this;`
`109`	`112`	`}`
`110`	`113`
Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,10 @@ ConditionalFormatting::ConditionalFormatting(const ConditionalFormatting &other)`
`147`	`147`	`*/`
`148`	`148`	`ConditionalFormatting &ConditionalFormatting::operator=(const ConditionalFormatting &other)`
`149`	`149`	`{`
`150`		`- this->d = other.d;`
	`150`	`+ if (this != &other) // Self-assignment check [cert-oop54-cpp]`
	`151`	`+ {`
	`152`	`+ this->d = other.d;`
	`153`	`+ }`
`151`	`154`	`return *this;`
`152`	`155`	`}`
`153`	`156`
`@@ -786,9 +789,11 @@ bool ConditionalFormatting::saveToXml(QXmlStreamWriter &writer) const`
`786`	`789`	`writer.writeTextElement(QStringLiteral("formula"),`
`787`	`790`	`it.value().toString().arg(startCell));`
`788`	`791`	`}`
`789`		`- } else if ((it = rule->attrs.constFind(XlsxCfRuleData::A_formula1)) !=`
`790`		`- rule->attrs.constEnd()) {`
`791`		`- writer.writeTextElement(QStringLiteral("formula"), it.value().toString());`
	`792`	`+ } else {`
	`793`	`+ it = rule->attrs.constFind(XlsxCfRuleData::A_formula1);`
	`794`	`+ if (it != rule->attrs.constEnd()) {`
	`795`	`+ writer.writeTextElement(QStringLiteral("formula"), it.value().toString());`
	`796`	`+ }`
`792`	`797`	`}`
`793`	`798`
`794`	`799`	`it = rule->attrs.constFind(XlsxCfRuleData::A_formula2);`