Skip to content

Commit

Permalink
Fix clazy, cppcheck warnings in expression builder widgets
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Aug 17, 2021
1 parent 2f7efed commit 06b3b16
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 53 deletions.
8 changes: 4 additions & 4 deletions python/gui/auto_generated/qgsexpressionbuilderwidget.sip.in
View file
Expand Up @@ -42,21 +42,21 @@ Create a new expression builder widget with an optional parent.
%End
~QgsExpressionBuilderWidget();

void init( const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), const Flags &flags = LoadAll );
void init( const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), QgsExpressionBuilderWidget::Flags flags = LoadAll );
%Docstring
Initialize without any layer

.. versionadded:: 3.14
%End

void initWithLayer( QgsVectorLayer *layer, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), const Flags &flags = LoadAll );
void initWithLayer( QgsVectorLayer *layer, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), QgsExpressionBuilderWidget::Flags flags = LoadAll );
%Docstring
Initialize with a layer

.. versionadded:: 3.14
%End

void initWithFields( const QgsFields &fields, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), const Flags &flags = LoadAll );
void initWithFields( const QgsFields &fields, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), QgsExpressionBuilderWidget::Flags flags = LoadAll );
%Docstring
Initialize with given fields without any layer

Expand Down Expand Up @@ -200,7 +200,7 @@ Loads the user expressions.
.. versionadded:: 3.12
%End

void saveToUserExpressions( const QString &label, const QString expression, const QString &helpText ) /Deprecated/;
void saveToUserExpressions( const QString &label, const QString &expression, const QString &helpText ) /Deprecated/;
%Docstring
Stores the user ``expression`` with given ``label`` and ``helpText``.

Expand Down
10 changes: 9 additions & 1 deletion python/gui/auto_generated/qgsexpressiontreeview.sip.in
View file
Expand Up @@ -89,10 +89,18 @@ level down
virtual bool filterAcceptsRow( int source_row, const QModelIndex &source_parent ) const;


void setFilterString( const QString &string );
%Docstring
Sets the search filter ``string``.

.. versionadded:: 3.22
%End

protected:

virtual bool lessThan( const QModelIndex &left, const QModelIndex &right ) const;


};

class QgsExpressionTreeView : QTreeView
Expand Down Expand Up @@ -213,7 +221,7 @@ Adds the current expression to the given ``collection``.
By default it is saved to the collection "generic".
%End

void saveToUserExpressions( const QString &label, const QString expression, const QString &helpText );
void saveToUserExpressions( const QString &label, const QString &expression, const QString &helpText );
%Docstring
Stores the user ``expression`` with given ``label`` and ``helpText``.
%End
Expand Down
19 changes: 10 additions & 9 deletions src/gui/qgsexpressionbuilderwidget.cpp
View file
Expand Up @@ -251,7 +251,7 @@ QgsExpressionBuilderWidget::~QgsExpressionBuilderWidget()
delete mExpressionTreeMenuProvider;
}

void QgsExpressionBuilderWidget::init( const QgsExpressionContext &context, const QString &recentCollection, const Flags &flags )
void QgsExpressionBuilderWidget::init( const QgsExpressionContext &context, const QString &recentCollection, QgsExpressionBuilderWidget::Flags flags )
{
setExpressionContext( context );

Expand All @@ -262,13 +262,13 @@ void QgsExpressionBuilderWidget::init( const QgsExpressionContext &context, cons
mExpressionTreeView->loadUserExpressions();
}

void QgsExpressionBuilderWidget::initWithLayer( QgsVectorLayer *layer, const QgsExpressionContext &context, const QString &recentCollection, const Flags &flags )
void QgsExpressionBuilderWidget::initWithLayer( QgsVectorLayer *layer, const QgsExpressionContext &context, const QString &recentCollection, QgsExpressionBuilderWidget::Flags flags )
{
init( context, recentCollection, flags );
setLayer( layer );
}

void QgsExpressionBuilderWidget::initWithFields( const QgsFields &fields, const QgsExpressionContext &context, const QString &recentCollection, const Flags &flags )
void QgsExpressionBuilderWidget::initWithFields( const QgsFields &fields, const QgsExpressionContext &context, const QString &recentCollection, QgsExpressionBuilderWidget::Flags flags )
{
init( context, recentCollection, flags );
mExpressionTreeView->loadFieldNames( fields );
Expand Down Expand Up @@ -402,10 +402,10 @@ void QgsExpressionBuilderWidget::updateFunctionFileList( const QString &path )
if ( cmbFileNames->count() == 0 )
{
// Create default sample entry.
newFunctionFile( "default" );
newFunctionFile( QStringLiteral( "default" ) );
txtPython->setText( QStringLiteral( "'''\n#Sample custom function file\n"
"#(uncomment to use and customize or Add button to create a new file) \n%1 \n '''" ).arg( txtPython->text() ) );
saveFunctionFile( "default" );
saveFunctionFile( QStringLiteral( "default" ) );
}
}

Expand Down Expand Up @@ -598,7 +598,7 @@ void QgsExpressionBuilderWidget::loadUserExpressions( )
mExpressionTreeView->loadUserExpressions();
}

