CI: Enable a bunch of bugprone- clang-tidy warnings

.gitlab-ci.yml

@@ -31,4 +31,6 @@ build_clang_tidy:
   script:
     - srcdir=`pwd` && mkdir -p /tmp/okular_build && cd /tmp/okular_build && CC=clang CXX=clang++ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -G Ninja $srcdir && cat compile_commands.json | jq  '[.[] | select(.file | contains("'"$srcdir"'"))]' > compile_commands.aux.json && cat compile_commands.aux.json | jq  '[.[] | select(.file | contains("/synctex/")| not)]' > compile_commands.json
     - ninja
-    - "run-clang-tidy -header-filter='.*/okular/.*' -checks='-*,readability-inconsistent-declaration-parameter-name,readability-string-compare,modernize-redundant-void-arg,modernize-use-bool-literals,modernize-make-unique,modernize-make-shared,modernize-use-override,modernize-use-equals-delete,modernize-use-emplace' -config=\"{WarningsAsErrors: '*'}\""
+    # Fix the poppler header, remove when debian:unstable ships poppler 0.82 or later
+    - sed -i "N;N;N;N; s#class MediaRendition\;\nclass MovieAnnotation\;\nclass ScreenAnnotation;#class MediaRendition\;#g"  /usr/include/poppler/qt5/poppler-link.h
+    - "run-clang-tidy -header-filter='.*/okular/.*' -checks='-*,bugprone-*,readability-inconsistent-declaration-parameter-name,readability-string-compare,modernize-redundant-void-arg,modernize-use-bool-literals,modernize-make-unique,modernize-make-shared,modernize-use-override,modernize-use-equals-delete,modernize-use-emplace,-bugprone-macro-parentheses,-bugprone-narrowing-conversions,-bugprone-branch-clone,-bugprone-incorrect-roundings' -config=\"{WarningsAsErrors: '*'}\""

conf/editannottooldialog.cpp

