Dev/expressionparser

src/datahandling/expressionparser.cpp

@@ -6,30 +6,33 @@
 
 const QString ExpressionParser::_cRegisterFunctionTemplate = "r(%1%2)";
 
-ExpressionParser::ExpressionParser(QStringList& expressions)
+ExpressionParser::ExpressionParser(const QStringList& expressions) : _findRegRegex(ExpressionRegex::cMatchRegister)
 {
-    _findRegRegex.setPattern(ExpressionRegex::cMatchRegister);
-    _findRegRegex.optimize();
-
     parseExpressions(expressions);
 }
 
-void ExpressionParser::dataPoints(QList<DataPoint>& dataPointList)
+/*!
+ * \brief Returns the deduplicated list of data points found during parsing.
+ */
+QList<DataPoint> ExpressionParser::dataPoints() const
 {
-    dataPointList = _dataPoints;
+    return _dataPoints;
 }
 
-void ExpressionParser::processedExpressions(QStringList& expressionList)
+/*!
+ * \brief Returns the list of processed expressions with register references replaced.
+ */
+QStringList ExpressionParser::processedExpressions() const
 {
-    expressionList = _processedExpressions;
+    return _processedExpressions;
 }
 
-void ExpressionParser::parseExpressions(QStringList& expressions)
+void ExpressionParser::parseExpressions(const QStringList& expressions)
 {
     _processedExpressions.clear();
     _dataPoints.clear();
 
-    for (const QString& expression : std::as_const(expressions))
+    for (const QString& expression : expressions)
     {
         _processedExpressions.append(processExpression(expression));
     }
@@ -42,8 +45,7 @@ QString ExpressionParser::processExpression(QString const& graphExpr)
 
     if (!i.hasNext() && resultExpr.contains("$"))
     {
-        auto msg = QString("Expression evaluation parsing failed (\"%1\")").arg(resultExpr);
-        qCWarning(scopeComm) << msg;
+        qCWarning(scopeComm) << QString("Expression evaluation parsing failed (\"%1\")").arg(resultExpr);
     }
 
     while (i.hasNext())
@@ -65,7 +67,7 @@ QString ExpressionParser::processExpression(QString const& graphExpr)
     return resultExpr;
 }
 