void QgsExpressionBuilderWidget::saveToUserExpressions( const QString &label, const QString expression, const QString &helpText )
void QgsExpressionBuilderWidget::saveToUserExpressions( const QString &label, const QString &expression, const QString &helpText )
{
mExpressionTreeView->saveToUserExpressions( label, expression, helpText );
}
Expand Down Expand Up @@ -685,7 +685,7 @@ void QgsExpressionBuilderWidget::showEvent( QShowEvent *e )
txtExpressionString->setFocus();
}

void QgsExpressionBuilderWidget::createErrorMarkers( QList<QgsExpression::ParserError> errors )
void QgsExpressionBuilderWidget::createErrorMarkers( const QList<QgsExpression::ParserError> &errors )
{
clearErrors();
for ( const QgsExpression::ParserError &error : errors )
Expand Down Expand Up @@ -769,7 +769,8 @@ void QgsExpressionBuilderWidget::createMarkers( const QgsExpressionNode *inNode
case QgsExpressionNode::NodeType::ntCondition:
{
const QgsExpressionNodeCondition *node = static_cast<const QgsExpressionNodeCondition *>( inNode );
for ( QgsExpressionNodeCondition::WhenThen *cond : node->conditions() )
const QList<QgsExpressionNodeCondition::WhenThen *> conditions = node->conditions();
for ( QgsExpressionNodeCondition::WhenThen *cond : conditions )
{
createMarkers( cond->whenExp() );
createMarkers( cond->thenExp() );
Expand Down Expand Up @@ -1123,7 +1124,7 @@ QMenu *QgsExpressionBuilderWidget::ExpressionTreeMenuProvider::createContextMenu

if ( formatterCanProvideAvailableValues( layer, item->data( QgsExpressionItem::ITEM_NAME_ROLE ).toString() ) )
{
menu->addAction( tr( "Load First 10 Unique Used Values" ), mExpressionBuilderWidget, SLOT( loadSampleUsedValues() ) );
menu->addAction( tr( "Load First 10 Unique Used Values" ), mExpressionBuilderWidget, &QgsExpressionBuilderWidget::loadSampleUsedValues );
menu->addAction( tr( "Load All Unique Used Values" ), mExpressionBuilderWidget, &QgsExpressionBuilderWidget::loadAllUsedValues );
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/gui/qgsexpressionbuilderwidget.h
View file
Expand Up @@ -69,19 +69,19 @@ class GUI_EXPORT QgsExpressionBuilderWidget : public QWidget, private Ui::QgsExp
* Initialize without any layer
* \since QGIS 3.14
*/
void init( const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), const Flags &flags = LoadAll );
void init( const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), QgsExpressionBuilderWidget::Flags flags = LoadAll );

/**
* Initialize with a layer
* \since QGIS 3.14
*/
void initWithLayer( QgsVectorLayer *layer, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), const Flags &flags = LoadAll );
void initWithLayer( QgsVectorLayer *layer, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), QgsExpressionBuilderWidget::Flags flags = LoadAll );

/**
* Initialize with given fields without any layer
* \since QGIS 3.14
*/
void initWithFields( const QgsFields &fields, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), const Flags &flags = LoadAll );
void initWithFields( const QgsFields &fields, const QgsExpressionContext &context = QgsExpressionContext(), const QString &recentCollection = QStringLiteral( "generic" ), QgsExpressionBuilderWidget::Flags flags = LoadAll );

/**
* Sets layer in order to get the fields and values
Expand Down Expand Up @@ -186,7 +186,7 @@ class GUI_EXPORT QgsExpressionBuilderWidget : public QWidget, private Ui::QgsExp
* \deprecated since QGIS 3.14 use expressionTree()->saveToUserExpressions() instead
* \since QGIS 3.12
*/
Q_DECL_DEPRECATED void saveToUserExpressions( const QString &label, const QString expression, const QString &helpText ) SIP_DEPRECATED;
Q_DECL_DEPRECATED void saveToUserExpressions( const QString &label, const QString &expression, const QString &helpText ) SIP_DEPRECATED;

