Add test to ban brace initialization of QVariant variables

This test checks that brace initializers are never used for QVariant variables. On some compilers the value will be converted to a list. and on others a list of lists. Always use = initialization to avoid this ambiguity! (cherry picked from commit f31ec3d)
qgis · Mar 1, 2021 · e491f40 · e491f40
1 parent 015f593
commit e491f40
Show file tree

Hide file tree

Showing 19 changed files with 49 additions and 28 deletions.
diff --git a/.github/workflows/code_layout.yml b/.github/workflows/code_layout.yml
@@ -86,6 +86,14 @@ jobs:
       - name: Def Window Title Test
         run: ./tests/code_layout/test_defwindowtitle.sh
 
+  qvariant_no_brace_init:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+      - name: No brace initialization of QVariant variables
+        run: ./tests/code_layout/test_qvariant_no_brace_init.sh
+
   doxygen_layout_check:
     runs-on: ubuntu-latest
     steps:

diff --git a/src/app/gps/qgsgpsinformationwidget.cpp b/src/app/gps/qgsgpsinformationwidget.cpp
@@ -1187,7 +1187,7 @@ void QgsGpsInformationWidget::mBtnCloseFeature_clicked()
   int idx { vlayer->fields().indexOf( mCboTimestampField->currentText() ) };
   if ( idx != -1 )
   {
-    QVariant ts { timestamp( vlayer, idx ) };
+    QVariant ts = timestamp( vlayer, idx );
     if ( ts.isValid() )
     {
       attrMap[ idx ] = ts;

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -2302,7 +2302,7 @@ void QgisApp::resolveVectorLayerDependencies( QgsVectorLayer *vl, QgsMapLayer::S
         {
           QString tableSchema;
           QString tableName;
-          const QVariantMap sourceParts { providerMetadata->decodeUri( dependency.source ) };
+          const QVariantMap sourceParts = providerMetadata->decodeUri( dependency.source );
 
           // This part should really be abstracted out to the connection classes or to the providers directly.
           // Different providers decode the uri differently, for example we don't get the table name out of OGR

diff --git a/src/core/providers/memory/qgsmemoryprovider.cpp b/src/core/providers/memory/qgsmemoryprovider.cpp
@@ -430,7 +430,7 @@ bool QgsMemoryProvider::addFeatures( QgsFeatureList &flist, Flags flags )
     QString errorMessage;
     for ( int i = 0; i < mFields.count(); ++i )
     {
-      QVariant attrValue { it->attribute( i ) };
+      QVariant attrValue = it->attribute( i );
       if ( ! attrValue.isNull() && ! mFields.at( i ).convertCompatible( attrValue, &errorMessage ) )
       {
         // Push first conversion error only
@@ -614,7 +614,7 @@ bool QgsMemoryProvider::changeAttributeValues( const QgsChangedAttributesMap &at
     // Break on errors
     for ( QgsAttributeMap::const_iterator it2 = attrs.constBegin(); it2 != attrs.constEnd(); ++it2 )
     {
-      QVariant attrValue { it2.value() };
+      QVariant attrValue = it2.value();
       // Check attribute conversion
       const bool conversionError { ! attrValue.isNull()
                                    && ! mFields.at( it2.key() ).convertCompatible( attrValue, &errorMessage ) };

diff --git a/src/core/qgssettings.cpp b/src/core/qgssettings.cpp
@@ -294,7 +294,7 @@ void QgsSettings::setValue( const QString &key, const QVariant &value, const Qgs
   // The valid check is required because different invalid QVariant types
   // like QVariant(QVariant::String) and QVariant(QVariant::Int))
   // may be considered different and we don't want to store the value in that case.
-  QVariant currentValue { QgsSettings::value( prefixedKey( key, section ) ) };
+  QVariant currentValue = QgsSettings::value( prefixedKey( key, section ) );
   if ( ( currentValue.isValid() || value.isValid() ) && ( currentValue != value ) )
   {
     mUserSettings->setValue( prefixedKey( key, section ), value );

diff --git a/src/gui/editorwidgets/qgstexteditwrapper.cpp b/src/gui/editorwidgets/qgstexteditwrapper.cpp
@@ -328,7 +328,7 @@ void QgsTextEditWrapper::setWidgetValue( const QVariant &val )
     v = v.remove( QLocale().groupSeparator() );
   }
 
-  const QVariant currentValue { value( ) };
+  const QVariant currentValue = value( );
   // Note: comparing QVariants leads to funny (and wrong) results:
   // QVariant(0.0) == QVariant(QVariant.Double) -> True
   const bool changed { val != currentValue || val.isNull() != currentValue.isNull() };

diff --git a/src/gui/processing/qgsprocessingaggregatewidgets.cpp b/src/gui/processing/qgsprocessingaggregatewidgets.cpp
@@ -546,7 +546,7 @@ void QgsAggregateMappingWidget::AggregateDelegate::setEditorData( QWidget *edito
   if ( ! editorWidget )
     return;
 
-  const QVariant value { index.model()->data( index, Qt::EditRole ) };
+  const QVariant value = index.model()->data( index, Qt::EditRole );
   editorWidget->setCurrentIndex( editorWidget->findData( value ) );
 }
 
@@ -556,7 +556,7 @@ void QgsAggregateMappingWidget::AggregateDelegate::setModelData( QWidget *editor
   if ( ! editorWidget )
     return;
 
-  const QVariant currentValue { editorWidget->currentData( ) };
+  const QVariant currentValue = editorWidget->currentData( );
   model->setData( index, currentValue, Qt::EditRole );
 }
 

diff --git a/src/gui/qgsfieldmappingwidget.cpp b/src/gui/qgsfieldmappingwidget.cpp
@@ -343,7 +343,7 @@ void QgsFieldMappingWidget::TypeDelegate::setEditorData( QWidget *editor, const
   if ( ! editorWidget )
     return;
 
-  const QVariant value { index.model()->data( index, Qt::EditRole ) };
+  const QVariant value = index.model()->data( index, Qt::EditRole );
   editorWidget->setCurrentIndex( editorWidget->findData( value ) );
 }
 
@@ -353,6 +353,6 @@ void QgsFieldMappingWidget::TypeDelegate::setModelData( QWidget *editor, QAbstra
   if ( ! editorWidget )
     return;
 
-  const QVariant currentValue { editorWidget->currentData( ) };
+  const QVariant currentValue = editorWidget->currentData( );
   model->setData( index, currentValue, Qt::EditRole );
 }
diff --git a/src/gui/qgslocaleawarenumericlineeditdelegate.cpp b/src/gui/qgslocaleawarenumericlineeditdelegate.cpp
@@ -42,7 +42,7 @@ void QgsLocaleAwareNumericLineEditDelegate::setEditorData( QWidget *editor, cons
   QLineEdit *lineEdit { qobject_cast<QLineEdit *>( editor ) };
   if ( lineEdit )
   {
-    const QVariant value { index.data( ) };
+    const QVariant value = index.data( );
     lineEdit->setText( displayText( value, QLocale() ) );
   }
   else

diff --git a/src/gui/qgsnewdatabasetablenamewidget.cpp b/src/gui/qgsnewdatabasetablenamewidget.cpp
@@ -205,7 +205,7 @@ void QgsNewDatabaseTableNameWidget::updateUri()
     QgsAbstractProviderConnection *conn { dataProviderMetadata->findConnection( mConnectionName ) };
     if ( conn )
     {
-      QVariantMap uriParts { dataProviderMetadata->decodeUri( conn->uri() ) };
+      QVariantMap uriParts = dataProviderMetadata->decodeUri( conn->uri() );
       uriParts[ QStringLiteral( "layerName" ) ] = mTableName;
       uriParts[ QStringLiteral( "schema" ) ] = mSchemaName;
       uriParts[ QStringLiteral( "table" ) ] = mTableName;

diff --git a/src/providers/mssql/qgsmssqlproviderconnection.cpp b/src/providers/mssql/qgsmssqlproviderconnection.cpp
@@ -368,8 +368,8 @@ QList<QgsMssqlProviderConnection::TableProperty> QgsMssqlProviderConnection::tab
     table.setSchema( row[0].toString() );
     table.setTableName( row[1].toString() );
     table.setGeometryColumn( row[2].toString() );
-    //const QVariant srid { row[3] };
-    //const QVariant type { row[4] }; // GEOMETRY|GEOGRAPHY
+    //const QVariant srid = row[3];
+    //const QVariant type = row[4]; // GEOMETRY|GEOGRAPHY
     if ( row[5].toBool() )
       table.setFlag( QgsMssqlProviderConnection::TableFlag::View );
 

diff --git a/src/providers/postgres/qgspostgresproviderconnection.cpp b/src/providers/postgres/qgspostgresproviderconnection.cpp
@@ -270,7 +270,7 @@ QList<QVariantList> QgsPostgresProviderConnection::executeSqlPrivate( const QStr
             break;
           }
           const Oid oid { res.PQftype( rowIdx ) };
-          QList<QVariantList> typeRes { executeSqlPrivate( QStringLiteral( "SELECT typname FROM pg_type WHERE oid = %1" ).arg( oid ), false, nullptr, pgconn ) };
+          QList<QVariantList> typeRes = executeSqlPrivate( QStringLiteral( "SELECT typname FROM pg_type WHERE oid = %1" ).arg( oid ), false, nullptr, pgconn );
           // Set the default to string
           QVariant::Type vType { QVariant::Type::String };
           if ( typeRes.size() > 0 && typeRes.first().size() > 0 )
@@ -335,7 +335,7 @@ QList<QVariantList> QgsPostgresProviderConnection::executeSqlPrivate( const QStr
           if ( resolveTypes )
           {
             const QVariant::Type vType { typeMap.value( colIdx, QVariant::Type::String ) };
-            QVariant val { res.PQgetvalue( rowIdx, colIdx ) };
+            QVariant val = res.PQgetvalue( rowIdx, colIdx );
             // Special case for bools: 'f' and 't'
             if ( vType == QVariant::Bool )
             {

diff --git a/src/providers/postgres/raster/qgspostgresrastershareddata.cpp b/src/providers/postgres/raster/qgspostgresrastershareddata.cpp
@@ -197,7 +197,7 @@ QgsPostgresRasterSharedData::Tile const *QgsPostgresRasterSharedData::setTileDat
   }
 
   Tile *const tile { mTiles[ cacheKey ][ tileId ].get() };
-  const QVariantMap parsedData { QgsPostgresRasterUtils::parseWkb( data ) };
+  const QVariantMap parsedData = QgsPostgresRasterUtils::parseWkb( data );
   for ( int bandCnt = 1; bandCnt <= tile->numBands; ++bandCnt )
   {
     tile->data.emplace_back( parsedData[ QStringLiteral( "band%1" ).arg( bandCnt ) ].toByteArray() );
@@ -375,7 +375,7 @@ QgsPostgresRasterSharedData::TilesResponse QgsPostgresRasterSharedData::fetchTil
 
       int dataRead;
       GByte *binaryData { CPLHexToBinary( dataResult.PQgetvalue( row, 11 ).toLatin1().constData(), &dataRead ) };
-      const QVariantMap parsedData { QgsPostgresRasterUtils::parseWkb( QByteArray::fromRawData( reinterpret_cast<char *>( binaryData ), dataRead ) ) };
+      const QVariantMap parsedData = QgsPostgresRasterUtils::parseWkb( QByteArray::fromRawData( reinterpret_cast<char *>( binaryData ), dataRead ) );
       CPLFree( binaryData );
       for ( int bandCnt = 1; bandCnt <= tile->numBands; ++bandCnt )
       {

diff --git a/src/providers/spatialite/qgsspatialiteprovider.cpp b/src/providers/spatialite/qgsspatialiteprovider.cpp
@@ -1080,7 +1080,7 @@ QVariant QgsSpatiaLiteProvider::defaultValue( int fieldId ) const
        providerProperty( EvaluateDefaultValues, false ).toBool() )
   {
     QString errorMessage;
-    QVariant nextVal { QgsSqliteUtils::nextSequenceValue( sqliteHandle(), mTableName, errorMessage ) };
+    QVariant nextVal = QgsSqliteUtils::nextSequenceValue( sqliteHandle(), mTableName, errorMessage );
     if ( errorMessage.isEmpty() && nextVal != -1 )
     {
       resultVar = nextVal;

diff --git a/src/server/services/wfs3/qgswfs3handlers.cpp b/src/server/services/wfs3/qgswfs3handlers.cpp
@@ -1115,7 +1115,7 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
   const std::string title { mapLayer->title().isEmpty() ? mapLayer->name().toStdString() : mapLayer->title().toStdString() };
 
   // Get parameters
-  QVariantMap params { values( context )};
+  QVariantMap params = values( context );
 
   switch ( context.request()->method() )
   {
@@ -1463,7 +1463,7 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
           {
             authorizedFieldNames.push_back( f.name() );
           }
-          const QVariantMap properties { QgsJsonUtils::parseJson( postData["properties"].dump( ) ).toMap( ) };
+          const QVariantMap properties = QgsJsonUtils::parseJson( postData["properties"].dump( ) ).toMap( );
           const QgsFields fields = mapLayer->fields();
           for ( const auto &field : fields )
           {
@@ -1475,7 +1475,7 @@ void QgsWfs3CollectionsItemsHandler::handleRequest( const QgsServerApiContext &c
               }
               else
               {
-                QVariant value { properties.value( field.name() ) };
+                QVariant value = properties.value( field.name() );
                 // Convert blobs
                 if ( ! properties.value( field.name() ).isNull() && static_cast<QMetaType::Type>( field.type() ) == QMetaType::QByteArray )
                 {
@@ -1730,7 +1730,7 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
           {
             authorizedFieldNames.push_back( f.name() );
           }
-          const QVariantMap properties { QgsJsonUtils::parseJson( postData["properties"].dump( ) ).toMap( ) };
+          const QVariantMap properties = QgsJsonUtils::parseJson( postData["properties"].dump( ) ).toMap( );
           const QgsFields fields = mapLayer->fields();
           int fieldIndex = 0;
           for ( const auto &field : fields )
@@ -1743,7 +1743,7 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
               }
               else
               {
-                QVariant value { properties.value( field.name() ) };
+                QVariant value = properties.value( field.name() );
                 // Convert blobs
                 if ( ! properties.value( field.name() ).isNull() && static_cast<QMetaType::Type>( field.type() ) == QMetaType::QByteArray )
                 {
@@ -1850,7 +1850,7 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
           {
             authorizedFieldNames.push_back( f.name() );
           }
-          const QVariantMap properties { QgsJsonUtils::parseJson( postData["modify"].dump( ) ).toMap( ) };
+          const QVariantMap properties = QgsJsonUtils::parseJson( postData["modify"].dump( ) ).toMap( );
           const QgsFields fields = mapLayer->fields();
           int fieldIndex = 0;
           for ( const auto &field : fields )
@@ -1863,7 +1863,7 @@ void QgsWfs3CollectionsFeatureHandler::handleRequest( const QgsServerApiContext
               }
               else
               {
-                QVariant value { properties.value( field.name() ) };
+                QVariant value = properties.value( field.name() );
                 // Convert blobs
                 if ( ! properties.value( field.name() ).isNull() && static_cast<QMetaType::Type>( field.type() ) == QMetaType::QByteArray )
                 {

diff --git a/tests/code_layout/CMakeLists.txt b/tests/code_layout/CMakeLists.txt
@@ -6,6 +6,7 @@ add_test(qgis_banned_keywords ${CMAKE_SOURCE_DIR}/tests/code_layout/test_banned_
 add_test(qgis_licenses ${CMAKE_SOURCE_DIR}/tests/code_layout/test_licenses.sh)
 add_test(qgis_spelling ${CMAKE_SOURCE_DIR}/scripts/spell_check/spell_test.sh)
 add_test(qgis_defwindowtitle ${CMAKE_SOURCE_DIR}/tests/code_layout/test_defwindowtitle.sh)
+add_test(qgis_qvariant_no_brace_init ${CMAKE_SOURCE_DIR}/tests/code_layout/test_qvariant_no_brace_init.sh)
 
 add_test(qgis_shellcheck ${CMAKE_SOURCE_DIR}/tests/code_layout/test_shellcheck.sh)
 add_test(qgis_sipify ${CMAKE_SOURCE_DIR}/tests/code_layout/test_sipify.sh)

diff --git a/tests/code_layout/test_qvariant_no_brace_init.sh b/tests/code_layout/test_qvariant_no_brace_init.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+# This test checks that brace initializers are never used for QVariant variables. On some compilers the value will be
+# converted to a list, on others a list of lists.
+# Always use = initialization to avoid this ambiguity!
+
+if git grep -P 'QVariant(?!Map|List)[^\(\)&>:]+ {' &> /dev/null; then
+  echo ' *** Brace initializers should never be used for QVariant variables -- the value may become a list of lists on some compilers'
+  git grep -n -P 'QVariant(?!Map|List)[^\(\)&>:]+ {'
+  exit 1
+fi
+
diff --git a/tests/src/app/testqgsgpsinformationwidget.cpp b/tests/src/app/testqgsgpsinformationwidget.cpp
@@ -213,7 +213,7 @@ void TestQgsGpsInformationWidget::testTimestamp()
   QVERIFY( fieldIdx != -1 );
   // UTC
   widget->mCboTimestampFormat->setCurrentIndex( widget->mCboTimestampFormat->findData( Qt::TimeSpec::UTC ) );
-  QVariant dt { widget->timestamp( tempLayerDateTime, fieldIdx ) };
+  QVariant dt = widget->timestamp( tempLayerDateTime, fieldIdx );
   QCOMPARE( dt.toDateTime(), dateTime );
 
   // Local time

diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp
@@ -730,7 +730,7 @@ void TestQgsField::convertCompatible()
 
   // Test 0 on int fields
   intField = QgsField( QStringLiteral( "int" ), QVariant::Int, QStringLiteral( "Integer" ), 10 );
-  QVariant vZero { 0 };
+  QVariant vZero = 0;
   QVERIFY( intField.convertCompatible( vZero ) );
 }