@@ -42,7 +42,7 @@ EditAnnotToolDialog::EditAnnotToolDialog( QWidget *parent, const QDomElement &in
     setLayout(mainLayout);
     QPushButton *okButton = buttonBox->button(QDialogButtonBox::Ok);
     okButton->setDefault(true);
-    okButton->setShortcut(Qt::CTRL | Qt::Key_Return);
+    okButton->setShortcut(Qt::CTRL | Qt::Key_Return); // NOLINT(bugprone-suspicious-enum-usage)
     connect(buttonBox, &QDialogButtonBox::accepted, this, &EditAnnotToolDialog::accept);
     connect(buttonBox, &QDialogButtonBox::rejected, this, &EditAnnotToolDialog::reject);
     okButton->setDefault(true);

conf/editdrawingtooldialog.cpp

@@ -28,7 +28,7 @@ EditDrawingToolDialog::EditDrawingToolDialog( const QDomElement &initialState, Q
 
     QPushButton *okButton = buttonBox->button( QDialogButtonBox::Ok );
     okButton->setDefault( true );
-    okButton->setShortcut( Qt::CTRL | Qt::Key_Return );
+    okButton->setShortcut( Qt::CTRL | Qt::Key_Return ); // NOLINT(bugprone-suspicious-enum-usage)
     connect( buttonBox, &QDialogButtonBox::accepted, this, &EditDrawingToolDialog::accept );
     connect( buttonBox, &QDialogButtonBox::rejected, this, &EditDrawingToolDialog::reject );
     okButton->setDefault( true );

core/area.cpp

@@ -285,7 +285,9 @@ RegularAreaRect::~RegularAreaRect()
 
 RegularAreaRect& RegularAreaRect::operator=( const RegularAreaRect& rar )
 {
-    RegularArea< NormalizedRect, QRect >::operator=( rar );
+    if (this != &rar) {
+        RegularArea< NormalizedRect, QRect >::operator=( rar );
+    }
     return *this;
 }
 

core/chooseenginedialog.cpp

@@ -28,7 +28,7 @@ ChooseEngineDialog::ChooseEngineDialog( const QStringList &generators, const QMi
     setLayout(mainLayout);
     QPushButton *okButton = buttonBox->button(QDialogButtonBox::Ok);
     okButton->setDefault(true);
-    okButton->setShortcut(Qt::CTRL | Qt::Key_Return);
+    okButton->setShortcut(Qt::CTRL | Qt::Key_Return); // NOLINT(bugprone-suspicious-enum-usage)
     connect(buttonBox, &QDialogButtonBox::accepted, this, &ChooseEngineDialog::accept);
     connect(buttonBox, &QDialogButtonBox::rejected, this, &ChooseEngineDialog::reject);
     okButton->setDefault(true);

core/document.cpp

@@ -5695,8 +5695,10 @@ DocumentInfo::DocumentInfo(const DocumentInfo &info) : d(new DocumentInfoPrivate
 
 DocumentInfo& DocumentInfo::operator=(const DocumentInfo &info)
 {
-    d->values = info.d->values;
-    d->titles = info.d->titles;
+    if (this != &info) {
+        d->values = info.d->values;
+        d->titles = info.d->titles;
+    }
     return *this;
 }
 

core/textpage.cpp

@@ -118,14 +118,14 @@ static bool doesConsumeY(const NormalizedRect& first, const NormalizedRect& seco
  */
 class TinyTextEntity
 {
-    static const int MaxStaticChars = sizeof( QChar * ) / sizeof( QChar );
+    static const int MaxStaticChars = sizeof( void* ) / sizeof( QChar );
 
     public:
         TinyTextEntity( const QString &text, const NormalizedRect &rect )
             : area( rect )
         {
             Q_ASSERT_X( !text.isEmpty(), "TinyTextEntity", "empty string" );
-            Q_ASSERT_X( sizeof( d ) == sizeof( QChar * ), "TinyTextEntity",
+            Q_ASSERT_X( sizeof( d ) == sizeof( void * ), "TinyTextEntity",
                         "internal storage is wider than QChar*, fix it!" );
             length = text.length();
             switch ( length )

generators/chm/lib/ebook_chm.cpp

@@ -899,7 +899,7 @@ void EBook_CHM::fillTopicsUrlMap()
 	|| !RetrieveObject( &m_chmSTRINGS, (unsigned char*) strings.data(), 0, m_chmSTRINGS.length ) )
 		return;
 
-	for ( unsigned int i = 0; i < m_chmTOPICS.length; i += TOPICS_ENTRY_LEN )
+	for ( LONGUINT64 i = 0; i < m_chmTOPICS.length; i += TOPICS_ENTRY_LEN )
 	{
 		unsigned int off_title = get_int32_le( (unsigned int *)(topics.data() + i + 4) );
 		unsigned int off_url = get_int32_le( (unsigned int *)(topics.data() + i + 8) );

generators/dvi/TeXFont_PFB.cpp

@@ -251,10 +251,10 @@ glyph* TeXFont_PFB::getGlyph(quint16 ch, bool generateCharacterPixmap, const QCo
         quint16 gInv = 0xFF - color.green();
         quint16 bInv = 0xFF - color.blue();
 
-        for(quint16 y=0; y<slot->bitmap.rows; y++) {
+        for(unsigned int y=0; y<slot->bitmap.rows; y++) {
           quint8 *srcScanLine = slot->bitmap.buffer + y*slot->bitmap.pitch;
           unsigned int *destScanLine = (unsigned int *)imgi.scanLine(y);
-          for(quint16 col=0; col<slot->bitmap.width; col++) {
+          for(unsigned int col=0; col<slot->bitmap.width; col++) {
             quint16 data =  *srcScanLine;
             // The value stored in "data" now has the following meaning:
             // data = 0 -> white; data = 0xff -> use "color"

generators/dvi/TeXFont_PK.cpp

@@ -275,9 +275,9 @@ glyph* TeXFont_PK::getGlyph(quint16 ch, bool generateCharacterPixmap, const QCol
       // character outline only using the alpha channel. That ensures
       // good quality rendering for overlapping characters.
       im32.fill(qRgb(color.red(), color.green(), color.blue()));
-      for(quint16 y=0; y<shrunk_height; y++) {
+      for(int y=0; y<shrunk_height; y++) {
         quint8 *destScanLine = (quint8 *)im32.scanLine(y);
-        for(quint16 col=0; col<shrunk_width; col++)
+        for(int col=0; col<shrunk_width; col++)
           destScanLine[4*col+3] = xydata[shrunk_width*y + col];
       }
     } else {
@@ -295,9 +295,9 @@ glyph* TeXFont_PK::getGlyph(quint16 ch, bool generateCharacterPixmap, const QCol
       quint16 bInv = 0xFF - color.blue();
 
       quint8 *srcScanLine = xydata;
-      for(quint16 y=0; y<shrunk_height; y++) {
+      for(int y=0; y<shrunk_height; y++) {
         unsigned int *destScanLine = (unsigned int *)im32.scanLine(y);
-        for(quint16 col=0; col<shrunk_width; col++) {
+        for(int col=0; col<shrunk_width; col++) {
           quint16 data =  *srcScanLine;
           // The value stored in "data" now has the following meaning:
           // data = 0 -> white; data = 0xff -> use "color"

generators/dvi/dviRenderer.cpp

@@ -424,7 +424,7 @@ bool dviRenderer::isValidFile(const QString& filename) const
 
   unsigned char trailer[4] = { 0xdf,0xdf,0xdf,0xdf };
 
-  if ( f.read( (char *)test, 4 )<4 || strncmp( (char *)test, (char *) trailer, 4 ) )
+  if ( f.read( (char *)test, 4 )<4 || strncmp( (char *)test, (char *) trailer, 4 ) != 0 )
     return false;
   // We suppose now that the dvi file is complete and OK
   return true;

generators/fax/faxexpand.cpp

@@ -704,19 +704,19 @@ static const unsigned char zerotab[256] = {
     prezeros >>= 4;							\
     if (prezeros == 8) {						\
 	zeros += 8;							\
-	continue;							\
+	break;							\
     }									\
     if (zeros + prezeros < 11) {					\
 	empty = 0;							\
 	zeros = postzeros;						\
-	continue;							\
+	break;							\
     }									\
     zeros = postzeros;							\
     if (empty)								\
 	EOLcnt++;							\
     lines++;								\
     empty = 1;								\
-} while (0)
+} while (false)
 
 /* count fax lines */
 int

ui/pageview.cpp

@@ -3928,8 +3928,8 @@ QPoint PageView::viewportToContentArea( const Okular::DocumentViewport & vp ) co
         else
         {
             // TopLeft
-            c.rx() += qRound( normClamp( vp.rePos.normalizedX, 0.0 ) * (double)r.width() + viewport()->width() / 2 );
-            c.ry() += qRound( normClamp( vp.rePos.normalizedY, 0.0 ) * (double)r.height() + viewport()->height() / 2 );
+            c.rx() += qRound( normClamp( vp.rePos.normalizedX, 0.0 ) * (double)r.width() + viewport()->width() / 2.0 );
+            c.ry() += qRound( normClamp( vp.rePos.normalizedY, 0.0 ) * (double)r.height() + viewport()->height() / 2.0 );
         }
     }
     else
@@ -4061,7 +4061,7 @@ double PageView::zoomFactorFitMode( ZoomMode mode )
     const bool facingCentered = Okular::Settings::viewMode() == Okular::Settings::EnumViewMode::FacingFirstCentered || (Okular::Settings::viewMode() == Okular::Settings::EnumViewMode::Facing && pageCount == 1);
     const bool overrideCentering = facingCentered && pageCount < 3;
     const int nCols = overrideCentering ? 1 : viewColumns();
-    const double colWidth = viewport()->width() / nCols - kcolWidthMargin;
+    const double colWidth = viewport()->width() / static_cast<double>(nCols) - kcolWidthMargin;
     const double rowHeight = viewport()->height() - krowHeightMargin;
     const PageViewItem * currentItem = d->items[ qMax( 0, (int)d->document->currentPage()) ];
     // prevent segmentation fault when opening a new document;
@@ -4957,8 +4957,8 @@ void PageView::slotRequestVisiblePixmaps( int newValue )
         {
             const QRect & geometry = i->croppedGeometry();
             // compute distance between item center and viewport center (slightly moved left)
-            double distance = hypot( (geometry.left() + geometry.right()) / 2 - (viewportCenterX - 4),
-                                     (geometry.top() + geometry.bottom()) / 2 - viewportCenterY );
+            const double distance = hypot( (geometry.left() + geometry.right()) / 2.0 - (viewportCenterX - 4),
+                                           (geometry.top() + geometry.bottom()) / 2.0 - viewportCenterY );
             if ( distance >= minDistance && nearPageNumber != -1 )
                 continue;
             nearPageNumber = i->pageNumber();

ui/presentationwidget.cpp

@@ -1225,7 +1225,7 @@ void PresentationWidget::generateOverlay()
     qreal dpr = qApp->devicePixelRatio();
 
     // calculate overlay geometry and resize pixmap if needed
-    int side = m_width / 16;
+    double side = m_width / 16.0;
     m_overlayGeometry.setRect( m_width - side - 4, 4, side, side );
 
     // note: to get a sort of antialiasing, we render the pixmap double sized