-bool ExpressionParser::processRegisterExpression(QString regExpr, DataPoint& dataPoint)
+bool ExpressionParser::processRegisterExpression(const QString& regExpr, DataPoint& dataPoint)
 {
     static const QRegularExpression regParseRegex(ExpressionRegex::cParseReg);
     QRegularExpressionMatch match = regParseRegex.match(regExpr);
@@ -96,20 +98,16 @@ bool ExpressionParser::processRegisterExpression(QString regExpr, DataPoint& dat
 
 QString ExpressionParser::constructInternalRegisterFunction(DataPoint const& dataPoint, int size)
 {
-    quint32 idx;
-    if (_dataPoints.contains(dataPoint))
-    {
-        idx = _dataPoints.indexOf(dataPoint);
-    }
-    else
+    qsizetype idx = _dataPoints.indexOf(dataPoint);
+    if (idx < 0)
     {
         _dataPoints.append(dataPoint);
         idx = _dataPoints.size() - 1;
     }
 
     /* Add dummy whitespaces to make sure positions in internal representations match visible expressions */
-    QString regIdx = QString("%1").arg(idx);
-    const int spacesCount = size - 3 - regIdx.size(); /* ignore ${} and idx string length */
+    QString regIdx = QString::number(idx);
+    const int spacesCount = qMax(0, size - 3 - regIdx.size()); /* ignore ${} and idx string length */
     QString spaces = QString(" ").repeated(spacesCount);
 
     return QString(_cRegisterFunctionTemplate).arg(idx).arg(spaces);

src/datahandling/expressionparser.h

@@ -5,26 +5,25 @@
 #include <QRegularExpression>
 #include <QStringList>
 
-class ExpressionParser : public QObject
+class ExpressionParser
 {
-    Q_OBJECT
 public:
-    explicit ExpressionParser(QStringList& expressions);
+    explicit ExpressionParser(const QStringList& expressions);
 
-    void dataPoints(QList<DataPoint>& dataPointList);
-    void processedExpressions(QStringList& expressionList);
+    QList<DataPoint> dataPoints() const;
+    QStringList processedExpressions() const;
 
 private:
-    void parseExpressions(QStringList& expressions);
+    void parseExpressions(const QStringList& expressions);
 
     QString processExpression(QString const& expr);
-    bool processRegisterExpression(QString regExpr, DataPoint& dataPoint);
+    bool processRegisterExpression(const QString& regExpr, DataPoint& dataPoint);
     QString constructInternalRegisterFunction(DataPoint const& dataPoint, int size);
 
     QStringList _processedExpressions;
     QList<DataPoint> _dataPoints;
 
-    QRegularExpression _findRegRegex;
+    const QRegularExpression _findRegRegex;
 
     static const QString _cRegisterFunctionTemplate;
 };

src/datahandling/graphdatahandler.cpp

@@ -14,7 +14,6 @@
 void GraphDataHandler::setupExpressions(GraphDataModel* pGraphDataModel, QList<DataPoint>& registerList)
 {
     QStringList exprList;
-    QList<DataPoint> regList;
 
     pGraphDataModel->activeGraphIndexList(&_activeIndexList);
     for (quint16 graphIdx : std::as_const(_activeIndexList))
@@ -23,12 +22,11 @@ void GraphDataHandler::setupExpressions(GraphDataModel* pGraphDataModel, QList<D
     }
 
     ExpressionParser exprParser(exprList);
-    exprParser.dataPoints(regList);
+    const QList<DataPoint> regList = exprParser.dataPoints();
 
     qCInfo(scopeComm) << "Active registers: " << DataPoint::dumpListToString(regList);
 
-    QStringList processedExpList;
-    exprParser.processedExpressions(processedExpList);
+    const QStringList processedExpList = exprParser.processedExpressions();
 
     _valueParsers.clear();
 

tests/datahandling/tst_expressionparser.cpp

@@ -134,6 +134,15 @@ void TestExpressionParser::multiRegistersDuplicate()
     verifyParsing(input, expDataPoints, expExpressions);
 }
 
+void TestExpressionParser::singleExpressionDuplicate()
+{
+    auto input = QStringList() << "${45332} + ${45332}";
+    auto expExpressions = QStringList() << "r(0    ) + r(0    )";
+    auto expDataPoints = QList<DataPoint>() << DataPoint("${45332}", Device::cFirstDeviceId);
+
+    verifyParsing(input, expDataPoints, expExpressions);
+}
+
 void TestExpressionParser::failure()
 {
     auto input = QStringList() << "${}";
@@ -236,16 +245,34 @@ void TestExpressionParser::manyRegisters()
     verifyParsing(input, expDataPoints, expExpressions);
 }
 
-void TestExpressionParser::verifyParsing(QStringList exprList,
-                                         QList<DataPoint>& expectedDataPoints,
-                                         QStringList& expectedExpression)
+void TestExpressionParser::manyRegistersHighIndex()
 {
-    QList<DataPoint> actualDataPoints;
-    QStringList actualExpressionList;
+    /* Verify qMax(0,...) guard: ${0} is 4 chars, but assigned index 10 (after 10 prior registers).
+     * Unguarded: spacesCount = 4-3-2 = -1. Guarded: clamped to 0, producing r(10) instead of garbage. */
+    auto input = QStringList() << "${1}" << "${2}" << "${3}" << "${4}" << "${5}"
+                               << "${6}" << "${7}" << "${8}" << "${9}" << "${10}" << "${0}";
+    auto expDataPoints = QList<DataPoint>()
+                         << DataPoint("${1}", Device::cFirstDeviceId) << DataPoint("${2}", Device::cFirstDeviceId)
+                         << DataPoint("${3}", Device::cFirstDeviceId) << DataPoint("${4}", Device::cFirstDeviceId)
+                         << DataPoint("${5}", Device::cFirstDeviceId) << DataPoint("${6}", Device::cFirstDeviceId)
+                         << DataPoint("${7}", Device::cFirstDeviceId) << DataPoint("${8}", Device::cFirstDeviceId)
+                         << DataPoint("${9}", Device::cFirstDeviceId) << DataPoint("${10}", Device::cFirstDeviceId)
+                         << DataPoint("${0}", Device::cFirstDeviceId);
+    /* ${10} at idx=9: size=5, spacesCount=5-3-1=1 → r(9 )
+     * ${0}  at idx=10: size=4, spacesCount=qMax(0,4-3-2)=0 → r(10) */
+    auto expExpressions = QStringList() << "r(0)" << "r(1)" << "r(2)" << "r(3)" << "r(4)"
+                                        << "r(5)" << "r(6)" << "r(7)" << "r(8)" << "r(9 )" << "r(10)";
 
+    verifyParsing(input, expDataPoints, expExpressions);
+}
+
+void TestExpressionParser::verifyParsing(const QStringList& exprList,
+                                         const QList<DataPoint>& expectedDataPoints,
+                                         const QStringList& expectedExpression)
+{
     ExpressionParser parser(exprList);
-    parser.dataPoints(actualDataPoints);
-    parser.processedExpressions(actualExpressionList);
+    const QList<DataPoint> actualDataPoints = parser.dataPoints();
+    const QStringList actualExpressionList = parser.processedExpressions();
 
     QVERIFY(actualDataPoints == expectedDataPoints);
     QVERIFY(actualExpressionList == expectedExpression);

tests/datahandling/tst_expressionparser.h

@@ -24,6 +24,7 @@ private slots:
     void singleRegisterConnType();
     void multiRegisters();
     void multiRegistersDuplicate();
+    void singleExpressionDuplicate();
     void failure();
     void failureMulti();
     void combinations();
@@ -34,7 +35,11 @@ private slots:
     void constant();
     void manyRegisters();
 
-    void verifyParsing(QStringList exprList, QList<DataPoint>& expectedDataPoints, QStringList& expectedExpression);
+    void manyRegistersHighIndex();
+
+    void verifyParsing(const QStringList& exprList,
+                       const QList<DataPoint>& expectedDataPoints,
+                       const QStringList& expectedExpression);
 
 private:
 };