/**
* Removes the expression \a label from the user stored expressions.
Expand Down Expand Up @@ -407,7 +407,7 @@ class GUI_EXPORT QgsExpressionBuilderWidget : public QWidget, private Ui::QgsExp

int FUNCTION_MARKER_ID = 25;

void createErrorMarkers( QList<QgsExpression::ParserError> errors );
void createErrorMarkers( const QList<QgsExpression::ParserError> &errors );
void createMarkers( const QgsExpressionNode *node );
void clearFunctionMarkers();
void clearErrors();
Expand Down
64 changes: 32 additions & 32 deletions src/gui/qgsexpressiontreeview.cpp
View file
Expand Up @@ -327,7 +327,7 @@ void QgsExpressionTreeView::registerItem( const QString &group,
const QString &label,
const QString &expressionText,
const QString &helpText,
QgsExpressionItem::ItemType type, bool highlightedItem, int sortOrder, QIcon icon, const QStringList &tags, const QString &name )
QgsExpressionItem::ItemType type, bool highlightedItem, int sortOrder, const QIcon &icon, const QStringList &tags, const QString &name )
{
QgsExpressionItem *item = new QgsExpressionItem( label, expressionText, helpText, type );
item->setData( label, Qt::UserRole );
Expand Down Expand Up @@ -505,7 +505,7 @@ void QgsExpressionTreeView::saveToRecent( const QString &expressionText, const Q
loadRecent( collection );
}

void QgsExpressionTreeView::saveToUserExpressions( const QString &label, const QString expression, const QString &helpText )
void QgsExpressionTreeView::saveToUserExpressions( const QString &label, const QString &expression, const QString &helpText )
{
QgsSettings settings;
const QString location = QStringLiteral( "user" );
Expand Down Expand Up @@ -542,7 +542,6 @@ void QgsExpressionTreeView::loadUserExpressions( )
QgsSettings settings;
const QString location = QStringLiteral( "user" );
settings.beginGroup( location, QgsSettings::Section::Expressions );
QString label;
QString helpText;
QString expression;
int i = 0;
Expand Down Expand Up @@ -598,7 +597,7 @@ QJsonDocument QgsExpressionTreeView::exportUserExpressions()
settings.endGroup();
}

exportObject["expressions"] = exportList;
exportObject[QStringLiteral( "expressions" )] = exportList;
QJsonDocument exportJson = QJsonDocument( exportObject );

return exportJson;
Expand All @@ -613,14 +612,14 @@ void QgsExpressionTreeView::loadExpressionsFromJson( const QJsonDocument &expres
QJsonObject expressionsObject = expressionsDocument.object();

// validate json for manadatory fields
if ( ! expressionsObject["qgis_version"].isString()
|| ! expressionsObject["exported_at"].isString()
|| ! expressionsObject["author"].isString()
|| ! expressionsObject["expressions"].isArray() )
if ( ! expressionsObject[QStringLiteral( "qgis_version" )].isString()
|| ! expressionsObject[QStringLiteral( "exported_at" )].isString()
|| ! expressionsObject[QStringLiteral( "author" )].isString()
|| ! expressionsObject[QStringLiteral( "expressions" )].isArray() )
return;

// validate versions
QVersionNumber qgisJsonVersion = QVersionNumber::fromString( expressionsObject["qgis_version"].toString() );
QVersionNumber qgisJsonVersion = QVersionNumber::fromString( expressionsObject[QStringLiteral( "qgis_version" )].toString() );
QVersionNumber qgisVersion = QVersionNumber::fromString( Qgis::version() );

// if the expressions are from newer version of QGIS, we ask the user to confirm
Expand Down Expand Up @@ -654,7 +653,8 @@ void QgsExpressionTreeView::loadExpressionsFromJson( const QJsonDocument &expres
settings.beginGroup( QStringLiteral( "user" ), QgsSettings::Section::Expressions );
mUserExpressionLabels = settings.childGroups();

for ( const QJsonValue && expressionValue : expressionsObject["expressions"].toArray() )
const QJsonArray expressions = expressionsObject[QStringLiteral( "expressions" )].toArray();
for ( const QJsonValue && expressionValue : expressions )
{
// validate the type of the array element, can be anything
if ( ! expressionValue.isObject() )
Expand All @@ -667,43 +667,43 @@ void QgsExpressionTreeView::loadExpressionsFromJson( const QJsonDocument &expres
QJsonObject expressionObj = expressionValue.toObject();

// make sure the required keys are the correct types
if ( ! expressionObj["name"].isString()
|| ! expressionObj["type"].isString()
|| ! expressionObj["expression"].isString()
|| ! expressionObj["group"].isString()
|| ! expressionObj["description"].isString() )
if ( ! expressionObj[QStringLiteral( "name" )].isString()
|| ! expressionObj[QStringLiteral( "type" )].isString()
|| ! expressionObj[QStringLiteral( "expression" )].isString()
|| ! expressionObj[QStringLiteral( "group" )].isString()
|| ! expressionObj[QStringLiteral( "description" )].isString() )
{
// try to stringify and put an indicator what happened. Try to stringify the name, if fails, go with the expression.
if ( ! expressionObj["name"].toString().isEmpty() )
skippedExpressionLabels.append( expressionObj["name"].toString() );
if ( ! expressionObj[QStringLiteral( "name" )].toString().isEmpty() )
skippedExpressionLabels.append( expressionObj[QStringLiteral( "name" )].toString() );
else
skippedExpressionLabels.append( expressionObj["expression"].toString() );
skippedExpressionLabels.append( expressionObj[QStringLiteral( "expression" )].toString() );

continue;
}

// we want to import only items of type expression for now
if ( expressionObj["type"].toString() != QLatin1String( "expression" ) )
if ( expressionObj[QStringLiteral( "type" )].toString() != QLatin1String( "expression" ) )
{
skippedExpressionLabels.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj[QStringLiteral( "name" )].toString() );
continue;
}

// we want to import only items of type expression for now
if ( expressionObj["group"].toString() != QLatin1String( "user" ) )
if ( expressionObj[QStringLiteral( "group" )].toString() != QLatin1String( "user" ) )
{
skippedExpressionLabels.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj[QStringLiteral( "name" )].toString() );
continue;
}

const QString label = expressionObj["name"].toString();
const QString expression = expressionObj["expression"].toString();
const QString helpText = expressionObj["description"].toString();
const QString label = expressionObj[QStringLiteral( "name" )].toString();
const QString expression = expressionObj[QStringLiteral( "expression" )].toString();
const QString helpText = expressionObj[QStringLiteral( "description" )].toString();

// make sure they have valid name
if ( label.contains( "\\" ) || label.contains( '/' ) )
if ( label.contains( QLatin1String( "\\" ) ) || label.contains( '/' ) )
{
skippedExpressionLabels.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj[QStringLiteral( "name" )].toString() );
continue;
}

Expand Down Expand Up @@ -736,24 +736,24 @@ void QgsExpressionTreeView::loadExpressionsFromJson( const QJsonDocument &expres
if ( ! skippedExpressionLabels.isEmpty() )
{
QStringList skippedExpressionLabelsQuoted;
skippedExpressionLabelsQuoted.reserve( skippedExpressionLabels.size() );
for ( const QString &skippedExpressionLabel : skippedExpressionLabels )
skippedExpressionLabelsQuoted.append( QStringLiteral( "'%1'" ).arg( skippedExpressionLabel ) );

QMessageBox::information( this,
tr( "Skipped Expression Imports" ),
QStringLiteral( "%1\n%2" ).arg( tr( "The following expressions have been skipped:" ),
skippedExpressionLabelsQuoted.join( ", " ) ) );
skippedExpressionLabelsQuoted.join( QStringLiteral( ", " ) ) ) );
}
}

const QList<QgsExpressionItem *> QgsExpressionTreeView::findExpressions( const QString &label )
{
QList<QgsExpressionItem *> result;
const QList<QStandardItem *> found { mModel->findItems( label, Qt::MatchFlag::MatchRecursive ) };
for ( const auto &item : std::as_const( found ) )
{
result.push_back( static_cast<QgsExpressionItem *>( item ) );
}
result.reserve( found.size() );
std::transform( found.begin(), found.end(), std::back_inserter( result ),
[]( QStandardItem * item ) -> QgsExpressionItem* { return static_cast<QgsExpressionItem *>( item ); } );
return result;
}

Expand Down
4 changes: 2 additions & 2 deletions src/gui/qgsexpressiontreeview.h
View file
Expand Up @@ -248,7 +248,7 @@ class GUI_EXPORT QgsExpressionTreeView : public QTreeView
/**
* Stores the user \a expression with given \a label and \a helpText.
*/
void saveToUserExpressions( const QString &label, const QString expression, const QString &helpText );
void saveToUserExpressions( const QString &label, const QString &expression, const QString &helpText );

/**
* Removes the expression \a label from the user stored expressions.
Expand Down Expand Up @@ -322,7 +322,7 @@ class GUI_EXPORT QgsExpressionTreeView : public QTreeView
const QString &helpText = QString(),
QgsExpressionItem::ItemType type = QgsExpressionItem::ExpressionNode,
bool highlightedItem = false, int sortOrder = 1,
QIcon icon = QIcon(),
const QIcon &icon = QIcon(),
const QStringList &tags = QStringList(),
const QString &name = QString() );

Expand Down

0 comments on commit 06b3b16

Please sign in to comment.